diff --git a/browser/extensions/formautofill/FormAutofillSync.jsm b/browser/extensions/formautofill/FormAutofillSync.jsm index 58cb7ee169ae..8fc07f1f0367 100644 --- a/browser/extensions/formautofill/FormAutofillSync.jsm +++ b/browser/extensions/formautofill/FormAutofillSync.jsm @@ -184,8 +184,7 @@ function FormAutofillTracker(name, engine) { FormAutofillTracker.prototype = { __proto__: Tracker.prototype, - observe: function observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); + async observe(subject, topic, data) { if (topic != "formautofill-storage-changed") { return; } @@ -214,11 +213,11 @@ FormAutofillTracker.prototype = { // setting a read-only property. set ignoreAll(value) {}, - startTracking() { + onStart() { Services.obs.addObserver(this, "formautofill-storage-changed"); }, - stopTracking() { + onStop() { Services.obs.removeObserver(this, "formautofill-storage-changed"); }, diff --git a/browser/extensions/formautofill/test/unit/test_sync.js b/browser/extensions/formautofill/test/unit/test_sync.js index 1d45570d38f2..eb418d5af931 100644 --- a/browser/extensions/formautofill/test/unit/test_sync.js +++ b/browser/extensions/formautofill/test/unit/test_sync.js @@ -142,6 +142,7 @@ add_task(async function test_outgoing() { }, ]); + await engine._tracker.asyncObserver.promiseObserversComplete(); // The tracker should have a score recorded for the 2 additions we had. equal(engine._tracker.score, SCORE_INCREMENT_XLARGE * 2); diff --git a/services/sync/modules/addonsreconciler.js b/services/sync/modules/addonsreconciler.js index 8498b2894b18..4ca1bb83a30d 100644 --- a/services/sync/modules/addonsreconciler.js +++ b/services/sync/modules/addonsreconciler.js @@ -57,7 +57,7 @@ this.EXPORTED_SYMBOLS = ["AddonsReconciler", "CHANGE_INSTALLED", * * The usage pattern for instances of this class is: * - * let reconciler = new AddonsReconciler(); + * let reconciler = new AddonsReconciler(...); * await reconciler.ensureStateLoaded(); * * // At this point, your instance should be ready to use. @@ -113,9 +113,10 @@ this.EXPORTED_SYMBOLS = ["AddonsReconciler", "CHANGE_INSTALLED", * events will occur immediately. However, we still see disabling events and * heed them like they were normal. In the end, the state is proper. */ -this.AddonsReconciler = function AddonsReconciler() { +this.AddonsReconciler = function AddonsReconciler(queueCaller) { this._log = Log.repository.getLogger("Sync.AddonsReconciler"); this._log.manageLevelFromPref("services.sync.log.logger.addonsreconciler"); + this.queueCaller = queueCaller; Svc.Obs.add("xpcom-shutdown", this.stopListening, this); }; @@ -585,35 +586,35 @@ AddonsReconciler.prototype = { // AddonListeners onEnabling: function onEnabling(addon, requiresRestart) { - Async.promiseSpinningly(this._handleListener("onEnabling", addon, requiresRestart)); + this.queueCaller.enqueueCall(() => this._handleListener("onEnabling", addon, requiresRestart)); }, onEnabled: function onEnabled(addon) { - Async.promiseSpinningly(this._handleListener("onEnabled", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onEnabled", addon)); }, onDisabling: function onDisabling(addon, requiresRestart) { - Async.promiseSpinningly(this._handleListener("onDisabling", addon, requiresRestart)); + this.queueCaller.enqueueCall(() => this._handleListener("onDisabling", addon, requiresRestart)); }, onDisabled: function onDisabled(addon) { - Async.promiseSpinningly(this._handleListener("onDisabled", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onDisabled", addon)); }, onInstalling: function onInstalling(addon, requiresRestart) { - Async.promiseSpinningly(this._handleListener("onInstalling", addon, requiresRestart)); + this.queueCaller.enqueueCall(() => this._handleListener("onInstalling", addon, requiresRestart)); }, onInstalled: function onInstalled(addon) { - Async.promiseSpinningly(this._handleListener("onInstalled", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onInstalled", addon)); }, onUninstalling: function onUninstalling(addon, requiresRestart) { - Async.promiseSpinningly(this._handleListener("onUninstalling", addon, requiresRestart)); + this.queueCaller.enqueueCall(() => this._handleListener("onUninstalling", addon, requiresRestart)); }, onUninstalled: function onUninstalled(addon) { - Async.promiseSpinningly(this._handleListener("onUninstalled", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onUninstalled", addon)); }, onOperationCancelled: function onOperationCancelled(addon) { - Async.promiseSpinningly(this._handleListener("onOperationCancelled", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onOperationCancelled", addon)); }, // InstallListeners onInstallEnded: function onInstallEnded(install, addon) { - Async.promiseSpinningly(this._handleListener("onInstallEnded", addon)); + this.queueCaller.enqueueCall(() => this._handleListener("onInstallEnded", addon)); } }; diff --git a/services/sync/modules/bookmark_repair.js b/services/sync/modules/bookmark_repair.js index 4e948edc84a1..171eca2554d3 100644 --- a/services/sync/modules/bookmark_repair.js +++ b/services/sync/modules/bookmark_repair.js @@ -703,7 +703,7 @@ class BookmarkRepairResponder extends CollectionRepairResponder { return; } log.debug(`bookmarks engine has uploaded stuff - creating a repair response`, subject); - Async.promiseSpinningly(this._finishRepair()); + this.service.clientsEngine._tracker.asyncObserver.enqueueCall(() => this._finishRepair()); } async _finishRepair() { diff --git a/services/sync/modules/engines.js b/services/sync/modules/engines.js index 281484465ae8..e04142a0b819 100644 --- a/services/sync/modules/engines.js +++ b/services/sync/modules/engines.js @@ -66,9 +66,7 @@ this.Tracker = function Tracker(name, engine) { }); this.ignoreAll = false; - Svc.Obs.add("weave:engine:start-tracking", this); - Svc.Obs.add("weave:engine:stop-tracking", this); - + this.asyncObserver = Async.asyncObserver(this, this._log); }; Tracker.prototype = { @@ -96,11 +94,6 @@ Tracker.prototype = { return ensureDirectory(this._storage.path); }, - get changedIDs() { - Async.promiseSpinningly(this._storage.load()); - return this._storage.data; - }, - set score(value) { this._score = value; Observers.notify("weave:engine:score:updated", this.name); @@ -113,6 +106,11 @@ Tracker.prototype = { persistChangedIDs: true, + async getChangedIDs() { + await this._storage.load(); + return this._storage.data; + }, + _saveChangedIDs() { if (!this.persistChangedIDs) { this._log.debug("Not saving changedIDs."); @@ -136,13 +134,14 @@ Tracker.prototype = { this._ignored.splice(index, 1); }, - _saveChangedID(id, when) { + async _saveChangedID(id, when) { this._log.trace(`Adding changed ID: ${id}, ${JSON.stringify(when)}`); - this.changedIDs[id] = when; + const changedIDs = await this.getChangedIDs(); + changedIDs[id] = when; this._saveChangedIDs(); }, - addChangedID(id, when) { + async addChangedID(id, when) { if (!id) { this._log.warn("Attempted to add undefined ID to tracker"); return false; @@ -157,15 +156,16 @@ Tracker.prototype = { when = this._now(); } + const changedIDs = await this.getChangedIDs(); // Add/update the entry if we have a newer time. - if ((this.changedIDs[id] || -Infinity) < when) { - this._saveChangedID(id, when); + if ((changedIDs[id] || -Infinity) < when) { + await this._saveChangedID(id, when); } return true; }, - removeChangedID(...ids) { + async removeChangedID(...ids) { if (!ids.length || this.ignoreAll) { return false; } @@ -178,19 +178,20 @@ Tracker.prototype = { this._log.debug(`Not removing ignored ID ${id} from tracker`); continue; } - if (this.changedIDs[id] != null) { + const changedIDs = await this.getChangedIDs(); + if (changedIDs[id] != null) { this._log.trace("Removing changed ID " + id); - delete this.changedIDs[id]; + delete changedIDs[id]; } } - this._saveChangedIDs(); + await this._saveChangedIDs(); return true; }, - clearChangedIDs() { + async clearChangedIDs() { this._log.trace("Clearing changed ID list"); this._storage.data = {}; - this._saveChangedIDs(); + await this._saveChangedIDs(); }, _now() { @@ -199,13 +200,31 @@ Tracker.prototype = { _isTracking: false, - // Override these in your subclasses. - startTracking() { + start() { + if (!this.engineIsEnabled()) { + return; + } + this._log.trace("start()."); + if (!this._isTracking) { + this.onStart(); + this._isTracking = true; + } }, - stopTracking() { + async stop() { + this._log.trace("stop()."); + if (this._isTracking) { + await this.asyncObserver.promiseObserversComplete(); + this.onStop(); + this._isTracking = false; + } }, + // Override these in your subclasses. + onStart() {}, + onStop() {}, + async observe(subject, topic, data) {}, + engineIsEnabled() { if (!this.engine) { // Can't tell -- we must be running in a test! @@ -214,48 +233,20 @@ Tracker.prototype = { return this.engine.enabled; }, - onEngineEnabledChanged(engineEnabled) { + async onEngineEnabledChanged(engineEnabled) { if (engineEnabled == this._isTracking) { return; } if (engineEnabled) { - this.startTracking(); - this._isTracking = true; + this.start(); } else { - this.stopTracking(); - this._isTracking = false; - this.clearChangedIDs(); - } - }, - - observe(subject, topic, data) { - switch (topic) { - case "weave:engine:start-tracking": - if (!this.engineIsEnabled()) { - return; - } - this._log.trace("Got start-tracking."); - if (!this._isTracking) { - this.startTracking(); - this._isTracking = true; - } - return; - case "weave:engine:stop-tracking": - this._log.trace("Got stop-tracking."); - if (this._isTracking) { - this.stopTracking(); - this._isTracking = false; - } + await this.stop(); + await this.clearChangedIDs(); } }, async finalize() { - // Stop listening for tracking and engine enabled change notifications. - // Important for tests where we unregister the engine during cleanup. - Svc.Obs.remove("weave:engine:start-tracking", this); - Svc.Obs.remove("weave:engine:stop-tracking", this); - // Persist all pending tracked changes to disk, and wait for the final write // to finish. this._saveChangedIDs(); @@ -660,6 +651,7 @@ this.Engine = function Engine(name, service) { XPCOMUtils.defineLazyPreferenceGetter(this, "_enabled", `services.sync.engine.${this.prefName}`, false, (data, previous, latest) => + // We do not await on the promise onEngineEnabledChanged returns. this._tracker.onEngineEnabledChanged(latest)); }; Engine.prototype = { @@ -712,6 +704,15 @@ Engine.prototype = { return tracker; }, + startTracking() { + this._tracker.start(); + }, + + // Returns a promise + stopTracking() { + return this._tracker.stop(); + }, + async sync() { if (!this.enabled) { return false; @@ -741,7 +742,7 @@ Engine.prototype = { this._tracker.ignoreAll = true; await this._store.wipe(); this._tracker.ignoreAll = false; - this._tracker.clearChangedIDs(); + await this._tracker.clearChangedIDs(); }, async wipeClient() { @@ -957,7 +958,7 @@ SyncEngine.prototype = { * method to bypass the tracker for certain or all changed items. */ async getChangedIDs() { - return this._tracker.changedIDs; + return this._tracker.getChangedIDs(); }, // Create a new record using the store and add in metadata. @@ -983,7 +984,6 @@ SyncEngine.prototype = { // Any setup that needs to happen at the beginning of each sync. async _syncStartup() { - // Determine if we need to wipe on outdated versions let metaGlobal = await this.service.recordManager.get(this.metaURL); let engines = metaGlobal.payload.engines || {}; @@ -1039,7 +1039,7 @@ SyncEngine.prototype = { this._modified.replace(initialChanges); // Clear the tracker now. If the sync fails we'll add the ones we failed // to upload back. - this._tracker.clearChangedIDs(); + await this._tracker.clearChangedIDs(); this._tracker.resetScore(); this._log.info(this._modified.count() + @@ -1337,7 +1337,7 @@ SyncEngine.prototype = { if (this._shouldDeleteRemotely(item)) { this._log.trace("Deleting item from server without applying", item); - this._deleteId(item.id); + await this._deleteId(item.id); return { shouldApply: false, error: null }; } @@ -1412,8 +1412,8 @@ SyncEngine.prototype = { return true; }, - _deleteId(id) { - this._tracker.removeChangedID(id); + async _deleteId(id) { + await this._tracker.removeChangedID(id); this._noteDeletedId(id); }, @@ -1427,7 +1427,7 @@ SyncEngine.prototype = { async _switchItemToDupe(localDupeGUID, incomingItem) { // The local, duplicate ID is always deleted on the server. - this._deleteId(localDupeGUID); + await this._deleteId(localDupeGUID); // We unconditionally change the item's ID in case the engine knows of // an item but doesn't expose it through itemExists. If the API @@ -1765,6 +1765,7 @@ SyncEngine.prototype = { } } } + await this._tracker.asyncObserver.promiseObserversComplete(); }, async _syncCleanup() { @@ -1908,6 +1909,7 @@ SyncEngine.prototype = { * @return A `Changeset` object. */ async pullNewChanges() { + await this._tracker.asyncObserver.promiseObserversComplete(); return this.getChangedIDs(); }, @@ -1918,7 +1920,7 @@ SyncEngine.prototype = { */ async trackRemainingChanges() { for (let [id, change] of this._modified.entries()) { - this._tracker.addChangedID(id, change); + await this._tracker.addChangedID(id, change); } }, diff --git a/services/sync/modules/engines/addons.js b/services/sync/modules/engines/addons.js index 94a6519ce4a8..d7ad61566ed1 100644 --- a/services/sync/modules/engines/addons.js +++ b/services/sync/modules/engines/addons.js @@ -114,7 +114,7 @@ Utils.deferGetSet(AddonRecord, "cleartext", ["addonID", this.AddonsEngine = function AddonsEngine(service) { SyncEngine.call(this, "Addons", service); - this._reconciler = new AddonsReconciler(); + this._reconciler = new AddonsReconciler(this._tracker.asyncObserver); }; AddonsEngine.prototype = { __proto__: SyncEngine.prototype, @@ -159,7 +159,8 @@ AddonsEngine.prototype = { */ async getChangedIDs() { let changes = {}; - for (let [id, modified] of Object.entries(this._tracker.changedIDs)) { + const changedIDs = await this._tracker.getChangedIDs(); + for (let [id, modified] of Object.entries(changedIDs)) { changes[id] = modified; } @@ -708,20 +709,18 @@ AddonsTracker.prototype = { return; } - if (this.addChangedID(addon.guid, date.getTime() / 1000)) { + const added = await this.addChangedID(addon.guid, date.getTime() / 1000); + if (added) { this.score += SCORE_INCREMENT_XLARGE; } }, - startTracking() { - if (this.engine.enabled) { - this.reconciler.startListening(); - } - + onStart() { + this.reconciler.startListening(); this.reconciler.addChangeListener(this); }, - stopTracking() { + onStop() { this.reconciler.removeChangeListener(this); this.reconciler.stopListening(); }, diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index d0205afe5e56..3ccf8b220411 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -660,10 +660,6 @@ BookmarksEngine.prototype = { return mapped ? mapped.toString() : mapped; }, - async pullNewChanges() { - return this._tracker.promiseChangedIDs(); - }, - // Called when _findDupe returns a dupe item and the engine has decided to // switch the existing item to the new incoming item. async _switchItemToDupe(localDupeGUID, incomingItem) { @@ -1143,9 +1139,9 @@ BufferedBookmarksStore.prototype = { // all concepts of "add a changed ID." However, it still registers an observer // to bump the score, so that changed bookmarks are synced immediately. function BookmarksTracker(name, engine) { + Tracker.call(this, name, engine); this._batchDepth = 0; this._batchSawScoreIncrement = false; - Tracker.call(this, name, engine); } BookmarksTracker.prototype = { __proto__: Tracker.prototype, @@ -1164,18 +1160,18 @@ BookmarksTracker.prototype = { // in Places. persistChangedIDs: false, - startTracking() { + onStart() { PlacesUtils.bookmarks.addObserver(this, true); - Svc.Obs.add("bookmarks-restore-begin", this); - Svc.Obs.add("bookmarks-restore-success", this); - Svc.Obs.add("bookmarks-restore-failed", this); + Svc.Obs.add("bookmarks-restore-begin", this.asyncObserver); + Svc.Obs.add("bookmarks-restore-success", this.asyncObserver); + Svc.Obs.add("bookmarks-restore-failed", this.asyncObserver); }, - stopTracking() { + onStop() { PlacesUtils.bookmarks.removeObserver(this); - Svc.Obs.remove("bookmarks-restore-begin", this); - Svc.Obs.remove("bookmarks-restore-success", this); - Svc.Obs.remove("bookmarks-restore-failed", this); + Svc.Obs.remove("bookmarks-restore-begin", this.asyncObserver); + Svc.Obs.remove("bookmarks-restore-success", this.asyncObserver); + Svc.Obs.remove("bookmarks-restore-failed", this.asyncObserver); }, // Ensure we aren't accidentally using the base persistence. @@ -1191,24 +1187,16 @@ BookmarksTracker.prototype = { // instead of throwing. clearChangedIDs() {}, - promiseChangedIDs() { + async getChangedIDs() { return PlacesSyncUtils.bookmarks.pullChanges(); }, - get changedIDs() { - throw new Error("Use promiseChangedIDs"); - }, - set changedIDs(obj) { throw new Error("Don't set initial changed bookmark IDs"); }, - observe: function observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); - + async observe(subject, topic, data) { switch (topic) { - case "weave:engine:start-tracking": - break; case "bookmarks-restore-begin": this._log.debug("Ignoring changes from importing bookmarks."); break; @@ -1217,12 +1205,10 @@ BookmarksTracker.prototype = { if (data == "json") { this._log.debug("Restore succeeded: wiping server and other clients."); - Async.promiseSpinningly((async () => { - await this.engine.service.resetClient([this.name]); - await this.engine.service.wipeServer([this.name]); - await this.engine.service.clientsEngine.sendCommand("wipeEngine", [this.name], - null, { reason: "bookmark-restore" }); - })()); + await this.engine.service.resetClient([this.name]); + await this.engine.service.wipeServer([this.name]); + await this.engine.service.clientsEngine.sendCommand("wipeEngine", [this.name], + null, { reason: "bookmark-restore" }); } else { // "html", "html-initial", or "json-append" this._log.debug("Import succeeded."); diff --git a/services/sync/modules/engines/clients.js b/services/sync/modules/engines/clients.js index 424bb65480f1..a8c942ea96e2 100644 --- a/services/sync/modules/engines/clients.js +++ b/services/sync/modules/engines/clients.js @@ -365,7 +365,7 @@ ClientEngine.prototype = { this.isFirstSync = !this.lastRecordUpload; // Reupload new client record periodically. if (Date.now() / 1000 - this.lastRecordUpload > CLIENTS_TTL_REFRESH) { - this._tracker.addChangedID(this.localID); + await this._tracker.addChangedID(this.localID); this.lastRecordUpload = Date.now() / 1000; } return SyncEngine.prototype._syncStartup.call(this); @@ -633,7 +633,7 @@ ClientEngine.prototype = { // It's a bad client record. Save it to be deleted at the end of the sync. this._log.debug("Bad client record detected. Scheduling for deletion."); - this._deleteId(item.id); + await this._deleteId(item.id); // Neither try again nor error; we're going to delete it. return SyncEngine.kRecoveryStrategy.ignore; @@ -684,7 +684,7 @@ ClientEngine.prototype = { if ((await this._addClientCommand(clientId, action))) { this._log.trace(`Client ${clientId} got a new action`, [command, args]); - this._tracker.addChangedID(clientId); + await this._tracker.addChangedID(clientId); try { telemetryExtra.deviceID = this.service.identity.hashedDeviceID(clientId); } catch (_) {} @@ -800,7 +800,7 @@ ClientEngine.prototype = { } } if (didRemoveCommand) { - this._tracker.addChangedID(this.localID); + await this._tracker.addChangedID(this.localID); } if (URIsToDisplay.length) { @@ -914,7 +914,7 @@ ClientEngine.prototype = { async _removeRemoteClient(id) { delete this._store._remoteClients[id]; - this._tracker.removeChangedID(id); + await this._tracker.removeChangedID(id); await this._removeClientCommands(id); this._modified.delete(id); }, @@ -1047,31 +1047,24 @@ ClientStore.prototype = { function ClientsTracker(name, engine) { Tracker.call(this, name, engine); - Svc.Obs.add("weave:engine:start-tracking", this); - Svc.Obs.add("weave:engine:stop-tracking", this); } ClientsTracker.prototype = { __proto__: Tracker.prototype, _enabled: false, - observe: function observe(subject, topic, data) { + onStart() { + Svc.Prefs.observe("client.name", this.asyncObserver); + }, + onStop() { + Svc.Prefs.ignore("client.name", this.asyncObserver); + }, + + async observe(subject, topic, data) { switch (topic) { - case "weave:engine:start-tracking": - if (!this._enabled) { - Svc.Prefs.observe("client.name", this); - this._enabled = true; - } - break; - case "weave:engine:stop-tracking": - if (this._enabled) { - Svc.Prefs.ignore("client.name", this); - this._enabled = false; - } - break; case "nsPref:changed": this._log.debug("client.name preference changed"); - this.addChangedID(this.engine.localID); + await this.addChangedID(this.engine.localID); this.score += SCORE_INCREMENT_XLARGE; break; } diff --git a/services/sync/modules/engines/extension-storage.js b/services/sync/modules/engines/extension-storage.js index 944db3de2bc1..1fd101533a60 100644 --- a/services/sync/modules/engines/extension-storage.js +++ b/services/sync/modules/engines/extension-storage.js @@ -61,17 +61,15 @@ function ExtensionStorageTracker(name, engine) { ExtensionStorageTracker.prototype = { __proto__: Tracker.prototype, - startTracking() { - Svc.Obs.add("ext.storage.sync-changed", this); + onStart() { + Svc.Obs.add("ext.storage.sync-changed", this.asyncObserver); }, - stopTracking() { - Svc.Obs.remove("ext.storage.sync-changed", this); + onStop() { + Svc.Obs.remove("ext.storage.sync-changed", this.asyncObserver); }, - observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); - + async observe(subject, topic, data) { if (this.ignoreAll) { return; } diff --git a/services/sync/modules/engines/forms.js b/services/sync/modules/engines/forms.js index a332a94c834d..0fe7a719c4df 100644 --- a/services/sync/modules/engines/forms.js +++ b/services/sync/modules/engines/forms.js @@ -221,16 +221,15 @@ FormTracker.prototype = { Ci.nsIObserver, Ci.nsISupportsWeakReference]), - startTracking() { - Svc.Obs.add("satchel-storage-changed", this); + onStart() { + Svc.Obs.add("satchel-storage-changed", this.asyncObserver); }, - stopTracking() { - Svc.Obs.remove("satchel-storage-changed", this); + onStop() { + Svc.Obs.remove("satchel-storage-changed", this.asyncObserver); }, - observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); + async observe(subject, topic, data) { if (this.ignoreAll) { return; } @@ -238,14 +237,15 @@ FormTracker.prototype = { case "satchel-storage-changed": if (data == "formhistory-add" || data == "formhistory-remove") { let guid = subject.QueryInterface(Ci.nsISupportsString).toString(); - this.trackEntry(guid); + await this.trackEntry(guid); } break; } }, - trackEntry(guid) { - if (this.addChangedID(guid)) { + async trackEntry(guid) { + const added = await this.addChangedID(guid); + if (added) { this.score += SCORE_INCREMENT_MEDIUM; } }, diff --git a/services/sync/modules/engines/history.js b/services/sync/modules/engines/history.js index b39f34cde310..8cf3d5228f3b 100644 --- a/services/sync/modules/engines/history.js +++ b/services/sync/modules/engines/history.js @@ -74,14 +74,15 @@ HistoryEngine.prototype = { }, async pullNewChanges() { - let modifiedGUIDs = Object.keys(this._tracker.changedIDs); + const changedIDs = await this._tracker.getChangedIDs(); + let modifiedGUIDs = Object.keys(changedIDs); if (!modifiedGUIDs.length) { return {}; } let guidsToRemove = await PlacesSyncUtils.history.determineNonSyncableGuids(modifiedGUIDs); - this._tracker.removeChangedID(...guidsToRemove); - return this._tracker.changedIDs; + await this._tracker.removeChangedID(...guidsToRemove); + return changedIDs; }, }; @@ -437,12 +438,12 @@ function HistoryTracker(name, engine) { HistoryTracker.prototype = { __proto__: Tracker.prototype, - startTracking() { + onStart() { this._log.info("Adding Places observer."); PlacesUtils.history.addObserver(this, true); }, - stopTracking() { + onStop() { this._log.info("Removing Places observer."); PlacesUtils.history.removeObserver(this); }, @@ -452,25 +453,34 @@ HistoryTracker.prototype = { Ci.nsISupportsWeakReference ]), - onDeleteAffectsGUID(uri, guid, reason, source, increment) { + async onDeleteAffectsGUID(uri, guid, reason, source, increment) { if (this.ignoreAll || reason == Ci.nsINavHistoryObserver.REASON_EXPIRED) { return; } this._log.trace(source + ": " + uri.spec + ", reason " + reason); - if (this.addChangedID(guid)) { + const added = await this.addChangedID(guid); + if (added) { this.score += increment; } }, onDeleteVisits(uri, visitTime, guid, reason) { - this.onDeleteAffectsGUID(uri, guid, reason, "onDeleteVisits", SCORE_INCREMENT_SMALL); + this.asyncObserver.enqueueCall(() => + this.onDeleteAffectsGUID(uri, guid, reason, "onDeleteVisits", SCORE_INCREMENT_SMALL) + ); }, onDeleteURI(uri, guid, reason) { - this.onDeleteAffectsGUID(uri, guid, reason, "onDeleteURI", SCORE_INCREMENT_XLARGE); + this.asyncObserver.enqueueCall(() => + this.onDeleteAffectsGUID(uri, guid, reason, "onDeleteURI", SCORE_INCREMENT_XLARGE) + ); }, onVisits(aVisits) { + this.asyncObserver.enqueueCall(() => this._onVisits(aVisits)); + }, + + async _onVisits(aVisits) { if (this.ignoreAll) { this._log.trace("ignoreAll: ignoring visits [" + aVisits.map(v => v.guid).join(",") + "]"); @@ -478,7 +488,7 @@ HistoryTracker.prototype = { } for (let {uri, guid} of aVisits) { this._log.trace("onVisits: " + uri.spec); - if (this.engine.shouldSyncURL(uri.spec) && this.addChangedID(guid)) { + if (this.engine.shouldSyncURL(uri.spec) && (await this.addChangedID(guid))) { this.score += SCORE_INCREMENT_SMALL; } } diff --git a/services/sync/modules/engines/passwords.js b/services/sync/modules/engines/passwords.js index 6e7ca0078068..afdd99521704 100644 --- a/services/sync/modules/engines/passwords.js +++ b/services/sync/modules/engines/passwords.js @@ -329,23 +329,19 @@ PasswordStore.prototype = { function PasswordTracker(name, engine) { Tracker.call(this, name, engine); - Svc.Obs.add("weave:engine:start-tracking", this); - Svc.Obs.add("weave:engine:stop-tracking", this); } PasswordTracker.prototype = { __proto__: Tracker.prototype, - startTracking() { - Svc.Obs.add("passwordmgr-storage-changed", this); + onStart() { + Svc.Obs.add("passwordmgr-storage-changed", this.asyncObserver); }, - stopTracking() { - Svc.Obs.remove("passwordmgr-storage-changed", this); + onStop() { + Svc.Obs.remove("passwordmgr-storage-changed", this.asyncObserver); }, - observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); - + async observe(subject, topic, data) { if (this.ignoreAll) { return; } @@ -361,7 +357,8 @@ PasswordTracker.prototype = { this._log.trace(`${data}: Ignoring change for ${newLogin.guid}`); break; } - if (this._trackLogin(newLogin)) { + const tracked = await this._trackLogin(newLogin); + if (tracked) { this._log.trace(`${data}: Tracking change for ${newLogin.guid}`); } break; @@ -370,7 +367,8 @@ PasswordTracker.prototype = { case "addLogin": case "removeLogin": subject.QueryInterface(Ci.nsILoginMetaInfo).QueryInterface(Ci.nsILoginInfo); - if (this._trackLogin(subject)) { + const tracked = await this._trackLogin(subject); + if (tracked) { this._log.trace(data + ": " + subject.guid); } break; @@ -382,12 +380,13 @@ PasswordTracker.prototype = { } }, - _trackLogin(login) { + async _trackLogin(login) { if (Utils.getSyncCredentialsHosts().has(login.hostname)) { // Skip over Weave password/passphrase changes. return false; } - if (!this.addChangedID(login.guid)) { + const added = await this.addChangedID(login.guid); + if (!added) { return false; } this.score += SCORE_INCREMENT_XLARGE; diff --git a/services/sync/modules/engines/prefs.js b/services/sync/modules/engines/prefs.js index 3602635f6d62..908cb5dc5cf2 100644 --- a/services/sync/modules/engines/prefs.js +++ b/services/sync/modules/engines/prefs.js @@ -235,9 +235,7 @@ PrefStore.prototype = { function PrefTracker(name, engine) { Tracker.call(this, name, engine); - Svc.Obs.add("profile-before-change", this); - Svc.Obs.add("weave:engine:start-tracking", this); - Svc.Obs.add("weave:engine:stop-tracking", this); + Svc.Obs.add("profile-before-change", this.asyncObserver); } PrefTracker.prototype = { __proto__: Tracker.prototype, @@ -261,21 +259,19 @@ PrefTracker.prototype = { return this.__prefs; }, - startTracking() { - Services.prefs.addObserver("", this); + onStart() { + Services.prefs.addObserver("", this.asyncObserver); }, - stopTracking() { + onStop() { this.__prefs = null; - Services.prefs.removeObserver("", this); + Services.prefs.removeObserver("", this.asyncObserver); }, - observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); - + async observe(subject, topic, data) { switch (topic) { case "profile-before-change": - this.stopTracking(); + await this.stop(); break; case "nsPref:changed": if (this.ignoreAll) { diff --git a/services/sync/modules/engines/tabs.js b/services/sync/modules/engines/tabs.js index 0b6422e819d5..69f0ccf3b073 100644 --- a/services/sync/modules/engines/tabs.js +++ b/services/sync/modules/engines/tabs.js @@ -277,8 +277,6 @@ TabStore.prototype = { function TabTracker(name, engine) { Tracker.call(this, name, engine); - Svc.Obs.add("weave:engine:start-tracking", this); - Svc.Obs.add("weave:engine:stop-tracking", this); // Make sure "this" pointer is always set correctly for event listeners. this.onTab = Utils.bind2(this, this.onTab); @@ -322,25 +320,23 @@ TabTracker.prototype = { } }, - startTracking() { - Svc.Obs.add("domwindowopened", this); + onStart() { + Svc.Obs.add("domwindowopened", this.asyncObserver); let wins = Services.wm.getEnumerator("navigator:browser"); while (wins.hasMoreElements()) { this._registerListenersForWindow(wins.getNext()); } }, - stopTracking() { - Svc.Obs.remove("domwindowopened", this); + onStop() { + Svc.Obs.remove("domwindowopened", this.asyncObserver); let wins = Services.wm.getEnumerator("navigator:browser"); while (wins.hasMoreElements()) { this._unregisterListenersForWindow(wins.getNext()); } }, - observe(subject, topic, data) { - Tracker.prototype.observe.call(this, subject, topic, data); - + async observe(subject, topic, data) { switch (topic) { case "domwindowopened": let onLoad = () => { diff --git a/services/sync/modules/service.js b/services/sync/modules/service.js index cf529c4b3daf..069b42abc2ec 100644 --- a/services/sync/modules/service.js +++ b/services/sync/modules/service.js @@ -302,6 +302,7 @@ Sync11Service.prototype = { 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); @@ -319,10 +320,11 @@ Sync11Service.prototype = { "Weave, since it will not work correctly."); } - 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); + 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)); if (!this.enabled) { this._log.info("Firefox Sync disabled."); @@ -332,7 +334,7 @@ Sync11Service.prototype = { let status = this._checkSetup(); if (status != STATUS_DISABLED && status != CLIENT_NOT_CONFIGURED) { - Svc.Obs.notify("weave:engine:start-tracking"); + this._startTracking(); } // Send an event now that Weave service is ready. We don't do this @@ -416,52 +418,43 @@ Sync11Service.prototype = { this.engineManager.setDeclined(declined); }, - QueryInterface: XPCOMUtils.generateQI([Ci.nsIObserver, - Ci.nsISupportsWeakReference]), - - // nsIObserver - - observe: function 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) - Svc.Obs.notify("weave:engine:start-tracking"); - break; - case "nsPref:changed": - if (this._ignorePrefObserver) - return; - let engine = data.slice((PREFS_BRANCH + "engine.").length); - this._handleEngineStatusChanged(engine); - break; + 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); } }, - _handleEngineStatusChanged: function handleEngineDisabled(engine) { + prefObserver(subject, topic, data) { + if (this._ignorePrefObserver) { + return; + } + const engine = data.slice((PREFS_BRANCH + "engine.").length); 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. @@ -472,6 +465,23 @@ Sync11Service.prototype = { } }, + async _startTracking() { + const engines = this.engineManager.getAll(); + for (let engine of engines) { + engine.startTracking(); + } + // This is for TPS. We should try to do better. + Svc.Obs.notify("weave:service:tracking-started"); + }, + + async _stopTracking() { + const engines = this.engineManager.getAll(); + for (let engine of engines) { + await engine.stopTracking(); + } + Svc.Obs.notify("weave:service:tracking-stopped"); + }, + /** * Obtain a Resource instance with authentication credentials. */ @@ -778,7 +788,7 @@ Sync11Service.prototype = { async startOver() { this._log.trace("Invoking Service.startOver."); - Svc.Obs.notify("weave:engine:stop-tracking"); + await this._stopTracking(); this.status.resetSync(); // Deletion doesn't make sense if we aren't set up yet! @@ -1089,6 +1099,7 @@ Sync11Service.prototype = { 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. diff --git a/services/sync/tests/unit/head_helpers.js b/services/sync/tests/unit/head_helpers.js index ba34c4cc6c82..0089120b261d 100644 --- a/services/sync/tests/unit/head_helpers.js +++ b/services/sync/tests/unit/head_helpers.js @@ -148,12 +148,19 @@ async function installAddonFromInstall(install) { * * @param name * String name of add-on to install. e.g. test_install1 + * @param reconciler + * addons reconciler, if passed we will wait on the events to be + * processed before resolving * @return addon object that was installed */ -async function installAddon(name) { +async function installAddon(name, reconciler = null) { let install = await getAddonInstall(name); Assert.notEqual(null, install); - return installAddonFromInstall(install); + const addon = await installAddonFromInstall(install); + if (reconciler) { + await reconciler.queueCaller.promiseCallsComplete(); + } + return addon; } /** @@ -161,19 +168,27 @@ async function installAddon(name) { * * @param addon * Addon instance to uninstall + * @param reconciler + * addons reconciler, if passed we will wait on the events to be + * processed before resolving */ -function uninstallAddon(addon) { - return new Promise(res => { - let listener = {onUninstalled(uninstalled) { - if (uninstalled.id == addon.id) { - AddonManager.removeAddonListener(listener); - res(uninstalled); +async function uninstallAddon(addon, reconciler = null) { + const uninstallPromise = new Promise(res => { + let listener = { + onUninstalled(uninstalled) { + if (uninstalled.id == addon.id) { + AddonManager.removeAddonListener(listener); + res(uninstalled); + } } - }}; - + }; AddonManager.addAddonListener(listener); - addon.uninstall(); }); + addon.uninstall(); + await uninstallPromise; + if (reconciler) { + await reconciler.queueCaller.promiseCallsComplete(); + } } async function generateNewKeys(collectionKeys, collections = null) { diff --git a/services/sync/tests/unit/test_412.js b/services/sync/tests/unit/test_412.js index 151b9e76125c..af19444efd46 100644 --- a/services/sync/tests/unit/test_412.js +++ b/services/sync/tests/unit/test_412.js @@ -27,7 +27,7 @@ add_task(async function test_412_not_treated_as_failure() { // create a new record that should be uploaded and arrange for our lastSync // timestamp to be wrong so we get a 412. engine._store.items = {new: "new record"}; - engine._tracker.addChangedID("new", 0); + await engine._tracker.addChangedID("new", 0); let saw412 = false; let _uploadOutgoing = engine._uploadOutgoing; diff --git a/services/sync/tests/unit/test_addons_engine.js b/services/sync/tests/unit/test_addons_engine.js index 19c1709af71e..d9a628678385 100644 --- a/services/sync/tests/unit/test_addons_engine.js +++ b/services/sync/tests/unit/test_addons_engine.js @@ -27,7 +27,7 @@ async function resetReconciler() { await reconciler.saveState(); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); } add_task(async function setup() { @@ -68,7 +68,7 @@ add_task(async function test_find_dupe() { // test, so we do it manually. await engine._refreshReconcilerState(); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let record = { id: Utils.makeGUID(), @@ -85,7 +85,7 @@ add_task(async function test_find_dupe() { dupe = await engine._findDupe(record); Assert.equal(null, dupe); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); await resetReconciler(); }); @@ -101,7 +101,7 @@ add_task(async function test_get_changed_ids() { let now = new Date(); let changeTime = now.getTime() / 1000; let guid1 = Utils.makeGUID(); - tracker.addChangedID(guid1, changeTime); + await tracker.addChangedID(guid1, changeTime); changes = await engine.getChangedIDs(); Assert.equal("object", typeof(changes)); @@ -109,11 +109,11 @@ add_task(async function test_get_changed_ids() { Assert.ok(guid1 in changes); Assert.equal(changeTime, changes[guid1]); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); _("Ensure reconciler changes are populated."); - let addon = await installAddon("test_bootstrap1_1"); - tracker.clearChangedIDs(); // Just in case. + let addon = await installAddon("test_bootstrap1_1", reconciler); + await tracker.clearChangedIDs(); // Just in case. changes = await engine.getChangedIDs(); Assert.equal("object", typeof(changes)); Assert.equal(1, Object.keys(changes).length); @@ -123,7 +123,7 @@ add_task(async function test_get_changed_ids() { let oldTime = changes[addon.syncGUID]; let guid2 = addon.syncGUID; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); changes = await engine.getChangedIDs(); Assert.equal(1, Object.keys(changes).length); Assert.ok(guid2 in changes); diff --git a/services/sync/tests/unit/test_addons_reconciler.js b/services/sync/tests/unit/test_addons_reconciler.js index 345123fdf7b6..96cf47c5a7db 100644 --- a/services/sync/tests/unit/test_addons_reconciler.js +++ b/services/sync/tests/unit/test_addons_reconciler.js @@ -4,6 +4,7 @@ "use strict"; ChromeUtils.import("resource://gre/modules/AddonManager.jsm"); +ChromeUtils.import("resource://services-common/async.js"); ChromeUtils.import("resource://services-sync/addonsreconciler.js"); ChromeUtils.import("resource://services-sync/engines/addons.js"); ChromeUtils.import("resource://services-sync/service.js"); @@ -12,7 +13,13 @@ ChromeUtils.import("resource://services-sync/util.js"); loadAddonTestFunctions(); startupManager(); -add_task(async function run_test() { +function makeAddonsReconciler() { + const log = Service.engineManager.get("addons")._log; + const queueCaller = Async.asyncQueueCaller(log); + return new AddonsReconciler(queueCaller); +} + +add_task(async function setup() { Svc.Prefs.set("engine.addons", true); await Service.engineManager.register(AddonsEngine); }); @@ -20,7 +27,7 @@ add_task(async function run_test() { add_task(async function test_defaults() { _("Ensure new objects have reasonable defaults."); - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); Assert.ok(!reconciler._listening); @@ -33,7 +40,7 @@ add_task(async function test_defaults() { add_task(async function test_load_state_empty_file() { _("Ensure loading from a missing file results in defaults being set."); - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); let loaded = await reconciler.loadState(); @@ -47,7 +54,7 @@ add_task(async function test_load_state_empty_file() { add_task(async function test_install_detection() { _("Ensure that add-on installation results in appropriate side-effects."); - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); reconciler.startListening(); @@ -86,7 +93,7 @@ add_task(async function test_install_detection() { add_task(async function test_uninstall_detection() { _("Ensure that add-on uninstallation results in appropriate side-effects."); - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); reconciler.startListening(); @@ -97,7 +104,7 @@ add_task(async function test_uninstall_detection() { let id = addon.id; reconciler._changes = []; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); Assert.equal(1, Object.keys(reconciler.addons).length); Assert.ok(id in reconciler.addons); @@ -116,7 +123,7 @@ add_task(async function test_load_state_future_version() { const FILENAME = "TEST_LOAD_STATE_FUTURE_VERSION"; - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); // First we populate our new file. @@ -137,7 +144,7 @@ add_task(async function test_load_state_future_version() { add_task(async function test_prune_changes_before_date() { _("Ensure that old changes are pruned properly."); - let reconciler = new AddonsReconciler(); + let reconciler = makeAddonsReconciler(); await reconciler.ensureStateLoaded(); reconciler._changes = []; diff --git a/services/sync/tests/unit/test_addons_store.js b/services/sync/tests/unit/test_addons_store.js index fc8fa55849c8..2385f7b0af31 100644 --- a/services/sync/tests/unit/test_addons_store.js +++ b/services/sync/tests/unit/test_addons_store.js @@ -119,7 +119,7 @@ add_task(async function setup() { add_task(async function test_remove() { _("Ensure removing add-ons from deleted records works."); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let record = createRecordForThisApp(addon.syncGUID, addon.id, true, true); let failed = await store.applyIncomingBatch([record]); @@ -132,7 +132,7 @@ add_task(async function test_remove() { add_task(async function test_apply_enabled() { _("Ensures that changes to the userEnabled flag apply."); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); Assert.ok(addon.isActive); Assert.ok(!addon.userDisabled); @@ -164,7 +164,7 @@ add_task(async function test_apply_enabled() { Assert.ok(!addon.userDisabled); records = []; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); Svc.Prefs.reset("addons.ignoreUserEnabledChanges"); }); @@ -197,14 +197,14 @@ add_task(async function test_apply_enabled_appDisabled() { await checkReconcilerUpToDate(addon); records = []; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); }); add_task(async function test_ignore_different_appid() { _("Ensure that incoming records with a different application ID are ignored."); // We test by creating a record that should result in an update. - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); Assert.ok(!addon.userDisabled); let record = createRecordForThisApp(addon.syncGUID, addon.id, false, false); @@ -216,13 +216,13 @@ add_task(async function test_ignore_different_appid() { let newAddon = await AddonManager.getAddonByID(addon.id); Assert.ok(!newAddon.userDisabled); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); }); add_task(async function test_ignore_unknown_source() { _("Ensure incoming records with unknown source are ignored."); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let record = createRecordForThisApp(addon.syncGUID, addon.id, false, false); record.source = "DUMMY_SOURCE"; @@ -233,13 +233,13 @@ add_task(async function test_ignore_unknown_source() { let newAddon = await AddonManager.getAddonByID(addon.id); Assert.ok(!newAddon.userDisabled); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); }); add_task(async function test_apply_uninstall() { _("Ensures that uninstalling an add-on from a record works."); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let records = []; records.push(createRecordForThisApp(addon.syncGUID, addon.id, true, true)); @@ -258,7 +258,7 @@ add_task(async function test_addon_syncability() { Assert.ok(!(await store.isAddonSyncable(null))); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); Assert.ok((await store.isAddonSyncable(addon))); let dummy = {}; @@ -285,7 +285,7 @@ add_task(async function test_addon_syncability() { Assert.ok(!(await store.isAddonSyncable(dummy))); dummy.foreignInstall = false; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); Assert.ok(!store.isSourceURITrusted(null)); @@ -329,9 +329,9 @@ add_task(async function test_get_all_ids() { // So if any tests above ever add a new addon ID, they are going to need to // be added here too. // Assert.equal(0, Object.keys(store.getAllIDs()).length); - let addon1 = await installAddon("test_install1"); - let addon2 = await installAddon("test_bootstrap1_1"); - let addon3 = await installAddon("test_install3"); + let addon1 = await installAddon("test_install1", reconciler); + let addon2 = await installAddon("test_bootstrap1_1", reconciler); + let addon3 = await installAddon("test_install3", reconciler); _("Ensure they're syncable."); Assert.ok((await store.isAddonSyncable(addon1))); @@ -347,14 +347,14 @@ add_task(async function test_get_all_ids() { Assert.ok(addon3.syncGUID in ids); addon1.install.cancel(); - await uninstallAddon(addon2); - await uninstallAddon(addon3); + await uninstallAddon(addon2, reconciler); + await uninstallAddon(addon3, reconciler); }); add_task(async function test_change_item_id() { _("Ensures that changeItemID() works properly."); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let oldID = addon.syncGUID; let newID = Utils.makeGUID(); @@ -365,7 +365,7 @@ add_task(async function test_change_item_id() { Assert.notEqual(null, newAddon); Assert.equal(newID, newAddon.syncGUID); - await uninstallAddon(newAddon); + await uninstallAddon(newAddon, reconciler); }); add_task(async function test_create() { @@ -373,9 +373,9 @@ add_task(async function test_create() { let server = createAndStartHTTPServer(HTTP_PORT); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let id = addon.id; - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); let guid = Utils.makeGUID(); let record = createRecordForThisApp(guid, id, true, false); @@ -388,7 +388,7 @@ add_task(async function test_create() { Assert.equal(guid, newAddon.syncGUID); Assert.ok(!newAddon.userDisabled); - await uninstallAddon(newAddon); + await uninstallAddon(newAddon, reconciler); await promiseStopServer(server); }); @@ -486,7 +486,7 @@ add_task(async function test_incoming_system() { add_task(async function test_wipe() { _("Ensures that wiping causes add-ons to be uninstalled."); - let addon1 = await installAddon("test_bootstrap1_1"); + let addon1 = await installAddon("test_bootstrap1_1", reconciler); await store.wipe(); @@ -500,7 +500,7 @@ add_task(async function test_wipe_and_install() { // This tests the reset sync flow where remote data is replaced by local. The // receiving client will see a wipe followed by a record which should undo // the wipe. - let installed = await installAddon("test_bootstrap1_1"); + let installed = await installAddon("test_bootstrap1_1", reconciler); let record = createRecordForThisApp(installed.syncGUID, installed.id, true, false); diff --git a/services/sync/tests/unit/test_addons_tracker.js b/services/sync/tests/unit/test_addons_tracker.js index f6695388b11d..e7fb6bdced22 100644 --- a/services/sync/tests/unit/test_addons_tracker.js +++ b/services/sync/tests/unit/test_addons_tracker.js @@ -21,11 +21,10 @@ let tracker; const addon1ID = "addon1@tests.mozilla.org"; async function cleanup() { - Svc.Obs.notify("weave:engine:stop-tracking"); - tracker.stopTracking(); + tracker.stop(); tracker.resetScore(); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); reconciler._addons = {}; reconciler._changes = []; @@ -48,7 +47,7 @@ add_task(async function setup() { add_task(async function test_empty() { _("Verify the tracker is empty to start with."); - Assert.equal(0, Object.keys(tracker.changedIDs).length); + Assert.equal(0, Object.keys((await tracker.getChangedIDs())).length); Assert.equal(0, tracker.score); await cleanup(); @@ -57,10 +56,10 @@ add_task(async function test_empty() { add_task(async function test_not_tracking() { _("Ensures the tracker doesn't do anything when it isn't tracking."); - let addon = await installAddon("test_bootstrap1_1"); - await uninstallAddon(addon); + let addon = await installAddon("test_bootstrap1_1", reconciler); + await uninstallAddon(addon, reconciler); - Assert.equal(0, Object.keys(tracker.changedIDs).length); + Assert.equal(0, Object.keys((await tracker.getChangedIDs())).length); Assert.equal(0, tracker.score); await cleanup(); @@ -71,17 +70,17 @@ add_task(async function test_track_install() { reconciler.startListening(); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); Assert.equal(0, tracker.score); - let addon = await installAddon("test_bootstrap1_1"); - let changed = tracker.changedIDs; + let addon = await installAddon("test_bootstrap1_1", reconciler); + let changed = await tracker.getChangedIDs(); Assert.equal(1, Object.keys(changed).length); Assert.ok(addon.syncGUID in changed); Assert.equal(SCORE_INCREMENT_XLARGE, tracker.score); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); await cleanup(); }); @@ -90,14 +89,14 @@ add_task(async function test_track_uninstall() { reconciler.startListening(); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); let guid = addon.syncGUID; Assert.equal(0, tracker.score); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); - await uninstallAddon(addon); - let changed = tracker.changedIDs; + await uninstallAddon(addon, reconciler); + let changed = await tracker.getChangedIDs(); Assert.equal(1, Object.keys(changed).length); Assert.ok(guid in changed); Assert.equal(SCORE_INCREMENT_XLARGE, tracker.score); @@ -110,12 +109,12 @@ add_task(async function test_track_user_disable() { reconciler.startListening(); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); Assert.ok(!addon.userDisabled); Assert.ok(!addon.appDisabled); Assert.ok(addon.isActive); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); Assert.equal(0, tracker.score); let disabledPromise = new Promise(res => { @@ -138,13 +137,14 @@ add_task(async function test_track_user_disable() { addon.userDisabled = true; _("Disabling started..."); await disabledPromise; + await reconciler.queueCaller.promiseCallsComplete(); - let changed = tracker.changedIDs; + let changed = await tracker.getChangedIDs(); Assert.equal(1, Object.keys(changed).length); Assert.ok(addon.syncGUID in changed); Assert.equal(SCORE_INCREMENT_XLARGE, tracker.score); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); await cleanup(); }); @@ -153,21 +153,22 @@ add_task(async function test_track_enable() { reconciler.startListening(); - let addon = await installAddon("test_bootstrap1_1"); + let addon = await installAddon("test_bootstrap1_1", reconciler); addon.userDisabled = true; await Async.promiseYield(); Assert.equal(0, tracker.score); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); addon.userDisabled = false; await Async.promiseYield(); + await reconciler.queueCaller.promiseCallsComplete(); - let changed = tracker.changedIDs; + let changed = await tracker.getChangedIDs(); Assert.equal(1, Object.keys(changed).length); Assert.ok(addon.syncGUID in changed); Assert.equal(SCORE_INCREMENT_XLARGE, tracker.score); - await uninstallAddon(addon); + await uninstallAddon(addon, reconciler); await cleanup(); }); diff --git a/services/sync/tests/unit/test_bookmark_duping.js b/services/sync/tests/unit/test_bookmark_duping.js index 35c50b82bc65..e4c749570434 100644 --- a/services/sync/tests/unit/test_bookmark_duping.js +++ b/services/sync/tests/unit/test_bookmark_duping.js @@ -23,13 +23,13 @@ async function sharedSetup() { let collection = server.user("foo").collection("bookmarks"); - Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup... + engine._tracker.start(); // We skip usual startup... return { engine, store, server, collection }; } async function cleanup(engine, server) { - Svc.Obs.notify("weave:engine:stop-tracking"); + await engine._tracker.stop(); let promiseStartOver = promiseOneObserver("weave:service:start-over:finish"); await Service.startOver(); await promiseStartOver; diff --git a/services/sync/tests/unit/test_bookmark_engine.js b/services/sync/tests/unit/test_bookmark_engine.js index 1e327b5abfdb..f1887c37f140 100644 --- a/services/sync/tests/unit/test_bookmark_engine.js +++ b/services/sync/tests/unit/test_bookmark_engine.js @@ -69,7 +69,7 @@ add_bookmark_test(async function test_delete_invalid_roots_from_server(engine) { let collection = server.user("foo").collection("bookmarks"); - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let placesRecord = await store.createRecord("places"); @@ -116,7 +116,7 @@ add_bookmark_test(async function test_delete_invalid_roots_from_server(engine) { Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); await promiseStopServer(server); - Svc.Obs.notify("weave:engine:stop-tracking"); + await engine._tracker.stop(); } }); @@ -254,7 +254,7 @@ async function test_restoreOrImport(engine, { replace }) { let collection = server.user("foo").collection("bookmarks"); - Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup... + engine._tracker.start(); // We skip usual startup... try { @@ -308,6 +308,7 @@ async function test_restoreOrImport(engine, { replace }) { _(`Now ${verb} from a backup.`); await bookmarkUtils.importFromFile(backupFilePath, replace); + await engine._tracker.asyncObserver.promiseObserversComplete(); let bookmarksCollection = server.user("foo").collection("bookmarks"); if (replace) { @@ -700,7 +701,7 @@ add_bookmark_test(async function test_sync_dateAdded(engine) { // intermittently - reset the last sync date so that we'll get all bookmarks. await engine.setLastSync(1); - Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup... + engine._tracker.start(); // We skip usual startup... // Just matters that it's in the past, not how far. let now = Date.now(); @@ -849,7 +850,7 @@ add_task(async function test_sync_imap_URLs() { let collection = server.user("foo").collection("bookmarks"); - Svc.Obs.notify("weave:engine:start-tracking"); // We skip usual startup... + engine._tracker.start(); // We skip usual startup... try { collection.insert("menu", encryptPayload({ diff --git a/services/sync/tests/unit/test_bookmark_store.js b/services/sync/tests/unit/test_bookmark_store.js index df2b12ef12d2..d6f68c78580b 100644 --- a/services/sync/tests/unit/test_bookmark_store.js +++ b/services/sync/tests/unit/test_bookmark_store.js @@ -337,7 +337,7 @@ add_task(async function test_move_order() { let tracker = engine._tracker; // Make sure the tracker is turned on. - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); try { _("Create two bookmarks"); let bmk1 = await PlacesUtils.bookmarks.insert({ @@ -374,7 +374,7 @@ add_task(async function test_move_order() { Assert.deepEqual(newChildIds, [bmk2.guid, bmk1.guid]); } finally { - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); _("Clean up."); await store.wipe(); await engine.finalize(); diff --git a/services/sync/tests/unit/test_bookmark_tracker.js b/services/sync/tests/unit/test_bookmark_tracker.js index ceb62b05f00a..5f1c90f46e09 100644 --- a/services/sync/tests/unit/test_bookmark_tracker.js +++ b/services/sync/tests/unit/test_bookmark_tracker.js @@ -27,7 +27,7 @@ add_task(async function setup() { // Test helpers. async function verifyTrackerEmpty() { await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}); equal(tracker.score, 0); } @@ -42,23 +42,19 @@ async function cleanup() { engine._needWeakUpload.clear(); await store.wipe(); await resetTracker(); - await stopTracking(); + await tracker.stop(); } // startTracking is a signal that the test wants to notice things that happen // after this is called (ie, things already tracked should be discarded.) async function startTracking() { - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); await PlacesTestUtils.markBookmarksAsSynced(); } -async function stopTracking() { - Svc.Obs.notify("weave:engine:stop-tracking"); -} - async function verifyTrackedItems(tracked) { await PlacesTestUtils.promiseAsyncUpdates(); - let changedIDs = await tracker.promiseChangedIDs(); + let changedIDs = await tracker.getChangedIDs(); let trackedIDs = new Set(Object.keys(changedIDs)); for (let guid of tracked) { ok(guid in changedIDs, `${guid} should be tracked`); @@ -72,7 +68,7 @@ async function verifyTrackedItems(tracked) { async function verifyTrackedCount(expected) { await PlacesTestUtils.promiseAsyncUpdates(); - let changedIDs = await tracker.promiseChangedIDs(); + let changedIDs = await tracker.getChangedIDs(); do_check_attribute_count(changedIDs, expected); } @@ -190,7 +186,7 @@ add_task(async function test_leftPaneFolder() { { await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "New left pane queries should not be tracked"); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); } @@ -200,7 +196,7 @@ add_task(async function test_leftPaneFolder() { { await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(Object.keys(changes).sort(), ["menu", "mobile", "toolbar", "unfiled"], "Left pane queries should not be tracked after reset"); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); @@ -226,7 +222,7 @@ add_task(async function test_leftPaneFolder() { _(`Left pane root ID after deleting unrelated folder: ${leftPaneId}`); await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "Should not track left pane items after deleting unrelated folder"); } @@ -240,7 +236,7 @@ add_task(async function test_leftPaneFolder() { _(`Left pane root ID after restoring version: ${leftPaneId}`); await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "Should not track left pane items after restoring version"); } @@ -254,7 +250,7 @@ add_task(async function test_leftPaneFolder() { _(`Left pane root ID after detecting nonexistent item: ${leftPaneId}`); await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "Should not track left pane items after detecting nonexistent item"); } @@ -273,7 +269,7 @@ add_task(async function test_leftPaneFolder() { _(`Left pane root ID after restoring moved query: ${leftPaneId}`); await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "Should not track left pane items after restoring moved query"); } @@ -293,7 +289,7 @@ add_task(async function test_leftPaneFolder() { _(`Left pane root ID after removing dupe query: ${leftPaneId}`); await PlacesTestUtils.promiseAsyncUpdates(); - let changes = await tracker.promiseChangedIDs(); + let changes = await tracker.getChangedIDs(); deepEqual(changes, {}, "Should not track left pane items after removing dupe query"); } @@ -525,7 +521,7 @@ add_task(async function test_async_onItemChanged() { _("Items updated using the asynchronous bookmarks API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert a bookmark"); let fxBmk = await PlacesUtils.bookmarks.insert({ @@ -560,7 +556,7 @@ add_task(async function test_onItemChanged_itemDates() { _("Changes to item dates should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert a bookmark"); let fx_id = PlacesUtils.bookmarks.insertBookmark( @@ -596,7 +592,7 @@ add_task(async function test_onItemTagged() { _("Items tagged using the synchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Create a folder"); let folder = PlacesUtils.bookmarks.createFolder( @@ -633,7 +629,7 @@ add_task(async function test_onItemUntagged() { _("Items untagged using the synchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert tagged bookmarks"); let uri = CommonUtils.makeURI("http://getfirefox.com"); @@ -665,7 +661,7 @@ add_task(async function test_async_onItemUntagged() { _("Items untagged using the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert tagged bookmarks"); let fxBmk1 = await PlacesUtils.bookmarks.insert({ @@ -708,7 +704,7 @@ add_task(async function test_async_onItemTagged() { _("Items tagged using the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert untagged bookmarks"); let folder1 = await PlacesUtils.bookmarks.insert({ @@ -766,7 +762,7 @@ add_task(async function test_onItemKeywordChanged() { _("Keyword changes via the synchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); let folder = PlacesUtils.bookmarks.createFolder( PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent", PlacesUtils.bookmarks.DEFAULT_INDEX); @@ -798,7 +794,7 @@ add_task(async function test_async_onItemKeywordChanged() { _("Keyword changes via the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert two bookmarks with the same URL"); let fxBmk1 = await PlacesUtils.bookmarks.insert({ @@ -835,7 +831,7 @@ add_task(async function test_async_onItemKeywordDeleted() { _("Keyword deletions via the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert two bookmarks with the same URL and keywords"); let fxBmk1 = await PlacesUtils.bookmarks.insert({ @@ -872,7 +868,7 @@ add_task(async function test_onItemAnnoChanged() { _("Item annotations should be tracked"); try { - await stopTracking(); + await tracker.stop(); let folder = PlacesUtils.bookmarks.createFolder( PlacesUtils.bookmarks.bookmarksMenuFolder, "Parent", PlacesUtils.bookmarks.DEFAULT_INDEX); @@ -947,7 +943,7 @@ add_task(async function test_onItemDeleted_filtered_root() { _("Deleted items outside the change roots should not be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert a bookmark underneath the Places root"); let rootBmkID = PlacesUtils.bookmarks.insertBookmark( @@ -974,7 +970,7 @@ add_task(async function test_onPageAnnoChanged() { _("Page annotations should not be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert a bookmark without an annotation"); let pageURI = CommonUtils.makeURI("http://getfirefox.com"); @@ -1008,7 +1004,7 @@ add_task(async function test_onFaviconChanged() { _("Favicon changes should not be tracked"); try { - await stopTracking(); + await tracker.stop(); let pageURI = CommonUtils.makeURI("http://getfirefox.com"); let iconURI = CommonUtils.makeURI("http://getfirefox.com/icon"); @@ -1074,7 +1070,7 @@ add_task(async function test_onLivemarkDeleted() { _("Deleted livemarks should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert a livemark"); let livemark = await PlacesUtils.livemarks.addLivemark({ @@ -1144,7 +1140,7 @@ add_task(async function test_async_onItemMoved_update() { _("Items moved via the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, @@ -1189,7 +1185,7 @@ add_task(async function test_async_onItemMoved_reorder() { _("Items reordered via the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Insert out-of-order bookmarks"); let fxBmk = await PlacesUtils.bookmarks.insert({ @@ -1236,7 +1232,7 @@ add_task(async function test_onItemDeleted_removeFolderTransaction() { _("Folders removed in a transaction should be tracked"); try { - await stopTracking(); + await tracker.stop(); _("Create a folder with two children"); let folder_id = PlacesUtils.bookmarks.createFolder( @@ -1369,7 +1365,7 @@ add_task(async function test_async_onItemDeleted() { _("Bookmarks deleted via the asynchronous API should be tracked"); try { - await stopTracking(); + await tracker.stop(); let fxBmk = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, @@ -1401,7 +1397,7 @@ add_task(async function test_async_onItemDeleted_eraseEverything() { _("Erasing everything should track all deleted items"); try { - await stopTracking(); + await tracker.stop(); let fxBmk = await PlacesUtils.bookmarks.insert({ type: PlacesUtils.bookmarks.TYPE_BOOKMARK, diff --git a/services/sync/tests/unit/test_clients_engine.js b/services/sync/tests/unit/test_clients_engine.js index 4992d5b99a88..0030997bc001 100644 --- a/services/sync/tests/unit/test_clients_engine.js +++ b/services/sync/tests/unit/test_clients_engine.js @@ -58,7 +58,7 @@ add_task(async function setup() { async function cleanup() { Svc.Prefs.resetBranch(""); - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); await engine._resetClient(); // un-cleanup the logs (the resetBranch will have reset their levels), since // not all the tests use SyncTestingInfrastructure, and it's cheap. @@ -335,26 +335,29 @@ add_task(async function test_client_name_change() { engine.localID; // Needed to increase the tracker changedIDs count. let initialName = engine.localName; - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); _("initial name: " + initialName); // Tracker already has data, so clear it. - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); let initialScore = tracker.score; - equal(Object.keys(tracker.changedIDs).length, 0); + let changedIDs = await tracker.getChangedIDs(); + equal(Object.keys(changedIDs).length, 0); Svc.Prefs.set("client.name", "new name"); + await tracker.asyncObserver.promiseObserversComplete(); _("new name: " + engine.localName); notEqual(initialName, engine.localName); - equal(Object.keys(tracker.changedIDs).length, 1); - ok(engine.localID in tracker.changedIDs); + changedIDs = await tracker.getChangedIDs(); + equal(Object.keys(changedIDs).length, 1); + ok(engine.localID in changedIDs); ok(tracker.score > initialScore); ok(tracker.score >= SCORE_INCREMENT_XLARGE); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); await cleanup(); }); @@ -439,7 +442,9 @@ add_task(async function test_send_command() { deepEqual(command.args, args); ok(command.flowID); - notEqual(tracker.changedIDs[remoteId], undefined); + + const changes = await tracker.getChangedIDs(); + notEqual(changes[remoteId], undefined); await cleanup(); }); @@ -501,7 +506,8 @@ add_task(async function test_command_validation() { deepEqual(command.args, args); notEqual(engine._tracker, undefined); - notEqual(engine._tracker.changedIDs[remoteId], undefined); + const changes = await engine._tracker.getChangedIDs(); + notEqual(changes[remoteId], undefined); } else { _("Ensuring command is scrubbed: " + action); equal(clientCommands, undefined); @@ -946,7 +952,7 @@ add_task(async function test_send_uri_to_client_for_display() { await store.create(rec); await store.createRecord(remoteId, "clients"); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); let initialScore = tracker.score; let uri = "http://www.mozilla.org/"; @@ -1537,7 +1543,7 @@ add_task(async function test_command_sync() { equal(collection.count(), 3, "3 remote records written (+1 for the synced local record)"); await engine.sendCommand("wipeAll", []); - engine._tracker.addChangedID(engine.localID); + await engine._tracker.addChangedID(engine.localID); const getClientFxaDeviceId = sinon.stub(engine, "getClientFxaDeviceId", (id) => "fxa-" + id); const engineMock = sinon.mock(engine); let _notifyCollectionChanged = engineMock.expects("_notifyCollectionChanged") @@ -1550,7 +1556,7 @@ add_task(async function test_command_sync() { getClientFxaDeviceId.restore(); } finally { await cleanup(); - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); try { server.deleteCollections("foo"); @@ -1748,11 +1754,13 @@ add_task(async function device_disconnected_notification_updates_known_stale_cli 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(); diff --git a/services/sync/tests/unit/test_engine.js b/services/sync/tests/unit/test_engine.js index 021fe74742ab..6e68bb4d5178 100644 --- a/services/sync/tests/unit/test_engine.js +++ b/services/sync/tests/unit/test_engine.js @@ -2,6 +2,7 @@ http://creativecommons.org/publicdomain/zero/1.0/ */ ChromeUtils.import("resource://gre/modules/osfile.jsm"); +ChromeUtils.import("resource://gre/modules/PromiseUtils.jsm"); ChromeUtils.import("resource://services-common/observers.js"); ChromeUtils.import("resource://services-sync/engines.js"); ChromeUtils.import("resource://services-sync/service.js"); @@ -70,7 +71,7 @@ async function cleanup(engine) { engine.wasReset = false; engine.wasSynced = false; engineObserver.reset(); - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); await engine.finalize(); } @@ -123,12 +124,13 @@ add_task(async function test_invalidChangedIDs() { { tmpPath: tracker._storage.path + ".tmp" }); ok(!tracker._storage.dataReady); - tracker.changedIDs.placeholder = true; - deepEqual(tracker.changedIDs, { placeholder: true }, + const changes = await tracker.getChangedIDs(); + changes.placeholder = true; + deepEqual(changes, { placeholder: true }, "Accessing changed IDs should load changes from disk as a side effect"); ok(tracker._storage.dataReady); - Assert.ok(tracker.changedIDs.placeholder); + Assert.ok(changes.placeholder); await cleanup(engine); }); @@ -137,13 +139,15 @@ add_task(async function test_wipeClient() { let engine = new SteamEngine("Steam", Service); Assert.ok(!engine.wasReset); Assert.ok(!engine._store.wasWiped); - Assert.ok(engine._tracker.addChangedID("a-changed-id")); - Assert.ok("a-changed-id" in engine._tracker.changedIDs); + Assert.ok((await engine._tracker.addChangedID("a-changed-id"))); + let changes = await engine._tracker.getChangedIDs(); + Assert.ok("a-changed-id" in changes); await engine.wipeClient(); Assert.ok(engine.wasReset); Assert.ok(engine._store.wasWiped); - Assert.equal(JSON.stringify(engine._tracker.changedIDs), "{}"); + changes = await engine._tracker.getChangedIDs(); + Assert.equal(JSON.stringify(changes), "{}"); Assert.equal(engineObserver.topics[0], "weave:engine:wipe-client:start"); Assert.equal(engineObserver.topics[1], "weave:engine:reset-client:start"); Assert.equal(engineObserver.topics[2], "weave:engine:reset-client:finish"); @@ -197,23 +201,36 @@ add_task(async function test_disabled_no_track() { Assert.ok(!engine.enabled); Assert.ok(!tracker._isTracking); - do_check_empty(tracker.changedIDs); + let changes = await tracker.getChangedIDs(); + do_check_empty(changes); Assert.ok(!tracker.engineIsEnabled()); - tracker.observe(null, "weave:engine:start-tracking", null); Assert.ok(!tracker._isTracking); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); - engine.enabled = true; - tracker.observe(null, "weave:engine:start-tracking", null); + let promisePrefChangeHandled = PromiseUtils.defer(); + const origMethod = tracker.onEngineEnabledChanged; + tracker.onEngineEnabledChanged = async (...args) => { + await origMethod.apply(tracker, args); + promisePrefChangeHandled.resolve(); + }; + + engine.enabled = true; // Also enables the tracker automatically. + await promisePrefChangeHandled.promise; Assert.ok(tracker._isTracking); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); - tracker.addChangedID("abcdefghijkl"); - Assert.ok(0 < tracker.changedIDs.abcdefghijkl); + await tracker.addChangedID("abcdefghijkl"); + changes = await tracker.getChangedIDs(); + Assert.ok(0 < changes.abcdefghijkl); + promisePrefChangeHandled = PromiseUtils.defer(); Svc.Prefs.set("engine." + engine.prefName, false); + await promisePrefChangeHandled.promise; Assert.ok(!tracker._isTracking); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); await cleanup(engine); }); diff --git a/services/sync/tests/unit/test_engine_abort.js b/services/sync/tests/unit/test_engine_abort.js index d7cc4ceffb21..e15519b3e017 100644 --- a/services/sync/tests/unit/test_engine_abort.js +++ b/services/sync/tests/unit/test_engine_abort.js @@ -62,6 +62,6 @@ add_task(async function test_processIncoming_abort() { Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); await engine.finalize(); }); diff --git a/services/sync/tests/unit/test_engine_changes_during_sync.js b/services/sync/tests/unit/test_engine_changes_during_sync.js index b6ae67cf0134..e193c51ec47d 100644 --- a/services/sync/tests/unit/test_engine_changes_during_sync.js +++ b/services/sync/tests/unit/test_engine_changes_during_sync.js @@ -25,7 +25,7 @@ async function assertChildGuids(folderGuid, expectedChildGuids, message) { } async function cleanup(engine, server) { - Svc.Obs.notify("weave:engine:stop-tracking"); + await engine._tracker.stop(); await engine._store.wipe(); Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); @@ -52,10 +52,11 @@ add_task(async function test_history_change_during_sync() { } finally { _("Inserting local history visit"); await addVisit("during_sync"); + await engine._tracker.asyncObserver.promiseObserversComplete(); } }; - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let remoteRec = new HistoryRec("history", "UrOOuzE5QM-e"); @@ -106,10 +107,11 @@ add_task(async function test_passwords_change_during_sync() { let login = new LoginInfo("https://example.com", "", null, "username", "password", "", ""); Services.logins.addLogin(login); + await engine._tracker.asyncObserver.promiseObserversComplete(); } }; - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let remoteRec = new LoginRec("passwords", "{765e3d6e-071d-d640-a83d-81a7eb62d3ed}"); @@ -162,10 +164,11 @@ add_task(async function test_prefs_change_during_sync() { _("Updating local pref value"); // Change the value of a synced pref. Services.prefs.setCharPref(TEST_PREF, "hello"); + await engine._tracker.asyncObserver.promiseObserversComplete(); } }; - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { // All synced prefs are stored in a single record, so we'll only ever @@ -228,10 +231,11 @@ add_task(async function test_forms_change_during_sync() { handleCompletion: resolve, }); }); + await engine._tracker.asyncObserver.promiseObserversComplete(); } }; - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { // Add an existing remote form history entry. We shouldn't bump the score when @@ -298,6 +302,7 @@ add_task(async function test_bookmark_change_during_sync() { url: "https://mozilla.org/", title: "Mozilla", }); + await engine._tracker.asyncObserver.promiseObserversComplete(); } }; @@ -316,7 +321,7 @@ add_task(async function test_bookmark_change_during_sync() { }); _(`Thunderbird GUID: ${tbBmk.guid}`); - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let bmk2_guid = "get-firefox1"; // New child of Folder 1, created remotely. diff --git a/services/sync/tests/unit/test_extension_storage_tracker.js b/services/sync/tests/unit/test_extension_storage_tracker.js index 385a9dbdeaba..7e046d90459d 100644 --- a/services/sync/tests/unit/test_extension_storage_tracker.js +++ b/services/sync/tests/unit/test_extension_storage_tracker.js @@ -23,7 +23,7 @@ add_task(async function setup() { add_task(async function test_changing_extension_storage_changes_score() { const tracker = engine._tracker; const extension = {id: "my-extension-id"}; - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); await withSyncContext(async function(context) { await extensionStorageSync.set(extension, {"a": "b"}, context); }); @@ -35,5 +35,5 @@ add_task(async function test_changing_extension_storage_changes_score() { }); Assert.equal(tracker.score, SCORE_INCREMENT_MEDIUM); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); }); diff --git a/services/sync/tests/unit/test_forms_tracker.js b/services/sync/tests/unit/test_forms_tracker.js index 8be6052046b2..98eff9bdbff8 100644 --- a/services/sync/tests/unit/test_forms_tracker.js +++ b/services/sync/tests/unit/test_forms_tracker.js @@ -14,55 +14,64 @@ add_task(async function run_test() { // Don't do asynchronous writes. tracker.persistChangedIDs = false; - do_check_empty(tracker.changedIDs); + let changes = await tracker.getChangedIDs(); + do_check_empty(changes); Log.repository.rootLogger.addAppender(new Log.DumpAppender()); async function addEntry(name, value) { await engine._store.create({name, value}); + await engine._tracker.asyncObserver.promiseObserversComplete(); } async function removeEntry(name, value) { let guid = await engine._findDupe({name, value}); await engine._store.remove({id: guid}); + await engine._tracker.asyncObserver.promiseObserversComplete(); } try { _("Create an entry. Won't show because we haven't started tracking yet"); await addEntry("name", "John Doe"); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); _("Tell the tracker to start tracking changes."); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); await removeEntry("name", "John Doe"); await addEntry("email", "john@doe.com"); - do_check_attribute_count(tracker.changedIDs, 2); + changes = await tracker.getChangedIDs(); + do_check_attribute_count(changes, 2); _("Notifying twice won't do any harm."); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); await addEntry("address", "Memory Lane"); - do_check_attribute_count(tracker.changedIDs, 3); + changes = await tracker.getChangedIDs(); + do_check_attribute_count(changes, 3); _("Check that ignoreAll is respected"); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); tracker.score = 0; tracker.ignoreAll = true; await addEntry("username", "johndoe123"); await addEntry("favoritecolor", "green"); await removeEntry("name", "John Doe"); tracker.ignoreAll = false; - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); equal(tracker.score, 0); _("Let's stop tracking again."); - tracker.clearChangedIDs(); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.clearChangedIDs(); + await tracker.stop(); await removeEntry("address", "Memory Lane"); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); _("Notifying twice won't do any harm."); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); await removeEntry("email", "john@doe.com"); - do_check_empty(tracker.changedIDs); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); diff --git a/services/sync/tests/unit/test_fxa_node_reassignment.js b/services/sync/tests/unit/test_fxa_node_reassignment.js index 24d2980ac2df..9aa17433493f 100644 --- a/services/sync/tests/unit/test_fxa_node_reassignment.js +++ b/services/sync/tests/unit/test_fxa_node_reassignment.js @@ -252,7 +252,7 @@ add_task(async function test_momentary_401_engine() { "weave:service:sync:finish", Service.storageURL + "rotary"); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); diff --git a/services/sync/tests/unit/test_history_engine.js b/services/sync/tests/unit/test_history_engine.js index 19b50dcf2c24..93d3ab46f7dc 100644 --- a/services/sync/tests/unit/test_history_engine.js +++ b/services/sync/tests/unit/test_history_engine.js @@ -137,7 +137,7 @@ add_task(async function test_history_visit_roundtrip() { let server = await serverForFoo(engine); await SyncTestingInfrastructure(server); - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); let id = "aaaaaaaaaaaa"; let oneHourMS = 60 * 60 * 1000; @@ -192,7 +192,7 @@ add_task(async function test_history_visit_dedupe_old() { let server = await serverForFoo(engine); await SyncTestingInfrastructure(server); - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); await PlacesUtils.history.insert({ url: "https://www.example.com", diff --git a/services/sync/tests/unit/test_history_tracker.js b/services/sync/tests/unit/test_history_tracker.js index 2b6ff6374a5a..66c7a2210701 100644 --- a/services/sync/tests/unit/test_history_tracker.js +++ b/services/sync/tests/unit/test_history_tracker.js @@ -47,23 +47,15 @@ async function verifyTrackedItems(tracked) { JSON.stringify(Array.from(trackedIDs))}`); } -async function startTracking() { - Svc.Obs.notify("weave:engine:start-tracking"); -} - -async function stopTracking() { - Svc.Obs.notify("weave:engine:stop-tracking"); -} - async function resetTracker() { - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); tracker.resetScore(); } async function cleanup() { await PlacesTestUtils.clearHistory(); await resetTracker(); - await stopTracking(); + await tracker.stop(); } add_task(async function test_empty() { @@ -102,7 +94,7 @@ add_task(async function test_start_tracking() { }); _("Tell the tracker to start tracking changes."); - await startTracking(); + tracker.start(); let scorePromise = promiseOneObserver("weave:engine:score:updated"); await addVisit("start_tracking"); await scorePromise; @@ -119,13 +111,13 @@ add_task(async function test_start_tracking() { add_task(async function test_start_tracking_twice() { _("Verifying preconditions."); - await startTracking(); + tracker.start(); await addVisit("start_tracking_twice1"); await verifyTrackedCount(1); Assert.equal(tracker.score, SCORE_INCREMENT_SMALL); _("Notifying twice won't do any harm."); - await startTracking(); + tracker.start(); let scorePromise = promiseOneObserver("weave:engine:score:updated"); await addVisit("start_tracking_twice2"); await scorePromise; @@ -146,7 +138,7 @@ add_task(async function test_track_delete() { let guid = await engine._store.GUIDForUri(uri.spec); await verifyTrackerEmpty(); - await startTracking(); + tracker.start(); let visitRemovedPromise = promiseVisit("removed", uri); let scorePromise = promiseOneObserver("weave:engine:score:updated"); await PlacesUtils.history.remove(uri); @@ -166,7 +158,7 @@ add_task(async function test_dont_track_expiration() { await resetTracker(); await verifyTrackerEmpty(); - await startTracking(); + tracker.start(); let visitRemovedPromise = promiseVisit("removed", uriToRemove); let scorePromise = promiseOneObserver("weave:engine:score:updated"); @@ -191,7 +183,7 @@ add_task(async function test_dont_track_expiration() { add_task(async function test_stop_tracking() { _("Let's stop tracking again."); - await stopTracking(); + await tracker.stop(); await addVisit("stop_tracking"); await verifyTrackerEmpty(); @@ -199,11 +191,11 @@ add_task(async function test_stop_tracking() { }); add_task(async function test_stop_tracking_twice() { - await stopTracking(); + await tracker.stop(); await addVisit("stop_tracking_twice1"); _("Notifying twice won't do any harm."); - await stopTracking(); + await tracker.stop(); await addVisit("stop_tracking_twice2"); await verifyTrackerEmpty(); @@ -211,7 +203,7 @@ add_task(async function test_stop_tracking_twice() { }); add_task(async function test_filter_file_uris() { - await startTracking(); + tracker.start(); let uri = CommonUtils.makeURI("file:///Users/eoger/tps/config.json"); let visitAddedPromise = promiseVisit("added", uri); @@ -223,12 +215,12 @@ add_task(async function test_filter_file_uris() { await visitAddedPromise; await verifyTrackerEmpty(); - await stopTracking(); + await tracker.stop(); await cleanup(); }); add_task(async function test_filter_hidden() { - await startTracking(); + tracker.start(); _("Add visit; should be hidden by the redirect"); let hiddenURI = await addVisit("hidden"); diff --git a/services/sync/tests/unit/test_hmac_error.js b/services/sync/tests/unit/test_hmac_error.js index b045c26a7019..fdea949f2f42 100644 --- a/services/sync/tests/unit/test_hmac_error.js +++ b/services/sync/tests/unit/test_hmac_error.js @@ -27,7 +27,7 @@ async function shared_setup() { engine.lastSync = 123; // Needs to be non-zero so that tracker is queried. engine._store.items = {flying: "LNER Class A3 4472", scotsman: "Flying Scotsman"}; - tracker.addChangedID("scotsman", 0); + await tracker.addChangedID("scotsman", 0); Assert.equal(1, Service.engineManager.getEnabled().length); let engines = {rotary: {version: engine.version, @@ -92,7 +92,7 @@ add_task(async function hmac_error_during_404() { // Two rotary items, one client record... no errors. Assert.equal(hmacErrorCount, 0); } finally { - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); @@ -221,7 +221,7 @@ add_task(async function hmac_error_during_node_reassignment() { Svc.Obs.remove("weave:service:sync:error", obs); (async () => { - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); diff --git a/services/sync/tests/unit/test_node_reassignment.js b/services/sync/tests/unit/test_node_reassignment.js index 77fe08610d70..caaed668d079 100644 --- a/services/sync/tests/unit/test_node_reassignment.js +++ b/services/sync/tests/unit/test_node_reassignment.js @@ -177,7 +177,7 @@ add_task(async function test_momentary_401_engine() { "weave:service:sync:finish", Service.storageURL + "rotary"); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); @@ -490,6 +490,6 @@ add_task(async function test_loop_avoidance_engine() { await Service.sync(); await deferred.promise; - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); diff --git a/services/sync/tests/unit/test_password_engine.js b/services/sync/tests/unit/test_password_engine.js index da5dced9e720..d36adea9e6c6 100644 --- a/services/sync/tests/unit/test_password_engine.js +++ b/services/sync/tests/unit/test_password_engine.js @@ -9,7 +9,7 @@ const PropertyBag = Components.Constructor( "@mozilla.org/hash-property-bag;1", Ci.nsIWritablePropertyBag); async function cleanup(engine, server) { - Svc.Obs.notify("weave:engine:stop-tracking"); + await engine._tracker.stop(); await engine.wipeClient(); Svc.Prefs.resetBranch(""); Service.recordManager.clearCache(); @@ -34,7 +34,7 @@ add_task(async function test_ignored_fields() { null, "username", "password", "", "")); login.QueryInterface(Ci.nsILoginMetaInfo); // For `guid`. - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let nonSyncableProps = new PropertyBag(); @@ -67,7 +67,7 @@ add_task(async function test_ignored_sync_credentials() { enableValidationPrefs(); - Svc.Obs.notify("weave:engine:start-tracking"); + engine._tracker.start(); try { let login = Services.logins.addLogin(new LoginInfo(FXA_PWDMGR_HOST, null, @@ -155,7 +155,7 @@ add_task(async function test_password_engine() { collection.insert(oldLogin.guid, encryptPayload(rec.cleartext)); } - Svc.Obs.notify("weave:engine:start-tracking"); + await engine._tracker.stop(); try { await sync_engine_and_validate_telem(engine, false); diff --git a/services/sync/tests/unit/test_password_tracker.js b/services/sync/tests/unit/test_password_tracker.js index b966104e3ef7..89a55caea61d 100644 --- a/services/sync/tests/unit/test_password_tracker.js +++ b/services/sync/tests/unit/test_password_tracker.js @@ -25,9 +25,10 @@ add_task(async function test_tracking() { let recordNum = 0; _("Verify we've got an empty tracker to work with."); - do_check_empty(tracker.changedIDs); + let changes = await tracker.getChangedIDs(); + do_check_empty(changes); - function createPassword() { + async function createPassword() { _("RECORD NUM: " + recordNum); let record = {id: "GUID" + recordNum, hostname: "http://foo.bar.com", @@ -39,62 +40,70 @@ add_task(async function test_tracking() { recordNum++; let login = store._nsLoginInfoFromRecord(record); Services.logins.addLogin(login); + await tracker.asyncObserver.promiseObserversComplete(); } try { _("Create a password record. Won't show because we haven't started tracking yet"); - createPassword(); - do_check_empty(tracker.changedIDs); + await createPassword(); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); Assert.equal(tracker.score, 0); _("Tell the tracker to start tracking changes."); - Svc.Obs.notify("weave:engine:start-tracking"); - createPassword(); - do_check_attribute_count(tracker.changedIDs, 1); + tracker.start(); + await createPassword(); + changes = await tracker.getChangedIDs(); + do_check_attribute_count(changes, 1); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); - _("Notifying twice won't do any harm."); - Svc.Obs.notify("weave:engine:start-tracking"); - createPassword(); - do_check_attribute_count(tracker.changedIDs, 2); + _("Starting twice won't do any harm."); + tracker.start(); + await createPassword(); + changes = await tracker.getChangedIDs(); + do_check_attribute_count(changes, 2); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2); _("Let's stop tracking again."); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); tracker.resetScore(); - Svc.Obs.notify("weave:engine:stop-tracking"); - createPassword(); - do_check_empty(tracker.changedIDs); + await tracker.stop(); + await createPassword(); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); Assert.equal(tracker.score, 0); - _("Notifying twice won't do any harm."); - Svc.Obs.notify("weave:engine:stop-tracking"); - createPassword(); - do_check_empty(tracker.changedIDs); + _("Stopping twice won't do any harm."); + await tracker.stop(); + await createPassword(); + changes = await tracker.getChangedIDs(); + do_check_empty(changes); Assert.equal(tracker.score, 0); } finally { _("Clean up."); await store.wipe(); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); tracker.resetScore(); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); } }); add_task(async function test_onWipe() { _("Verify we've got an empty tracker to work with."); - do_check_empty(tracker.changedIDs); + const changes = await tracker.getChangedIDs(); + do_check_empty(changes); Assert.equal(tracker.score, 0); try { _("A store wipe should increment the score"); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); await store.wipe(); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); } finally { tracker.resetScore(); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); } }); diff --git a/services/sync/tests/unit/test_prefs_tracker.js b/services/sync/tests/unit/test_prefs_tracker.js index 7a4a964f8cb0..330266da4771 100644 --- a/services/sync/tests/unit/test_prefs_tracker.js +++ b/services/sync/tests/unit/test_prefs_tracker.js @@ -49,40 +49,45 @@ add_task(async function run_test() { Assert.equal(tracker.score, 0); _("Tell the tracker to start tracking changes."); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); prefs.set("testing.int", 23); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE); Assert.equal(tracker.modified, true); _("Clearing changed IDs reset modified status."); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); Assert.equal(tracker.modified, false); _("Resetting a pref ups the score, too."); prefs.reset("testing.int"); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 2); Assert.equal(tracker.modified, true); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); _("So does changing a pref sync pref."); Svc.Prefs.set("prefs.sync.testing.int", false); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3); Assert.equal(tracker.modified, true); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); _("Now that the pref sync pref has been flipped, changes to it won't be picked up."); prefs.set("testing.int", 42); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3); Assert.equal(tracker.modified, false); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); _("Changing some other random pref won't do anything."); prefs.set("testing.other", "blergh"); + await tracker.asyncObserver.promiseObserversComplete(); Assert.equal(tracker.score, SCORE_INCREMENT_XLARGE * 3); Assert.equal(tracker.modified, false); } finally { - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); prefs.resetBranch(""); } }); diff --git a/services/sync/tests/unit/test_score_triggers.js b/services/sync/tests/unit/test_score_triggers.js index 856bc3f52a54..d76b188bcec9 100644 --- a/services/sync/tests/unit/test_score_triggers.js +++ b/services/sync/tests/unit/test_score_triggers.js @@ -64,7 +64,7 @@ add_task(async function test_tracker_score_updated() { } finally { Svc.Obs.remove("weave:engine:score:updated", onScoreUpdated); tracker.resetScore(); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); } }); @@ -86,7 +86,7 @@ add_task(async function test_sync_triggered() { await Service.startOver(); await promiseStopServer(server); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); @@ -113,7 +113,7 @@ add_task(async function test_clients_engine_sync_triggered() { await Service.startOver(); await promiseStopServer(server); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); @@ -147,6 +147,6 @@ add_task(async function test_incorrect_credentials_sync_not_triggered() { await Service.startOver(); await promiseStopServer(server); - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); await Service.engineManager.unregister(engine); }); diff --git a/services/sync/tests/unit/test_syncengine_sync.js b/services/sync/tests/unit/test_syncengine_sync.js index a91ddb284777..36f87108edcf 100644 --- a/services/sync/tests/unit/test_syncengine_sync.js +++ b/services/sync/tests/unit/test_syncengine_sync.js @@ -19,7 +19,7 @@ async function clean(engine) { Svc.Prefs.resetBranch(""); Svc.Prefs.set("log.logger.engine.rotary", "Trace"); Service.recordManager.clearCache(); - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); await engine.finalize(); } @@ -98,7 +98,8 @@ add_task(async function test_syncStartup_emptyOrOutdatedGlobalsResetsSync() { try { // Confirm initial environment - Assert.equal(engine._tracker.changedIDs.rekolok, undefined); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.rekolok, undefined); let metaGlobal = await Service.recordManager.get(engine.metaURL); Assert.equal(metaGlobal.payload.engines, undefined); Assert.ok(!!collection.payload("flying")); @@ -173,7 +174,8 @@ add_task(async function test_syncStartup_syncIDMismatchResetsClient() { // Confirm initial environment Assert.equal(engine.syncID, "fake-guid-00"); - Assert.equal(engine._tracker.changedIDs.rekolok, undefined); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.rekolok, undefined); engine.lastSync = Date.now() / 1000; engine.lastSyncLocal = Date.now(); @@ -331,9 +333,9 @@ add_task(async function test_processIncoming_reconcile() { long_original: "Long Original Entry", nukeme: "Nuke me!"}; // Make this record 1 min old, thus older than the one on the server - engine._tracker.addChangedID("newerserver", Date.now() / 1000 - 60); + await engine._tracker.addChangedID("newerserver", Date.now() / 1000 - 60); // This record has been changed 2 mins later than the one on the server - engine._tracker.addChangedID("olderidentical", Date.now() / 1000); + await engine._tracker.addChangedID("olderidentical", Date.now() / 1000); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); @@ -348,7 +350,8 @@ add_task(async function test_processIncoming_reconcile() { Assert.equal(engine._store.items.olderidentical, "Older but identical"); Assert.equal(engine._store.items.updateclient, "Got data?"); Assert.equal(engine._store.items.nukeme, "Nuke me!"); - Assert.ok(engine._tracker.changedIDs.olderidentical > 0); + let changes = await engine._tracker.getChangedIDs(); + Assert.ok(changes.olderidentical > 0); await engine._syncStartup(); await engine._processIncoming(); @@ -366,7 +369,8 @@ add_task(async function test_processIncoming_reconcile() { // The data for 'olderidentical' is identical on the server, so // it's no longer marked as changed anymore. Assert.equal(engine._store.items.olderidentical, "Older but identical"); - Assert.equal(engine._tracker.changedIDs.olderidentical, undefined); + changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.olderidentical, undefined); // Updated with server data. Assert.equal(engine._store.items.updateclient, "Get this!"); @@ -460,7 +464,7 @@ add_task(async function test_processIncoming_reconcile_locally_deleted_dupe_new( // Simulate a locally-deleted item. engine._store.items = {}; - engine._tracker.addChangedID("DUPE_LOCAL", now + 3); + await engine._tracker.addChangedID("DUPE_LOCAL", now + 3); Assert.equal(false, (await engine._store.itemExists("DUPE_LOCAL"))); Assert.equal(false, (await engine._store.itemExists("DUPE_INCOMING"))); Assert.equal("DUPE_LOCAL", (await engine._findDupe({id: "DUPE_INCOMING"}))); @@ -499,7 +503,7 @@ add_task(async function test_processIncoming_reconcile_locally_deleted_dupe_old( // Simulate a locally-deleted item. engine._store.items = {}; - engine._tracker.addChangedID("DUPE_LOCAL", now + 1); + await engine._tracker.addChangedID("DUPE_LOCAL", now + 1); Assert.equal(false, (await engine._store.itemExists("DUPE_LOCAL"))); Assert.equal(false, (await engine._store.itemExists("DUPE_INCOMING"))); Assert.equal("DUPE_LOCAL", (await engine._findDupe({id: "DUPE_INCOMING"}))); @@ -535,7 +539,7 @@ add_task(async function test_processIncoming_reconcile_changed_dupe() { server.insertWBO(user, "rotary", wbo); await engine._store.create({id: "DUPE_LOCAL", denomination: "local"}); - engine._tracker.addChangedID("DUPE_LOCAL", now + 3); + await engine._tracker.addChangedID("DUPE_LOCAL", now + 3); Assert.ok((await engine._store.itemExists("DUPE_LOCAL"))); Assert.equal("DUPE_LOCAL", (await engine._findDupe({id: "DUPE_INCOMING"}))); @@ -574,7 +578,7 @@ add_task(async function test_processIncoming_reconcile_changed_dupe_new() { server.insertWBO(user, "rotary", wbo); await engine._store.create({id: "DUPE_LOCAL", denomination: "local"}); - engine._tracker.addChangedID("DUPE_LOCAL", now + 1); + await engine._tracker.addChangedID("DUPE_LOCAL", now + 1); Assert.ok((await engine._store.itemExists("DUPE_LOCAL"))); Assert.equal("DUPE_LOCAL", (await engine._findDupe({id: "DUPE_INCOMING"}))); @@ -1056,7 +1060,7 @@ add_task(async function test_uploadOutgoing_toEmptyServer() { engine._store.items = {flying: "LNER Class A3 4472", scotsman: "Flying Scotsman"}; // Mark one of these records as changed - engine._tracker.addChangedID("scotsman", 0); + await engine._tracker.addChangedID("scotsman", 0); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); @@ -1082,7 +1086,8 @@ add_task(async function test_uploadOutgoing_toEmptyServer() { Assert.ok(!!collection.payload("scotsman")); Assert.equal(JSON.parse(collection.wbo("scotsman").data.ciphertext).id, "scotsman"); - Assert.equal(engine._tracker.changedIDs.scotsman, undefined); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.scotsman, undefined); // The 'flying' record wasn't marked so it wasn't uploaded Assert.equal(collection.payload("flying"), undefined); @@ -1112,8 +1117,8 @@ async function test_uploadOutgoing_max_record_payload_bytes(allowSkippedRecord) engine.lastSync = 1; engine._store.items = { flying: "a".repeat(1024 * 1024), scotsman: "abcd" }; - engine._tracker.addChangedID("flying", 1000); - engine._tracker.addChangedID("scotsman", 1000); + await engine._tracker.addChangedID("flying", 1000); + await engine._tracker.addChangedID("scotsman", 1000); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); @@ -1138,7 +1143,8 @@ async function test_uploadOutgoing_max_record_payload_bytes(allowSkippedRecord) // Check we uploaded the other record to the server Assert.ok(collection.payload("scotsman")); // And that we won't try to upload the huge record next time. - Assert.equal(engine._tracker.changedIDs.flying, undefined); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, undefined); } catch (e) { if (allowSkippedRecord) { @@ -1148,7 +1154,8 @@ async function test_uploadOutgoing_max_record_payload_bytes(allowSkippedRecord) await engine.trackRemainingChanges(); // Check that we will try to upload the huge record next time - Assert.equal(engine._tracker.changedIDs.flying, 1000); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, 1000); } finally { // Check we didn't upload the oversized record to the server Assert.equal(collection.payload("flying"), undefined); @@ -1190,9 +1197,9 @@ add_task(async function test_uploadOutgoing_failed() { const FLYING_CHANGED = 12345; const SCOTSMAN_CHANGED = 23456; const PEPPERCORN_CHANGED = 34567; - engine._tracker.addChangedID("flying", FLYING_CHANGED); - engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); - engine._tracker.addChangedID("peppercorn", PEPPERCORN_CHANGED); + await engine._tracker.addChangedID("flying", FLYING_CHANGED); + await engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); + await engine._tracker.addChangedID("peppercorn", PEPPERCORN_CHANGED); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); @@ -1204,9 +1211,10 @@ add_task(async function test_uploadOutgoing_failed() { // Confirm initial environment Assert.equal(engine.lastSyncLocal, 0); Assert.equal(collection.payload("flying"), undefined); - Assert.equal(engine._tracker.changedIDs.flying, FLYING_CHANGED); - Assert.equal(engine._tracker.changedIDs.scotsman, SCOTSMAN_CHANGED); - Assert.equal(engine._tracker.changedIDs.peppercorn, PEPPERCORN_CHANGED); + let changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, FLYING_CHANGED); + Assert.equal(changes.scotsman, SCOTSMAN_CHANGED); + Assert.equal(changes.peppercorn, PEPPERCORN_CHANGED); engine.enabled = true; await sync_engine_and_validate_telem(engine, true); @@ -1216,12 +1224,13 @@ add_task(async function test_uploadOutgoing_failed() { // Ensure the 'flying' record has been uploaded and is no longer marked. Assert.ok(!!collection.payload("flying")); - Assert.equal(engine._tracker.changedIDs.flying, undefined); + changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, undefined); // The 'scotsman' and 'peppercorn' records couldn't be uploaded so // they weren't cleared from the tracker. - Assert.equal(engine._tracker.changedIDs.scotsman, SCOTSMAN_CHANGED); - Assert.equal(engine._tracker.changedIDs.peppercorn, PEPPERCORN_CHANGED); + Assert.equal(changes.scotsman, SCOTSMAN_CHANGED); + Assert.equal(changes.peppercorn, PEPPERCORN_CHANGED); } finally { await promiseClean(engine, server); @@ -1255,8 +1264,8 @@ async function createRecordFailTelemetry(allowSkippedRecord) { // Mark these records as changed const FLYING_CHANGED = 12345; const SCOTSMAN_CHANGED = 23456; - engine._tracker.addChangedID("flying", FLYING_CHANGED); - engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); + await engine._tracker.addChangedID("flying", FLYING_CHANGED); + await engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); @@ -1268,8 +1277,9 @@ async function createRecordFailTelemetry(allowSkippedRecord) { // Confirm initial environment Assert.equal(engine.lastSyncLocal, 0); Assert.equal(collection.payload("flying"), undefined); - Assert.equal(engine._tracker.changedIDs.flying, FLYING_CHANGED); - Assert.equal(engine._tracker.changedIDs.scotsman, SCOTSMAN_CHANGED); + let changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, FLYING_CHANGED); + Assert.equal(changes.scotsman, SCOTSMAN_CHANGED); engine.enabled = true; ping = await sync_engine_and_validate_telem(engine, true, onErrorPing => { @@ -1282,7 +1292,8 @@ async function createRecordFailTelemetry(allowSkippedRecord) { // Ensure the 'flying' record has been uploaded and is no longer marked. Assert.ok(!!collection.payload("flying")); - Assert.equal(engine._tracker.changedIDs.flying, undefined); + changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.flying, undefined); } catch (err) { if (allowSkippedRecord) { do_throw("should not get here"); @@ -1290,7 +1301,8 @@ async function createRecordFailTelemetry(allowSkippedRecord) { // Ensure the 'flying' record has not been uploaded and is still marked Assert.ok(!collection.payload("flying")); - Assert.ok(engine._tracker.changedIDs.flying); + const changes = await engine._tracker.getChangedIDs(); + Assert.ok(changes.flying); } finally { // Local timestamp has been set. Assert.ok(engine.lastSyncLocal > 0); @@ -1301,7 +1313,8 @@ async function createRecordFailTelemetry(allowSkippedRecord) { // In any case, the 'scotsman' record couldn't be created so it wasn't // uploaded nor it was not cleared from the tracker. Assert.ok(!collection.payload("scotsman")); - Assert.equal(engine._tracker.changedIDs.scotsman, SCOTSMAN_CHANGED); + const changes = await engine._tracker.getChangedIDs(); + Assert.equal(changes.scotsman, SCOTSMAN_CHANGED); engine._store.createRecord = oldCreateRecord; await promiseClean(engine, server); @@ -1326,7 +1339,7 @@ add_task(async function test_uploadOutgoing_largeRecords() { let engine = makeRotaryEngine(); engine.allowSkippedRecord = false; engine._store.items["large-item"] = "Y".repeat(Service.getMaxRecordPayloadSize() * 2); - engine._tracker.addChangedID("large-item", 0); + await engine._tracker.addChangedID("large-item", 0); collection.insert("large-item"); @@ -1498,7 +1511,7 @@ add_task(async function test_sync_partialUpload() { for (let i = 0; i < 234; i++) { let id = "record-no-" + i; engine._store.items[id] = "Record No. " + i; - engine._tracker.addChangedID(id, i); + await engine._tracker.addChangedID(id, i); // Let two items in the first upload batch fail. if ((i != 23) && (i != 42)) { collection.insert(id); @@ -1525,6 +1538,7 @@ add_task(async function test_sync_partialUpload() { // The timestamp has been updated. Assert.ok(engine.lastSyncLocal > 456); + const changes = await engine._tracker.getChangedIDs(); for (let i = 0; i < 234; i++) { let id = "record-no-" + i; // Ensure failed records are back in the tracker: @@ -1532,9 +1546,9 @@ add_task(async function test_sync_partialUpload() { // * records after the third batch and higher couldn't be uploaded because // we failed hard on the 3rd upload. if ((i == 23) || (i == 42) || (i >= 200)) - Assert.equal(engine._tracker.changedIDs[id], i); + Assert.equal(changes[id], i); else - Assert.equal(false, id in engine._tracker.changedIDs); + Assert.equal(false, id in changes); } } finally { diff --git a/services/sync/tests/unit/test_tab_tracker.js b/services/sync/tests/unit/test_tab_tracker.js index 7c6a5b29376d..82ad565f4731 100644 --- a/services/sync/tests/unit/test_tab_tracker.js +++ b/services/sync/tests/unit/test_tab_tracker.js @@ -64,7 +64,7 @@ add_task(async function run_test() { _("Test listeners are registered on windows"); logs = fakeSvcWinMediator(); - Svc.Obs.notify("weave:engine:start-tracking"); + tracker.start(); Assert.equal(logs.length, 2); for (let log of logs) { Assert.equal(log.addTopics.length, 5); @@ -80,7 +80,7 @@ add_task(async function run_test() { _("Test listeners are unregistered on windows"); logs = fakeSvcWinMediator(); - Svc.Obs.notify("weave:engine:stop-tracking"); + await tracker.stop(); Assert.equal(logs.length, 2); for (let log of logs) { Assert.equal(log.addTopics.length, 0); @@ -97,7 +97,7 @@ add_task(async function run_test() { _("Test tab listener"); for (let evttype of ["TabOpen", "TabClose", "TabSelect"]) { // Pretend we just synced. - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); Assert.ok(!tracker.modified); // Send a fake tab event @@ -108,7 +108,7 @@ add_task(async function run_test() { } // Pretend we just synced. - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); Assert.ok(!tracker.modified); tracker.onTab({type: "pageshow", originalTarget: "pageshow"}); @@ -116,7 +116,7 @@ add_task(async function run_test() { [clientsEngine.localID])); // Pretend we just synced and saw some progress listeners. - tracker.clearChangedIDs(); + await tracker.clearChangedIDs(); Assert.ok(!tracker.modified); tracker.onLocationChange({ isTopLevel: false }, undefined, undefined, 0); Assert.ok(!tracker.modified, "non-toplevel request didn't flag as modified"); diff --git a/services/sync/tests/unit/test_telemetry.js b/services/sync/tests/unit/test_telemetry.js index 491e9065e2df..5acc7f73562a 100644 --- a/services/sync/tests/unit/test_telemetry.js +++ b/services/sync/tests/unit/test_telemetry.js @@ -58,7 +58,7 @@ function BogusEngine(service) { BogusEngine.prototype = Object.create(SteamEngine.prototype); async function cleanAndGo(engine, server) { - engine._tracker.clearChangedIDs(); + await engine._tracker.clearChangedIDs(); Svc.Prefs.resetBranch(""); syncTestLogging(); Service.recordManager.clearCache(); @@ -220,16 +220,17 @@ add_task(async function test_upload_failed() { const FLYING_CHANGED = 12345; const SCOTSMAN_CHANGED = 23456; const PEPPERCORN_CHANGED = 34567; - engine._tracker.addChangedID("flying", FLYING_CHANGED); - engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); - engine._tracker.addChangedID("peppercorn", PEPPERCORN_CHANGED); + await engine._tracker.addChangedID("flying", FLYING_CHANGED); + await engine._tracker.addChangedID("scotsman", SCOTSMAN_CHANGED); + await engine._tracker.addChangedID("peppercorn", PEPPERCORN_CHANGED); let meta_global = Service.recordManager.set(engine.metaURL, new WBORecord(engine.metaURL)); meta_global.payload.engines = { rotary: { version: engine.version, syncID: engine.syncID } }; try { + let changes = await engine._tracker.getChangedIDs(); _(`test_upload_failed: Rotary tracker contents at first sync: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); engine.enabled = true; let ping = await sync_engine_and_validate_telem(engine, true); ok(!!ping); @@ -239,8 +240,9 @@ add_task(async function test_upload_failed() { engine.lastSync = 123; engine.lastSyncLocal = 456; + changes = await engine._tracker.getChangedIDs(); _(`test_upload_failed: Rotary tracker contents at second sync: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); ping = await sync_engine_and_validate_telem(engine, true); ok(!!ping); equal(ping.engines.length, 1); @@ -270,7 +272,7 @@ add_task(async function test_sync_partialUpload() { for (let i = 0; i < 234; i++) { let id = "record-no-" + i; engine._store.items[id] = "Record No. " + i; - engine._tracker.addChangedID(id, i); + await engine._tracker.addChangedID(id, i); // Let two items in the first upload batch fail. if (i != 23 && i != 42) { collection.insert(id); @@ -283,8 +285,9 @@ add_task(async function test_sync_partialUpload() { syncID: engine.syncID}}; try { + let changes = await engine._tracker.getChangedIDs(); _(`test_sync_partialUpload: Rotary tracker contents at first sync: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); engine.enabled = true; let ping = await sync_engine_and_validate_telem(engine, true); @@ -299,15 +302,16 @@ add_task(async function test_sync_partialUpload() { collection.post = function() { throw new Error("Failure"); }; engine._store.items["record-no-1000"] = "Record No. 1000"; - engine._tracker.addChangedID("record-no-1000", 1000); + await engine._tracker.addChangedID("record-no-1000", 1000); collection.insert("record-no-1000", 1000); engine.lastSync = 123; engine.lastSyncLocal = 456; ping = null; + changes = await engine._tracker.getChangedIDs(); _(`test_sync_partialUpload: Rotary tracker contents at second sync: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); try { // should throw await sync_engine_and_validate_telem(engine, true, errPing => ping = errPing); @@ -348,8 +352,9 @@ add_task(async function test_generic_engine_fail() { engine._errToThrow = e; try { + const changes = await engine._tracker.getChangedIDs(); _(`test_generic_engine_fail: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await sync_and_validate_telem(true); equal(ping.status.service, SYNC_FAILED_PARTIAL); deepEqual(ping.engines.find(err => err.name === "steam").failureReason, { @@ -412,8 +417,9 @@ add_task(async function test_engine_fail_ioerror() { ok(engine._errToThrow, "expecting exception"); try { + const changes = await engine._tracker.getChangedIDs(); _(`test_engine_fail_ioerror: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await sync_and_validate_telem(true); equal(ping.status.service, SYNC_FAILED_PARTIAL); let failureReason = ping.engines.find(e => e.name === "steam").failureReason; @@ -438,8 +444,9 @@ add_task(async function test_clean_urls() { engine._errToThrow = new TypeError("http://www.google .com is not a valid URL."); try { + const changes = await engine._tracker.getChangedIDs(); _(`test_clean_urls: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await sync_and_validate_telem(true); equal(ping.status.service, SYNC_FAILED_PARTIAL); let failureReason = ping.engines.find(e => e.name === "steam").failureReason; @@ -472,8 +479,9 @@ add_task(async function test_initial_sync_engines() { )); await SyncTestingInfrastructure(server); try { + const changes = await engine._tracker.getChangedIDs(); _(`test_initial_sync_engines: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await wait_for_ping(() => Service.sync(), true); equal(ping.engines.find(e => e.name === "clients").outgoing[0].sent, 1); @@ -506,8 +514,9 @@ add_task(async function test_nserror() { await SyncTestingInfrastructure(server); engine._errToThrow = Components.Exception("NS_ERROR_UNKNOWN_HOST", Cr.NS_ERROR_UNKNOWN_HOST); try { + const changes = await engine._tracker.getChangedIDs(); _(`test_nserror: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await sync_and_validate_telem(true); deepEqual(ping.status, { service: SYNC_FAILED_PARTIAL, @@ -536,8 +545,9 @@ add_task(async function test_sync_why() { engine._errToThrow = e; try { + const changes = await engine._tracker.getChangedIDs(); _(`test_generic_engine_fail: Steam tracker contents: ${ - JSON.stringify(engine._tracker.changedIDs)}`); + JSON.stringify(changes)}`); let ping = await wait_for_ping(() => Service.sync({why: "user"}), true, false); _(JSON.stringify(ping)); equal(ping.why, "user"); diff --git a/services/sync/tests/unit/test_tracker_addChanged.js b/services/sync/tests/unit/test_tracker_addChanged.js index 428880c43f9b..4863bd14ca7d 100644 --- a/services/sync/tests/unit/test_tracker_addChanged.js +++ b/services/sync/tests/unit/test_tracker_addChanged.js @@ -12,23 +12,28 @@ add_task(async function test_tracker_basics() { let id = "the_id!"; _("Make sure nothing exists yet.."); - Assert.equal(tracker.changedIDs[id], null); + let changes = await tracker.getChangedIDs(); + Assert.equal(changes[id], null); _("Make sure adding of time 0 works"); - tracker.addChangedID(id, 0); - Assert.equal(tracker.changedIDs[id], 0); + await tracker.addChangedID(id, 0); + changes = await tracker.getChangedIDs(); + Assert.equal(changes[id], 0); _("A newer time will replace the old 0"); - tracker.addChangedID(id, 10); - Assert.equal(tracker.changedIDs[id], 10); + await tracker.addChangedID(id, 10); + changes = await tracker.getChangedIDs(); + Assert.equal(changes[id], 10); _("An older time will not replace the newer 10"); - tracker.addChangedID(id, 5); - Assert.equal(tracker.changedIDs[id], 10); + await tracker.addChangedID(id, 5); + changes = await tracker.getChangedIDs(); + Assert.equal(changes[id], 10); _("Adding without time defaults to current time"); - tracker.addChangedID(id); - Assert.ok(tracker.changedIDs[id] > 10); + await tracker.addChangedID(id); + changes = await tracker.getChangedIDs(); + Assert.ok(changes[id] > 10); }); add_task(async function test_tracker_persistence() { @@ -44,12 +49,13 @@ add_task(async function test_tracker_persistence() { }; }); - tracker.addChangedID(id, 5); + await tracker.addChangedID(id, 5); await promiseSave; _("IDs saved."); - Assert.equal(5, tracker.changedIDs[id]); + const changes = await tracker.getChangedIDs(); + Assert.equal(5, changes[id]); let json = await Utils.jsonLoad("changes/tracker", tracker); Assert.equal(5, json[id]); diff --git a/services/sync/tps/extensions/tps/resource/tps.jsm b/services/sync/tps/extensions/tps/resource/tps.jsm index 29f2b1c15c5e..5f4ff1adfe8b 100644 --- a/services/sync/tps/extensions/tps/resource/tps.jsm +++ b/services/sync/tps/extensions/tps/resource/tps.jsm @@ -98,8 +98,8 @@ const OBSERVER_TOPICS = ["fxaccounts:onlogin", "private-browsing", "profile-before-change", "sessionstore-windows-restored", - "weave:engine:start-tracking", - "weave:engine:stop-tracking", + "weave:service:tracking-started", + "weave:service:tracking-stopped", "weave:service:login:error", "weave:service:setup-complete", "weave:service:sync:finish", @@ -247,11 +247,11 @@ var TPS = { this._syncActive = true; break; - case "weave:engine:start-tracking": + case "weave:service:tracking-started": this._isTracking = true; break; - case "weave:engine:stop-tracking": + case "weave:service:tracking-stopped": this._isTracking = false; break; } @@ -1099,7 +1099,7 @@ var TPS = { */ async waitForTracking() { if (!this._isTracking) { - await this.waitForEvent("weave:engine:start-tracking"); + await this.waitForEvent("weave:service:tracking-started"); } }, diff --git a/toolkit/components/places/tests/unifiedcomplete/test_remote_tab_matches.js b/toolkit/components/places/tests/unifiedcomplete/test_remote_tab_matches.js index 6517f1c6a46e..d97ec8897fb1 100644 --- a/toolkit/components/places/tests/unifiedcomplete/test_remote_tab_matches.js +++ b/toolkit/components/places/tests/unifiedcomplete/test_remote_tab_matches.js @@ -17,6 +17,7 @@ function MockTabsEngine() { MockTabsEngine.prototype = { name: "tabs", + startTracking() {}, getAllClients() { return this.clients; },