Push document metadata from guest to sidebar

Change the way document metadata and URIs for guest frames gets from the
guest to the sidebar, so that the guest _pushes_ the information instead
of the sidebar requesting it. This achieves two things:

 1. It enables the sidebar to get the initial document URI faster, as
    the guest can fetch the information and send it to the sidebar over
    the message channel while the sidebar is still loading. This in turn
    allows the initial annotation search request to be made sooner and
    for annotations to appear quicker.

    This fixes https://github.com/hypothesis/client/issues/4094.

    In practice this doesn't improve the best case metadata fetch time
    significantly (only 5-10ms or so) but does seem to reduce the variance in
    fetch times.

 2. It will in future enable the guest to inform the sidebar of
    URI/metadata changes after the initial load. This can happen if a
    client-side navigation happens in an HTML document for example or if
    the currently loaded PDF is changed in PDF.js.

src/annotator/guest.js

@@ -176,6 +176,12 @@ export default class Guest {
       source: 'guest',
     });
 
+    /**
+     * Integration that handles document-type specific functionality in the
+     * guest.
+     */
+    this._integration = createIntegration(this);
+
     /**
      * Channel for host-guest communication.
      *
@@ -196,12 +202,6 @@ export default class Guest {
     // iframes in this frame.
     this._hypothesisInjector = new HypothesisInjector(this.element, config);
 
-    /**
-     * Integration that handles document-type specific functionality in the
-     * guest.
-     */
-    this._integration = createIntegration(this);
-
     this._bucketBarClient =
       this._frameIdentifier === null
         ? new BucketBarClient({
@@ -417,13 +417,6 @@ export default class Guest {
       }
     );
 
-    // Handler for when sidebar requests metadata for the current document
-    this._sidebarRPC.on('getDocumentInfo', cb => {
-      this.getDocumentInfo()
-        .then(info => cb(null, info))
-        .catch(reason => cb(reason));
-    });
-
     // Handler for controls on the sidebar
     this._sidebarRPC.on('setHighlightsVisible', showHighlights => {
       this.setHighlightsVisible(showHighlights);
@@ -441,10 +434,16 @@ export default class Guest {
       annotations => annotations.forEach(annotation => this.anchor(annotation))
     );
 
-    // Discover and connect to the sidebar frame. All RPC events must be
-    // registered before creating the channel.
-    const sidebarPort = await this._portFinder.discover('sidebar');
-    this._sidebarRPC.connect(sidebarPort);
+    // Connect to sidebar and send document info/URIs to it.
+    //
+    // RPC calls are deferred until a connection is made, so these steps can
+    // complete in either order.
+    this._portFinder.discover('sidebar').then(port => {
+      this._sidebarRPC.connect(port);
+    });
+    this.getDocumentInfo().then(metadata =>
+      this._sidebarRPC.call('documentInfoChanged', metadata)
+    );
   }
 
   destroy() {

src/annotator/test/guest-test.js

@@ -141,7 +141,7 @@ describe('Guest', () => {
     FakeHypothesisInjector = sinon.stub().returns(fakeHypothesisInjector);
 
     fakePortFinder = {
-      discover: sinon.stub(),
+      discover: sinon.stub().resolves({}),
       destroy: sinon.stub(),
     };
 
@@ -359,40 +359,6 @@ describe('Guest', () => {
       });
     });
 
-    describe('on "getDocumentInfo" event', () => {
-      let guest;
-
-      afterEach(() => {
-        guest.destroy();
-        sandbox.restore();
-      });
-
-      function createCallback(expectedUri, expectedMetadata, done) {
-        return (err, result) => {
-          assert.strictEqual(err, null);
-          try {
-            assert.equal(result.uri, expectedUri);
-            assert.deepEqual(result.metadata, expectedMetadata);
-            done();
-          } catch (e) {
-            done(e);
-          }
-        };
-      }
-
-      it('calls the callback with document URL and metadata', done => {
-        guest = createGuest();
-        const metadata = { title: 'hi' };
-
-        fakeIntegration.getMetadata.resolves(metadata);
-
-        emitSidebarEvent(
-          'getDocumentInfo',
-          createCallback('https://example.com/test.pdf', metadata, done)
-        );
-      });
-    });
-
     describe('on "setHighlightsVisible" event', () => {
       it('sets visibility of highlights in document', () => {
         const guest = createGuest();
@@ -818,6 +784,8 @@ describe('Guest', () => {
   describe('#createAnnotation', () => {
     it('creates an annotation if host calls "createAnnotationIn" RPC method', async () => {
       createGuest();
+      await delay(0);
+      sidebarRPC().call.resetHistory(); // Discard `documentInfoChanged` call
 
       emitHostEvent('createAnnotationIn', 'dummy');
       await delay(0);
@@ -1299,6 +1267,19 @@ describe('Guest', () => {
     assert.notCalled(FakeBucketBarClient);
   });
 
+  it('sends document metadata and URIs to sidebar', async () => {
+    createGuest();
+    await delay(0);
+    assert.calledWith(sidebarRPC().call, 'documentInfoChanged', {
+      uri: 'https://example.com/test.pdf',
+      metadata: {
+        title: 'Test title',
+        documentFingerprint: 'test-fingerprint',
+      },
+      frameIdentifier: null,
+    });
+  });
+
   describe('#fitSideBySide', () => {
     it('attempts to fit content alongside sidebar', () => {
       const guest = createGuest();

src/sidebar/services/frame-sync.js

@@ -182,6 +182,18 @@ export class FrameSyncService {
     /** @type {PortRPC<GuestToSidebarEvent, SidebarToGuestEvent>} */
     const guestRPC = new PortRPC();
 
+    // Update document metadata for this guest. We currently assume that the
+    // guest will make this call once after it connects. To handle updates
+    // to the document, we'll need to change `connectFrame` to update rather than
+    // add to the frame list.
+    guestRPC.on('documentInfoChanged', info => {
+      this._store.connectFrame({
+        id: info.frameIdentifier,
+        metadata: info.metadata,
+        uri: info.uri,
+      });
+    });
+
     // A new annotation, note or highlight was created in the frame
     guestRPC.on(
       'createAnnotation',
@@ -267,21 +279,6 @@ export class FrameSyncService {
 
     // Synchronize highlight visibility in this guest with the sidebar's controls.
     guestRPC.call('setHighlightsVisible', this._highlightsVisible);
-
-    // Fetch document metadata. This could be optimized by having the guest
-    // push metadata to the sidebar. See https://github.com/hypothesis/client/issues/4094.
-    guestRPC.call('getDocumentInfo', (err, info) => {
-      if (err) {
-        guestRPC.destroy();
-        return;
-      }
-
-      this._store.connectFrame({
-        id: info.frameIdentifier,
-        metadata: info.metadata,
-        uri: info.uri,
-      });
-    });
   }
 
   /**

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

@@ -25,8 +25,8 @@ const fixtures = {
     },
   },
 
-  // Response to the `getDocumentInfo` channel message for a frame displaying
-  // an HTML document
+  // Argument to the `documentInfoChanged` call made by a guest displaying an HTML
+  // document.
   htmlDocumentInfo: {
     uri: 'http://example.org',
     metadata: {
@@ -35,17 +35,6 @@ const fixtures = {
     frameIdentifier: null,
   },
 
-  // Response to the `getDocumentInfo` channel message for a frame displaying
-  // a PDF
-  pdfDocumentInfo: {
-    uri: 'http://example.org/paper.pdf',
-    metadata: {
-      documentFingerprint: '1234',
-      link: [{ href: 'http://example.org/paper.pdf' }, { href: 'urn:1234' }],
-    },
-    frameIdentifier: null,
-  },
-
   // The entry in the list of frames currently connected
   framesListEntry: {
     id: 'abc',
@@ -457,13 +446,8 @@ describe('FrameSyncService', () => {
 
     it("adds the page's metadata to the frames list", async () => {
       const frameInfo = fixtures.htmlDocumentInfo;
-      setupPortRPC = rpc => {
-        rpc.call.withArgs('getDocumentInfo').callsFake((method, callback) => {
-          callback(null, frameInfo);
-        });
-      };
-
       await connectGuest();
+      emitGuestEvent('documentInfoChanged', frameInfo);
 
       assert.calledWith(fakeStore.connectFrame, {
         id: frameInfo.frameIdentifier,
@@ -472,22 +456,6 @@ describe('FrameSyncService', () => {
       });
     });
 
-    it('closes the channel and does not add frame to store if getting document info fails', async () => {
-      let channel;
-      setupPortRPC = rpc => {
-        rpc.call.withArgs('getDocumentInfo').callsFake((method, callback) => {
-          callback('Error getting document info');
-        });
-        channel = rpc;
-      };
-
-      await connectGuest();
-
-      assert.ok(channel);
-      assert.called(channel.destroy);
-      assert.notCalled(fakeStore.connectFrame);
-    });
-
     it("synchronizes highlight visibility in the guest with the sidebar's controls", async () => {
       let channel;
       setupPortRPC = rpc => {

src/types/port-rpc-events.d.ts

@@ -46,6 +46,9 @@ export type GuestToSidebarEvent =
    */
   | 'openSidebar'
 
+  /** The guest is notifying the sidebar of the current document metadata and URIs. */
+  | 'documentInfoChanged'
+
   /**
    * The guest is asking the sidebar to display some annotations.
    */
@@ -118,11 +121,6 @@ export type SidebarToGuestEvent =
    */
   | 'focusAnnotations'
 
-  /**
-   * The sidebar is asking the guest(s) for the URL and other metadata about the document.
-   */
-  | 'getDocumentInfo'
-
   /**
    * The sidebar is asking the guest(s) to load annotations.
    */