Bug 1331313 - Let add-on install() / uninstall() wait for the expected add-on. r=ato
authorHenrik Skupin <mail@hskupin.info>
Wed, 23 Aug 2017 15:53:45 +0200
changeset 379172 cd25a0a7bccae993b29436de5224e3fde81018f7
parent 379171 cccd53c58f081e6fa08a233bbd12fe036f2e5d80
child 379173 e00a0a13c7fa19ef4aac8627cf2046991b663cc2
push id32451
push userkwierso@gmail.com
push dateWed, 06 Sep 2017 22:51:37 +0000
treeherdermozilla-central@d8e238b811d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersato
bugs1331313
milestone57.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 1331313 - Let add-on install() / uninstall() wait for the expected add-on. r=ato Currently the listener for addon installs misses a check for the addon id, to only resolve the promise when it has been called for the expected addon. This can cause race-conditions if other addons are getting installed at the same time. The same applies to uninstall which doesn't wait at all until the operation has been completed. MozReview-Commit-ID: 5GsomMoAVZ1
testing/marionette/addon.js
testing/marionette/harness/marionette_harness/tests/unit/test_addons.py
testing/marionette/harness/marionette_harness/tests/unit/webextension-invalid.xpi
--- a/testing/marionette/addon.js
+++ b/testing/marionette/addon.js
@@ -25,16 +25,53 @@ addon.Errors = {
   [-5]: "ERROR_SIGNEDSTATE_REQUIRED: The addon must be signed and isn't.",
 };
 
 function lookupError(code) {
   let msg = addon.Errors[code];
   return new UnknownError(msg);
 }
 
