Backed out changeset 40c48d65e382 (bug 853388)
authorTim Taubert <ttaubert@mozilla.com>
Fri, 09 Aug 2013 04:20:03 +0200
changeset 154744 bc2cb7f532f2fee494a5c9ef68407f9c2c0a0b52
parent 154743 960ecd36a7147c7d566eb0c9986c15ace3f1c425
child 154745 a64c1ad76ba1b5e3b2631f86c7ae679eee78e9e3
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)
bugs853388
milestone26.0a1
backs out40c48d65e38287193f978193268b10cbe9a051f1
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
Backed out changeset 40c48d65e382 (bug 853388)
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,18 +3213,19 @@ 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,18 +15,16 @@ 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];
@@ -156,30 +154,16 @@ 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
@@ -386,42 +370,16 @@ 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.
@@ -447,22 +405,16 @@ 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);
@@ -472,37 +424,39 @@ 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" and "saveChanges never called" cases
-    if (!this._deferredSave) {
+    // handle the "in memory only" case
+    if (this.lockedDatabase) {
       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 [, addon] of this.addonDB) {
+    for (let [key, addon] of this.addonDB) {
       addons.push(addon);
     }
     let toSave = {
       schemaVersion: DB_SCHEMA,
       addons: addons
     };
     return toSave;
   },
@@ -530,35 +484,37 @@ this.XPIDatabase = {
     }
     LOG("Migrating data from sqlite");
     let migrateData = this.getMigrateDataFromDatabase(connection);
     connection.close();
     return migrateData;
   },
 
   /**
-   * Synchronously opens and reads the database file, upgrading from old
+   * 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)
    */
-  syncLoadDB: function XPIDB_syncLoadDB(aRebuildOnError) {
-    // XXX TELEMETRY report synchronous opens (startup time) vs. delayed opens
+  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;
     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);
@@ -569,174 +525,101 @@ 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);
         }
-        this.parseDB(data, aRebuildOnError);
+        // 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;
       }
       catch(e) {
-        ERROR("Failed to load XPI JSON data from profile", 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);
       }
       finally {
         if (cstream)
           cstream.close();
       }
     }
     catch (e) {
       if (e.result == Cr.NS_ERROR_FILE_NOT_FOUND) {
-        this.upgradeDB(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);
       }
       else {
-        this.rebuildUnreadableDB(e, aRebuildOnError);
+        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);
       }
     }
     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);
-  },
 
-  /**
-   * 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);
-    }
-  },
+    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();
+    // 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;
     }
-
-    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
@@ -762,22 +645,30 @@ 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 persistent descriptors for the directories
+   * @return an array of persisitent descriptors for the directories
    */
   getActiveBundles: function XPIDB_getActiveBundles() {
     let bundles = [];
 
     let addonsList = FileUtils.getFile(KEY_PROFILEDIR, [FILE_XPI_ADDONS_LIST],
                                        true);
 
     if (!addonsList.exists())
@@ -986,21 +877,37 @@ 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
+          // Clear out the cached addons data loaded from JSON and recreate
+          // the getter to allow database re-loads during testing.
           delete this.addonDB;
-          delete this._dbPromise;
+          Object.defineProperty(this, "addonDB", {
+            get: function addonsGetter() {
+              this.openConnection(true);
+              return this.addonDB;
+            },
+            configurable: true
+          });
           // 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 {
@@ -1008,235 +915,218 @@ 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. Only called from XPIProvider.processFileChanges, when
-   * the database should already be loaded.
+   * longer exists.
    *
    * @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;
   },
 
   /**
-   * Asynchronously list all addons that match the filter function
+   * 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
-   * @param  aCallback
-   *         Called back with an array of addons matching aFilter
-   *         or an empty array if none match
+   * @return an array of DBAddonInternals
    */
-  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)([]);
-        });
+  _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;
   },
 
   /**
-   * (Possibly asynchronously) get the first addon that matches the filter function
+   * Find 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
-   * @param  aCallback
-   *         Called back with the addon, or null if no matching addon is found
+   * @return The first DBAddonInternal for which the filter returns true
    */
