Bug 1659152 - Use ExperimentAPI to check enabled property of experiment branches r=k88hudson,andreio

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

browser/actors/AboutNewTabChild.jsm

@@ -13,6 +13,9 @@ const { XPCOMUtils } = ChromeUtils.import(
 const { AppConstants } = ChromeUtils.import(
   "resource://gre/modules/AppConstants.jsm"
 );
+const { ExperimentAPI } = ChromeUtils.import(
+  "resource://messaging-system/experiments/ExperimentAPI.jsm"
+);
 
 XPCOMUtils.defineLazyPreferenceGetter(
   this,
@@ -23,7 +26,7 @@ XPCOMUtils.defineLazyPreferenceGetter(
 
 XPCOMUtils.defineLazyPreferenceGetter(
   this,
-  "SEPARATE_ABOUT_WELCOME",
+  "isAboutWelcomePrefEnabled",
   "browser.aboutwelcome.enabled",
   false
 );
@@ -34,7 +37,9 @@ class AboutNewTabChild extends JSWindowActorChild {
       // If the separate about:welcome page is enabled, we can skip all of this,
       // since that mode doesn't load any of the Activity Stream bits.
       if (
-        SEPARATE_ABOUT_WELCOME &&
+        isAboutWelcomePrefEnabled &&
+        // about:welcome should be enabled by default if no experiment exists.
+        ExperimentAPI.isFeatureEnabled("aboutwelcome", true) &&
         this.contentWindow.location.pathname.includes("welcome")
       ) {
         return;

browser/components/newtab/AboutNewTabService.jsm

@@ -41,6 +41,10 @@ const { E10SUtils } = ChromeUtils.import(
   "resource://gre/modules/E10SUtils.jsm"
 );
 
+const { ExperimentAPI } = ChromeUtils.import(
+  "resource://messaging-system/experiments/ExperimentAPI.jsm"
+);
+
 /**
  * BEWARE: Do not add variables for holding state in the global scope.
  * Any state variables should be properties of the appropriate class
@@ -54,8 +58,8 @@ const PREF_ABOUT_HOME_CACHE_ENABLED =
   "browser.startup.homepage.abouthome_cache.enabled";
 const PREF_ABOUT_HOME_CACHE_TESTING =
   "browser.startup.homepage.abouthome_cache.testing";
-const PREF_SEPARATE_ABOUT_WELCOME = "browser.aboutwelcome.enabled";
-const SEPARATE_ABOUT_WELCOME_URL =
+const PREF_ABOUT_WELCOME_ENABLED = "browser.aboutwelcome.enabled";
+const ABOUT_WELCOME_URL =
   "resource://activity-stream/aboutwelcome/aboutwelcome.html";
 
 ChromeUtils.defineModuleGetter(
@@ -297,8 +301,8 @@ class BaseAboutNewTabService {
 
     XPCOMUtils.defineLazyPreferenceGetter(
       this,
-      "isSeparateAboutWelcome",
-      PREF_SEPARATE_ABOUT_WELCOME,
+      "isAboutWelcomePrefEnabled",
+      PREF_ABOUT_WELCOME_ENABLED,
       false
     );
 
@@ -339,14 +343,19 @@ class BaseAboutNewTabService {
     ].join("");
   }
 
-  /*
-   * Returns the about:welcome URL
-   *
-   * This is calculated in the same way the default URL is.
-   */
   get welcomeURL() {
-    if (this.isSeparateAboutWelcome) {
-      return SEPARATE_ABOUT_WELCOME_URL;
+    /*
+     * Returns the about:welcome URL
+     *
+     * This is calculated in the same way the default URL is.
+     */
+
+    if (
+      this.isAboutWelcomePrefEnabled &&
+      // about:welcome should be enabled by default if no experiment exists.
+      ExperimentAPI.isFeatureEnabled("aboutwelcome", true)
+    ) {
+      return ABOUT_WELCOME_URL;
     }
     return this.defaultURL;
   }

browser/components/newtab/test/browser/browser_aboutwelcome_multistage.js

@@ -178,6 +178,54 @@ async function onButtonClick(browser, elementId) {
   );
 }
 
+/**
+ * Test the zero onboarding using ExperimentAPI
+ */
+add_task(async function test_multistage_zeroOnboarding_experimentAPI() {
+  await setAboutWelcomePref(true);
+  let updatePromise = new Promise(resolve =>
+    ExperimentAPI._store.on("update:mochitest-1-aboutwelcome", resolve)
+  );
+  ExperimentAPI._store.addExperiment({
+    slug: "mochitest-1-aboutwelcome",
+    branch: {
+      slug: "mochitest-1-aboutwelcome",
+      feature: {
+        enabled: false,
+        featureId: "aboutwelcome",
+        value: null,
+      },
+    },
+    active: true,
+  });
+
+  await updatePromise;
+  ExperimentAPI._store._syncToChildren({ flush: true });
+
+  let tab = await BrowserTestUtils.openNewForegroundTab(
+    gBrowser,
+    "about:welcome",
+    true
+  );
+  registerCleanupFunction(() => {
+    BrowserTestUtils.removeTab(tab);
+  });
+
+  const browser = tab.linkedBrowser;
+
+  await test_screen_content(
+    browser,
+    "Opens new tab",
+    // Expected selectors:
+    ["div.search-wrapper", "body.activity-stream"],
+    // Unexpected selectors:
+    ["div.multistageContainer", "main.AW_STEP1"]
+  );
+
+  ExperimentAPI._store._deleteForTests("mochitest-1-aboutwelcome");
+  Assert.equal(ExperimentAPI._store.getAll().length, 0, "Cleanup done");
+});
+
 /**
  * Test the multistage welcome UI using ExperimentAPI
  */
@@ -192,6 +240,7 @@ add_task(async function test_multistage_aboutwelcome_experimentAPI() {
     branch: {
       slug: "mochitest-aboutwelcome",
       feature: {
+        enabled: true,
         featureId: "aboutwelcome",
         value: TEST_MULTISTAGE_CONTENT,
       },

toolkit/components/messaging-system/experiments/ExperimentAPI.jsm

@@ -56,10 +56,19 @@ const ExperimentAPI = {
   /**
    * Lookup feature in active experiments and return status.
    * @param {string} featureId Feature to lookup
+   * @param {boolean} defaultValue
    * @returns {boolean}
    */
-  isFeatureEnabled(featureId) {
-    return this._store.getFeature(featureId)?.enabled;
+  isFeatureEnabled(featureId, defaultValue) {
+    try {
+      const featureConfig = this._store.getFeature(featureId);
+      if (featureConfig && featureConfig.enabled !== undefined) {
+        return featureConfig.enabled;
+      }
+    } catch (e) {
+      Cu.reportError(e);
+    }
+    return defaultValue;
   },
 
   /**

toolkit/components/messaging-system/experiments/ExperimentStore.jsm

@@ -74,7 +74,12 @@ class ExperimentStore extends SharedDataMap {
    * @returns {Enrollment[]}
    */
   getAll() {
-    return Object.values(this._data);
+    try {
+      return Object.values(this._data);
+    } catch (e) {
+      Cu.reportError(e);
+    }
+    return [];
   }
 
   /**

toolkit/components/messaging-system/test/unit/test_ExperimentAPI.js

@@ -6,9 +6,6 @@ const { ExperimentAPI } = ChromeUtils.import(
 const { ExperimentFakes } = ChromeUtils.import(
   "resource://testing-common/MSTestUtils.jsm"
 );
-const { FileTestUtils } = ChromeUtils.import(
-  "resource://testing-common/FileTestUtils.jsm"
-);
 const { TestUtils } = ChromeUtils.import(
   "resource://testing-common/TestUtils.jsm"
 );
@@ -115,6 +112,58 @@ add_task(async function test_getValue() {
   sandbox.restore();
 });
 
+/**
+ * #isFeatureEnabled
+ */
+
+add_task(async function test_isFeatureEnabledDefault() {
+  const sandbox = sinon.createSandbox();
+  const manager = ExperimentFakes.manager();
+  const FEATURE_ENABLED_DEFAULT = true;
+  const expected = ExperimentFakes.experiment("foo", {
+    branch: { slug: "treatment" },
+  });
+
+  await manager.onStartup();
+
+  sandbox.stub(ExperimentAPI, "_store").get(() => manager.store);
+
+  manager.store.addExperiment(expected);
+
+  Assert.deepEqual(
+    ExperimentAPI.isFeatureEnabled("aboutwelcome", FEATURE_ENABLED_DEFAULT),
+    FEATURE_ENABLED_DEFAULT,
+    "should return enabled true as default"
+  );
+  sandbox.restore();
+});
+
+add_task(async function test_isFeatureEnabled() {
+  const sandbox = sinon.createSandbox();
+  const manager = ExperimentFakes.manager();
+  const feature = {
+    featureId: "aboutwelcome",
+    enabled: false,
+    value: null,
+  };
+  const expected = ExperimentFakes.experiment("foo", {
+    branch: { slug: "treatment", feature },
+  });
+
+  await manager.onStartup();
+
+  sandbox.stub(ExperimentAPI, "_store").get(() => manager.store);
+
+  manager.store.addExperiment(expected);
+
+  Assert.deepEqual(
+    ExperimentAPI.isFeatureEnabled("aboutwelcome", true),
+    feature.enabled,
+    "should return feature as disabled"
+  );
+  sandbox.restore();
+});
+
 /**
  * #getRecipe
  */