commit eb3f9ea0ff5ee79d4998a032d2e460d011a43c4d
parent 626cbe672edf7b0323fbd49c38ac0a599774a811
Author: Beth Rennie <beth@brennie.ca>
Date: Fri, 31 Oct 2025 03:39:49 +0000
Bug 1972647 - Allow Firefox Labs to be enabled even if studies or telemetry are disabled r=mkaply,nimbus-reviewers,relud
Both Firefox Labs and one of either studies or telemetry must now be
disabled to fully disable Nimbus.
Differential Revision: https://phabricator.services.mozilla.com/D269920
Diffstat:
9 files changed, 282 insertions(+), 117 deletions(-)
diff --git a/browser/components/enterprisepolicies/Policies.sys.mjs b/browser/components/enterprisepolicies/Policies.sys.mjs
@@ -2924,6 +2924,10 @@ export var Policies = {
);
}
if ("FirefoxLabs" in param) {
+ if (!param.FirefoxLabs) {
+ manager.disallowFeature("FirefoxLabs");
+ }
+
PoliciesUtils.setDefaultPref(
"browser.preferences.experimental",
param.FirefoxLabs,
diff --git a/toolkit/components/nimbus/ExperimentAPI.sys.mjs b/toolkit/components/nimbus/ExperimentAPI.sys.mjs
@@ -162,6 +162,8 @@ export const ExperimentAPI = new (class {
telemetryEnabled: false,
};
+ #studiesEnabled = false;
+
constructor() {
if (IS_MAIN_PROCESS) {
// Ensure that the profile ID is cached in a pref.
@@ -181,24 +183,7 @@ export const ExperimentAPI = new (class {
}
}
- const onStudiesEnabledChanged = this.#onStudiesEnabledChanged.bind(this);
-
- XPCOMUtils.defineLazyPreferenceGetter(
- this.#prefValues,
- "studiesEnabled",
- STUDIES_OPT_OUT_PREF,
- false,
- onStudiesEnabledChanged
- );
-
- XPCOMUtils.defineLazyPreferenceGetter(
- this.#prefValues,
- "telemetryEnabled",
- UPLOAD_ENABLED_PREF,
- false,
- onStudiesEnabledChanged
- );
-
+ this._onStudiesEnabledChanged = this._onStudiesEnabledChanged.bind(this);
this._annotateCrashReport = this._annotateCrashReport.bind(this);
this._removeCrashReportAnnotator =
this._removeCrashReportAnnotator.bind(this);
@@ -244,6 +229,10 @@ export const ExperimentAPI = new (class {
this.#initialized = true;
+ // Compute the enabled state and cache it. It is possible for the enabled
+ // state to change during ExperimentAPI initialization, but we do not
+ // register our observers until the end of this function.
+ this.#computeEnabled();
const studiesEnabled = this.studiesEnabled;
try {
@@ -310,10 +299,13 @@ export const ExperimentAPI = new (class {
);
}
+ Services.prefs.addObserver(STUDIES_OPT_OUT_PREF, this._onStudiesEnabledChanged);
+ Services.prefs.addObserver(UPLOAD_ENABLED_PREF, this._onStudiesEnabledChanged);
+
// If Nimbus was disabled between the start of this function and registering
// the pref observers we have not handled it yet.
if (studiesEnabled !== this.studiesEnabled) {
- await this.#onStudiesEnabledChanged();
+ await this._onStudiesEnabledChanged();
}
return true;
@@ -369,17 +361,35 @@ export const ExperimentAPI = new (class {
this.#experimentManager?.store.off("update", this._annotateCrashReport);
this.#experimentManager = null;
+ Services.prefs.removeObserver(STUDIES_OPT_OUT_PREF, this._onStudiesEnabledChanged);
+ Services.prefs.removeObserver(UPLOAD_ENABLED_PREF, this._onStudiesEnabledChanged);
+
this.#initialized = false;
}
- get studiesEnabled() {
- return (
+ #computeEnabled() {
+ this.#prefValues.studiesEnabled = Services.prefs.getBoolPref(STUDIES_OPT_OUT_PREF, false);
+ this.#prefValues.telemetryEnabled = Services.prefs.getBoolPref(UPLOAD_ENABLED_PREF, false)
+
+ this.#studiesEnabled = (
this.#prefValues.studiesEnabled &&
this.#prefValues.telemetryEnabled &&
Services.policies.isAllowed("Shield")
);
}
+ get enabled() {
+ return this.studiesEnabled || this.labsEnabled;
+ }
+
+ get labsEnabled() {
+ return Services.policies.isAllowed("FirefoxLabs");
+ }
+
+ get studiesEnabled() {
+ return this.#studiesEnabled;
+ }
+
/**
* Return the profile ID.
*
@@ -445,18 +455,35 @@ export const ExperimentAPI = new (class {
}
}
- async #onStudiesEnabledChanged() {
+ async _onStudiesEnabledChanged(_topic, _subject, prefName) {
+ const studiesPreviouslyEnabled = this.studiesEnabled;
+
+ switch (prefName) {
+ case STUDIES_OPT_OUT_PREF:
+ case UPLOAD_ENABLED_PREF:
+ this.#computeEnabled();
+ break;
+
+ default:
+ return;
+ }
+
if (!this.#initialized) {
return;
}
- if (!this.studiesEnabled) {
- await this.manager._handleStudiesOptOut();
- }
+ if (studiesPreviouslyEnabled !== this.studiesEnabled) {
+ if (!this.studiesEnabled) {
+ this.manager._handleStudiesOptOut();
+ }
- await this._rsLoader.onEnabledPrefChange();
+ // Labs is disabled only by policy, so it cannot be disabled at runtime.
+ // Thus we only need to notify the RemoteSettingsExperimentLoader when
+ // studies become enabled or disabled.
+ await this._rsLoader.onEnabledPrefChange();
- Services.obs.notifyObservers(null, this.STUDIES_ENABLED_CHANGED);
+ Services.obs.notifyObservers(null, this.STUDIES_ENABLED_CHANGED);
+ }
}
/**
diff --git a/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs b/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs
@@ -308,10 +308,6 @@ export class ExperimentManager {
return;
}
- if (result.ok && recipe.isFirefoxLabsOptIn) {
- this.optInRecipes.push(recipe);
- }
-
if (!result.ok) {
lazy.NimbusTelemetry.recordEnrollmentStatus({
slug: recipe.slug,
@@ -322,8 +318,15 @@ export class ExperimentManager {
return;
}
+ // Unenrollment due to studies becoming disabled is handled in
+ // `_handleStudiesOptOut`.
+ if (result.status === lazy.MatchStatus.DISABLED) {
+ return;
+ }
+
if (recipe.isFirefoxLabsOptIn) {
// We do not enroll directly into Firefox Labs opt-ins.
+ this.optInRecipes.push(recipe);
return;
}
@@ -749,8 +752,17 @@ export class ExperimentManager {
const { EnrollmentStatus, EnrollmentStatusReason, UnenrollReason } =
lazy.NimbusTelemetry;
- if (result.ok && recipe?.isFirefoxLabsOptIn) {
- this.optInRecipes.push(recipe);
+ if (result.ok) {
+ // Unenrollment due to studies becoming disabled is handled in
+ // `_handleStudiesOptOut`. Firefox Labs can only be disabled by policy and
+ // thus its enabled state cannot change at runtime.
+ if (result.status === lazy.MatchStatus.DISABLED) {
+ return false;
+ }
+
+ if (recipe?.isFirefoxLabsOptIn) {
+ this.optInRecipes.push(recipe);
+ }
}
if (enrollment.active) {
@@ -925,8 +937,10 @@ export class ExperimentManager {
/**
* Unenroll from all active studies if user opts out.
*/
- async _handleStudiesOptOut() {
- for (const enrollment of this.store.getAllActiveExperiments()) {
+ _handleStudiesOptOut() {
+ for (const enrollment of this.store
+ .getAll()
+ .filter(e => e.active && !e.isFirefoxLabsOptIn)) {
this._unenroll(
enrollment,
UnenrollmentCause.fromReason(
@@ -934,16 +948,6 @@ export class ExperimentManager {
)
);
}
- for (const enrollment of this.store.getAllActiveRollouts()) {
- this._unenroll(
- enrollment,
- UnenrollmentCause.fromReason(
- lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT
- )
- );
- }
-
- this.optInRecipes = [];
}
/**
diff --git a/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs b/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs
@@ -114,6 +114,7 @@ export const MatchStatus = Object.freeze({
TARGETING_ONLY: "TARGETING_ONLY",
TARGETING_AND_BUCKETING: "TARGETING_AND_BUCKETING",
UNENROLLED_IN_ANOTHER_PROFILE: "UNENROLLED_IN_ANOTHER_PROFILE",
+ DISABLED: "DISABLED",
});
export const CheckRecipeResult = {
@@ -257,40 +258,39 @@ export class RemoteSettingsExperimentLoader {
);
}
- if (this._enabled) {
- return;
- }
+ if (!this._enabled) {
+ if (!lazy.ExperimentAPI.enabled) {
+ lazy.log.debug(
+ "Not enabling RemoteSettingsExperimentLoader: Nimbus disabled"
+ );
+ return;
+ }
- if (!lazy.ExperimentAPI.studiesEnabled) {
- lazy.log.debug(
- "Not enabling RemoteSettingsExperimentLoader: studies disabled"
- );
- return;
- }
+ if (
+ Services.startup.isInOrBeyondShutdownPhase(
+ Ci.nsIAppStartup.SHUTDOWN_PHASE_APPSHUTDOWNCONFIRMED
+ )
+ ) {
+ lazy.log.debug(
+ "Not enabling RemoteSettingsExperimentLoader: shutting down"
+ );
+ return;
+ }
- if (
- Services.startup.isInOrBeyondShutdownPhase(
- Ci.nsIAppStartup.SHUTDOWN_PHASE_APPSHUTDOWNCONFIRMED
- )
- ) {
- lazy.log.debug(
- "Not enabling RemoteSettingsExperimentLoader: shutting down"
- );
- return;
- }
+ this.#shutdownBlocker = async () => {
+ await this.finishedUpdating();
+ this.disable();
+ };
- this.#shutdownBlocker = async () => {
- await this.finishedUpdating();
- this.disable();
- };
+ lazy.AsyncShutdown.appShutdownConfirmed.addBlocker(
+ "RemoteSettingsExperimentLoader: disabling",
+ this.#shutdownBlocker
+ );
- lazy.AsyncShutdown.appShutdownConfirmed.addBlocker(
- "RemoteSettingsExperimentLoader: disabling",
- this.#shutdownBlocker
- );
+ this.setTimer();
- this.setTimer();
- this._enabled = true;
+ this._enabled = true;
+ }
await this.updateRecipes("enabled", { forceSync });
}
@@ -431,6 +431,8 @@ export class RemoteSettingsExperimentLoader {
recipeValidator,
{
validationEnabled,
+ labsEnabled: lazy.ExperimentAPI.labsEnabled,
+ studiesEnabled: lazy.ExperimentAPI.studiesEnabled,
shouldCheckTargeting: true,
unenrolledExperimentSlugs,
}
@@ -768,19 +770,14 @@ export class RemoteSettingsExperimentLoader {
}
/**
- * Handles feature status based on STUDIES_OPT_OUT_PREF.
- *
- * Changing this pref to false will turn off any recipe fetching and
- * processing.
+ * Disable the RemoteSettingsExperimentLoader if Nimbus has become disabled
+ * and vice versa.
*/
async onEnabledPrefChange() {
- if (this._enabled && !lazy.ExperimentAPI.studiesEnabled) {
- this.disable();
- } else if (!this._enabled && lazy.ExperimentAPI.studiesEnabled) {
- // If the feature pref is turned on then turn on recipe processing.
- // If the opt in pref is turned on then turn on recipe processing only if
- // the feature pref is also enabled.
+ if (lazy.ExperimentAPI.enabled) {
await this.enable();
+ } else {
+ this.disable();
}
}
@@ -823,7 +820,7 @@ export class RemoteSettingsExperimentLoader {
* immediately.
*/
finishedUpdating() {
- if (!lazy.ExperimentAPI.studiesEnabled || !this._enabled) {
+ if (!lazy.ExperimentAPI.enabled || !this._enabled) {
return Promise.resolve();
}
@@ -912,12 +909,17 @@ export class EnrollmentsContext {
validationEnabled = true,
shouldCheckTargeting = true,
unenrolledExperimentSlugs,
+ studiesEnabled = true,
+ labsEnabled = true,
} = {}
) {
this.manager = manager;
this.recipeValidator = recipeValidator;
this.validationEnabled = validationEnabled;
+ this.studiesEnabled = studiesEnabled;
+ this.labsEnabled = labsEnabled;
+
this.validatorCache = {};
this.shouldCheckTargeting = shouldCheckTargeting;
this.unenrolledExperimentSlugs = unenrolledExperimentSlugs;
@@ -951,6 +953,13 @@ export class EnrollmentsContext {
}
}
+ if (
+ (recipe.isFirefoxLabsOptIn && !this.labsEnabled) ||
+ (!recipe.isFirefoxLabsOptIn && !this.studiesEnabled)
+ ) {
+ return CheckRecipeResult.Ok(MatchStatus.DISABLED);
+ }
+
// We don't include missing features here because if validation is enabled we report those errors later.
const unsupportedFeatureIds = recipe.featureIds.filter(
featureId =>
diff --git a/toolkit/components/nimbus/lib/Telemetry.sys.mjs b/toolkit/components/nimbus/lib/Telemetry.sys.mjs
@@ -81,6 +81,7 @@ const UnenrollReason = Object.freeze({
CHANGED_PREF: "changed-pref",
FORCE_ENROLLMENT: "force-enrollment",
INDIVIDUAL_OPT_OUT: "individual-opt-out",
+ LABS_DIABLED: "labs-disabled",
LABS_OPT_OUT: "labs-opt-out",
PREF_FLIPS_CONFLICT: "prefFlips-conflict",
PREF_FLIPS_FAILED: "prefFlips-failed",
diff --git a/toolkit/components/nimbus/test/browser/browser_remotesettings_experiment_enroll.js b/toolkit/components/nimbus/test/browser/browser_remotesettings_experiment_enroll.js
@@ -45,7 +45,11 @@ add_task(async function test_experimentEnrollment_startup() {
set: [["app.shield.optoutstudies.enabled", false]],
});
- Assert.ok(!ExperimentAPI._rsLoader._enabled, "Should be disabled");
+ Assert.ok(ExperimentAPI.enabled, "ExperimentAPI is still enabled");
+ Assert.ok(ExperimentAPI._rsLoader._enabled, "RemoteSettingsExperimentLoader is still enabled");
+
+ Assert.ok(!ExperimentAPI.studiesEnabled, "Studies disabled");
+ Assert.ok(ExperimentAPI.labsEnabled, "Firefox Labs enabled");
await SpecialPowers.pushPrefEnv({
set: [["app.shield.optoutstudies.enabled", true]],
diff --git a/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js b/toolkit/components/nimbus/test/browser/browser_remotesettingsexperimentloader_remote_defaults.js
@@ -87,14 +87,21 @@ add_task(async function test_remote_fetch_and_ready() {
"This prop does not exist before we sync"
);
+ await SpecialPowers.pushPrefEnv({
+ set: [
+ ["datareporting.healthreport.uploadEnabled", true],
+ ["app.shield.optoutstudies.enabled", true]
+ ]
+ });
+
await ExperimentAPI.ready();
+ await ExperimentAPI._rsLoader.finishedUpdating();
const { cleanup } = await setup();
// Fake being initialized so we can update recipes
// we don't need to start any timers
- ExperimentAPI._rsLoader._enabled = true;
- await ExperimentAPI._rsLoader.updateRecipes("browser_rsel_remote_defaults");
+ await ExperimentAPI._rsLoader.updateRecipes("test");
Assert.equal(
NimbusFeatures.foo.getVariable("remoteValue"),
@@ -210,6 +217,8 @@ add_task(async function test_remote_fetch_and_ready() {
await cleanup();
cleanupTestFeatures();
+
+ await SpecialPowers.popPrefEnv();
});
add_task(async function test_remote_fetch_on_updateRecipes() {
@@ -239,6 +248,7 @@ add_task(async function test_remote_fetch_on_updateRecipes() {
Assert.ok(updateRecipesStub.calledOnce, "Timer calls function");
Assert.equal(updateRecipesStub.firstCall.args[0], "timer", "Called by timer");
sandbox.restore();
+ await SpecialPowers.popPrefEnv();
// This will un-register the timer
ExperimentAPI._rsLoader.disable();
Services.prefs.clearUserPref(
@@ -435,6 +445,14 @@ add_task(async function remote_defaults_active_remote_defaults() {
);
const { cleanup } = await setup([rollout1, rollout2]);
+
+ SpecialPowers.pushPrefEnv({
+ set: [
+ ["datareporting.healthreport.uploadEnabled", true],
+ ["app.shield.optoutstudies.enabled", true],
+ ]
+ });
+
await ExperimentAPI._rsLoader.updateRecipes("mochitest");
Assert.ok(barFeature.getVariable("enabled"), "Enabled on first sync");
@@ -451,6 +469,8 @@ add_task(async function remote_defaults_active_remote_defaults() {
ExperimentAPI.manager.store._deleteForTests("bar");
await NimbusTestUtils.flushStore();
+ await SpecialPowers.popPrefEnv();
+
await cleanup();
cleanupTestFeatures();
});
diff --git a/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js b/toolkit/components/nimbus/test/unit/test_RemoteSettingsExperimentLoader.js
@@ -63,17 +63,21 @@ add_task(async function test_init_with_opt_in() {
await initExperimentAPI();
- equal(
+ Assert.equal(
loader.setTimer.callCount,
- 0,
- `should not initialize if ${STUDIES_OPT_OUT_PREF} pref is false`
+ 1,
+ `should initialize even if ${STUDIES_OPT_OUT_PREF} pref is false`
);
- Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, true);
- Assert.ok(loader.setTimer.calledOnce, "should call .setTimer");
- Assert.ok(loader.updateRecipes.calledOnce, "should call .updateRecipes");
+ Assert.equal(
+ loader.updateRecipes.callCount,
+ 1,
+ "should call updateRecipes()"
+ );
await cleanup();
+
+ Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, true);
});
add_task(async function test_updateRecipes() {
diff --git a/toolkit/components/nimbus/test/unit/test_policy.js b/toolkit/components/nimbus/test/unit/test_policy.js
@@ -8,18 +8,43 @@ const { EnterprisePolicyTesting } = ChromeUtils.importESModule(
"resource://testing-common/EnterprisePolicyTesting.sys.mjs"
);
+const RECIPES = [
+ NimbusTestUtils.factories.recipe.withFeatureConfig("experiment", {
+ featureId: "no-feature-firefox-desktop",
+ }),
+ NimbusTestUtils.factories.recipe.withFeatureConfig(
+ "rollout",
+ { featureId: "no-feature-firefox-desktop" },
+ { isRollout: true }
+ ),
+ NimbusTestUtils.factories.recipe.withFeatureConfig(
+ "optin",
+ { featureId: "no-feature-firefox-desktop" },
+ {
+ isRollout: true,
+ isFirefoxLabsOptIn: true,
+ firefoxLabsTitle: "title",
+ firefoxLabsDescription: "description",
+ firefoxLabsGroup: "group",
+ requiresRestart: false,
+ }
+ ),
+];
+
add_setup(function setup() {
// Instantiate the enterprise policy service.
void Cc["@mozilla.org/enterprisepolicies;1"].getService(Ci.nsIObserver);
});
-add_task(async function testPolicyDisablesNimbus() {
+async function doTest({
+ policies,
+ labsEnabled,
+ studiesEnabled,
+ expectedEnrollments,
+ expectedOptIns,
+}) {
info("Enabling policy");
- await EnterprisePolicyTesting.setupPolicyEngineWithJson({
- policies: {
- DisableFirefoxStudies: true,
- },
- });
+ await EnterprisePolicyTesting.setupPolicyEngineWithJson({ policies });
info("Is policy engine active?");
Assert.equal(
@@ -28,28 +53,95 @@ add_task(async function testPolicyDisablesNimbus() {
"Policy engine is active"
);
- const loader = NimbusTestUtils.stubs.rsLoader();
- const manager = loader.manager;
- await manager.store.init();
- await manager.onStartup();
+ const { initExperimentAPI, cleanup, loader } =
+ await NimbusTestUtils.setupTest({
+ init: false,
+ experiments: RECIPES,
+ });
- Assert.ok(!manager.studiesEnabled, "ExperimentManager is disabled");
+ sinon.spy(loader, "updateRecipes");
+ sinon.spy(loader, "setTimer");
- const setTimerStub = sinon.stub(loader, "setTimer");
- const updateRecipes = sinon.stub(loader, "updateRecipes");
+ await initExperimentAPI();
- await loader.enable();
+ Assert.equal(
+ ExperimentAPI.studiesEnabled,
+ studiesEnabled,
+ "Studies are enabled"
+ );
+ Assert.equal(
+ ExperimentAPI.labsEnabled,
+ labsEnabled,
+ "FirefoxLabs is enabled"
+ );
+
+ Assert.equal(
+ loader._enabled,
+ studiesEnabled || labsEnabled,
+ "RemoteSettingsExperimentLoader initialized"
+ );
+
+ Assert.equal(
+ loader.setTimer.called,
+ studiesEnabled || labsEnabled,
+ "RemoteSettingsExperimentLoader polling for recipes"
+ );
- Assert.ok(
- !loader._initialized,
- "RemoteSettingsExperimentLoader not initailized"
+ Assert.equal(
+ loader.updateRecipes.called,
+ studiesEnabled || labsEnabled,
+ "RemoteSettingsExperimentLoader polling for recipes"
);
- Assert.ok(
- setTimerStub.notCalled,
- "RemoteSettingsExperimentLoader not polling for recipes"
+
+ Assert.deepEqual(
+ ExperimentAPI.manager.store
+ .getAll()
+ .filter(e => e.active)
+ .map(e => e.slug)
+ .sort(),
+ expectedEnrollments.sort(),
+ "Should have expected enrollments"
);
- Assert.ok(
- updateRecipes.notCalled,
- "RemoteSettingsExperimentLoader not updating recipes after startup"
+
+ Assert.deepEqual(
+ ExperimentAPI.manager.optInRecipes.map(e => e.slug).sort(),
+ expectedOptIns,
+ "Should have expected available opt-ins"
);
+
+ await NimbusTestUtils.cleanupManager(expectedEnrollments);
+ await cleanup();
+}
+
+add_task(async function testDisableStudiesPolicy() {
+ await doTest({
+ policies: { DisableFirefoxStudies: true },
+ labsEnabled: true,
+ studiesEnabled: false,
+ expectedEnrollments: [],
+ expectedOptIns: ["optin"],
+ });
+});
+
+add_task(async function testDisableLabsPolicy() {
+ await doTest({
+ policies: { UserMessaging: { FirefoxLabs: false } },
+ labsEnabled: false,
+ studiesEnabled: true,
+ expectedEnrollments: ["experiment", "rollout"],
+ expectedOptIns: [],
+ });
+});
+
+add_task(async function testNimbusDisabled() {
+ await doTest({
+ policies: {
+ DisableFirefoxStudies: true,
+ UserMessaging: { FirefoxLabs: false },
+ },
+ labsEnabled: false,
+ studiesEnabled: false,
+ expectedEnrollments: [],
+ expectedOptIns: [],
+ });
});