-  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);
-        });
+  _findAddon: function XPIDB_findAddon(aFilter) {
+    if (!this.addonDB)
+      return null;
+
+    for (let [key, addon] of this.addonDB) {
+      if (aFilter(addon)) {
+        return addon;
+      }
+    }
+
+    return 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 _filterDB(this.addonDB, aAddon => (aAddon.location == aLocation));
+    return this._listAddons(function inLocation(aAddon) {return (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) {
-    this.asyncLoadDB().then(
-        addonDB => getRepositoryAddon(addonDB.get(aLocation + ":" + aId),
-                                      safeCallback(aCallback)));
+    getRepositoryAddon(this.addonDB.get(aLocation + ":" + aId), aCallback);
   },
 
   /**
-   * Asynchronously gets the add-on with the specified ID that is visible.
+   * Asynchronously gets the add-on with an ID that is visible.
+   * XXX IRVING sync
    *
    * @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) {
-    this.getAddon(aAddon => ((aAddon.id == aId) && aAddon.visible),
-                  aCallback);
+    let addon = this._findAddon(function visibleID(aAddon) {return ((aAddon.id == aId) && aAddon.visible)});
+    getRepositoryAddon(addon, 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) {
-    this.getAddonList(aAddon => (aAddon.visible &&
-                                 (!aTypes || (aTypes.length == 0) ||
-                                  (aTypes.indexOf(aAddon.type) > -1))),
-                      aCallback);
+    let addons = this._listAddons(function visibleType(aAddon) {
+      return (aAddon.visible && (!aTypes || (aTypes.length == 0) || (aTypes.indexOf(aAddon.type) > -1)))
+    });
+    asyncMap(addons, getRepositoryAddon, 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) {
-    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));
+    return this._listAddons(function byType(aAddon) { return 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) {
-    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));
+    return this._findAddon(function visibleInternalName(aAddon) {
+      return (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) {
 
-    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);
+    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);
   },
 
   /**
    * 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) {
-    this.getAddon(aAddon => aAddon.syncGUID == aGUID,
-                  aCallback);
+    let addon = this._findAddon(function bySyncGUID(aAddon) { return aAddon.syncGUID == aGUID; });
+    getRepositoryAddon(addon, 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() {
-    if (!this.addonDB) {
-      return [];
-    }
-    return _filterDB(this.addonDB, aAddon => true);
+    return this._listAddons(function(aAddon) {return 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 (!this.addonDB) {
-      // XXX telemetry. Should never happen on platforms that have a default theme
-      this.syncLoadDB(false);
-    }
+    // 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);
 
     let newAddon = new DBAddonInternal(aAddon);
     newAddon.descriptor = aDescriptor;
     this.addonDB.set(newAddon._key, newAddon);
     if (newAddon.visible) {
       this.makeAddonVisible(newAddon);
     }
 
@@ -1287,17 +1177,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 [, otherAddon] of this.addonDB) {
+    for (let [key, 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();
   },
@@ -1314,30 +1204,29 @@ 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 = _findAddon(this.addonDB, excludeSyncGUID);
+    let otherAddon = this._findAddon(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();
   },
 
@@ -1370,47 +1259,43 @@ 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 [, addon] of this.addonDB) {
+    for (let [key, 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 = _filterDB(
-      this.addonDB,
-      aAddon => aAddon.active && !aAddon.bootstrap && (aAddon.type != "theme"));
+    let activeAddons = this._listAddons(function active(aAddon) {
+      return 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;
 
@@ -1420,23 +1305,22 @@ this.XPIDatabase = {
 
     let dssEnabled = false;
     try {
       dssEnabled = Services.prefs.getBoolPref(PREF_EM_DSS_ENABLED);
     } catch (e) {}
 
     let themes = [];
     if (dssEnabled) {
-      themes = _filterDB(this.addonDB, aAddon => aAddon.type == "theme");
+      themes = this._listAddons(function isTheme(aAddon){ return aAddon.type == "theme"; });
     }
     else {
-      let activeTheme = _findAddon(
-        this.addonDB,
-        aAddon => (aAddon.type == "theme") &&
-                  (aAddon.internalName == XPIProvider.selectedSkin));
+      let activeTheme = this._findAddon(function isSelected(aAddon) {
+        return ((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,50 +40,49 @@ 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 = {};
 
-  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;
+  [["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;
+  });
   stmt.finalize();
 
-  db.schemaVersion = 14;
+  db.schemaVersion = 15;
   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();