Bug 989137 - Part 8: Prevent unknown experiments from being installed; r=Unfocused

The experiments service insists on being in control of experiments.
Before, it wasn't being as assertive as it needed to be: other browser
components could install experiments behind its back. This patch
reasserts the experiments service as king of experiments add-on
management.

--HG--
extra : rebase_source : 91439622e47419fae7675d83890e83cf37db453b

browser/experiments/Experiments.jsm

@@ -98,6 +98,10 @@ let gPolicyCounter = 0;
 let gExperimentsCounter = 0;
 let gExperimentEntryCounter = 0;
 
+// Tracks active AddonInstall we know about so we can deny external
+// installs.
+let gActiveInstallURLs = new Set();
+
 let gLogger;
 let gLogDumping = false;
 
@@ -359,7 +363,7 @@ Experiments.Experiments.prototype = {
     AsyncShutdown.profileBeforeChange.addBlocker("Experiments.jsm shutdown",
       this.uninit.bind(this));
 
-    AddonManager.addAddonListener(this);
+    this._startWatchingAddons();
 
     this._loadTask = Task.spawn(this._loadFromCache.bind(this));
     this._loadTask.then(
@@ -380,7 +384,7 @@ Experiments.Experiments.prototype = {
    */
   uninit: function () {
     if (!this._shutdown) {
-      AddonManager.removeAddonListener(this);
+      this._stopWatchingAddons();
 
       gPrefs.ignore(PREF_LOGGING, configureLogging);
       gPrefs.ignore(PREF_MANIFEST_URI, this.updateManifest, this);
@@ -400,6 +404,16 @@ Experiments.Experiments.prototype = {
     return Promise.resolve();
   },
 
+  _startWatchingAddons: function () {
+    AddonManager.addAddonListener(this);
+    AddonManager.addInstallListener(this);
+  },
+
+  _stopWatchingAddons: function () {
+    AddonManager.removeInstallListener(this);
+    AddonManager.removeAddonListener(this);
+  },
+
   /**
    * Throws an exception if we've already shut down.
    */
@@ -644,6 +658,42 @@ Experiments.Experiments.prototype = {
     this.disableExperiment();
   },
 
+  onInstallStarted: function (install) {
+    if (install.addon.type != "experiment") {
+      return;
+    }
+
+    // We want to be in control of all experiment add-ons: reject installs
+    // for add-ons that we don't know about.
+
+    // We have a race condition of sorts to worry about here. We have 2
+    // onInstallStarted listeners. This one (the global one) and the one
+    // created as part of ExperimentEntry._installAddon. Because of the order
+    // they are registered in, this one likely executes first. Unfortunately,
+    // this means that the add-on ID is not yet set on the ExperimentEntry.
+    // So, we can't just look at this._trackedAddonIds because the new experiment
+    // will have its add-on ID set to null. We work around this by storing a
+    // identifying field - the source URL of the install - in a module-level
+    // variable (so multiple Experiments instances doesn't cancel each other
+    // out).
+
+    if (this._trackedAddonIds.has(install.addon.id)) {
+      this._log.info("onInstallStarted allowing install because add-on ID " +
+                     "tracked by us.");
+      return;
+    }
+
+    if (gActiveInstallURLs.has(install.sourceURI.spec)) {
+      this._log.info("onInstallStarted allowing install because install " +
+                     "tracked by us.");
+      return;
+    }
+
+    this._log.warn("onInstallStarted cancelling install of unknown " +
+                   "experiment add-on: " + install.addon.id);
+    return false;
+  },
+
   // END OF ADD-ON LISTENERS.
 
   _getExperimentByAddonId: function (addonId) {
@@ -851,6 +901,17 @@ Experiments.Experiments.prototype = {
     return this._run();
   },
 
+  /**
+   * The Set of add-on IDs that we know about from manifests.
+   */
+  get _trackedAddonIds() {
+    if (!this._experiments) {
+      return new Set();
+    }
+
+    return new Set([e._addonId for ([,e] of this._experiments) if (e._addonId)]);
+  },
+
   /*
    * Task function to check applicability of experiments, disable the active
    * experiment if needed and activate the first applicable candidate.
@@ -875,7 +936,7 @@ Experiments.Experiments.prototype = {
     // should have some record of it. In the end, we decide to discard all
     // knowledge for these unknown experiment add-ons.
     let installedExperiments = yield installedExperimentAddons();
-    let expectedAddonIds = new Set([e._addonId for ([,e] of this._experiments)]);
+    let expectedAddonIds = this._trackedAddonIds;
     let unknownAddons = [a for (a of installedExperiments) if (!expectedAddonIds.has(a.id))];
     if (unknownAddons.length) {
       this._log.warn("_evaluateExperiments() - unknown add-ons in AddonManager: " +
@@ -1402,11 +1463,14 @@ Experiments.ExperimentEntry.prototype = {
 
     let install = yield addonInstallForURL(this._manifestData.xpiURL,
                                            this._manifestData.xpiHash);
+    gActiveInstallURLs.add(install.sourceURI.spec);
+
     let failureHandler = (install, handler) => {
       let message = "AddonInstall " + handler + " for " + this.id + ", state=" +
                    (install.state || "?") + ", error=" + install.error;
       this._log.error("_installAddon() - " + message);
       this._failedStart = true;
+      gActiveInstallURLs.delete(install.sourceURI.spec);
 
       TelemetryLog.log(TELEMETRY_LOG.ACTIVATION_KEY,
                       [TELEMETRY_LOG.ACTIVATION.INSTALL_FAILURE, this.id]);
@@ -1446,6 +1510,8 @@ Experiments.ExperimentEntry.prototype = {
 
       onInstallEnded: install => {
         this._log.trace("_installAddon() - install ended for " + this.id);
+        gActiveInstallURLs.delete(install.sourceURI.spec);
+
         this._lastChangedDate = this._policy.now();
         this._startDate = this._policy.now();
         this._enabled = true;

browser/experiments/test/xpcshell/test_activate.js

@@ -86,6 +86,12 @@ add_task(function* test_startStop() {
   });
   let experiment = new Experiments.ExperimentEntry(gPolicy);
   experiment.initFromManifestData(manifestData);
+
+  // We need to associate it with the singleton so the onInstallStarted
+  // Addon Manager listener will know about it.
+  Experiments.instance()._experiments = new Map();
+  Experiments.instance()._experiments.set(experiment.id, experiment);
+
   let result;
 
   defineNow(gPolicy, baseDate);

browser/experiments/test/xpcshell/test_api.js

@@ -151,6 +151,8 @@ add_task(function* test_getExperiments() {
                "Experiments observer should not have been called yet.");
   let list = yield experiments.getExperiments();
   Assert.equal(list.length, 0, "Experiment list should be empty.");
+  let addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "Precondition: No experiment add-ons are installed.");
 
   // Trigger update, clock set for experiment 1 to start.
 
@@ -164,6 +166,8 @@ add_task(function* test_getExperiments() {
 
   list = yield experiments.getExperiments();
   Assert.equal(list.length, 1, "Experiment list should have 1 entry now.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "An experiment add-on was installed.");
 
   experimentListData[1].active = true;
   experimentListData[1].endDate = now.getTime() + 10 * MS_IN_ONE_DAY;
@@ -187,6 +191,8 @@ add_task(function* test_getExperiments() {
 
   list = yield experiments.getExperiments();
   Assert.equal(list.length, 1, "Experiment list should have 1 entry.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "The experiment add-on should be uninstalled.");
 
   experimentListData[1].active = false;
   experimentListData[1].endDate = now.getTime();
@@ -211,6 +217,8 @@ add_task(function* test_getExperiments() {
 
   list = yield experiments.getExperiments();
   Assert.equal(list.length, 2, "Experiment list should have 2 entries now.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "An experiment add-on is installed.");
 
   experimentListData[0].active = true;
   experimentListData[0].endDate = now.getTime() + 10 * MS_IN_ONE_DAY;
@@ -236,6 +244,8 @@ add_task(function* test_getExperiments() {
 
   list = yield experiments.getExperiments();
   Assert.equal(list.length, 2, "Experiment list should have 2 entries now.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "No experiments add-ons are installed.");
 
   experimentListData[0].active = false;
   experimentListData[0].endDate = now.getTime();
@@ -301,11 +311,6 @@ add_task(function* test_addonAlreadyInstalled() {
   let list = yield experiments.getExperiments();
   Assert.equal(list.length, 0, "Experiment list should be empty.");
 
-  // Install conflicting addon.
-
-  let installed = yield installAddon(gDataRoot + EXPERIMENT1_XPI_NAME, EXPERIMENT1_XPI_SHA1);
-  Assert.ok(installed, "Addon should have been installed.");
-
   // Trigger update, clock set for the experiment to start.
 
   now = futureDate(startDate, 10 * MS_IN_ONE_DAY);
@@ -320,6 +325,20 @@ add_task(function* test_addonAlreadyInstalled() {
   Assert.equal(list[0].id, EXPERIMENT1_ID, "Experiment 1 should be the sole entry.");
   Assert.equal(list[0].active, true, "Experiment 1 should be active.");
 
+  let addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "1 add-on is installed.");
+
+  // Install conflicting addon.
+
+  let installed = yield installAddon(gDataRoot + EXPERIMENT1_XPI_NAME, EXPERIMENT1_XPI_SHA1);
+  Assert.ok(installed, "Addon should have been installed.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "1 add-on is installed.");
+  list = yield experiments.getExperiments();
+  Assert.equal(list.length, 1, "Experiment list should still have 1 entry.");
+  Assert.equal(list[0].id, EXPERIMENT1_ID, "Experiment 1 should be the sole entry.");
+  Assert.equal(list[0].active, true, "Experiment 1 should be active.");
+
   // Cleanup.
 
   Services.obs.removeObserver(observer, OBSERVER_TOPIC);
@@ -1351,7 +1370,12 @@ add_task(function* testUnknownExperimentsUninstalled() {
 
   let addons = yield getExperimentAddons();
   Assert.equal(addons.length, 0, "Precondition: No experiment add-ons are present.");
+
+  // Simulate us not listening.
+  experiments._stopWatchingAddons();
   yield installAddon(gDataRoot + EXPERIMENT1_XPI_NAME, EXPERIMENT1_XPI_SHA1);
+  experiments._startWatchingAddons();
+
   addons = yield getExperimentAddons();
   Assert.equal(addons.length, 1, "Experiment 1 installed via AddonManager");
 
@@ -1372,3 +1396,24 @@ add_task(function* testUnknownExperimentsUninstalled() {
   yield experiments.uninit();
   yield removeCacheFile();
 });
+
+// If someone else installs an experiment add-on, we detect and stop that.
+add_task(function* testForeignExperimentInstall() {
+  let experiments = new Experiments.Experiments(gPolicy);
+
+  gManifestObject = {
+    "version": 1,
+    experiments: [],
+  };
+
+  yield experiments.init();
+
+  let addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "Precondition: No experiment add-ons present.");
+  yield installAddon(gDataRoot + EXPERIMENT1_XPI_NAME, EXPERIMENT1_XPI_SHA1);
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "Add-on install should have been cancelled.");
+
+  yield experiments.uninit();
+  yield removeCacheFile();
+});

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

@@ -5,6 +5,7 @@
 let gManagerWindow;
 let gCategoryUtilities;
 let gInstalledAddons = [];
+let gContext = this;
 
 function test() {
   waitForExplicitFinish();
@@ -13,6 +14,14 @@ function test() {
     gManagerWindow = win;
     gCategoryUtilities = new CategoryUtilities(win);
 
+    // The Experiments Manager will interfere with us by preventing installs
+    // of experiments it doesn't know about. We remove it from the equation
+    // because here we are only concerned with core Addon Manager operation,
+    // not the super set Experiments Manager has imposed.
+    if ("@mozilla.org/browser/experiments-service;1" in Components.classes) {
+      Components.utils.import("resource:///modules/experiments/Experiments.jsm", gContext);
+      gContext.Experiments.instance()._stopWatchingAddons();
+    }
     run_next_test();
   });
 }
@@ -22,7 +31,13 @@ function end_test() {
     addon.uninstall();
   }
 
-  close_manager(gManagerWindow, finish);
+  close_manager(gManagerWindow, () => {
+    if ("@mozilla.org/browser/experiments-service;1" in Components.classes) {
+      gContext.Experiments.instance()._startWatchingAddons();
+    }
+
+    finish();
+  });
 }
 
 // On an empty profile with no experiments, the experiment category