diff --git a/services/sync/modules/policies.js b/services/sync/modules/policies.js index df8f27df77eb..de469c77a164 100644 --- a/services/sync/modules/policies.js +++ b/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 diff --git a/services/sync/tests/unit/test_syncscheduler.js b/services/sync/tests/unit/test_syncscheduler.js index e0011999e7af..b21c86686c6c 100644 --- a/services/sync/tests/unit/test_syncscheduler.js +++ b/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(); + } +});