Bug 987512: Part 1 - Support both promises and callbacks in the AddonManager API. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 15 Dec 2016 10:39:33 -1000
changeset 450036 02e6647866cdfcc0b2866e2da93e2a1a419d90db
parent 449919 756b4447cae3fa1877d1252526384988d9e570f2
child 450037 577848861bb5d7eef6d561c1cdd66ccccd50868d
push id38748
push usermaglione.k@gmail.com
push dateThu, 15 Dec 2016 22:03:52 +0000
reviewersrhelmer
bugs987512
milestone53.0a1
Bug 987512: Part 1 - Support both promises and callbacks in the AddonManager API. r?rhelmer MozReview-Commit-ID: 4pXVzfiZoaz
toolkit/mozapps/extensions/AddonManager.jsm
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -200,16 +200,45 @@ function safeCall(aCallback, ...aArgs) {
  */
 function makeSafe(aCallback) {
   return function(...aArgs) {
     safeCall(aCallback, ...aArgs);
   }
 }
 
 /**
+ * Given a promise and an optional callback, either:
+ *
+ * 1) Returns the promise, if no callback was provided, or,
+ * 2) Calls the callback with the promise resolution value, and reports
+ *    any errors.
+ *
+ * @param {Promise} promise
+ *        The promise to return, or report to the callback function.
+ * @param {function | null} callback
+ *        The optional callback function to call with the promise
+ *        resolution.
+ * @returns {Promise?}
+ */
+function promiseOrCallback(promise, callback) {
+  if (!callback)
+    return promise;
+
+  if (typeof callback !== "function")
+    throw Components.Exception("aCallback must be a function",
+                               Cr.NS_ERROR_INVALID_ARG);
+
+  promise.then(makeSafe(callback)).catch(error => {
+    logger.error(error);
+  });
+
+  return undefined;
+}
+
+/**
  * Report an exception thrown by a provider API method.
  */
 function reportProviderError(aProvider, aMethod, aError) {
   let method = `provider ${providerName(aProvider)}.${aMethod}`;
   AddonManagerPrivate.recordException("AMI", method, aError);
   logger.error("Exception calling " + method, aError);
 }
 
@@ -335,62 +364,16 @@ function webAPIForAddon(addon) {
   // A few properties are computed for a nicer API
   result.isEnabled = !addon.userDisabled;
   result.canUninstall = Boolean(addon.permissions & AddonManager.PERM_CAN_UNINSTALL);
 
   return result;
 }
 
 /**
- * A helper class to repeatedly call a listener with each object in an array
- * optionally checking whether the object has a method in it.
- *
- * @param  aObjects
- *         The array of objects to iterate through
- * @param  aMethod
- *         An optional method name, if not null any objects without this method
- *         will not be passed to the listener
- * @param  aListener
- *         A listener implementing nextObject and noMoreObjects methods. The
- *         former will be called with the AsyncObjectCaller as the first
- *         parameter and the object as the second. noMoreObjects will be passed
- *         just the AsyncObjectCaller
- */
-function AsyncObjectCaller(aObjects, aMethod, aListener) {
-  this.objects = [...aObjects];
-  this.method = aMethod;
-  this.listener = aListener;
-
-  this.callNext();
-}
-
-AsyncObjectCaller.prototype = {
-  objects: null,
-  method: null,
-  listener: null,
-
-  /**
-   * Passes the next object to the listener or calls noMoreObjects if there
-   * are none left.
-   */
-  callNext: function() {
-    if (this.objects.length == 0) {
-      this.listener.noMoreObjects(this);
-      return;
-    }
-
-    let object = this.objects.shift();
-    if (!this.method || this.method in object)
-      this.listener.nextObject(this, object);
-    else
-      this.callNext();
-  }
-};
-
-/**
  * Listens for a browser changing origin and cancels the installs that were
  * started by it.
  */
 function BrowserListener(aBrowser, aInstallingPrincipal, aInstalls) {
   this.browser = aBrowser;
   this.principal = aInstallingPrincipal;
   this.installs = aInstalls;
   this.installCount = aInstalls.length;
@@ -1441,17 +1424,17 @@ var AddonManagerInternal = {
 
       Services.obs.notifyObservers(null, "addons-background-update-start", null);
 
       if (this.updateEnabled) {
         let scope = {};
         Components.utils.import("resource://gre/modules/LightweightThemeManager.jsm", scope);
         scope.LightweightThemeManager.updateCurrentTheme();
 
-        let allAddons = yield new Promise((resolve, reject) => this.getAllAddons(resolve));
+        let allAddons = yield this.getAllAddons();
 
         // Repopulate repository cache first, to ensure compatibility overrides
         // are up to date before checking for addon updates.
         yield AddonRepository.backgroundUpdateCheck();
 
         // Keep track of all the async add-on updates happening in parallel
         let updates = [];
 
@@ -1818,40 +1801,30 @@ var AddonManagerInternal = {
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     this.callProviders("updateAddonAppDisabledStates");
   },
 
   /**
    * Notifies all providers that the repository has updated its data for
    * installed add-ons.
-   *
-   * @param  aCallback
-   *         Function to call when operation is complete.
    */
-  updateAddonRepositoryData: function(aCallback) {
+  updateAddonRepositoryData: function() {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    new AsyncObjectCaller(this.providers, "updateAddonRepositoryData", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "updateAddonRepositoryData",
-                          aCaller.callNext.bind(aCaller));
-      },
-      noMoreObjects: function(aCaller) {
-        safeCall(aCallback);
-        // only tests should care about this
-        Services.obs.notifyObservers(null, "TEST:addon-repository-data-updated", null);
+    return Task.spawn(function*() {
+      for (let provider of this.providers) {
+        yield promiseCallProvider(provider, "updateAddonRepositoryData");
       }
-    });
+
+      // only tests should care about this
+      Services.obs.notifyObservers(null, "TEST:addon-repository-data-updated", null);
+    }.bind(this));
   },
 
   /**
    * Asynchronously gets an AddonInstall for a URL.
    *
    * @param  aUrl
    *         The string represenation of the URL the add-on is located at
    * @param  aCallback
@@ -1928,110 +1901,87 @@ var AddonManagerInternal = {
     safeCall(aCallback, null);
   },
 
   /**
    * Asynchronously gets an AddonInstall for an nsIFile.
    *
    * @param  aFile
    *         The nsIFile where the add-on is located
-   * @param  aCallback
-   *         A callback to pass the AddonInstall to
    * @param  aMimetype
    *         An optional mimetype hint for the add-on
    * @throws if the aFile or aCallback arguments are not specified
    */
-  getInstallForFile: function(aFile, aCallback, aMimetype) {
+  getInstallForFile: function(aFile, aMimetype) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (!(aFile instanceof Ci.nsIFile))
       throw Components.Exception("aFile must be a nsIFile",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
     if (aMimetype && typeof aMimetype != "string")
       throw Components.Exception("aMimetype must be a string or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    new AsyncObjectCaller(this.providers, "getInstallForFile", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "getInstallForFile", aFile,
-                          function(aInstall) {
-          if (aInstall)
-            safeCall(aCallback, aInstall);
-          else
-            aCaller.callNext();
-        });
-      },
-
-      noMoreObjects: function(aCaller) {
-        safeCall(aCallback, null);
+    return Task.spawn(function*() {
+      for (let provider of this.providers) {
+        let install = yield promiseCallProvider(
+          provider, "getInstallForFile", aFile);
+
+        if (install)
+          return install;
       }
-    });
+
+      return null;
+    }.bind(this));
   },
 
   /**
    * Asynchronously gets all current AddonInstalls optionally limiting to a list
    * of types.
    *
    * @param  aTypes
    *         An optional array of types to retrieve. Each type is a string name
-   * @param  aCallback
-   *         A callback which will be passed an array of AddonInstalls
    * @throws If the aCallback argument is not specified
    */
-  getInstallsByTypes: function(aTypes, aCallback) {
+  getInstallsByTypes: function(aTypes) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (aTypes && !Array.isArray(aTypes))
       throw Components.Exception("aTypes must be an array or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    let installs = [];
-
-    new AsyncObjectCaller(this.providers, "getInstallsByTypes", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "getInstallsByTypes", aTypes,
-                          function(aProviderInstalls) {
-          if (aProviderInstalls) {
-            installs = installs.concat(aProviderInstalls);
-          }
-          aCaller.callNext();
-        });
-      },
-
-      noMoreObjects: function(aCaller) {
-        safeCall(aCallback, installs);
+    return Task.spawn(function*() {
+      let installs = [];
+
+      for (let provider of this.providers) {
+        let providerInstalls = yield promiseCallProvider(
+          provider, "getInstallsByTypes", aTypes);
+
+        if (providerInstalls)
+          installs.push(...providerInstalls);
       }
-    });
+
+      return installs;
+    }.bind(this));
   },
 
   /**
    * Asynchronously gets all current AddonInstalls.
-   *
-   * @param  aCallback
-   *         A callback which will be passed an array of AddonInstalls
    */
-  getAllInstalls: function(aCallback) {
+  getAllInstalls: function() {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
-    this.getInstallsByTypes(null, aCallback);
+    return this.getInstallsByTypes(null);
   },
 
   /**
    * Synchronously map a URI to the corresponding Addon ID.
    *
    * Mappable URIs are limited to in-application resources belonging to the
    * add-on, such as Javascript compartments, XUL windows, XBL bindings, etc.
    * but do not include URIs from meta data, such as the add-on homepage.
@@ -2465,49 +2415,38 @@ var AddonManagerInternal = {
     });
   },
 
   /**
    * Asynchronously get an add-on with a specific Sync GUID.
    *
    * @param  aGUID
    *         String GUID of add-on to retrieve
-   * @param  aCallback
-   *         The callback to pass the retrieved add-on to.
-   * @throws if the aGUID or aCallback arguments are not specified
+   * @throws if the aGUID argument is not specified
    */
-  getAddonBySyncGUID: function(aGUID, aCallback) {
+  getAddonBySyncGUID: function(aGUID) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (!aGUID || typeof aGUID != "string")
       throw Components.Exception("aGUID must be a non-empty string",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    new AsyncObjectCaller(this.providers, "getAddonBySyncGUID", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "getAddonBySyncGUID", aGUID,
-                          function(aAddon) {
-          if (aAddon) {
-            safeCall(aCallback, aAddon);
-          } else {
-            aCaller.callNext();
-          }
-        });
-      },
-
-      noMoreObjects: function(aCaller) {
-        safeCall(aCallback, null);
+    return Task.spawn(function*() {
+      for (let provider of this.providers) {
+        let addon = yield promiseCallProvider(
+          provider, "getAddonBySyncGUID", aGUID);
+
+        if (addon)
+          return addon;
       }
-    });
+
+      return null;
+    }.bind(this));
   },
 
   /**
    * Asynchronously gets an array of add-ons.
    *
    * @param  aIDs
    *         The array of IDs to retrieve
    * @return {Promise}
@@ -2528,110 +2467,81 @@ var AddonManagerInternal = {
     return Promise.all(promises);
   },
 
   /**
    * Asynchronously gets add-ons of specific types.
    *
    * @param  aTypes
    *         An optional array of types to retrieve. Each type is a string name
-   * @param  aCallback
-   *         The callback to pass an array of Addons to.
-   * @throws if the aCallback argument is not specified
    */
-  getAddonsByTypes: function(aTypes, aCallback) {
+  getAddonsByTypes: function(aTypes) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (aTypes && !Array.isArray(aTypes))
       throw Components.Exception("aTypes must be an array or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    let addons = [];
-
-    new AsyncObjectCaller(this.providers, "getAddonsByTypes", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "getAddonsByTypes", aTypes,
-                          function(aProviderAddons) {
-          if (aProviderAddons) {
-            addons = addons.concat(aProviderAddons);
-          }
-          aCaller.callNext();
-        });
-      },
-
-      noMoreObjects: function(aCaller) {
-        safeCall(aCallback, addons);
+    return Task.spawn(function*() {
+      let addons = [];
+
+      for (let provider of this.providers) {
+        let providerAddons = yield promiseCallProvider(
+          provider, "getAddonsByTypes", aTypes);
+
+        if (providerAddons)
+          addons.push(...providerAddons);
       }
-    });
+
+      return addons;
+    }.bind(this));
   },
 
   /**
    * Asynchronously gets all installed add-ons.
-   *
-   * @param  aCallback
-   *         A callback which will be passed an array of Addons
    */
