Bug 1547034: Migrate PreferenceExperiments to allow for multiple prefs r=mythmon,leplatrem
authorEthan Glasser-Camp <ethan@betacantrips.com>
Thu, 16 May 2019 15:04:12 +0000
changeset 474245 137380671f01c6185f61a04b3fd55aac9617da4a
parent 474244 8189bd3d8078b3155a425043b34f6975e9a799d8
child 474246 da4380fe5ce1ea4d7dba885f1127fac61860552e
push id36027
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:24:38 +0000
treeherdermozilla-central@c94c54aff466 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmythmon, leplatrem
bugs1547034
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1547034: Migrate PreferenceExperiments to allow for multiple prefs r=mythmon,leplatrem This is part 1 of the required changes. This just addresses the storage mechanism and any place that uses experiments in their raw form. This updates most callers to support studies with multiple preferences. We update about-studies to assume only one preference. This seems counterproductive, but studies with multiple preferences will include a description field that obviates the need for this. Differential Revision: https://phabricator.services.mozilla.com/D29293
toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
toolkit/components/normandy/content/about-studies/about-studies.js
toolkit/components/normandy/lib/PreferenceExperiments.jsm
toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
toolkit/components/normandy/test/browser/browser_about_studies.js
toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
toolkit/components/normandy/test/browser/head.js
tools/lint/eslint/modules.json
--- a/toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
+++ b/toolkit/components/normandy/actions/PreferenceExperimentAction.jsm
@@ -52,17 +52,17 @@ class PreferenceExperimentAction extends
 
     this.seenExperimentNames.push(slug);
 
     // If we're not in the experiment, enroll!
     const hasSlug = await PreferenceExperiments.has(slug);
     if (!hasSlug) {
       // If there's already an active experiment using this preference, abort.
       const activeExperiments = await PreferenceExperiments.getAllActive();
-      const hasConflicts = activeExperiments.some(exp => exp.preferenceName === preferenceName);
+      const hasConflicts = activeExperiments.some(exp => exp.preferences.hasOwnProperty(preferenceName));
       if (hasConflicts) {
         throw new Error(
           `Experiment ${slug} ignored; another active experiment is already using the
           ${preferenceName} preference.`
         );
       }
 
       // Determine if enrollment is currently paused for this experiment.
--- a/toolkit/components/normandy/content/about-studies/about-studies.js
+++ b/toolkit/components/normandy/content/about-studies/about-studies.js
@@ -246,24 +246,26 @@ class PreferenceStudyListItem extends Re
       experimentName: this.props.study.name,
       reason: "individual-opt-out",
     });
   }
 
   render() {
     const { study, translations } = this.props;
 
+    // Assume there is exactly one preference (old-style preference experiment).
+    const [preferenceName, { preferenceValue }] = Object.entries(study.preferences)[0];
     // Sanitize the values by setting them as the text content of an element,
     // and then getting the HTML representation of that text. This will have the
     // browser safely sanitize them. Use outerHTML to also include the <code>
     // element in the string.
     const sanitizer = document.createElement("code");
-    sanitizer.textContent = study.preferenceName;
+    sanitizer.textContent = preferenceName;
     const sanitizedPreferenceName = sanitizer.outerHTML;
-    sanitizer.textContent = study.preferenceValue;
+    sanitizer.textContent = preferenceValue;
     const sanitizedPreferenceValue = sanitizer.outerHTML;
     const description = translations.preferenceStudyDescription
       .replace(/%(?:1\$)?S/, sanitizedPreferenceName)
       .replace(/%(?:2\$)?S/, sanitizedPreferenceValue);
 
     return (
       r("li", {
         className: classnames("study pref-study", { disabled: study.expired }),
--- a/toolkit/components/normandy/lib/PreferenceExperiments.jsm
+++ b/toolkit/components/normandy/lib/PreferenceExperiments.jsm
@@ -14,58 +14,64 @@
  * Active preference experiments are stopped if they aren't active on the recipe
  * server. They also expire if Firefox isn't able to contact the recipe server
  * after a period of time, as well as if the user modifies the preference during
  * an active experiment.
  */
 
 /**
  * Experiments store info about an active or expired preference experiment.
- * They are single-depth objects to simplify cloning.
  * @typedef {Object} Experiment
  * @property {string} name
  *   Unique name of the experiment
  * @property {string} branch
  *   Experiment branch that the user was matched to
  * @property {boolean} expired
  *   If false, the experiment is active.
  * @property {string} lastSeen
  *   ISO-formatted date string of when the experiment was last seen from the
  *   recipe server.
- * @property {string} preferenceName
- *   Name of the preference affected by this experiment.
+ * @property {Object} preferences
+ *   An object consisting of all the preferences that are set by this experiment.
+ *   Keys are the name of each preference affected by this experiment.
+ *   Values are Preference Objects, about which see below.
+ * @property {string} experimentType
+ *   The type to report to Telemetry's experiment marker API.
+ */
+
+/**
+ * Each Preference stores information about a preference that an
+ * experiment sets.
  * @property {string|integer|boolean} preferenceValue
  *   Value to change the preference to during the experiment.
  * @property {string} preferenceType
  *   Type of the preference value being set.
  * @property {string|integer|boolean|undefined} previousPreferenceValue
  *   Value of the preference prior to the experiment, or undefined if it was
  *   unset.
  * @property {PreferenceBranchType} preferenceBranchType
  *   Controls how we modify the preference to affect the client.
  *
  *   If "default", when the experiment is active, the default value for the
  *   preference is modified on startup of the add-on. If "user", the user value
  *   for the preference is modified when the experiment starts, and is reset to
  *   its original value when the experiment ends.
- * @property {string} experimentType
- *   The type to report to Telemetry's experiment marker API.
  */
 
 "use strict";
 
 ChromeUtils.defineModuleGetter(this, "Services", "resource://gre/modules/Services.jsm");
 ChromeUtils.defineModuleGetter(this, "CleanupManager", "resource://normandy/lib/CleanupManager.jsm");
 ChromeUtils.defineModuleGetter(this, "JSONFile", "resource://gre/modules/JSONFile.jsm");
 ChromeUtils.defineModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
 ChromeUtils.defineModuleGetter(this, "LogManager", "resource://normandy/lib/LogManager.jsm");
 ChromeUtils.defineModuleGetter(this, "TelemetryEnvironment", "resource://gre/modules/TelemetryEnvironment.jsm");
 ChromeUtils.defineModuleGetter(this, "TelemetryEvents", "resource://normandy/lib/TelemetryEvents.jsm");
 
-var EXPORTED_SYMBOLS = ["PreferenceExperiments"];
+var EXPORTED_SYMBOLS = ["PreferenceExperiments", "migrateStorage"];
 
 const EXPERIMENT_FILE = "shield-preference-experiments.json";
 const STARTUP_EXPERIMENT_PREFS_BRANCH = "app.normandy.startupExperimentPrefs.";
 
 const MAX_EXPERIMENT_TYPE_LENGTH = 20; // enforced by TelemetryEnvironment
 const EXPERIMENT_TYPE_PREFIX = "normandy-";
 const MAX_EXPERIMENT_SUBTYPE_LENGTH = MAX_EXPERIMENT_TYPE_LENGTH - EXPERIMENT_TYPE_PREFIX.length;
 
@@ -90,21 +96,76 @@ const PreferenceBranchType = {
 /**
  * Asynchronously load the JSON file that stores experiment status in the profile.
  */
 let gStorePromise;
 function ensureStorage() {
   if (gStorePromise === undefined) {
     const path = OS.Path.join(OS.Constants.Path.profileDir, EXPERIMENT_FILE);
     const storage = new JSONFile({path});
-    gStorePromise = storage.load().then(() => storage);
+    gStorePromise = storage.load().then(() => {
+      migrateStorage(storage);
+      return storage;
+    });
   }
   return gStorePromise;
 }
 
+/**
+ * Migrate storage of experiments from old format (one preference per
+ * experiment) to new format.
+ *
+ * This function is exported for testing purposes but should not be
+ * called otherwise.
+ */
+function migrateStorage(storage) {
+  if (storage.data.__version == 2) {
+    return;
+  }
+  const newData = {
+    __version: 2,
+    experiments: {},
+  };
+  for (let [expName, experiment] of Object.entries(storage.data)) {
+    if (expName == "__version") {
+      continue;
+    }
+
+    const {
+      name,
+      branch,
+      expired,
+      lastSeen,
+      preferenceName,
+      preferenceValue,
+      preferenceType,
+      previousPreferenceValue,
+      preferenceBranchType,
+      experimentType,
+    } = experiment;
+    const newExperiment = {
+      name,
+      branch,
+      expired,
+      lastSeen,
+      preferences: {
+        [preferenceName]: {
+          preferenceBranchType,
+          preferenceType,
+          preferenceValue,
+          previousPreferenceValue,
+        },
+      },
+      experimentType,
+    };
+    newData.experiments[expName] = newExperiment;
+  }
+  storage.data = newData;
+}
+
 const log = LogManager.getLogger("preference-experiments");
 
 // List of active preference observers. Cleaned up on shutdown.
 let experimentObservers = new Map();
 CleanupManager.addCleanupHandler(() => PreferenceExperiments.stopAllObservers());
 
 function getPref(prefBranch, prefName, prefType) {
   if (prefBranch.getPrefType(prefName) === 0) {
@@ -150,25 +211,27 @@ function setPref(prefBranch, prefName, p
 var PreferenceExperiments = {
   /**
    * Update the the experiment storage with changes that happened during early startup.
    * @param {object} studyPrefsChanged Map from pref name to previous pref value
    */
   async recordOriginalValues(studyPrefsChanged) {
     const store = await ensureStorage();
 
-    for (const experiment of Object.values(store.data)) {
-      if (studyPrefsChanged.hasOwnProperty(experiment.preferenceName)) {
-        if (experiment.expired) {
-          log.warn("Expired preference experiment changed value during startup");
+    for (const experiment of Object.values(store.data.experiments)) {
+      for (const [prefName, prefInfo] of Object.entries(experiment.preferences)) {
+        if (studyPrefsChanged.hasOwnProperty(prefName)) {
+          if (experiment.expired) {
+            log.warn("Expired preference experiment changed value during startup");
+          }
+          if (prefInfo.preferenceBranch !== "default") {
+            log.warn("Non-default branch preference experiment changed value during startup");
+          }
+          prefInfo.previousPreferenceValue = studyPrefsChanged[prefName];
         }
-        if (experiment.branch !== "default") {
-          log.warn("Non-default branch preference experiment changed value during startup");
-        }
-        experiment.previousPreferenceValue = studyPrefsChanged[experiment.preferenceName];
       }
     }
 
     // not calling store.saveSoon() because if the data doesn't get
     // written, it will get updated with fresher data next time the
     // browser starts.
   },
 
@@ -176,35 +239,44 @@ var PreferenceExperiments = {
    * Set the default preference value for active experiments that use the
    * default preference branch.
    */
   async init() {
     CleanupManager.addCleanupHandler(this.saveStartupPrefs.bind(this));
 
     for (const experiment of await this.getAllActive()) {
       // Check that the current value of the preference is still what we set it to
-      if (getPref(UserPreferences, experiment.preferenceName, experiment.preferenceType) !== experiment.preferenceValue) {
-        // if not, stop the experiment, and skip the remaining steps
-        log.info(`Stopping experiment "${experiment.name}" because its value changed`);
-        await this.stop(experiment.name, {
-          resetValue: false,
-          reason: "user-preference-changed-sideload",
-        });
+      let stopped = false;
+      for (const [prefName, prefInfo] of Object.entries(experiment.preferences)) {
+        if (getPref(UserPreferences, prefName, prefInfo.preferenceType) !== prefInfo.preferenceValue) {
+          // if not, stop the experiment, and skip the remaining steps
+          log.info(`Stopping experiment "${experiment.name}" because its value changed`);
+          await this.stop(experiment.name, {
+            resetValue: false,
+            reason: "user-preference-changed-sideload",
+          });
+          stopped = true;
+          break;
+        }
+      }
+      if (stopped) {
         continue;
       }
 
       // Notify Telemetry of experiments we're running, since they don't persist between restarts
       TelemetryEnvironment.setExperimentActive(
         experiment.name,
         experiment.branch,
         {type: EXPERIMENT_TYPE_PREFIX + experiment.experimentType}
       );
 
-      // Watch for changes to the experiment's preference
-      this.startObserver(experiment.name, experiment.preferenceName, experiment.preferenceType, experiment.preferenceValue);
+      for (const [prefName, prefInfo] of Object.entries(experiment.preferences)) {
+        // Watch for changes to the experiment's preference
+        this.startObserver(experiment.name, prefName, prefInfo.preferenceType, prefInfo.preferenceValue);
+      }
     }
   },
 
   /**
    * Save in-progress, default-branch preference experiments in a sub-branch of
    * the normandy preferences. On startup, we read these to set the
    * experimental values.
    *
@@ -215,23 +287,25 @@ var PreferenceExperiments = {
    * impact on the performance of startup.
    */
   async saveStartupPrefs() {
     const prefBranch = Services.prefs.getBranch(STARTUP_EXPERIMENT_PREFS_BRANCH);
     for (const pref of prefBranch.getChildList("")) {
       prefBranch.clearUserPref(pref);
     }
 
-    // Filter out non-default-branch experiments (user-branch), because they
-    // don't need to be set on the default branch during early startup. Doing so
-    // would make the user branch and the default branch the same, which would
-    // cause the user branch to not be saved, and the user branch preference
-    // would be erased.
-    const defaultBranchExperiments = (await this.getAllActive()).filter(exp => exp.preferenceBranchType === "default");
-    for (const {preferenceName, preferenceValue} of defaultBranchExperiments) {
+    // Only store prefs to set on the default branch.
+    // Be careful not to store user branch prefs here, because this
+    // would cause the default branch to match the user branch,
+    // causing the user branch pref to get cleared.
+    const allExperiments = await this.getAllActive();
+    const defaultBranchPrefs =
+          allExperiments.flatMap(exp => Object.entries(exp.preferences))
+          .filter(([preferenceName, preferenceInfo]) => preferenceInfo.preferenceBranchType === "default");
+    for (const [preferenceName, {preferenceValue}] of defaultBranchPrefs) {
       switch (typeof preferenceValue) {
         case "string":
           prefBranch.setCharPref(preferenceName, preferenceValue);
           break;
 
         case "number":
           prefBranch.setIntPref(preferenceName, preferenceValue);
           break;
@@ -248,21 +322,25 @@ var PreferenceExperiments = {
 
   /**
    * Test wrapper that temporarily replaces the stored experiment data with fake
    * data for testing.
    */
   withMockExperiments(mockExperiments = []) {
     return function wrapper(testFunction) {
       return async function wrappedTestFunction(...args) {
-        const data = {};
+        const experiments = {};
 
         for (const exp of mockExperiments) {
-          data[exp.name] = exp;
+          experiments[exp.name] = exp;
         }
+        const data = {
+          __version: 2,
+          experiments,
+        };
 
         const oldPromise = gStorePromise;
         gStorePromise = Promise.resolve({
           data,
           saveSoon() { },
         });
         const oldObservers = experimentObservers;
         experimentObservers = new Map();
@@ -277,17 +355,20 @@ var PreferenceExperiments = {
     };
   },
 
   /**
    * Clear all stored data about active and past experiments.
    */
   async clearAllExperimentStorage() {
     const store = await ensureStorage();
-    store.data = {};
+    store.data = {
+      __version: 2,
+      experiments: {},
+    };
     store.saveSoon();
   },
 
   /**
    * Start a new preference experiment.
    * @param {Object} experiment
    * @param {string} experiment.name
    * @param {string} experiment.branch
@@ -306,24 +387,24 @@ var PreferenceExperiments = {
     preferenceValue,
     preferenceBranchType,
     preferenceType,
     experimentType = "exp",
   }) {
     log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
 
     const store = await ensureStorage();
-    if (name in store.data) {
+    if (name in store.data.experiments) {
       TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "name-conflict"});
       throw new Error(`A preference experiment named "${name}" already exists.`);
     }
 
-    const activeExperiments = Object.values(store.data).filter(e => !e.expired);
+    const activeExperiments = Object.values(store.data.experiments).filter(e => !e.expired);
     const hasConflictingExperiment = activeExperiments.some(
-      e => e.preferenceName === preferenceName
+      e => e.preferences.hasOwnProperty(preferenceName)
     );
     if (hasConflictingExperiment) {
       TelemetryEvents.sendEvent("enrollFailed", "preference_study", name, {reason: "pref-conflict"});
       throw new Error(
         `Another preference experiment for the pref "${preferenceName}" is currently active.`
       );
     }
 
@@ -358,27 +439,30 @@ var PreferenceExperiments = {
     }
 
     /** @type {Experiment} */
     const experiment = {
       name,
       branch,
       expired: false,
       lastSeen: new Date().toJSON(),
-      preferenceName,
-      preferenceValue,
-      preferenceType,
-      previousPreferenceValue: getPref(preferences, preferenceName, preferenceType),
-      preferenceBranchType,
+      preferences: {
+        [preferenceName]: {
+          preferenceBranchType,
+          preferenceValue,
+          preferenceType,
+          previousPreferenceValue: getPref(preferences, preferenceName, preferenceType),
+        },
+      },
       experimentType,
     };
 
     setPref(preferences, preferenceName, preferenceType, preferenceValue);
     PreferenceExperiments.startObserver(name, preferenceName, preferenceType, preferenceValue);
-    store.data[name] = experiment;
+    store.data.experiments[name] = experiment;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentActive(name, branch, {type: EXPERIMENT_TYPE_PREFIX + experimentType});
     TelemetryEvents.sendEvent("enroll", "preference_study", name, {experimentType, branch});
     await this.saveStartupPrefs();
   },
 
   /**
@@ -460,21 +544,21 @@ var PreferenceExperiments = {
    * @param {string} experimentName
    * @rejects {Error}
    *   If there is no stored experiment with the given name.
    */
   async markLastSeen(experimentName) {
     log.debug(`PreferenceExperiments.markLastSeen(${experimentName})`);
 
     const store = await ensureStorage();
-    if (!(experimentName in store.data)) {
+    if (!(experimentName in store.data.experiments)) {
       throw new Error(`Could not find a preference experiment named "${experimentName}"`);
     }
 
-    store.data[experimentName].lastSeen = new Date().toJSON();
+    store.data.experiments[experimentName].lastSeen = new Date().toJSON();
     store.saveSoon();
   },
 
   /**
    * Stop an active experiment, deactivate preference watchers, and optionally
    * reset the associated preference to its previous value.
    * @param {string} experimentName
    * @param {Object} options
@@ -490,107 +574,122 @@ var PreferenceExperiments = {
    */
   async stop(experimentName, {resetValue = true, reason = "unknown"} = {}) {
     log.debug(`PreferenceExperiments.stop(${experimentName}, {resetValue: ${resetValue}, reason: ${reason}})`);
     if (reason === "unknown") {
       log.warn(`experiment ${experimentName} ending for unknown reason`);
     }
 
     const store = await ensureStorage();
-    if (!(experimentName in store.data)) {
+    if (!(experimentName in store.data.experiments)) {
       TelemetryEvents.sendEvent("unenrollFailed", "preference_study", experimentName, {reason: "does-not-exist"});
       throw new Error(`Could not find a preference experiment named "${experimentName}"`);
     }
 
-    const experiment = store.data[experimentName];
+    const experiment = store.data.experiments[experimentName];
     if (experiment.expired) {
       TelemetryEvents.sendEvent("unenrollFailed", "preference_study", experimentName, {reason: "already-unenrolled"});
       throw new Error(
         `Cannot stop preference experiment "${experimentName}" because it is already expired`
       );
     }
 
     if (PreferenceExperiments.hasObserver(experimentName)) {
       PreferenceExperiments.stopObserver(experimentName);
     }
 
     if (resetValue) {
-      const {preferenceName, preferenceType, previousPreferenceValue, preferenceBranchType} = experiment;
-      const preferences = PreferenceBranchType[preferenceBranchType];
+      for (const [preferenceName, prefInfo] of Object.entries(experiment.preferences)) {
+        const {preferenceType, previousPreferenceValue, preferenceBranchType} = prefInfo;
+        const preferences = PreferenceBranchType[preferenceBranchType];
 
-      if (previousPreferenceValue !== null) {
-        setPref(preferences, preferenceName, preferenceType, previousPreferenceValue);
-      } else if (preferenceBranchType === "user") {
-        // Remove the "user set" value (which Shield set), but leave the default intact.
-        preferences.clearUserPref(preferenceName);
-      } else {
-        log.warn(
-          `Can't revert pref for experiment ${experimentName} because it had no default value. `
-          + `Preference will be reset at the next restart.`
-        );
-        // It would seem that Services.prefs.deleteBranch() could be used for
-        // this, but in Normandy's case it does not work. See bug 1502410.
+        if (previousPreferenceValue !== null) {
+          setPref(preferences, preferenceName, preferenceType, previousPreferenceValue);
+        } else if (preferenceBranchType === "user") {
+          // Remove the "user set" value (which Shield set), but leave the default intact.
+          preferences.clearUserPref(preferenceName);
+        } else {
+          log.warn(
+            `Can't revert pref ${preferenceName} for experiment ${experimentName} `
+              + `because it had no default value. `
+              + `Preference will be reset at the next restart.`
+          );
+          // It would seem that Services.prefs.deleteBranch() could be used for
+          // this, but in Normandy's case it does not work. See bug 1502410.
+        }
       }
     }
 
     experiment.expired = true;
     store.saveSoon();
 
     TelemetryEnvironment.setExperimentInactive(experimentName);
     TelemetryEvents.sendEvent("unenroll", "preference_study", experimentName, {
       didResetValue: resetValue ? "true" : "false",
       branch: experiment.branch,
       reason,
     });
     await this.saveStartupPrefs();
   },
 
   /**
+   * Clone an experiment using knowledge of its structure to avoid
+   * having to serialize/deserialize it.
+   *
+   * We do this in places where return experiments so clients can't
+   * accidentally mutate our data underneath us.
+   */
+  _cloneExperiment(experiment) {
+    return {
+      ...experiment,
+      preferences: {
+        ...experiment.preferences,
+      },
+    };
+  },
+
+  /**
    * Get the experiment object for the named experiment.
    * @param {string} experimentName
    * @resolves {Experiment}
    * @rejects {Error}
    *   If no preference experiment exists with the given name.
    */
   async get(experimentName) {
     log.debug(`PreferenceExperiments.get(${experimentName})`);
     const store = await ensureStorage();
-    if (!(experimentName in store.data)) {
+    if (!(experimentName in store.data.experiments)) {
       throw new Error(`Could not find a preference experiment named "${experimentName}"`);
     }
 
-    // Return a copy so mutating it doesn't affect the storage.
-    return Object.assign({}, store.data[experimentName]);
+    return this._cloneExperiment(store.data.experiments[experimentName]);
   },
 
   /**
    * Get a list of all stored experiment objects.
    * @resolves {Experiment[]}
    */
   async getAll() {
     const store = await ensureStorage();
 
-    // Return copies so that mutating returned experiments doesn't affect the
-    // stored values.
-    return Object.values(store.data).map(experiment => Object.assign({}, experiment));
+    return Object.values(store.data.experiments).map(experiment => this._cloneExperiment(experiment));
   },
 
   /**
   * Get a list of experiment objects for all active experiments.
   * @resolves {Experiment[]}
   */
   async getAllActive() {
     const store = await ensureStorage();
-    // Return copies so mutating them doesn't affect the storage.
-    return Object.values(store.data).filter(e => !e.expired).map(e => Object.assign({}, e));
+    return Object.values(store.data.experiments).filter(e => !e.expired).map(e => this._cloneExperiment(e));
   },
 
   /**
    * Check if an experiment exists with the given name.
    * @param {string} experimentName
    * @resolves {boolean} True if the experiment exists, false if it doesn't.
    */
   async has(experimentName) {
     log.debug(`PreferenceExperiments.has(${experimentName})`);
     const store = await ensureStorage();
-    return experimentName in store.data;
+    return experimentName in store.data.experiments;
   },
 };
--- a/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
+++ b/toolkit/components/normandy/test/browser/browser_PreferenceExperiments.js
@@ -7,29 +7,123 @@ ChromeUtils.import("resource://normandy/
 ChromeUtils.import("resource://normandy/lib/TelemetryEvents.jsm", this);
 
 // Save ourselves some typing
 const {withMockExperiments} = PreferenceExperiments;
 const DefaultPreferences = new Preferences({defaultBranch: true});
 const startupPrefs = "app.normandy.startupExperimentPrefs";
 
 function experimentFactory(attrs) {
+  const defaultPref = {
+    "fake.preference": {},
+  };
+  const defaultPrefInfo = {
+    preferenceValue: "fakevalue",
+    preferenceType: "string",
+    previousPreferenceValue: "oldfakevalue",
+    preferenceBranchType: "default",
+  };
+  const preferences = {};
+  for (const [prefName, prefInfo] of Object.entries(attrs.preferences || defaultPref)) {
+    preferences[prefName] = { ...defaultPrefInfo, ...prefInfo };
+  }
+
   return Object.assign({
     name: "fakename",
     branch: "fakebranch",
     expired: false,
     lastSeen: new Date().toJSON(),
-    preferenceName: "fake.preference",
-    preferenceValue: "fakevalue",
-    preferenceType: "string",
-    previousPreferenceValue: "oldfakevalue",
+    experimentType: "exp",
+  }, attrs, {
+    preferences,
+  });
+}
+
+const mockOldData = {
+  "hypothetical_experiment": {
+    name: "hypothetical_experiment",
+    branch: "hypo_1",
+    expired: false,
+    lastSeen: new Date().toJSON(),
+    preferenceName: "some.pref",
+    preferenceValue: 2,
+    preferenceType: "integer",
+    previousPreferenceValue: 1,
+    preferenceBranchType: "user",
+    experimentType: "exp",
+  },
+  "another_experiment": {
+    name: "another_experiment",
+    branch: "another_4",
+    expired: true,
+    lastSeen: new Date().toJSON(),
+    preferenceName: "another.pref",
+    preferenceValue: true,
+    preferenceType: "boolean",
+    previousPreferenceValue: false,
     preferenceBranchType: "default",
     experimentType: "exp",
-  }, attrs);
-}
+  },
+};
+
+add_task(function migrateStorage_migrates_to_new_format() {
+  const mockJsonFile = {
+    // Deep clone the data in case migrateStorage mutates it.
+    data: JSON.parse(JSON.stringify(mockOldData)),
+  };
+  migrateStorage(mockJsonFile);
+  Assert.deepEqual(mockJsonFile.data, {
+    __version: 2,
+    experiments: {
+      "hypothetical_experiment": {
+        name: "hypothetical_experiment",
+        branch: "hypo_1",
+        expired: false,
+        lastSeen: mockOldData.hypothetical_experiment.lastSeen,
+        preferences: {
+          "some.pref": {
+            preferenceValue: 2,
+            preferenceType: "integer",
+            previousPreferenceValue: 1,
+            preferenceBranchType: "user",
+          },
+        },
+        experimentType: "exp",
+      },
+      "another_experiment": {
+        name: "another_experiment",
+        branch: "another_4",
+        expired: true,
+        lastSeen: mockOldData.another_experiment.lastSeen,
+        preferences: {
+          "another.pref": {
+            preferenceValue: true,
+            preferenceType: "boolean",
+            previousPreferenceValue: false,
+            preferenceBranchType: "default",
+          },
+        },
+        experimentType: "exp",
+      },
+    },
+  });
+});
+
+add_task(function migrateStorage_is_idempotent() {
+  const mockJsonFileOnce = {
+    data: JSON.parse(JSON.stringify(mockOldData)),
+  };
+  const mockJsonFileTwice = {
+    data: JSON.parse(JSON.stringify(mockOldData)),
+  };
+  migrateStorage(mockJsonFileOnce);
+  migrateStorage(mockJsonFileTwice);
+  migrateStorage(mockJsonFileTwice);
+  Assert.deepEqual(mockJsonFileOnce, mockJsonFileTwice);
+});
 
 // clearAllExperimentStorage
 decorate_task(
   withMockExperiments([experimentFactory({ name: "test" })]),
   async function(experiments) {
     ok(await PreferenceExperiments.has("test"), "Mock experiment is detected.");
     await PreferenceExperiments.clearAllExperimentStorage();
     ok(
@@ -60,17 +154,17 @@ decorate_task(
     sendEventStub.assertEvents(
       [["enrollFailed", "preference_study", "test", {reason: "name-conflict"}]]
     );
   }
 );
 
 // start should throw if an experiment for the given preference is active
 decorate_task(
-  withMockExperiments([experimentFactory({ name: "test", preferenceName: "fake.preference" })]),
+  withMockExperiments([experimentFactory({ name: "test", preferences: {"fake.preference": {}} })]),
   withSendEventStub,
   async function(experiments, sendEventStub) {
     await Assert.rejects(
       PreferenceExperiments.start({
         name: "different",
         branch: "branch",
         preferenceName: "fake.preference",
         preferenceValue: "value",
@@ -135,21 +229,24 @@ decorate_task(
       startObserverStub.calledWith("test", "fake.preference", "string", "newvalue"),
       "start registered an observer",
     );
 
     const expectedExperiment = {
       name: "test",
       branch: "branch",
       expired: false,
-      preferenceName: "fake.preference",
-      preferenceValue: "newvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "default",
+      preferences: {
+        "fake.preference": {
+          preferenceValue: "newvalue",
+          preferenceType: "string",
+          previousPreferenceValue: "oldvalue",
+          preferenceBranchType: "default",
+        },
+      },
     };
     const experiment = {};
     const actualExperiment = await PreferenceExperiments.get("test");
     Object.keys(expectedExperiment).forEach(key => experiment[key] = actualExperiment[key]);
     Assert.deepEqual(experiment, expectedExperiment, "start saved the experiment");
 
     is(
       DefaultPreferences.get("fake.preference"),
@@ -190,21 +287,24 @@ decorate_task(
       startObserver.calledWith("test", "fake.preference", "string", "newvalue"),
       "start registered an observer",
     );
 
     const expectedExperiment = {
       name: "test",
       branch: "branch",
       expired: false,
-      preferenceName: "fake.preference",
-      preferenceValue: "newvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "user",
+      preferences: {
+        "fake.preference": {
+          preferenceValue: "newvalue",
+          preferenceType: "string",
+          previousPreferenceValue: "oldvalue",
+          preferenceBranchType: "user",
+        },
+      },
     };
 
     const experiment = {};
     const actualExperiment = await PreferenceExperiments.get("test");
     Object.keys(expectedExperiment).forEach(key => experiment[key] = actualExperiment[key]);
     Assert.deepEqual(experiment, expectedExperiment, "start saved the experiment");
 
     Assert.notEqual(
@@ -442,21 +542,24 @@ decorate_task(
 // stop should mark the experiment as expired, stop its observer, and revert the
 // preference value.
 decorate_task(
   withMockExperiments([
     experimentFactory({
       name: "test",
       expired: false,
       branch: "fakebranch",
-      preferenceName: "fake.preference",
-      preferenceValue: "experimentvalue",
-      preferenceType: "string",
-      previousPreferenceValue: "oldvalue",
-      preferenceBranchType: "default",
+      preferences: {
+        "fake.preference": {
+          preferenceValue: "experimentvalue",
+          preferenceType: "string",
+          previousPreferenceValue: "oldvalue",
+          preferenceBranchType: "default",
+        },
+      },
     }),
   ]),
   withMockPreferences,
   withSpy(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStop(experiments, mockPreferences, stopObserverSpy, sendEventStub) {
     // this assertion is mostly useful for --verify test runs, to make
     // sure that tests clean up correctly.
@@ -492,21 +595,24 @@ decorate_task(
   },
 );
 
 // stop should also support user pref experiments
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
     expired: false,
-    preferenceName: "fake.preference",
-    preferenceValue: "experimentvalue",
-    preferenceType: "string",
-    previousPreferenceValue: "oldvalue",
-    preferenceBranchType: "user",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experimentvalue",
+        preferenceType: "string",
+        previousPreferenceValue: "oldvalue",
+        preferenceBranchType: "user",
+      },
+    },
   })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withStub(PreferenceExperiments, "hasObserver"),
   async function testStopUserPrefs(experiments, mockPreferences, stopObserver, hasObserver) {
     hasObserver.returns(true);
 
     mockPreferences.set("fake.preference", "experimentvalue", "user");
@@ -526,21 +632,24 @@ decorate_task(
   }
 );
 
 // stop should remove a preference that had no value prior to an experiment for user prefs
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
     expired: false,
-    preferenceName: "fake.preference",
-    preferenceValue: "experimentvalue",
-    preferenceType: "string",
-    previousPreferenceValue: null,
-    preferenceBranchType: "user",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experimentvalue",
+        preferenceType: "string",
+        previousPreferenceValue: null,
+        preferenceBranchType: "user",
+      },
+    },
   })]),
   withMockPreferences,
   async function(experiments, mockPreferences) {
     const stopObserver = sinon.stub(PreferenceExperiments, "stopObserver");
     mockPreferences.set("fake.preference", "experimentvalue", "user");
 
     await PreferenceExperiments.stop("test");
     ok(
@@ -553,21 +662,24 @@ decorate_task(
 );
 
 // stop should not modify a preference if resetValue is false
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
     expired: false,
     branch: "fakebranch",
-    preferenceName: "fake.preference",
-    preferenceValue: "experimentvalue",
-    preferenceType: "string",
-    previousPreferenceValue: "oldvalue",
-    preferenceBranchType: "default",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experimentvalue",
+        preferenceType: "string",
+        previousPreferenceValue: "oldvalue",
+        preferenceBranchType: "default",
+      },
+    },
   })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopReset(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "customvalue", "default");
 
     await PreferenceExperiments.stop("test", {reason: "test-reason", resetValue: false});
@@ -678,20 +790,23 @@ decorate_task(
   }
 );
 
 // init should register telemetry experiments
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
     branch: "branch",
-    preferenceName: "fake.pref",
-    preferenceValue: "experiment value",
-    expired: false,
-    preferenceBranchType: "default",
+    preferences: {
+      "fake.pref": {
+        preferenceValue: "experiment value",
+        expired: false,
+        preferenceBranchType: "default",
+      },
+    },
   })]),
   withMockPreferences,
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(PreferenceExperiments, "startObserver"),
   async function testInit(experiments, mockPreferences, setActiveStub, startObserverStub) {
     mockPreferences.set("fake.pref", "experiment value");
     await PreferenceExperiments.init();
     ok(
@@ -701,18 +816,21 @@ decorate_task(
   },
 );
 
 // init should use the provided experiment type
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
     branch: "branch",
-    preferenceName: "fake.pref",
-    preferenceValue: "experiment value",
+    preferences: {
+      "fake.pref": {
+        preferenceValue: "experiment value",
+      },
+    },
     experimentType: "pref-test",
   })]),
   withMockPreferences,
   withStub(TelemetryEnvironment, "setExperimentActive"),
   withStub(PreferenceExperiments, "startObserver"),
   async function testInit(experiments, mockPreferences, setActiveStub, startObserverStub) {
     mockPreferences.set("fake.pref", "experiment value");
     await PreferenceExperiments.init();
@@ -808,18 +926,21 @@ decorate_task(
     ok(!setActiveStub.called, "Expired experiment is not registered by init");
   },
 );
 
 // Experiments should end if the preference has been changed when init() is called
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
-    preferenceName: "fake.preference",
-    preferenceValue: "experiment value",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experiment value",
+      },
+    },
   })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stop"),
   async function testInitChanges(experiments, mockPreferences, stopStub) {
     mockPreferences.set("fake.preference", "experiment value", "default");
     mockPreferences.set("fake.preference", "changed value", "user");
     await PreferenceExperiments.init();
 
@@ -835,18 +956,21 @@ decorate_task(
     );
   },
 );
 
 // init should register an observer for experiments
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
-    preferenceName: "fake.preference",
-    preferenceValue: "experiment value",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experiment value",
+      },
+    },
   })]),
   withMockPreferences,
   withStub(PreferenceExperiments, "startObserver"),
   withStub(PreferenceExperiments, "stop"),
   withStub(CleanupManager, "addCleanupHandler"),
   async function testInitRegistersObserver(experiments, mockPreferences, startObserver, stop) {
     stop.throws("Stop should not be called");
     mockPreferences.set("fake.preference", "experiment value", "default");
@@ -862,28 +986,37 @@ decorate_task(
   }
 );
 
 // saveStartupPrefs
 decorate_task(
   withMockExperiments([
     experimentFactory({
       name: "char",
-      preferenceName: `fake.char`,
-      preferenceValue: "string",
+      preferences: {
+        "fake.char": {
+          preferenceValue: "string",
+        },
+      },
     }),
     experimentFactory({
       name: "int",
-      preferenceName: `fake.int`,
-      preferenceValue: 2,
+      preferences: {
+        "fake.int": {
+          preferenceValue: 2,
+        },
+      },
     }),
     experimentFactory({
       name: "bool",
-      preferenceName: `fake.bool`,
-      preferenceValue: true,
+      preferences: {
+        "fake.bool": {
+          preferenceValue: true,
+        },
+      },
     }),
   ]),
   async function testSaveStartupPrefs(experiments) {
     Services.prefs.deleteBranch(startupPrefs);
     Services.prefs.setBoolPref(`${startupPrefs}.fake.old`, true);
     await PreferenceExperiments.saveStartupPrefs();
 
     ok(
@@ -906,42 +1039,51 @@ decorate_task(
     );
   },
 );
 
 // saveStartupPrefs errors for invalid pref type
 decorate_task(
   withMockExperiments([experimentFactory({
     name: "test",
-    preferenceName: "fake.invalidValue",
-    preferenceValue: new Date(),
+    preferences: {
+      "fake.invalidValue": {
+        preferenceValue: new Date(),
+      },
+    },
   })]),
   async function testSaveStartupPrefsError(experiments) {
     await Assert.rejects(
       PreferenceExperiments.saveStartupPrefs(),
       /invalid preference type/i,
       "saveStartupPrefs throws if an experiment has an invalid preference value type",
     );
   },
 );
 
 // saveStartupPrefs should not store values for user-branch recipes
 decorate_task(
   withMockExperiments([
     experimentFactory({
       name: "defaultBranchRecipe",
-      preferenceName: "fake.default",
-      preferenceValue: "experiment value",
-      preferenceBranchType: "default",
+      preferences: {
+        "fake.default": {
+          preferenceValue: "experiment value",
+          preferenceBranchType: "default",
+        },
+      },
     }),
     experimentFactory({
       name: "userBranchRecipe",
-      preferenceName: "fake.user",
-      preferenceValue: "experiment value",
-      preferenceBranchType: "user",
+      preferences: {
+        "fake.user": {
+          preferenceValue: "experiment value",
+          preferenceBranchType: "user",
+        },
+      },
     }),
   ]),
   async function testSaveStartupPrefsUserBranch(experiments) {
     Assert.deepEqual(Services.prefs.getChildList(startupPrefs), [], "As a prerequisite no startup prefs are set");
 
     await PreferenceExperiments.saveStartupPrefs();
 
     Assert.deepEqual(
@@ -1056,17 +1198,17 @@ decorate_task(
       "DEFAULT",
       "Preference should be absent",
     );
   },
 ).skip(/* bug 1502410 and bug 1505941 */);
 
 // stop should pass "unknown" to telemetry event for `reason` if none is specified
 decorate_task(
-  withMockExperiments([experimentFactory({ name: "test", preferenceName: "fake.preference" })]),
+  withMockExperiments([experimentFactory({ name: "test", preferences: { "fake.preference": {} }})]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopUnknownReason(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference", "default value", "default");
     await PreferenceExperiments.stop("test");
     is(
       sendEventStub.getCall(0).args[3].reason,
@@ -1074,18 +1216,18 @@ decorate_task(
       "PreferenceExperiments.stop() should use unknown as the default reason",
     );
   }
 );
 
 // stop should pass along the value for resetValue to Telemetry Events as didResetValue
 decorate_task(
   withMockExperiments([
-    experimentFactory({ name: "test1", preferenceName: "fake.preference1" }),
-    experimentFactory({ name: "test2", preferenceName: "fake.preference2" }),
+    experimentFactory({ name: "test1", preferences: { "fake.preference1": {} }}),
+    experimentFactory({ name: "test2", preferences: { "fake.preference2": {} }}),
   ]),
   withMockPreferences,
   withStub(PreferenceExperiments, "stopObserver"),
   withSendEventStub,
   async function testStopResetValue(experiments, mockPreferences, stopObserverStub, sendEventStub) {
     mockPreferences.set("fake.preference1", "default value", "default");
     await PreferenceExperiments.stop("test1", { resetValue: true });
     is(sendEventStub.callCount, 1);
@@ -1110,21 +1252,24 @@ decorate_task(
 // the user changed preferences during a browser run.
 decorate_task(
   withMockPreferences,
   withSendEventStub,
   withMockExperiments([experimentFactory({
     name: "test",
     expired: false,
     branch: "fakebranch",
-    preferenceName: "fake.preference",
-    preferenceValue: "experimentvalue",
-    preferenceType: "string",
-    previousPreferenceValue: "oldvalue",
-    preferenceBranchType: "default",
+    preferences: {
+      "fake.preference": {
+        preferenceValue: "experimentvalue",
+        preferenceType: "string",
+        previousPreferenceValue: "oldvalue",
+        preferenceBranchType: "default",
+      },
+    },
   })]),
   async function testPrefChangeEventTelemetry(mockPreferences, sendEventStub, mockExperiments) {
     is(Preferences.get("fake.preference"), null, "preference should start unset");
     mockPreferences.set("fake.preference", "oldvalue", "default");
     PreferenceExperiments.startObserver("test", "fake.preference", "string", "experimentvalue");
 
     // setting the preference on the user branch should trigger the observer to stop the experiment
     mockPreferences.set("fake.preference", "uservalue", "user");
--- a/toolkit/components/normandy/test/browser/browser_about_studies.js
+++ b/toolkit/components/normandy/test/browser/browser_about_studies.js
@@ -167,18 +167,19 @@ decorate_task(
         "Inactive studies are marked as complete."
       );
       ok(
         !inactiveAddonStudy.querySelector(".remove-button"),
         "Inactive studies do not show a remove button"
       );
 
       const activePrefStudy = getStudyRow(doc, prefStudies[0].name);
+      const preferenceName = Object.keys(prefStudies[0].preferences)[0];
       ok(
-        activePrefStudy.querySelector(".study-description").textContent.includes(prefStudies[0].preferenceName),
+        activePrefStudy.querySelector(".study-description").textContent.includes(preferenceName),
         "Preference studies show the preference they are changing"
       );
       is(
         activePrefStudy.querySelector(".study-status").textContent,
         "Active",
         "Active studies show an 'Active' indicator."
       );
       ok(
--- a/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
+++ b/toolkit/components/normandy/test/browser/browser_actions_PreferenceExperimentAction.js
@@ -175,17 +175,19 @@ decorate_task(
 );
 
 decorate_task(
   withStub(PreferenceExperiments, "start"),
   withStub(Uptake, "reportRecipe"),
   PreferenceExperiments.withMockExperiments([
     {
       name: "conflict",
-      preferenceName: "conflict.pref",
+      preferences: {
+        "conflict.pref": {},
+      },
       expired: false,
     },
   ]),
   async function do_nothing_if_preference_is_already_being_tested(startStub, reportRecipeStub) {
     const action = new PreferenceExperimentAction();
     const recipe = preferenceExperimentFactory({
       slug: "new",
       preferenceName: "conflict.pref",
--- a/toolkit/components/normandy/test/browser/head.js
+++ b/toolkit/components/normandy/test/browser/head.js
@@ -279,28 +279,39 @@ this.addonStudyFactory = function(attrs)
     extensionApiId: 1,
     extensionHash: "ade1c14196ec4fe0aa0a6ba40ac433d7c8d1ec985581a8a94d43dc58991b5171",
     extensionHashAlgorithm: "sha256",
   }, attrs);
 };
 
 let _preferenceStudyFactoryId = 0;
 this.preferenceStudyFactory = function(attrs) {
+  const defaultPref = {
+    "test.study": {},
+  };
+  const defaultPrefInfo = {
+    preferenceValue: false,
+    preferenceType: "boolean",
+    previousPreferenceValue: undefined,
+    preferenceBranchType: "default",
+  };
+  const preferences = {};
+  for (const [prefName, prefInfo] of Object.entries(attrs.preferences || defaultPref)) {
+    preferences[prefName] = { ...defaultPrefInfo, ...prefInfo };
+  }
+
   return Object.assign({
     name: `Test study ${_preferenceStudyFactoryId++}`,
     branch: "control",
     expired: false,
     lastSeen: new Date().toJSON(),
-    preferenceName: "test.study",
-    preferenceValue: false,
-    preferenceType: "boolean",
-    previousPreferenceValue: undefined,
-    preferenceBranchType: "default",
     experimentType: "exp",
-  }, attrs);
+  }, attrs, {
+    preferences,
+  });
 };
 
 this.withStub = function(...stubArgs) {
   return function wrapper(testFunction) {
     return async function wrappedTestFunction(...args) {
       const stub = sinon.stub(...stubArgs);
       try {
         await testFunction(...args, stub);
--- a/tools/lint/eslint/modules.json
+++ b/tools/lint/eslint/modules.json
@@ -159,16 +159,17 @@
   "Parser.jsm": ["Parser", "ParserHelpers", "SyntaxTreeVisitor"],
   "passwords.js": ["PasswordEngine", "LoginRec", "PasswordValidator"],
   "passwords.jsm": ["Password", "DumpPasswords"],
   "PdfJsNetwork.jsm": ["NetworkManager"],
   "PhoneNumberMetaData.jsm": ["PHONE_NUMBER_META_DATA"],
   "PluginProvider.jsm": [],
   "PointerAdapter.jsm": ["PointerRelay", "PointerAdapter"],
   "policies.js": ["ErrorHandler", "SyncScheduler"],
+  "PreferenceExperiments.jsm": ["PreferenceExperiments", "migrateStorage"],
   "prefs.js": ["Branch", "MarionettePrefs"],
   "prefs.js": ["PrefsEngine", "PrefRec"],
   "prefs.jsm": ["Preference"],
   "pretty-fast.js": ["prettyFast"],
   "profiler_get_symbols.js": ["wasm_bindgen"],
   "PromiseWorker.jsm": ["BasePromiseWorker"],
   "PushCrypto.jsm": ["PushCrypto", "concatArray"],
   "quit.js": ["goQuitApplication"],