Bug 987512: Part 5 - Remove manual AddonManager promise wrappers. r?rhelmer draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 15 Dec 2016 12:03:24 -1000
changeset 450040 8ea0998add6cc258afee979c667fed9764ef2f5b
parent 450039 0a7a85c2d95ebc63b8465224e8b6289bb52c5339
child 539657 19590a71860626c9e686ecc908a5fd9bc9403c4b
push id38748
push usermaglione.k@gmail.com
push dateThu, 15 Dec 2016 22:03:52 +0000
reviewersrhelmer
bugs987512
milestone53.0a1
Bug 987512: Part 5 - Remove manual AddonManager promise wrappers. r?rhelmer MozReview-Commit-ID: LrEiGbQGrt6
browser/experiments/Experiments.jsm
mobile/android/chrome/content/browser.js
services/sync/modules/engines/addons.js
services/sync/modules/policies.js
toolkit/components/telemetry/TelemetryEnvironment.jsm
toolkit/mozapps/extensions/content/update.js
toolkit/mozapps/extensions/internal/AddonRepository.jsm
toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
--- a/browser/experiments/Experiments.jsm
+++ b/browser/experiments/Experiments.jsm
@@ -155,30 +155,25 @@ function loadJSONAsync(file, options) {
       throw ex;
     }
     return data;
   });
 }
 
 // Returns a promise that is resolved with the AddonInstall for that URL.
 function addonInstallForURL(url, hash) {
-  let deferred = Promise.defer();
-  AddonManager.getInstallForURL(url, install => deferred.resolve(install),
-                                "application/x-xpinstall", hash);
-  return deferred.promise;
+  return AddonManager.getInstallForURL(url, null, "application/x-xpinstall", hash);
 }
 
 // Returns a promise that is resolved with an Array<Addon> of the installed
 // experiment addons.
 function installedExperimentAddons() {
-  let deferred = Promise.defer();
-  AddonManager.getAddonsByTypes(["experiment"], (addons) => {
-    deferred.resolve(addons.filter(a => !a.appDisabled));
+  return AddonManager.getAddonsByTypes(["experiment"]).then(addons => {
+    return addons.filter(a => !a.appDisabled);
   });
-  return deferred.promise;
 }
 
 // Takes an Array<Addon> and returns a promise that is resolved when the
 // addons are uninstalled.
 function uninstallAddons(addons) {
   let ids = new Set(addons.map(addon => addon.id));
   let deferred = Promise.defer();
 
@@ -2020,28 +2015,24 @@ Experiments.ExperimentEntry.prototype = 
    *
    * @return Promise<Addon|null>
    */
   _getAddon: function() {
     if (!this._addonId) {
       return Promise.resolve(null);
     }
 
-    let deferred = Promise.defer();
-
-    AddonManager.getAddonByID(this._addonId, (addon) => {
+    return AddonManager.getAddonByID(this._addonId).then(addon => {
       if (addon && addon.appDisabled) {
         // Don't return PreviousExperiments.
-        addon = null;
+        return null;
       }
 
-      deferred.resolve(addon);
+      return addon;
     });
-
-    return deferred.promise;
   },
 
   _logTermination: function(terminationKind, terminationReason) {
     if (terminationKind === undefined) {
       return;
     }
 
     if (!(terminationKind in TELEMETRY_LOG.TERMINATION)) {
--- a/mobile/android/chrome/content/browser.js
+++ b/mobile/android/chrome/content/browser.js
@@ -6718,19 +6718,17 @@ var Distribution = {
     try {
       yield it.forEach(entry => {
         // Only support extensions that are zipped in .xpi files.
         if (entry.isDir || !entry.name.endsWith(".xpi")) {
           dump("Ignoring distribution add-on that isn't an XPI: " + entry.path);
           return;
         }
 
-        new Promise((resolve, reject) => {
-          AddonManager.getInstallForFile(new FileUtils.File(entry.path), resolve);
-        }).then(install => {
+        AddonManager.getInstallForFile(new FileUtils.File(entry.path)).then(install => {
           let id = entry.name.substring(0, entry.name.length - 4);
           if (install.addon.id !== id) {
             Cu.reportError("File entry " + entry.path + " contains an add-on with an incorrect ID");
             return;
           }
           this.pendingAddonInstalls.add(install);
           install.install();
         }).catch(e => {
--- a/services/sync/modules/engines/addons.js
+++ b/services/sync/modules/engines/addons.js
@@ -754,20 +754,18 @@ class AddonValidator extends CollectionV
       "applicationID",
       "source"
     ]);
     this.engine = engine;
   }
 
   getClientItems() {
     return Promise.all([
-      new Promise(resolve =>
-        AddonManager.getAllAddons(resolve)),
-      new Promise(resolve =>
-        AddonManager.getAddonsWithOperationsByTypes(["extension", "theme"], resolve)),
+      AddonManager.getAllAddons(),
+      AddonManager.getAddonsWithOperationsByTypes(["extension", "theme"]),
     ]).then(([installed, addonsWithPendingOperation]) => {
       // Addons pending install won't be in the first list, but addons pending
       // uninstall/enable/disable will be in both lists.
       let all = new Map(installed.map(addon => [addon.id, addon]));
       for (let addon of addonsWithPendingOperation) {
         all.set(addon.id, addon);
       }
       // Convert to an array since Map.prototype.values returns an iterable
--- a/services/sync/modules/policies.js
+++ b/services/sync/modules/policies.js
@@ -698,24 +698,22 @@ ErrorHandler.prototype = {
 
     this.dontIgnoreErrors = true;
     Utils.nextTick(this.service.sync, this.service);
   },
 
   _dumpAddons: function _dumpAddons() {
     // Just dump the items that sync may be concerned with. Specifically,
     // active extensions that are not hidden.
-    let addonPromise = new Promise(resolve => {
-      try {
-        AddonManager.getAddonsByTypes(["extension"], resolve);
-      } catch (e) {
-        this._log.warn("Failed to dump addons", e)
-        resolve([])
-      }
-    });
+    let addonPromise = Promise.resolve([]);
+    try {
+      addonPromise = AddonManager.getAddonsByTypes(["extension"]);
+    } catch (e) {
+      this._log.warn("Failed to dump addons", e)
+    }
 
     return addonPromise.then(addons => {
       let relevantAddons = addons.filter(x => x.isActive && !x.hidden);
       this._log.debug("Addons installed", relevantAddons.length);
       for (let addon of relevantAddons) {
         this._log.debug(" - ${name}, version ${version}, id ${id}", addon);
       }
     });
--- a/toolkit/components/telemetry/TelemetryEnvironment.jsm
+++ b/toolkit/components/telemetry/TelemetryEnvironment.jsm
@@ -249,27 +249,16 @@ function getSystemLocale() {
   try {
     return Services.locale.getLocaleComponentForUserAgent();
   } catch (e) {
     return null;
   }
 }
 
 /**
- * Asynchronously get a list of addons of the specified type from the AddonManager.
- * @param aTypes An array containing the types of addons to request.
- * @return Promise<Array> resolved when AddonManager has finished, returning an
- *         array of addons.
- */
-function promiseGetAddonsByTypes(aTypes) {
-  return new Promise((resolve) =>
-                     AddonManager.getAddonsByTypes(aTypes, (addons) => resolve(addons)));
-}
-
-/**
  * Safely get a sysinfo property and return its value. If the property is not
  * available, return aDefault.
  *
  * @param aPropertyName the property name to get.
  * @param aDefault the value to return if aPropertyName is not available.
  * @return The property value, if available, or aDefault.
  */
 function getSysinfoProperty(aPropertyName, aDefault) {
@@ -564,17 +553,17 @@ EnvironmentAddonBuilder.prototype = {
   }),
 
   /**
    * Get the addon data in object form.
    * @return Promise<object> containing the addon data.
    */
   _getActiveAddons: Task.async(function* () {
     // Request addons, asynchronously.
-    let allAddons = yield promiseGetAddonsByTypes(["extension", "service"]);
+    let allAddons = yield AddonManager.getAddonsByTypes(["extension", "service"]);
 
     let activeAddons = {};
     for (let addon of allAddons) {
       // Skip addons which are not active.
       if (!addon.isActive) {
         continue;
       }
 
@@ -615,17 +604,17 @@ EnvironmentAddonBuilder.prototype = {
   }),
 
   /**
    * Get the currently active theme data in object form.
    * @return Promise<object> containing the active theme data.
    */
   _getActiveTheme: Task.async(function* () {
     // Request themes, asynchronously.
-    let themes = yield promiseGetAddonsByTypes(["theme"]);
+    let themes = yield AddonManager.getAddonsByTypes(["theme"]);
 
     let activeTheme = {};
     // We only store information about the active theme.
     let theme = themes.find(theme => theme.isActive);
     if (theme) {
       // Make sure to have valid dates.
       let installDate = new Date(Math.max(0, theme.installDate));
       let updateDate = new Date(Math.max(0, theme.updateDate));
@@ -691,17 +680,17 @@ EnvironmentAddonBuilder.prototype = {
    * Get the GMPlugins data in object form.
    * @return Object containing the GMPlugins data.
    *
    * This should only be called from _pendingTask; otherwise we risk
    * running this during addon manager shutdown.
    */
   _getActiveGMPlugins: Task.async(function* () {
     // Request plugins, asynchronously.
-    let allPlugins = yield promiseGetAddonsByTypes(["plugin"]);
+    let allPlugins = yield AddonManager.getAddonsByTypes(["plugin"]);
 
     let activeGMPlugins = {};
     for (let plugin of allPlugins) {
       // Only get info for active GMplugins.
       if (!plugin.isGMPlugin || !plugin.isActive) {
         continue;
       }
 
--- a/toolkit/mozapps/extensions/content/update.js
+++ b/toolkit/mozapps/extensions/content/update.js
@@ -203,18 +203,17 @@ var gVersionInfoPage = {
     let idlist = Array.from(gUpdateWizard.affectedAddonIDs).filter(
       a => a.id != AddonManager.hotfixID);
     if (idlist.length < 1) {
       gVersionInfoPage.onAllUpdatesFinished();
       return;
     }
 
     logger.debug("Fetching affected addons " + idlist.toSource());
-    let fetchedAddons = yield new Promise((resolve, reject) =>
-      AddonManager.getAddonsByIDs(idlist, resolve));
+    let fetchedAddons = yield AddonManager.getAddonsByIDs(idlist);
     // We shouldn't get nulls here, but let's be paranoid...
     gUpdateWizard.addons = fetchedAddons.filter(a => a);
     if (gUpdateWizard.addons.length < 1) {
       gVersionInfoPage.onAllUpdatesFinished();
       return;
     }
 
     gVersionInfoPage._totalCount = gUpdateWizard.addons.length;
--- a/toolkit/mozapps/extensions/internal/AddonRepository.jsm
+++ b/toolkit/mozapps/extensions/internal/AddonRepository.jsm
@@ -17,16 +17,18 @@ Components.utils.import("resource://gre/
 XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
                                   "resource://gre/modules/NetUtil.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS",
                                   "resource://gre/modules/osfile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "DeferredSave",
                                   "resource://gre/modules/DeferredSave.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "AddonRepository_SQLiteMigrator",
                                   "resource://gre/modules/addons/AddonRepository_SQLiteMigrator.jsm");
+XPCOMUtils.defineLazyModuleGetter(this, "Preferences",
+                                  "resource://gre/modules/Preferences.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Promise",
                                   "resource://gre/modules/Promise.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "Task",
                                   "resource://gre/modules/Task.jsm");
 
 
 this.EXPORTED_SYMBOLS = [ "AddonRepository" ];
 
@@ -116,46 +118,36 @@ function convertHTMLToPlainText(html) {
   converter.convert("text/html", input, input.data.length, "text/unicode",
                     output, {});
 
   if (output.value instanceof Ci.nsISupportsString)
     return output.value.data.replace(/\r\n/g, "\n");
   return html;
 }
 
-function getAddonsToCache(aIds, aCallback) {
-  try {
-    var types = Services.prefs.getCharPref(PREF_GETADDONS_CACHE_TYPES);
-  }
-  catch (e) { }
-  if (!types)
-    types = DEFAULT_CACHE_TYPES;
+function getAddonsToCache(aIds) {
+  let types = Preferences.get(PREF_GETADDONS_CACHE_TYPES) || DEFAULT_CACHE_TYPES;
 
   types = types.split(",");
 
-  AddonManager.getAddonsByIDs(aIds, function(aAddons) {
+  return AddonManager.getAddonsByIDs(aIds).then(addons => {
     let enabledIds = [];
-    for (var i = 0; i < aIds.length; i++) {
+    for (let [i, addon] of addons.entries()) {
       var preference = PREF_GETADDONS_CACHE_ID_ENABLED.replace("%ID%", aIds[i]);
-      try {
-        if (!Services.prefs.getBoolPref(preference))
-          continue;
-      } catch (e) {
-        // If the preference doesn't exist caching is enabled by default
-      }
+      // If the preference doesn't exist caching is enabled by default
+      if (!Preferences.get(preference, false))
+        continue;
 
       // The add-ons manager may not know about this ID yet if it is a pending
       // install. In that case we'll just cache it regardless
-      if (aAddons[i] && (types.indexOf(aAddons[i].type) == -1))
-        continue;
-
-      enabledIds.push(aIds[i]);
+      if (!addon || types.includes(addon.type))
+        enabledIds.push(aIds[i]);
     }
 
-    aCallback(enabledIds);
+    return enabledIds;
   });
 }
 
 function AddonSearchResult(aId) {
   this.id = aId;
   this.icons = {};
   this._unsupportedProperties = {};
 }
@@ -604,40 +596,36 @@ this.AddonRepository = {
 
   /*
    * Clear and delete the AddonRepository database
    * @return Promise{null} resolves when the database is deleted
    */
   _clearCache: function() {
     this._addons = null;
     return AddonDatabase.delete().then(() =>
-      new Promise((resolve, reject) =>
-        AddonManagerPrivate.updateAddonRepositoryData(resolve))
-    );
+      AddonManagerPrivate.updateAddonRepositoryData());
   },
 
   _repopulateCacheInternal: Task.async(function*(aSendPerformance, aTimeout) {
-    let allAddons = yield new Promise((resolve, reject) =>
-      AddonManager.getAllAddons(resolve));
+    let allAddons = yield AddonManager.getAllAddons();
 
     // Filter the hotfix out of our list of add-ons
     allAddons = allAddons.filter(a => a.id != AddonManager.hotfixID);
 
     // Completely remove cache if caching is not enabled
     if (!this.cacheEnabled) {
       logger.debug("Clearing cache because it is disabled");
       yield this._clearCache();
       return;
     }
 
     let ids = allAddons.map(a => a.id);
     logger.debug("Repopulate add-on cache with " + ids.toSource());
 
-    let addonsToCache = yield new Promise((resolve, reject) =>
-      getAddonsToCache(ids, resolve));
+    let addonsToCache = yield getAddonsToCache(ids);
 
     // Completely remove cache if there are no add-ons to cache
     if (addonsToCache.length == 0) {
       logger.debug("Clearing cache because 0 add-ons were requested");
       yield this._clearCache();
       return;
     }
 
@@ -652,18 +640,17 @@ this.AddonRepository = {
         },
         searchFailed: () => {
           logger.warn("Search failed when repopulating cache");
           resolve();
         }
       }, aSendPerformance, aTimeout));
 
     // Always call AddonManager updateAddonRepositoryData after we refill the cache
-    yield new Promise((resolve, reject) =>
-      AddonManagerPrivate.updateAddonRepositoryData(resolve));
+    yield AddonManagerPrivate.updateAddonRepositoryData();
   }),
 
   /**
    * Asynchronously add add-ons to the cache corresponding to the specified
    * ids. If caching is disabled, the cache is unchanged and the callback is
    * immediately called if it is defined.
    *
    * @param  aIds
@@ -674,17 +661,17 @@ this.AddonRepository = {
   cacheAddons: function(aIds, aCallback) {
     logger.debug("cacheAddons: enabled " + this.cacheEnabled + " IDs " + aIds.toSource());
     if (!this.cacheEnabled) {
       if (aCallback)
         aCallback();
       return;
     }
 
-    getAddonsToCache(aIds, aAddons => {
+    getAddonsToCache(aIds).then(aAddons => {
       // If there are no add-ons to cache, act as if caching is disabled
       if (aAddons.length == 0) {
         if (aCallback)
           aCallback();
         return;
       }
 
       this.getAddonsByIDs(aAddons, {
--- a/toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_blocklistchange.js
@@ -454,21 +454,20 @@ function Pbackground_update() {
   AddonManagerPrivate.backgroundUpdateCheck();
   return updated;
 }
 
 // Manually updates the test add-ons to the given version
 function Pmanual_update(aVersion) {
   let Pinstalls = [];
   for (let name of ["soft1", "soft2", "soft3", "soft4", "soft5", "hard1", "regexp1"]) {
-    Pinstalls.push(new Promise((resolve, reject) => {
-      AddonManager.getInstallForURL("http://localhost:" + gPort + "/addons/blocklist_"
-                                       + name + "_" + aVersion + ".xpi",
-                                    resolve, "application/x-xpinstall");
-    }));
+    Pinstalls.push(
+      AddonManager.getInstallForURL(
+        `http://localhost:${gPort}/addons/blocklist_${name}_${aVersion}.xpi`,
+        null, "application/x-xpinstall"));
   }
 
   return Promise.all(Pinstalls).then(installs => {
     let completePromises = [];
     for (let install of installs) {
       completePromises.push(new Promise(resolve => {
         install.addListener({
           onDownloadCancelled: resolve,
@@ -1284,19 +1283,17 @@ add_task(function* run_local_install_tes
     do_get_file("addons/blocklist_soft2_1.xpi"),
     do_get_file("addons/blocklist_soft3_1.xpi"),
     do_get_file("addons/blocklist_soft4_1.xpi"),
     do_get_file("addons/blocklist_soft5_1.xpi"),
     do_get_file("addons/blocklist_hard1_1.xpi"),
     do_get_file("addons/blocklist_regexp1_1.xpi")
   ]);
 
-  let aInstalls = yield new Promise((resolve, reject) => {
-    AddonManager.getAllInstalls(resolve)
-  });
+  let aInstalls = yield AddonManager.getAllInstalls();
   // Should have finished all installs without needing to restart
   do_check_eq(aInstalls.length, 0);
 
   let [s1, s2, s3, /* s4 */, /* s5 */, h, r] = yield promiseAddonsByIDs(ADDON_IDS);
 
   check_addon(s1, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s2, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);
   check_addon(s3, "1.0", true, true, Ci.nsIBlocklistService.STATE_SOFTBLOCKED);