Adopt new `PortProvider` and `PortFinder`

This adoption simplifies the discovery of frames.

I did a little bit of clean-up in the tests: rearranged a few variables
alphabetically, and delete unneeded lines and variables.

src/annotator/guest.js

 import { Bridge } from '../shared/bridge';
+import { PortFinder } from '../shared/port-finder';
 import { ListenerCollection } from '../shared/listener-collection';
 
 import { Adder } from './adder';
@@ -117,11 +118,8 @@ export default class Guest {
    * @param {EventBus} eventBus -
    *   Enables communication between components sharing the same eventBus
    * @param {Record<string, any>} [config]
-   * @param {Window} [hostFrame] -
-   *   Host frame which this guest is associated with. This is expected to be
-   *   an ancestor of the guest frame. It may be same or cross origin.
    */
-  constructor(element, eventBus, config = {}, hostFrame = window) {
+  constructor(element, eventBus, config = {}) {
     this.element = element;
     this._emitter = eventBus.createEmitter();
     this._highlightsVisible = false;
@@ -162,10 +160,17 @@ export default class Guest {
 
     // Set the frame identifier if it's available.
     // The "top" guest instance will have this as null since it's in a top frame not a sub frame
-    this._frameIdentifier = config.subFrameIdentifier || null;
+    /** @type {string|null} */
+    this._frameIdentifier = config.subFrameIdentifier;
+    this._hostFrame =
+      config.subFrameIdentifier === null ? window : window.parent;
+    this._portFinder = new PortFinder({
+      hostFrame: this._hostFrame,
+      source: 'guest',
+    });
 
     /**
-     * Channel for sidebar-guest communication.
+     * Channel for guest-sidebar communication.
      *
      * @type {Bridge<GuestToSidebarEvent,SidebarToGuestEvent>}
      */
@@ -177,6 +182,10 @@ export default class Guest {
     this._annotationSync = new AnnotationSync(eventBus, this._bridge);
     this._connectAnnotationSync();
 
+    // Discover and connect to the sidebar frame. All RPC events must be
+    // registered before creating the channel.
+    this._connectToSidebar();
+
     // Set up automatic and integration-triggered injection of client into
     // iframes in this frame.
     this._hypothesisInjector = new HypothesisInjector(this.element, config);
@@ -202,7 +211,6 @@ export default class Guest {
      */
     this._focusedAnnotations = new Set();
 
-    this._hostFrame = hostFrame;
     this._listeners.add(window, 'unload', () => this._notifyGuestUnload());
   }
 
@@ -361,7 +369,16 @@ export default class Guest {
     });
   }
 
+  /**
+   * Attempt to connect to the sidebar frame.
+   */
+  async _connectToSidebar() {
+    const sidebarPort = await this._portFinder.discover('sidebar');
+    this._bridge.createChannel(sidebarPort);
+  }
+
   destroy() {
+    this._portFinder.destroy();
     this._notifyGuestUnload();
     this._hypothesisInjector.destroy();
     this._listeners.removeAll();
@@ -387,25 +404,8 @@ export default class Guest {
       type: 'hypothesisGuestUnloaded',
       frameIdentifier: this._frameIdentifier,
     };
-    this._hostFrame.postMessage(message, '*');
-  }
 
-  /**
-   * Attempt to connect to the sidebar frame.
-   *
-   * @param {Window} frame - The window containing the sidebar application
-   * @param {string} origin - Origin of the sidebar application (eg. 'https://hypothes.is/')
-   */
-  connectToSidebar(frame, origin) {
-    const channel = new MessageChannel();
-    frame.postMessage(
-      {
-        type: 'hypothesisGuestReady',
-      },
-      origin,
-      [channel.port2]
-    );
-    this._bridge.createChannel(channel.port1);
+    this._hostFrame.postMessage(message, '*');
   }
 
   /**

src/annotator/index.js

@@ -19,8 +19,6 @@ import Notebook from './notebook';
 import Sidebar from './sidebar';
 import { EventBus } from './util/emitter';
 
-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 sidebarLinkElement = /** @type {HTMLLinkElement} */ (
@@ -42,63 +40,26 @@ const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
  * client is initially loaded, is also the only guest frame.
  */
 function init() {
-  // Create an internal global used to share data between same-origin guest and
-  // host frames.
-  window_.__hypothesis = {};
-
   const annotatorConfig = getConfig('annotator');
-  const hostFrame = annotatorConfig.subFrameIdentifier ? window.parent : window;
+  const isHostFrame = annotatorConfig.subFrameIdentifier === null;
 
   // Create the guest that handles creating annotations and displaying highlights.
   const eventBus = new EventBus();
-  const guest = new Guest(document.body, eventBus, annotatorConfig, hostFrame);
+  const guest = new Guest(document.body, eventBus, annotatorConfig);
 
   let sidebar;
-  if (window === hostFrame) {
+  let notebook;
+  if (isHostFrame) {
     sidebar = new Sidebar(document.body, eventBus, guest, getConfig('sidebar'));
 
-    // Expose sidebar window reference for use by same-origin guest frames.
-    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.
-  let sidebarWindow = window_.__hypothesis.sidebarWindow;
-  try {
-    // If this is a guest-only frame which doesn't have its own sidebar, try
-    // to connect to the one created by the parent frame. This only works if
-    // the host and guest frames are same-origin.
-    if (!sidebarWindow) {
-      sidebarWindow = /** @type {HypothesisWindow} */ (window.parent)
-        .__hypothesis?.sidebarWindow;
-    }
-  } catch {
-    // `window.parent` access can fail due to it being cross-origin.
-  }
-
-  if (sidebarWindow) {
-    const sidebarOrigin = new URL(sidebarLinkElement.href).origin;
-    sidebarWindow.then(([frame]) =>
-      guest.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.
-Guest frames can only connect to sidebars in their same-origin parent frame.`
-    );
+    notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
   }
 
   sidebarLinkElement.addEventListener('destroy', () => {
-    delete window_.__hypothesis;
     sidebar?.destroy();
-
-    notebook.destroy();
+    notebook?.destroy();
     guest.destroy();
 
     // Remove all the `<link>`, `<script>` and `<style>` elements added to the

src/annotator/sidebar.js

@@ -2,6 +2,7 @@ import Hammer from 'hammerjs';
 
 import { Bridge } from '../shared/bridge';
 import { ListenerCollection } from '../shared/listener-collection';
+import { PortProvider } from '../shared/port-provider';
 
 import { annotationCounts } from './annotation-counts';
 import BucketBar from './bucket-bar';
@@ -64,9 +65,11 @@ export default class Sidebar {
    */
   constructor(element, eventBus, guest, config = {}) {
     this._emitter = eventBus.createEmitter();
+    const hypothesisAppsOrigin = new URL(config.sidebarAppUrl).origin;
+    this._portProvider = new PortProvider(hypothesisAppsOrigin);
 
     /**
-     * Channel for sidebar-host communication.
+     * Channel for host-sidebar communication.
      *
      * @type {Bridge<HostToSidebarEvent,SidebarToHostEvent>}
      */
@@ -172,23 +175,7 @@ export default class Sidebar {
     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, ports } = /** @type {MessageEvent} */ (event);
-        if (data?.type === 'hypothesisSidebarReady') {
-          this._sidebarRPC.createChannel(ports[0]);
-          resolve();
-        }
-      });
-    });
-
-    this.ready.then(() => {
+    this._sidebarRPC.onConnect(() => {
       // Show the UI
       if (this.iframeContainer) {
         this.iframeContainer.style.display = '';
@@ -210,6 +197,10 @@ export default class Sidebar {
       }
     });
 
+    // Create channel *after* all bridge events are registered with the `on` method.
+    this._sidebarRPC.createChannel(this._portProvider.sidebarPort);
+    this._portProvider.listen();
+
     // Notify sidebar when a guest is unloaded. This message is routed via
     // the host frame because in Safari guest frames are unable to send messages
     // directly to the sidebar during a window's 'unload' event.
@@ -223,6 +214,7 @@ export default class Sidebar {
   }
 
   destroy() {
+    this._portProvider.destroy();
     this.bucketBar?.destroy();
     this._listeners.removeAll();
     this._hammerManager?.destroy();

src/annotator/test/guest-test.js

+import { delay } from '../../test-util/wait';
 import Guest, { $imports } from '../guest';
 import { EventBus } from '../util/emitter';
 
@@ -30,36 +31,29 @@ class FakeTextRange {
 describe('Guest', () => {
   const sandbox = sinon.createSandbox();
   let eventBus;
+  let guests;
   let highlighter;
   let rangeUtil;
   let notifySelectionChanged;
 
-  let hostFrame;
-
   let FakeAnnotationSync;
   let fakeAnnotationSync;
-  let FakeBridge;
   let fakeBridge;
-
-  let fakeCreateIntegration;
   let fakeIntegration;
-
   let FakeHypothesisInjector;
   let fakeHypothesisInjector;
-
-  let guests;
+  let fakePortFinder;
 
   const createGuest = (config = {}) => {
     const element = document.createElement('div');
     eventBus = new EventBus();
-    const guest = new Guest(element, eventBus, config, hostFrame);
+    const guest = new Guest(element, eventBus, config);
     guests.push(guest);
     return guest;
   };
 
   beforeEach(() => {
     guests = [];
-    FakeAdder.instance = null;
     highlighter = {
       getHighlightsContainingNode: sinon.stub().returns([]),
       highlightRange: sinon.stub().returns([]),
@@ -75,6 +69,8 @@ describe('Guest', () => {
     };
     notifySelectionChanged = null;
 
+    FakeAdder.instance = null;
+
     fakeAnnotationSync = {
       destroy: sinon.stub(),
       sync: sinon.stub(),
@@ -87,7 +83,6 @@ describe('Guest', () => {
       destroy: sinon.stub(),
       on: sinon.stub(),
     };
-    FakeBridge = sinon.stub().returns(fakeBridge);
 
     fakeIntegration = {
       anchor: sinon.stub(),
@@ -104,18 +99,17 @@ describe('Guest', () => {
       uri: sinon.stub().resolves('https://example.com/test.pdf'),
     };
 
-    fakeCreateIntegration = sinon.stub().returns(fakeIntegration);
-
-    hostFrame = {
-      postMessage: sinon.stub(),
-    };
-
     fakeHypothesisInjector = {
       destroy: sinon.stub(),
       injectClient: sinon.stub().resolves(),
     };
     FakeHypothesisInjector = sinon.stub().returns(fakeHypothesisInjector);
 
+    fakePortFinder = {
+      discover: sinon.stub(),
+      destroy: sinon.stub(),
+    };
+
     class FakeSelectionObserver {
       constructor(callback) {
         notifySelectionChanged = callback;
@@ -123,14 +117,21 @@ describe('Guest', () => {
       }
     }
 
+    sandbox.stub(window.parent, 'postMessage');
+
     $imports.$mock({
-      '../shared/bridge': { Bridge: FakeBridge },
+      '../shared/bridge': { Bridge: sinon.stub().returns(fakeBridge) },
+      '../shared/port-finder': {
+        PortFinder: sinon.stub().returns(fakePortFinder),
+      },
       './adder': { Adder: FakeAdder },
       './anchoring/text-range': {
         TextRange: FakeTextRange,
       },
       './annotation-sync': { AnnotationSync: FakeAnnotationSync },
-      './integrations': { createIntegration: fakeCreateIntegration },
+      './integrations': {
+        createIntegration: sinon.stub().returns(fakeIntegration),
+      },
       './highlighter': highlighter,
       './hypothesis-injector': { HypothesisInjector: FakeHypothesisInjector },
       './range-util': rangeUtil,
@@ -1135,7 +1136,7 @@ describe('Guest', () => {
       guest.destroy();
 
       assert.calledWith(
-        hostFrame.postMessage,
+        window.parent.postMessage,
         {
           type: 'hypothesisGuestUnloaded',
           frameIdentifier: 'frame-id',
@@ -1151,7 +1152,7 @@ describe('Guest', () => {
     window.dispatchEvent(new Event('unload'));
 
     assert.calledWith(
-      hostFrame.postMessage,
+      window.parent.postMessage,
       {
         type: 'hypothesisGuestUnloaded',
         frameIdentifier: 'frame-id',
@@ -1166,6 +1167,19 @@ describe('Guest', () => {
     assert.calledWith(fakeHypothesisInjector.destroy);
   });
 
+  it('discovers and creates a channel for communication with the sidebar', async () => {
+    const { port1 } = new MessageChannel();
+    fakePortFinder.discover.resolves(port1);
+    createGuest();
+
+    await delay(0);
+
+    assert.calledWith(
+      fakeBridge.createChannel,
+      sinon.match.instanceOf(MessagePort)
+    );
+  });
+
   describe('#contentContainer', () => {
     it('returns document content container', () => {
       const guest = createGuest();
@@ -1213,35 +1227,4 @@ describe('Guest', () => {
       assert.calledWith(fakeHypothesisInjector.injectClient, frame);
     });
   });
-
-  describe('#connectToSidebar', () => {
-    it('sends a `hypothesisGuestReady` notification to the sidebar', async () => {
-      const guest = createGuest();
-      const sidebarFrame = { postMessage: sinon.stub() };
-      const sidebarOrigin = 'https://dummy.hypothes.is/';
-
-      guest.connectToSidebar(sidebarFrame, sidebarOrigin);
-
-      assert.calledWith(
-        sidebarFrame.postMessage,
-        {
-          type: 'hypothesisGuestReady',
-        },
-        sidebarOrigin,
-        [sinon.match.instanceOf(MessagePort)]
-      );
-    });
-
-    it('creates a channel for communication with the sidebar', () => {
-      const guest = createGuest();
-      const sidebarFrame = { postMessage: sinon.stub() };
-
-      guest.connectToSidebar(sidebarFrame, 'https://dummy.hypothes.is');
-
-      assert.calledWith(
-        fakeBridge.createChannel,
-        sinon.match.instanceOf(MessagePort)
-      );
-    });
-  });
 });

src/annotator/test/integration/anchoring-test.js

 // Tests that the expected parts of the page are highlighted when annotations
 // with various combinations of selector are anchored.
 
-import Guest from '../../guest';
+import Guest, { $imports } from '../../guest';
 import { EventBus } from '../../util/emitter';
 import testPageHTML from './test-page.html';
 
@@ -50,6 +50,15 @@ describe('anchoring', () => {
     container.innerHTML = testPageHTML;
     document.body.appendChild(container);
     const eventBus = new EventBus();
+    const fakePortFinder = {
+      discover: sinon.stub().resolves(new MessageChannel().port1),
+      destroy: sinon.stub(),
+    };
+    $imports.$mock({
+      '../shared/port-finder': {
+        PortFinder: sinon.stub().returns(fakePortFinder),
+      },
+    });
     guest = new Guest(container, eventBus);
   });
 
@@ -57,6 +66,7 @@ describe('anchoring', () => {
     guest.destroy();
     container.remove();
     console.warn.restore();
+    $imports.$restore();
   });
 
   [

src/annotator/test/sidebar-test.js

@@ -5,25 +5,6 @@ const DEFAULT_WIDTH = 350;
 const DEFAULT_HEIGHT = 600;
 const EXTERNAL_CONTAINER_SELECTOR = 'test-external-container';
 
-/**
- * Simulate the sidebar application notifying the host frame that it has loaded
- * and is ready to communicate with the host and guest frames.
- */
-function sendSidebarReadyMessage() {
-  const channel = new MessageChannel();
-  const event = new MessageEvent(
-    'message',
-    {
-      data: { type: 'hypothesisSidebarReady' },
-    },
-    [channel.port1]
-  );
-
-  window.dispatchEvent(event);
-
-  return event;
-}
-
 describe('Sidebar', () => {
   const sandbox = sinon.createSandbox();
   let fakeGuest;
@@ -32,14 +13,13 @@ describe('Sidebar', () => {
   let containers;
   let sidebars;
 
+  let fakeBridge;
   let FakeBucketBar;
   let fakeBucketBar;
-
+  let fakePortProvider;
   let FakeToolbarController;
   let fakeToolbar;
 
-  let fakeBridge;
-
   before(() => {
     sinon.stub(window, 'requestAnimationFrame').yields();
   });
@@ -48,14 +28,22 @@ describe('Sidebar', () => {
     window.requestAnimationFrame.restore();
   });
 
+  /**
+   * Simulate the sidebar application connecting with the host frame. This happens
+   * when the sidebar has loaded and is ready.
+   */
+  const connectSidebarApp = () => {
+    const callback = fakeBridge.onConnect.getCall(0).args[0];
+
+    callback();
+  };
+
   const createSidebar = (config = {}) => {
-    config = Object.assign(
-      {
+    config = {
       // Dummy sidebar app.
-        sidebarAppUrl: '/base/annotator/test/empty.html',
-      },
-      config
-    );
+      sidebarAppUrl: 'https://hyp.is/base/annotator/test/empty.html',
+      ...config,
+    };
     const container = document.createElement('div');
     document.body.appendChild(container);
     containers.push(container);
@@ -83,6 +71,19 @@ describe('Sidebar', () => {
     sidebars = [];
     containers = [];
 
+    fakeBridge = {
+      call: sinon.stub(),
+      createChannel: sinon.stub(),
+      on: sinon.stub(),
+      onConnect: sinon.stub(),
+    };
+
+    fakeBucketBar = {
+      destroy: sinon.stub(),
+      update: sinon.stub(),
+    };
+    FakeBucketBar = sinon.stub().returns(fakeBucketBar);
+
     class FakeGuest {
       constructor() {
         this.element = document.createElement('div');
@@ -94,6 +95,12 @@ describe('Sidebar', () => {
     }
     fakeGuest = new FakeGuest();
 
+    fakePortProvider = {
+      listen: sinon.stub(),
+      sidebarPort: sinon.stub(),
+      destroy: sinon.stub(),
+    };
+
     fakeToolbar = {
       getWidth: sinon.stub().returns(100),
       useMinimalControls: false,
@@ -104,26 +111,15 @@ describe('Sidebar', () => {
     };
     FakeToolbarController = sinon.stub().returns(fakeToolbar);
 
-    fakeBucketBar = {
-      destroy: sinon.stub(),
-      update: sinon.stub(),
-    };
-    FakeBucketBar = sandbox.stub().returns(fakeBucketBar);
-
-    sidebars = [];
-
-    fakeBridge = {
-      call: sinon.stub(),
-      createChannel: sinon.stub(),
-      on: sinon.stub(),
-    };
-
     $imports.$mock({
       '../shared/bridge': { Bridge: sinon.stub().returns(fakeBridge) },
+      '../shared/port-provider': {
+        PortProvider: sinon.stub().returns(fakePortProvider),
+      },
+      './bucket-bar': { default: FakeBucketBar },
       './toolbar': {
         ToolbarController: FakeToolbarController,
       },
-      './bucket-bar': { default: FakeBucketBar },
     });
   });
 
@@ -159,7 +155,7 @@ describe('Sidebar', () => {
 
     it('becomes visible when the sidebar application has loaded', async () => {
       const sidebar = createSidebar();
-      sendSidebarReadyMessage();
+      connectSidebarApp();
       await sidebar.ready;
       assert.equal(sidebar.iframeContainer.style.display, '');
     });
@@ -175,29 +171,6 @@ describe('Sidebar', () => {
     });
   });
 
-  it('creates Bridge for host <-> sidebar RPC calls when `hypothesisSidebarReady` message is received', () => {
-    createSidebar();
-    const event = sendSidebarReadyMessage();
-    assert.calledWith(fakeBridge.createChannel, event.ports[0]);
-  });
-
-  describe('#ready', () => {
-    it('returns a promise that resolves when `hypothesisSidebarReady` message is received', async () => {
-      const sidebar = createSidebar();
-
-      // Check `sidebar.ready` is not already resolved, by racing it against
-      // an immediately resolved promise.
-      assert.equal(
-        await Promise.race([sidebar.ready, Promise.resolve('not-ready')]),
-        'not-ready'
-      );
-
-      sendSidebarReadyMessage();
-
-      return sidebar.ready;
-    });
-  });
-
   it('notifies sidebar app when a guest frame is unloaded', () => {
     createSidebar();
 
@@ -218,14 +191,11 @@ describe('Sidebar', () => {
   }
 
   it('creates sidebar iframe and passes configuration to it', () => {
-    const appURL = new URL(
-      '/base/annotator/test/empty.html',
-      window.location.href
-    );
+    const appURL = 'https://hyp.is/base/annotator/test/empty.html';
     const sidebar = createSidebar({ annotations: '1234' });
     assert.equal(
       getConfigString(sidebar),
-      appURL + configFragment({ annotations: '1234' })
+      `${appURL}${configFragment({ annotations: '1234' })}`
     );
   });
 
@@ -534,7 +504,7 @@ describe('Sidebar', () => {
       it(`opens the sidebar when ${test}`, async () => {
         const sidebar = createSidebar(config);
         const open = sandbox.stub(sidebar, 'open');
-        sendSidebarReadyMessage();
+        connectSidebarApp();
         await sidebar.ready;
         assert.calledOnce(open);
       });
@@ -543,7 +513,7 @@ describe('Sidebar', () => {
     it('does not open the sidebar if not configured to', async () => {
       const sidebar = createSidebar();
       const open = sandbox.stub(sidebar, 'open');
-      sendSidebarReadyMessage();
+      connectSidebarApp();
       await sidebar.ready;
       assert.notCalled(open);
     });
@@ -644,8 +614,7 @@ describe('Sidebar', () => {
       // Configure the sidebar to open on load and wait for the initial open to
       // complete.
       sidebar = createSidebar({ openSidebar: true });
-      sendSidebarReadyMessage();
-      await sidebar.ready;
+      connectSidebarApp();
     });
 
     it('hides the sidebar if window width is < MIN_RESIZE', () => {
@@ -695,7 +664,6 @@ describe('Sidebar', () => {
         layoutChangeHandlerSpy = sandbox.stub();
         sidebar = createSidebar({
           onLayoutChange: layoutChangeHandlerSpy,
-          sidebarAppUrl: '/',
         });
 
         // remove info about call that happens on creation of sidebar
@@ -803,7 +771,6 @@ describe('Sidebar', () => {
         layoutChangeHandlerSpy = sandbox.stub();
         const layoutChangeExternalConfig = {
           onLayoutChange: layoutChangeHandlerSpy,
-          sidebarAppUrl: '/',
           externalContainerSelector: `.${EXTERNAL_CONTAINER_SELECTOR}`,
         };
         sidebar = createSidebar(layoutChangeExternalConfig);

src/sidebar/services/frame-sync.js

 import debounce from 'lodash.debounce';
 
 import { Bridge } from '../../shared/bridge';
+import { ListenerCollection } from '../../shared/listener-collection';
+import { PortFinder } from '../../shared/port-finder';
+import { isMessageEqual } from '../../shared/port-util';
 import { isReply, isPublic } from '../helpers/annotation-metadata';
 import { watch } from '../util/watch';
 
@@ -58,6 +61,14 @@ export class FrameSyncService {
    * @param {import('../store').SidebarStore} store
    */
   constructor($window, annotationsService, store) {
+    this._window = $window;
+    this._store = store;
+    this._portFinder = new PortFinder({
+      hostFrame: this._window.parent,
+      source: 'sidebar',
+    });
+    this._listeners = new ListenerCollection();
+
     /**
      * Channel for sidebar-host communication.
      *
@@ -72,9 +83,6 @@ export class FrameSyncService {
      */
     this._guestRPC = new Bridge();
 
-    this._store = store;
-    this._window = $window;
-
     // Set of tags of annotations that are currently loaded into the frame
     const inFrame = new Set();
 
@@ -226,7 +234,7 @@ export class FrameSyncService {
   /**
    * Connect to the host frame and guest frame(s) in the current browser tab.
    */
-  connect() {
+  async connect() {
     /**
      * Query the guest in a frame for the URL and metadata of the document that
      * is currently loaded and add the result to the set of connected frames.
@@ -253,24 +261,6 @@ export class FrameSyncService {
 
     this._guestRPC.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.
-    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._guestRPC.createChannel(port);
-    });
-
     this._setupSyncToGuests();
     this._setupSyncFromGuests();
 
@@ -295,14 +285,24 @@ export class FrameSyncService {
       this._guestRPC.call('setHighlightsVisible', visible);
     });
 
-    // Create channel for sidebar <-> host communication and send port to host.
-    //
-    // This also serves to notify the host that the sidebar application is ready.
-    const hostChannel = new MessageChannel();
-    this._hostRPC.createChannel(hostChannel.port1);
-    this._window.parent.postMessage({ type: 'hypothesisSidebarReady' }, '*', [
-      hostChannel.port2,
-    ]);
+    // Create channel for sidebar-host communication and send port to host.
+    const hostPort = await this._portFinder.discover('host');
+    this._hostRPC.createChannel(hostPort);
+
+    // Create channel for guest-sidebar communication
+    hostPort.start();
+    this._listeners.add(hostPort, 'message', event => {
+      const { data, ports } = /** @type {MessageEvent} */ (event);
+      if (
+        isMessageEqual(data, {
+          frame1: 'guest',
+          frame2: 'sidebar',
+          type: 'offer',
+        })
+      ) {
+        this._guestRPC.createChannel(ports[0]);
+      }
+    });
   }
 
   /**
@@ -336,4 +336,10 @@ export class FrameSyncService {
   scrollToAnnotation(tag) {
     this._guestRPC.call('scrollToAnnotation', tag);
   }
+
+  // Only used to cleanup tests
+  destroy() {
+    this._portFinder.destroy();
+    this._listeners.removeAll();
+  }
 }

src/sidebar/services/test/frame-sync-test.js

@@ -3,6 +3,7 @@ import EventEmitter from 'tiny-emitter';
 import { Injector } from '../../../shared/injector';
 import * as annotationFixtures from '../../test/annotation-fixtures';
 import createFakeStore from '../../test/fake-redux-store';
+import { delay } from '../../../test-util/wait';
 
 import { FrameSyncService, $imports, formatAnnot } from '../frame-sync';
 
@@ -57,12 +58,37 @@ describe('FrameSyncService', () => {
   let FakeBridge;
 
   let fakeAnnotationsService;
-  let fakeStore;
   let fakeBridges;
+  let fakePortFinder;
+  let fakeStore;
   let frameSync;
   let fakeWindow;
+  const { port1: hostPort, port2: sidebarPort } = new MessageChannel();
 
   beforeEach(() => {
+    fakeAnnotationsService = { create: sinon.stub() };
+
+    fakeBridges = [];
+    FakeBridge = sinon.stub().callsFake(() => {
+      const emitter = new EventEmitter();
+      const bridge = {
+        call: sinon.stub(),
+        createChannel: sinon.stub(),
+        emit: emitter.emit.bind(emitter),
+        on: emitter.on.bind(emitter),
+        onConnect: function (listener) {
+          emitter.on('connect', listener);
+        },
+      };
+      fakeBridges.push(bridge);
+      return bridge;
+    });
+
+    fakePortFinder = {
+      destroy: sinon.stub(),
+      discover: sinon.stub().resolves(sidebarPort),
+    };
+
     fakeStore = createFakeStore(
       { annotations: [] },
       {
@@ -84,29 +110,14 @@ describe('FrameSyncService', () => {
       }
     );
 
-    fakeAnnotationsService = { create: sinon.stub() };
-
-    fakeBridges = [];
-    FakeBridge = sinon.stub().callsFake(() => {
-      const emitter = new EventEmitter();
-      const bridge = {
-        call: sinon.stub(),
-        createChannel: sinon.stub(),
-        emit: emitter.emit.bind(emitter),
-        on: emitter.on.bind(emitter),
-        onConnect: function (listener) {
-          emitter.on('connect', listener);
-        },
-      };
-      fakeBridges.push(bridge);
-      return bridge;
-    });
-
     fakeWindow = new FakeWindow();
     fakeWindow.parent = new FakeWindow();
 
     $imports.$mock({
       '../../shared/bridge': { Bridge: FakeBridge },
+      '../../shared/port-finder': {
+        PortFinder: sinon.stub().returns(fakePortFinder),
+      },
     });
 
     frameSync = new Injector()
@@ -118,6 +129,7 @@ describe('FrameSyncService', () => {
   });
 
   afterEach(() => {
+    frameSync.destroy();
     $imports.$restore();
   });
 
@@ -134,63 +146,27 @@ describe('FrameSyncService', () => {
   }
 
   describe('#connect', () => {
-    let testChannel;
+    it('discovers and connects to the host frame', async () => {
+      await frameSync.connect();
 
-    beforeEach(() => {
-      testChannel = new MessageChannel();
-
-      sinon.stub(console, 'warn');
-      sinon.stub(window, 'MessageChannel');
-      window.MessageChannel.returns(testChannel);
+      assert.calledWith(hostBridge().createChannel, sidebarPort);
     });
 
-    afterEach(() => {
-      console.warn.restore();
-      window.MessageChannel.restore();
-    });
+    it('connects to new guests when they are ready', async () => {
+      const { port1 } = new MessageChannel();
 
-    it('sends `hypothesisSidebarReady` notification to host frame with message port', () => {
       frameSync.connect();
-
-      assert.calledWith(hostBridge().createChannel, testChannel.port1);
-      assert.calledWith(
-        fakeWindow.parent.postMessage,
+      hostPort.postMessage(
         {
-          type: 'hypothesisSidebarReady',
+          frame1: 'guest',
+          frame2: 'sidebar',
+          type: 'offer',
         },
-        '*',
-        [testChannel.port2]
+        [port1]
       );
-    });
+      await delay(0);
 
-    it('connects to new guests when they are ready', () => {
-      const channel = new MessageChannel();
-
-      frameSync.connect();
-      fakeWindow.dispatchEvent(
-        new MessageEvent('message', {
-          data: { type: 'hypothesisGuestReady' },
-          ports: [channel.port1],
-        })
-      );
-
-      assert.calledWith(guestBridge().createChannel, channel.port1);
-    });
-
-    [
-      { data: 'not-an-object' },
-      { data: {} },
-      { data: { type: 'unknownType' } },
-      {
-        // No ports provided with message
-        data: { type: 'hypothesisGuestReady' },
-      },
-    ].forEach(messageInit => {
-      it('ignores `hypothesisGuestReady` messages that are invalid', () => {
-        frameSync.connect();
-        fakeWindow.dispatchEvent(new MessageEvent('message', messageInit));
-        assert.notCalled(guestBridge().createChannel);
-      });
+      assert.calledWith(guestBridge().createChannel, port1);
     });
   });
 

src/types/annotator.js

@@ -149,15 +149,6 @@
  * @typedef Globals
  * @prop {import('./pdfjs').PDFViewerApplication} [PDFViewerApplication] -
  *   PDF.js entry point. If set, triggers loading of PDF rather than HTML integration.
- * @prop {object} [__hypothesis] - Internal data related to supporting guests in iframes
- *   @prop {Promise<[Window]>} [__hypothesis.sidebarWindow] -
- *     The sidebar window that is active in this frame. This resolves once the sidebar
- *     application has started and is ready to connect to guests.
- *
- *     The `Window` object is wrapped in an array to avoid issues with
- *     combining promises with cross-origin objects in old browsers
- *     (eg. Safari 11, Chrome <= 63) which trigger an exception when trying to
- *     test if the object has a `then` method. See https://github.com/whatwg/dom/issues/536.
  */
 
 /**