Bug 1456051 - Make addon.uninstall race safe. r?maja_zf draft
authorAndreas Tolfsen <ato@sny.no>
Mon, 23 Apr 2018 08:05:35 +0100
changeset 786520 dde46c4dfec5cb17059d07006bb3ea2f1bcecd9f
parent 786519 5aaea9dc6e67eca74339261acd2a06f352e2b8b2
child 786521 d8ead6865f09b9eb1300eefcbf88cddc563bd2a1
push id107501
push userbmo:ato@sny.no
push dateMon, 23 Apr 2018 14:06:07 +0000
reviewersmaja_zf
bugs1456051
milestone61.0a1
Bug 1456051 - Make addon.uninstall race safe. r?maja_zf addon.uninstall would return immediately, not waiting for the onUninstalled listener event to fire. We can eliminate this race condition by using a promise that resolves when it fires. MozReview-Commit-ID: E4Mh797X9n8
testing/marionette/addon.js
--- a/testing/marionette/addon.js
+++ b/testing/marionette/addon.js
@@ -21,34 +21,34 @@ const ERRORS = {
   [-3]: "ERROR_CORRUPT_FILE: The file appears to be corrupt.",
   [-4]: "ERROR_FILE_ACCESS: There was an error accessing the filesystem.",
   [-5]: "ERROR_SIGNEDSTATE_REQUIRED: The addon must be signed and isn't.",
 };
 
 async function installAddon(file) {
   let install = await AddonManager.getInstallForFile(file);
 
-  return new Promise((resolve, reject) => {
+  return new Promise(resolve => {
     if (install.error) {
-      reject(new UnknownError(ERRORS[install.error]));
+      throw new UnknownError(ERRORS[install.error]);
     }
 
     let addonId = install.addon.id;
 
     let success = install => {
       if (install.addon.id === addonId) {
         install.removeListener(listener);
         resolve(install.addon);
       }
     };
 
     let fail = install => {
       if (install.addon.id === addonId) {
         install.removeListener(listener);
-        reject(new UnknownError(ERRORS[install.error]));
+        throw new UnknownError(ERRORS[install.error]);
       }
     };
 
     let listener = {
       onDownloadCancelled: fail,
       onDownloadFailed: fail,
       onInstallCancelled: fail,
       onInstallFailed: fail,
@@ -118,28 +118,31 @@ addon.install = async function(path, tem
  *     ID of the addon to uninstall.
  *
  * @return {Promise}
  *
  * @throws {UnknownError}
  *     If there is a problem uninstalling the addon.
  */
 addon.uninstall = async function(id) {
-  return AddonManager.getAddonByID(id).then(addon => {
+  let candidate = await AddonManager.getAddonByID(id);
+
+  return new Promise(resolve => {
     let listener = {
       onOperationCancelled: addon => {
-        if (addon.id === id) {
+        if (addon.id === candidate.id) {
           AddonManager.removeAddonListener(listener);
-          throw new UnknownError(`Uninstall of ${id} has been canceled`);
+          throw new UnknownError(`Uninstall of ${candidate.id} has been canceled`);
         }
       },
+
       onUninstalled: addon => {
-        if (addon.id === id) {
+        if (addon.id === candidate.id) {
           AddonManager.removeAddonListener(listener);
-          Promise.resolve();
+          resolve();
         }
       },
     };
 
     AddonManager.addAddonListener(listener);
     addon.uninstall();
   });
 };