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 231932 4339077e7826b456717a0a2a115812914b55a85f
parent 231931 649217a03879447535e6c8ebef3756e0f42236a1
child 231933 d1244379f050e03782e72416c9f2511f05a067c2
push id4187
push userbhearsum@mozilla.com
push dateFri, 28 Nov 2014 15:29:12 +0000
treeherdermozilla-beta@f23cc6a30c11 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs675397
milestone35.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 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) {