Bug 1453887 - Avoid syncing as the network link changes to down r?eoger
MozReview-Commit-ID: 5C2qDX4iITU
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -197,21 +197,37 @@ SyncScheduler.prototype = {
switch (topic) {
case "weave:engine:score:updated":
if (Status.login == LOGIN_SUCCEEDED) {
CommonUtils.namedTimer(this.calculateScore, SCORE_UPDATE_DELAY, this,
"_scoreTimer");
}
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
this._log.trace("Network offline status change: " + data);
this.checkSyncStatus();
break;
case "weave:service:sync:start":
// Clear out any potentially pending syncs now that we're syncing
--- a/services/sync/tests/unit/test_syncscheduler.js
+++ b/services/sync/tests/unit/test_syncscheduler.js
@@ -1039,8 +1039,30 @@ add_task(async function test_proper_inte
reconciled: 0
});
await Async.promiseYield();
scheduler.adjustSyncInterval();
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();
+ }
+});