Bug 911621 - Ensure that XPI and AddonRepository JSON is completely written before shutdown. r=Unfocused
authorIrving Reid <irving@mozilla.com>
Thu, 12 Sep 2013 23:11:30 -0400
changeset 160265 6a16ae20601751299bec39198a744a3afced8da3
parent 160264 26b48b6dbff4ca8d83b81307da0eea5e3663f41d
child 160266 2a4109a72ff7f2ec9b55551d835779dc97b1bc33
push id2961
push userlsblakk@mozilla.com
push dateMon, 28 Oct 2013 21:59:28 +0000
treeherdermozilla-beta@73ef4f13486f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersUnfocused
bugs911621
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 911621 - Ensure that XPI and AddonRepository JSON is completely written before shutdown. r=Unfocused
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/AddonRepository.jsm
toolkit/mozapps/extensions/XPIProvider.jsm
toolkit/mozapps/extensions/XPIProviderUtils.js
toolkit/mozapps/extensions/test/xpcshell/head_addons.js
toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -35,22 +35,26 @@ const PREF_EM_CHECK_COMPATIBILITY_BASE =
 #ifdef MOZ_COMPATIBILITY_NIGHTLY
 var PREF_EM_CHECK_COMPATIBILITY = PREF_EM_CHECK_COMPATIBILITY_BASE + ".nightly";
 #else
 var PREF_EM_CHECK_COMPATIBILITY;
 #endif
 
 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 = {};
   Components.utils.import("resource://gre/modules/CertUtils.jsm", certUtils);
   return certUtils;
 });
 
 
@@ -448,18 +452,16 @@ var AddonManagerInternal = {
    * them.
    */
   startup: function AMI_startup() {
     if (gStarted)
       return;
 
     this.recordTimestamp("AMI_startup_begin");
 
-    Services.obs.addObserver(this, SHUTDOWN_EVENT, false);
-
     let appChanged = undefined;
 
     let oldAppVersion = null;
     try {
       oldAppVersion = Services.prefs.getCharPref(PREF_EM_LAST_APP_VERSION);
       appChanged = Services.appinfo.version != oldAppVersion;
     }
     catch (e) { }
@@ -542,16 +544,20 @@ var AddonManagerInternal = {
         Components.utils.import(url, {});
       }
       catch (e) {
         ERROR("Exception loading provider " + entry + " from category \"" +
               url + "\"", e);
       }
     }
 
+    // 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;
 
     this.callProviders("startup", appChanged, oldAppVersion,
                        oldPlatformVersion);
 
     // If this is a new profile just pretend that there were no changes
     if (appChanged === undefined) {
@@ -674,55 +680,108 @@ var AddonManagerInternal = {
       }
       catch (e) {
         ERROR("Exception calling provider " + aMethod, e);
       }
     }
   },
 
   /**
+   * 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);
     Services.prefs.removeObserver(PREF_EM_UPDATE_ENABLED, this);
     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;
   },
 
   /**
    * Notified when a preference we're interested in has changed.
    *
    * @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;
         try {
           gCheckCompatibility = Services.prefs.getBoolPref(PREF_EM_CHECK_COMPATIBILITY);
         } catch(e) {
           gCheckCompatibility = true;
         }
@@ -905,28 +964,27 @@ var AddonManagerInternal = {
         Services.obs.notifyObservers(null,
                                      "addons-background-update-complete",
                                      null);
       }
     }
 
     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();
 
       pendingUpdates++;
       this.getAllAddons(function getAddonsCallback(aAddons) {
         // If there is a known hotfix then exclude it from the list of add-ons to update.
         var ids = [a.id for each (a in aAddons) if (a.id != hotfixID)];
 
         // 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() {
 
             pendingUpdates += aAddons.length;
             aAddons.forEach(function BUC_forEachCallback(aAddon) {
               if (aAddon.id == hotfixID) {
                 notifyComplete();
@@ -1139,17 +1197,17 @@ var AddonManagerInternal = {
    * the extraListeners parameter are passed to the listener.
    *
    * @param  aMethod
    *         The method on the listeners to call
    * @param  aExtraListeners
    *         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",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (!aMethod || typeof aMethod != "string")
       throw Components.Exception("aMethod must be a non-empty string",
                                  Cr.NS_ERROR_INVALID_ARG);
@@ -1243,17 +1301,17 @@ var AddonManagerInternal = {
    */
   updateAddonAppDisabledStates: function AMI_updateAddonAppDisabledStates() {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     this.callProviders("updateAddonAppDisabledStates");
   },
