Assume annotations for other document segments will anchor

When annotations are loaded into the sidebar, they are not displayed in the
"Annotations" tab immediately. Instead the client waits until the guest reports
that the annotation was successfully anchored, or a timeout expires. This is to
avoid annotations briefly appearing in the Annotations tab and then "jumping" to
the Orphans tab a moment later if they fail to anchor.

For annotations in document segments (eg. EPUB chapters) other than the one
currently loaded in a guest frame, the annotation will never anchor in the
guest, and so the annotation did not appear in the sidebar until the anchoring
timeout expired. The result was that current-chapter annotations appeared
quickly, and other-chapter annotations appeared in the sidebar several hundred
ms later.

This commit improves the experience by optimisitically assuming annotations
in other chapters will anchor when the user navigates to those chapters, and
immediately marking them as "anchored" in the store. If they fail to anchor
when the user navigates to the chapter, then the status will be updated and
the annotation will be marked as an orphan at that point.

src/sidebar/services/frame-sync.ts

 import debounce from 'lodash.debounce';
+import type { DebouncedFunction } from 'lodash.debounce';
 import shallowEqual from 'shallowequal';
 
 import { ListenerCollection } from '../../shared/listener-collection';
@@ -141,6 +142,21 @@ export class FrameSyncService {
    */
   private _pendingHoverTag: string | null;
 
+  /**
+   * Map of annotation tag to anchoring status. This holds status updates
+   * which have been received from guest frames but not yet committed to the store.
+   *
+   * Commits are batched to reduce the reduce the overhead from re-rendering
+   * etc. triggered by `SidebarStore.updateAnchorStatus` calls.
+   */
+  private _pendingAnchorStatusUpdates: Map<string, 'anchored' | 'orphan'>;
+
+  /**
+   * Schedule a commit of the anchoring status updates in
+   * {@link _pendingAnchorStatusUpdates} to the store.
+   */
+  private _scheduleAnchorStatusUpdate: DebouncedFunction<[]>;
+
   // Test seam
   private _window: Window;
 
@@ -165,6 +181,15 @@ export class FrameSyncService {
 
     this._pendingScrollToTag = null;
     this._pendingHoverTag = null;
+    this._pendingAnchorStatusUpdates = new Map();
+
+    this._scheduleAnchorStatusUpdate = debounce(() => {
+      const records = Object.fromEntries(
+        this._pendingAnchorStatusUpdates.entries()
+      );
+      this._store.updateAnchorStatus(records);
+      this._pendingAnchorStatusUpdates.clear();
+    }, 10);
 
     this._setupSyncToGuests();
     this._setupHostEvents();
@@ -214,6 +239,15 @@ export class FrameSyncService {
       if (added.length > 0) {
         const addedByFrame = new Map<string | null, Annotation[]>();
 
+        // List of annotations to immediately mark as anchored, as opposed to
+        // waiting for the guest to report the status. This is used for
+        // annotations associated with content that is different from what is
+        // currently loaded in the guest frame (eg. different EPUB chapter).
+        //
+        // For these annotations, we optimistically assume they will anchor
+        // when the appropriate content is loaded.
+        const anchorImmediately = [];
+
         for (const annotation of added) {
           const frame = frameForAnnotation(frames, annotation);
           if (
@@ -221,6 +255,7 @@ export class FrameSyncService {
             (frame.segment &&
               !annotationMatchesSegment(annotation, frame.segment))
           ) {
+            anchorImmediately.push(annotation.$tag);
             continue;
           }
           const anns = addedByFrame.get(frame.id) ?? [];
@@ -228,6 +263,10 @@ export class FrameSyncService {
           addedByFrame.set(frame.id, anns);
         }
 
+        if (anchorImmediately.length > 0) {
+          this._updateAnchorStatus(anchorImmediately, 'anchored');
+        }
+
         for (const [frameId, anns] of addedByFrame) {
           const rpc = this._guestRPC.get(frameId);
           if (rpc) {
@@ -280,6 +319,17 @@ export class FrameSyncService {
     );
   }
 
+  /**
+   * Schedule an update of the anchoring status of annotation(s) in the store.
+   */
+  _updateAnchorStatus(tag: string | string[], state: 'orphan' | 'anchored') {
+    const tags = Array.isArray(tag) ? tag : [tag];
+    for (const tag of tags) {
+      this._pendingAnchorStatusUpdates.set(tag, state);
+    }
+    this._scheduleAnchorStatusUpdate();
+  }
+
   /**
    * Set up a connection to a new guest frame.
    *
@@ -352,26 +402,10 @@ export class FrameSyncService {
       this._annotationsService.create(annot);
     });
 
-    // Map of annotation tag to anchoring status
-    // ('anchored'|'orphan'|'timeout').
-    //
-    // Updates are coalesced to reduce the overhead from processing
-    // triggered by each `UPDATE_ANCHOR_STATUS` action that is dispatched.
-
-    let anchoringStatusUpdates: Record<
-      string,
-      'anchored' | 'orphan' | 'timeout'
-    > = {};
-    const scheduleAnchoringStatusUpdate = debounce(() => {
-      this._store.updateAnchorStatus(anchoringStatusUpdates);
-      anchoringStatusUpdates = {};
-    }, 10);
-
     // Anchoring an annotation in the frame completed
     guestRPC.on('syncAnchoringStatus', ({ $tag, $orphan }: AnnotationData) => {
       this._inFrame.add($tag);
-      anchoringStatusUpdates[$tag] = $orphan ? 'orphan' : 'anchored';
-      scheduleAnchoringStatusUpdate();
+      this._updateAnchorStatus($tag, $orphan ? 'orphan' : 'anchored');
 
       if ($tag === this._pendingHoverTag) {
         this._pendingHoverTag = null;

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

@@ -249,6 +249,13 @@ describe('FrameSyncService', () => {
     };
   }
 
+  /**
+   * "Wait" for the debouncing timeout for anchoring status updates to expire.
+   */
+  function expireDebounceTimeout(clock) {
+    clock.tick(20);
+  }
+
   describe('#connect', () => {
     it('discovers and connects to the host frame', async () => {
       await frameSync.connect();
@@ -377,7 +384,6 @@ describe('FrameSyncService', () => {
     });
 
     context('in EPUB documents', () => {
-      it('sends annotations to frame if current segment matches frame', async () => {
       const bookURI = 'https://publisher.com/books/1234';
 
       const chapter1ann = {
@@ -394,6 +400,7 @@ describe('FrameSyncService', () => {
           },
         ],
       };
+
       const chapter2ann = {
         uri: bookURI,
         target: [
@@ -409,6 +416,9 @@ describe('FrameSyncService', () => {
         ],
       };
 
+      let clock;
+
+      beforeEach(async () => {
         await connectGuest();
         emitGuestEvent('documentInfoChanged', {
           uri: bookURI,
@@ -417,6 +427,13 @@ describe('FrameSyncService', () => {
             url: '/chapters/02.xhtml',
           },
         });
+      });
+
+      afterEach(() => {
+        clock?.restore();
+      });
+
+      it('sends annotations to frame only if current segment matches frame', () => {
         fakeStore.setState({ annotations: [chapter1ann, chapter2ann] });
 
         assert.calledWithMatch(
@@ -425,6 +442,19 @@ describe('FrameSyncService', () => {
           sinon.match([formatAnnot(chapter2ann)])
         );
       });
+
+      it('"immediately" marks annotations for other document segments as anchored', () => {
+        clock = sinon.useFakeTimers();
+
+        fakeStore.setState({ annotations: [chapter1ann, chapter2ann] });
+        assert.notCalled(fakeStore.updateAnchorStatus);
+
+        expireDebounceTimeout(clock);
+
+        assert.calledWith(fakeStore.updateAnchorStatus, {
+          [chapter1ann.$tag]: 'anchored',
+        });
+      });
     });
 
     it('sends a "loadAnnotations" message only for new annotations', async () => {
@@ -634,16 +664,10 @@ describe('FrameSyncService', () => {
       clock.restore();
     });
 
-    function expireDebounceTimeout() {
-      // "Wait" for debouncing timeout to expire and pending anchoring status
-      // updates to be applied.
-      clock.tick(20);
-    }
-
     it('updates the anchoring status for the annotation', () => {
       emitGuestEvent('syncAnchoringStatus', { $tag: 't1', $orphan: false });
 
-      expireDebounceTimeout();
+      expireDebounceTimeout(clock);
 
       assert.calledWith(fakeStore.updateAnchorStatus, { t1: 'anchored' });
     });
@@ -658,7 +682,7 @@ describe('FrameSyncService', () => {
         $orphan: true,
       });
 
-      expireDebounceTimeout();
+      expireDebounceTimeout(clock);
 
       assert.calledWith(fakeStore.updateAnchorStatus, {
         t1: 'anchored',