Support feature flags in guest frames

Rewrite the infrastructure that sends feature flags to annotator code, to
support feature-flagging functionality in guest frames, instead of just the host
frame.

 - Remove `FeaturesService` in the sidebar and move the logic for
   syncing feature flags from the sidebar to other frames into
   `FrameSyncService`.  This is a better place since `FrameSyncService`
   knows when frames connect and thus when to send flags to them. The
   logic for monitoring for flag changes is very simple and doesn't
   needs its own service.

 - Within `FrameSyncService`, send "featureFlagsUpdated" notifications to
   both host frames and guest frames.

 - Convert the functions and global variables in `annotator/features.js`
   into a `FeatureFlags` class, which encapsulates storage of flags,
   access to them and notifying consumers when flags change.

 - Instantiate `FeatureFlags` instances in the host frame and guest
   frame and update the flags stored in them when "featureFlagsUpdated" events
   are received from the sidebar.

src/annotator/features.js

-import { warnOnce } from '../shared/warn-once';
+import { TinyEmitter } from 'tiny-emitter';
 
-/** @type {Record<string, boolean>} */
-let _features = {};
+import { warnOnce } from '../shared/warn-once';
 
-/** @param {Record<string, boolean>} features */
-const _set = features => {
-  _features = features || {};
-};
+/**
+ * List of feature flags that annotator code tests for.
+ *
+ * @type {string[]}
+ */
+const annotatorFlags = [];
 