-  
+
   /**
    * Notifies all providers that the repository has updated its data for
    * installed add-ons.
    *
    * @param  aCallback
    *         Function to call when operation is complete.
    */
   updateAddonRepositoryData: function AMI_updateAddonRepositoryData(aCallback) {
@@ -1627,17 +1685,17 @@ var AddonManagerInternal = {
    *
    * @param  aListener
    *         The InstallListener to add
    */
   addInstallListener: function AMI_addInstallListener(aListener) {
     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);
   },
 
   /**
    * Removes an InstallListener if the listener is registered.
    *
@@ -1758,17 +1816,17 @@ var AddonManagerInternal = {
     if (typeof aCallback != "function")
       throw Components.Exception("aCallback must be a function",
                                  Cr.NS_ERROR_INVALID_ARG);
 
     let addons = [];
 
     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();
         });
       },
 
       noMoreObjects: function getAddonsByIDs_noMoreObjects(aCaller) {
         safeCall(aCallback, addons);
@@ -2439,17 +2497,17 @@ this.AddonManager = {
    * @param  aAddon
    *         The Addon representing the add-on
    * @return true if the addon should auto-update, false otherwise.
    */
   shouldAutoUpdate: function AM_shouldAutoUpdate(aAddon) {
     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)
       return true;
     if (aAddon.applyBackgroundUpdates == AddonManager.AUTOUPDATE_DISABLE)
       return false;
     return this.autoUpdateDefault;
   },
