Bug 1525635 - Properly handle mozAddonManager compatibility failure. r=aswan
authorMichael Kaply <mozilla@kaply.com>
Wed, 13 Feb 2019 14:54:24 +0000
changeset 458896 203309bfe541
parent 458895 d8c0471be578
child 458897 97dd82032154
push id35551
push usershindli@mozilla.com
push dateWed, 13 Feb 2019 21:34:09 +0000
treeherdermozilla-central@08f794a4928e [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