Bug 1287748 - set the lightweight theme directly when Syncing the theme preference. r=Gijs
authorMark Hammond <mhammond@skippinet.com.au>
Thu, 01 Sep 2016 11:57:05 +1000
changeset 313048 d3524b6a9a758bc5ab26cb3e606dd39615bb6cca
parent 313047 d5338a598b481b9c4653c8b64ab70db66b9362b4
child 313049 2f93e05955075697aa425a7bda795af8dbcc2b02
push id20479
push userkwierso@gmail.com
push dateThu, 08 Sep 2016 01:08:46 +0000
treeherderfx-team@fb7c6b034329 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1287748
milestone51.0a1
Bug 1287748 - set the lightweight theme directly when Syncing the theme preference. r=Gijs MozReview-Commit-ID: 9t3nxAitYrC
services/sync/modules/engines/prefs.js
services/sync/tests/unit/test_prefs_store.js
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -105,51 +105,63 @@ PrefStore.prototype = {
       if (this._isSynced(pref)) {
         // Missing and default prefs get the null value.
         values[pref] = this._prefs.isSet(pref) ? this._prefs.get(pref, null) : null;
       }
     }
     return values;
   },
 
+  _updateLightWeightTheme (themeID) {
+    let themeObject = null;
+    if (themeID) {
+      themeObject = LightweightThemeManager.getUsedTheme(themeID);
+    }
+    LightweightThemeManager.currentTheme = themeObject;
+  },
+
   _setAllPrefs: function (values) {
     let selectedThemeIDPref = "lightweightThemes.selectedThemeID";
     let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
+    let selectedThemeIDAfter = selectedThemeIDBefore;
 
     // Update 'services.sync.prefs.sync.foo.pref' before 'foo.pref', otherwise
     // _isSynced returns false when 'foo.pref' doesn't exist (e.g., on a new device).
     let prefs = Object.keys(values).sort(a => -a.indexOf(PREF_SYNC_PREFS_PREFIX));
     for (let pref of prefs) {
       if (!this._isSynced(pref)) {
         continue;
       }
 
       let value = values[pref];
 
-      // Pref has gone missing. The best we can do is reset it.
-      if (value == null) {
-        this._prefs.reset(pref);
-        continue;
-      }
+      switch (pref) {
+        // Some special prefs we don't want to set directly.
+        case selectedThemeIDPref:
+          selectedThemeIDAfter = value;
+          break;
 
-      try {
-        this._prefs.set(pref, value);
-      } catch(ex) {
-        this._log.trace("Failed to set pref: " + pref + ": " + ex);
-      } 
+        // default is to just set the pref
+        default:
+          if (value == null) {
+            // Pref has gone missing. The best we can do is reset it.
+            this._prefs.reset(pref);
+          } else {
+            try {
+              this._prefs.set(pref, value);
+            } catch(ex) {
+              this._log.trace("Failed to set pref: " + pref + ": " + ex);
+            }
+          }
+      }
     }
 
     // Notify the lightweight theme manager if the selected theme has changed.
-    let selectedThemeIDAfter = this._prefs.get(selectedThemeIDPref, null);
     if (selectedThemeIDBefore != selectedThemeIDAfter) {
-      // The currentTheme getter will reflect the theme with the new
-      // selectedThemeID (if there is one).  Just reset it to itself
-      let currentTheme = LightweightThemeManager.currentTheme;
-      LightweightThemeManager.currentTheme = null;
-      LightweightThemeManager.currentTheme = currentTheme;
+      this._updateLightWeightTheme(selectedThemeIDAfter);
     }
   },
 
   getAllIDs: function () {
     /* We store all prefs in just one WBO, with just one GUID */
     let allprefs = {};
     allprefs[PREFS_GUID] = true;
     return allprefs;
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -126,12 +126,43 @@ function run_test() {
     _("Only the current app's preferences are applied.");
     record = new PrefRec("prefs", "some-fake-app");
     record.value = {
       "testing.int": 98
     };
     store.update(record);
     do_check_eq(prefs.get("testing.int"), 42);
 
+    _("The light-weight theme preference is handled correctly.");
+    let lastThemeID = undefined;
+    let orig_updateLightWeightTheme = store._updateLightWeightTheme;
+    store._updateLightWeightTheme = function(themeID) {
+      lastThemeID = themeID;
+    }
+    try {
+      record = new PrefRec("prefs", PREFS_GUID);
+      record.value = {
+        "testing.int": 42,
+      };
+      store.update(record);
+      do_check_true(lastThemeID === undefined,
+                    "should not have tried to change the theme with an unrelated pref.");
+      Services.prefs.setCharPref("lightweightThemes.selectedThemeID", "foo");
+      record.value = {
+        "lightweightThemes.selectedThemeID": "foo",
+      };
+      store.update(record);
+      do_check_true(lastThemeID === undefined,
+                    "should not have tried to change the theme when the incoming pref matches current value.");
+
+      record.value = {
+        "lightweightThemes.selectedThemeID": "bar",
+      };
+      store.update(record);
+      do_check_eq(lastThemeID, "bar",
+                  "should have tried to change the theme when the incoming pref was different.");
+    } finally {
+      store._updateLightWeightTheme = orig_updateLightWeightTheme;
+    }
   } finally {
     prefs.resetBranch("");
   }
 }