Bug 1711218 - Don't undo user pref changes when Normandy experiments end r=Gijs

When a user changes a preference involved in a Normandy experiment, we no
longer immediately end the experiment. This caused a problem because Normandy
didn't check for user changes when resetting prefereces at the end of an
experiment. As a result it would reset all preferences back to their original
values on the specified branch, even if that would mean changing a preference
it knows the user had modified.

This change causes Normandy to never reset the user branch of an experiment
preference that the user has changed. This is true even if the user changes the
preference away from the experimental value and then back to it.

Differential Revision: https://phabricator.services.mozilla.com/D115713

toolkit/components/normandy/lib/PreferenceExperiments.jsm

@@ -792,10 +792,17 @@ var PreferenceExperiments = {
     }
 
     if (resetValue) {
-      for (const [preferenceName, prefInfo] of Object.entries(
-        experiment.preferences
-      )) {
-        const { previousPreferenceValue, preferenceBranchType } = prefInfo;
+      for (const [
+        preferenceName,
+        { previousPreferenceValue, preferenceBranchType, overridden },
+      ] of Object.entries(experiment.preferences)) {
+        // Overridden user prefs should keep their new value, even if that value
+        // is the same as the experimental value, since it is the value the user
+        // chose.
+        if (overridden && preferenceBranchType === "user") {
+          continue;
+        }
+
         const preferences = PreferenceBranchType[preferenceBranchType];
 
         if (previousPreferenceValue !== null) {

toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js

@@ -845,3 +845,70 @@ decorate_task(
     );
   }
 );
+
+// Unenrolling from an experiment where a user has changed some prefs should not override user choice
+decorate_task(
+  withStudiesEnabled(),
+  withMockPreferences(),
+  PreferenceExperiments.withMockExperiments(),
+  async function testUserPrefNoReset({ mockPreferences }) {
+    mockPreferences.set("test.pref.should-reset", "builtin value", "default");
+    mockPreferences.set("test.pref.user-override", "builtin value", "default");
+
+    await PreferenceExperiments.start({
+      slug: "test-experiment",
+      actionName: "PreferenceExperimentAction",
+      isHighPopulation: false,
+      isEnrollmentPaused: false,
+      userFacingName: "Test Experiment",
+      userFacingDescription: "Test description",
+      branch: "test",
+      preferences: {
+        "test.pref.should-reset": {
+          preferenceValue: "experiment value",
+          preferenceType: "string",
+          previousPreferenceValue: "builtin value",
+          preferenceBranchType: "user",
+          overridden: false,
+        },
+        "test.pref.user-override": {
+          preferenceValue: "experiment value",
+          preferenceType: "string",
+          previousPreferenceValue: "builtin value",
+          preferenceBranchType: "user",
+          overridden: false,
+        },
+      },
+    });
+
+    mockPreferences.set("test.pref.user-override", "user value", "user");
+
+    let exp = await PreferenceExperiments.get("test-experiment");
+    is(
+      exp.preferences["test.pref.user-override"].overridden,
+      true,
+      "Changed pref should be marked as overridden"
+    );
+    is(
+      exp.preferences["test.pref.should-reset"].overridden,
+      false,
+      "Unchanged pref should not be marked as overridden"
+    );
+
+    await PreferenceExperiments.stop("test-experiment", {
+      resetValue: true,
+      reason: "test-reason",
+    });
+
+    is(
+      Services.prefs.getCharPref("test.pref.should-reset"),
+      "builtin value",
+      "pref that was not overridden should reset to builtin"
+    );
+    is(
+      Services.prefs.getCharPref("test.pref.user-override"),
+      "user value",
+      "pref that was overridden should keep user value"
+    );
+  }
+);