Bug 989137 - Part 9: Ensure add-ons are enabled when they should be; r=Unfocused, r=gfritzsche

Experiment add-ons are disabled by default in the Addon Manager. This
patch will manually enable them if necessary.

--HG--
extra : rebase_source : 2271372b232b5d85af32a4a96f2e3beb867cabf9

browser/experiments/Experiments.jsm

@@ -212,6 +212,10 @@ function uninstallAddons(addons) {
   AddonManager.addAddonListener(listener);
 
   for (let addon of addons) {
+    // Disabling the add-on before uninstalling is necessary to cause tests to
+    // pass. This might be indicative of a bug in XPIProvider.
+    // TODO follow up in bug 992396.
+    addon.userDisabled = true;
     addon.uninstall();
   }
 
@@ -978,6 +982,8 @@ Experiments.Experiments.prototype = {
           }
           this._dirty = true;
           activeChanged = true;
+        } else {
+          yield activeExperiment.ensureActive();
         }
       } finally {
         this._pendingUninstall = null;
@@ -1479,6 +1485,8 @@ Experiments.ExperimentEntry.prototype = {
     };
 
     let listener = {
+      _expectedID: null,
+
       onDownloadEnded: install => {
         this._log.trace("_installAddon() - onDownloadEnded for " + this.id);
 
@@ -1503,9 +1511,6 @@ Experiments.ExperimentEntry.prototype = {
           this._log.error("_installAddon() - onInstallStarted, wrong addon type");
           return false;
         }
-
-        // Experiment add-ons default to userDisabled = true.
-        install.addon.userDisabled = false;
       },
 
       onInstallEnded: install => {
@@ -1525,6 +1530,26 @@ Experiments.ExperimentEntry.prototype = {
         this._description = addon.description || "";
         this._homepageURL = addon.homepageURL || "";
 
+        // Experiment add-ons default to userDisabled=true. Enable if needed.
+        if (addon.userDisabled) {
+          this._log.trace("Add-on is disabled. Enabling.");
+          listener._expectedID = addon.id;
+          AddonManager.addAddonListener(listener);
+          addon.userDisabled = false;
+        } else {
+          this._log.trace("Add-on is enabled. start() completed.");
+          deferred.resolve();
+        }
+      },
+
+      onEnabled: addon => {
+        this._log.info("onEnabled() for " + addon.id);
+
+        if (addon.id != listener._expectedID) {
+          return;
+        }
+
+        AddonManager.removeAddonListener(listener);
         deferred.resolve();
       },
     };
@@ -1562,7 +1587,7 @@ Experiments.ExperimentEntry.prototype = {
       this._endDate = now;
     };
 
-    AddonManager.getAddonByID(this._addonId, addon => {
+    this._getAddon().then((addon) => {
       if (!addon) {
         let message = "could not get Addon for " + this.id;
         this._log.warn("stop() - " + message);
@@ -1579,6 +1604,61 @@ Experiments.ExperimentEntry.prototype = {
     return deferred.promise;
   },
 
+  /**
+   * Try to ensure this experiment is active.
+   *
+   * The returned promise will be resolved if the experiment is active
+   * in the Addon Manager or rejected if it isn't.
+   *
+   * @return Promise<>
+   */
+  ensureActive: Task.async(function* () {
+    this._log.trace("ensureActive() for " + this.id);
+
+    let addon = yield this._getAddon();
+    if (!addon) {
+      this._log.warn("Experiment is not installed: " + this._addonId);
+      throw new Error("Experiment is not installed: " + this._addonId);
+    }
+
+    // User disabled likely means the experiment is disabled at startup,
+    // since the permissions don't allow it to be disabled by the user.
+    if (!addon.userDisabled) {
+      return;
+    }
+
+    let deferred = Promise.defer();
+
+    let listener = {
+      onEnabled: enabledAddon => {
+        if (enabledAddon.id != addon.id) {
+          return;
+        }
+
+        AddonManager.removeAddonListener(listener);
+        deferred.resolve();
+      },
+    };
+
+    this._log.info("Activating add-on: " + addon.id);
+    AddonManager.addAddonListener(listener);
+    addon.userDisabled = false;
+    yield deferred.promise;
+  }),
+
+  /**
+   * Obtain the underlying Addon from the Addon Manager.
+   *
+   * @return Promise<Addon|null>
+   */
+  _getAddon: function () {
+    let deferred = Promise.defer();
+
+    AddonManager.getAddonByID(this._addonId, deferred.resolve);
+
+    return deferred.promise;
+  },
+
   _logTermination: function (terminationKind, terminationReason) {
     if (terminationKind === undefined) {
       return;

browser/experiments/test/xpcshell/test_activate.js

@@ -99,25 +99,43 @@ add_task(function* test_startStop() {
   Assert.equal(result.applicable, false, "Experiment should not be applicable.");
   Assert.equal(experiment.enabled, false, "Experiment should not be enabled.");
 
+  let addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "No experiment add-ons are installed.");
+
   defineNow(gPolicy, futureDate(startDate, 5 * MS_IN_ONE_DAY));
   result = yield isApplicable(experiment);
   Assert.equal(result.applicable, true, "Experiment should now be applicable.");
   Assert.equal(experiment.enabled, false, "Experiment should not be enabled.");
 
   yield experiment.start();
+  addons = yield getExperimentAddons();
   Assert.equal(experiment.enabled, true, "Experiment should now be enabled.");
+  Assert.equal(addons.length, 1, "1 experiment add-on is installed.");
+  Assert.equal(addons[0].id, experiment._addonId, "The add-on is the one we expect.");
+  Assert.equal(addons[0].userDisabled, false, "The add-on is not userDisabled.");
+  Assert.ok(addons[0].isActive, "The add-on is active.");
 
   yield experiment.stop();
+  addons = yield getExperimentAddons();
   Assert.equal(experiment.enabled, false, "Experiment should not be enabled.");
+  Assert.equal(addons.length, 0, "Experiment should be uninstalled from the Addon Manager.");
 
   yield experiment.start();
+  addons = yield getExperimentAddons();
   Assert.equal(experiment.enabled, true, "Experiment should now be enabled.");
+  Assert.equal(addons.length, 1, "1 experiment add-on is installed.");
+  Assert.equal(addons[0].id, experiment._addonId, "The add-on is the one we expect.");
+  Assert.equal(addons[0].userDisabled, false, "The add-on is not userDisabled.");
+  Assert.ok(addons[0].isActive, "The add-on is active.");
 
   let result = yield experiment._shouldStop();
   Assert.equal(result.shouldStop, false, "shouldStop should be false.");
   let maybeStop = yield experiment.maybeStop();
   Assert.equal(maybeStop, false, "Experiment should not have been stopped.");
   Assert.equal(experiment.enabled, true, "Experiment should be enabled.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "Experiment still in add-ons manager.");
+  Assert.ok(addons[0].isActive, "The add-on is still active.");
 
   defineNow(gPolicy, futureDate(endDate, MS_IN_ONE_DAY));
   result = yield experiment._shouldStop();
@@ -125,4 +143,6 @@ add_task(function* test_startStop() {
   maybeStop = yield experiment.maybeStop();
   Assert.equal(maybeStop, true, "Experiment should have been stopped.");
   Assert.equal(experiment.enabled, false, "Experiment should be disabled.");
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "Experiment add-on is uninstalled.");
 });

browser/experiments/test/xpcshell/test_api.js

@@ -1417,3 +1417,52 @@ add_task(function* testForeignExperimentInstall() {
   yield experiments.uninit();
   yield removeCacheFile();
 });
+
+// Experiment add-ons will be disabled after Addon Manager restarts. Ensure
+// we enable them automatically.
+add_task(function* testEnabledAfterRestart() {
+  let experiments = new Experiments.Experiments(gPolicy);
+
+  gManifestObject = {
+    "version": 1,
+    experiments: [
+      {
+        id: EXPERIMENT1_ID,
+        xpiURL: gDataRoot + EXPERIMENT1_XPI_NAME,
+        xpiHash: EXPERIMENT1_XPI_SHA1,
+        startTime: gPolicy.now().getTime() / 1000 - 60,
+        endTime: gPolicy.now().getTime() / 1000 + 60,
+        maxActiveSeconds: 10 * SEC_IN_ONE_DAY,
+        appName: ["XPCShell"],
+        channel: ["nightly"],
+      },
+    ],
+  };
+
+  let addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 0, "Precondition: No experimenta add-ons installed.");
+
+  yield experiments.updateManifest();
+  let fromManifest = yield experiments.getExperiments();
+  Assert.equal(fromManifest.length, 1, "A single experiment is known.");
+
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "A single experiment add-on is installed.");
+  Assert.ok(addons[0].isActive, "That experiment is active.");
+
+  dump("Restarting Addon Manager\n");
+  experiments._stopWatchingAddons();
+  restartManager();
+  experiments._startWatchingAddons();
+
+  addons = yield getExperimentAddons();
+  Assert.equal(addons.length, 1, "The experiment is still there after restart.");
+  Assert.ok(addons[0].userDisabled, "But it is disabled.");
+  Assert.equal(addons[0].isActive, false, "And not active.");
+
+  yield experiments.updateManifest();
+  Assert.ok(addons[0].isActive, "It activates when the manifest is evaluated.");
+
+  yield experiments.uninit();
+  yield removeCacheFile();
+});