Bug 1436637 - Revert back to a sync observer in Service. r?markh
MozReview-Commit-ID: 2X7mo3stuy7
--- a/services/sync/modules/service.js
+++ b/services/sync/modules/service.js
@@ -292,17 +292,16 @@ Sync11Service.prototype = {
this.scheduler = new SyncScheduler(this);
this.errorHandler = new ErrorHandler(this);
this._log = Log.repository.getLogger("Sync.Service");
this._log.manageLevelFromPref("services.sync.log.logger.service.main");
this._log.info("Loading Weave " + WEAVE_VERSION);
- this.asyncObserver = Async.asyncObserver(this, this._log);
this._clusterManager = this.identity.createClusterManager(this);
this.recordManager = new RecordManager(this);
this.enabled = true;
await this._registerEngines();
let ua = Cc["@mozilla.org/network/protocol;1?name=http"].
@@ -310,21 +309,20 @@ Sync11Service.prototype = {
this._log.info(ua);
if (!this._checkCrypto()) {
this.enabled = false;
this._log.info("Could not load the Weave crypto component. Disabling " +
"Weave, since it will not work correctly.");
}
- Svc.Obs.add("weave:service:setup-complete", this.asyncObserver);
- Svc.Obs.add("sync:collection_changed", this.asyncObserver); // Pulled from FxAccountsCommon
- Svc.Obs.add("fxaccounts:device_disconnected", this.asyncObserver);
- // We use a different synchronous observer to make testing easier.
- Services.prefs.addObserver(PREFS_BRANCH + "engine.", this.prefObserver.bind(this));
+ Svc.Obs.add("weave:service:setup-complete", this);
+ Svc.Obs.add("sync:collection_changed", this); // Pulled from FxAccountsCommon
+ Svc.Obs.add("fxaccounts:device_disconnected", this);
+ Services.prefs.addObserver(PREFS_BRANCH + "engine.", this);
if (!this.enabled) {
this._log.info("Firefox Sync disabled.");
}
this._updateCachedURLs();
let status = this._checkSetup();
@@ -408,53 +406,62 @@ Sync11Service.prototype = {
} catch (ex) {
this._log.warn("Could not register engine " + name, ex);
}
}
this.engineManager.setDeclined(declined);
},
- async observe(subject, topic, data) {
- try {
- switch (topic) {
- // Ideally this observer should be in the SyncScheduler, but it would require
- // some work to know about the sync specific engines. We should move this there once it does.
- case "sync:collection_changed":
- // We check if we're running TPS here to avoid TPS failing because it
- // couldn't get to get the sync lock, due to us currently syncing the
- // clients engine.
- if (data.includes("clients") && !Svc.Prefs.get("testing.tps", false)) {
- // [] = clients collection only
- await this.sync({why: "collection_changed", engines: []});
- }
- break;
- case "fxaccounts:device_disconnected":
- data = JSON.parse(data);
- if (!data.isLocalDevice) {
- await this.clientsEngine.updateKnownStaleClients();
- }
- break;
- case "weave:service:setup-complete":
- let status = this._checkSetup();
- if (status != STATUS_DISABLED && status != CLIENT_NOT_CONFIGURED) {
- this._startTracking();
- }
- break;
- }
- } catch (e) {
- this._log.error(e);
+ QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver,
+ Ci.nsISupportsWeakReference]),
+
+ observe(subject, topic, data) {
+ switch (topic) {
+ // Ideally this observer should be in the SyncScheduler, but it would require
+ // some work to know about the sync specific engines. We should move this there once it does.
+ case "sync:collection_changed":
+ // We check if we're running TPS here to avoid TPS failing because it
+ // couldn't get to get the sync lock, due to us currently syncing the
+ // clients engine.
+ if (data.includes("clients") && !Svc.Prefs.get("testing.tps", false)) {
+ // Sync in the background (it's fine not to wait on the returned promise
+ // because sync() has a lock).
+ // [] = clients collection only
+ this.sync({why: "collection_changed", engines: []}).catch(e => {
+ this._log.error(e);
+ });
+ }
+ break;
+ case "fxaccounts:device_disconnected":
+ data = JSON.parse(data);
+ if (!data.isLocalDevice) {
+ // Refresh the known stale clients list in the background.
+ this.clientsEngine.updateKnownStaleClients().catch(e => {
+ this._log.error(e);
+ });
+ }
+ break;
+ case "weave:service:setup-complete":
+ let status = this._checkSetup();
+ if (status != STATUS_DISABLED && status != CLIENT_NOT_CONFIGURED) {
+ this._startTracking();
+ }
+ break;
+ case "nsPref:changed":
+ if (this._ignorePrefObserver) {
+ return;
+ }
+ const engine = data.slice((PREFS_BRANCH + "engine.").length);
+ this._handleEngineStatusChanged(engine);
+ break;
}
},
- prefObserver(subject, topic, data) {
- if (this._ignorePrefObserver) {
- return;
- }
- const engine = data.slice((PREFS_BRANCH + "engine.").length);
+ _handleEngineStatusChanged(engine) {
this._log.trace("Status for " + engine + " engine changed.");
if (Svc.Prefs.get("engineStatusChanged." + engine, false)) {
// The enabled status being changed back to what it was before.
Svc.Prefs.reset("engineStatusChanged." + engine);
} else {
// Remember that the engine status changed locally until the next sync.
Svc.Prefs.set("engineStatusChanged." + engine, true);
}
@@ -1089,17 +1096,16 @@ Sync11Service.prototype = {
return reason;
},
async sync({engines, why} = {}) {
let dateStr = Utils.formatTimestamp(new Date());
this._log.debug("User-Agent: " + Utils.userAgent);
await this.promiseInitialized;
- await this.asyncObserver.promiseObserversComplete();
this._log.info(`Starting sync at ${dateStr} in browser session ${browserSessionID}`);
return this._catch(async function() {
// Make sure we're logged in.
if (this._shouldLogin()) {
this._log.debug("In sync: should login.");
if (!(await this.login())) {
this._log.debug("Not syncing: login returned false.");
return;
--- a/services/sync/tests/unit/test_clients_engine.js
+++ b/services/sync/tests/unit/test_clients_engine.js
@@ -1749,23 +1749,21 @@ add_task(async function test_other_clien
}
});
add_task(async function device_disconnected_notification_updates_known_stale_clients() {
const spyUpdate = sinon.spy(engine, "updateKnownStaleClients");
Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
JSON.stringify({ isLocalDevice: false }));
- await Service.asyncObserver.promiseObserversComplete();
ok(spyUpdate.calledOnce, "updateKnownStaleClients should be called");
spyUpdate.reset();
Services.obs.notifyObservers(null, "fxaccounts:device_disconnected",
JSON.stringify({ isLocalDevice: true }));
- await Service.asyncObserver.promiseObserversComplete();
ok(spyUpdate.notCalled, "updateKnownStaleClients should not be called");
spyUpdate.restore();
});
add_task(async function update_known_stale_clients() {
const makeFakeClient = (id) => ({ id, fxaDeviceId: `fxa-${id}` });
const clients = [makeFakeClient("one"), makeFakeClient("two"), makeFakeClient("three")];