Bug 1456051 - Make addon.uninstall race safe. r=maja_zf
authorAndreas Tolfsen <ato@sny.no>
Mon, 23 Apr 2018 08:05:35 +0100
changeset 468588 db7a5dec3809ae5d66946b1316ff7ac6a3761eea
parent 468587 52c5217fc17b08444fb3d5e426361e885aae1631
child 468589 f0baa9e18c42c7d1b20bd9a450b23ab3ab19f401
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmaja_zf
bugs1456051
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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();
   });
 };