Bug 566626: Add-ons will randomly not appear in the UI. r=robstrong
authorDave Townsend <dtownsend@oxymoronical.com>
Thu, 27 May 2010 09:33:10 -0700
changeset 42870 03bd98ae2bf3deff8e1ab5d89a6ab4dad3209270
parent 42869 373675ded1805ff895c63837786fee0d3f449cc1
child 42871 19b38b92bda36907cbb4360169a15d9ddddbbb3b
push id13489
push userdtownsend@mozilla.com
push dateThu, 27 May 2010 16:36:33 +0000
treeherdermozilla-central@03bd98ae2bf3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrobstrong
bugs566626
milestone1.9.3a5pre
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
Bug 566626: Add-ons will randomly not appear in the UI. r=robstrong
toolkit/mozapps/extensions/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_bug566626.js
--- a/toolkit/mozapps/extensions/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
@@ -2284,16 +2284,58 @@ const FIELDS_ADDON = "internal_id, id, l
                      "pendingUninstall, descriptor, installDate, updateDate, " +
                      "applyBackgroundUpdates, bootstrap";
 
 // A helper function to simply log any errors that occur during async statements.
 function asyncErrorLogger(aError) {
   ERROR("SQL error " + aError.result + ": " + aError.message);
 }
 
+/**
+ * A mozIStorageStatementCallback that will asynchronously build DBAddonInternal
+ * instances from the results it receives. Once the statement has completed
+ * executing and all of the metadata for all of the add-ons has been retrieved
+ * they will be passed as an array to aCallback.
+ *
+ * @param  aCallback
+ *         A callback function to pass the array of DBAddonInternals to
+ */
+function AsyncAddonListCallback(aCallback) {
+  this.callback = aCallback;
+  this.addons = [];
+}
+
+AsyncAddonListCallback.prototype = {
+  callback: null,
+  complete: false,
+  count: 0,
+  addons: null,
+
+  handleResult: function(aResults) {
+    let row = null;
+    while (row = aResults.getNextRow()) {
+      this.count++;
+      let self = this;
+      XPIDatabase.makeAddonFromRowAsync(row, function(aAddon) {
+        self.addons.push(aAddon);
+        if (self.complete && self.addons.length == self.count)
+          self.callback(self.addons);
+      });
+    }
+  },
+
+  handleError: asyncErrorLogger,
+
+  handleCompletion: function(aReason) {
+    this.complete = true;
+    if (this.addons.length == this.count)
+      this.callback(this.addons);
+  }
+};
+
 var XPIDatabase = {
   // true if the database connection has been opened
   initialized: false,
   // A cache of statements that are used and need to be finalized on shutdown
   statementCache: {},
   // A cache of weak referenced DBAddonInternals so we can reuse objects where
   // possible
   addonCache: [],
@@ -2580,19 +2622,19 @@ var XPIDatabase = {
    * Synchronously reads the multi-value locale strings for a locale
    *
    * @param  aLocale
    *         The locale object to read into
    */
   _readLocaleStrings: function XPIDB__readLocaleStrings(aLocale) {
     let stmt = this.getStatement("_readLocaleStrings");
 
-    stmt.params.id = locale.id;
+    stmt.params.id = aLocale.id;
     for (let row in resultRows(stmt)) {
-      if (!(row.type in locale))
+      if (!(row.type in aLocale))
         aLocale[row.type] = [];
       aLocale[row.type].push(row.value);
     }
   },
 
   /**
    * Synchronously reads the locales for an add-on
    *
@@ -2689,17 +2731,17 @@ var XPIDatabase = {
   /**
    * Asynchronously fetches additional metadata for a DBAddonInternal.
    *
    * @param  aAddon
    *         The DBAddonInternal
    * @param  aCallback
    *         The callback to call when the metadata is completely retrieved
    */
-  fetchAddonMetadata: function XPIDB_fetchAddonMetadata(aAddon, aCallback) {
+  fetchAddonMetadata: function XPIDB_fetchAddonMetadata(aAddon) {
     function readLocaleStrings(aLocale, aCallback) {
       let stmt = XPIDatabase.getStatement("_readLocaleStrings");
 
       stmt.params.id = aLocale.id;
       stmt.executeAsync({
         handleResult: function(aResults) {
           let row = null;
           while (row = aResults.getNextRow()) {
@@ -2721,17 +2763,17 @@ var XPIDatabase = {
     function readDefaultLocale() {
       delete aAddon.defaultLocale;
       let stmt = XPIDatabase.getStatement("_getDefaultLocale");
 
       stmt.params.id = aAddon._defaultLocale;
       stmt.executeAsync({
         handleResult: function(aResults) {
           aAddon.defaultLocale = copyRowProperties(aResults.getNextRow(),
-                                                  PROP_LOCALE_SINGLE);
+                                                   PROP_LOCALE_SINGLE);
           aAddon.defaultLocale.id = aAddon._defaultLocale;
         },
 
         handleError: asyncErrorLogger,
 
         handleCompletion: function(aReason) {
           if (aAddon.defaultLocale) {
             readLocaleStrings(aAddon.defaultLocale, readLocales);
@@ -2790,56 +2832,70 @@ var XPIDatabase = {
           let row = null;
           while (row = aResults.getNextRow())
             aAddon.targetApplications.push(copyRowProperties(row, PROP_TARGETAPP));
         },
 
         handleError: asyncErrorLogger,
 
         handleCompletion: function(aReason) {
-          aCallback(aAddon);
+          let callbacks = aAddon._pendingCallbacks;
+          delete aAddon._pendingCallbacks;
+          callbacks.forEach(function(aCallback) {
+            aCallback(aAddon);
+          });
         }
       });
     }
 
     readDefaultLocale();
   },
 
   /**
    * Synchronously makes a DBAddonInternal from a mozIStorageRow or returns one
    * from the cache.
    *
    * @param  aRow
    *         The mozIStorageRow to make the DBAddonInternal from
    * @return a DBAddonInternal
    */
-  makeAddonFromRowAsync: function XPIDB_makeAddonFromRowAsync(aRow) {
+  makeAddonFromRowAsync: function XPIDB_makeAddonFromRowAsync(aRow, aCallback) {
     let internal_id = aRow.getResultByName("internal_id");
     if (this.addonCache[internal_id]) {
       let addon = this.addonCache[internal_id].get();
-      if (addon)
-        return addon;
+      if (addon) {
+        // If metadata is still pending for this instance add our callback to
+        // the list to be called when complete, otherwise pass the addon to
+        // our callback
+        if ("_pendingCallbacks" in addon)
+          addon._pendingCallbacks.push(aCallback);
+        else
+          aCallback(addon);
+        return;
+      }
     }
 
     let addon = new DBAddonInternal();
     addon._internal_id = internal_id;
     let location = aRow.getResultByName("location");
     addon._installLocation = XPIProvider.installLocationsByName[location];
     addon._descriptor = aRow.getResultByName("descriptor");
     copyRowProperties(aRow, PROP_METADATA, addon);
     addon._defaultLocale = aRow.getResultByName("defaultLocale");
     addon.installDate = aRow.getResultByName("installDate");
     addon.updateDate = aRow.getResultByName("updateDate");
     ["visible", "active", "userDisabled", "appDisabled",
      "pendingUninstall", "applyBackgroundUpdates",
      "bootstrap"].forEach(function(aProp) {
       addon[aProp] = aRow.getResultByName(aProp) != 0;
     });
+
     this.addonCache[internal_id] = Components.utils.getWeakReference(addon);
-    return addon;
+    addon._pendingCallbacks = [aCallback];
+    this.fetchAddonMetadata(addon);
   },
 
   /**
    * Synchronously reads 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.
    *
@@ -2876,62 +2932,52 @@ var XPIDatabase = {
    * @param  aCallback
    *         A callback to pass the DBAddonInternal to
    */
   getAddonInLocation: function XPIDB_getAddonInLocation(aId, aLocation, aCallback) {
     let stmt = this.getStatement("getAddonInLocation");
 
     stmt.params.id = aId;
     stmt.params.location = aLocation;
-    stmt.executeAsync({
-      addon: null,
-
-      handleResult: function(aResults) {
-        this.addon = XPIDatabase.makeAddonFromRowAsync(aResults.getNextRow());
-      },
-
-      handleError: asyncErrorLogger,
-
-      handleCompletion: function(aReason) {
-        if (this.addon)
-          XPIDatabase.fetchAddonMetadata(this.addon, aCallback);
-        else
-          aCallback(null);
+    stmt.executeAsync(new AsyncAddonListCallback(function(aAddons) {
+      if (aAddons.length == 0) {
+        aCallback(null);
+        return;
       }
-    });
+      // This should never happen but indicates invalid data in the database if
+      // it does
+      if (aAddons.length > 1)
+        ERROR("Multiple addons with ID " + aId + " found in location " + aLocation);
+      aCallback(aAddons[0]);
+    }));
   },
 
   /**
    * Asynchronously gets the add-on with an 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 stmt = this.getStatement("getVisibleAddonForID");
 
     stmt.params.id = aId;
-    stmt.executeAsync({
-      addon: null,
-
-      handleResult: function(aResults) {
-        this.addon = XPIDatabase.makeAddonFromRowAsync(aResults.getNextRow());
-      },
-
-      handleError: asyncErrorLogger,
-
-      handleCompletion: function(aReason) {
-        if (this.addon)
-          XPIDatabase.fetchAddonMetadata(this.addon, aCallback);
-        else
-          aCallback(null);
+    stmt.executeAsync(new AsyncAddonListCallback(function(aAddons) {
+      if (aAddons.length == 0) {
+        aCallback(null);
+        return;
       }
-    });
+      // This should never happen but indicates invalid data in the database if
+      // it does
+      if (aAddons.length > 1)
+        ERROR("Multiple visible addons with ID " + aId + " found");
+      aCallback(aAddons[0]);
+    }));
   },
 
   /**
    * Asynchronously gets the visible add-ons, optionally restricting by type.
    *
    * @param  aTypes
    *         An array of types to include or null to include all types
    * @param  aCallback
@@ -2952,38 +2998,17 @@ var XPIDatabase = {
       sql += ")";
 
       // Note that binding to index 0 sets the value for the ?1 parameter
       stmt = this.getStatement("getVisibleAddons_" + aTypes.length, sql);
       for (let i = 0; i < aTypes.length; i++)
         stmt.bindStringParameter(i, aTypes[i]);
     }
 
-    let addons = [];
-    stmt.executeAsync({
-      handleResult: function(aResults) {
-        let row = null;
-        while (row = aResults.getNextRow())
-          addons.push(XPIDatabase.makeAddonFromRowAsync(row));
-      },
-
-      handleError: asyncErrorLogger,
-
-      handleCompletion: function(aReason) {
-        let pos = 0;
-        function readNextAddon() {
-          if (pos < addons.length)
-            XPIDatabase.fetchAddonMetadata(addons[pos++], readNextAddon);
-          else
-            aCallback(addons);
-        }
-
-        readNextAddon();
-      }
-    });
+    stmt.executeAsync(new AsyncAddonListCallback(aCallback));
   },
 
   /**
    * Synchronously gets all add-ons of a particular type.
    *
    * @param  aType
    *         The type of add-on to retrieve
    * @return an array of DBAddonInternals
@@ -3022,38 +3047,17 @@ var XPIDatabase = {
 
       // Note that binding to index 0 sets the value for the ?1 parameter
       stmt = this.getStatement("getVisibleAddonsWithPendingOperations_" +
                                aTypes.length, sql);
       for (let i = 0; i < aTypes.length; i++)
         stmt.bindStringParameter(i, aTypes[i]);
     }
 
-    let addons = [];
-    stmt.executeAsync({
-      handleResult: function(aResults) {
-        let row = null;
-        while (row = aResults.getNextRow())
-          addons.push(XPIDatabase.makeAddonFromRowAsync(row));
-      },
-
-      handleError: asyncErrorLogger,
-
-      handleCompletion: function(aReason) {
-        let pos = 0;
-        function readNextAddon() {
-          if (pos < addons.length)
-            XPIDatabase.fetchAddonMetadata(addons[pos++], readNextAddon);
-          else
-            aCallback(addons);
-        }
-
-        readNextAddon();
-      }
-    });
+    stmt.executeAsync(new AsyncAddonListCallback(aCallback));
   },
 
   /**
    * Synchronously gets all add-ons in the database.
    *
    * @return  an array of DBAddonInternals
    */
   getAddons: function XPIDB_getAddons() {
new file mode 100644
--- /dev/null
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_bug566626.js
@@ -0,0 +1,114 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+// This verifies that multiple calls to the async API return fully formed
+// add-ons
+
+var addon1 = {
+  id: "addon1@tests.mozilla.org",
+  version: "1.0",
+  name: "Test 1",
+  targetApplications: [{
+    id: "xpcshell@tests.mozilla.org",
+    minVersion: "1",
+    maxVersion: "1"
+  }]
+};
+
+const profileDir = gProfD.clone();
+profileDir.append("extensions");
+
+var gAddon;
+
+// Sets up the profile by installing an add-on.
+function run_test() {
+  do_test_pending();
+
+  createAppInfo("xpcshell@tests.mozilla.org", "XPCShell", "1", "1.9.2");
+
+  var dest = profileDir.clone();
+  dest.append("addon1@tests.mozilla.org");
+  writeInstallRDFToDir(addon1, dest);
+
+  startupManager(1);
+
+  run_test_1();
+}
+
+// Verifies that multiple calls to get an add-on at various stages of execution
+// return an add-on with a valid name.
+function run_test_1() {
+  var count = 0;
+
+  AddonManager.getAddonByID("addon1@tests.mozilla.org", function(a1) {
+    do_check_neq(a1, null);
+    do_check_eq(a1.name, "Test 1");
+
+    if (count == 0)
+      gAddon = a1;
+    else
+      do_check_eq(a1, gAddon);
+    count++;
+    if (count == 4)
+      run_test_2();
+  });
+
+  AddonManager.getAddonByID("addon1@tests.mozilla.org", function(a1) {
+    do_check_neq(a1, null);
+    do_check_eq(a1.name, "Test 1");
+
+    if (count == 0)
+      gAddon = a1;
+    else
+      do_check_eq(a1, gAddon);
+    count++;
+    if (count == 4)
+      run_test_2();
+  });
+
+  do_execute_soon(function() {
+    AddonManager.getAddonByID("addon1@tests.mozilla.org", function(a1) {
+      do_check_neq(a1, null);
+      do_check_eq(a1.name, "Test 1");
+
+      if (count == 0)
+        gAddon = a1;
+      else
+        do_check_eq(a1, gAddon);
+      count++;
+      if (count == 4)
+        run_test_2();
+    });
+  });
+
+  do_execute_soon(function() {
+    do_execute_soon(function() {
+      AddonManager.getAddonByID("addon1@tests.mozilla.org", function(a1) {
+        do_check_neq(a1, null);
+        do_check_eq(a1.name, "Test 1");
+
+        if (count == 0)
+          gAddon = a1;
+        else
+          do_check_eq(a1, gAddon);
+        count++;
+        if (count == 4)
+          run_test_2();
+      });
+    });
+  });
+}
+
+// Verifies that a subsequent call gets the same add-on from the cache
+function run_test_2() {
+  AddonManager.getAddonByID("addon1@tests.mozilla.org", function(a1) {
+    do_check_neq(a1, null);
+    do_check_eq(a1.name, "Test 1");
+
+    do_check_eq(a1, gAddon);
+
+    do_test_finished();
+  });
+
+}