diff --git a/browser/experiments/Experiments.jsm b/browser/experiments/Experiments.jsm index 56f43fbd4433..84e6359a5024 100644 --- a/browser/experiments/Experiments.jsm +++ b/browser/experiments/Experiments.jsm @@ -594,7 +594,7 @@ Experiments.Experiments.prototype = { }), _telemetryStatusChanged() { - this._toggleExperimentsEnabled(gExperimentsEnabled); + this._toggleExperimentsEnabled(gPrefs.get(PREF_ENABLED, false)); }, /** diff --git a/browser/experiments/ExperimentsService.js b/browser/experiments/ExperimentsService.js index da230879e956..d33a70aaf860 100644 --- a/browser/experiments/ExperimentsService.js +++ b/browser/experiments/ExperimentsService.js @@ -16,30 +16,19 @@ XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "CommonUtils", "resource://services-common/utils.js"); +XPCOMUtils.defineLazyModuleGetter(this, "TelemetryUtils", + "resource://gre/modules/TelemetryUtils.jsm"); + const PREF_EXPERIMENTS_ENABLED = "experiments.enabled"; const PREF_ACTIVE_EXPERIMENT = "experiments.activeExperiment"; // whether we have an active experiment -const PREF_TELEMETRY_ENABLED = "toolkit.telemetry.enabled"; -const PREF_TELEMETRY_UNIFIED = "toolkit.telemetry.unified"; const DELAY_INIT_MS = 30 * 1000; -// Whether the FHR/Telemetry unification features are enabled. -// Changing this pref requires a restart. -const IS_UNIFIED_TELEMETRY = Preferences.get(PREF_TELEMETRY_UNIFIED, false); - XPCOMUtils.defineLazyGetter( this, "gPrefs", () => { return new Preferences(); }); -XPCOMUtils.defineLazyGetter( - this, "gExperimentsEnabled", () => { - // We can enable experiments if either unified Telemetry or FHR is on, and the user - // has opted into Telemetry. - return gPrefs.get(PREF_EXPERIMENTS_ENABLED, false) && - IS_UNIFIED_TELEMETRY && gPrefs.get(PREF_TELEMETRY_ENABLED, false); - }); - XPCOMUtils.defineLazyGetter( this, "gActiveExperiment", () => { return gPrefs.get(PREF_ACTIVE_EXPERIMENT); @@ -54,8 +43,15 @@ ExperimentsService.prototype = { classID: Components.ID("{f7800463-3b97-47f9-9341-b7617e6d8d49}"), QueryInterface: XPCOMUtils.generateQI([Ci.nsITimerCallback, Ci.nsIObserver]), + get _experimentsEnabled() { + // We can enable experiments if either unified Telemetry or FHR is on, and the user + // has opted into Telemetry. + return gPrefs.get(PREF_EXPERIMENTS_ENABLED, false) && + TelemetryUtils.isTelemetryEnabled; + }, + notify(timer) { - if (!gExperimentsEnabled) { + if (!this._experimentsEnabled) { return; } if (OS.Constants.Path.profileDir === undefined) { @@ -63,7 +59,11 @@ ExperimentsService.prototype = { } let instance = Experiments.instance(); if (instance.isReady) { - instance.updateManifest(); + instance.updateManifest().catch(error => { + // Don't throw, as this breaks tests. In any case the best we can do here + // is to log the failure. + Cu.reportError(error); + }); } }, @@ -77,7 +77,7 @@ ExperimentsService.prototype = { observe(subject, topic, data) { switch (topic) { case "profile-after-change": - if (gExperimentsEnabled) { + if (this._experimentsEnabled) { Services.obs.addObserver(this, "quit-application"); Services.obs.addObserver(this, "sessionstore-state-finalized"); Services.obs.addObserver(this, "EM-loaded"); diff --git a/browser/experiments/test/xpcshell/test_telemetry_disabled.js b/browser/experiments/test/xpcshell/test_telemetry_disabled.js index 74f85ccfc7e8..7ef5f87cead9 100644 --- a/browser/experiments/test/xpcshell/test_telemetry_disabled.js +++ b/browser/experiments/test/xpcshell/test_telemetry_disabled.js @@ -13,9 +13,16 @@ add_test(function test_experiments_activation() { Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, false); let experiments = Experiments.instance(); + Assert.ok(!experiments.enabled, "Experiments must be disabled if Telemetry is disabled."); - // TODO: Test that Experiments are turned back on when bug 1232648 lands. + // Patch updateManifest to not do anything when the pref is switched back to true, + // otherwise it attempts to connect to the server. + experiments.updateManifest = () => Promise.resolve(); + + Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true); + + Assert.ok(experiments.enabled, "Experiments must be re-enabled if Telemetry is re-enabled"); run_next_test(); }); diff --git a/toolkit/components/telemetry/tests/unit/head.js b/toolkit/components/telemetry/tests/unit/head.js index 544d39885078..402d147fe2b9 100644 --- a/toolkit/components/telemetry/tests/unit/head.js +++ b/toolkit/components/telemetry/tests/unit/head.js @@ -311,6 +311,9 @@ if (runningInParent) { // if receive an unexpected ping. Let's globally disable the shutdown ping sender: // the relevant tests will enable this pref when needed. Services.prefs.setBoolPref("toolkit.telemetry.shutdownPingSender.enabled", false); + // Ensure browser experiments are also disabled, to avoid network activity + // when toggling PREF_ENABLED. + Services.prefs.setBoolPref("experiments.enabled", false); fakePingSendTimer((callback, timeout) => {