--- a/toolkit/mozapps/extensions/AddonRepository.jsm
+++ b/toolkit/mozapps/extensions/AddonRepository.jsm
@@ -18,16 +18,18 @@ XPCOMUtils.defineLazyModuleGetter(this, 
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 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" ];
 
 const PREF_GETADDONS_CACHE_ENABLED       = "extensions.getAddons.cache.enabled";
 const PREF_GETADDONS_CACHE_TYPES         = "extensions.getAddons.cache.types";
 const PREF_GETADDONS_CACHE_ID_ENABLED    = "extensions.%ID%.getAddons.cache.enabled"
 const PREF_GETADDONS_BROWSEADDONS        = "extensions.getAddons.browseAddons";
 const PREF_GETADDONS_BYIDS               = "extensions.getAddons.get.url";
@@ -521,43 +523,26 @@ this.AddonRepository = {
    * searchFailed - Called when an error occurred when performing a search.
    */
   _callback: null,
 
   // 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);
   },
 
   /**
    * Asynchronously get a cached add-on by id. The add-on (or null if the
    * add-on is not found) is passed to the specified callback. If caching is
    * disabled, null is passed to the specified callback.
    *
    * @param  aId
@@ -1509,17 +1494,16 @@ this.AddonRepository = {
           Services.vc.compare(appVersion, override.appMaxVersion) <= 0) {
         return override;
       }
     }
     return null;
   }
 
 };
-AddonRepository.initialize();
 
 var AddonDatabase = {
   // true if the database connection has been opened
   initialized: false,
   // false if there was an unrecoverable error openning the database
   databaseOk: true,
 
   // the in-memory database
@@ -1639,55 +1623,54 @@ var AddonDatabase = {
    * cached objects
    *
    * @param  aCallback
    *         An optional callback to call once complete
    * @param  aSkipFlush
    *         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;
 
     this.__defineGetter__("connection", function shutdown_connectionGetter() {
       return this.openConnection();
     });
 
     if (aSkipFlush) {
-      aCallback();
+      return Promise.resolve(0);
     } else {
-      this.Writer.flush().then(aCallback, aCallback);
+      return this.Writer.flush();
     }
   },
 
   /**
    * Asynchronously deletes the database, shutting down the connection
    * first if initialized
    *
    * @param  aCallback
    *         An optional callback to call once complete
    */
   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() {
     let json = {
       schema: this.DB.schema,
       addons: []
     }
 
--- a/toolkit/mozapps/extensions/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
@@ -1680,16 +1680,18 @@ var XPIProvider = {
    *         if it is a new profile or the version is unknown
    */
   startup: function XPI_startup(aAppChanged, aOldAppVersion, aOldPlatformVersion) {
     LOG("startup");
     this.runPhase = XPI_STARTING;
     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");
 
     function addDirectoryInstallLocation(aName, aKey, aPaths, aScope, aLocked) {
       try {
         var dir = FileUtils.getDir(aKey, aPaths);
       }
       catch (e) {
@@ -1868,16 +1870,19 @@ var XPIProvider = {
     AddonManagerPrivate.recordTimestamp("XPI_startup_end");
 
     this.extensionsActive = true;
     this.runPhase = XPI_BEFORE_UI_STARTUP;
   },
 
   /**
    * 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");
 
     this.bootstrappedAddons = {};
     this.bootstrapScopes = {};
     this.enabledAddons = null;
     this.allAppGlobal = true;
@@ -1898,20 +1903,29 @@ var XPIProvider = {
 
     // This is needed to allow xpcshell tests to simulate a restart
     this.extensionsActive = false;
 
     // Remove URI mappings again
     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");
       Services.obs.notifyObservers(null, "xpi-provider-shutdown", null);
     }
   },
 
   /**
@@ -3185,16 +3199,19 @@ var XPIProvider = {
     if (updated) {
       updateReasons.push("pendingFileChanges");
     }
 
     // This will be true if the previous session made changes that affect the
     // 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)) {
       updateReasons.push("schemaChanged");
     }
 
     // If the application has changed then check for new distribution add-ons
     if (aAppChanged !== false &&
@@ -3244,19 +3261,16 @@ var XPIProvider = {
       }
     }
 
     // Catch and log any errors during the main startup
     try {
       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);
         try {
           extensionListChanged = this.processFileChanges(state, manifests,
                                                          aAppChanged,
                                                          aOldAppVersion,
                                                          aOldPlatformVersion);
--- a/toolkit/mozapps/extensions/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/XPIProviderUtils.js
@@ -973,18 +973,21 @@ this.XPIDatabase = {
         stmt.finalize();
     }
 
     return migrateData;
   },
 
   /**
    * 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.
       if (this.lastError)
         this.saveChanges();
 
       this.initialized = false;
 
@@ -992,47 +995,38 @@ this.XPIDatabase = {
         AddonManagerPrivate.recordSimpleMeasure(
             "XPIDB_saves_total", this._deferredSave.totalSaves);
         AddonManagerPrivate.recordSimpleMeasure(
             "XPIDB_saves_overlapped", this._deferredSave.overlappedSaves);
         AddonManagerPrivate.recordSimpleMeasure(
             "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;
           // same for the deferred save
           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);
   },
 
   /**
    * Return a list of all install locations known about by the database. This
    * is often a a subset of the total install locations when not all have
    * installed add-ons, occasionally a superset when an install location no
    * longer exists. Only called from XPIProvider.processFileChanges, when
    * the database should already be loaded.
@@ -1377,16 +1371,23 @@ this.XPIDatabase = {
     aAddon.active = aActive;
     this.saveChanges();
   },
 
   /**
    * 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 &&
                       !addon.softDisabled && !addon.appDisabled &&
                       !addon.pendingUninstall);
       if (newActive != addon.active) {
         addon.active = newActive;
         this.saveChanges();
--- a/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/head_addons.js
@@ -395,56 +395,46 @@ function restartManager(aNewVersion) {
     startupManager(false);
   }
 }
 
 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();
+  scope.AddonManagerInternal.shutdown()
+    .then(
+        () => shutdownDone = true,
+        err => shutdownDone = true);
+
   gInternalManager = null;
 
-  AddonRepository.shutdown();
-
   // Load the add-ons list as it was after application shutdown
   loadAddonsList();
 
   // Clear any crash report annotations
   gAppInfo.annotations = {};
 
   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");
 }
 
 function loadAddonsList() {
   function readDirectories(aSection) {
     var dirs = [];
     var keys = parser.getKeys(aSection);
--- a/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_migrateAddonRepository.js
@@ -73,56 +73,55 @@ function run_test() {
   stmt.params.thumbnailURL = "http://localhost/thumbnail1-1.png";
   stmt.params.caption = "Caption 1 - 1";
   stmt.execute();
   stmt.finalize();
 
   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) {
     do_check_neq(aAddon, null);
     do_check_eq(aAddon.screenshots.length, 1);
     do_check_true(aAddon.screenshots[0].width === null);
     do_check_true(aAddon.screenshots[0].height === null);
     do_check_true(aAddon.screenshots[0].thumbnailWidth === null);
     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
+    );
   });
 }