+async function installAddon(file) {
+  let install = await AddonManager.getInstallForFile(file);
+
+  return new Promise((resolve, reject) => {
+    if (install.error != 0) {
+      reject(new UnknownError(lookupError(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(lookupError(install.error)));
+      }
+    };
+
+    let listener = {
+      onDownloadCancelled: fail,
+      onDownloadFailed: fail,
+      onInstallCancelled: fail,
+      onInstallFailed: fail,
+      onInstallEnded: success,
+    };
+
+    install.addListener(listener);
+    install.install();
+  });
+}
+
 /**
  * Install a Firefox addon.
  *
  * If the addon is restartless, it can be used right away.  Otherwise a
  * restart is required.
  *
  * Temporary addons will automatically be uninstalled on shutdown and
  * do not need to be signed, though they must be restartless.
@@ -45,61 +82,61 @@ function lookupError(code) {
  *     True to install the addon temporarily, false (default) otherwise.
  *
  * @return {Promise.<string>}
  *     Addon ID.
  *
  * @throws {UnknownError}
  *     If there is a problem installing the addon.
  */
-addon.install = function(path, temporary = false) {
-  return new Promise((resolve, reject) => {
-    let file = new FileUtils.File(path);
-
-    let listener = {
-      onInstallEnded(install, addon) {
-        resolve(addon.id);
-      },
-
-      onInstallFailed(install) {
-        reject(lookupError(install.error));
-      },
+addon.install = async function(path, temporary = false) {
+  let file = new FileUtils.File(path);
+  let addon;
 
-      onInstalled(addon) {
-        AddonManager.removeAddonListener(listener);
-        resolve(addon.id);
-      },
-    };
+  try {
+    if (temporary) {
+      addon = await AddonManager.installTemporaryAddon(file);
+    } else {
+      addon = await installAddon(file);
+    }
+  } catch (e) {
+    throw new UnknownError(
+        `Could not install add-on at '${path}': ${e.message}`);
+  }
 
-    if (!temporary) {
-      AddonManager.getInstallForFile(file, function(aInstall) {
-        if (aInstall.error !== 0) {
-          reject(lookupError(aInstall.error));
-        }
-        aInstall.addListener(listener);
-        aInstall.install();
-      });
-    } else {
-      AddonManager.addAddonListener(listener);
-      AddonManager.installTemporaryAddon(file);
-    }
-  });
+  return addon.id;
 };
 
 /**
  * Uninstall a Firefox addon.
  *
  * If the addon is restartless it will be uninstalled right away.
  * Otherwise, Firefox must be restarted for the change to take effect.
  *
  * @param {string} id
  *     ID of the addon to uninstall.
  *
  * @return {Promise}
+ *
+ * @throws {UnknownError}
+ *     If there is a problem uninstalling the addon.
  */
-addon.uninstall = function(id) {
-  return new Promise(resolve => {
-    AddonManager.getAddonByID(id, function(addon) {
-      addon.uninstall();
-      resolve();
-    });
+addon.uninstall = async function(id) {
+  return AddonManager.getAddonByID(id).then(addon => {
+    let listener = {
+      onOperationCancelled: addon => {
+        if (addon.id === id) {
+          AddonManager.removeAddonListener(listener);
+          throw new UnknownError(`Uninstall of ${id} has been canceled`);
+        }
+      },
+      onUninstalled: addon => {
+        if (addon.id === id) {
+          AddonManager.removeAddonListener(listener);
+          Promise.resolve();
+        }
+      },
+    };
+
+    AddonManager.addAddonListener(listener);
+    addon.uninstall();
   });
 };
--- a/testing/marionette/harness/marionette_harness/tests/unit/test_addons.py
+++ b/testing/marionette/harness/marionette_harness/tests/unit/test_addons.py
@@ -1,58 +1,93 @@
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 import os
 
 from marionette_driver.addons import Addons, AddonInstallException
-from marionette_harness import MarionetteTestCase, skip
+from marionette_harness import MarionetteTestCase
 
 
 here = os.path.abspath(os.path.dirname(__file__))
 
 
 class TestAddons(MarionetteTestCase):
 
     def setUp(self):
-        MarionetteTestCase.setUp(self)
+        super(TestAddons, self).setUp()
+
         self.addons = Addons(self.marionette)
+        self.preinstalled_addons = self.all_addon_ids
+
+    def tearDown(self):
+        self.reset_addons()
+
+        super(TestAddons, self).tearDown()
 
     @property
     def all_addon_ids(self):
-        with self.marionette.using_context('chrome'):
+        with self.marionette.using_context("chrome"):
             addons = self.marionette.execute_async_script("""
               Components.utils.import("resource://gre/modules/AddonManager.jsm");
-              AddonManager.getAllAddons(function(addons){
-                let ids = addons.map(function(x) {
-                  return x.id;
-                });
+              AddonManager.getAllAddons(function(addons) {
+                let ids = addons.map(x => x.id);
                 marionetteScriptFinished(ids);
               });
             """)
 
-        return addons
+        return set(addons)
 
-    def test_install_and_remove_temporary_unsigned_addon(self):
-        addon_path = os.path.join(here, 'webextension-unsigned.xpi')
+    def reset_addons(self):
+        with self.marionette.using_context("chrome"):
+            for addon in (self.all_addon_ids - self.preinstalled_addons):
+                addon_id = self.marionette.execute_async_script("""
+                  Components.utils.import("resource://gre/modules/AddonManager.jsm");
+                  return new Promise(resolve => {
+                    AddonManager.getAddonByID(arguments[0], function(addon) {
+                      addon.uninstall();
+                      marionetteScriptFinished(addon.id);
+                    });
+                  });
+                """, script_args=(addon,))
+                self.assertEqual(addon_id, addon,
+                                 msg="Failed to uninstall {}".format(addon))
+
+    def test_temporary_install_and_remove_unsigned_addon(self):
+        addon_path = os.path.join(here, "webextension-unsigned.xpi")
 
         addon_id = self.addons.install(addon_path, temp=True)
         self.assertIn(addon_id, self.all_addon_ids)
+        self.assertEqual(addon_id, "{d3e7c1f1-2e35-4a49-89fe-9f46eb8abf0a}")
 
         self.addons.uninstall(addon_id)
         self.assertNotIn(addon_id, self.all_addon_ids)
 
-    def test_install_unsigned_addon(self):
-        addon_path = os.path.join(here, 'webextension-unsigned.xpi')
+    def test_temporary_install_invalid_addon(self):
+        addon_path = os.path.join(here, "webextension-invalid.xpi")
+
+        with self.assertRaises(AddonInstallException):
+            self.addons.install(addon_path, temp=True)
+        self.assertNotIn("{d3e7c1f1-2e35-4a49-89fe-9f46eb8abf0a}", self.all_addon_ids)
+
+    def test_install_and_remove_signed_addon(self):
+        addon_path = os.path.join(here, "webextension-signed.xpi")
+
+        addon_id = self.addons.install(addon_path)
+        self.assertIn(addon_id, self.all_addon_ids)
+        self.assertEqual(addon_id, "{d3e7c1f1-2e35-4a49-89fe-9f46eb8abf0a}")
+
+        self.addons.uninstall(addon_id)
+        self.assertNotIn(addon_id, self.all_addon_ids)
+
+    def test_install_invalid_addon(self):
+        addon_path = os.path.join(here, "webextension-invalid.xpi")
 
         with self.assertRaises(AddonInstallException):
             self.addons.install(addon_path)
-
-    @skip("Need to get the test extension signed")
-    def test_install_and_remove_signed_addon(self):
-        addon_path = os.path.join(here, 'mn-restartless-signed.xpi')
+        self.assertNotIn("{d3e7c1f1-2e35-4a49-89fe-9f46eb8abf0a}", self.all_addon_ids)
 
-        addon_id = self.addons.install(addon_path)
-        self.assertIn(addon_id, self.all_addon_ids)
+    def test_install_unsigned_addon_fails(self):
+        addon_path = os.path.join(here, "webextension-unsigned.xpi")
 
-        self.addons.uninstall(addon_id)
-        self.assertNotIn(addon_id, self.all_addon_ids)
+        with self.assertRaises(AddonInstallException):
+            self.addons.install(addon_path)
new file mode 100644
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..bd1177462ea7ba987f2b31c3f9ff9b200edb5e90
GIT binary patch
literal 295
zc$^FHW@Zs#U|`^2h^i9z-jTV)vK+`;0mQrvG7Pzid6{Xc#U*-K#rb)mA)E}%yBp?4
z&H>`m3T_5QmamKq3}EfK-Hv>R6a-wq&q*@MHaxm$+r2cdW<@2{))MC(yBF!*{k!LT
zohXCE%aaw%Ukt^$7`dJ|wdd)iZ~htbN9sSjb^cHFtA#%{^l2`A!2316ZZ}s&`saxK
zmv7GtxBhfhWBHTL<D0K6Nw+-P_w>z|*6E#M<^j$b7hHt(1H2iT<d|`}UxEPyfKFs+
dXaup)e8URy4Vq5^yjj^G+87yvf%HDGApkgdUgrP+