commit 3f5fb17381deff5fb3c1ef134a53adb699a3c48c
parent e71c0e7f70ba6622d877e585b25c20608e1500c0
Author: Beth Rennie <beth@brennie.ca>
Date: Fri, 7 Nov 2025 19:25:17 +0000
Bug 1993057 - Record whether or not pref flips that caused unenrollments happened via about:config r=nimbus-reviewers,relud
Differential Revision: https://phabricator.services.mozilla.com/D271731
Diffstat:
8 files changed, 367 insertions(+), 25 deletions(-)
diff --git a/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs b/toolkit/components/nimbus/lib/ExperimentManager.sys.mjs
@@ -84,15 +84,18 @@ export const UnenrollmentCause = {
*
* @param {object} changedPref
* @param {string} changedPref.name The pref that changed.
- * @param {string | undefined} changedPref.branch The branch on which the pref
+ * @param {string} changedPref.branch The branch on which the pref
* was changed.
+ * @param {boolean} isAboutConfigChange Whether or not the change was caused
+ * by the user via about:config.
*
* @returns {object} The unenrollment cause.
*/
- ChangedPref(changedPref) {
+ ChangedPref(changedPref, isAboutConfigChange) {
return {
reason: lazy.NimbusTelemetry.UnenrollReason.CHANGED_PREF,
changedPref,
+ isAboutConfigChange,
};
},
@@ -130,6 +133,9 @@ export const UnenrollmentCause = {
* and sending experiment-related Telemetry.
*/
export class ExperimentManager {
+ /** @type AboutConfigObserver */
+ #aboutConfigObserver;
+
constructor({ id = "experimentmanager", store } = {}) {
this.id = id;
this.store = store || new lazy.ExperimentStore();
@@ -154,6 +160,8 @@ export class ExperimentManager {
//
// This can only be used in the parent process ExperimentManager.
this._prefFlips = null;
+
+ this.#aboutConfigObserver = new AboutConfigObserver();
}
/**
@@ -1593,17 +1601,20 @@ export class ExperimentManager {
pref.featureId
);
- const changedPref = {
- name: pref.name,
- branch: PrefFlipsFeature.determinePrefChangeBranch(
- pref.name,
- pref.branch,
- feature.value[pref.variable]
- ),
- };
+ const cause = UnenrollmentCause.ChangedPref(
+ {
+ name: pref.name,
+ branch: PrefFlipsFeature.determinePrefChangeBranch(
+ pref.name,
+ pref.branch,
+ feature.value[pref.variable]
+ ),
+ },
+ this.isPrefBeingChangedViaAboutConfig(pref.name)
+ );
for (const enrollment of enrollments) {
- this._unenroll(enrollment, UnenrollmentCause.ChangedPref(changedPref));
+ this._unenroll(enrollment, cause);
}
}
@@ -1662,6 +1673,18 @@ export class ExperimentManager {
}
/**
+ * Return whether or not the given pref is being changed by a user on
+ * about:config.
+ *
+ * @param {string} pref The preference to check.
+ *
+ * @returns {boolean}
+ */
+ isPrefBeingChangedViaAboutConfig(pref) {
+ return this.#aboutConfigObserver.isBeingChanged(pref);
+ }
+
+ /**
* Return the feature configuration with the matching feature ID from the
* given branch.
*
@@ -1678,3 +1701,74 @@ export class ExperimentManager {
return branch.features.find(f => f.featureId === featureId);
}
}
+
+const ABOUT_CONFIG_WILL_CHANGE_PREF_TOPIC = "about-config-will-change-pref";
+const ABOUT_CONFIG_CHANGED_PREF_TOPIC = "about-config-changed-pref";
+
+/**
+ * Keep track of what prefs are being changed via about:config.
+ */
+class AboutConfigObserver {
+ /**
+ * Prefs that are currently being changed via about:config.
+ *
+ * @type Set<string>
+ */
+ #changes;
+
+ constructor() {
+ this.#changes = new Set();
+
+ Services.obs.addObserver(this, ABOUT_CONFIG_WILL_CHANGE_PREF_TOPIC);
+ Services.obs.addObserver(this, ABOUT_CONFIG_CHANGED_PREF_TOPIC);
+ }
+
+ /**
+ * Handle a notification from about:config.
+ *
+ * @param {any} _subject Unused.
+ * @param {string} topic The topic that indicates the rising vs. the falling
+ * edge of the event.
+ * @param {string} data The name of the pref being changed.
+ * @returns
+ */
+ observe(_subject, topic, data) {
+ switch (topic) {
+ case ABOUT_CONFIG_WILL_CHANGE_PREF_TOPIC:
+ this.#onWillChange(data);
+ break;
+
+ case ABOUT_CONFIG_CHANGED_PREF_TOPIC:
+ this.#onChanged(data);
+ break;
+ }
+ }
+
+ /**
+ * Record that a pref is about to change.
+ * @param {string} pref The pref.
+ */
+ #onWillChange(pref) {
+ this.#changes.add(pref);
+ }
+
+ /**
+ * Record that a pref has finished changing.
+ * @param {string} pref The pref.
+ */
+ #onChanged(pref) {
+ this.#changes.delete(pref);
+ }
+
+ /**
+ * Return whether or not a pref is in the process of being changed via
+ * `about:config`.
+ *
+ * @param {string} pref The pref in question.
+ *
+ * @returns {boolean} Whether or not the pref is being changed.
+ */
+ isBeingChanged(pref) {
+ return this.#changes.has(pref);
+ }
+}
diff --git a/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs b/toolkit/components/nimbus/lib/PrefFlipsFeature.sys.mjs
@@ -2,6 +2,8 @@
* 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 "./ExperimentManager.sys.mjs" */
+
const lazy = {};
ChromeUtils.defineESModuleGetters(lazy, {
@@ -72,6 +74,8 @@ const FEATURE_ID = "prefFlips";
*
* This should *only* be instantiated by the active `ExperimentManager` in the
* parent process.
+ *
+ * @property {ExperimentManager} manager The ExperimentManager that owns this feature.
*/
export class PrefFlipsFeature {
/**
@@ -565,14 +569,17 @@ export class PrefFlipsFeature {
this.#prefs.delete(pref);
Services.prefs.removeObserver(pref, entry.observer);
- const cause = lazy.UnenrollmentCause.ChangedPref({
- name: pref,
- branch: PrefFlipsFeature.determinePrefChangeBranch(
- pref,
- entry.branch,
- entry.value
- ),
- });
+ const cause = lazy.UnenrollmentCause.ChangedPref(
+ {
+ name: pref,
+ branch: PrefFlipsFeature.determinePrefChangeBranch(
+ pref,
+ entry.branch,
+ entry.value
+ ),
+ },
+ this.manager.isPrefBeingChangedViaAboutConfig(pref)
+ );
// Now we can trigger unenrollment of these slugs. Every enrollment settings
// this pref has to stop tracking it.
diff --git a/toolkit/components/nimbus/lib/Telemetry.sys.mjs b/toolkit/components/nimbus/lib/Telemetry.sys.mjs
@@ -242,6 +242,10 @@ export const NimbusTelemetry = {
case UnenrollReason.CHANGED_PREF:
legacyEvent.changedPref = cause.changedPref.name;
gleanEvent.changed_pref = cause.changedPref.name;
+
+ // We've hit the limit of extra keys that can go on the legacy
+ // unenrollment event, so this key does not get mirrored.
+ gleanEvent.about_config_change = cause.isAboutConfigChange;
break;
case UnenrollReason.PREF_FLIPS_CONFLICT:
diff --git a/toolkit/components/nimbus/metrics.yaml b/toolkit/components/nimbus/metrics.yaml
@@ -849,6 +849,11 @@ nimbus_events:
description: >
If reason == "l10n-missing-entry" or "l10n-missing-locale" this is the
locale we failed to find in the set of available translations.
+ about_config_change:
+ description: >
+ For nimbus_experiment, if reason = "changed-pref", whether or not the
+ change was triggerd via about:config.
+ type: string
bugs:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1773563
- https://bugzilla.mozilla.org/show_bug.cgi?id=1781953
@@ -856,6 +861,7 @@ nimbus_events:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1896718
- https://bugzilla.mozilla.org/show_bug.cgi?id=1907649
- https://bugzilla.mozilla.org/show_bug.cgi?id=1976329
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1993057
data_reviews:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1773563
- https://bugzilla.mozilla.org/show_bug.cgi?id=1781953
@@ -863,6 +869,7 @@ nimbus_events:
- https://bugzilla.mozilla.org/show_bug.cgi?id=1896718
- https://bugzilla.mozilla.org/show_bug.cgi?id=1907649
- https://bugzilla.mozilla.org/show_bug.cgi?id=1976329
+ - https://bugzilla.mozilla.org/show_bug.cgi?id=1993057
data_sensitivity:
- technical
notification_emails:
diff --git a/toolkit/components/nimbus/test/browser/browser.toml b/toolkit/components/nimbus/test/browser/browser.toml
@@ -1,5 +1,8 @@
[DEFAULT]
-support-files = ["head.js"]
+support-files = [
+ "head.js",
+ "!/toolkit/components/aboutconfig/test/browser/head.js"
+]
prefs = [
"app.normandy.run_interval_seconds=0", # This turns off the update interval for fetching recipes from Remote Settings
"messaging-system.log=all", # Enable all Nimbus logging.
@@ -11,6 +14,8 @@ skip-if = [
["browser_SharedDataMap.js"]
+["browser_about_config_pref_change.js"]
+
["browser_experiment_evaluate_jexl.js"]
["browser_experimentapi_child.js"]
diff --git a/toolkit/components/nimbus/test/browser/browser_about_config_pref_change.js b/toolkit/components/nimbus/test/browser/browser_about_config_pref_change.js
@@ -0,0 +1,216 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/* import-globals-from ../../../aboutconfig/test/browser/head.js */
+
+"use strict";
+
+Services.scriptloader.loadSubScript(
+ "chrome://mochitests/content/browser/toolkit/components/aboutconfig/test/browser/head.js",
+ this
+);
+
+const {
+ NimbusTelemetry: { UnenrollReason },
+} = ChromeUtils.importESModule("resource://nimbus/lib/Telemetry.sys.mjs");
+
+add_setup(async function setup() {
+ const cleanup = await setupTest();
+ Services.fog.testResetFOG();
+
+ registerCleanupFunction(async function () {
+ await cleanup();
+ Services.fog.testResetFOG();
+ });
+});
+
+const PREF_PREFIX = "nimbus.test-only";
+const BOOL_PREF = `${PREF_PREFIX}.foo`;
+const STRING_PREF = `${PREF_PREFIX}.bar`;
+
+add_task(async function testSetPref() {
+ using disposer = new DisposableStack();
+
+ disposer.defer(
+ NimbusTestUtils.addTestFeatures(
+ new ExperimentFeature("test-bool-feature", {
+ variables: {
+ foo: {
+ type: "boolean",
+ setPref: {
+ branch: "user",
+ pref: BOOL_PREF,
+ },
+ },
+ },
+ }),
+ new ExperimentFeature("test-string-feature", {
+ variables: {
+ bar: {
+ type: "string",
+ setPref: {
+ branch: "user",
+ pref: STRING_PREF,
+ },
+ },
+ },
+ })
+ )
+ );
+
+ disposer.defer(() => Services.prefs.deleteBranch(PREF_PREFIX));
+
+ await NimbusTestUtils.enrollWithFeatureConfig(
+ { featureId: "test-bool-feature", value: { foo: true } },
+ { slug: "bool-recipe" }
+ );
+ await NimbusTestUtils.enrollWithFeatureConfig(
+ { featureId: "test-string-feature", value: { bar: "string" } },
+ { slug: "string-recipe" }
+ );
+
+ await AboutConfigTest.withNewTab(async function () {
+ this.search(PREF_PREFIX);
+
+ const boolRow = this.getRow(BOOL_PREF);
+ boolRow.editColumnButton.click();
+
+ const boolEnrollment = ExperimentAPI.manager.store.get("bool-recipe");
+ Assert.ok(!boolEnrollment.active, "Enrollment is not active");
+ Assert.equal(boolEnrollment.unenrollReason, UnenrollReason.CHANGED_PREF);
+
+ Assert.deepEqual(
+ Glean.nimbusEvents.unenrollment
+ .testGetValue("events")
+ ?.map(ev => ev.extra),
+ [
+ {
+ experiment: "bool-recipe",
+ branch: "control",
+ reason: UnenrollReason.CHANGED_PREF,
+ changed_pref: BOOL_PREF,
+ about_config_change: "true",
+ },
+ ]
+ );
+
+ Services.fog.testResetFOG();
+
+ const stringRow = this.getRow(STRING_PREF);
+ stringRow.editColumnButton.click();
+ stringRow.valueInput.value = "different";
+ stringRow.valueInput.blur();
+ stringRow.editColumnButton.click();
+
+ const stringEnrollment = ExperimentAPI.manager.store.get("string-recipe");
+ Assert.ok(!stringEnrollment.active, "Enrollment is not active");
+ Assert.equal(stringEnrollment.unenrollReason, UnenrollReason.CHANGED_PREF);
+
+ Assert.deepEqual(
+ Glean.nimbusEvents.unenrollment
+ .testGetValue("events")
+ ?.map(ev => ev.extra),
+ [
+ {
+ experiment: "string-recipe",
+ branch: "control",
+ reason: UnenrollReason.CHANGED_PREF,
+ changed_pref: STRING_PREF,
+ about_config_change: "true",
+ },
+ ]
+ );
+
+ Services.fog.testResetFOG();
+ });
+});
+
+add_task(async function testPrefFlips() {
+ using disposer = new DisposableStack();
+ disposer.defer(() => Services.prefs.deleteBranch(PREF_PREFIX));
+
+ await NimbusTestUtils.enrollWithFeatureConfig(
+ {
+ featureId: "prefFlips",
+ value: {
+ prefs: {
+ [BOOL_PREF]: {
+ branch: "user",
+ value: true,
+ },
+ },
+ },
+ },
+ { slug: "bool-flip" }
+ );
+
+ await NimbusTestUtils.enrollWithFeatureConfig(
+ {
+ featureId: "prefFlips",
+ value: {
+ prefs: {
+ [STRING_PREF]: {
+ branch: "user",
+ value: "string",
+ },
+ },
+ },
+ },
+ { slug: "string-flip" }
+ );
+
+ await AboutConfigTest.withNewTab(async function () {
+ this.search(PREF_PREFIX);
+
+ const boolRow = this.getRow(BOOL_PREF);
+ boolRow.editColumnButton.click();
+
+ const boolEnrollment = ExperimentAPI.manager.store.get("bool-flip");
+ Assert.ok(!boolEnrollment.active, "Enrollment is not active");
+ Assert.equal(boolEnrollment.unenrollReason, UnenrollReason.CHANGED_PREF);
+
+ Assert.deepEqual(
+ Glean.nimbusEvents.unenrollment
+ .testGetValue("events")
+ ?.map(ev => ev.extra),
+ [
+ {
+ experiment: "bool-flip",
+ branch: "control",
+ reason: UnenrollReason.CHANGED_PREF,
+ changed_pref: BOOL_PREF,
+ about_config_change: "true",
+ },
+ ]
+ );
+
+ Services.fog.testResetFOG();
+
+ const stringRow = this.getRow(STRING_PREF);
+ stringRow.editColumnButton.click();
+ stringRow.valueInput.value = "different";
+ stringRow.valueInput.blur();
+ stringRow.editColumnButton.click();
+
+ const stringEnrollment = ExperimentAPI.manager.store.get("string-flip");
+ Assert.ok(!stringEnrollment.active, "Enrollment is not active");
+ Assert.equal(stringEnrollment.unenrollReason, UnenrollReason.CHANGED_PREF);
+
+ Assert.deepEqual(
+ Glean.nimbusEvents.unenrollment
+ .testGetValue("events")
+ ?.map(ev => ev.extra),
+ [
+ {
+ experiment: "string-flip",
+ branch: "control",
+ reason: UnenrollReason.CHANGED_PREF,
+ changed_pref: STRING_PREF,
+ about_config_change: "true",
+ },
+ ]
+ );
+
+ Services.fog.testResetFOG();
+ });
+});
diff --git a/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js b/toolkit/components/nimbus/test/unit/test_ExperimentManager_prefs.js
@@ -1821,6 +1821,7 @@ add_task(async function test_prefChange() {
value: slugs[enrollmentKind],
extra: {
reason: "changed-pref",
+ branch: "control",
changedPref: pref,
},
}));
@@ -1828,15 +1829,13 @@ add_task(async function test_prefChange() {
TelemetryTestUtils.assertEvents(expectedLegacyEvents, LEGACY_FILTER);
if (expectedLegacyEvents.length) {
- const processedGleanEvents = gleanEvents.map(event => ({
- reason: event.extra.reason,
- experiment: event.extra.experiment,
- changed_pref: event.extra.changed_pref,
- }));
+ const processedGleanEvents = gleanEvents.map(event => event.extra);
const expectedGleanEvents = expectedLegacyEvents.map(event => ({
experiment: event.value,
+ branch: event.extra.branch,
reason: event.extra.reason,
changed_pref: event.extra.changedPref,
+ about_config_change: "false",
}));
Assert.deepEqual(
diff --git a/toolkit/components/nimbus/test/unit/test_prefFlips.js b/toolkit/components/nimbus/test/unit/test_prefFlips.js
@@ -553,6 +553,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_FOO,
+ about_config_change: "false",
},
},
{
@@ -587,6 +588,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_FOO,
+ about_config_change: "false",
},
},
{
@@ -606,6 +608,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_FOO,
+ about_config_change: "false",
},
},
// Single enrollment case, multiple prefs being reset
@@ -630,6 +633,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_BAR,
+ about_config_change: "false",
},
},
{
@@ -675,6 +679,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_BAR,
+ about_config_change: "false",
},
},
{
@@ -701,6 +706,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_BAR,
+ about_config_change: "false",
},
},
// Multiple enrollment cases
@@ -734,6 +740,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_BAR,
+ about_config_change: "false",
},
},
{
@@ -767,6 +774,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_BAR,
+ about_config_change: "false",
},
},
// - test that we unenroll from all conflicting experiments
@@ -795,6 +803,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_FOO,
+ about_config_change: "false",
},
},
{
@@ -822,6 +831,7 @@ add_task(async function test_prefFlips_unenrollment() {
unenrollReason: UnenrollReason.CHANGED_PREF,
unenrollTelemetry: {
changed_pref: PREF_FOO,
+ about_config_change: "false",
},
},
// - test we unenroll when the experiments conflict with eachother.
