Bug 1458856 - Handle prefs with only a user value in preference rollout r?Gijs draft
authorMike Cooper <mcooper@mozilla.com>
Thu, 03 May 2018 13:38:30 -0700
changeset 791248 ef93ee94ce8553c9a82a3d9823ce87ca9dea7248
parent 791136 35165f43d0b97f500dafb4f0f7b9ca8d91416812
push id108757
push userbmo:mcooper@mozilla.com
push dateThu, 03 May 2018 21:21:48 +0000
reviewersGijs
bugs1458856
milestone61.0a1
Bug 1458856 - Handle prefs with only a user value in preference rollout r?Gijs MozReview-Commit-ID: 5t99YT4lJED
toolkit/components/normandy/lib/PrefUtils.jsm
toolkit/components/normandy/test/browser/browser_actions_PreferenceRolloutAction.js
--- a/toolkit/components/normandy/lib/PrefUtils.jsm
+++ b/toolkit/components/normandy/lib/PrefUtils.jsm
@@ -18,34 +18,44 @@ var PrefUtils = {
    * @param {string} branchName One of "default" or "user"
    * @param {string} pref
    * @param {string|boolean|integer|null} [default]
    *   The value to return if the preference does not exist. Defaults to null.
    */
   getPref(branchName, pref, defaultValue = null) {
     const branch = kPrefBranches[branchName];
     const type = branch.getPrefType(pref);
-    switch (type) {
-      case Services.prefs.PREF_BOOL: {
-        return branch.getBoolPref(pref);
+
+    try {
+      switch (type) {
+        case Services.prefs.PREF_BOOL: {
+          return branch.getBoolPref(pref);
+        }
+        case Services.prefs.PREF_STRING: {
+          return branch.getStringPref(pref);
+        }
+        case Services.prefs.PREF_INT: {
+          return branch.getIntPref(pref);
+        }
+        case Services.prefs.PREF_INVALID: {
+          return defaultValue;
+        }
       }
-      case Services.prefs.PREF_STRING: {
-        return branch.getStringPref(pref);
-      }
-      case Services.prefs.PREF_INT: {
-        return branch.getIntPref(pref);
-      }
-      case Services.prefs.PREF_INVALID: {
+    } catch (e) {
+      if (branchName === "default" && e.result === Cr.NS_ERROR_UNEXPECTED) {
+        // There is a value for the pref on the user branch but not on the default branch. This is ok.
         return defaultValue;
       }
-      default: {
-        // This should never happen
-        throw new TypeError(`Unknown preference type (${type}) for ${pref}.`);
-      }
+      // Unexpected error, re-throw it
+      throw e;
     }
+
+    // If `type` isn't any of the above, throw an error. Don't do this in a
+    // default branch of switch so that error handling is easier.
+    throw new TypeError(`Unknown preference type (${type}) for ${pref}.`);
   },
 
   /**
    * Set a preference on the named branch
    * @param {string} branchName One of "default" or "user"
    * @param {string} pref
    * @param {string|boolean|integer|null} value
    *   The value to set. Must match the type named in `type`.
--- a/toolkit/components/normandy/test/browser/browser_actions_PreferenceRolloutAction.js
+++ b/toolkit/components/normandy/test/browser/browser_actions_PreferenceRolloutAction.js
@@ -352,8 +352,36 @@ decorate_task(
       "the rollout is added to the db with the correct previous value",
     );
 
     // Cleanup
     Services.prefs.getDefaultBranch("").deleteBranch("test.pref");
     Services.prefs.deleteBranch("test.pref");
   },
 );
+
+// Enrollment works for prefs with only a user branch value, and no default value.
+decorate_task(
+  PreferenceRollouts.withTestMock,
+  async function simple_recipe_enrollment(setExperimentActiveStub, sendEventStub) {
+    const recipe = {
+      id: 1,
+      arguments: {
+        slug: "test-rollout",
+        preferences: [{preferenceName: "test.pref", value: 1}],
+      },
+    };
+
+    // Set a pref on the user branch only
+    Services.prefs.setIntPref("test.pref", 2);
+
+    const action = new PreferenceRolloutAction();
+    await action.runRecipe(recipe);
+    await action.finalize();
+
+    is(Services.prefs.getIntPref("test.pref"), 2, "original user branch value still visible");
+    is(Services.prefs.getDefaultBranch("").getIntPref("test.pref"), 1, "default branch was set");
+    is(Services.prefs.getIntPref("app.normandy.startupRolloutPrefs.test.pref"), 1, "startup pref is est");
+
+    // Cleanup
+    Services.prefs.getDefaultBranch("").deleteBranch("test.pref");
+  },
+);