Remove `PortProvider#hostPortFrom` method

There are two methods to get the host port from `PortProvider`:

- `#hostPortFrom` which returns the host port exclusively from the
  `sidebar-host` channel.

- `#on` allows to register a listener that is fired upon requests
  of host frames for channels other than the `sidebar-host`.

This is an attempt to simplify the API of `PortProvider` and merge both
methods into one.

Advantages:

- simplifies the `Sidebar` constructor signature (no need to pass the
  host port)

- avoids the pitfall of creating host-sidebar RPC channel before
  registering the RPC methods

- prepares for future guest-host channel see https://github.com/hypothesis/client/blob/ad93317debdac0d2c3abec83eb9d690934632ef3/src/annotator/index.js#L64-L66 (where the `#on` method is use to create the guest-host channel).

src/annotator/index.js

@@ -54,15 +54,13 @@ function init() {
     const hypothesisAppsOrigin = new URL(sidebarConfig.sidebarAppUrl).origin;
     portProvider = new PortProvider(hypothesisAppsOrigin);
 
-    sidebar = new Sidebar(
-      document.body,
-      eventBus,
-      portProvider.hostPortFor('sidebar'),
-      guest,
-      sidebarConfig
-    );
+    sidebar = new Sidebar(document.body, eventBus, guest, sidebarConfig);
     notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
 
+    portProvider.on('frameConnected', (source, port) =>
+      sidebar.onFrameConnected(source, port)
+    );
+
     portProvider.listen();
   }
 

src/annotator/sidebar.js

@@ -56,18 +56,14 @@ export default class Sidebar {
    * @param {HTMLElement} element
    * @param {import('./util/emitter').EventBus} eventBus -
    *   Enables communication between components sharing the same eventBus
-   * @param {MessagePort} hostPort -
-   *   Host port for the host-sidebar communication channel. The sidebar app will
-   *   request its side of the channel when it starts.
    * @param {Guest} guest -
    *   The `Guest` instance for the current frame. It is currently assumed that
    *   it is always possible to annotate in the frame where the sidebar is
    *   displayed.
    * @param {Record<string, any>} [config]
    */
-  constructor(element, eventBus, hostPort, guest, config = {}) {
+  constructor(element, eventBus, guest, config = {}) {
     this._emitter = eventBus.createEmitter();
-    this._hostPort = hostPort;
 
     /**
      * Channel for host-sidebar communication.
@@ -198,9 +194,6 @@ export default class Sidebar {
       }
     });
 
-    // Create channel *after* all bridge events are registered with the `on` method.
-    this._sidebarRPC.createChannel(this._hostPort);
-
     // 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.
@@ -225,6 +218,18 @@ export default class Sidebar {
     this._emitter.destroy();
   }
 
+  /**
+   * Establish RPC channels from specific source frames
+   *
+   * @param {'guest'|'sidebar'} source
+   * @param {MessagePort} port
+   */
+  onFrameConnected(source, port) {
+    if (source === 'sidebar') {
+      this._sidebarRPC.createChannel(port);
+    }
+  }
+
   _setupSidebarEvents() {
     annotationCounts(document.body, this._sidebarRPC);
     sidebarTrigger(document.body, () => this.open());

src/annotator/test/sidebar-test.js

@@ -48,8 +48,7 @@ describe('Sidebar', () => {
     containers.push(container);
 
     const eventBus = new EventBus();
-    const { port1 } = new MessageChannel();
-    const sidebar = new Sidebar(container, eventBus, port1, fakeGuest, config);
+    const sidebar = new Sidebar(container, eventBus, fakeGuest, config);
     sidebars.push(sidebar);
 
     return sidebar;
@@ -531,6 +530,24 @@ describe('Sidebar', () => {
     });
   });
 
+  describe('#onFrameConnected', () => {
+    it('ignores unrecognized source frames', () => {
+      const sidebar = createSidebar();
+      const { port1 } = new MessageChannel();
+      sidebar.onFrameConnected('dummy', port1);
+
+      assert.notCalled(fakeBridge.createChannel);
+    });
+
+    it('create RPC channels for recognized source frames', () => {
+      const sidebar = createSidebar();
+      const { port1 } = new MessageChannel();
+      sidebar.onFrameConnected('sidebar', port1);
+
+      assert.calledWith(fakeBridge.createChannel, port1);
+    });
+  });
+
   describe('#open', () => {
     it('shows highlights if "showHighlights" is set to "whenSidebarOpen"', () => {
       const sidebar = createSidebar({ showHighlights: 'whenSidebarOpen' });

src/shared/port-provider.js

@@ -135,7 +135,7 @@ export class PortProvider {
 
   /**
    * @param {'frameConnected'} eventName
-   * @param {(source: 'guest', port: MessagePort) => void} handler - this handler
+   * @param {(source: 'guest'|'sidebar', port: MessagePort) => void} handler - this handler
    *   fires when a request for the host frame has been granted. Currently, only
    *   the 'guest-host' channel triggers this listener.
    */
@@ -143,16 +143,6 @@ export class PortProvider {
     this._emitter.on(eventName, handler);
   }
 
-  /**
-   * Returns the `host` port from the `sidebar-host` channel.
-   *
-   * @param {'sidebar'} _targetFrame
-   */
-  // eslint-disable-next-line no-unused-vars
-  hostPortFor(_targetFrame) {
-    return this._sidebarHostChannel.port2;
-  }
-
   /**
    * Initiate the listener of port requests by other frames.
    */
@@ -209,7 +199,7 @@ export class PortProvider {
         this._sidebarHostChannel.port2.postMessage(message, [
           messageChannel.port2,
         ]);
-      } else if (frame2 === 'host' && frame1 === 'guest') {
+      } else if (frame2 === 'host') {
         this._emitter.emit('frameConnected', frame1, messageChannel.port2);
       }
     });

src/shared/test/port-provider-test.js

@@ -46,12 +46,6 @@ describe('PortProvider', () => {
     });
   });
 
-  describe('#hostPortFor', () => {
-    it('returns the `host` port of the `sidebar-host` channel', () => {
-      assert.instanceOf(portProvider.hostPortFor('sidebar'), MessagePort);
-    });
-  });
-
   describe('#listen', () => {
     it('ignores all port requests before `listen` is called', async () => {
       portProvider.listen();
@@ -210,13 +204,27 @@ describe('PortProvider', () => {
       portProvider.listen();
       const handler = sinon.stub();
       portProvider.on('frameConnected', handler);
-      const data = {
-        frame1: 'guest',
-        frame2: 'host',
-        type: 'request',
-      };
       await sendPortFinderRequest({
-        data,
+        data: {
+          frame1: 'sidebar',
+          frame2: 'host',
+          type: 'request',
+        },
+      });
+
+      assert.calledWith(
+        handler,
+        'sidebar',
+        sinon.match.instanceOf(MessagePort)
+      );
+
+      handler.resetHistory();
+      await sendPortFinderRequest({
+        data: {
+          frame1: 'guest',
+          frame2: 'host',
+          type: 'request',
+        },
       });
 
       assert.calledWith(handler, 'guest', sinon.match.instanceOf(MessagePort));