Bug 1416807 - Don't schedule syncs while we're behind to a captive portal. r?markh
MozReview-Commit-ID: 2CrO3CfKgC3
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -20,16 +20,22 @@ Cu.import("resource://services-common/ut
XPCOMUtils.defineLazyModuleGetter(this, "Status",
"resource://services-sync/status.js");
XPCOMUtils.defineLazyModuleGetter(this, "AddonManager",
"resource://gre/modules/AddonManager.jsm");
XPCOMUtils.defineLazyServiceGetter(this, "IdleService",
"@mozilla.org/widget/idleservice;1",
"nsIIdleService");
+XPCOMUtils.defineLazyServiceGetter(this, "NetworkLinkService",
+ "@mozilla.org/network/network-link-service;1",
+ "nsINetworkLinkService");
+XPCOMUtils.defineLazyServiceGetter(this, "CaptivePortalService",
+ "@mozilla.org/network/captive-portal-service;1",
+ "nsICaptivePortalService");
// Get the value for an interval that's stored in preferences. To save users
// from themselves (and us from them!) the minimum time they can specify
// is 60s.
function getThrottledIntervalPreference(prefName) {
return Math.max(Svc.Prefs.get(prefName), 60) * 1000;
}
@@ -108,21 +114,53 @@ SyncScheduler.prototype = {
return Svc.Prefs.get("clients.devices.desktop", 0) +
Svc.Prefs.get("clients.devices.mobile", 0);
},
set numClients(value) {
throw new Error("Don't set numClients - the clients engine manages it.");
},
+ get offline() {
+ // Services.io.offline has slowly become fairly useless over the years - it
+ // no longer attempts to track the actual network state by default, but one
+ // thing stays true: if it says we're offline then we are definitely not online.
+ //
+ // There is also a network link service that tracks if there is any network
+ // adaptor connected. Sadly this too is unreliable - eg, an adaptor on a
+ // local private network (eg, a VMWare local network) may be technically
+ // connected but still unable to hit the public network - but it should
+ // only be unreliable in terms of indicating we online when we aren't but
+ // not indicate we are offline when we are online.
+ //
+ // Finally, if both of these services don't tell us we're offline for sure,
+ // we'll ask the captive portal service if we are behind a locked captive
+ // portal.
+ //
+ // With these 3 services combined, we believe we can avoid all false positives
+ // and make a good guess on whether an user is online or not in most cases.
+ try {
+ if (Services.io.offline ||
+ !NetworkLinkService.isLinkUp ||
+ CaptivePortalService.state == CaptivePortalService.LOCKED_PORTAL) {
+ return true;
+ }
+ } catch (ex) {
+ this._log.warn("Could not determine network status.", ex);
+ }
+ return false;
+ },
+
init: function init() {
this._log.level = Log.Level[Svc.Prefs.get("log.logger.service.main")];
this.setDefaults();
Svc.Obs.add("weave:engine:score:updated", this);
Svc.Obs.add("network:offline-status-changed", this);
+ Svc.Obs.add("network:link-status-changed", this);
+ Svc.Obs.add("captive-portal-detected", this);
Svc.Obs.add("weave:service:sync:start", this);
Svc.Obs.add("weave:service:sync:finish", this);
Svc.Obs.add("weave:engine:sync:finish", this);
Svc.Obs.add("weave:engine:sync:error", this);
Svc.Obs.add("weave:service:login:error", this);
Svc.Obs.add("weave:service:logout:finish", this);
Svc.Obs.add("weave:service:sync:error", this);
Svc.Obs.add("weave:service:backoff:interval", this);
@@ -145,16 +183,18 @@ 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:offline-status-changed":
+ case "network:link-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
this.clearSyncTriggers();
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -829,17 +829,17 @@ Sync11Service.prototype = {
// reflected in the UI.
Svc.Obs.notify("weave:service:start-over:finish");
}
},
async login() {
async function onNotify() {
this._loggedIn = false;
- if (Services.io.offline) {
+ if (this.scheduler.offline) {
this.status.login = LOGIN_FAILED_NETWORK_ERROR;
throw new Error("Application is offline, login should not be called");
}
this._log.info("Logging in the user.");
// Just let any errors bubble up - they've more context than we do!
try {
await this.identity.ensureLoggedIn();
@@ -1039,17 +1039,17 @@ Sync11Service.prototype = {
* Return whether we should attempt login at the start of a sync.
*
* Note that this function has strong ties to _checkSync: callers
* of this function should typically use _checkSync to verify that
* any necessary login took place.
*/
_shouldLogin: function _shouldLogin() {
return this.enabled &&
- !Services.io.offline &&
+ !this.scheduler.offline &&
!this.isLoggedIn &&
Async.isAppReady();
},
/**
* Determine if a sync should run.
*
* @param ignore [optional]
@@ -1059,17 +1059,17 @@ Sync11Service.prototype = {
*/
_checkSync: function _checkSync(ignore) {
let reason = "";
// Ideally we'd call _checkSetup() here but that has too many side-effects.
if (Status.service == CLIENT_NOT_CONFIGURED)
reason = kSyncNotConfigured;
else if (Status.service == STATUS_DISABLED || !this.enabled)
reason = kSyncWeaveDisabled;
- else if (Services.io.offline)
+ else if (this.scheduler.offline)
reason = kSyncNetworkOffline;
else if (this.status.minimumNextSync > Date.now())
reason = kSyncBackoffNotMet;
else if ((this.status.login == MASTER_PASSWORD_LOCKED) &&
Utils.mpLocked())
reason = kSyncMasterPasswordLocked;
else if (Svc.Prefs.get("firstSync") == "notReady")
reason = kFirstSyncChoiceNotMade;