Explicitly specify guest/sidebar frames to ping in startDiscovery

The logic used by guest/host and sidebar frames to find each other in
`Discovery#startDiscovery` relied on traversing the frame tree starting
from `window.top` using `window.frames`. Since `window.frames` doesn't
include frames in shadow roots, this did not work if _both_ the guest and
sidebar are contained within a shadow root. The sidebar is always
contained in a shadow root created by the `<hypothesis-sidebar>`
element, so this simplifies to only working if the guest/host is not
contained in a shadow root.

For current use cases it is possible for guest frames to get a direct
reference to the sidebar frame they should be communicating with and for
the sidebar to get direct a reference to the host frame (its parent).
This avoids the need for `window.frames` traversal.

This commit leverages that by changing `startDiscovery` to take an
explicit array of frames to ping. Guest frames invoke `startDiscovery`
passing a reference to the sidebar frame and sidebar frames invoke
`startDiscovery` passing a reference to the parent (host) frame.

This fixes the following scenarios:

 - Hypothesis not completing loading if host frame is contained within a
   shadow root. eg. In the VitalSource book reader.

 - Same-origin guest frames loading Hypothesis after the sidebar has
   already loaded. See http://localhost:3000/document/parent-frame test
   case in client dev server.

 - Multiple sidebars attempting to connect to the same guest frame, if
   Hypothesis is loaded multiple times in different parts of the frame
   tree. See http://localhost:3000/document/multi-frames test case in
   client dev server.

This approach has some limitations:

 - Child guest frames which are not same-origin with the parent host frame
   are not supported.
 - Child guest frames which attempt to connect to the sidebar before it
   has loaded will fail to connect. This could be remedied by making the
   guest wait for confirmation of the sidebar having loaded before
   calling `connectToSidebar`. This doesn't affect the host frame since
   the sidebar pings that frame directly.

These limitations are acceptable in the short term but will be remedied
by a larger overhaul of inter-frame communication that is currently
being worked on.

docs/publishers/embedding.rst

@@ -23,7 +23,8 @@ above script tag to the document displayed in the iframe. This will display the
 sidebar in the iframe itself.
 
 Additionally Hypothesis has limited support for enabling annotation of iframed
-content while showing the sidebar in the top-level document. To use this:
+content while showing the sidebar in the top-level document where Hypothesis
+was initially loaded. To use this:
 
 1. Add the above script tag to the top-level document
 
@@ -38,6 +39,8 @@ content while showing the sidebar in the top-level document. To use this:
    ...
    </iframe>
 
-This method *only* works for iframes which are same-origin with the top-level
-document. The client will watch for new iframes being added to the document and
-will automatically enable annotation for them.
+This method *only* works for iframes which are direct children of the top-level
+document and have the same origin.
+
+The client will watch for new iframes being added to the document and will
+automatically enable annotation for them.

src/annotator/cross-frame.js

@@ -63,12 +63,26 @@ export class CrossFrame {
       frameIdentifiers.delete(frame);
     };
 
-    // Initiate connection to the sidebar.
-    const onDiscoveryCallback = (source, origin, token) =>
-      bridge.createChannel(source, origin, token);
-    discovery.startDiscovery(onDiscoveryCallback);
     frameObserver.observe(injectIntoFrame, iframeUnloaded);
 
+    /**
+     * Attempt to connect to the sidebar frame.
+     *
+     * @param {Window} frame
+     * @return {Promise<void>}
+     */
+    this.connectToSidebar = frame => {
+      return new Promise(resolve => {
+        discovery.startDiscovery(
+          (source, origin, token) => {
+            bridge.createChannel(source, origin, token);
+            resolve();
+          },
+          [frame]
+        );
+      });
+    };
+
     /**
      * Remove the connection between the sidebar and annotator.
      */

src/annotator/index.js

