Bug 1453887 - Avoid syncing as the network link changes to down r=eoger

MozReview-Commit-ID: 5C2qDX4iITU

--HG--
extra : rebase_source : 807cbdc8b9f66754dbe726ddb9ce81849210a585

services/sync/modules/policies.js

@@ -202,11 +202,27 @@ SyncScheduler.prototype = {
         }
         break;
       case "network:link-status-changed":
-        if (!this.offline) {
+        // Note: NetworkLinkService is unreliable, we get false negatives for it
+        // in cases such as VMs (bug 1420802), so we don't want to use it in
+        // `get offline`, but we assume that it's probably reliable if we're
+        // getting status changed events. (We might be wrong about this, but
+        // if that's true, then the only downside is that we won't sync as
+        // promptly).
+        let isOffline = this.offline;
+        this._log.debug(`Network link status changed to "${data}". Offline?`,
+                        isOffline);
+        // Data may be one of `up`, `down`, `change`, or `unknown`. We only want
+        // to sync if it's "up".
+        if (data == "up" && !isOffline) {
           this._log.debug("Network link looks up. Syncing.");
           this.scheduleNextSync(0, {why: topic});
+        } else if (data == "down") {
+          // Unschedule pending syncs if we know we're going down. We don't do
+          // this via `checkSyncStatus`, since link status isn't reflected in
+          // `this.offline`.
+          this.clearSyncTriggers();
         }
-        // Intended fallthrough
+        break;
       case "network:offline-status-changed":
       case "captive-portal-detected":
         // Whether online or offline, we'll reschedule syncs

services/sync/tests/unit/test_syncscheduler.js

@@ -1044,3 +1044,25 @@ add_task(async function test_proper_interval_on_only_failing() {
   Assert.ok(!scheduler.hasIncomingItems);
   Assert.equal(scheduler.syncInterval, scheduler.singleDeviceInterval);
 });
+
+add_task(async function test_link_status_change() {
+  _("Check that we only attempt to sync when link status is up");
+  try {
+    sinon.spy(scheduler, "scheduleNextSync");
+
+    Svc.Obs.notify("network:link-status-changed", null, "down");
+    equal(scheduler.scheduleNextSync.callCount, 0);
+
+    Svc.Obs.notify("network:link-status-changed", null, "change");
+    equal(scheduler.scheduleNextSync.callCount, 0);
+
+    Svc.Obs.notify("network:link-status-changed", null, "up");
+    equal(scheduler.scheduleNextSync.callCount, 1);
+
+    Svc.Obs.notify("network:link-status-changed", null, "change");
+    equal(scheduler.scheduleNextSync.callCount, 1);
+
+  } finally {
+    scheduler.scheduleNextSync.restore();
+  }
+});