diff --git a/toolkit/mozapps/extensions/AddonManager.jsm b/toolkit/mozapps/extensions/AddonManager.jsm index 38eb2098242f..7bef7c567132 100644 --- a/toolkit/mozapps/extensions/AddonManager.jsm +++ b/toolkit/mozapps/extensions/AddonManager.jsm @@ -40,12 +40,16 @@ var PREF_EM_CHECK_COMPATIBILITY; const TOOLKIT_ID = "toolkit@mozilla.org"; -const SHUTDOWN_EVENT = "profile-before-change"; - const VALID_TYPES_REGEXP = /^[\w\-]+$/; -Components.utils.import("resource://gre/modules/Services.jsm"); -Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/AsyncShutdown.jsm"); + +XPCOMUtils.defineLazyModuleGetter(this, "Promise", + "resource://gre/modules/Promise.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository", + "resource://gre/modules/AddonRepository.jsm"); XPCOMUtils.defineLazyGetter(this, "CertUtils", function certUtilsLazyGetter() { let certUtils = {}; @@ -453,8 +457,6 @@ var AddonManagerInternal = { this.recordTimestamp("AMI_startup_begin"); - Services.obs.addObserver(this, SHUTDOWN_EVENT, false); - let appChanged = undefined; let oldAppVersion = null; @@ -547,6 +549,10 @@ var AddonManagerInternal = { } } + // Register our shutdown handler with the AsyncShutdown manager + AsyncShutdown.profileBeforeChange.addBlocker("AddonManager: shutting down providers", + this.shutdown.bind(this)); + // Once we start calling providers we must allow all normal methods to work. gStarted = true; @@ -678,13 +684,58 @@ var AddonManagerInternal = { } }, + /** + * Calls a method on all registered providers, if the provider implements + * the method. The called method is expected to return a promise, and + * callProvidersAsync returns a promise that resolves when every provider + * method has either resolved or rejected. Rejection reasons are logged + * but otherwise ignored. Return values are ignored. Any parameters after the + * method parameter are passed to the provider's method. + * + * @param aMethod + * The method name to call + * @see callProvider + */ + callProvidersAsync: function AMI_callProviders(aMethod, ...aArgs) { + if (!aMethod || typeof aMethod != "string") + throw Components.Exception("aMethod must be a non-empty string", + Cr.NS_ERROR_INVALID_ARG); + + let allProviders = []; + + let providers = this.providers.slice(0); + for (let provider of providers) { + try { + if (aMethod in provider) { + // Resolve a new promise with the result of the method, to handle both + // methods that return values (or nothing) and methods that return promises. + let providerResult = provider[aMethod].apply(provider, aArgs); + let nextPromise = Promise.resolve(providerResult); + // Log and swallow the errors from methods that do return promises. + nextPromise = nextPromise.then( + null, + e => ERROR("Exception calling provider " + aMethod, e)); + allProviders.push(nextPromise); + } + } + catch (e) { + ERROR("Exception calling provider " + aMethod, e); + } + } + // Because we use promise.then to catch and log all errors above, Promise.all() + // will never exit early because of a rejection. + return Promise.all(allProviders); + }, + /** * Shuts down the addon manager and all registered providers, this must clean * up everything in order for automated tests to fake restarts. + * @return Promise{null} that resolves when all providers and dependent modules + * have finished shutting down */ shutdown: function AMI_shutdown() { LOG("shutdown"); - Services.obs.removeObserver(this, SHUTDOWN_EVENT); + // Clean up listeners Services.prefs.removeObserver(PREF_EM_CHECK_COMPATIBILITY, this); Services.prefs.removeObserver(PREF_EM_STRICT_COMPATIBILITY, this); Services.prefs.removeObserver(PREF_EM_CHECK_UPDATE_SECURITY, this); @@ -692,19 +743,32 @@ var AddonManagerInternal = { Services.prefs.removeObserver(PREF_EM_AUTOUPDATE_DEFAULT, this); Services.prefs.removeObserver(PREF_EM_HOTFIX_ID, this); - // Always clean up listeners, but only shutdown providers if they've been - // started. - if (gStarted) - this.callProviders("shutdown"); + // Only shut down providers if they've been started. Shut down + // AddonRepository after providers (if any). + let shuttingDown = null; + if (gStarted) { + shuttingDown = this.callProvidersAsync("shutdown") + .then(null, + err => ERROR("Failure during async provider shutdown", err)) + .then(() => AddonRepository.shutdown()); + } + else { + shuttingDown = AddonRepository.shutdown(); + } - this.managerListeners.splice(0, this.managerListeners.length); - this.installListeners.splice(0, this.installListeners.length); - this.addonListeners.splice(0, this.addonListeners.length); - this.typeListeners.splice(0, this.typeListeners.length); - for (let type in this.startupChanges) - delete this.startupChanges[type]; - gStarted = false; - gStartupComplete = false; + shuttingDown.then(val => LOG("Async provider shutdown done"), + err => ERROR("Failure during AddonRepository shutdown", err)) + .then(() => { + this.managerListeners.splice(0, this.managerListeners.length); + this.installListeners.splice(0, this.installListeners.length); + this.addonListeners.splice(0, this.addonListeners.length); + this.typeListeners.splice(0, this.typeListeners.length); + for (let type in this.startupChanges) + delete this.startupChanges[type]; + gStarted = false; + gStartupComplete = false; + }); + return shuttingDown; }, /** @@ -713,11 +777,6 @@ var AddonManagerInternal = { * @see nsIObserver */ observe: function AMI_observe(aSubject, aTopic, aData) { - if (aTopic == SHUTDOWN_EVENT) { - this.shutdown(); - return; - } - switch (aData) { case PREF_EM_CHECK_COMPATIBILITY: { let oldValue = gCheckCompatibility; @@ -910,7 +969,6 @@ var AddonManagerInternal = { if (this.updateEnabled) { let scope = {}; - Components.utils.import("resource://gre/modules/AddonRepository.jsm", scope); Components.utils.import("resource://gre/modules/LightweightThemeManager.jsm", scope); scope.LightweightThemeManager.updateCurrentTheme(); @@ -921,7 +979,7 @@ var AddonManagerInternal = { // Repopulate repository cache first, to ensure compatibility overrides // are up to date before checking for addon updates. - scope.AddonRepository.backgroundUpdateCheck( + AddonRepository.backgroundUpdateCheck( ids, function BUC_backgroundUpdateCheckCallback() { AddonManagerInternal.updateAddonRepositoryData( function BUC_updateAddonCallback() { @@ -1144,7 +1202,7 @@ var AddonManagerInternal = { * An optional array of extra InstallListeners to also call * @return false if any of the listeners returned false, true otherwise */ - callInstallListeners: function AMI_callInstallListeners(aMethod, + callInstallListeners: function AMI_callInstallListeners(aMethod, aExtraListeners, ...aArgs) { if (!gStarted) throw Components.Exception("AddonManager is not initialized", @@ -1248,7 +1306,7 @@ var AddonManagerInternal = { this.callProviders("updateAddonAppDisabledStates"); }, - + /** * Notifies all providers that the repository has updated its data for * installed add-ons. @@ -1632,7 +1690,7 @@ var AddonManagerInternal = { if (!aListener || typeof aListener != "object") throw Components.Exception("aListener must be a InstallListener object", Cr.NS_ERROR_INVALID_ARG); - + if (!this.installListeners.some(function addInstallListener_matchListener(i) { return i == aListener; })) this.installListeners.push(aListener); @@ -1763,7 +1821,7 @@ var AddonManagerInternal = { new AsyncObjectCaller(aIDs, null, { nextObject: function getAddonsByIDs_nextObject(aCaller, aID) { - AddonManagerInternal.getAddonByID(aID, + AddonManagerInternal.getAddonByID(aID, function getAddonsByIDs_getAddonByID(aAddon) { addons.push(aAddon); aCaller.callNext(); @@ -2444,7 +2502,7 @@ this.AddonManager = { if (!aAddon || typeof aAddon != "object") throw Components.Exception("aAddon must be specified", Cr.NS_ERROR_INVALID_ARG); - + if (!("applyBackgroundUpdates" in aAddon)) return false; if (aAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_ENABLE) diff --git a/toolkit/mozapps/extensions/AddonRepository.jsm b/toolkit/mozapps/extensions/AddonRepository.jsm index 71efe214128f..93bfc145f4c4 100644 --- a/toolkit/mozapps/extensions/AddonRepository.jsm +++ b/toolkit/mozapps/extensions/AddonRepository.jsm @@ -23,6 +23,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "DeferredSave", "resource://gre/modules/DeferredSave.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository_SQLiteMigrator", "resource://gre/modules/AddonRepository_SQLiteMigrator.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Promise", + "resource://gre/modules/Promise.jsm"); this.EXPORTED_SYMBOLS = [ "AddonRepository" ]; @@ -525,34 +527,17 @@ this.AddonRepository = { // Maximum number of results to return _maxResults: null, - /** - * Initialize AddonRepository. - */ - initialize: function AddonRepo_initialize() { - Services.obs.addObserver(this, "xpcom-shutdown", false); - }, - - /** - * Observe xpcom-shutdown notification, so we can shutdown cleanly. - */ - observe: function AddonRepo_observe(aSubject, aTopic, aData) { - if (aTopic == "xpcom-shutdown") { - Services.obs.removeObserver(this, "xpcom-shutdown"); - this.shutdown(); - } - }, - /** * Shut down AddonRepository + * return: promise{integer} resolves with the result of flushing + * the AddonRepository database */ shutdown: function AddonRepo_shutdown() { this.cancelSearch(); this._addons = null; this._pendingCallbacks = null; - AddonDatabase.shutdown(function shutdown_databaseShutdown() { - Services.obs.notifyObservers(null, "addon-repository-shutdown", null); - }); + return AddonDatabase.shutdown(false); }, /** @@ -1514,7 +1499,6 @@ this.AddonRepository = { } }; -AddonRepository.initialize(); var AddonDatabase = { // true if the database connection has been opened @@ -1644,13 +1628,11 @@ var AddonDatabase = { * An optional boolean to skip flushing data to disk. Useful * when the database is going to be deleted afterwards. */ - shutdown: function AD_shutdown(aCallback, aSkipFlush) { + shutdown: function AD_shutdown(aSkipFlush) { this.databaseOk = true; - aCallback = aCallback || function() {}; if (!this.initialized) { - aCallback(); - return; + return Promise.resolve(0); } this.initialized = false; @@ -1660,9 +1642,9 @@ var AddonDatabase = { }); if (aSkipFlush) { - aCallback(); + return Promise.resolve(0); } else { - this.Writer.flush().then(aCallback, aCallback); + return this.Writer.flush(); } }, @@ -1676,13 +1658,14 @@ var AddonDatabase = { delete: function AD_delete(aCallback) { this.DB = BLANK_DB(); - this.Writer.flush().then(null, () => {}).then(() => { - this.shutdown(() => { - let promise = OS.File.remove(this.jsonFile.path, {}); - if (aCallback) - promise.then(aCallback, aCallback); - }, true); - }); + this.Writer.flush() + .then(null, () => {}) + // shutdown(true) never rejects + .then(() => this.shutdown(true)) + .then(() => OS.File.remove(this.jsonFile.path, {})) + .then(null, error => ERROR("Unable to delete Addon Repository file " + + this.jsonFile.path, error)) + .then(aCallback); }, toJSON: function AD_toJSON() { diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm index 67bc4034e0f9..a406e42774b6 100644 --- a/toolkit/mozapps/extensions/XPIProvider.jsm +++ b/toolkit/mozapps/extensions/XPIProvider.jsm @@ -1685,6 +1685,8 @@ var XPIProvider = { this.installs = []; this.installLocations = []; this.installLocationsByName = {}; + // Hook for tests to detect when saving database at shutdown time fails + this._shutdownError = null; AddonManagerPrivate.recordTimestamp("XPI_startup_begin"); @@ -1873,6 +1875,9 @@ var XPIProvider = { /** * Shuts down the database and releases all references. + * Return: Promise{integer} resolves / rejects with the result of + * flushing the XPI Database if it was loaded, + * 0 otherwise. */ shutdown: function XPI_shutdown() { LOG("shutdown"); @@ -1903,10 +1908,19 @@ var XPIProvider = { delete this._uriMappings; if (gLazyObjectsLoaded) { - XPIDatabase.shutdown(function shutdownCallback(saveError) { - LOG("Notifying XPI shutdown observers"); - Services.obs.notifyObservers(null, "xpi-provider-shutdown", saveError); - }); + let done = XPIDatabase.shutdown(); + done.then( + ret => { + LOG("Notifying XPI shutdown observers"); + Services.obs.notifyObservers(null, "xpi-provider-shutdown", null); + }, + err => { + LOG("Notifying XPI shutdown observers"); + this._shutdownError = err; + Services.obs.notifyObservers(null, "xpi-provider-shutdown", err); + } + ); + return done; } else { LOG("Notifying XPI shutdown observers"); @@ -3190,6 +3204,9 @@ var XPIProvider = { // active state of add-ons but didn't commit them properly (normally due // to the application crashing) let hasPendingChanges = Prefs.getBoolPref(PREF_PENDING_OPERATIONS); + if (hasPendingChanges) { + updateReasons.push("hasPendingChanges"); + } // If the schema appears to have changed then we should update the database if (DB_SCHEMA != Prefs.getIntPref(PREF_DB_SCHEMA, 0)) { @@ -3249,9 +3266,6 @@ var XPIProvider = { let extensionListChanged = false; // If the database needs to be updated then open it and then update it // from the filesystem - if (hasPendingChanges) { - updateReasons.push("hasPendingChanges"); - } if (updateReasons.length > 0) { AddonManagerPrivate.recordSimpleMeasure("XPIDB_startup_load_reasons", updateReasons); XPIDatabase.syncLoadDB(false); diff --git a/toolkit/mozapps/extensions/XPIProviderUtils.js b/toolkit/mozapps/extensions/XPIProviderUtils.js index 9db1488aa500..ec65d5cf8986 100644 --- a/toolkit/mozapps/extensions/XPIProviderUtils.js +++ b/toolkit/mozapps/extensions/XPIProviderUtils.js @@ -978,8 +978,11 @@ this.XPIDatabase = { /** * Shuts down the database connection and releases all cached objects. + * Return: Promise{integer} resolves / rejects with the result of the DB + * flush after the database is flushed and + * all cleanup is done */ - shutdown: function XPIDB_shutdown(aCallback) { + shutdown: function XPIDB_shutdown() { LOG("shutdown"); if (this.initialized) { // If our last database I/O had an error, try one last time to save. @@ -997,21 +1000,17 @@ this.XPIDatabase = { "XPIDB_saves_late", this._deferredSave.dirty ? 1 : 0); } - // Make sure any pending writes of the DB are complete, and we - // finish cleaning up, and then call back - this.flush() - .then(null, error => { + // Return a promise that any pending writes of the DB are complete and we + // are finished cleaning up + let flushPromise = this.flush(); + flushPromise.then(null, error => { ERROR("Flush of XPI database failed", error); AddonManagerPrivate.recordSimpleMeasure("XPIDB_shutdownFlush_failed", 1); - return 0; - }) - .then(count => { // If our last attempt to read or write the DB failed, force a new // extensions.ini to be written to disk on the next startup - let lastSaveFailed = this.lastError; - if (lastSaveFailed) - Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true); - + Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true); + }) + .then(count => { // Clear out the cached addons data loaded from JSON delete this.addonDB; delete this._dbPromise; @@ -1019,15 +1018,10 @@ this.XPIDatabase = { delete this._deferredSave; // re-enable the schema version setter delete this._schemaVersionSet; - - if (aCallback) - aCallback(lastSaveFailed); }); + return flushPromise; } - else { - if (aCallback) - aCallback(null); - } + return Promise.resolve(0); }, /** @@ -1382,6 +1376,13 @@ this.XPIDatabase = { * Synchronously calculates and updates all the active flags in the database. */ updateActiveAddons: function XPIDB_updateActiveAddons() { + if (!this.addonDB) { + WARN("updateActiveAddons called when DB isn't loaded"); + // force the DB to load + AddonManagerPrivate.recordSimpleMeasure("XPIDB_lateOpen_updateActive", + XPIProvider.runPhase); + this.syncLoadDB(true); + } LOG("Updating add-on states"); for (let [, addon] of this.addonDB) { let newActive = (addon.visible && !addon.userDisabled && diff --git a/toolkit/mozapps/extensions/test/xpcshell/head_addons.js b/toolkit/mozapps/extensions/test/xpcshell/head_addons.js index 2ab3358b1735..00f2c628b4c3 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/head_addons.js +++ b/toolkit/mozapps/extensions/test/xpcshell/head_addons.js @@ -400,29 +400,16 @@ function shutdownManager() { if (!gInternalManager) return; - let xpiShutdown = false; - Services.obs.addObserver({ - observe: function(aSubject, aTopic, aData) { - xpiShutdown = true; - gXPISaveError = aData; - Services.obs.removeObserver(this, "xpi-provider-shutdown"); - } - }, "xpi-provider-shutdown", false); - - let repositoryShutdown = false; - Services.obs.addObserver({ - observe: function(aSubject, aTopic, aData) { - repositoryShutdown = true; - Services.obs.removeObserver(this, "addon-repository-shutdown"); - } - }, "addon-repository-shutdown", false); + let shutdownDone = false; Services.obs.notifyObservers(null, "quit-application-granted", null); let scope = Components.utils.import("resource://gre/modules/AddonManager.jsm"); - scope.AddonManagerInternal.shutdown(); - gInternalManager = null; + scope.AddonManagerInternal.shutdown() + .then( + () => shutdownDone = true, + err => shutdownDone = true); - AddonRepository.shutdown(); + gInternalManager = null; // Load the add-ons list as it was after application shutdown loadAddonsList(); @@ -433,13 +420,16 @@ function shutdownManager() { let thr = Services.tm.mainThread; // Wait until we observe the shutdown notifications - while (!repositoryShutdown || !xpiShutdown) { + while (!shutdownDone) { thr.processNextEvent(true); } // Force the XPIProvider provider to reload to better // simulate real-world usage. scope = Components.utils.import("resource://gre/modules/XPIProvider.jsm"); + // This would be cleaner if I could get it as the rejection reason from + // the AddonManagerInternal.shutdown() promise + gXPISaveError = scope.XPIProvider._shutdownError; AddonManagerPrivate.unregisterProvider(scope.XPIProvider); Components.utils.unload("resource://gre/modules/XPIProvider.jsm"); } diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js b/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js index b87dd030ad53..ad8bd5bca085 100644 --- a/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js +++ b/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js @@ -78,40 +78,6 @@ function run_test() { db.schemaVersion = 1; db.close(); - Services.obs.addObserver({ - observe: function () { - Services.obs.removeObserver(this, "addon-repository-shutdown"); - // Check the DB schema has changed once AddonRepository has freed it. - db = AM_Cc["@mozilla.org/storage/service;1"]. - getService(AM_Ci.mozIStorageService). - openDatabase(dbfile); - do_check_eq(db.schemaVersion, EXPECTED_SCHEMA_VERSION); - do_check_true(db.indexExists("developer_idx")); - do_check_true(db.indexExists("screenshot_idx")); - do_check_true(db.indexExists("compatibility_override_idx")); - do_check_true(db.tableExists("compatibility_override")); - do_check_true(db.indexExists("icon_idx")); - do_check_true(db.tableExists("icon")); - - // Check the trigger is working - db.executeSimpleSQL("INSERT INTO addon (id, type, name) VALUES('test_addon', 'extension', 'Test Addon')"); - let internalID = db.lastInsertRowID; - db.executeSimpleSQL("INSERT INTO compatibility_override (addon_internal_id, num, type) VALUES('" + internalID + "', '1', 'incompatible')"); - - let stmt = db.createStatement("SELECT COUNT(*) AS count FROM compatibility_override"); - stmt.executeStep(); - do_check_eq(stmt.row.count, 1); - stmt.reset(); - - db.executeSimpleSQL("DELETE FROM addon"); - stmt.executeStep(); - do_check_eq(stmt.row.count, 0); - stmt.finalize(); - - db.close(); - do_test_finished(); - } - }, "addon-repository-shutdown", null); Services.prefs.setBoolPref("extensions.getAddons.cache.enabled", true); AddonRepository.getCachedAddonByID("test1@tests.mozilla.org", function (aAddon) { @@ -123,6 +89,39 @@ function run_test() { do_check_true(aAddon.screenshots[0].thumbnailHeight === null); do_check_eq(aAddon.iconURL, undefined); do_check_eq(JSON.stringify(aAddon.icons), "{}"); - AddonRepository.shutdown(); + AddonRepository.shutdown().then( + function checkAfterRepoShutdown() { + // Check the DB schema has changed once AddonRepository has freed it. + db = AM_Cc["@mozilla.org/storage/service;1"]. + getService(AM_Ci.mozIStorageService). + openDatabase(dbfile); + do_check_eq(db.schemaVersion, EXPECTED_SCHEMA_VERSION); + do_check_true(db.indexExists("developer_idx")); + do_check_true(db.indexExists("screenshot_idx")); + do_check_true(db.indexExists("compatibility_override_idx")); + do_check_true(db.tableExists("compatibility_override")); + do_check_true(db.indexExists("icon_idx")); + do_check_true(db.tableExists("icon")); + + // Check the trigger is working + db.executeSimpleSQL("INSERT INTO addon (id, type, name) VALUES('test_addon', 'extension', 'Test Addon')"); + let internalID = db.lastInsertRowID; + db.executeSimpleSQL("INSERT INTO compatibility_override (addon_internal_id, num, type) VALUES('" + internalID + "', '1', 'incompatible')"); + + let stmt = db.createStatement("SELECT COUNT(*) AS count FROM compatibility_override"); + stmt.executeStep(); + do_check_eq(stmt.row.count, 1); + stmt.reset(); + + db.executeSimpleSQL("DELETE FROM addon"); + stmt.executeStep(); + do_check_eq(stmt.row.count, 0); + stmt.finalize(); + + db.close(); + do_test_finished(); + }, + do_report_unexpected_exception + ); }); }