Bug 1525635 - Properly handle mozAddonManager compatibility failure. r=aswan
☠☠ backed out by 5f6a77f08081 ☠ ☠
authorMichael Kaply <mozilla@kaply.com>
Wed, 13 Feb 2019 00:09:29 +0000
changeset 458829 ba745ab8ec4a
parent 458828 b52799e37195
child 458830 6de9408be22f
push id35548
push useropoprus@mozilla.com
push dateWed, 13 Feb 2019 09:48:26 +0000
treeherdermozilla-central@93e37c529818 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1525635
milestone67.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 1525635 - Properly handle mozAddonManager compatibility failure. r=aswan Differential Revision: https://phabricator.services.mozilla.com/D18859
toolkit/mozapps/extensions/AddonManager.jsm
toolkit/mozapps/extensions/test/browser/browser_webapi_install.js
toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
toolkit/mozapps/extensions/test/xpinstall/incompatible.xpi
--- a/toolkit/mozapps/extensions/AddonManager.jsm
+++ b/toolkit/mozapps/extensions/AddonManager.jsm
@@ -1804,17 +1804,17 @@ var AddonManagerInternal = {
 
       onDownloadFailed() {
         install.removeListener(listener);
         self.installNotifyObservers("addon-install-failed", browser, url, install);
       },
 
       onDownloadEnded() {
         if (install.addon.appDisabled) {
-          // App disabled items are not compatible and so fail to install
+          // App disabled items are not compatible and so fail to install.
           install.removeListener(listener);
           install.cancel();
           self.installNotifyObservers("addon-install-failed", browser, url, install);
         }
       },
 
       onInstallCancelled() {
         install.removeListener(listener);
@@ -2610,55 +2610,16 @@ var AddonManagerInternal = {
     // helper to copy (and convert) the properties we care about
     copyProps(install, obj) {
       obj.state = AddonManager.stateToString(install.state);
       obj.error = AddonManager.errorToString(install.error);
       obj.progress = install.progress;
       obj.maxProgress = install.maxProgress;
     },
 
-    makeListener(id, mm) {
-      const events = [
-        "onDownloadStarted",
-        "onDownloadProgress",
-        "onDownloadEnded",
-        "onDownloadCancelled",
-        "onDownloadFailed",
-        "onInstallStarted",
-        "onInstallEnded",
-        "onInstallCancelled",
-        "onInstallFailed",
-      ];
-
-      let listener = {};
-      let installPromise = new Promise((resolve, reject) => {
-        events.forEach(event => {
-          listener[event] = (install, addon) => {
-            let data = {event, id};
-            AddonManager.webAPI.copyProps(install, data);
-            this.sendEvent(mm, data);
-            if (event == "onInstallEnded") {
-              resolve(addon);
-            } else if (event == "onDownloadFailed" || event == "onInstallFailed") {
-              reject({message: "install failed"});
-            } else if (event == "onDownloadCancelled" || event == "onInstallCancelled") {
-              reject({message: "install cancelled"});
-            }
-          };
-        });
-      });
-
-      // We create the promise here since this is where we're setting
-      // up the InstallListener, but if the install is never started,
-      // no handlers will be attached so make sure we terminate errors.
-      installPromise.catch(() => {});
-
-      return {listener, installPromise};
-    },
-
     forgetInstall(id) {
       let info = this.installs.get(id);
       if (!info) {
         throw new Error(`forgetInstall cannot find ${id}`);
       }
       info.install.removeListener(info.listener);
       this.installs.delete(id);
     },
@@ -2674,33 +2635,78 @@ var AddonManagerInternal = {
         if (Services.prefs.getBoolPref(PREF_WEBAPI_TESTING)
             && WEBAPI_TEST_INSTALL_HOSTS.includes(host)) {
           return;
         }
 
         throw new Error(`Install from ${host} not permitted`);
       }
 
+      const makeListener = (id, mm) => {
+        const events = [
+          "onDownloadStarted",
+          "onDownloadProgress",
+          "onDownloadEnded",
+          "onDownloadCancelled",
+          "onDownloadFailed",
+          "onInstallStarted",
+          "onInstallEnded",
+          "onInstallCancelled",
+          "onInstallFailed",
+         ];
+
+        let listener = {};
+        let installPromise = new Promise((resolve, reject) => {
+          events.forEach(event => {
+            listener[event] = (install, addon) => {
+              let data = {event, id};
+              AddonManager.webAPI.copyProps(install, data);
+              this.sendEvent(mm, data);
+              if (event == "onInstallEnded") {
+                resolve(addon);
+              } else if (event == "onDownloadFailed" || event == "onInstallFailed") {
+                reject({message: "install failed"});
+              } else if (event == "onDownloadCancelled" || event == "onInstallCancelled") {
+                reject({message: "install cancelled"});
+              } else if (event == "onDownloadEnded") {
+                if (install.addon.appDisabled) {
+                  // App disabled items are not compatible and so fail to install
+                  install.cancel();
+                  AddonManagerInternal.installNotifyObservers("addon-install-failed", target, Services.io.newURI(options.url), install);
+                }
+              }
+            };
+          });
+        });
+
+        // We create the promise here since this is where we're setting
+        // up the InstallListener, but if the install is never started,
+        // no handlers will be attached so make sure we terminate errors.
+        installPromise.catch(() => {});
+
+        return {listener, installPromise};
+     }
+
       try {
         checkInstallUrl(options.url);
       } catch (err) {
         return Promise.reject({message: err.message});
       }
 
       return AddonManagerInternal.getInstallForURL(options.url, {
         hash: options.hash,
         telemetryInfo: {
           source: AddonManager.getInstallSourceFromHost(options.sourceHost),
           method: "amWebAPI",
         },
       }).then(install => {
         AddonManagerInternal.setupPromptHandler(target, null, install, false, "AMO");
 
         let id = this.nextInstall++;
-        let {listener, installPromise} = this.makeListener(id, target.messageManager);
+        let {listener, installPromise} = makeListener(id, target.messageManager);
         install.addListener(listener);
 
         this.installs.set(id, {install, target, listener, installPromise});
 
         let result = {id};
         this.copyProps(install, result);
         return result;
       });
--- a/toolkit/mozapps/extensions/test/browser/browser_webapi_install.js
+++ b/toolkit/mozapps/extensions/test/browser/browser_webapi_install.js
@@ -323,8 +323,29 @@ add_task(async function test_permissions
 
   await testBadUrl("i am not a url", "NS_ERROR_MALFORMED_URI",
                    "Installing from an unparseable URL fails");
 
   await testBadUrl("https://addons.not-really-mozilla.org/impostor.xpi",
                    "not permitted",
                    "Installing from non-approved URL fails");
 });
+
+add_task(makeInstallTest(async function(browser) {
+  let xpiURL = `${SECURE_TESTROOT}../xpinstall/incompatible.xpi`;
+  let id = "incompatible-xpi@tests.mozilla.org";
+
+  let steps = [
+    {action: "install", expectError: true},
+    {
+      event: "onDownloadStarted",
+      props: {state: "STATE_DOWNLOADING"},
+    },
+    {event: "onDownloadProgress"},
+    {event: "onDownloadEnded"},
+    {event: "onDownloadCancelled"},
+  ];
+
+  await testInstall(browser, {url: xpiURL}, steps, "install of an incompatible XPI fails");
+
+  let addons = await promiseAddonsByIDs([id]);
+  is(addons[0], null, "The addon was not installed");
+}));
--- a/toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
+++ b/toolkit/mozapps/extensions/test/xpinstall/browser_doorhanger_installs.js
@@ -370,19 +370,21 @@ async function test_incompatible() {
   let triggers = encodeURIComponent(JSON.stringify({
     "XPI": "incompatible.xpi",
   }));
   BrowserTestUtils.openNewForegroundTab(gBrowser, TESTROOT + "installtrigger.html?" + triggers);
   await progressPromise;
   let panel = await failPromise;
 
   let notification = panel.childNodes[0];
+  let brandBundle = Services.strings.createBundle("chrome://branding/locale/brand.properties");
+  let brandShortName = brandBundle.GetStringFromName("brandShortName");
+  let message = `XPI Test could not be installed because it is not compatible with ${brandShortName} ${Services.appinfo.version}.`;
   is(notification.getAttribute("label"),
-     "The add-on downloaded from this site could not be installed " +
-     "because it appears to be corrupt.",
+     message,
      "Should have seen the right message");
 
   Services.perms.remove(makeURI("http://example.com/"), "install");
   await removeTabAndWaitForNotificationClose();
 },
 
 async function test_localFile() {
   let cr = Cc["@mozilla.org/chrome/chrome-registry;1"]
index 262ed38a7928eb78a9df5e57aa4b1dc71259abd6..de895fd1d9edcb0b249a439852d9e34c5b974af3
GIT binary patch
literal 428
zc$^FHW@h1H00HA5cfY=SrnP^7Y!K#UkYUJ8%*#wmEiTc^D$dUf4dG;9R*H&=0pZdL
zZU#n{uZ#=~EFwU~)m#b+N-)*&WvNBQnfZB2RtiQsTp*de#N1RMPbnh6Qy~PXLP-ZC
zicqFxqGzB4QJPegUtXMA6klABnw*)InH*o7T2hjkmtG812C@aHBt11bJ0H#k3S_2$
z%*f13&d)7KEXhpDN!6_=$aE+H+ElEUn_rcglar{IUz85E7ObPVq$o4FBtADYFCN(y
zhG1u)sY$FrQ=_E>R#M9a!nIrh-i%Cg%(wzkf&m7WG=fO9AZ3LFDOzv_c(byB6f**0
KFpzd;1n~f6E^DCx