Bug 1943119 - Remove ExperimentAPI._store r=nalexander,nimbus-reviewers,emcminn

ExperimentAPI._store used to create a new, separate ExperimentStore in
the child process until bug 1941948 (which was unnecessary) and now only
exists for vestigial reasons. Use ExperimentAPI._manager.store
everywhere.

Differential Revision: https://phabricator.services.mozilla.com/D240414

browser/components/shell/test/browser_doesAppNeedPin.js

@@ -2,15 +2,10 @@
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
 ChromeUtils.defineESModuleGetters(this, {
-  ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs",
   ExperimentFakes: "resource://testing-common/NimbusTestUtils.sys.mjs",
   NimbusFeatures: "resource://nimbus/ExperimentAPI.sys.mjs",
 });
 
-registerCleanupFunction(() => {
-  ExperimentAPI._store._deleteForTests("shellService");
-});
-
 let defaultValue;
 add_task(async function default_need() {
   defaultValue = await ShellService.doesAppNeedPin();
@@ -47,8 +42,6 @@ add_task(async function restore_default() {
     return;
   }
 
-  ExperimentAPI._store._deleteForTests("shellService");
-
   Assert.equal(
     await ShellService.doesAppNeedPin(),
     defaultValue,

browser/components/shell/test/browser_setDefaultBrowser.js

@@ -33,8 +33,6 @@ const sendTriggerStub = sinon.stub(ASRouter, "sendTriggerMessage");
 
 registerCleanupFunction(() => {
   sinon.restore();
-
-  ExperimentAPI._store._deleteForTests("shellService");
 });
 
 let defaultUserChoice;
@@ -91,7 +89,6 @@ add_task(async function restore_default() {
 
   userChoiceStub.resetHistory();
   setDefaultStub.resetHistory();
-  ExperimentAPI._store._deleteForTests("shellService");
 
   await ShellService.setDefaultBrowser();
 

toolkit/components/nimbus/ExperimentAPI.sys.mjs

@@ -143,7 +143,7 @@ export const ExperimentAPI = {
    *          store
    */
   async ready() {
-    return this._store.ready();
+    return this._manager.store.ready();
   },
 
   async _annotateCrashReport() {
@@ -181,9 +181,9 @@ export const ExperimentAPI = {
     let experimentData;
     try {
       if (slug) {
-        experimentData = this._store.get(slug);
+        experimentData = this._manager.store.get(slug);
       } else if (featureId) {
-        experimentData = this._store.getExperimentForFeature(featureId);
+        experimentData = this._manager.store.getExperimentForFeature(featureId);
       }
     } catch (e) {
       console.error(e);
@@ -216,12 +216,13 @@ export const ExperimentAPI = {
     let experimentData;
     try {
       if (slug) {
-        experimentData = this._store.get(slug);
+        experimentData = this._manager.store.get(slug);
       } else if (featureId) {
         if (isRollout) {
-          experimentData = this._store.getRolloutForFeature(featureId);
+          experimentData = this._manager.store.getRolloutForFeature(featureId);
         } else {
-          experimentData = this._store.getExperimentForFeature(featureId);
+          experimentData =
+            this._manager.store.getExperimentForFeature(featureId);
         }
       }
     } catch (e) {
@@ -263,9 +264,9 @@ export const ExperimentAPI = {
     let experiment = null;
     try {
       if (slug) {
-        experiment = this._store.get(slug);
+        experiment = this._manager.store.get(slug);
       } else if (featureId) {
-        experiment = this._store.getExperimentForFeature(featureId);
+        experiment = this._manager.store.getExperimentForFeature(featureId);
       }
     } catch (e) {
       console.error(e);
@@ -286,7 +287,7 @@ export const ExperimentAPI = {
    * This should noly be called from the main process.
    *
    * Note that the recipe is directly fetched from RemoteSettings, which has
-   * all the recipe metadata available without relying on the `this._store`.
+   * all the recipe metadata available without relying on the `this._manager.store`.
    * Therefore, calling this function does not require to call `this.ready()` first.
    *
    * @param slug {String} An experiment identifier
@@ -387,7 +388,7 @@ export class _ExperimentFeature {
           fallbackPref,
           null,
           () => {
-            ExperimentAPI._store._emitFeatureUpdate(
+            ExperimentAPI._manager.store._emitFeatureUpdate(
               this.featureId,
               "pref-updated"
             );
@@ -428,7 +429,9 @@ export class _ExperimentFeature {
   getAllVariables({ defaultValues = null } = {}) {
     let enrollment = null;
     try {
-      enrollment = ExperimentAPI._store.getExperimentForFeature(this.featureId);
+      enrollment = ExperimentAPI._manager.store.getExperimentForFeature(
+        this.featureId
+      );
     } catch (e) {
       console.error(e);
     }
@@ -436,7 +439,9 @@ export class _ExperimentFeature {
 
     if (typeof featureValue === "undefined") {
       try {
-        enrollment = ExperimentAPI._store.getRolloutForFeature(this.featureId);
+        enrollment = ExperimentAPI._manager.store.getRolloutForFeature(
+          this.featureId
+        );
       } catch (e) {
         console.error(e);
       }
@@ -463,7 +468,9 @@ export class _ExperimentFeature {
     // Next, check if an experiment is defined
     let enrollment = null;
     try {
-      enrollment = ExperimentAPI._store.getExperimentForFeature(this.featureId);
+      enrollment = ExperimentAPI._manager.store.getExperimentForFeature(
+        this.featureId
+      );
     } catch (e) {
       console.error(e);
     }
@@ -474,7 +481,9 @@ export class _ExperimentFeature {
 
     // Next, check for a rollout.
     try {
-      enrollment = ExperimentAPI._store.getRolloutForFeature(this.featureId);
+      enrollment = ExperimentAPI._manager.store.getRolloutForFeature(
+        this.featureId
+      );
     } catch (e) {
       console.error(e);
     }
@@ -489,7 +498,7 @@ export class _ExperimentFeature {
   }
 
   getRollout() {
-    let remoteConfig = ExperimentAPI._store.getRolloutForFeature(
+    let remoteConfig = ExperimentAPI._manager.store.getRolloutForFeature(
       this.featureId
     );
     if (!remoteConfig) {
@@ -536,11 +545,11 @@ export class _ExperimentFeature {
   }
 
   onUpdate(callback) {
-    ExperimentAPI._store._onFeatureUpdate(this.featureId, callback);
+    ExperimentAPI._manager.store._onFeatureUpdate(this.featureId, callback);
   }
 
   offUpdate(callback) {
-    ExperimentAPI._store._offFeatureUpdate(this.featureId, callback);
+    ExperimentAPI._manager.store._offFeatureUpdate(this.featureId, callback);
   }
 
   /**
@@ -731,11 +740,6 @@ ChromeUtils.defineLazyGetter(ExperimentAPI, "_manager", function () {
   return lazy.ExperimentManager;
 });
 
-Object.defineProperty(ExperimentAPI, "_store", {
-  configurable: true,
-  get: () => ExperimentAPI._manager.store,
-});
-
 ChromeUtils.defineLazyGetter(ExperimentAPI, "_rsLoader", function () {
   return lazy.RemoteSettingsExperimentLoader;
 });

toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs

@@ -189,7 +189,7 @@ export const ExperimentFakes = {
   },
   waitForExperimentUpdate(ExperimentAPI, slug) {
     return new Promise(resolve =>
-      ExperimentAPI._store.once(`update:${slug}`, resolve)
+      ExperimentAPI._manager.store.once(`update:${slug}`, resolve)
     );
   },
   /**

toolkit/components/nimbus/test/browser/browser_remotesettings_experiment_enroll.js

@@ -71,7 +71,7 @@ add_task(async function test_experimentEnrollment() {
   });
 
   Assert.ok(!experiment.active, "Experiment is no longer active");
-  ExperimentAPI._store._deleteForTests(recipe.slug);
+  ExperimentAPI._manager.store._deleteForTests(recipe.slug);
 });
 
 add_task(async function test_experimentEnrollment_startup() {

toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js

@@ -252,10 +252,10 @@ add_task(async function test_remote_fetch_and_ready() {
   );
   Assert.ok(!barInstance.getVariable("remoteValue"), "Should be missing");
 
-  ExperimentAPI._store._deleteForTests("foo");
-  ExperimentAPI._store._deleteForTests("bar");
-  ExperimentAPI._store._deleteForTests(REMOTE_CONFIGURATION_FOO.slug);
-  ExperimentAPI._store._deleteForTests(REMOTE_CONFIGURATION_BAR.slug);
+  ExperimentAPI._manager.store._deleteForTests("foo");
+  ExperimentAPI._manager.store._deleteForTests("bar");
+  ExperimentAPI._manager.store._deleteForTests(REMOTE_CONFIGURATION_FOO.slug);
+  ExperimentAPI._manager.store._deleteForTests(REMOTE_CONFIGURATION_BAR.slug);
   sandbox.restore();
 
   cleanupTestFeatures();
@@ -413,8 +413,8 @@ add_task(async function test_finalizeRemoteConfigs_cleanup() {
   // This will also remove the inactive recipe from the store
   // the previous update (from recipe not seen code path)
   // only sets the recipe as inactive
-  ExperimentAPI._store._deleteForTests("bar-rollout");
-  ExperimentAPI._store._deleteForTests("foo-rollout");
+  ExperimentAPI._manager.store._deleteForTests("bar-rollout");
+  ExperimentAPI._manager.store._deleteForTests("foo-rollout");
 
   cleanupTestFeatures();
   cleanup();
@@ -424,7 +424,7 @@ add_task(async function test_finalizeRemoteConfigs_cleanup() {
 // this test should not throw
 add_task(async function remote_defaults_no_mutation() {
   let sandbox = sinon.createSandbox();
-  sandbox.stub(ExperimentAPI._store, "getRolloutForFeature").returns(
+  sandbox.stub(ExperimentAPI._manager.store, "getRolloutForFeature").returns(
     Cu.cloneInto(
       {
         featureIds: ["foo"],
@@ -446,8 +446,8 @@ add_task(async function remote_defaults_no_mutation() {
 });
 
 add_task(async function remote_defaults_active_remote_defaults() {
-  ExperimentAPI._store._deleteForTests("foo");
-  ExperimentAPI._store._deleteForTests("bar");
+  ExperimentAPI._manager.store._deleteForTests("foo");
+  ExperimentAPI._manager.store._deleteForTests("bar");
   let barFeature = new ExperimentFeature("bar", {
     description: "mochitest",
     variables: { enabled: { type: "boolean" } },
@@ -514,8 +514,8 @@ add_task(async function remote_defaults_active_remote_defaults() {
   await featureUpdate;
 
   Assert.ok(fooFeature.getVariable("enabled"), "Targeting should match");
-  ExperimentAPI._store._deleteForTests("foo");
-  ExperimentAPI._store._deleteForTests("bar");
+  ExperimentAPI._manager.store._deleteForTests("foo");
+  ExperimentAPI._manager.store._deleteForTests("bar");
 
   cleanup();
   cleanupTestFeatures();
@@ -586,6 +586,6 @@ add_task(async function remote_defaults_variables_storage() {
     "Variable pref is cleared"
   );
   Assert.ok(!barFeature.getVariable("string"), "Variable is no longer defined");
-  ExperimentAPI._store._deleteForTests("bar");
-  ExperimentAPI._store._deleteForTests("bar-rollout");
+  ExperimentAPI._manager.store._deleteForTests("bar");
+  ExperimentAPI._manager.store._deleteForTests("bar-rollout");
 });

toolkit/components/nimbus/test/unit/test_ExperimentAPI.js

@@ -107,8 +107,10 @@ add_task(function test_getExperimentMetaData_safe() {
   const sandbox = sinon.createSandbox();
   let exposureStub = sandbox.stub(ExperimentAPI, "recordExposureEvent");
 
-  sandbox.stub(ExperimentAPI._store, "get").throws();
-  sandbox.stub(ExperimentAPI._store, "getExperimentForFeature").throws();
+  sandbox.stub(ExperimentAPI._manager.store, "get").throws();
+  sandbox
+    .stub(ExperimentAPI._manager.store, "getExperimentForFeature")
+    .throws();
 
   try {
     let metadata = ExperimentAPI.getExperimentMetaData({ slug: "foo" });
@@ -117,7 +119,7 @@ add_task(function test_getExperimentMetaData_safe() {
     Assert.ok(false, "Error should be caught in ExperimentAPI");
   }
 
-  Assert.ok(ExperimentAPI._store.get.calledOnce, "Sanity check");
+  Assert.ok(ExperimentAPI._manager.store.get.calledOnce, "Sanity check");
 
   try {
     let metadata = ExperimentAPI.getExperimentMetaData({ featureId: "foo" });
@@ -127,7 +129,7 @@ add_task(function test_getExperimentMetaData_safe() {
   }
 
   Assert.ok(
-    ExperimentAPI._store.getExperimentForFeature.calledOnce,
+    ExperimentAPI._manager.store.getExperimentForFeature.calledOnce,
     "Sanity check"
   );
 
@@ -138,7 +140,9 @@ add_task(function test_getExperimentMetaData_safe() {
 
 add_task(async function test_getExperiment_safe() {
   const sandbox = sinon.createSandbox();
-  sandbox.stub(ExperimentAPI._store, "getExperimentForFeature").throws();
+  sandbox
+    .stub(ExperimentAPI._manager.store, "getExperimentForFeature")
+    .throws();
 
   try {
     Assert.equal(
@@ -163,7 +167,7 @@ add_task(async function test_getExperiment_featureAccess() {
     },
   });
   const stub = sandbox
-    .stub(ExperimentAPI._store, "getExperimentForFeature")
+    .stub(ExperimentAPI._manager.store, "getExperimentForFeature")
     .returns(expected);
 
   let { branch } = ExperimentAPI.getExperiment({ featureId: "cfr" });
@@ -185,7 +189,7 @@ add_task(async function test_getExperiment_featureAccess_backwardsCompat() {
     },
   });
   const stub = sandbox
-    .stub(ExperimentAPI._store, "getExperimentForFeature")
+    .stub(ExperimentAPI._manager.store, "getExperimentForFeature")
     .returns(expected);
 
   let { branch } = ExperimentAPI.getExperiment({ featureId: "cfr" });
@@ -477,7 +481,9 @@ add_task(async function test_getActiveBranch() {
 
 add_task(async function test_getActiveBranch_safe() {
   const sandbox = sinon.createSandbox();
-  sandbox.stub(ExperimentAPI._store, "getAllActiveExperiments").throws();
+  sandbox
+    .stub(ExperimentAPI._manager.store, "getAllActiveExperiments")
+    .throws();
 
   try {
     Assert.equal(