@@ -34,32 +34,49 @@ const appLinkEl = /** @type {Element} */ (
 );
 
 function init() {
+  window_.__hypothesis = {};
+
   const annotatorConfig = getConfig('annotator');
   const isPDF = typeof window_.PDFViewerApplication !== 'undefined';
 
-  if (annotatorConfig.subFrameIdentifier) {
-    // Other modules use this to detect if this
-    // frame context belongs to hypothesis.
-    // Needs to be a global property that's set.
-    window_.__hypothesis_frame = true;
-  }
-
   const eventBus = new EventBus();
   const guest = new Guest(document.body, eventBus, {
     ...annotatorConfig,
     // Load the PDF anchoring/metadata integration.
     // nb. documentType is an internal config property only
     documentType: isPDF ? 'pdf' : 'html',
   });
+
   const sidebar = !annotatorConfig.subFrameIdentifier
     ? new Sidebar(document.body, eventBus, guest, getConfig('sidebar'))
     : null;
+  if (sidebar) {
+    // Expose sidebar window reference for use by same-origin guest frames.
+    window_.__hypothesis.sidebarWindow = sidebar.sidebarWindow;
+  }
+
   // Clear `annotations` value from the notebook's config to prevent direct-linked
   // annotations from filtering the threads.
   const notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
 
+  // Setup guest <-> sidebar communication.
+  const sidebarWindow = sidebar
+    ? sidebar.sidebarWindow
+    : /** @type {HypothesisWindow} */ (window.parent).__hypothesis
+        ?.sidebarWindow;
+  if (sidebarWindow) {
+    guest.crossframe.connectToSidebar(sidebarWindow);
+  } else {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `Hypothesis guest frame in ${location.origin} could not find a sidebar to connect to`
+    );
+  }
+
   appLinkEl.addEventListener('destroy', () => {
+    delete window_.__hypothesis;
     sidebar?.destroy();
+
     notebook.destroy();
     guest.destroy();
 

src/annotator/sidebar.js

@@ -192,6 +192,13 @@ export default class Sidebar {
     this._setupSidebarEvents();
   }
 
+  /**
+   * Return a reference to the `Window` containing the sidebar application.
+   */
+  get sidebarWindow() {
+    return /** @type {Window} */ (this.iframe.contentWindow);
+  }
+
   destroy() {
     this.bucketBar?.destroy();
     this._listeners.removeAll();

src/annotator/test/integration/multi-frame-test.js

@@ -137,7 +137,7 @@ describe('CrossFrame multi-frame scenario', () => {
     const frame = document.createElement('iframe');
     frame.setAttribute('enable-annotation', '');
     container.appendChild(frame);
-    frame.contentWindow.eval('window.__hypothesis_frame = true');
+    frame.contentWindow.eval('window.__hypothesis = {}');
 
     crossFrame = createCrossFrame();
 

src/annotator/util/frame-util.js

@@ -17,7 +17,7 @@ export function findFrames(container) {
 
 // Check if the iframe has already been injected
 export function hasHypothesis(iframe) {
-  return iframe.contentWindow.__hypothesis_frame === true;
+  return '__hypothesis' in iframe.contentWindow;
 }
 
 // Inject embed.js into the iframe

src/shared/discovery.js

@@ -72,8 +72,9 @@ export default class Discovery {
    *
    * @param {DiscoveryCallback} onDiscovery - Callback to invoke with a token when
    *   another frame is discovered.
+   * @param {Window[]} frames - A list of frames to attempt to connect to.
    */
-  startDiscovery(onDiscovery) {
+  startDiscovery(onDiscovery, frames) {
     if (this.onDiscovery) {
       throw new Error(
         'Discovery is already in progress. Call stopDiscovery() first'
@@ -84,7 +85,14 @@ export default class Discovery {
 
     // Listen for messages from other frames.
     this._listeners.add(this.target, 'message', this._onMessage);
-    this._beacon();
+
+    // Ping specified other frames to tell them about the existence of this frame.
+    const beaconMessage = this.server
+      ? '__cross_frame_dhcp_offer'
+      : '__cross_frame_dhcp_discovery';
+    for (let frame of frames) {
+      frame.postMessage(beaconMessage, this.origin);
+    }
   }
 
   /**
@@ -95,33 +103,6 @@ export default class Discovery {
     this._listeners.removeAll();
   }
 
-  /**
-   * Send a message to other frames in the current window to inform them about
-   * the existence of this frame and tell them whether this frame is a client
-   * or server.
-   */
-  _beacon() {
-    let beaconMessage;
-    if (this.server) {
-      beaconMessage = '__cross_frame_dhcp_offer';
-    } else {
-      beaconMessage = '__cross_frame_dhcp_discovery';
-    }
-
-    // Perform a top-down, breadth-first traversal of frames in the current
-    // window and send messages to them.
-    const queue = [this.target.top];
-    while (queue.length > 0) {
-      const parent = /** @type {Window} */ (queue.shift());
-      if (parent !== this.target) {
-        parent.postMessage(beaconMessage, this.origin);
-      }
-      for (let i = 0; i < parent.frames.length; i++) {
-        queue.push(parent.frames[i]);
-      }
-    }
-  }
-
   /**
    * Handle a `MessageEvent` from another frame which _may_ be from a
    * `Discovery` instance.

src/sidebar/services/frame-sync.js

@@ -236,7 +236,10 @@ export class FrameSyncService {
     };
 
     const discovery = new Discovery(window, { server: true });
-    discovery.startDiscovery(this._bridge.createChannel.bind(this._bridge));
+    discovery.startDiscovery(this._bridge.createChannel.bind(this._bridge), [
+      // Ping the host frame which is in most cases also the only guest.
+      window.parent,
+    ]);
     this._bridge.onConnect(addFrame);
 
     this._setupSyncToFrame();

src/types/annotator.js

@@ -142,9 +142,8 @@
  * @typedef Globals
  * @prop {import('./pdfjs').PDFViewerApplication} [PDFViewerApplication] -
  *   PDF.js entry point. If set, triggers loading of PDF rather than HTML integration.
- * @prop {boolean} [__hypothesis_frame] -
- *   Flag used to indicate that the "annotator" part of Hypothesis is loaded in
- *   the current frame.
+ * @prop {object} [__hypothesis] - Internal data related to supporting guests in iframes
+ *   @prop {Window} [sidebarWindow] - The sidebar window that is active in this frame.
  */
 
 /**