Bug 633062 p7 - Make most of the addon reconciler async. r?markh
Except observers, which will be handled in the last commit of this series.
MozReview-Commit-ID: IvwyleXBcNH
--- a/services/sync/modules/addonsreconciler.js
+++ b/services/sync/modules/addonsreconciler.js
@@ -324,17 +324,17 @@ AddonsReconciler.prototype = {
let installs;
let addons = await AddonManager.getAllAddons();
let ids = {};
for (let addon of addons) {
ids[addon.id] = true;
- this.rectifyStateFromAddon(addon);
+ await this.rectifyStateFromAddon(addon);
}
// Look for locally-defined add-ons that no longer exist and update their
// record.
for (let [id, addon] of Object.entries(this._addons)) {
if (id in ids) {
continue;
}
@@ -360,17 +360,17 @@ AddonsReconciler.prototype = {
if (installFound) {
continue;
}
if (addon.installed) {
addon.installed = false;
this._log.debug("Adding change because add-on not present in " +
"Add-on Manager: " + id);
- this._addChange(new Date(), CHANGE_UNINSTALLED, addon);
+ await this._addChange(new Date(), CHANGE_UNINSTALLED, addon);
}
}
// See note for _shouldPersist.
if (this._shouldPersist) {
await this.saveState();
}
},
@@ -382,17 +382,17 @@ AddonsReconciler.prototype = {
* apply changes to the local state to reflect it."
*
* This function could result in change listeners being called if the local
* state differs from the passed add-on's state.
*
* @param addon
* Addon instance being updated.
*/
- rectifyStateFromAddon(addon) {
+ async rectifyStateFromAddon(addon) {
this._log.debug(`Rectifying state for addon ${addon.name} (version=${addon.version}, id=${addon.id})`);
let id = addon.id;
let enabled = !addon.userDisabled;
let guid = addon.syncGUID;
let now = new Date();
if (!(id in this._addons)) {
@@ -405,17 +405,17 @@ AddonsReconciler.prototype = {
type: addon.type,
scope: addon.scope,
foreignInstall: addon.foreignInstall,
isSyncable: addon.isSyncable,
};
this._addons[id] = record;
this._log.debug("Adding change because add-on not present locally: " +
id);
- this._addChange(now, CHANGE_INSTALLED, record);
+ await this._addChange(now, CHANGE_INSTALLED, record);
return;
}
let record = this._addons[id];
record.isSyncable = addon.isSyncable;
if (!record.installed) {
// It is possible the record is marked as uninstalled because an
@@ -426,17 +426,17 @@ AddonsReconciler.prototype = {
}
}
if (record.enabled != enabled) {
record.enabled = enabled;
record.modified = now;
let change = enabled ? CHANGE_ENABLED : CHANGE_DISABLED;
this._log.debug("Adding change because enabled state changed: " + id);
- this._addChange(new Date(), change, record);
+ await this._addChange(new Date(), change, record);
}
if (record.guid != guid) {
record.guid = guid;
// We don't record a change because the Sync engine rectifies this on its
// own. This is tightly coupled with Sync. If this code is ever lifted
// outside of Sync, this exception should likely be removed.
}
@@ -447,25 +447,25 @@ AddonsReconciler.prototype = {
*
* @param date
* Date at which the change occurred.
* @param change
* The type of the change. A CHANGE_* constant.
* @param state
* The new state of the add-on. From this.addons.
*/
- _addChange: function _addChange(date, change, state) {
+ async _addChange(date, change, state) {
this._log.info("Change recorded for " + state.id);
this._changes.push([date, change, state.id]);
for (let listener of this._listeners) {
try {
- listener.changeListener(date, change, state);
+ await listener.changeListener(date, change, state);
} catch (ex) {
- this._log.warn("Exception calling change listener", ex);
+ this._log.error("Exception calling change listener", ex);
}
}
},
/**
* Obtain the set of changes to add-ons since the date passed.
*
* This will return an array of arrays. Each entry in the array has the
@@ -527,17 +527,17 @@ AddonsReconciler.prototype = {
}
return null;
},
/**
* Handler that is invoked as part of the AddonManager listeners.
*/
- _handleListener(action, addon, requiresRestart) {
+ async _handleListener(action, addon, requiresRestart) {
// Since this is called as an observer, we explicitly trap errors and
// log them to ourselves so we don't see errors reported elsewhere.
try {
let id = addon.id;
this._log.debug("Add-on change: " + action + " to " + id);
// We assume that every event for non-restartless add-ons is
// followed by another event and that this follow-up event is the most
@@ -551,69 +551,69 @@ AddonsReconciler.prototype = {
switch (action) {
case "onEnabling":
case "onEnabled":
case "onDisabling":
case "onDisabled":
case "onInstalled":
case "onInstallEnded":
case "onOperationCancelled":
- this.rectifyStateFromAddon(addon);
+ await this.rectifyStateFromAddon(addon);
break;
case "onUninstalling":
case "onUninstalled":
let id = addon.id;
let addons = this.addons;
if (id in addons) {
let now = new Date();
let record = addons[id];
record.installed = false;
record.modified = now;
this._log.debug("Adding change because of uninstall listener: " +
id);
- this._addChange(now, CHANGE_UNINSTALLED, record);
+ await this._addChange(now, CHANGE_UNINSTALLED, record);
}
}
// See note for _shouldPersist.
if (this._shouldPersist) {
- Async.promiseSpinningly(this.saveState());
+ await this.saveState();
}
} catch (ex) {
this._log.warn("Exception", ex);
}
},
// AddonListeners
onEnabling: function onEnabling(addon, requiresRestart) {
- this._handleListener("onEnabling", addon, requiresRestart);
+ Async.promiseSpinningly(this._handleListener("onEnabling", addon, requiresRestart));
},
onEnabled: function onEnabled(addon) {
- this._handleListener("onEnabled", addon);
+ Async.promiseSpinningly(this._handleListener("onEnabled", addon));
},
onDisabling: function onDisabling(addon, requiresRestart) {
- this._handleListener("onDisabling", addon, requiresRestart);
+ Async.promiseSpinningly(this._handleListener("onDisabling", addon, requiresRestart));
},
onDisabled: function onDisabled(addon) {
- this._handleListener("onDisabled", addon);
+ Async.promiseSpinningly(this._handleListener("onDisabled", addon));
},
onInstalling: function onInstalling(addon, requiresRestart) {
- this._handleListener("onInstalling", addon, requiresRestart);
+ Async.promiseSpinningly(this._handleListener("onInstalling", addon, requiresRestart));
},
onInstalled: function onInstalled(addon) {
- this._handleListener("onInstalled", addon);
+ Async.promiseSpinningly(this._handleListener("onInstalled", addon));
},
onUninstalling: function onUninstalling(addon, requiresRestart) {
- this._handleListener("onUninstalling", addon, requiresRestart);
+ Async.promiseSpinningly(this._handleListener("onUninstalling", addon, requiresRestart));
},
onUninstalled: function onUninstalled(addon) {
- this._handleListener("onUninstalled", addon);
+ Async.promiseSpinningly(this._handleListener("onUninstalled", addon));
},
onOperationCancelled: function onOperationCancelled(addon) {
- this._handleListener("onOperationCancelled", addon);
+ Async.promiseSpinningly(this._handleListener("onOperationCancelled", addon));
},
// InstallListeners
onInstallEnded: function onInstallEnded(install, addon) {
- this._handleListener("onInstallEnded", addon);
+ Async.promiseSpinningly(this._handleListener("onInstallEnded", addon));
}
};
--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -43,17 +43,16 @@ ChromeUtils.import("resource://gre/modul
ChromeUtils.import("resource://gre/modules/Preferences.jsm");
ChromeUtils.import("resource://services-sync/addonutils.js");
ChromeUtils.import("resource://services-sync/addonsreconciler.js");
ChromeUtils.import("resource://services-sync/engines.js");
ChromeUtils.import("resource://services-sync/record.js");
ChromeUtils.import("resource://services-sync/util.js");
ChromeUtils.import("resource://services-sync/constants.js");
ChromeUtils.import("resource://services-sync/collection_validator.js");
-ChromeUtils.import("resource://services-common/async.js");
ChromeUtils.defineModuleGetter(this, "AddonManager",
"resource://gre/modules/AddonManager.jsm");
ChromeUtils.defineModuleGetter(this, "AddonRepository",
"resource://gre/modules/addons/AddonRepository.jsm");
this.EXPORTED_SYMBOLS = ["AddonsEngine", "AddonValidator"];
@@ -377,17 +376,17 @@ AddonsStore.prototype = {
// We wouldn't get here if the incoming record was for a deletion. So,
// check for pending uninstall and cancel if necessary.
if (addon.pendingOperations & AddonManager.PENDING_UNINSTALL) {
addon.cancelUninstall();
// We continue with processing because there could be state or ID change.
}
- this.updateUserDisabled(addon, !record.enabled);
+ await this.updateUserDisabled(addon, !record.enabled);
},
/**
* Provide core Store API to determine if a record exists.
*/
async itemExists(guid) {
let addon = this.reconciler.getAddonStateFromSyncGUID(guid);
@@ -646,34 +645,34 @@ AddonsStore.prototype = {
* not catch or handle exceptions thrown by the addon manager. If no action
* is needed it will return immediately.
*
* @param addon
* Addon instance to manipulate.
* @param value
* Boolean to which to set userDisabled on the passed Addon.
*/
- updateUserDisabled(addon, value) {
+ async updateUserDisabled(addon, value) {
if (addon.userDisabled == value) {
return;
}
// A pref allows changes to the enabled flag to be ignored.
if (Svc.Prefs.get("addons.ignoreUserEnabledChanges", false)) {
this._log.info("Ignoring enabled state change due to preference: " +
addon.id);
return;
}
AddonUtils.updateUserDisabled(addon, value);
// updating this flag doesn't send a notification for appDisabled addons,
// meaning the reconciler will not update its state and may resync the
// addon - so explicitly rectify the state (bug 1366994)
if (addon.appDisabled) {
- this.reconciler.rectifyStateFromAddon(addon);
+ await this.reconciler.rectifyStateFromAddon(addon);
}
},
};
/**
* The add-ons tracker keeps track of real-time changes to add-ons.
*
* It hooks up to the reconciler and receives notifications directly from it.
@@ -691,24 +690,24 @@ AddonsTracker.prototype = {
get store() {
return this.engine._store;
},
/**
* This callback is executed whenever the AddonsReconciler sends out a change
* notification. See AddonsReconciler.addChangeListener().
*/
- changeListener: function changeHandler(date, change, addon) {
+ async changeListener(date, change, addon) {
this._log.debug("changeListener invoked: " + change + " " + addon.id);
// Ignore changes that occur during sync.
if (this.ignoreAll) {
return;
}
- if (!Async.promiseSpinningly(this.store.isAddonSyncable(addon))) {
+ if (!(await this.store.isAddonSyncable(addon))) {
this._log.debug("Ignoring change because add-on isn't syncable: " +
addon.id);
return;
}
if (this.addChangedID(addon.guid, date.getTime() / 1000)) {
this.score += SCORE_INCREMENT_XLARGE;
}
--- a/services/sync/tests/unit/test_addons_engine.js
+++ b/services/sync/tests/unit/test_addons_engine.js
@@ -138,17 +138,17 @@ add_task(async function test_get_changed
enabled: true,
installed: true,
modified: new Date(),
type: "UNSUPPORTED",
scope: 0,
foreignInstall: false
};
reconciler.addons.DUMMY = record;
- reconciler._addChange(record.modified, CHANGE_INSTALLED, record);
+ await reconciler._addChange(record.modified, CHANGE_INSTALLED, record);
changes = await engine.getChangedIDs();
_(JSON.stringify(changes));
Assert.equal(0, Object.keys(changes).length);
await resetReconciler();
});
--- a/services/sync/tests/unit/test_addons_store.js
+++ b/services/sync/tests/unit/test_addons_store.js
@@ -90,19 +90,19 @@ function createAndStartHTTPServer(port)
}
return null; /* not hit, but keeps eslint happy! */
}
// A helper function to ensure that the reconciler's current view of the addon
// is the same as the addon itself. If it's not, then the reconciler missed a
// change, and is likely to re-upload the addon next sync because of the change
// it missed.
-function checkReconcilerUpToDate(addon) {
+async function checkReconcilerUpToDate(addon) {
let stateBefore = Object.assign({}, store.reconciler.addons[addon.id]);
- store.reconciler.rectifyStateFromAddon(addon);
+ await store.reconciler.rectifyStateFromAddon(addon);
let stateAfter = store.reconciler.addons[addon.id];
deepEqual(stateBefore, stateAfter);
}
add_task(async function setup() {
await Service.engineManager.register(AddonsEngine);
engine = Service.engineManager.get("addons");
tracker = engine._tracker;
@@ -138,26 +138,26 @@ add_task(async function test_apply_enabl
_("Ensure application of a disable record works as expected.");
let records = [];
records.push(createRecordForThisApp(addon.syncGUID, addon.id, false, false));
let failed = await store.applyIncomingBatch(records);
Assert.equal(0, failed.length);
addon = await AddonManager.getAddonByID(addon.id);
Assert.ok(addon.userDisabled);
- checkReconcilerUpToDate(addon);
+ await checkReconcilerUpToDate(addon);
records = [];
_("Ensure enable record works as expected.");
records.push(createRecordForThisApp(addon.syncGUID, addon.id, true, false));
failed = await store.applyIncomingBatch(records);
Assert.equal(0, failed.length);
addon = await AddonManager.getAddonByID(addon.id);
Assert.ok(!addon.userDisabled);
- checkReconcilerUpToDate(addon);
+ await checkReconcilerUpToDate(addon);
records = [];
_("Ensure enabled state updates don't apply if the ignore pref is set.");
records.push(createRecordForThisApp(addon.syncGUID, addon.id, false, false));
Svc.Prefs.set("addons.ignoreUserEnabledChanges", true);
failed = await store.applyIncomingBatch(records);
Assert.equal(0, failed.length);
addon = await AddonManager.getAddonByID(addon.id);
@@ -180,26 +180,26 @@ add_task(async function test_apply_enabl
store.reconciler.pruneChangesBeforeDate(Date.now() + 10);
store.reconciler._changes = [];
let records = [];
records.push(createRecordForThisApp(addon.syncGUID, addon.id, false, false));
let failed = await store.applyIncomingBatch(records);
Assert.equal(0, failed.length);
addon = await AddonManager.getAddonByID(addon.id);
Assert.ok(addon.userDisabled);
- checkReconcilerUpToDate(addon);
+ await checkReconcilerUpToDate(addon);
records = [];
_("Ensure enable record works as expected.");
records.push(createRecordForThisApp(addon.syncGUID, addon.id, true, false));
failed = await store.applyIncomingBatch(records);
Assert.equal(0, failed.length);
addon = await AddonManager.getAddonByID(addon.id);
Assert.ok(!addon.userDisabled);
- checkReconcilerUpToDate(addon);
+ await checkReconcilerUpToDate(addon);
records = [];
await uninstallAddon(addon);
});
add_task(async function test_ignore_different_appid() {
_("Ensure that incoming records with a different application ID are ignored.");