commit 34f255aa3378ab24e271512c2c549b4ee28ef472 parent 9e721d640f814da2ff176f1a7446b2e5b042279b Author: Alexandru Marc <amarc@mozilla.com> Date: Fri, 31 Oct 2025 10:34:39 +0200 Revert "Bug 1972647 - Don't hide Firefox Labs when studies or telemetry are disabled r=mkaply,akulyk" for causing marionette failures @ test_no_errors_clean_profile.py This reverts commit 444d4fbc9d66c4ca3a42132958750224bbafa588. Revert "Bug 1972647 - Unenroll from Firefox labs opt-ins during Nimbus initialization if Labs is disabled by policy r=nimbus-reviewers,relud" This reverts commit 8f2ec3f7c78314af60f32c9de59c83efda1df3e7. Revert "Bug 1972647 - Allow Firefox Labs to be enabled even if studies or telemetry are disabled r=mkaply,nimbus-reviewers,relud" This reverts commit eb3f9ea0ff5ee79d4998a032d2e460d011a43c4d. Revert "Bug 1972647 - Make ExperimentAPI a singleton instance of a class r=nimbus-reviewers,relud" This reverts commit 626cbe672edf7b0323fbd49c38ac0a599774a811. Revert "Bug 1972647 - Hide Firefox Labs opt-ins from about:studies r=nimbus-reviewers,relud" This reverts commit 8f71edffd04897bc2a673d04e7c48b7c74f32633. Diffstat:
18 files changed, 385 insertions(+), 559 deletions(-)
diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js @@ -1354,6 +1354,8 @@ pref("accessibility.typeaheadfind.timeout", 5000); pref("accessibility.typeaheadfind.linksonly", false); pref("accessibility.typeaheadfind.flashBar", 1); +// Whether we can show the "Firefox Labs" section. +pref("browser.preferences.experimental", true); // Whether we had to hide the "Firefox Labs" section because it would be empty. pref("browser.preferences.experimental.hidden", false); // Whether we show the "More from Mozilla" section. diff --git a/browser/components/enterprisepolicies/Policies.sys.mjs b/browser/components/enterprisepolicies/Policies.sys.mjs @@ -2923,8 +2923,12 @@ export var Policies = { param.Locked ); } - if ("FirefoxLabs" in param && !param.FirefoxLabs) { - manager.disallowFeature("FirefoxLabs"); + if ("FirefoxLabs" in param) { + PoliciesUtils.setDefaultPref( + "browser.preferences.experimental", + param.FirefoxLabs, + param.Locked + ); } }, }, diff --git a/browser/components/preferences/experimental.js b/browser/components/preferences/experimental.js @@ -26,6 +26,7 @@ const gExperimentalPane = { this._onCheckboxChanged = this._onCheckboxChanged.bind(this); this._onNimbusUpdate = this._onNimbusUpdate.bind(this); + this._onStudiesEnabledChanged = this._onStudiesEnabledChanged.bind(this); this._resetAllFeatures = this._resetAllFeatures.bind(this); setEventListener( @@ -34,6 +35,10 @@ const gExperimentalPane = { this._resetAllFeatures ); + Services.obs.addObserver( + this._onStudiesEnabledChanged, + ExperimentAPI.STUDIES_ENABLED_CHANGED + ); window.addEventListener("unload", () => this._removeObservers()); await this._maybeRenderLabsRecipes(); @@ -164,8 +169,24 @@ const gExperimentalPane = { } }, + async _onStudiesEnabledChanged() { + const studiesEnabled = ExperimentAPI.studiesEnabled; + + if (studiesEnabled) { + await this._maybeRenderLabsRecipes(); + } else { + this._setCategoryVisibility(true); + this._removeLabsRecipes(); + this._firefoxLabs = null; + } + }, + _removeObservers() { ExperimentAPI.manager.store.off("update", this._onNimbusUpdate); + Services.obs.removeObserver( + this._onStudiesEnabledChanged, + ExperimentAPI.STUDIES_ENABLED_CHANGED + ); }, // Reset the features to their default values diff --git a/browser/components/preferences/preferences.js b/browser/components/preferences/preferences.js @@ -81,7 +81,6 @@ ChromeUtils.defineESModuleGetters(this, { ContextualIdentityService: "resource://gre/modules/ContextualIdentityService.sys.mjs", DownloadUtils: "resource://gre/modules/DownloadUtils.sys.mjs", - ExperimentAPI: "resource://nimbus/ExperimentAPI.sys.mjs", ExtensionPreferencesManager: "resource://gre/modules/ExtensionPreferencesManager.sys.mjs", ExtensionSettingsStore: @@ -243,17 +242,16 @@ function init_all() { if (Services.prefs.getBoolPref("browser.translations.newSettingsUI.enable")) { register_module("paneTranslations", gTranslationsPane); } - if (ExperimentAPI.labsEnabled) { + if (Services.prefs.getBoolPref("browser.preferences.experimental")) { // Set hidden based on previous load's hidden value or if Nimbus is // disabled. document.getElementById("category-experimental").hidden = + !ExperimentAPI.studiesEnabled || Services.prefs.getBoolPref( "browser.preferences.experimental.hidden", false ); register_module("paneExperimental", gExperimentalPane); - } else { - document.getElementById("category-experimental").hidden = true; } NimbusFeatures.moreFromMozilla.recordExposureEvent({ once: true }); diff --git a/browser/components/preferences/tests/browser_experimental_features.js b/browser/components/preferences/tests/browser_experimental_features.js @@ -8,6 +8,23 @@ add_setup(async function setup() { registerCleanupFunction(cleanup); }); +add_task(async function testPrefRequired() { + await SpecialPowers.pushPrefEnv({ + set: [["browser.preferences.experimental", false]], + }); + + await openPreferencesViaOpenPreferencesAPI("paneHome", { leaveOpen: true }); + let doc = gBrowser.contentDocument; + + let experimentalCategory = doc.getElementById("category-experimental"); + ok(experimentalCategory, "The category exists"); + ok(experimentalCategory.hidden, "The category is hidden"); + + BrowserTestUtils.removeTab(gBrowser.selectedTab); + + await SpecialPowers.popPrefEnv(); +}); + add_task(async function testCanOpenWithPref() { await SpecialPowers.pushPrefEnv({ set: [["browser.preferences.experimental", true]], @@ -41,6 +58,10 @@ add_task(async function testCanOpenWithPref() { }); add_task(async function testSearchFindsExperiments() { + await SpecialPowers.pushPrefEnv({ + set: [["browser.preferences.experimental", true]], + }); + await openPreferencesViaOpenPreferencesAPI("paneHome", { leaveOpen: true }); let doc = gBrowser.contentDocument; @@ -59,4 +80,6 @@ add_task(async function testSearchFindsExperiments() { ); BrowserTestUtils.removeTab(gBrowser.selectedTab); + + await SpecialPowers.popPrefEnv(); }); diff --git a/browser/components/preferences/tests/browser_experimental_features_filter.js b/browser/components/preferences/tests/browser_experimental_features_filter.js @@ -36,6 +36,10 @@ add_task(async function testFilterFeatures() { ]; const cleanup = await setupLabsTest(recipes); + await SpecialPowers.pushPrefEnv({ + set: [["browser.preferences.experimental", true]], + }); + await BrowserTestUtils.openNewForegroundTab( gBrowser, "about:preferences#paneExperimental" diff --git a/browser/components/preferences/tests/browser_experimental_features_hidden_when_not_public.js b/browser/components/preferences/tests/browser_experimental_features_hidden_when_not_public.js @@ -6,6 +6,10 @@ add_task(async function testNonPublicFeaturesShouldntGetDisplayed() { const cleanup = await setupLabsTest(); + await SpecialPowers.pushPrefEnv({ + set: [["browser.preferences.experimental", true]], + }); + await BrowserTestUtils.openNewForegroundTab( gBrowser, "about:preferences#paneExperimental" @@ -38,6 +42,7 @@ add_task(async function testNonPublicFeaturesShouldntGetDisplayed() { BrowserTestUtils.removeTab(gBrowser.selectedTab); await cleanup(); + await SpecialPowers.popPrefEnv(); }); add_task(async function testNonPublicFeaturesShouldntGetDisplayed() { @@ -45,7 +50,10 @@ add_task(async function testNonPublicFeaturesShouldntGetDisplayed() { const cleanup = await setupLabsTest(DEFAULT_LABS_RECIPES.slice(2)); await SpecialPowers.pushPrefEnv({ - set: [["browser.preferences.experimental.hidden", false]], + set: [ + ["browser.preferences.experimental", true], + ["browser.preferences.experimental.hidden", false], + ], }); await BrowserTestUtils.openNewForegroundTab( diff --git a/browser/components/preferences/tests/browser_experimental_features_studies_disabled.js b/browser/components/preferences/tests/browser_experimental_features_studies_disabled.js @@ -3,20 +3,13 @@ "use strict"; -const { EnterprisePolicyTesting } = ChromeUtils.importESModule( - "resource://testing-common/EnterprisePolicyTesting.sys.mjs" -); - -add_task(async function testHiddenWhenLabsDisabled() { +add_task(async function testHiddenWhenStudiesDisabled() { const cleanup = await setupLabsTest(); await SpecialPowers.pushPrefEnv({ - set: [["browser.preferences.experimental.hidden", false]], - }); - - await EnterprisePolicyTesting.setupPolicyEngineWithJson({ - policies: { - UserMessaging: { FirefoxLabs: false }, - }, + set: [ + ["browser.preferences.experimental", true], + ["browser.preferences.experimental.hidden", false], + ], }); await BrowserTestUtils.openNewForegroundTab( @@ -26,27 +19,151 @@ add_task(async function testHiddenWhenLabsDisabled() { const doc = gBrowser.contentDocument; - await TestUtils.waitForCondition( - () => doc.getElementById("category-experimental").hidden, - "Wait for Experimental Features section label to become hidden" + await waitForExperimentalFeaturesShown(doc); + + const enrollPromises = [ + promiseNimbusStoreUpdate("nimbus-qa-1", true), + promiseNimbusStoreUpdate("nimbus-qa-2", true), + ]; + + await enrollByClick(doc.getElementById("nimbus-qa-1"), true); + await enrollByClick(doc.getElementById("nimbus-qa-2"), true); + + await enrollPromises; + + const unenrollPromises = [ + promiseNimbusStoreUpdate("nimbus-qa-1", false), + promiseNimbusStoreUpdate("nimbus-qa-2", false), + ]; + + // Disabling studies should remove the experimental pane. + await SpecialPowers.pushPrefEnv({ + set: [["app.shield.optoutstudies.enabled", false]], + }); + + await waitForExperimentalFeaturesHidden(doc); + await unenrollPromises; + + ok( + !ExperimentAPI._manager.store.get("nimbus-qa-1")?.active, + "Should unenroll from nimbus-qa-1" + ); + ok( + !ExperimentAPI._manager.store.get("nimbus-qa-2")?.active, + "Should unenroll from nimbus-qa-2" ); - is( - doc.getElementById("pane-experimental-featureGates"), - null, - "Experimental Features section not added to the DOM" + // Re-enabling studies should re-add it. + await SpecialPowers.popPrefEnv(); + await waitForExperimentalFeaturesShown(doc); + + // Navigate back to the experimental tab. + EventUtils.synthesizeMouseAtCenter( + doc.getElementById("category-experimental"), + {}, + gBrowser.contentWindow ); + await waitForPageFlush(); + is( doc.querySelector(".category[selected]").id, - "category-general", - "When the experimental features section is hidden, navigating to #experimental should redirect to #general" + "category-experimental", + "Experimental category selected" ); - BrowserTestUtils.removeTab(gBrowser.selectedTab); + ok( + !doc.getElementById("nimbus-qa-1").checked, + "nimbus-qa-1 checkbox unchecked" + ); + ok( + !doc.getElementById("nimbus-qa-2").checked, + "nimbus-qa-2 checkbox unchecked" + ); + + await enrollByClick(doc.getElementById("nimbus-qa-1"), true); + await enrollByClick(doc.getElementById("nimbus-qa-2"), true); + + // Likewise, disabling telemetry should remove the experimental pane. + await SpecialPowers.pushPrefEnv({ + set: [["datareporting.healthreport.uploadEnabled", false]], + }); + + await waitForExperimentalFeaturesHidden(doc); + + ok( + !ExperimentAPI._manager.store.get("nimbus-qa-1")?.active, + "Should unenroll from nimbus-qa-1" + ); + ok( + !ExperimentAPI._manager.store.get("nimbus-qa-2")?.active, + "Should unenroll from nimbus-qa-2" + ); await SpecialPowers.popPrefEnv(); - await cleanup(); - await EnterprisePolicyTesting.setupPolicyEngineWithJson({}); + // Re-enabling studies should re-add it. + await waitForExperimentalFeaturesShown(doc); + + ok( + !doc.getElementById("nimbus-qa-1").checked, + "nimbus-qa-1 checkbox unchecked" + ); + ok( + !doc.getElementById("nimbus-qa-2").checked, + "nimbus-qa-2 checkbox unchecked" + ); + + BrowserTestUtils.removeTab(gBrowser.selectedTab); + + await cleanup(); + await SpecialPowers.popPrefEnv(); }); + +async function waitForExperimentalFeaturesShown(doc) { + await TestUtils.waitForCondition( + () => doc.querySelector(".featureGate"), + "Wait for features to be added to the DOM" + ); + + ok( + !doc.getElementById("category-experimental").hidden, + "Experimental Features section should not be hidden" + ); + + ok( + !Services.prefs.getBoolPref("browser.preferences.experimental.hidden"), + "Hidden pref should be false" + ); +} + +async function waitForExperimentalFeaturesHidden(doc) { + await TestUtils.waitForCondition( + () => doc.getElementById("category-experimental").hidden, + "Wait for Experimental Features section to get hidden" + ); + + ok( + doc.getElementById("category-experimental").hidden, + "Experimental Features section should be hidden when all features are hidden" + ); + ok( + doc.getElementById("firefoxExperimentalCategory").hidden, + "Experimental Features header should be hidden when all features are hidden" + ); + is( + doc.querySelector(".category[selected]").id, + "category-general", + "When the experimental features section is hidden, navigating to #experimental should redirect to #general" + ); + ok( + Services.prefs.getBoolPref("browser.preferences.experimental.hidden"), + "Hidden pref should be true" + ); +} + +function waitForPageFlush() { + return new Promise(resolve => + requestAnimationFrame(() => requestAnimationFrame(resolve)) + ); +} diff --git a/toolkit/components/nimbus/ExperimentAPI.sys.mjs b/toolkit/components/nimbus/ExperimentAPI.sys.mjs @@ -2,11 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -/** - * @import { ExperimentManager } from "./lib/ExperimentManager.sys.mjs" - * @import { RemoteSettingsExperimentLoader } from "./lib/RemoteSettingsExperimentLoader.sys.mjs" - */ - import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs"; import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs"; @@ -79,6 +74,26 @@ const experimentBranchAccessor = { const NIMBUS_PROFILE_ID_PREF = "nimbus.profileId"; +let cachedProfileId = null; + +/** + * Ensure the Nimbus profile ID exists. + * + * @returns {string} The profile ID. + */ +function ensureNimbusProfileId() { + if (!cachedProfileId) { + if (Services.prefs.prefHasUserValue(NIMBUS_PROFILE_ID_PREF)) { + cachedProfileId = Services.prefs.getStringPref(NIMBUS_PROFILE_ID_PREF); + } else { + cachedProfileId = Services.uuid.generateUUID().toString().slice(1, -1); + Services.prefs.setStringPref(NIMBUS_PROFILE_ID_PREF, cachedProfileId); + } + } + + return cachedProfileId; +} + /** * Metadata about an enrollment. * @@ -116,83 +131,11 @@ export const EnrollmentType = Object.freeze({ ROLLOUT: "rollout", }); -export const ExperimentAPI = new (class { - /** - * Whether or not the ExperimentAPI has been initialized. - */ - #initialized = false; - - /** - * The current ExperimentManager. - * - * @type {ExperimentManager | null} - */ - #experimentManager = null; - - /** - * The current RemoteSettingsExperimentLoader. - * - * @type {RemoteSettingsExperimentLoader | null} - */ - #experimentLoader = null; - - /** - * The unique ID of this Profile. - * - * @type {string | null} - */ - #cachedProfileId = null; - - /** - * Cached pref values. - */ - #prefValues = { - /** - * Whether or not opt-out studies are enabled. - * - * @see {@link STUDIES_OPT_OUT_PREF} - */ - studiesEnabled: false, - - /** - * Whether or not telemetry is enabled. - * - * @see {@link UPLOAD_ENABLED_PREF} - */ - telemetryEnabled: false, - }; - - #studiesEnabled = false; - - constructor() { - if (IS_MAIN_PROCESS) { - // Ensure that the profile ID is cached in a pref. - if (Services.prefs.prefHasUserValue(NIMBUS_PROFILE_ID_PREF)) { - this.#cachedProfileId = Services.prefs.getStringPref( - NIMBUS_PROFILE_ID_PREF - ); - } else { - this.#cachedProfileId = Services.uuid - .generateUUID() - .toString() - .slice(1, -1); - Services.prefs.setStringPref( - NIMBUS_PROFILE_ID_PREF, - this.#cachedProfileId - ); - } - } - - this._onStudiesEnabledChanged = this._onStudiesEnabledChanged.bind(this); - this._annotateCrashReport = this._annotateCrashReport.bind(this); - this._removeCrashReportAnnotator = - this._removeCrashReportAnnotator.bind(this); - - ChromeUtils.defineLazyGetter(this, "_remoteSettingsClient", function () { - return lazy.RemoteSettings(lazy.COLLECTION_ID); - }); - } +let initialized = false; +let experimentManager = null; +let experimentLoader = null; +export const ExperimentAPI = { /** * The topic that is notified when either the studies enabled pref or the * telemetry enabled pref changes. @@ -202,7 +145,7 @@ export const ExperimentAPI = new (class { */ get STUDIES_ENABLED_CHANGED() { return "nimbus:studies-enabled-changed"; - } + }, /** * Initialize the ExperimentAPI. @@ -223,16 +166,14 @@ export const ExperimentAPI = new (class { * Whether or not the ExperimentAPI was initialized. */ async init({ extraContext, forceSync = false } = {}) { - if (this.#initialized) { + if (initialized) { return false; } - this.#initialized = true; + ensureNimbusProfileId(); + + 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 { @@ -300,11 +241,11 @@ export const ExperimentAPI = new (class { } Services.prefs.addObserver( - STUDIES_OPT_OUT_PREF, + UPLOAD_ENABLED_PREF, this._onStudiesEnabledChanged ); Services.prefs.addObserver( - UPLOAD_ENABLED_PREF, + STUDIES_OPT_OUT_PREF, this._onStudiesEnabledChanged ); @@ -315,121 +256,78 @@ export const ExperimentAPI = new (class { } return true; - } + }, /** * Return the global ExperimentManager. * * The ExperimentManager will be lazily created upon first access to this * property. - * - * @type {ExperimentManager} */ get manager() { - if (this.#experimentManager === null) { - this.#experimentManager = new lazy.ExperimentManager(); + if (experimentManager === null) { + experimentManager = new lazy.ExperimentManager(); } - return this.#experimentManager; - } + return experimentManager; + }, /** * Return the global ExperimentManager. * * @deprecated Use ExperimentAPI.Manager instead of this property. - * - * @type {ExperimentManager} */ get _manager() { return this.manager; - } + }, /** * Return the global RemoteSettingsExperimentLoader. - * - * @type {RemoteSettingsExperimentLoader} */ get _rsLoader() { - if (this.#experimentLoader === null) { - this.#experimentLoader = new lazy.RemoteSettingsExperimentLoader( - this.manager - ); + if (experimentLoader === null) { + experimentLoader = new lazy.RemoteSettingsExperimentLoader(this.manager); } - return this.#experimentLoader; - } + return experimentLoader; + }, _resetForTests() { - this.#experimentLoader?.disable(); - this.#experimentLoader = null; + experimentLoader?.disable(); + experimentLoader = null; - lazy.CleanupManager.removeCleanupHandler(this._removeCrashReportAnnotator); - 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; - } - - #computeEnabled() { - this.#prefValues.studiesEnabled = Services.prefs.getBoolPref( - STUDIES_OPT_OUT_PREF, - false - ); - this.#prefValues.telemetryEnabled = Services.prefs.getBoolPref( - UPLOAD_ENABLED_PREF, - false + lazy.CleanupManager.removeCleanupHandler( + ExperimentAPI._removeCrashReportAnnotator ); + experimentManager?.store.off("update", this._annotateCrashReport); + experimentManager = null; - 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"); - } + initialized = false; + }, get studiesEnabled() { - return this.#studiesEnabled; - } + return ( + Services.prefs.getBoolPref(UPLOAD_ENABLED_PREF, false) && + Services.prefs.getBoolPref(STUDIES_OPT_OUT_PREF, false) && + Services.policies.isAllowed("Shield") + ); + }, /** * Return the profile ID. * * This is used to distinguish different profiles in a shared profile group * apart. Each profile has a persistent and stable profile ID. It is stored as - * a user branch pref. + * a user branch pref but is locked to prevent tampering. * * This is still susceptible to user.js editing, but there's nothing we can do * about that. * - * @throws {Error} If accessed outside the main process. - * * @returns {string} The profile ID. */ get profileId() { - if (!IS_MAIN_PROCESS) { - throw new Error( - "ExperimentAPI.profileId is not available outside the main process" - ); - } - - return this.#cachedProfileId; - } + return ensureNimbusProfileId(); + }, /** * Wait for the ExperimentAPI to become ready. @@ -444,7 +342,7 @@ export const ExperimentAPI = new (class { */ async ready() { return this.manager.store.ready(); - } + }, /** * Annotate the current crash report with current enrollments. @@ -464,44 +362,23 @@ export const ExperimentAPI = new (class { "NimbusEnrollments", activeEnrollments ); - } + }, _removeCrashReportAnnotator() { - if (this.#initialized) { - this.#experimentManager?.store.off("update", this._annotateCrashReport); + if (initialized) { + experimentManager?.store.off("update", this._annotateCrashReport); } - } - - 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; + async _onStudiesEnabledChanged() { + if (!this.studiesEnabled) { + await this.manager._handleStudiesOptOut(); } - if (!this.#initialized) { - return; - } - - if (studiesPreviouslyEnabled !== this.studiesEnabled) { - if (!this.studiesEnabled) { - this.manager._handleStudiesOptOut(); - } - - // 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(); + await this._rsLoader.onEnabledPrefChange(); - Services.obs.notifyObservers(null, this.STUDIES_ENABLED_CHANGED); - } - } + Services.obs.notifyObservers(null, this.STUDIES_ENABLED_CHANGED); + }, /** * Returns the recipe for a given experiment slug @@ -538,7 +415,7 @@ export const ExperimentAPI = new (class { } return recipe; - } + }, /** * Returns all the branches for a given experiment slug @@ -560,7 +437,7 @@ export const ExperimentAPI = new (class { return recipe?.branches.map( branch => new Proxy(branch, experimentBranchAccessor) ); - } + }, /** * Opt-in to the given experiment on the given branch. @@ -588,8 +465,8 @@ export const ExperimentAPI = new (class { */ async optInToExperiment(options) { return this._rsLoader._optInToExperiment(options); - } -})(); + }, +}; /** * Singleton that holds lazy references to _ExperimentFeature instances @@ -1056,6 +933,21 @@ export class _ExperimentFeature { } } +ExperimentAPI._annotateCrashReport = + ExperimentAPI._annotateCrashReport.bind(ExperimentAPI); +ExperimentAPI._onStudiesEnabledChanged = + ExperimentAPI._onStudiesEnabledChanged.bind(ExperimentAPI); +ExperimentAPI._removeCrashReportAnnotator = + ExperimentAPI._removeCrashReportAnnotator.bind(ExperimentAPI); + +ChromeUtils.defineLazyGetter( + ExperimentAPI, + "_remoteSettingsClient", + function () { + return lazy.RemoteSettings(lazy.COLLECTION_ID); + } +); + class ExperimentLocalizationError extends Error { constructor(reason, locale) { super(`Localized experiment error (${reason})`); diff --git a/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs b/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs @@ -261,10 +261,6 @@ export class ExperimentManager { this._handleStudiesOptOut(); } - if (!lazy.ExperimentAPI.labsEnabled) { - this._handleLabsDisabled(); - } - lazy.NimbusFeatures.nimbusTelemetry.onUpdate(() => { // Providing default values ensure we disable metrics when unenrolling. const cfg = { @@ -312,6 +308,10 @@ export class ExperimentManager { return; } + if (result.ok && recipe.isFirefoxLabsOptIn) { + this.optInRecipes.push(recipe); + } + if (!result.ok) { lazy.NimbusTelemetry.recordEnrollmentStatus({ slug: recipe.slug, @@ -322,15 +322,8 @@ 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; } @@ -756,17 +749,8 @@ export class ExperimentManager { const { EnrollmentStatus, EnrollmentStatusReason, UnenrollReason } = lazy.NimbusTelemetry; - 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 after Nimbus is initialized. - if (result.status === lazy.MatchStatus.DISABLED) { - return false; - } - - if (recipe?.isFirefoxLabsOptIn) { - this.optInRecipes.push(recipe); - } + if (result.ok && recipe?.isFirefoxLabsOptIn) { + this.optInRecipes.push(recipe); } if (enrollment.active) { @@ -941,12 +925,8 @@ export class ExperimentManager { /** * Unenroll from all active studies if user opts out. */ - _handleStudiesOptOut() { - const enrollments = this.store - .getAll() - .filter(e => e.active && !e.isFirefoxLabsOptIn); - - for (const enrollment of enrollments) { + async _handleStudiesOptOut() { + for (const enrollment of this.store.getAllActiveExperiments()) { this._unenroll( enrollment, UnenrollmentCause.fromReason( @@ -954,26 +934,16 @@ export class ExperimentManager { ) ); } - } - - /** - * Unenroll from all active Firefox Labs opt-ins if Labs becomes disabled. - */ - _handleLabsDisabled() { - const enrollments = this.store - .getAll() - .filter(e => e.active && e.isFirefoxLabsOptIn); - - for (const enrollment of enrollments) { + for (const enrollment of this.store.getAllActiveRollouts()) { this._unenroll( enrollment, UnenrollmentCause.fromReason( - lazy.NimbusTelemetry.UnenrollReason.LABS_DISABLED + lazy.NimbusTelemetry.UnenrollReason.STUDIES_OPT_OUT ) ); } - this.optinRecipes = []; + this.optInRecipes = []; } /** diff --git a/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs b/toolkit/components/nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs @@ -114,7 +114,6 @@ 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 = { @@ -258,39 +257,40 @@ export class RemoteSettingsExperimentLoader { ); } - if (!this._enabled) { - if (!lazy.ExperimentAPI.enabled) { - lazy.log.debug( - "Not enabling RemoteSettingsExperimentLoader: Nimbus disabled" - ); - return; - } - - if ( - Services.startup.isInOrBeyondShutdownPhase( - Ci.nsIAppStartup.SHUTDOWN_PHASE_APPSHUTDOWNCONFIRMED - ) - ) { - lazy.log.debug( - "Not enabling RemoteSettingsExperimentLoader: shutting down" - ); - return; - } + if (this._enabled) { + return; + } - this.#shutdownBlocker = async () => { - await this.finishedUpdating(); - this.disable(); - }; + if (!lazy.ExperimentAPI.studiesEnabled) { + lazy.log.debug( + "Not enabling RemoteSettingsExperimentLoader: studies disabled" + ); + return; + } - lazy.AsyncShutdown.appShutdownConfirmed.addBlocker( - "RemoteSettingsExperimentLoader: disabling", - this.#shutdownBlocker + if ( + Services.startup.isInOrBeyondShutdownPhase( + Ci.nsIAppStartup.SHUTDOWN_PHASE_APPSHUTDOWNCONFIRMED + ) + ) { + lazy.log.debug( + "Not enabling RemoteSettingsExperimentLoader: shutting down" ); + return; + } - this.setTimer(); + this.#shutdownBlocker = async () => { + await this.finishedUpdating(); + this.disable(); + }; - this._enabled = true; - } + lazy.AsyncShutdown.appShutdownConfirmed.addBlocker( + "RemoteSettingsExperimentLoader: disabling", + this.#shutdownBlocker + ); + + this.setTimer(); + this._enabled = true; await this.updateRecipes("enabled", { forceSync }); } @@ -431,8 +431,6 @@ export class RemoteSettingsExperimentLoader { recipeValidator, { validationEnabled, - labsEnabled: lazy.ExperimentAPI.labsEnabled, - studiesEnabled: lazy.ExperimentAPI.studiesEnabled, shouldCheckTargeting: true, unenrolledExperimentSlugs, } @@ -770,14 +768,19 @@ export class RemoteSettingsExperimentLoader { } /** - * Disable the RemoteSettingsExperimentLoader if Nimbus has become disabled - * and vice versa. + * Handles feature status based on STUDIES_OPT_OUT_PREF. + * + * Changing this pref to false will turn off any recipe fetching and + * processing. */ async onEnabledPrefChange() { - if (lazy.ExperimentAPI.enabled) { - await this.enable(); - } else { + 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. + await this.enable(); } } @@ -815,12 +818,12 @@ export class RemoteSettingsExperimentLoader { * Resolves when the RemoteSettingsExperimentLoader has updated at least once * and is not in the middle of an update. * - * If Nimbus is disabled or the RemoteSettingsExperimentLoader has been + * If studies are disabled or the RemoteSettingsExperimentLoader has been * disabled (i.e., during shutdown), then this will always resolve * immediately. */ finishedUpdating() { - if (!lazy.ExperimentAPI.enabled || !this._enabled) { + if (!lazy.ExperimentAPI.studiesEnabled || !this._enabled) { return Promise.resolve(); } @@ -909,17 +912,12 @@ 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; @@ -953,13 +951,6 @@ 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,7 +81,6 @@ 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,14 +45,7 @@ add_task(async function test_experimentEnrollment_startup() { set: [["app.shield.optoutstudies.enabled", false]], }); - 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"); + Assert.ok(!ExperimentAPI._rsLoader._enabled, "Should be disabled"); 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,21 +87,14 @@ 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 - await ExperimentAPI._rsLoader.updateRecipes("test"); + ExperimentAPI._rsLoader._enabled = true; + await ExperimentAPI._rsLoader.updateRecipes("browser_rsel_remote_defaults"); Assert.equal( NimbusFeatures.foo.getVariable("remoteValue"), @@ -217,8 +210,6 @@ add_task(async function test_remote_fetch_and_ready() { await cleanup(); cleanupTestFeatures(); - - await SpecialPowers.popPrefEnv(); }); add_task(async function test_remote_fetch_on_updateRecipes() { @@ -248,7 +239,6 @@ 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( @@ -445,14 +435,6 @@ 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"); @@ -469,8 +451,6 @@ 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,21 +63,17 @@ add_task(async function test_init_with_opt_in() { await initExperimentAPI(); - Assert.equal( + equal( loader.setTimer.callCount, - 1, - `should initialize even if ${STUDIES_OPT_OUT_PREF} pref is false` + 0, + `should not initialize if ${STUDIES_OPT_OUT_PREF} pref is false` ); - Assert.equal( - loader.updateRecipes.callCount, - 1, - "should call updateRecipes()" - ); + Services.prefs.setBoolPref(STUDIES_OPT_OUT_PREF, true); + Assert.ok(loader.setTimer.calledOnce, "should call .setTimer"); + Assert.ok(loader.updateRecipes.calledOnce, "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,44 +8,18 @@ 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); }); -async function doTest({ - policies, - labsEnabled, - studiesEnabled, - existingEnrollments = [], - expectedEnrollments, - expectedOptIns, -}) { +add_task(async function testPolicyDisablesNimbus() { info("Enabling policy"); - await EnterprisePolicyTesting.setupPolicyEngineWithJson({ policies }); + await EnterprisePolicyTesting.setupPolicyEngineWithJson({ + policies: { + DisableFirefoxStudies: true, + }, + }); info("Is policy engine active?"); Assert.equal( @@ -54,122 +28,28 @@ async function doTest({ "Policy engine is active" ); - let storePath = undefined; - if (existingEnrollments) { - const store = NimbusTestUtils.stubs.store(); - await store.init(); - - for (const slug of existingEnrollments) { - NimbusTestUtils.addEnrollmentForRecipe( - RECIPES.find(e => e.slug === slug), - { store } - ); - } - - storePath = await NimbusTestUtils.saveStore(store); - } - - const { initExperimentAPI, cleanup, loader } = - await NimbusTestUtils.setupTest({ - init: false, - experiments: RECIPES, - storePath, - }); - - sinon.spy(loader, "updateRecipes"); - sinon.spy(loader, "setTimer"); + const loader = NimbusTestUtils.stubs.rsLoader(); + const manager = loader.manager; + await manager.store.init(); + await manager.onStartup(); - await initExperimentAPI(); + Assert.ok(!manager.studiesEnabled, "ExperimentManager is disabled"); - Assert.equal( - ExperimentAPI.studiesEnabled, - studiesEnabled, - "Studies are enabled" - ); - Assert.equal( - ExperimentAPI.labsEnabled, - labsEnabled, - "FirefoxLabs is enabled" - ); + const setTimerStub = sinon.stub(loader, "setTimer"); + const updateRecipes = sinon.stub(loader, "updateRecipes"); - Assert.equal( - loader._enabled, - studiesEnabled || labsEnabled, - "RemoteSettingsExperimentLoader initialized" - ); - - Assert.equal( - loader.setTimer.called, - studiesEnabled || labsEnabled, - "RemoteSettingsExperimentLoader polling for recipes" - ); + await loader.enable(); - Assert.equal( - loader.updateRecipes.called, - studiesEnabled || labsEnabled, - "RemoteSettingsExperimentLoader polling for recipes" + Assert.ok( + !loader._initialized, + "RemoteSettingsExperimentLoader not initailized" ); - - Assert.deepEqual( - ExperimentAPI.manager.store - .getAll() - .filter(e => e.active) - .map(e => e.slug) - .sort(), - expectedEnrollments.sort(), - "Should have expected enrollments" + Assert.ok( + setTimerStub.notCalled, + "RemoteSettingsExperimentLoader not polling for recipes" ); - - Assert.deepEqual( - ExperimentAPI.manager.optInRecipes.map(e => e.slug).sort(), - expectedOptIns, - "Should have expected available opt-ins" + Assert.ok( + updateRecipes.notCalled, + "RemoteSettingsExperimentLoader not updating recipes after startup" ); - - 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: [], - }); -}); - -add_task(async function testDisableLabsPolicyCausesUnenrollments() { - await doTest({ - policies: { UserMessaging: { FirefoxLabs: false } }, - labsEnabled: false, - studiesEnabled: true, - expectedEnrollments: ["experiment", "rollout"], - existingEnrollments: ["optin"], - expectedOptIns: [], - }); }); diff --git a/toolkit/components/normandy/content/AboutPages.sys.mjs b/toolkit/components/normandy/content/AboutPages.sys.mjs @@ -101,11 +101,7 @@ ChromeUtils.defineLazyGetter(AboutPages, "aboutStudies", () => { }, getMessagingSystemList() { - // Do not include Firefox Labs. Those are shown on - // about:preferences#experimental. - return lazy.ExperimentAPI.manager.store - .getAll() - .filter(e => !e.isFirefoxLabsOptIn); + return lazy.ExperimentAPI.manager.store.getAll(); }, async optInToExperiment(data) { @@ -224,7 +220,7 @@ ChromeUtils.defineLazyGetter(AboutPages, "aboutStudies", () => { ); this._sendToAll( "Shield:UpdateMessagingSystemExperimentList", - this.getMessagingSystemList() + lazy.ExperimentAPI.manager.store.getAll() ); }, diff --git a/toolkit/components/normandy/test/browser/browser_about_studies.js b/toolkit/components/normandy/test/browser/browser_about_studies.js @@ -12,8 +12,8 @@ const { NimbusTestUtils } = ChromeUtils.importESModule( const { ExperimentAPI } = ChromeUtils.importESModule( "resource://nimbus/ExperimentAPI.sys.mjs" ); -const { FirefoxLabs } = ChromeUtils.importESModule( - "resource://nimbus/FirefoxLabs.sys.mjs" +const { RemoteSettingsExperimentLoader } = ChromeUtils.importESModule( + "resource://nimbus/lib/RemoteSettingsExperimentLoader.sys.mjs" ); const { NormandyTestUtils } = ChromeUtils.importESModule( @@ -866,7 +866,7 @@ add_task(async function test_forceEnroll() { } ); - await NimbusTestUtils.assert.storeIsEmpty(ExperimentAPI.manager.store); + NimbusTestUtils.assert.storeIsEmpty(ExperimentAPI.manager.store); sandbox.restore(); }); @@ -956,51 +956,3 @@ add_task(async function test_inactive_rollouts_under_completed_studies() { // Cleanup for multiple test runs await NimbusTestUtils.assert.storeIsEmpty(ExperimentAPI.manager.store); }); - -add_task(async function testFirefoxLabs() { - const study = NimbusTestUtils.factories.recipe("study"); - await ExperimentAPI.manager.enroll(study, "rs-loader"); - - const optin = NimbusTestUtils.factories.recipe("optin", { - isRollout: true, - isFirefoxLabsOptIn: true, - firefoxLabsTitle: "title", - firefoxLabsDescription: "description", - firefoxLabsGroup: "group", - requiresRestart: false, - }); - - ExperimentAPI.manager.optInRecipes.push(optin); - - const labs = await FirefoxLabs.create(); - await labs.enroll("optin", "control"); - - await BrowserTestUtils.withNewTab( - { gBrowser, url: "about:studies" }, - async browser => { - const nimbusItems = await SpecialPowers.spawn(browser, [], async () => { - await ContentTaskUtils.waitForCondition( - () => content.document.querySelector(".nimbus .remove-button"), - "waiting for page to load" - ); - - return Array.from( - content.document.querySelectorAll(".nimbus"), - el => el.dataset.studySlug - ); - }); - - Assert.deepEqual( - nimbusItems, - ["study"], - "Firefox Labs opt-in not present" - ); - } - ); - - await labs.unenroll("optin", "control"); - await ExperimentAPI.manager.unenroll("study"); - ExperimentAPI.manager.optInRecipes.pop(); - - await NimbusTestUtils.assert.storeIsEmpty(ExperimentAPI.manager.store); -});