Bug 753289 - Apply prefs-related prefs before applying other synced prefs. r=rnewman
authoronemen <tabmix.onemen@gmail.com>
Thu, 10 Sep 2015 17:33:50 +0100
changeset 294279 06f396c07ac9ae10158eec2834faf4d5febbb167
parent 294278 4378a00214c5b591c4c4dce77e03a87619539f64
child 294280 e4da9601cb2b354baab50e3b5c437fcb54a9bf6c
push id5245
push userraliiev@mozilla.com
push dateThu, 29 Oct 2015 11:30:51 +0000
treeherdermozilla-beta@dac831dc1bd0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs753289
milestone43.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 753289 - Apply prefs-related prefs before applying other synced prefs. r=rnewman
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
@@ -38,30 +38,30 @@ PrefsEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: PrefStore,
   _trackerObj: PrefTracker,
   _recordObj: PrefRec,
   version: 2,
 
   syncPriority: 1,
 
-  getChangedIDs: function getChangedIDs() {
+  getChangedIDs: function () {
     // No need for a proper timestamp (no conflict resolution needed).
     let changedIDs = {};
     if (this._tracker.modified)
       changedIDs[PREFS_GUID] = 0;
     return changedIDs;
   },
 
-  _wipeClient: function _wipeClient() {
+  _wipeClient: function () {
     SyncEngine.prototype._wipeClient.call(this);
     this.justWiped = true;
   },
 
-  _reconcile: function _reconcile(item) {
+  _reconcile: function (item) {
     // Apply the incoming item if we don't care about the local data
     if (this.justWiped) {
       this.justWiped = false;
       return true;
     }
     return SyncEngine.prototype._reconcile.call(this, item);
   }
 };
@@ -73,56 +73,63 @@ function PrefStore(name, engine) {
     this.__prefs = null;
   }, this);
 }
 PrefStore.prototype = {
   __proto__: Store.prototype,
 
  __prefs: null,
   get _prefs() {
-    if (!this.__prefs)
+    if (!this.__prefs) {
       this.__prefs = new Preferences();
+    }
     return this.__prefs;
   },
 
-  _getSyncPrefs: function _getSyncPrefs() {
+  _getSyncPrefs: function () {
     let syncPrefs = Cc["@mozilla.org/preferences-service;1"]
                       .getService(Ci.nsIPrefService)
                       .getBranch(PREF_SYNC_PREFS_PREFIX)
                       .getChildList("", {});
     // Also sync preferences that determine which prefs get synced.
-    return syncPrefs.concat(
-      syncPrefs.map(function (pref) { return PREF_SYNC_PREFS_PREFIX + pref; }));
+    let controlPrefs = syncPrefs.map(pref => PREF_SYNC_PREFS_PREFIX + pref);
+    return controlPrefs.concat(syncPrefs);
   },
 
-  _isSynced: function _isSyncedPref(pref) {
-    return (pref.indexOf(PREF_SYNC_PREFS_PREFIX) == 0)
-            || this._prefs.get(PREF_SYNC_PREFS_PREFIX + pref, false);
+  _isSynced: function (pref) {
+    return pref.startsWith(PREF_SYNC_PREFS_PREFIX) ||
+           this._prefs.get(PREF_SYNC_PREFS_PREFIX + pref, false);
   },
 
   _getAllPrefs: function () {
     let values = {};
     for each (let pref in this._getSyncPrefs()) {
       if (this._isSynced(pref)) {
         // Missing prefs get the null value.
         values[pref] = this._prefs.get(pref, null);
       }
     }
     return values;
   },
 
-  _setAllPrefs: function PrefStore__setAllPrefs(values) {
+  _setAllPrefs: function (values) {
     let selectedThemeIDPref = "lightweightThemes.selectedThemeID";
     let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
 
-    for (let [pref, value] in Iterator(values)) {
-      if (!this._isSynced(pref))
+    // 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;
+      }
 
-      // Pref has gone missing, best we can do is reset it.
+      let value = values[pref];
+
+      // Pref has gone missing. The best we can do is reset it.
       if (value == null) {
         this._prefs.reset(pref);
         continue;
       }
 
       try {
         this._prefs.set(pref, value);
       } catch(ex) {
@@ -136,61 +143,61 @@ PrefStore.prototype = {
       // 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;
     }
   },
 
-  getAllIDs: function PrefStore_getAllIDs() {
+  getAllIDs: function () {
     /* We store all prefs in just one WBO, with just one GUID */
     let allprefs = {};
     allprefs[PREFS_GUID] = true;
     return allprefs;
   },
 
-  changeItemID: function PrefStore_changeItemID(oldID, newID) {
+  changeItemID: function (oldID, newID) {
     this._log.trace("PrefStore GUID is constant!");
   },
 
-  itemExists: function FormStore_itemExists(id) {
+  itemExists: function (id) {
     return (id === PREFS_GUID);
   },
 
-  createRecord: function createRecord(id, collection) {
+  createRecord: function (id, collection) {
     let record = new PrefRec(collection, id);
 
     if (id == PREFS_GUID) {
       record.value = this._getAllPrefs();
     } else {
       record.deleted = true;
     }
 
     return record;
   },
 
-  create: function PrefStore_create(record) {
+  create: function (record) {
     this._log.trace("Ignoring create request");
   },
 
-  remove: function PrefStore_remove(record) {
+  remove: function (record) {
     this._log.trace("Ignoring remove request");
   },
 
-  update: function PrefStore_update(record) {
+  update: function (record) {
     // Silently ignore pref updates that are for other apps.
     if (record.id != PREFS_GUID)
       return;
 
     this._log.trace("Received pref updates, applying...");
     this._setAllPrefs(record.value);
   },
 
-  wipe: function PrefStore_wipe() {
+  wipe: function () {
     this._log.trace("Ignoring wipe request");
   }
 };
 
 function PrefTracker(name, engine) {
   Tracker.call(this, name, engine);
   Svc.Obs.add("profile-before-change", this);
   Svc.Obs.add("weave:engine:start-tracking", this);
--- a/services/sync/tests/unit/test_prefs_store.js
+++ b/services/sync/tests/unit/test_prefs_store.js
@@ -76,24 +76,26 @@ function run_test() {
     _("Update some prefs, including one that's to be reset/deleted.");
     Svc.Prefs.set("testing.deleteme", "I'm going to be deleted!");
     record = new PrefRec("prefs", PREFS_GUID);
     record.value = {
       "testing.int": 42,
       "testing.string": "im in ur prefs",
       "testing.bool": false,
       "testing.deleteme": null,
+      "testing.somepref": "im a new pref from other device",
       "services.sync.prefs.sync.testing.somepref": true
     };
     store.update(record);
     do_check_eq(prefs.get("testing.int"), 42);
     do_check_eq(prefs.get("testing.string"), "im in ur prefs");
     do_check_eq(prefs.get("testing.bool"), false);
     do_check_eq(prefs.get("testing.deleteme"), undefined);
     do_check_eq(prefs.get("testing.dont.change"), "Please don't change me.");
+    do_check_eq(prefs.get("testing.somepref"), "im a new pref from other device");
     do_check_eq(Svc.Prefs.get("prefs.sync.testing.somepref"), true);
 
     _("Enable persona");
     // Ensure we don't go to the network to fetch personas and end up leaking
     // stuff.
     Services.io.offline = true;
     do_check_false(!!prefs.get("lightweightThemes.selectedThemeID"));
     do_check_eq(LightweightThemeManager.currentTheme, null);