-export const features = {
+/**
+ * An observable container of feature flags.
+ */
+export class FeatureFlags extends TinyEmitter {
   /**
-   * @param {import('../shared/messaging').PortRPC<'featureFlagsUpdated', string>} rpc - Channel for host-sidebar communication
+   * @param {string[]} knownFlags - Test seam. This is a list of known flags
+   *   used to catch mistakes where code checks for an obsolete feature flag.
    */
-  init: function (rpc) {
-    rpc.on('featureFlagsUpdated', _set);
-  },
+  constructor(knownFlags = annotatorFlags) {
+    super();
+
+    /**
+     * Map of flag name to enabled state.
+     *
+     * @type {Map<string, boolean>}
+     */
+    this._flags = new Map();
+    this._knownFlags = knownFlags;
+  }
 
-  reset: function () {
-    _set({});
-  },
+  /**
+   * Update the stored flags and notify observers via a "flagsChanged" event.
+   *
+   * @param {Record<string, boolean>} flags
+   */
+  update(flags) {
+    this._flags.clear();
+    for (let [flag, on] of Object.entries(flags)) {
+      this._flags.set(flag, on);
+    }
+    this.emit('flagsChanged', this._flags);
+  }
 
-  /** @param {string} flag */
-  flagEnabled: function (flag) {
-    if (!(flag in _features)) {
-      warnOnce('looked up unknown feature', flag);
+  /**
+   * Test if a feature flag is enabled.
+   *
+   * This will return false if the feature flags have not yet been received from
+   * the backend. Code that uses a feature flag should handle subsequent changes
+   * to the flag's state by listening for the "flagsChanged" event.
+   *
+   * @param {string} flag
+   * @return {boolean}
+   */
+  flagEnabled(flag) {
+    if (!this._knownFlags.includes(flag)) {
+      warnOnce('Looked up unknown feature', flag);
       return false;
     }
-    return _features[flag];
-  },
-};
+    return this._flags.get(flag) ?? false;
+  }
+
+  /**
+   * Return the state of all feature flags.
+   *
+   * To test whether an individual flag is enabled, use {@link flagEnabled}
+   * instead.
+   */
+  allFlags() {
+    return Object.fromEntries(this._flags);
+  }
+}

src/annotator/guest.js

@@ -5,6 +5,7 @@ import { generateHexString } from '../shared/random';
 import { Adder } from './adder';
 import { TextRange } from './anchoring/text-range';
 import { BucketBarClient } from './bucket-bar-client';
+import { FeatureFlags } from './features';
 import {
   getHighlightsContainingNode,
   highlightRange,
@@ -187,6 +188,8 @@ export class Guest {
       source: 'guest',
     });
 
+    this.features = new FeatureFlags();
+
     /**
      * Integration that handles document-type specific functionality in the
      * guest.
@@ -367,6 +370,10 @@ export class Guest {
   }
 
   async _connectSidebar() {
+    this._sidebarRPC.on('featureFlagsUpdated', flags =>
+      this.features.update(flags)
+    );
+
     // Handlers for events sent when user hovers or clicks on an annotation card
     // in the sidebar.
     this._sidebarRPC.on(

src/annotator/sidebar.js

@@ -8,7 +8,7 @@ import { PortRPC } from '../shared/messaging';
 import { annotationCounts } from './annotation-counts';
 import { BucketBar } from './bucket-bar';
 import { createAppConfig } from './config/app';
-import { features } from './features';
+import { FeatureFlags } from './features';
 import { sidebarTrigger } from './sidebar-trigger';
 import { ToolbarController } from './toolbar';
 import { createShadowRoot } from './util/shadow-root';
@@ -125,6 +125,8 @@ export class Sidebar {
     /** @type {BucketBar|null} */
     this.bucketBar = null;
 
+    this.features = new FeatureFlags();
+
     if (config.externalContainerSelector) {
       this.externalFrame =
         /** @type {HTMLElement} */
@@ -326,7 +328,10 @@ export class Sidebar {
   _setupSidebarEvents() {
     annotationCounts(document.body, this._sidebarRPC);
     sidebarTrigger(document.body, () => this.open());
-    features.init(this._sidebarRPC);
+
+    this._sidebarRPC.on('featureFlagsUpdated', flags =>
+      this.features.update(flags)
+    );
 
     this._sidebarRPC.on('connect', () => {
       // Show the UI

src/annotator/test/features-test.js

-import { features, $imports } from '../features';
+import { FeatureFlags, $imports } from '../features';
 
-describe('features - annotation layer', () => {
-  let featureFlagsUpdateHandler;
+describe('FeatureFlags', () => {
   let fakeWarnOnce;
 
-  const initialFeatures = {
+  const testFlags = {
     feature_on: true,
     feature_off: false,
   };
 
-  const setFeatures = function (features) {
-    featureFlagsUpdateHandler(features || initialFeatures);
-  };
-
   beforeEach(() => {
     fakeWarnOnce = sinon.stub();
     $imports.$mock({
       '../shared/warn-once': { warnOnce: fakeWarnOnce },
     });
-
-    features.init({
-      on: function (topic, handler) {
-        if (topic === 'featureFlagsUpdated') {
-          featureFlagsUpdateHandler = handler;
-        }
-      },
-    });
-
-    // set default features
-    setFeatures();
   });
 
   afterEach(() => {
-    features.reset();
     $imports.$restore();
   });
 
-  describe('flagEnabled', () => {
-    it('should retrieve features data', () => {
-      assert.equal(features.flagEnabled('feature_on'), true);
-      assert.equal(features.flagEnabled('feature_off'), false);
+  function createFeatureFlags() {
+    return new FeatureFlags(Object.keys(testFlags));
+  }
+
+  describe('#update', () => {
+    it('emits "flagsChanged" notification with new flags', () => {
+      const onFlagsChanged = sinon.stub();
+      const features = createFeatureFlags();
+
+      features.on('flagsChanged', onFlagsChanged);
+      features.update(testFlags);
+
+      assert.calledOnce(onFlagsChanged);
+      const flagMap = onFlagsChanged.getCall(0).args[0];
+      assert.instanceOf(flagMap, Map);
+      assert.deepEqual(
+        [...flagMap.entries()],
+        [
+          ['feature_on', true],
+          ['feature_off', false],
+        ]
+      );
     });
 
-    it('should return false if features have not been loaded', () => {
-      // simulate feature data not having been loaded yet
-      features.reset();
-      assert.equal(features.flagEnabled('feature_on'), false);
+    it('updates flags returned by `flagEnabled`', () => {
+      const features = createFeatureFlags();
+      assert.isFalse(features.flagEnabled('feature_on'));
+
+      features.update(testFlags);
+
+      assert.isTrue(features.flagEnabled('feature_on'));
     });
+  });
 
-    it('should return false for unknown flags', () => {
-      assert.isFalse(features.flagEnabled('unknown_feature'));
+  describe('#flagEnabled', () => {
+    it('returns current flag status', () => {
+      const features = createFeatureFlags();
+
+      features.update(testFlags);
+
+      assert.isTrue(features.flagEnabled('feature_on'));
+      assert.isFalse(features.flagEnabled('feature_off'));
+    });
+
+    it('returns false if flags are not loaded', () => {
+      const features = createFeatureFlags();
+      assert.isFalse(features.flagEnabled('feature_on'));
     });
 
     it('should warn when accessing unknown flags', () => {
+      const features = createFeatureFlags();
+
+      features.update(testFlags);
+
       assert.isFalse(features.flagEnabled('unknown_feature'));
       assert.calledOnce(fakeWarnOnce);
-      assert.calledWith(fakeWarnOnce, 'looked up unknown feature');
+      assert.calledWith(fakeWarnOnce, 'Looked up unknown feature');
+    });
+  });
+
+  describe('#allFlags', () => {
+    it('returns a record with all feature flags', () => {
+      const features = createFeatureFlags();
+
+      features.update(testFlags);
+
+      assert.deepEqual(features.allFlags(), testFlags);
     });
   });
 });

src/annotator/test/guest-test.js

@@ -472,6 +472,25 @@ describe('Guest', () => {
         assert.calledOnce(fakeBucketBarClient.update);
       });
     });
+
+    describe('on "featureFlagsUpdated" event', () => {
+      it('updates active feature flags', () => {
+        const flagsUpdated = sinon.stub();
+        const guest = createGuest();
+        guest.features.on('flagsChanged', flagsUpdated);
+
+        emitSidebarEvent('featureFlagsUpdated', {
+          some_flag: true,
+          other_flag: false,
+        });
+
+        assert.calledOnce(flagsUpdated);
+        assert.deepEqual(guest.features.allFlags(), {
+          some_flag: true,
+          other_flag: false,
+        });
+      });
+    });
   });
 
   describe('document events', () => {

src/annotator/test/sidebar-test.js

@@ -474,6 +474,22 @@ describe('Sidebar', () => {
 
         assert.called(onHelpRequest);
       }));
+
+    describe('on "featureFlagsUpdated" event', () => {
+      it('updates feature flags in host frame', () => {
+        const sidebar = createSidebar();
+
+        emitSidebarEvent('featureFlagsUpdated', {
+          some_flag: true,
+          other_flag: false,
+        });
+
+        assert.deepEqual(sidebar.features.allFlags(), {
+          some_flag: true,
+          other_flag: false,
+        });
+      });
+    });
   });
 
   describe('events from the guest frames', () => {

src/sidebar/index.js

@@ -71,19 +71,12 @@ function setupRoute(groups, session, router) {
  * to another.
  *
  * @param {import('./services/autosave').AutosaveService} autosaveService
- * @param {import('./services/features').FeaturesService} features
  * @param {import('./services/persisted-defaults').PersistedDefaultsService} persistedDefaults
  * @param {import('./services/service-url').ServiceURLService} serviceURL
  * @inject
  */
-function initServices(
-  autosaveService,
-  features,
-  persistedDefaults,
-  serviceURL
-) {
+function initServices(autosaveService, persistedDefaults, serviceURL) {
   autosaveService.init();
-  features.init();
   persistedDefaults.init();
   serviceURL.init();
 }
@@ -118,7 +111,6 @@ import { APIService } from './services/api';
 import { APIRoutesService } from './services/api-routes';
 import { AuthService } from './services/auth';
 import { AutosaveService } from './services/autosave';
-import { FeaturesService } from './services/features';
 import { FrameSyncService } from './services/frame-sync';
 import { GroupsService } from './services/groups';
 import { LoadAnnotationsService } from './services/load-annotations';
@@ -156,7 +148,6 @@ function startApp(settings, appEl) {
     .register('apiRoutes', APIRoutesService)
     .register('auth', AuthService)
     .register('autosaveService', AutosaveService)
-    .register('features', FeaturesService)
     .register('frameSync', FrameSyncService)
     .register('groups', GroupsService)
     .register('loadAnnotationsService', LoadAnnotationsService)

src/sidebar/services/features.js

-import { watch } from '../util/watch';
-
-/**
- * Service that provides operations related to feature flags.
- *
- * Feature flags information is part of the user's profile and in the sidebar
- * is accessed via the store. This service synchronizes the state of feature
- * flags to the `annotator` side of the application.
- *
- * Note that the state of feature flags can change whenever the active profile
- * information changes.
- *
- * @inject
- */
-export class FeaturesService {
-  /**
-   * @param {import('../services/frame-sync').FrameSyncService} frameSync
-   * @param {import('../store').SidebarStore} store
-   */
-  constructor(frameSync, store) {
-    this._frameSync = frameSync;
-    this._store = store;
-  }
-
-  init() {
-    watch(
-      this._store.subscribe,
-      () => this._store.profile().features,
-      flags => this._frameSync.notifyHost('featureFlagsUpdated', flags || {})
-    );
-  }
-}

src/sidebar/services/frame-sync.js

@@ -126,6 +126,7 @@ export class FrameSyncService {
 
     this._setupSyncToGuests();
     this._setupHostEvents();
+    this._setupFeatureFlagSync();
   }
 
   /**
@@ -364,6 +365,7 @@ export class FrameSyncService {
 
     // Synchronize highlight visibility in this guest with the sidebar's controls.
     guestRPC.call('setHighlightsVisible', this._highlightsVisible);
+    guestRPC.call('featureFlagsUpdated', this._store.profile().features);
   }
 
   /**
@@ -382,6 +384,28 @@ export class FrameSyncService {
     });
   }
 
+  /**
+   * Setup synchronization of feature flags to host and guest frames.
+   */
+  _setupFeatureFlagSync() {
+    const getFlags = () => this._store.profile().features;
+
+    /** @param {Record<string, boolean>} flags */
+    const sendFlags = flags => {
+      this._hostRPC.call('featureFlagsUpdated', flags);
+      for (let guest of this._guestRPC.values()) {
+        guest.call('featureFlagsUpdated', flags);
+      }
+    };
+
+    // Send current flags to host when it connects, and any already-connected
+    // guests.
+    sendFlags(getFlags());
+
+    // Watch for future flag changes.
+    watch(this._store.subscribe, getFlags, sendFlags);
+  }
+
   /**
    * Connect to the host frame and guest frame(s) in the current browser tab.
    */

src/sidebar/services/test/features-test.js

-import { FeaturesService } from '../features';
-
-describe('FeaturesService', () => {
-  let fakeFrameSync;
-  let fakeStore;
-
-  beforeEach(() => {
-    fakeFrameSync = {
-      notifyHost: sinon.stub(),
-    };
-
-    fakeStore = {
-      subscribe: sinon.stub(),
-      frames: sinon.stub().returns([]),
-      profile: sinon.stub().returns({
-        features: {
-          feature_on: true,
-          feature_off: false,
-        },
-      }),
-    };
-  });
-
-  function createService() {
-    const service = new FeaturesService(fakeFrameSync, fakeStore);
-    service.init();
-    return service;
-  }
-
-  function notifyStoreSubscribers() {
-    const subscribers = fakeStore.subscribe.args.map(args => args[0]);
-    subscribers.forEach(s => s());
-  }
-
-  it('should broadcast feature flags to annotator if flags change', () => {
-    createService();
-
-    // First update, with no changes to feature flags.
-    notifyStoreSubscribers();
-    assert.notCalled(fakeFrameSync.notifyHost);
-
-    // Second update, with changes to feature flags.
-    fakeStore.profile.returns({
-      features: {
-        feature_on: true,
-        feature_off: true,
-      },
-    });
-
-    notifyStoreSubscribers();
-
-    assert.calledWith(
-      fakeFrameSync.notifyHost,
-      'featureFlagsUpdated',
-      fakeStore.profile().features
-    );
-  });
-});

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

@@ -89,7 +89,7 @@ describe('FrameSyncService', () => {
     };
 
     fakeStore = fakeReduxStore(
-      { annotations: [], frames: [] },
+      { annotations: [], frames: [], profile: { features: {} } },
       {
         allAnnotations() {
           return this.getState().annotations;
@@ -115,6 +115,10 @@ describe('FrameSyncService', () => {
           return this.getState().frames;
         },
 
+        profile() {
+          return this.getState().profile;
+        },
+
         findIDsForTags: sinon.stub(),
         focusAnnotations: sinon.stub(),
         isLoggedIn: sinon.stub().returns(false),
@@ -737,4 +741,38 @@ describe('FrameSyncService', () => {
       assert.calledWith(hostRPC().call, 'openNotebook', 'group-id');
     });
   });
+
+  describe('sending feature flags to frames', () => {
+    const currentFlags = () => fakeStore.getState().profile.features;
+    const setFlags = features => fakeStore.setState({ profile: { features } });
+
+    beforeEach(async () => {
+      // Set some initial flags before the host frame is even connected.
+      setFlags({ some_flag: true, other_flag: false });
+
+      await frameSync.connect();
+    });
+
+    it('sends feature flags to host frame', () => {
+      assert.calledWith(hostRPC().call, 'featureFlagsUpdated', currentFlags());
+    });
+
+    it('sends feature flags to guest frames when they connect', async () => {
+      await connectGuest();
+      assert.calledWith(guestRPC().call, 'featureFlagsUpdated', currentFlags());
+    });
+
+    it('sends updated feature flags to host and guest frames', async () => {
+      await connectGuest();
+      hostRPC().call.resetHistory();
+      guestRPC().call.resetHistory();
+
+      // Simulate profile update changing feature flags.
+      const features = { some_flag: true, other_flag: true };
+      setFlags(features);
+
+      assert.calledWith(hostRPC().call, 'featureFlagsUpdated', currentFlags());
+      assert.calledWith(guestRPC().call, 'featureFlagsUpdated', currentFlags());
+    });
+  });
 });

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

@@ -112,6 +112,7 @@ export type SidebarToGuestEvent =
    * The sidebar is asking the guest(s) to delete an annotation.
    */
   | 'deleteAnnotation'
+  | 'featureFlagsUpdated'
 
   /**
    * The sidebar is asking the guest(s) to focus on certain annotations.