Convert host <-> guest communication to use MessageChannel

src/annotator/cross-frame.js

@@ -1,6 +1,5 @@
 import AnnotationSync from './annotation-sync';
 import Bridge from '../shared/bridge';
-import Discovery from '../shared/discovery';
 import * as frameUtil from './util/frame-util';
 import FrameObserver from './frame-observer';
 
@@ -25,13 +24,11 @@ export class CrossFrame {
    * @param {Element} element
    * @param {object} options
    *   @param {Record<string, any>} options.config,
-   *   @param {boolean} [options.server]
    *   @param {(event: string, ...args: any[]) => void} options.on
    *   @param {(event: string, ...args: any[]) => void } options.emit
    */
   constructor(element, options) {
-    const { config, server, on, emit } = options;
-    const discovery = new Discovery(window, { server });
+    const { config, on, emit } = options;
     const bridge = new Bridge();
     const annotationSync = new AnnotationSync(bridge, { on, emit });
     const frameObserver = new FrameObserver(element);
@@ -46,7 +43,8 @@ export class CrossFrame {
       }
 
       frameUtil.isLoaded(frame, () => {
-        const subFrameIdentifier = discovery.generateToken();
+        // Generate a random string to use as a frame ID. The format is not important.
+        const subFrameIdentifier = Math.random().toString().replace(/\D/g, '');
         frameIdentifiers.set(frame, subFrameIdentifier);
         const injectedConfig = {
           ...config,
@@ -71,26 +69,25 @@ export class CrossFrame {
      * Returns a promise that resolves once the connection has been established.
      *
      * @param {Window} frame - The window containing the sidebar application
-     * @return {Promise<void>}
+     * @param {string} origin - Origin of the sidebar application (eg. 'https://hypothes.is/')
      */
-    this.connectToSidebar = frame => {
-      return new Promise(resolve => {
-        discovery.startDiscovery(
-          (source, origin, token) => {
-            bridge.createChannel({ source, origin, token });
-            resolve();
-          },
-          [frame]
-        );
-      });
+    this.connectToSidebar = (frame, origin) => {
+      const channel = new MessageChannel();
+      frame.postMessage(
+        {
+          type: 'hypothesisGuestReady',
+        },
+        origin,
+        [channel.port2]
+      );
+      bridge.createChannel(channel.port1);
     };
 
     /**
      * Remove the connection between the sidebar and annotator.
      */
     this.destroy = () => {
       bridge.destroy();
-      discovery.stopDiscovery();
       frameObserver.disconnect();
     };
 

src/annotator/index.js

@@ -27,7 +27,7 @@ const window_ = /** @type {HypothesisWindow} */ (window);
 
 // Look up the URL of the sidebar. This element is added to the page by the
 // boot script before the "annotator" bundle loads.
-const appLinkEl = /** @type {Element} */ (
+const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
   document.querySelector(
     'link[type="application/annotator+html"][rel="sidebar"]'
   )
@@ -68,28 +68,39 @@ function init() {
     sidebar = new Sidebar(document.body, eventBus, guest, getConfig('sidebar'));
 
     // Expose sidebar window reference for use by same-origin guest frames.
-    window_.__hypothesis.sidebarWindow = sidebar.iframe.contentWindow;
+    window_.__hypothesis.sidebarWindow = sidebar.ready.then(
+      () => sidebar.iframe.contentWindow
+    );
   }
 
   // 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'));
 
   // Set up communication between this host/guest frame and the sidebar frame.
-  const sidebarWindow = sidebar
-    ? sidebar.iframe.contentWindow
-    : /** @type {HypothesisWindow} */ (window.parent).__hypothesis
-        ?.sidebarWindow;
+  let sidebarWindow = window_.__hypothesis.sidebarWindow;
+  try {
+    sidebarWindow = /** @type {HypothesisWindow} */ (window.parent).__hypothesis
+      ?.sidebarWindow;
+  } catch {
+    // `window.parent` access can fail due to it being cross-origin. This is
+    // handled below.
+  }
+
   if (sidebarWindow) {
-    guest.crossframe.connectToSidebar(sidebarWindow);
+    const sidebarOrigin = new URL(sidebarLinkElement.href).origin;
+    sidebarWindow.then(frame =>
+      guest.crossframe.connectToSidebar(frame, sidebarOrigin)
+    );
   } else {
     // eslint-disable-next-line no-console
     console.warn(
-      `Hypothesis guest frame in ${location.origin} could not find a sidebar to connect to`
+      `Hypothesis guest frame in ${location.origin} could not find a sidebar to connect to.
+Guest frames can only connect to sidebars in their same-origin parent frame.`
     );
   }
 
-  appLinkEl.addEventListener('destroy', () => {
+  sidebarLinkElement.addEventListener('destroy', () => {
     delete window_.__hypothesis;
     sidebar?.destroy();
 

src/annotator/sidebar.js

@@ -195,6 +195,21 @@ export default class Sidebar {
     // Initial layout notification
     this._notifyOfLayoutChange(false);
     this._setupSidebarEvents();
+
+    /**
+     * A promise that resolves when the sidebar application is ready to
+     * communicate with the host and guest frames.
+     *
+     * @type {Promise<void>}
+     */
+    this.ready = new Promise(resolve => {
+      this._listeners.add(window, 'message', event => {
+        const data = /** @type {MessageEvent} */ (event).data;
+        if (data?.type === 'hypothesisSidebarReady') {
+          resolve();
+        }
+      });
+    });
   }
 
   destroy() {

src/annotator/test/cross-frame-test.js

@@ -1,11 +1,9 @@
 import { CrossFrame, $imports } from '../cross-frame';
 
 describe('CrossFrame', () => {
-  let fakeDiscovery;
   let fakeBridge;
   let fakeAnnotationSync;
 
-  let proxyDiscovery;
   let proxyBridge;
   let proxyAnnotationSync;
 
@@ -20,11 +18,6 @@ describe('CrossFrame', () => {
   };
 
   beforeEach(() => {
-    fakeDiscovery = {
-      startDiscovery: sinon.stub(),
-      stopDiscovery: sinon.stub(),
-    };
-
     fakeBridge = {
       destroy: sinon.stub(),
       createChannel: sinon.stub(),
@@ -36,13 +29,11 @@ describe('CrossFrame', () => {
     fakeAnnotationSync = { sync: sinon.stub() };
 
     proxyAnnotationSync = sinon.stub().returns(fakeAnnotationSync);
-    proxyDiscovery = sinon.stub().returns(fakeDiscovery);
     proxyBridge = sinon.stub().returns(fakeBridge);
 
     $imports.$mock({
       './annotation-sync': proxyAnnotationSync,
       '../shared/bridge': proxyBridge,
-      '../shared/discovery': proxyDiscovery,
     });
   });
 
@@ -51,21 +42,6 @@ describe('CrossFrame', () => {
   });
 
   describe('CrossFrame constructor', () => {
-    it('instantiates the Discovery component', () => {
-      createCrossFrame();
-      assert.calledWith(proxyDiscovery, window);
-    });
-
-    it('passes the options along to the bridge', () => {
-      createCrossFrame({ server: true });
-      assert.calledWith(proxyDiscovery, window, { server: true });
-    });
-
-    it('instantiates the CrossFrame component', () => {
-      createCrossFrame();
-      assert.calledWith(proxyDiscovery);
-    });
-
     it('instantiates the AnnotationSync component', () => {
       createCrossFrame();
       assert.called(proxyAnnotationSync);
@@ -81,34 +57,37 @@ describe('CrossFrame', () => {
   });
 
   describe('#connectToSidebar', () => {
-    it('starts the discovery of new channels', async () => {
+    it('sends a `hypothesisGuestReady` notification to the sidebar', async () => {
       const cf = createCrossFrame();
-      const sidebarFrame = {};
-      fakeDiscovery.startDiscovery.callsFake((callback, frames) => {
-        setTimeout(() => callback(frames[0], 'ORIGIN', 'TOKEN'), 0);
-      });
+      const sidebarFrame = { postMessage: sinon.stub() };
+      const sidebarOrigin = 'https://dummy.hypothes.is/';
+
+      cf.connectToSidebar(sidebarFrame, sidebarOrigin);
+
+      assert.calledWith(
+        sidebarFrame.postMessage,
+        {
+          type: 'hypothesisGuestReady',
+        },
+        sidebarOrigin,
+        [sinon.match.instanceOf(MessagePort)]
+      );
+    });
 
-      await cf.connectToSidebar(sidebarFrame);
+    it('creates a channel for communication with the sidebar', () => {
+      const cf = createCrossFrame();
+      const sidebarFrame = { postMessage: sinon.stub() };
 
-      assert.calledWith(fakeDiscovery.startDiscovery, sinon.match.func, [
-        sidebarFrame,
-      ]);
-      assert.called(fakeBridge.createChannel);
-      assert.calledWith(fakeBridge.createChannel, {
-        source: sidebarFrame,
-        origin: 'ORIGIN',
-        token: 'TOKEN',
-      });
+      cf.connectToSidebar(sidebarFrame);
+
+      assert.calledWith(
+        fakeBridge.createChannel,
+        sinon.match.instanceOf(MessagePort)
+      );
     });
   });
 
   describe('#destroy', () => {
-    it('stops the discovery of new frames', () => {
-      const cf = createCrossFrame();
-      cf.destroy();
-      assert.called(fakeDiscovery.stopDiscovery);
-    });
-
     it('destroys the bridge object', () => {
       const cf = createCrossFrame();
       cf.destroy();

src/annotator/test/sidebar-test.js

@@ -155,6 +155,27 @@ describe('Sidebar', () => {
     });
   });
 
+  describe('#ready', () => {
+    it('returns a promise that resolves when `hypothesisSidebarReady` message is received', async () => {
+      const sidebar = createSidebar();
+
+      // Check `sidebar.ready` is not resolved before `hypothesisSidebarReady`
+      // message is received.
+      assert.equal(
+        await Promise.race([sidebar.ready, Promise.resolve('not-ready')]),
+        'not-ready'
+      );
+
+      window.dispatchEvent(
+        new MessageEvent('message', {
+          data: { type: 'hypothesisSidebarReady' },
+        })
+      );
+
+      return sidebar.ready;
+    });
+  });
+
   function getConfigString(sidebar) {
     return sidebar.iframe.src;
   }

src/sidebar/services/frame-sync.js

@@ -1,7 +1,6 @@
 import debounce from 'lodash.debounce';
 
 import bridgeEvents from '../../shared/bridge-events';
-import Discovery from '../../shared/discovery';
 import { isReply, isPublic } from '../helpers/annotation-metadata';
 import { watch } from '../util/watch';
 
@@ -46,13 +45,15 @@ export function formatAnnot(ann) {
  */
 export class FrameSyncService {
   /**
+   * @param {Window} $window - Test seam
    * @param {import('./annotations').AnnotationsService} annotationsService
    * @param {import('../../shared/bridge').default} bridge
    * @param {import('../store').SidebarStore} store
    */
-  constructor(annotationsService, bridge, store) {
+  constructor($window, annotationsService, bridge, store) {
     this._bridge = bridge;
     this._store = store;
+    this._window = $window;
 
     // Set of tags of annotations that are currently loaded into the frame
     const inFrame = new Set();
@@ -235,17 +236,30 @@ export class FrameSyncService {
       });
     };
 
-    const discovery = new Discovery(window, { server: true });
-    discovery.startDiscovery(
-      (source, origin, token) =>
-        this._bridge.createChannel({ source, origin, token }),
-      [
-        // Ping the host frame which is in most cases also the only guest.
-        window.parent,
-      ]
-    );
     this._bridge.onConnect(addFrame);
 
+    // Listen for messages from new guest frames that want to connect.
+    //
+    // The message will include a `MessagePort` to use for communication with
+    // the guest. Communication with the host currently relies on the host
+    // frame also always being a guest frame.
+    this._window.addEventListener('message', e => {
+      if (e.data?.type !== 'hypothesisGuestReady') {
+        return;
+      }
+      if (e.ports.length === 0) {
+        console.warn(
+          'Ignoring `hypothesisGuestReady` message without a MessagePort'
+        );
+        return;
+      }
+      const port = e.ports[0];
+      this._bridge.createChannel(port);
+    });
+
+    // Notify host frame that it is ready for guests to connect to it.
+    this._window.parent.postMessage({ type: 'hypothesisSidebarReady' }, '*');
+
     this._setupSyncToFrame();
     this._setupSyncFromFrame();
   }