Bug 1221906 - Allow Sync to only sync specified engines. r=rnewman

browser/base/content/sync/aboutSyncTabs.js

@@ -318,15 +318,8 @@ var RemoteTabViewer = {
       }
     }
 
-    // if Clients hasn't synced yet this session, we need to sync it as well.
-    if (Weave.Service.clientsEngine.lastSync == 0) {
-      Weave.Service.clientsEngine.sync();
-    }
-
-    // Force a sync only for the tabs engine
-    let engine = Weave.Service.engineManager.get("tabs");
-    engine.lastModified = null;
-    engine.sync();
+    // Ask Sync to just do the tabs engine if it can.
+    Weave.Service.sync(["tabs"]);
     Services.prefs.setIntPref("services.sync.lastTabFetch",
                               Math.floor(Date.now() / 1000));
 

services/sync/modules/service.js

@@ -1268,7 +1268,7 @@ Sync11Service.prototype = {
     return reason;
   },
 
-  sync: function sync() {
+  sync: function sync(engineNamesToSync) {
     if (!this.enabled) {
       this._log.debug("Not syncing as Sync is disabled.");
       return;
@@ -1288,14 +1288,14 @@ Sync11Service.prototype = {
       else {
         this._log.trace("In sync: no need to login.");
       }
-      return this._lockedSync.apply(this, arguments);
+      return this._lockedSync(engineNamesToSync);
     })();
   },
 
   /**
    * Sync up engines with the server.
    */
-  _lockedSync: function _lockedSync() {
+  _lockedSync: function _lockedSync(engineNamesToSync) {
     return this._lock("service.js: sync",
                       this._notify("sync", "", function onNotify() {
 
@@ -1306,7 +1306,7 @@ Sync11Service.prototype = {
       let cb = Async.makeSpinningCallback();
       synchronizer.onComplete = cb;
 
-      synchronizer.sync();
+      synchronizer.sync(engineNamesToSync);
       // wait() throws if the first argument is truthy, which is exactly what
       // we want.
       let result = cb.wait();

services/sync/modules/stages/enginesync.js

@@ -31,7 +31,7 @@ this.EngineSynchronizer = function EngineSynchronizer(service) {
 }
 
 EngineSynchronizer.prototype = {
-  sync: function sync() {
+  sync: function sync(engineNamesToSync) {
     if (!this.onComplete) {
       throw new Error("onComplete handler not installed.");
     }
@@ -96,6 +96,9 @@ EngineSynchronizer.prototype = {
       return;
     }
 
+    // We only honor the "hint" of what engines to Sync if this isn't
+    // a first sync.
+    let allowEnginesHint = false;
     // Wipe data in the desired direction if necessary
     switch (Svc.Prefs.get("firstSync")) {
       case "resetClient":
@@ -107,6 +110,9 @@ EngineSynchronizer.prototype = {
       case "wipeRemote":
         this.service.wipeRemote(engineManager.enabledEngineNames);
         break;
+      default:
+        allowEnginesHint = true;
+        break;
     }
 
     if (this.service.clientsEngine.localCommands) {
@@ -143,8 +149,17 @@ EngineSynchronizer.prototype = {
       return;
     }
 
+    // If the engines to sync has been specified, we sync in the order specified.
+    let enginesToSync;
+    if (allowEnginesHint && engineNamesToSync) {
+      this._log.info("Syncing specified engines", engineNamesToSync);
+      enginesToSync = engineManager.get(engineNamesToSync).filter(e => e.enabled);
+    } else {
+      this._log.info("Syncing all enabled engines.");
+      enginesToSync = engineManager.getEnabled();
+    }
     try {
-      for (let engine of engineManager.getEnabled()) {
+      for (let engine of enginesToSync) {
         // If there's any problems with syncing the engine, report the failure
         if (!(this._syncEngine(engine)) || this.service.status.enforceBackoff) {
           this._log.info("Aborting sync for failure in " + engine.name);

services/sync/tests/unit/test_service_sync_specified.js

@@ -0,0 +1,159 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+Cu.import("resource://services-sync/constants.js");
+Cu.import("resource://services-sync/engines.js");
+Cu.import("resource://services-sync/engines/clients.js");
+Cu.import("resource://services-sync/record.js");
+Cu.import("resource://services-sync/service.js");
+Cu.import("resource://services-sync/util.js");
+Cu.import("resource://testing-common/services/sync/utils.js");
+
+initTestLogging();
+Service.engineManager.clear();
+
+let syncedEngines = []
+
+function SteamEngine() {
+  SyncEngine.call(this, "Steam", Service);
+}
+SteamEngine.prototype = {
+  __proto__: SyncEngine.prototype,
+  _sync: function _sync() {
+    syncedEngines.push(this.name);
+  }
+};
+Service.engineManager.register(SteamEngine);
+
+function StirlingEngine() {
+  SyncEngine.call(this, "Stirling", Service);
+}
+StirlingEngine.prototype = {
+  __proto__: SteamEngine.prototype,
+  _sync: function _sync() {
+    syncedEngines.push(this.name);
+  }
+};
+Service.engineManager.register(StirlingEngine);
+
+// Tracking info/collections.
+var collectionsHelper = track_collections_helper();
+var upd = collectionsHelper.with_updated_collection;
+
+function sync_httpd_setup(handlers) {
+
+  handlers["/1.1/johndoe/info/collections"] = collectionsHelper.handler;
+  delete collectionsHelper.collections.crypto;
+  delete collectionsHelper.collections.meta;
+
+  let cr = new ServerWBO("keys");
+  handlers["/1.1/johndoe/storage/crypto/keys"] =
+    upd("crypto", cr.handler());
+
+  let cl = new ServerCollection();
+  handlers["/1.1/johndoe/storage/clients"] =
+    upd("clients", cl.handler());
+
+  return httpd_setup(handlers);
+}
+
+function setUp() {
+  syncedEngines = [];
+  let engine = Service.engineManager.get("steam");
+  engine.enabled = true;
+  engine.syncPriority = 1;
+
+  engine = Service.engineManager.get("stirling");
+  engine.enabled = true;
+  engine.syncPriority = 2;
+
+  let server = sync_httpd_setup({
+    "/1.1/johndoe/storage/meta/global": new ServerWBO("global", {}).handler(),
+  });
+  new SyncTestingInfrastructure(server, "johndoe", "ilovejane",
+                                "abcdeabcdeabcdeabcdeabcdea");
+  return server;
+}
+
+function run_test() {
+  initTestLogging("Trace");
+  Log.repository.getLogger("Sync.Service").level = Log.Level.Trace;
+  Log.repository.getLogger("Sync.ErrorHandler").level = Log.Level.Trace;
+
+  run_next_test();
+}
+
+add_test(function test_noEngines() {
+  _("Test: An empty array of engines to sync does nothing.");
+  let server = setUp();
+
+  try {
+    _("Sync with no engines specified.");
+    Service.sync([]);
+    deepEqual(syncedEngines, [], "no engines were synced");
+
+  } finally {
+    Service.startOver();
+    server.stop(run_next_test);
+  }
+});
+
+add_test(function test_oneEngine() {
+  _("Test: Only one engine is synced.");
+  let server = setUp();
+
+  try {
+
+    _("Sync with 1 engine specified.");
+    Service.sync(["steam"]);
+    deepEqual(syncedEngines, ["steam"])
+
+  } finally {
+    Service.startOver();
+    server.stop(run_next_test);
+  }
+});
+
+add_test(function test_bothEnginesSpecified() {
+  _("Test: All engines are synced when specified in the correct order (1).");
+  let server = setUp();
+
+  try {
+    _("Sync with both engines specified.");
+    Service.sync(["steam", "stirling"]);
+    deepEqual(syncedEngines, ["steam", "stirling"])
+
+  } finally {
+    Service.startOver();
+    server.stop(run_next_test);
+  }
+});
+
+add_test(function test_bothEnginesSpecified() {
+  _("Test: All engines are synced when specified in the correct order (2).");
+  let server = setUp();
+
+  try {
+    _("Sync with both engines specified.");
+    Service.sync(["stirling", "steam"]);
+    deepEqual(syncedEngines, ["stirling", "steam"])
+
+  } finally {
+    Service.startOver();
+    server.stop(run_next_test);
+  }
+});
+
+add_test(function test_bothEnginesDefault() {
+  _("Test: All engines are synced when nothing is specified.");
+  let server = setUp();
+
+  try {
+    Service.sync();
+    deepEqual(syncedEngines, ["steam", "stirling"])
+
+  } finally {
+    Service.startOver();
+    server.stop(run_next_test);
+  }
+});

services/sync/tests/unit/xpcshell.ini

@@ -93,6 +93,7 @@ skip-if = os == "mac" || os == "linux"
 [test_service_sync_remoteSetup.js]
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = os == "android"
+[test_service_sync_specified.js]
 [test_service_sync_updateEnabledEngines.js]
 # Bug 676978: test hangs on Android (see also testing/xpcshell/xpcshell.ini)
 skip-if = os == "android"