Commit 002d90f4 authored by Beth Rennie's avatar Beth Rennie Committed by brennie@mozilla.com
Browse files

Bug 1907633 - Do not instantiate the RemoteSettingsExperimentLoader on import... 

Bug 1907633 - Do not instantiate the RemoteSettingsExperimentLoader on import r=nimbus-reviewers,relud

Instead of creating the `RemoteSettingsExperimentLoader` by importing
`RemoteSettingsExperimentLoader.sys.mjs`, we instead create it the first
time we access the `ExperimentAPI._rsLoader` property.

The `RemoteSettingsExperimentLoader` is considered internal to Nimbus
and as such it is only exposed on the `_rsLoader` property so that other
Nimbus library code (such as `FirefoxLabs`) can access it.

Additionally, the `manager` argument to the
`RemoteSettingsExperimentLoader` is now required.

Differential Revision: https://phabricator.services.mozilla.com/D248074
parent 47f92726

toolkit/components/nimbus/ExperimentAPI.sys.mjs

+16 −5
Changes for toolkit/components/nimbus/ExperimentAPI.sys.mjs: 16 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -140,6 +140,7 @@ export const EnrollmentType = Object.freeze({ 


let initialized = false;

let experimentManager = null;

let experimentLoader = null;



export const ExperimentAPI = {

  /**
@@ -250,13 +251,27 @@ export const ExperimentAPI = { 
    return this.manager;

  },



  /**

   * Return the global RemoteSettingsExperimentLoader.

   */

  get _rsLoader() {

    if (experimentLoader === null) {

      experimentLoader = new lazy.RemoteSettingsExperimentLoader(this.manager);

    }



    return experimentLoader;

  },



  _resetForTests() {

    this._rsLoader.disable();

    experimentLoader?.disable();

    experimentLoader = null;



    lazy.CleanupManager.removeCleanupHandler(

      ExperimentAPI._removeCrashReportAnnotator

    );

    experimentManager?.store.off("update", this._annotateCrashReport);

    experimentManager = null;



    initialized = false;

  },
@@ -895,10 +910,6 @@ ExperimentAPI._onStudiesEnabledChanged = 
ExperimentAPI._removeCrashReportAnnotator =

  ExperimentAPI._removeCrashReportAnnotator.bind(ExperimentAPI);



ChromeUtils.defineLazyGetter(ExperimentAPI, "_rsLoader", function () {

  return lazy.RemoteSettingsExperimentLoader;

});



ChromeUtils.defineLazyGetter(

  ExperimentAPI,

  "_remoteSettingsClient",

toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs

+3 −7
Changes for toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs: 3 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -164,7 +164,7 @@ export const CheckRecipeResult = { 
  },

};



export class _RemoteSettingsExperimentLoader {

export class RemoteSettingsExperimentLoader {

  get LOCK_ID() {

    return "remote-settings-experiment-loader:update";

  }
@@ -174,6 +174,8 @@ export class _RemoteSettingsExperimentLoader { 
  }



  constructor(manager) {

    this.manager = manager;



    // Has the timer been set?

    this._enabled = false;

    // Are we in the middle of updating recipes already?
@@ -183,9 +185,6 @@ export class _RemoteSettingsExperimentLoader { 
    // deferred promise object that resolves after recipes are updated

    this._updatingDeferred = Promise.withResolvers();



    // Make it possible to override for testing

    this.manager = manager ?? lazy.ExperimentAPI.manager;



    this.remoteSettingsClients = {};

    ChromeUtils.defineLazyGetter(

      this.remoteSettingsClients,
@@ -1091,6 +1090,3 @@ export class EnrollmentsContext { 
    return schema;

  }

}
 


export const RemoteSettingsExperimentLoader =

  new _RemoteSettingsExperimentLoader();

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

+3 −3
Changes for toolkit/components/nimbus/test/NimbusTestUtils.sys.mjs: 3 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -16,7 +16,7 @@ ChromeUtils.defineESModuleGetters(lazy, { 
  JsonSchema: "resource://gre/modules/JsonSchema.sys.mjs",

  NetUtil: "resource://gre/modules/NetUtil.sys.mjs",

  ExperimentManager: "resource://nimbus/lib/ExperimentManager.sys.mjs",

  _RemoteSettingsExperimentLoader:

  RemoteSettingsExperimentLoader:

    "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs",

  sinon: "resource://testing-common/Sinon.sys.mjs",

});
@@ -282,7 +282,7 @@ export const NimbusTestUtils = { 
    },



    rsLoader(manager) {

      const loader = new lazy._RemoteSettingsExperimentLoader(

      const loader = new lazy.RemoteSettingsExperimentLoader(

        manager ?? NimbusTestUtils.stubs.manager()

      );
@@ -573,7 +573,7 @@ export const NimbusTestUtils = { 
   * @property {object} sandbox

   *           A sinon sandbox.

   *

   * @property {_RemoteSettingsExperimentLoader} loader

   * @property {RemoteSettingsExperimentLoader} loader

   *           A RemoteSettingsExperimentLoader instance that has stubbed

   *           RemoteSettings clients.

   *

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

+2 −3
Changes for toolkit/components/nimbus/test/browser/browser_experiment_evaluate_jexl.js: 2 added lines, 3 removed lines.
Original line number Diff line number Diff line 
"use strict";



const { EnrollmentsContext, RemoteSettingsExperimentLoader } =

  ChromeUtils.importESModule(

const { EnrollmentsContext } = ChromeUtils.importESModule(

  "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs"

);
@@ -17,7 +16,7 @@ add_setup(async function setup() { 
    await SpecialPowers.popPrefEnv();

  });



  CONTEXT = new EnrollmentsContext(RemoteSettingsExperimentLoader.manager);

  CONTEXT = new EnrollmentsContext(ExperimentAPI.manager);

});



let CONTEXT;

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

+4 −7
Changes for toolkit/components/nimbus/test/browser/browser_remotesettings_experiment_enroll.js: 4 added lines, 7 removed lines.
Original line number Diff line number Diff line
@@ -3,9 +3,6 @@ 
const { RemoteSettings } = ChromeUtils.importESModule(

  "resource://services-settings/remote-settings.sys.mjs"

);

const { RemoteSettingsExperimentLoader } = ChromeUtils.importESModule(

  "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs"

);



let rsClient;
@@ -22,7 +19,7 @@ add_setup(async function () { 
  });



  await ExperimentAPI.ready();

  await RemoteSettingsExperimentLoader.finishedUpdating();

  await ExperimentAPI._rsLoader.finishedUpdating();



  registerCleanupFunction(async () => {

    await SpecialPowers.popPrefEnv();
@@ -38,7 +35,7 @@ add_task(async function test_experimentEnrollment() { 
    clear: true,

  });



  await RemoteSettingsExperimentLoader.updateRecipes("mochitest");

  await ExperimentAPI._rsLoader.updateRecipes("mochitest");



  let meta = NimbusFeatures.testFeature.getEnrollmentMetadata();

  Assert.equal(meta.slug, recipe.slug, "Enrollment active");
@@ -58,11 +55,11 @@ add_task(async function test_experimentEnrollment_startup() { 
    set: [["app.shield.optoutstudies.enabled", false]],

  });



  Assert.ok(!RemoteSettingsExperimentLoader._enabled, "Should be disabled");

  Assert.ok(!ExperimentAPI._rsLoader._enabled, "Should be disabled");



  await SpecialPowers.pushPrefEnv({

    set: [["app.shield.optoutstudies.enabled", true]],

  });



  Assert.ok(RemoteSettingsExperimentLoader._enabled, "Should be enabled");

  Assert.ok(ExperimentAPI._rsLoader._enabled, "Should be enabled");

});