Bug 691988 - OS X: Spurious 'back' events from idle observer cause too many instant syncs. r=rnewman

services/sync/modules/constants.js

@@ -123,6 +123,10 @@ SCORE_INCREMENT_XLARGE:                300 + 1, //MULTI_DEVICE_THRESHOLD + 1
 // Delay before incrementing global score
 SCORE_UPDATE_DELAY:                    100,
 
+// Delay for the back observer debouncer. This is chosen to be longer than any
+// observed spurious idle/back events and short enough to pre-empt user activity.
+IDLE_OBSERVER_BACK_DELAY:              100,
+
 // Number of records to upload in a single POST (multiple POSTS if exceeded)
 // FIXME: Record size limit is 256k (new cluster), so this can be quite large!
 // (Bug 569295)

services/sync/modules/policies.js

@@ -226,12 +226,21 @@ let SyncScheduler = {
         this.adjustSyncInterval();
         break;
       case "back":
-        this._log.trace("We're no longer idle.");
+        this._log.trace("Received notification that we're back from idle.");
         this.idle = false;
-        // Trigger a sync if we have multiple clients.
-        if (this.numClients > 1) {
-          Utils.nextTick(Weave.Service.sync, Weave.Service);
-        }
+        Utils.namedTimer(function onBack() {
+          if (this.idle) {
+            this._log.trace("... and we're idle again. " +
+                            "Ignoring spurious back notification.");
+            return;
+          }
+
+          this._log.trace("Genuine return from idle. Syncing.");
+          // Trigger a sync if we have multiple clients.
+          if (this.numClients > 1) {
+            Utils.nextTick(Weave.Service.sync, Weave.Service);
+          }
+        }, IDLE_OBSERVER_BACK_DELAY, this, "idleDebouncerTimer");
         break;
     }
   },

services/sync/tests/unit/test_syncscheduler.js

@@ -500,6 +500,54 @@ add_test(function test_idle_adjustSyncInterval() {
   run_next_test();
 });
 
+add_test(function test_back_triggersSync() {
+  // Confirm defaults.
+  do_check_eq(SyncScheduler.idle, false);
+
+  // Set up: Define 2 clients and put the system in idle.
+  SyncScheduler.numClients = 2;
+  SyncScheduler.observe(null, "idle", Svc.Prefs.get("scheduler.idleTime"));
+  do_check_eq(SyncScheduler.idle, true);
+
+  // We don't actually expect the sync (or the login, for that matter) to
+  // succeed. We just want to ensure that it was attempted.
+  Svc.Obs.add("weave:service:login:error", function onLoginError() {
+    Svc.Obs.remove("weave:service:login:error", onLoginError);
+    SyncScheduler.setDefaults();    
+    run_next_test();
+  });
+
+  // Send a 'back' event to trigger sync soonish.
+  SyncScheduler.observe(null, "back", Svc.Prefs.get("scheduler.idleTime"));
+});
+
+add_test(function test_back_debouncing() {
+  _("Ensure spurious back-then-idle events, as observed on OS X, don't trigger a sync.");
+
+  // Confirm defaults.
+  do_check_eq(SyncScheduler.idle, false);
+
+  // Set up: Define 2 clients and put the system in idle.
+  SyncScheduler.numClients = 2;
+  SyncScheduler.observe(null, "idle", Svc.Prefs.get("scheduler.idleTime"));
+  do_check_eq(SyncScheduler.idle, true);
+
+  function onLoginStart() {
+    do_throw("Shouldn't have kicked off a sync!");
+  }
+  Svc.Obs.add("weave:service:login:start", onLoginStart);
+
+  // Create spurious back-then-idle events as observed on OS X:
+  SyncScheduler.observe(null, "back", Svc.Prefs.get("scheduler.idleTime"));
+  SyncScheduler.observe(null, "idle", Svc.Prefs.get("scheduler.idleTime"));
+
+  timer = Utils.namedTimer(function () {
+    Svc.Obs.remove("weave:service:login:start", onLoginStart);
+    SyncScheduler.setDefaults();
+    run_next_test();
+  }, IDLE_OBSERVER_BACK_DELAY * 1.5, {}, "timer");
+});
+
 add_test(function test_no_sync_node() {
   // Test when Status.sync == NO_SYNC_NODE_FOUND
   // it is not overwritten on sync:finish