Bug 675397 - Add syncing priority to be able to reshuffle the default syncing order. r=rnewman
authorMathias De Maré <mathias.demare@gmail.com>
Wed, 01 Oct 2014 08:44:52 +0200
changeset 208717 4339077e7826b456717a0a2a115812914b55a85f
parent 208716 649217a03879447535e6c8ebef3756e0f42236a1
child 208718 d1244379f050e03782e72416c9f2511f05a067c2
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersrnewman
bugs675397
milestone35.0a1
Bug 675397 - Add syncing priority to be able to reshuffle the default syncing order. r=rnewman
services/sync/modules/engines.js
services/sync/modules/engines/addons.js
services/sync/modules/engines/bookmarks.js
services/sync/modules/engines/forms.js
services/sync/modules/engines/history.js
services/sync/modules/engines/passwords.js
services/sync/modules/engines/prefs.js
services/sync/modules/engines/tabs.js
services/sync/tests/unit/head_helpers.js
services/sync/tests/unit/test_enginemanager.js
services/sync/tests/unit/test_keys.js
--- a/services/sync/modules/engines.js
+++ b/services/sync/modules/engines.js
@@ -478,17 +478,19 @@ EngineManager.prototype = {
   getAll: function () {
     return [engine for ([name, engine] in Iterator(this._engines))];
   },
 
   /**
    * N.B., does not pay attention to the declined list.
    */
   getEnabled: function () {
-    return this.getAll().filter(function(engine) engine.enabled);
+    return this.getAll()
+               .filter((engine) => engine.enabled)
+               .sort((a, b) => a.syncPriority - b.syncPriority);
   },
 
   get enabledEngineNames() {
     return [e.name for each (e in this.getEnabled())];
   },
 
   persistDeclined: function () {
     Svc.Prefs.set("declinedEngines", [...this._declined].join(","));
@@ -695,16 +697,23 @@ SyncEngine.kRecoveryStrategy = {
   error:  "error"
 };
 
 SyncEngine.prototype = {
   __proto__: Engine.prototype,
   _recordObj: CryptoWrapper,
   version: 1,
 
+  // A relative priority to use when computing an order
+  // for engines to be synced. Higher-priority engines
+  // (lower numbers) are synced first.
+  // It is recommended that a unique value be used for each engine,
+  // in order to guarantee a stable sequence.
+  syncPriority: 0,
+
   // How many records to pull in a single sync. This is primarily to avoid very
   // long first syncs against profiles with many history records.
   downloadLimit: null,
 
   // How many records to pull at one time when specifying IDs. This is to avoid
   // URI length limitations.
   guidFetchBatchSize: DEFAULT_GUID_FETCH_BATCH_SIZE,
   mobileGUIDFetchBatchSize: DEFAULT_MOBILE_GUID_FETCH_BATCH_SIZE,
--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -114,16 +114,18 @@ this.AddonsEngine = function AddonsEngin
 }
 AddonsEngine.prototype = {
   __proto__:              SyncEngine.prototype,
   _storeObj:              AddonsStore,
   _trackerObj:            AddonsTracker,
   _recordObj:             AddonRecord,
   version:                1,
 
+  syncPriority:           5,
+
   _reconciler:            null,
 
   /**
    * Override parent method to find add-ons by their public ID, not Sync GUID.
    */
   _findDupe: function _findDupe(item) {
     let id = item.addonID;
 
--- a/services/sync/modules/engines/bookmarks.js
+++ b/services/sync/modules/engines/bookmarks.js
@@ -198,16 +198,18 @@ this.BookmarksEngine = function Bookmark
 }
 BookmarksEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: PlacesItem,
   _storeObj: BookmarksStore,
   _trackerObj: BookmarksTracker,
   version: 2,
 
+  syncPriority: 4,
+
   _sync: function _sync() {
     let engine = this;
     let batchEx = null;
 
     // Try running sync in batch mode
     PlacesUtils.bookmarks.runInBatchMode({
       runBatched: function wrappedSync() {
         try {
--- a/services/sync/modules/engines/forms.js
+++ b/services/sync/modules/engines/forms.js
@@ -102,16 +102,18 @@ this.FormEngine = function FormEngine(se
 }
 FormEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: FormStore,
   _trackerObj: FormTracker,
   _recordObj: FormRec,
   applyIncomingBatchSize: FORMS_STORE_BATCH_SIZE,
 
+  syncPriority: 6,
+
   get prefName() "history",
 
   _findDupe: function _findDupe(item) {
     return FormWrapper.getGUID(item.name, item.value);
   }
 };
 
 function FormStore(name, engine) {
--- a/services/sync/modules/engines/history.js
+++ b/services/sync/modules/engines/history.js
@@ -36,17 +36,19 @@ this.HistoryEngine = function HistoryEng
   SyncEngine.call(this, "History", service);
 }
 HistoryEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _recordObj: HistoryRec,
   _storeObj: HistoryStore,
   _trackerObj: HistoryTracker,
   downloadLimit: MAX_HISTORY_DOWNLOAD,
-  applyIncomingBatchSize: HISTORY_STORE_BATCH_SIZE
+  applyIncomingBatchSize: HISTORY_STORE_BATCH_SIZE,
+
+  syncPriority: 7,
 };
 
 function HistoryStore(name, engine) {
   Store.call(this, name, engine);
 
   // Explicitly nullify our references to our cached services so we don't leak
   Svc.Obs.add("places-shutdown", function() {
     for each ([query, stmt] in Iterator(this._stmts)) {
--- a/services/sync/modules/engines/passwords.js
+++ b/services/sync/modules/engines/passwords.js
@@ -31,16 +31,18 @@ this.PasswordEngine = function PasswordE
 }
 PasswordEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: PasswordStore,
   _trackerObj: PasswordTracker,
   _recordObj: LoginRec,
   applyIncomingBatchSize: PASSWORDS_STORE_BATCH_SIZE,
 
+  syncPriority: 2,
+
   _syncFinish: function _syncFinish() {
     SyncEngine.prototype._syncFinish.call(this);
 
     // Delete the weave credentials from the server once
     if (!Svc.Prefs.get("deletePwdFxA", false)) {
       try {
         let ids = [];
         for (let host of Utils.getSyncCredentialsHosts()) {
--- a/services/sync/modules/engines/prefs.js
+++ b/services/sync/modules/engines/prefs.js
@@ -36,16 +36,18 @@ this.PrefsEngine = function PrefsEngine(
 }
 PrefsEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: PrefStore,
   _trackerObj: PrefTracker,
   _recordObj: PrefRec,
   version: 2,
 
+  syncPriority: 1,
+
   getChangedIDs: function getChangedIDs() {
     // No need for a proper timestamp (no conflict resolution needed).
     let changedIDs = {};
     if (this._tracker.modified)
       changedIDs[PREFS_GUID] = 0;
     return changedIDs;
   },
 
--- a/services/sync/modules/engines/tabs.js
+++ b/services/sync/modules/engines/tabs.js
@@ -40,16 +40,18 @@ this.TabEngine = function TabEngine(serv
   this._resetClient();
 }
 TabEngine.prototype = {
   __proto__: SyncEngine.prototype,
   _storeObj: TabStore,
   _trackerObj: TabTracker,
   _recordObj: TabSetRecord,
 
+  syncPriority: 3,
+
   getChangedIDs: function getChangedIDs() {
     // No need for a proper timestamp (no conflict resolution needed).
     let changedIDs = {};
     if (this._tracker.modified)
       changedIDs[this.service.clientsEngine.localID] = 0;
     return changedIDs;
   },
 
--- a/services/sync/tests/unit/head_helpers.js
+++ b/services/sync/tests/unit/head_helpers.js
@@ -187,8 +187,16 @@ function mockGetWindowEnumerator(url, nu
     hasMoreElements: function () {
       return elements.length;
     },
     getNext: function () {
       return elements.shift();
     },
   };
 }
+
+// Helper that allows checking array equality.
+function do_check_array_eq(a1, a2) {
+  do_check_eq(a1.length, a2.length);
+  for (let i = 0; i < a1.length; ++i) {
+    do_check_eq(a1[i], a2[i]);
+  }
+}
--- a/services/sync/tests/unit/test_enginemanager.js
+++ b/services/sync/tests/unit/test_enginemanager.js
@@ -70,16 +70,33 @@ add_test(function test_basics() {
   do_check_eq(engines.length, 1);
   do_check_eq(engines[0], petrol);
 
   dummy.enabled = true;
   diesel.enabled = true;
   engines = manager.getEnabled();
   do_check_eq(engines.length, 3);
 
+  _("getEnabled() returns enabled engines in sorted order");
+  petrol.syncPriority = 1;
+  dummy.syncPriority = 2;
+  diesel.syncPriority = 3;
+
+  engines = manager.getEnabled();
+
+  do_check_array_eq(engines, [petrol, dummy, diesel]);
+
+  _("Changing the priorities should change the order in getEnabled()");
+
+  dummy.syncPriority = 4;
+
+  engines = manager.getEnabled();
+
+  do_check_array_eq(engines, [petrol, diesel, dummy]);
+
   _("Unregister an engine by name");
   manager.unregister('dummy');
   do_check_eq(manager.get('dummy'), undefined);
   engines = manager.getAll();
   do_check_eq(engines.length, 2);
   do_check_eq(engines.indexOf(dummy), -1);
 
   _("Unregister an engine by value");
--- a/services/sync/tests/unit/test_keys.js
+++ b/services/sync/tests/unit/test_keys.js
@@ -9,23 +9,16 @@ Cu.import("resource://services-sync/util
 
 let collectionKeys = new CollectionKeyManager();
 
 function sha256HMAC(message, key) {
   let h = Utils.makeHMACHasher(Ci.nsICryptoHMAC.SHA256, key);
   return Utils.digestBytes(message, h);
 }
 
-function do_check_array_eq(a1, a2) {
-  do_check_eq(a1.length, a2.length);
-  for (let i = 0; i < a1.length; ++i) {
-    do_check_eq(a1[i], a2[i]);
-  }
-}
-
 function do_check_keypair_eq(a, b) {
   do_check_eq(2, a.length);
   do_check_eq(2, b.length);
   do_check_eq(a[0], b[0]);
   do_check_eq(a[1], b[1]);
 }
 
 function test_time_keyFromString(iterations) {