Bug 987512: Part 3 - Support promises in getInstallForFile and getInstallForURL. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 15 Dec 2016 11:03:35 -1000
changeset 450038 ff77a4cd1bb0a493714b31852170fe4cb9d241db
parent 450037 577848861bb5d7eef6d561c1cdd66ccccd50868d
child 450039 0a7a85c2d95ebc63b8465224e8b6289bb52c5339
push id38748
push usermaglione.k@gmail.com
push dateThu, 15 Dec 2016 22:03:52 +0000
reviewersrhelmer
bugs987512
milestone53.0a1
Bug 987512: Part 3 - Support promises in getInstallForFile and getInstallForURL. r?rhelmer MozReview-Commit-ID: IgLDBYhAwcg
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -1502,20 +1502,19 @@ var AddonManagerInternal = {
           // AUC.checkForUpdates already logged the error
         }
 
         // Check that we have a hotfix update, and it's newer than the one we already
         // have installed (if any)
         if (update) {
           if (Services.vc.compare(hotfixVersion, update.version) < 0) {
             logger.debug("Downloading hotfix version " + update.version);
-            let aInstall = yield new Promise((resolve, reject) =>
-              AddonManager.getInstallForURL(update.updateURL, resolve,
-                "application/x-xpinstall", update.updateHash, null,
-                null, update.version));
+            let aInstall = yield AddonManagerInternal.getInstallForURL(
+                update.updateURL, "application/x-xpinstall", update.updateHash,
+                null, null, update.version);
 
             aInstall.addListener({
               onDownloadEnded: function(aInstall) {
                 if (aInstall.addon.id != hotfixID) {
                   logger.warn("The downloaded hotfix add-on did not have the " +
                               "expected ID and so will not be installed.");
                   aInstall.cancel();
                   return;
@@ -1822,46 +1821,40 @@ var AddonManagerInternal = {
     }.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
-   *         A callback to pass the AddonInstall to
    * @param  aMimetype
    *         The mimetype of the add-on
    * @param  aHash
    *         An optional hash of the add-on
    * @param  aName
    *         An optional placeholder name while the add-on is being downloaded
    * @param  aIcons
    *         Optional placeholder icons while the add-on is being downloaded
    * @param  aVersion
    *         An optional placeholder version while the add-on is being downloaded
    * @param  aBrowser
    *         An optional <browser> element for download permissions prompts.
    * @throws if the aUrl, aCallback or aMimetype arguments are not specified
    */
-  getInstallForURL: function(aUrl, aCallback, aMimetype, aHash, aName,
+  getInstallForURL: function(aUrl, aMimetype, aHash, aName,
                              aIcons, aVersion, aBrowser) {
     if (!gStarted)
       throw Components.Exception("AddonManager is not initialized",
                                  Cr.NS_ERROR_NOT_INITIALIZED);
 
     if (!aUrl || typeof aUrl != "string")
       throw Components.Exception("aURL 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);
-
     if (!aMimetype || typeof aMimetype != "string")
       throw Components.Exception("aMimetype must be a non-empty string",
                                  Cr.NS_ERROR_INVALID_ARG);
 
     if (aHash && typeof aHash != "string")
       throw Components.Exception("aHash must be a string or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
@@ -1878,32 +1871,29 @@ var AddonManagerInternal = {
     } else {
       aIcons = {};
     }
 
     if (aVersion && typeof aVersion != "string")
       throw Components.Exception("aVersion must be a string or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    if (aBrowser && (!(aBrowser instanceof Ci.nsIDOMElement)))
+    if (aBrowser && !(aBrowser instanceof Ci.nsIDOMElement))
       throw Components.Exception("aBrowser must be a nsIDOMElement or null",
                                  Cr.NS_ERROR_INVALID_ARG);
 
-    let providers = [...this.providers];
-    for (let provider of providers) {
+    for (let provider of this.providers) {
       if (callProvider(provider, "supportsMimetype", false, aMimetype)) {
-        callProviderAsync(provider, "getInstallForURL",
-                          aUrl, aHash, aName, aIcons, aVersion, aBrowser,
-                          function  getInstallForURL_safeCall(aInstall) {
-          safeCall(aCallback, aInstall);
-        });
-        return;
+        return promiseCallProvider(
+          provider, "getInstallForURL", aUrl, aHash, aName, aIcons,
+          aVersion, aBrowser);
       }
     }
-    safeCall(aCallback, null);
+
+    return Promise.resolve(null);
   },
 
   /**
    * Asynchronously gets an AddonInstall for an nsIFile.
    *
    * @param  aFile
    *         The nsIFile where the add-on is located
    * @param  aMimetype
@@ -2835,36 +2825,33 @@ var AddonManagerInternal = {
         if (Services.prefs.getBoolPref(PREF_WEBAPI_TESTING)
             && WEBAPI_TEST_INSTALL_HOSTS.includes(host)) {
           return;
         }
 
         throw new Error(`Install from ${host} not permitted`);
       }
 
-      return new Promise((resolve, reject) => {
-        try {
-          checkInstallUrl(options.url);
-        } catch (err) {
-          reject({message: err.message});
-          return;
-        }
-
-        let newInstall = install => {
-          let id = this.nextInstall++;
-          let listener = this.makeListener(id, target.messageManager);
-          install.addListener(listener);
-
-          this.installs.set(id, {install, target, listener});
-
-          let result = {id};
-          this.copyProps(install, result);
-          resolve(result);
-        };
-        AddonManager.getInstallForURL(options.url, newInstall, "application/x-xpinstall", options.hash);
+      try {
+        checkInstallUrl(options.url);
+      } catch (err) {
+        return Promise.reject({message: err.message});
+      }
+
+      return AddonManagerInternal.getInstallForURL(options.url, "application/x-xpinstall",
+                                                   options.hash).then(install => {
+        let id = this.nextInstall++;
+        let listener = this.makeListener(id, target.messageManager);
+        install.addListener(listener);
+
+        this.installs.set(id, {install, target, listener});
+
+        let result = {id};
+        this.copyProps(install, result);
+        return result;
       });
     },
 
     addonUninstall(target, id) {
       return new Promise(resolve => {
         AddonManager.getAddonByID(id, addon => {
           if (!addon) {
             resolve(false);
@@ -3341,22 +3328,26 @@ this.AddonManager = {
 
   errorToString(err) {
     return err ? this._errorToString.get(err) : null;
   },
 
   getInstallForURL: function(aUrl, aCallback, aMimetype,
                                                  aHash, aName, aIcons,
                                                  aVersion, aBrowser) {
-    AddonManagerInternal.getInstallForURL(aUrl, aCallback, aMimetype, aHash,
-                                          aName, aIcons, aVersion, aBrowser);
+    return promiseOrCallback(
+      AddonManagerInternal.getInstallForURL(aUrl, aMimetype, aHash,
+                                            aName, aIcons, aVersion, aBrowser),
+      aCallback);
   },
 
   getInstallForFile: function(aFile, aCallback, aMimetype) {
-    AddonManagerInternal.getInstallForFile(aFile, aMimetype).then(aCallback);
+    return promiseOrCallback(
+      AddonManagerInternal.getInstallForFile(aFile, aMimetype),
+      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
--- a/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonTestUtils.jsm
@@ -1044,27 +1044,27 @@ var AddonTestUtils = {
    *        The file to install
    * @param {boolean} [ignoreIncompatible = false]
    *        Optional parameter to ignore add-ons that are incompatible
    *        with the application
    * @returns {Promise}
    *        Resolves when the install has completed.
    */
   promiseInstallFile(file, ignoreIncompatible = false) {
-    return new Promise((resolve, reject) => {
-      AddonManager.getInstallForFile(file, install => {
-        if (!install)
-          reject(new Error(`No AddonInstall created for ${file.path}`));
-        else if (install.state != AddonManager.STATE_DOWNLOADED)
-          reject(new Error(`Expected file to be downloaded for install of ${file.path}`));
-        else if (ignoreIncompatible && install.addon.appDisabled)
-          resolve();
-        else
-          resolve(this.promiseCompleteInstall(install));
-      });
+    return AddonManager.getInstallForFile(file).then(install => {
+      if (!install)
+        throw new Error(`No AddonInstall created for ${file.path}`);
+
+      if (install.state != AddonManager.STATE_DOWNLOADED)
+        throw new Error(`Expected file to be downloaded for install of ${file.path}`);
+
+      if (ignoreIncompatible && install.addon.appDisabled)
+        return null;
+
+      return this.promiseCompleteInstall(install);
     });
   },
 
   /**
    * A helper method to install an array of files.
    *
    * @param {Iterable<nsIFile>} files
    *        The files to install