Bug 1416003 - Handle preference studies on prefs with only user-branch values r=Gijs,a=sylvestre,ship57,etc
authorMike Cooper <mcooper@mozilla.com>
Thu, 09 Nov 2017 16:29:26 -0800
changeset 435357 8deb291f300b843a76cf31b30b9b44b341887c76
parent 435356 a5a689f01f0ce0dc4b61711bfc7f992e666bf7d9
child 435358 74203b7f9e0403df654f154583948ac53e0c6da4
push id1581
push usersledru@mozilla.com
push dateSun, 12 Nov 2017 08:27:45 +0000
treeherdermozilla-release@8deb291f300b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs, sylvestre, ship57, etc
bugs1416003
milestone57.0
Bug 1416003 - Handle preference studies on prefs with only user-branch values r=Gijs,a=sylvestre,ship57,etc CLOSED TREE to bypass the close tree because I do want to ship 57. MozReview-Commit-ID: BBReL4bEjPY
browser/extensions/shield-recipe-client/bootstrap.js
browser/extensions/shield-recipe-client/test/browser/browser_bootstrap.js
--- a/browser/extensions/shield-recipe-client/bootstrap.js
+++ b/browser/extensions/shield-recipe-client/bootstrap.js
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * 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/. */
 "use strict";
 
-const {utils: Cu} = Components;
+const {results: Cr, utils: Cu} = Components;
 Cu.import("resource://gre/modules/AppConstants.jsm");
 Cu.import("resource://gre/modules/Log.jsm");
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "LogManager",
   "resource://shield-recipe-client/lib/LogManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ShieldRecipeClient",
@@ -77,36 +77,46 @@ this.Bootstrap = {
       const realPrefType = defaultBranch.getPrefType(prefName);
 
       if (realPrefType !== Services.prefs.PREF_INVALID && realPrefType !== experimentPrefType) {
         log.error(`Error setting startup pref ${prefName}; pref type does not match.`);
         continue;
       }
 
       // record the value of the default branch before setting it
-      switch (realPrefType) {
-        case Services.prefs.PREF_STRING:
-          studyPrefsChanged[prefName] = defaultBranch.getCharPref(prefName);
-          break;
+      try {
+        switch (realPrefType) {
+          case Services.prefs.PREF_STRING:
+            studyPrefsChanged[prefName] = defaultBranch.getCharPref(prefName);
+            break;
 
-        case Services.prefs.PREF_INT:
-          studyPrefsChanged[prefName] = defaultBranch.getIntPref(prefName);
-          break;
+          case Services.prefs.PREF_INT:
+            studyPrefsChanged[prefName] = defaultBranch.getIntPref(prefName);
+            break;
+
+          case Services.prefs.PREF_BOOL:
+            studyPrefsChanged[prefName] = defaultBranch.getBoolPref(prefName);
+            break;
 
-        case Services.prefs.PREF_BOOL:
-          studyPrefsChanged[prefName] = defaultBranch.getBoolPref(prefName);
-          break;
+          case Services.prefs.PREF_INVALID:
+            studyPrefsChanged[prefName] = null;
+            break;
 
-        case Services.prefs.PREF_INVALID:
+          default:
+            // This should never happen
+            log.error(`Error getting startup pref ${prefName}; unknown value type ${experimentPrefType}.`);
+        }
+      } catch (e) {
+        if (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.
           studyPrefsChanged[prefName] = null;
-          break;
-
-        default:
-          // This should never happen
-          log.error(`Error getting startup pref ${prefName}; unknown value type ${experimentPrefType}.`);
+        } else {
+          // rethrow
+          throw e;
+        }
       }
 
       // now set the new default value
       switch (experimentPrefType) {
         case Services.prefs.PREF_STRING:
           defaultBranch.setCharPref(prefName, experimentBranch.getCharPref(prefName));
           break;
 
--- a/browser/extensions/shield-recipe-client/test/browser/browser_bootstrap.js
+++ b/browser/extensions/shield-recipe-client/test/browser/browser_bootstrap.js
@@ -247,8 +247,24 @@ decorate_task(
         [experimentPref2]: 1,
         [experimentPref3]: "original string",
         [experimentPref4]: null,  // null because it was not initially set.
       }],
       "finishStartup should record original values of the prefs initExperimentPrefs changed",
     );
   },
 );
+
+// Test that startup prefs are handled correctly when there is a value on the user branch but not the default branch.
+decorate_task(
+  withPrefEnv({
+    set: [
+      ["extensions.shield-recipe-client.startupExperimentPrefs.testing.does-not-exist", "foo"],
+      ["testing.does-not-exist", "foo"],
+    ],
+  }),
+  withBootstrap,
+  withStub(PreferenceExperiments, "recordOriginalValues"),
+  async function testInitExperimentPrefsNoDefaultValue(Bootstrap) {
+    Bootstrap.initExperimentPrefs();
+    ok(true, "initExperimentPrefs should not throw for non-existant prefs");
+  },
+);