diff --git a/browser/experiments/Experiments.jsm b/browser/experiments/Experiments.jsm index c2de25331a53..d47117268121 100644 --- a/browser/experiments/Experiments.jsm +++ b/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(); } @@ -972,6 +976,8 @@ Experiments.Experiments.prototype = { } this._dirty = true; activeChanged = true; + } else { + yield activeExperiment.ensureActive(); } } finally { this._pendingUninstall = null; @@ -1473,6 +1479,8 @@ Experiments.ExperimentEntry.prototype = { }; let listener = { + _expectedID: null, + onDownloadEnded: install => { this._log.trace("_installAddon() - onDownloadEnded for " + this.id); @@ -1497,9 +1505,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 => { @@ -1519,6 +1524,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(); }, }; @@ -1556,7 +1581,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); @@ -1573,6 +1598,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 + */ + _getAddon: function () { + let deferred = Promise.defer(); + + AddonManager.getAddonByID(this._addonId, deferred.resolve); + + return deferred.promise; + }, + _logTermination: function (terminationKind, terminationReason) { if (terminationKind === undefined) { return; diff --git a/browser/experiments/test/xpcshell/test_activate.js b/browser/experiments/test/xpcshell/test_activate.js index e30e57ea231d..fc2afe68be45 100644 --- a/browser/experiments/test/xpcshell/test_activate.js +++ b/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."); }); diff --git a/browser/experiments/test/xpcshell/test_api.js b/browser/experiments/test/xpcshell/test_api.js index 3faec84fbfdc..0b344324096f 100644 --- a/browser/experiments/test/xpcshell/test_api.js +++ b/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(); +});