-  getAllAddons: function(aCallback) {
+  getAllAddons: function() {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    this.getAddonsByTypes(null, aCallback);
+    return this.getAddonsByTypes(null);
   },
 
   /**
    * Asynchronously gets add-ons that have operations waiting for an application
    * restart to complete.
    *
    * @param  aTypes
    *         An optional array of types to retrieve. Each type is a string name
-   * @param  aCallback
-   *         The callback to pass the array of Addons to
-   * @throws if the aCallback argument is not specified
    */
-  getAddonsWithOperationsByTypes: function(aTypes, aCallback) {
+  getAddonsWithOperationsByTypes: function(aTypes) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (aTypes && !Array.isArray(aTypes))
       throw Components.Exception("aTypes must be an array or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    let addons = [];
-
-    new AsyncObjectCaller(this.providers, "getAddonsWithOperationsByTypes", {
-      nextObject: function(aCaller, aProvider) {
-        callProviderAsync(aProvider, "getAddonsWithOperationsByTypes", aTypes,
-                          function(aProviderAddons) {
-          if (aProviderAddons) {
-            addons = addons.concat(aProviderAddons);
-          }
-          aCaller.callNext();
-        });
-      },
-
-      noMoreObjects: function(caller) {
-        safeCall(aCallback, addons);
+    return Task.spawn(function*() {
+      let addons = [];
+
+      for (let provider of this.providers) {
+        let providerAddons = yield promiseCallProvider(
+          provider, "getAddonsWithOperationsByTypes", aTypes);
+
+        if (providerAddons)
+          addons.push(...providerAddons);
       }
-    });
+
+      return addons;
+    }.bind(this));
   },
 
   /**
    * Adds a new AddonManagerListener if the listener is not already registered.
    *
    * @param  aListener
    *         The listener to add
    */
@@ -3068,17 +2978,19 @@ this.AddonManagerPrivate = {
     AddonManagerInternal.notifyAddonChanged(aID, aType, aPendingRestart);
   },
 
   updateAddonAppDisabledStates: function() {
     AddonManagerInternal.updateAddonAppDisabledStates();
   },
 
   updateAddonRepositoryData: function(aCallback) {
-    AddonManagerInternal.updateAddonRepositoryData(aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.updateAddonRepositoryData(),
+      aCallback);
   },
 
   callInstallListeners: function(...aArgs) {
     return AddonManagerInternal.callInstallListeners.apply(AddonManagerInternal,
                                                            aArgs);
   },
 
   callAddonListeners: function(...aArgs) {
@@ -3434,74 +3346,78 @@ this.AddonManager = {
   getInstallForURL: function(aUrl, aCallback, aMimetype,
                                                  aHash, aName, aIcons,
                                                  aVersion, aBrowser) {
     AddonManagerInternal.getInstallForURL(aUrl, aCallback, aMimetype, aHash,
                                           aName, aIcons, aVersion, aBrowser);
   },
 
   getInstallForFile: function(aFile, aCallback, aMimetype) {
-    AddonManagerInternal.getInstallForFile(aFile, aCallback, aMimetype);
+    AddonManagerInternal.getInstallForFile(aFile, aMimetype).then(aCallback);
   },
 
   /**
    * Gets an array of add-on IDs that changed during the most recent startup.
    *
    * @param  aType
    *         The type of startup change to get
    * @return An array of add-on IDs
    */
   getStartupChanges: function(aType) {
     if (!(aType in AddonManagerInternal.startupChanges))
       return [];
     return AddonManagerInternal.startupChanges[aType].slice(0);
   },
 
   getAddonByID: function(aID, aCallback) {
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    AddonManagerInternal.getAddonByID(aID)
-                        .then(makeSafe(aCallback))
-                        .catch(logger.error);
+    return promiseOrCallback(
+      AddonManagerInternal.getAddonByID(aID),
+      aCallback);
   },
 
   getAddonBySyncGUID: function(aGUID, aCallback) {
-    AddonManagerInternal.getAddonBySyncGUID(aGUID, aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getAddonBySyncGUID(aGUID),
+      aCallback);
   },
 
   getAddonsByIDs: function(aIDs, aCallback) {
-    if (typeof aCallback != "function")
-      throw Components.Exception("aCallback must be a function",
-                                 Cr.NS_ERROR_INVALID_ARG);
-
-    AddonManagerInternal.getAddonsByIDs(aIDs)
-                        .then(makeSafe(aCallback))
-                        .catch(logger.error);
+    return promiseOrCallback(
+      AddonManagerInternal.getAddonsByIDs(aIDs),
+      aCallback);
   },
 
   getAddonsWithOperationsByTypes: function(aTypes, aCallback) {
-    AddonManagerInternal.getAddonsWithOperationsByTypes(aTypes, aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getAddonsWithOperationsByTypes(aTypes),
+      aCallback);
   },
 
   getAddonsByTypes: function(aTypes, aCallback) {
-    AddonManagerInternal.getAddonsByTypes(aTypes, aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getAddonsByTypes(aTypes),
+      aCallback);
   },
 
   getAllAddons: function(aCallback) {
-    AddonManagerInternal.getAllAddons(aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getAllAddons(),
+      aCallback);
   },
 
   getInstallsByTypes: function(aTypes, aCallback) {
-    AddonManagerInternal.getInstallsByTypes(aTypes, aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getInstallsByTypes(aTypes),
+      aCallback);
   },
 
   getAllInstalls: function(aCallback) {
-    AddonManagerInternal.getAllInstalls(aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getAllInstalls(),
+      aCallback);
   },
 
   mapURIToAddonID: function(aURI) {
     return AddonManagerInternal.mapURIToAddonID(aURI);
   },
 
   isInstallEnabled: function(aType) {
     return AddonManagerInternal.isInstallEnabled(aType);