Bug 853388: Load JSON database asynchronously outside of startup; r=unfocused
☠☠ backed out by bc2cb7f532f2 ☠ ☠
authorIrving Reid <irving@mozilla.com>
Thu, 08 Aug 2013 16:05:33 -0400
changeset 149810 40c48d65e38287193f978193268b10cbe9a051f1
parent 149809 f3cf78d7e62dd8e21a0860ed3bd11259a00774f3
child 149811 607b35c777f079c2b097aed34aa0bd528afadece
push idunknown
push userunknown
push dateunknown
reviewersunfocused
bugs853388
milestone26.0a1
Bug 853388: Load JSON database asynchronously outside of startup; r=unfocused
toolkit/mozapps/extensions/XPIProvider.jsm
toolkit/mozapps/extensions/XPIProviderUtils.js
toolkit/mozapps/extensions/test/xpcshell/test_migrate_max_version.js
--- a/toolkit/mozapps/extensions/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
@@ -3213,19 +3213,18 @@ 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 (updateDatabase || hasPendingChanges) {
+        XPIDatabase.syncLoadDB(false);
         try {
-          XPIDatabase.openConnection(false, true);
-
           extensionListChanged = this.processFileChanges(state, manifests,
                                                          aAppChanged,
                                                          aOldAppVersion,
                                                          aOldPlatformVersion);
         }
         catch (e) {
           ERROR("Failed to process extension changes at startup", e);
         }
--- a/toolkit/mozapps/extensions/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/XPIProviderUtils.js
@@ -15,16 +15,18 @@ Cu.import("resource://gre/modules/Servic
 XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository",
                                   "resource://gre/modules/AddonRepository.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "FileUtils",
                                   "resource://gre/modules/FileUtils.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "DeferredSave",
                                   "resource://gre/modules/DeferredSave.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/Promise.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "OS",
+                                  "resource://gre/modules/osfile.jsm");
 
 ["LOG", "WARN", "ERROR"].forEach(function(aName) {
   Object.defineProperty(this, aName, {
     get: function logFuncGetter () {
       Cu.import("resource://gre/modules/AddonLogging.jsm");
 
       LogManager.getLogger("addons.xpi-utils", this);
       return this[aName];
@@ -154,16 +156,30 @@ function getRepositoryAddon(aAddon, aCal
                                       aRepositoryAddon.compatibilityOverrides :
                                       null;
     aCallback(aAddon);
   }
   AddonRepository.getCachedAddonByID(aAddon.id, completeAddon);
 }
 
 /**
+ * Wrap an API-supplied function in an exception handler to make it safe to call
+ */
+function safeCallback(aCallback) {
+  return function(...aArgs) {
+    try {
+      aCallback.apply(null, aArgs);
+    }
+    catch(ex) {
+      WARN("XPI Database callback failed", ex);
+    }
+  }
+}
+
+/**
  * A helper method to asynchronously call a function on an array
  * of objects, calling a callback when function(x) has been gathered
  * for every element of the array.
  * WARNING: not currently error-safe; if the async function does not call
  * our internal callback for any of the array elements, asyncMap will not
  * call the callback parameter.
  *
  * @param  aObjects
@@ -370,16 +386,42 @@ DBAddonInternal.prototype = {
 
   toJSON: function() {
     return copyProperties(this, PROP_JSON_FIELDS);
   }
 }
 
 DBAddonInternal.prototype.__proto__ = AddonInternal.prototype;
 
+/**
+ * Internal interface: find an addon from an already loaded addonDB
+ */
+function _findAddon(addonDB, aFilter) {
+  for (let [, addon] of addonDB) {
+    if (aFilter(addon)) {
+      return addon;
+    }
+  }
+  return null;
+}
+
+/**
+ * Internal interface to get a filtered list of addons from a loaded addonDB
+ */
+function _filterDB(addonDB, aFilter) {
+  let addonList = [];
+  for (let [, addon] of addonDB) {
+    if (aFilter(addon)) {
+      addonList.push(addon);
+    }
+  }
+
+  return addonList;
+}
+
 this.XPIDatabase = {
   // true if the database connection has been opened
   initialized: false,
   // The database file
   jsonFile: FileUtils.getFile(KEY_PROFILEDIR, [FILE_JSON_DB], true),
   // Migration data loaded from an old version of the database.
   migrateData: null,
   // Active add-on directories loaded from extensions.ini and prefs at startup.
@@ -405,16 +447,22 @@ this.XPIDatabase = {
       throw new Error("Attempt to use XPI database when it is not initialized");
     }
 
     // handle the "in memory only" case
     if (this.lockedDatabase) {
       return;
     }
 
+    if (!this._deferredSave) {
+      this._deferredSave = new DeferredSave(this.jsonFile.path,
+                                            () => JSON.stringify(this),
+                                            ASYNC_SAVE_DELAY_MS);
+    }
+
     let promise = this._deferredSave.saveChanges();
     if (!this._schemaVersionSet) {
       this._schemaVersionSet = true;
       promise.then(
         count => {
           // Update the XPIDB schema version preference the first time we successfully
           // save the database.
           LOG("XPI Database saved, setting schema version preference to " + DB_SCHEMA);
@@ -424,39 +472,37 @@ this.XPIDatabase = {
           // Need to try setting the schema version again later
           this._schemaVersionSet = false;
           WARN("Failed to save XPI database", error);
         });
     }
   },
 
   flush: function() {
-    // handle the "in memory only" case
-    if (this.lockedDatabase) {
+    // handle the "in memory only" and "saveChanges never called" cases
+    if (!this._deferredSave) {
       let done = Promise.defer();
       done.resolve(0);
       return done.promise;
     }
 
     return this._deferredSave.flush();
   },
 
-  get _deferredSave() {
-    delete this._deferredSave;
-    return this._deferredSave =
-      new DeferredSave(this.jsonFile.path, () => JSON.stringify(this),
-                       ASYNC_SAVE_DELAY_MS);
-  },
-
   /**
    * Converts the current internal state of the XPI addon database to JSON
    */
   toJSON: function() {
+    if (!this.addonDB) {
+      // We never loaded the database?
+      throw new Error("Attempt to save database without loading it first");
+    }
+
     let addons = [];
-    for (let [key, addon] of this.addonDB) {
+    for (let [, addon] of this.addonDB) {
       addons.push(addon);
     }
     let toSave = {
       schemaVersion: DB_SCHEMA,
       addons: addons
     };
     return toSave;
   },
@@ -484,37 +530,35 @@ this.XPIDatabase = {
     }
     LOG("Migrating data from sqlite");
     let migrateData = this.getMigrateDataFromDatabase(connection);
     connection.close();
     return migrateData;
   },
 
   /**
-   * Opens and reads the database file, upgrading from old
+   * Synchronously opens and reads the database file, upgrading from old
    * databases or making a new DB if needed.
    *
    * The possibilities, in order of priority, are:
    * 1) Perfectly good, up to date database
    * 2) Out of date JSON database needs to be upgraded => upgrade
    * 3) JSON database exists but is mangled somehow => build new JSON
    * 4) no JSON DB, but a useable SQLITE db we can upgrade from => upgrade
    * 5) useless SQLITE DB => build new JSON
    * 6) useable RDF DB => upgrade
    * 7) useless RDF DB => build new JSON
    * 8) Nothing at all => build new JSON
    * @param  aRebuildOnError
    *         A boolean indicating whether add-on information should be loaded
    *         from the install locations if the database needs to be rebuilt.
    *         (if false, caller is XPIProvider.checkForChanges() which will rebuild)
    */
-  openConnection: function XPIDB_openConnection(aRebuildOnError, aForceOpen) {
-    // XXX TELEMETRY report opens with aRebuildOnError true (which implies delayed open)
-    // vs. aRebuildOnError false (DB loaded during startup)
-    delete this.addonDB;
+  syncLoadDB: function XPIDB_syncLoadDB(aRebuildOnError) {
+    // XXX TELEMETRY report synchronous opens (startup time) vs. delayed opens
     this.migrateData = null;
     let fstream = null;
     let data = "";
     try {
       LOG("Opening XPI database " + this.jsonFile.path);
       fstream = Components.classes["@mozilla.org/network/file-input-stream;1"].
               createInstance(Components.interfaces.nsIFileInputStream);
       fstream.init(this.jsonFile, -1, 0, 0);
@@ -525,101 +569,174 @@ this.XPIDatabase = {
         cstream.init(fstream, "UTF-8", 0, 0);
         let (str = {}) {
           let read = 0;
           do {
             read = cstream.readString(0xffffffff, str); // read as much as we can and put it in str.value
             data += str.value;
           } while (read != 0);
         }
-        // dump("Loaded JSON:\n" + data + "\n");
-        let inputAddons = JSON.parse(data);
-        // Now do some sanity checks on our JSON db
-        if (!("schemaVersion" in inputAddons) || !("addons" in inputAddons)) {
-          // Content of JSON file is bad, need to rebuild from scratch
-          ERROR("bad JSON file contents");
-          this.rebuildDatabase(aRebuildOnError);
-        }
-        if (inputAddons.schemaVersion != DB_SCHEMA) {
-          // Handle mismatched JSON schema version. For now, we assume
-          // compatibility for JSON data, though we throw away any fields we
-          // don't know about
-          // XXX preserve unknown fields during save/restore
-          LOG("JSON schema mismatch: expected " + DB_SCHEMA +
-              ", actual " + inputAddons.schemaVersion);
-        }
-        // If we got here, we probably have good data
-        // Make AddonInternal instances from the loaded data and save them
-        let addonDB = new Map();
-        inputAddons.addons.forEach(function(loadedAddon) {
-          let newAddon = new DBAddonInternal(loadedAddon);
-          addonDB.set(newAddon._key, newAddon);
-        });
-        this.addonDB = addonDB;
-        LOG("Successfully read XPI database");
-        this.initialized = true;
+        this.parseDB(data, aRebuildOnError);
       }
       catch(e) {
-        // If we catch and log a SyntaxError from the JSON
-        // parser, the xpcshell test harness fails the test for us: bug 870828
-        if (e.name == "SyntaxError") {
-          ERROR("Syntax error parsing saved XPI JSON data");
-        }
-        else {
-          ERROR("Failed to load XPI JSON data from profile", e);
-        }
+        ERROR("Failed to load XPI JSON data from profile", e);
         this.rebuildDatabase(aRebuildOnError);
       }
       finally {
         if (cstream)
           cstream.close();
       }
     }
     catch (e) {
       if (e.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
-        try {
-          let schemaVersion = Services.prefs.getIntPref(PREF_DB_SCHEMA);
-          if (schemaVersion <= LAST_SQLITE_DB_SCHEMA) {
-            // we should have an older SQLITE database
-            this.migrateData = this.getMigrateDataFromSQLITE();
-          }
-          // else we've upgraded before but the JSON file is gone, fall through
-          // and rebuild from scratch
-        }
-        catch(e) {
-          // No schema version pref means either a really old upgrade (RDF) or
-          // a new profile
-          this.migrateData = this.getMigrateDataFromRDF();
-        }
-
-        this.rebuildDatabase(aRebuildOnError);
+        this.upgradeDB(aRebuildOnError);
       }
       else {
-        WARN("Extensions database " + this.jsonFile.path +
-            " exists but is not readable; rebuilding in memory", e);
-        // XXX open question - if we can overwrite at save time, should we, or should we
-        // leave the locked database in case we can recover from it next time we start up?
-        // The old code made one attempt to remove the locked file before it rebuilt in memory
-        this.lockedDatabase = true;
-        // XXX TELEMETRY report when this happens?
-        this.rebuildDatabase(aRebuildOnError);
+        this.rebuildUnreadableDB(e, aRebuildOnError);
       }
     }
     finally {
       if (fstream)
         fstream.close();
     }
+    // If an async load was also in progress, resolve that promise with our DB;
+    // otherwise create a resolved promise
+    if (this._dbPromise)
+      this._dbPromise.resolve(this.addonDB);
+    else
+      this._dbPromise = Promise.resolve(this.addonDB);
+  },
 
-    return;
+  /**
+   * Parse loaded data, reconstructing the database if the loaded data is not valid
+   * @param aRebuildOnError
+   *        If true, synchronously reconstruct the database from installed add-ons
+   */
+  parseDB: function(aData, aRebuildOnError) {
+    try {
+      // dump("Loaded JSON:\n" + aData + "\n");
+      let inputAddons = JSON.parse(aData);
+      // Now do some sanity checks on our JSON db
+      if (!("schemaVersion" in inputAddons) || !("addons" in inputAddons)) {
+        // Content of JSON file is bad, need to rebuild from scratch
+        ERROR("bad JSON file contents");
+        this.rebuildDatabase(aRebuildOnError);
+        return;
+      }
+      if (inputAddons.schemaVersion != DB_SCHEMA) {
+        // Handle mismatched JSON schema version. For now, we assume
+        // compatibility for JSON data, though we throw away any fields we
+        // don't know about
+        // XXX preserve unknown fields during save/restore
+        LOG("JSON schema mismatch: expected " + DB_SCHEMA +
+            ", actual " + inputAddons.schemaVersion);
+      }
+      // If we got here, we probably have good data
+      // Make AddonInternal instances from the loaded data and save them
+      let addonDB = new Map();
+      inputAddons.addons.forEach(function(loadedAddon) {
+        let newAddon = new DBAddonInternal(loadedAddon);
+        addonDB.set(newAddon._key, newAddon);
+      });
+      this.addonDB = addonDB;
+      LOG("Successfully read XPI database");
+      this.initialized = true;
+    }
+    catch(e) {
+      // If we catch and log a SyntaxError from the JSON
+      // parser, the xpcshell test harness fails the test for us: bug 870828
+      if (e.name == "SyntaxError") {
+        ERROR("Syntax error parsing saved XPI JSON data");
+      }
+      else {
+        ERROR("Failed to load XPI JSON data from profile", e);
+      }
+      this.rebuildDatabase(aRebuildOnError);
+    }
+  },
 
-    // XXX what about aForceOpen? Appears to handle the case of "don't open DB file if there aren't any extensions"?
-    if (!aForceOpen && !this.dbfileExists) {
-      this.connection = null;
-      return;
+  /**
+   * Upgrade database from earlier (sqlite or RDF) version if available
+   */
+  upgradeDB: function(aRebuildOnError) {
+    try {
+      let schemaVersion = Services.prefs.getIntPref(PREF_DB_SCHEMA);
+      if (schemaVersion <= LAST_SQLITE_DB_SCHEMA) {
+        // we should have an older SQLITE database
+        this.migrateData = this.getMigrateDataFromSQLITE();
+      }
+      // else we've upgraded before but the JSON file is gone, fall through
+      // and rebuild from scratch
+    }
+    catch(e) {
+      // No schema version pref means either a really old upgrade (RDF) or
+      // a new profile
+      this.migrateData = this.getMigrateDataFromRDF();
     }
+
+    this.rebuildDatabase(aRebuildOnError);
+  },
+
+  /**
+   * Reconstruct when the DB file exists but is unreadable
+   * (for example because read permission is denied
+   */
+  rebuildUnreadableDB: function(aError, aRebuildOnError) {
+    WARN("Extensions database " + this.jsonFile.path +
+        " exists but is not readable; rebuilding in memory", aError);
+    // XXX open question - if we can overwrite at save time, should we, or should we
+    // leave the locked database in case we can recover from it next time we start up?
+    // The old code made one attempt to remove the locked file before it rebuilt in memory
+    this.lockedDatabase = true;
+    // XXX TELEMETRY report when this happens?
+    this.rebuildDatabase(aRebuildOnError);
+  },
+
+  /**
+   * Open and read the XPI database asynchronously, upgrading if
+   * necessary. If any DB load operation fails, we need to
+   * synchronously rebuild the DB from the installed extensions.
+   *
+   * @return Promise<Map> resolves to the Map of loaded JSON data stored
+   *         in this.addonDB; never rejects.
+   */
+  asyncLoadDB: function XPIDB_asyncLoadDB(aDBCallback) {
+    // Already started (and possibly finished) loading
+    if (this._dbPromise) {
+      return this._dbPromise;
+    }
+
+    LOG("Starting async load of XPI database " + this.jsonFile.path);
+    return this._dbPromise = OS.File.read(this.jsonFile.path).then(
+      byteArray => {
+        if (this._addonDB) {
+          LOG("Synchronous load completed while waiting for async load");
+          return this.addonDB;
+        }
+        LOG("Finished async read of XPI database, parsing...");
+        let decoder = new TextDecoder();
+        let data = decoder.decode(byteArray);
+        this.parseDB(data, true);
+        return this.addonDB;
+      })
+    .then(null,
+      error => {
+        if (this._addonDB) {
+          LOG("Synchronous load completed while waiting for async load");
+          return this.addonDB;
+        }
+        if (error.becauseNoSuchFile) {
+          this.upgradeDB(true);
+        }
+        else {
+          // it's there but unreadable
+          this.rebuildUnreadableDB(error, true);
+        }
+        return this.addonDB;
+      });
   },
 
   /**
    * Rebuild the database from addon install directories. If this.migrateData
    * is available, uses migrated information for settings on the addons found
    * during rebuild
    * @param aRebuildOnError
    *         A boolean indicating whether add-on information should be loaded
@@ -645,30 +762,22 @@ this.XPIDatabase = {
         ERROR("Failed to rebuild XPI database from installed extensions", e);
       }
       // Make to update the active add-ons and add-ons list on shutdown
       Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
     }
   },
 
   /**
-   * Lazy getter for the addons database
-   */
-  get addonDB() {
-    this.openConnection(true);
-    return this.addonDB;
-  },
-
-  /**
    * Gets the list of file descriptors of active extension directories or XPI
    * files from the add-ons list. This must be loaded from disk since the
    * directory service gives no easy way to get both directly. This list doesn't
    * include themes as preferences already say which theme is currently active
    *
-   * @return an array of persisitent descriptors for the directories
+   * @return an array of persistent descriptors for the directories
    */
   getActiveBundles: function XPIDB_getActiveBundles() {
     let bundles = [];
 
     let addonsList = FileUtils.getFile(KEY_PROFILEDIR, [FILE_XPI_ADDONS_LIST],
                                        true);
 
     if (!addonsList.exists())
@@ -877,37 +986,21 @@ this.XPIDatabase = {
       this.flush()
         .then(null, error => {
           ERROR("Flush of XPI database failed", error);
           Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
           result = error;
           return 0;
         })
         .then(count => {
-          // Clear out the cached addons data loaded from JSON and recreate
-          // the getter to allow database re-loads during testing.
+          // Clear out the cached addons data loaded from JSON
           delete this.addonDB;
-          Object.defineProperty(this, "addonDB", {
-            get: function addonsGetter() {
-              this.openConnection(true);
-              return this.addonDB;
-            },
-            configurable: true
-          });
+          delete this._dbPromise;
           // same for the deferred save
           delete this._deferredSave;
-          Object.defineProperty(this, "_deferredSave", {
-            set: function deferredSaveGetter() {
-              delete this._deferredSave;
-              return this._deferredSave =
-                new DeferredSave(this.jsonFile.path, this.formJSON.bind(this),
-                                 ASYNC_SAVE_DELAY_MS);
-            },
-            configurable: true
-          });
           // re-enable the schema version setter
           delete this._schemaVersionSet;
 
           if (aCallback)
             aCallback(result);
         });
     }
     else {
@@ -915,218 +1008,235 @@ this.XPIDatabase = {
         aCallback(null);
     }
   },
 
   /**
    * 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.
+   * longer exists. Only called from XPIProvider.processFileChanges, when
+   * the database should already be loaded.
    *
    * @return  a Set of names of install locations
    */
   getInstallLocations: function XPIDB_getInstallLocations() {
     let locations = new Set();
     if (!this.addonDB)
       return locations;
 
     for (let [, addon] of this.addonDB) {
       locations.add(addon.location);
     }
     return locations;
   },
 
   /**
-   * List all addons that match the filter function
+   * Asynchronously list all addons that match the filter function
    * @param  aFilter
    *         Function that takes an addon instance and returns
    *         true if that addon should be included in the selected array
-   * @return an array of DBAddonInternals
+   * @param  aCallback
+   *         Called back with an array of addons matching aFilter
+   *         or an empty array if none match
    */
-  _listAddons: function XPIDB_listAddons(aFilter) {
-    if (!this.addonDB)
-      return [];
-
-    let addonList = [];
-    for (let [key, addon] of this.addonDB) {
-      if (aFilter(addon)) {
-        addonList.push(addon);
-      }
-    }
-
-    return addonList;
+  getAddonList: function(aFilter, aCallback) {
+    this.asyncLoadDB().then(
+      addonDB => {
+        let addonList = _filterDB(addonDB, aFilter);
+        asyncMap(addonList, getRepositoryAddon, safeCallback(aCallback));
+      })
+    .then(null,
+        error => {
+          ERROR("getAddonList failed", e);
+          safeCallback(aCallback)([]);
+        });
   },
 
   /**
-   * Find the first addon that matches the filter function
+   * (Possibly asynchronously) get the first addon that matches the filter function
    * @param  aFilter
    *         Function that takes an addon instance and returns
    *         true if that addon should be selected
-   * @return The first DBAddonInternal for which the filter returns true
+   * @param  aCallback
+   *         Called back with the addon, or null if no matching addon is found
    */
-  _findAddon: function XPIDB_findAddon(aFilter) {
-    if (!this.addonDB)
-      return null;
-
-    for (let [key, addon] of this.addonDB) {
-      if (aFilter(addon)) {
-        return addon;
-      }
-    }
-
-    return null;
+  getAddon: function(aFilter, aCallback) {
+    return this.asyncLoadDB().then(
+      addonDB => {
+        getRepositoryAddon(_findAddon(addonDB, aFilter), safeCallback(aCallback));
+      })
+    .then(null,
+        error => {
+          ERROR("getAddon failed", e);
+          safeCallback(aCallback)(null);
+        });
   },
 
   /**
    * Synchronously reads all the add-ons in a particular install location.
+   * Always called with the addon database already loaded.
    *
    * @param  aLocation
    *         The name of the install location
    * @return an array of DBAddonInternals
    */
   getAddonsInLocation: function XPIDB_getAddonsInLocation(aLocation) {
-    return this._listAddons(function inLocation(aAddon) {return (aAddon.location == aLocation);});
+    return _filterDB(this.addonDB, aAddon => (aAddon.location == aLocation));
   },
 
   /**
    * Asynchronously gets an add-on with a particular ID in a particular
    * install location.
-   * XXX IRVING sync for now
    *
    * @param  aId
    *         The ID of the add-on to retrieve
    * @param  aLocation
    *         The name of the install location
    * @param  aCallback
    *         A callback to pass the DBAddonInternal to
    */
   getAddonInLocation: function XPIDB_getAddonInLocation(aId, aLocation, aCallback) {
-    getRepositoryAddon(this.addonDB.get(aLocation + ":" + aId), aCallback);
+    this.asyncLoadDB().then(
+        addonDB => getRepositoryAddon(addonDB.get(aLocation + ":" + aId),
+                                      safeCallback(aCallback)));
   },
 
   /**
-   * Asynchronously gets the add-on with an ID that is visible.
-   * XXX IRVING sync
+   * Asynchronously gets the add-on with the specified ID that is visible.
    *
    * @param  aId
    *         The ID of the add-on to retrieve
    * @param  aCallback
    *         A callback to pass the DBAddonInternal to
    */
   getVisibleAddonForID: function XPIDB_getVisibleAddonForID(aId, aCallback) {
-    let addon = this._findAddon(function visibleID(aAddon) {return ((aAddon.id == aId) && aAddon.visible)});
-    getRepositoryAddon(addon, aCallback);
+    this.getAddon(aAddon => ((aAddon.id == aId) && aAddon.visible),
+                  aCallback);
   },
 
   /**
    * Asynchronously gets the visible add-ons, optionally restricting by type.
-   * XXX IRVING sync
    *
    * @param  aTypes
    *         An array of types to include or null to include all types
    * @param  aCallback
    *         A callback to pass the array of DBAddonInternals to
    */
   getVisibleAddons: function XPIDB_getVisibleAddons(aTypes, aCallback) {
-    let addons = this._listAddons(function visibleType(aAddon) {
-      return (aAddon.visible && (!aTypes || (aTypes.length == 0) || (aTypes.indexOf(aAddon.type) > -1)))
-    });
-    asyncMap(addons, getRepositoryAddon, aCallback);
+    this.getAddonList(aAddon => (aAddon.visible &&
+                                 (!aTypes || (aTypes.length == 0) ||
+                                  (aTypes.indexOf(aAddon.type) > -1))),
+                      aCallback);
   },
 
   /**
    * Synchronously gets all add-ons of a particular type.
    *
    * @param  aType
    *         The type of add-on to retrieve
    * @return an array of DBAddonInternals
    */
   getAddonsByType: function XPIDB_getAddonsByType(aType) {
-    return this._listAddons(function byType(aAddon) { return aAddon.type == aType; });
+    if (!this.addonDB) {
+      // jank-tastic! Must synchronously load DB if the theme switches from
+      // an XPI theme to a lightweight theme before the DB has loaded,
+      // because we're called from sync XPIProvider.addonChanged
+      WARN("Synchronous load of XPI database due to getAddonsByType(" + aType + ")");
+      this.syncLoadDB(true);
+    }
+    return _filterDB(this.addonDB, aAddon => (aAddon.type == aType));
   },
 
   /**
    * Synchronously gets an add-on with a particular internalName.
    *
    * @param  aInternalName
    *         The internalName of the add-on to retrieve
    * @return a DBAddonInternal
    */
   getVisibleAddonForInternalName: function XPIDB_getVisibleAddonForInternalName(aInternalName) {
-    return this._findAddon(function visibleInternalName(aAddon) {
-      return (aAddon.visible && (aAddon.internalName == aInternalName));
-    });
+    if (!this.addonDB) {
+      // This may be called when the DB hasn't otherwise been loaded
+      // XXX TELEMETRY
+      WARN("Synchronous load of XPI database due to getVisibleAddonForInternalName");
+      this.syncLoadDB(true);
+    }
+    
+    return _findAddon(this.addonDB,
+                      aAddon => aAddon.visible &&
+                                (aAddon.internalName == aInternalName));
   },
 
   /**
    * Asynchronously gets all add-ons with pending operations.
-   * XXX IRVING sync
    *
    * @param  aTypes
    *         The types of add-ons to retrieve or null to get all types
    * @param  aCallback
    *         A callback to pass the array of DBAddonInternal to
    */
   getVisibleAddonsWithPendingOperations:
     function XPIDB_getVisibleAddonsWithPendingOperations(aTypes, aCallback) {
 
-    let addons = this._listAddons(function visibleType(aAddon) {
-      return (aAddon.visible &&
-        (aAddon.pendingUninstall ||
-         // Logic here is tricky. If we're active but either
-         // disabled flag is set, we're pending disable; if we're not
-         // active and neither disabled flag is set, we're pending enable
-         (aAddon.active == (aAddon.userDisabled || aAddon.appDisabled))) &&
-        (!aTypes || (aTypes.length == 0) || (aTypes.indexOf(aAddon.type) > -1)))
-    });
-    asyncMap(addons, getRepositoryAddon, aCallback);
+    this.getAddonList(
+        aAddon => (aAddon.visible &&
+                   (aAddon.pendingUninstall ||
+                    // Logic here is tricky. If we're active but either
+                    // disabled flag is set, we're pending disable; if we're not
+                    // active and neither disabled flag is set, we're pending enable
+                    (aAddon.active == (aAddon.userDisabled || aAddon.appDisabled))) &&
+                   (!aTypes || (aTypes.length == 0) || (aTypes.indexOf(aAddon.type) > -1))),
+        aCallback);
   },
 
   /**
    * Asynchronously get an add-on by its Sync GUID.
-   * XXX IRVING sync
    *
    * @param  aGUID
    *         Sync GUID of add-on to fetch
    * @param  aCallback
    *         A callback to pass the DBAddonInternal record to. Receives null
    *         if no add-on with that GUID is found.
    *
    */
   getAddonBySyncGUID: function XPIDB_getAddonBySyncGUID(aGUID, aCallback) {
-    let addon = this._findAddon(function bySyncGUID(aAddon) { return aAddon.syncGUID == aGUID; });
-    getRepositoryAddon(addon, aCallback);
+    this.getAddon(aAddon => aAddon.syncGUID == aGUID,
+                  aCallback);
   },
 
   /**
    * Synchronously gets all add-ons in the database.
+   * This is only called from the preference observer for the default
+   * compatibility version preference, so we can return an empty list if
+   * we haven't loaded the database yet.
    *
    * @return  an array of DBAddonInternals
    */
   getAddons: function XPIDB_getAddons() {
-    return this._listAddons(function(aAddon) {return true;});
+    if (!this.addonDB) {
+      return [];
+    }
+    return _filterDB(this.addonDB, aAddon => true);
   },
 
   /**
    * Synchronously adds an AddonInternal's metadata to the database.
    *
    * @param  aAddon
    *         AddonInternal to add
    * @param  aDescriptor
    *         The file descriptor of the add-on
    * @return The DBAddonInternal that was added to the database
    */
   addAddonMetadata: function XPIDB_addAddonMetadata(aAddon, aDescriptor) {
-    // If there is no DB yet then forcibly create one
-    // XXX IRVING I don't think this will work as expected because the addonDB
-    // getter will kick in. Might not matter because of the way the new DB
-    // creates itself.
-    if (!this.addonDB)
-      this.openConnection(false, true);
+    if (!this.addonDB) {
+      // XXX telemetry. Should never happen on platforms that have a default theme
+      this.syncLoadDB(false);
+    }
 
     let newAddon = new DBAddonInternal(aAddon);
     newAddon.descriptor = aDescriptor;
     this.addonDB.set(newAddon._key, newAddon);
     if (newAddon.visible) {
       this.makeAddonVisible(newAddon);
     }
 
@@ -1177,17 +1287,17 @@ this.XPIDatabase = {
    *
    * @param  aAddon
    *         The DBAddonInternal to make visible
    * @param  callback
    *         A callback to pass the DBAddonInternal to
    */
   makeAddonVisible: function XPIDB_makeAddonVisible(aAddon) {
     LOG("Make addon " + aAddon._key + " visible");
-    for (let [key, otherAddon] of this.addonDB) {
+    for (let [, otherAddon] of this.addonDB) {
       if ((otherAddon.id == aAddon.id) && (otherAddon._key != aAddon._key)) {
         LOG("Hide addon " + otherAddon._key);
         otherAddon.visible = false;
       }
     }
     aAddon.visible = true;
     this.saveChanges();
   },
@@ -1204,29 +1314,30 @@ this.XPIDatabase = {
     for (let key in aProperties) {
       aAddon[key] = aProperties[key];
     }
     this.saveChanges();
   },
 
   /**
    * Synchronously sets the Sync GUID for an add-on.
+   * Only called when the database is already loaded.
    *
    * @param  aAddon
    *         The DBAddonInternal being updated
    * @param  aGUID
    *         GUID string to set the value to
    * @throws if another addon already has the specified GUID
    */
   setAddonSyncGUID: function XPIDB_setAddonSyncGUID(aAddon, aGUID) {
     // Need to make sure no other addon has this GUID
     function excludeSyncGUID(otherAddon) {
       return (otherAddon._key != aAddon._key) && (otherAddon.syncGUID == aGUID);
     }
-    let otherAddon = this._findAddon(excludeSyncGUID);
+    let otherAddon = _findAddon(this.addonDB, excludeSyncGUID);
     if (otherAddon) {
       throw new Error("Addon sync GUID conflict for addon " + aAddon._key +
           ": " + otherAddon._key + " already has GUID " + aGUID);
     }
     aAddon.syncGUID = aGUID;
     this.saveChanges();
   },
 
@@ -1259,43 +1370,47 @@ this.XPIDatabase = {
 
   /**
    * Synchronously calculates and updates all the active flags in the database.
    */
   updateActiveAddons: function XPIDB_updateActiveAddons() {
     // XXX IRVING this may get called during XPI-utils shutdown
     // XXX need to make sure PREF_PENDING_OPERATIONS handling is clean
     LOG("Updating add-on states");
-    for (let [key, addon] of this.addonDB) {
+    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();
       }
     }
   },
 
   /**
    * Writes out the XPI add-ons list for the platform to read.
    */
   writeAddonsList: function XPIDB_writeAddonsList() {
+    if (!this.addonDB) {
+      // Unusual condition, force the DB to load
+      this.syncLoadDB(true);
+    }
     Services.appinfo.invalidateCachesOnRestart();
 
     let addonsList = FileUtils.getFile(KEY_PROFILEDIR, [FILE_XPI_ADDONS_LIST],
                                        true);
     let enabledAddons = [];
     let text = "[ExtensionDirs]\r\n";
     let count = 0;
     let fullCount = 0;
 
-    let activeAddons = this._listAddons(function active(aAddon) {
-      return aAddon.active && !aAddon.bootstrap && (aAddon.type != "theme");
-    });
+    let activeAddons = _filterDB(
+      this.addonDB,
+      aAddon => aAddon.active && !aAddon.bootstrap && (aAddon.type != "theme"));
 
     for (let row of activeAddons) {
       text += "Extension" + (count++) + "=" + row.descriptor + "\r\n";
       enabledAddons.push(encodeURIComponent(row.id) + ":" +
                          encodeURIComponent(row.version));
     }
     fullCount += count;
 
@@ -1305,22 +1420,23 @@ this.XPIDatabase = {
 
     let dssEnabled = false;
     try {
       dssEnabled = Services.prefs.getBoolPref(PREF_EM_DSS_ENABLED);
     } catch (e) {}
 
     let themes = [];
     if (dssEnabled) {
-      themes = this._listAddons(function isTheme(aAddon){ return aAddon.type == "theme"; });
+      themes = _filterDB(this.addonDB, aAddon => aAddon.type == "theme");
     }
     else {
-      let activeTheme = this._findAddon(function isSelected(aAddon) {
-        return ((aAddon.type == "theme") && (aAddon.internalName == XPIProvider.selectedSkin));
-      });
+      let activeTheme = _findAddon(
+        this.addonDB,
+        aAddon => (aAddon.type == "theme") &&
+                  (aAddon.internalName == XPIProvider.selectedSkin));
       if (activeTheme) {
         themes.push(activeTheme);
       }
     }
 
     if (themes.length > 0) {
       count = 0;
       for (let row of themes) {
--- a/toolkit/mozapps/extensions/test/xpcshell/test_migrate_max_version.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_migrate_max_version.js
@@ -40,49 +40,50 @@ function run_test() {
                           "userDisabled INTEGER, installDate INTEGER");
   db.createTable("targetApplication", "addon_internal_id INTEGER, " +
                                       "id TEXT, minVersion TEXT, maxVersion TEXT");
   let stmt = db.createStatement("INSERT INTO addon VALUES (NULL, :id, :location, " +
                                 ":version, :active, :userDisabled, :installDate)");
 
   let internal_ids = {};
 
-  [["addon1@tests.mozilla.org", "app-profile", "1.0", "0", "1", "0"]
-   ].forEach(function(a) {
-    stmt.params.id = a[0];
-    stmt.params.location = a[1];
-    stmt.params.version = a[2];
-    stmt.params.active = a[3];
-    stmt.params.userDisabled = a[4];
-    stmt.params.installDate = a[5];
-    stmt.execute();
-    internal_ids[a[0]] = db.lastInsertRowID;
-  });
+  let a = ["addon1@tests.mozilla.org", "app-profile", "1.0", "0", "1", "0"];
+  stmt.params.id = a[0];
+  stmt.params.location = a[1];
+  stmt.params.version = a[2];
+  stmt.params.active = a[3];
+  stmt.params.userDisabled = a[4];
+  stmt.params.installDate = a[5];
+  stmt.execute();
+  internal_ids[a[0]] = db.lastInsertRowID;
   stmt.finalize();
 
-  db.schemaVersion = 15;
+  db.schemaVersion = 14;
   Services.prefs.setIntPref("extensions.databaseSchema", 14);
   db.close();
 
   startupManager();
+  run_next_test();
+}
 
+add_test(function before_rebuild() {
   AddonManager.getAddonByID("addon1@tests.mozilla.org",
                             function check_before_rebuild (a1) {
     // First check that it migrated OK once
     // addon1 was disabled in the database
     do_check_neq(a1, null);
     do_check_true(a1.userDisabled);
     do_check_false(a1.appDisabled);
     do_check_false(a1.isActive);
     do_check_false(a1.strictCompatibility);
     do_check_false(a1.foreignInstall);
 
     run_next_test();
   });
-}
+});
 
 // now shut down, remove the JSON database, 
 // start up again, and make sure the data didn't migrate this time
 add_test(function rebuild_again() {
   shutdownManager();
   gExtensionsJSON.remove(true);
   startupManager();