Bug 985084 - Experiment add-ons should be disabled by default; r=Unfocused

Experiment add-ons are now disabled by default on application load. It
is up to the Experiments Manager to enable them.

This means that experiments may not be able to reliably collect data or
modify behavior close to application startup. (There is a window between
when the Addon Manager initializes and when the Experiments Manager
initializes.) This window is acceptable for the initial version of the
experiments feature.

The Experiments Manager doesn't currently enable experiments on startup.
This will be addressed in a subsequent patch. Its tests do not regress
(indicating a lack of test coverage), so no harm no foul.

browser/experiments/Experiments.jsm

@@ -611,6 +611,8 @@ Experiments.Experiments.prototype = {
     return this._run();
   },
 
+  // START OF ADD-ON LISTENERS
+
   onDisabled: function (addon) {
     gLogger.trace("Experiments::onDisabled() - addon id: " + addon.id);
     if (addon.id == this._pendingUninstall) {
@@ -636,6 +638,8 @@ Experiments.Experiments.prototype = {
     this.disableExperiment();
   },
 
+  // END OF ADD-ON LISTENERS.
+
   _getExperimentByAddonId: function (addonId) {
     for (let [, entry] of this._experiments) {
       if (entry._addonId === addonId) {
@@ -1402,6 +1406,9 @@ Experiments.ExperimentEntry.prototype = {
           gLogger.error("ExperimentEntry::_installAddon() - onInstallStarted, wrong addon type");
           return false;
         }
+
+        // Experiment add-ons default to userDisabled = true.
+        install.addon.userDisabled = false;
       }.bind(entry),
 
       onInstallEnded: function (install) {

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -908,6 +908,11 @@ function loadManifestFromRDF(aUri, aStream) {
     addon.userDisabled = !!LightweightThemeManager.currentTheme ||
                          addon.internalName != XPIProvider.selectedSkin;
   }
+  // Experiments are disabled by default. It is up to the Experiments Manager
+  // to enable them (it drives installation).
+  else if (addon.type == "experiment") {
+    addon.userDisabled = true;
+  }
   else {
     addon.userDisabled = false;
     addon.softDisabled = addon.blocklistState == Ci.nsIBlocklistService.STATE_SOFTBLOCKED;
@@ -2082,8 +2087,19 @@ var XPIProvider = {
    * Persists changes to XPIProvider.bootstrappedAddons to its store (a pref).
    */
   persistBootstrappedAddons: function XPI_persistBootstrappedAddons() {
+    // Experiments are disabled upon app load, so don't persist references.
+    let filtered = {};
+    for (let id in this.bootstrappedAddons) {
+      let entry = this.bootstrappedAddons[id];
+      if (entry.type == "experiment") {
+        continue;
+      }
+
+      filtered[id] = entry;
+    }
+
     Services.prefs.setCharPref(PREF_BOOTSTRAP_ADDONS,
-                               JSON.stringify(this.bootstrappedAddons));
+                               JSON.stringify(filtered));
   },
 
   /**
@@ -4238,12 +4254,16 @@ var XPIProvider = {
     // no onDisabling/onEnabling is sent - so send a onPropertyChanged.
     let appDisabledChanged = aAddon.appDisabled != appDisabled;
 
-    // Update the properties in the database
-    XPIDatabase.setAddonProperties(aAddon, {
-      userDisabled: aUserDisabled,
-      appDisabled: appDisabled,
-      softDisabled: aSoftDisabled
-    });
+    // Update the properties in the database.
+    // We never persist this for experiments because the disabled flags
+    // are controlled by the Experiments Manager.
+    if (aAddon.type != "experiment") {
+      XPIDatabase.setAddonProperties(aAddon, {
+        userDisabled: aUserDisabled,
+        appDisabled: appDisabled,
+        softDisabled: aSoftDisabled
+      });
+    }
 
     if (appDisabledChanged) {
       AddonManagerPrivate.callAddonListeners("onPropertyChanged",

toolkit/mozapps/extensions/test/browser/browser_experiments.js

@@ -50,7 +50,8 @@ add_test(function testActiveExperiment() {
   install_addon("addons/browser_experiment1.xpi", (addon) => {
     gInstalledAddons.push(addon);
 
-    Assert.ok(addon.isActive, "Add-on is active.");
+    Assert.ok(addon.userDisabled, "Add-on is disabled upon initial install.");
+    Assert.equal(addon.isActive, false, "Add-on is not active.");
 
     Assert.ok(gCategoryUtilities.isTypeVisible("experiment"), "Experiment tab visible.");
 
@@ -143,6 +144,9 @@ add_test(function testButtonPresence() {
     // Corresponds to lack of disable permission.
     el = item.ownerDocument.getAnonymousElementByAttribute(item, "anonid", "disable-btn");
     is_element_hidden(el, "Disable button not visible.");
+    // Corresponds to lack of enable permission.
+    el = item.ownerDocument.getAnonymousElementByAttribute(item, "anonid", "enable-btn");
+    is_element_hidden(el, "Enable button not visible.");
 
     run_next_test();
   });

toolkit/mozapps/extensions/test/xpcshell/test_experiment.js

@@ -1,6 +1,9 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 
+let scope = Components.utils.import("resource://gre/modules/addons/XPIProvider.jsm");
+const XPIProvider = scope.XPIProvider;
+
 function run_test() {
   createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
   startupManager();
@@ -14,7 +17,8 @@ add_test(function test_experiment() {
       AddonManager.getAddonByID("experiment1@tests.mozilla.org", (addon) => {
         Assert.ok(addon, "Addon is found.");
 
-        Assert.ok(addon.isActive, "Add-on is active.");
+        Assert.ok(addon.userDisabled, "Experiments are userDisabled by default.");
+        Assert.equal(addon.isActive, false, "Add-on is not active.");
         Assert.equal(addon.updateURL, null, "No updateURL for experiments.");
         Assert.equal(addon.applyBackgroundUpdates, AddonManager.AUTOUPDATE_DISABLE,
                      "Background updates are disabled.");
@@ -46,3 +50,40 @@ add_test(function test_experiment() {
     });
   });
 });
+
+// Changes to userDisabled should not be persisted to the database.
+add_test(function test_userDisabledNotPersisted() {
+  AddonManager.getAddonByID("experiment1@tests.mozilla.org", (addon) => {
+    Assert.ok(addon, "Addon is found.");
+
+    let listener = {
+      onEnabled: (addon2) => {
+        Assert.equal(addon2.id, addon.id, "Changed add-on matches expected.");
+        Assert.ok(addon2.isActive, "Add-on is no longer disabled.");
+
+        Assert.ok("experiment1@tests.mozilla.org" in XPIProvider.bootstrappedAddons,
+                  "Experiment add-on listed in XPIProvider bootstrapped list.");
+
+        AddonManager.getAddonByID("experiment1@tests.mozilla.org", (addon) => {
+          Assert.ok(addon, "Add-on retrieved.");
+          Assert.ok(addon.userDisabled, "Add-on is disabled according to database.");
+
+          restartManager();
+          let persisted = JSON.parse(Services.prefs.getCharPref("extensions.bootstrappedAddons"));
+          Assert.ok(!("experiment1@tests.mozilla.org" in persisted),
+                    "Experiment add-on not persisted to bootstrappedAddons.");
+
+          AddonManager.getAddonByID("experiment1@tests.mozilla.org", (addon) => {
+            Assert.ok(addon, "Add-on retrieved.");
+            Assert.ok(addon.userDisabled, "Add-on is disabled after restart.");
+
+            run_next_test();
+          });
+        });
+      },
+    };
+
+    AddonManager.addAddonListener(listener);
+    addon.userDisabled = false;
+  });
+});