Bug 1643896 - Convert sync onMessage listener exceptions into async rejections r=mixedpuppy
authorTomislav Jovanovic <tomica@gmail.com>
Thu, 11 Jun 2020 15:24:58 +0000
changeset 535135 acd373902b3786bef83a3e4b47c5c78f5a6eac3b
parent 535134 e3591c07b467fbcd9bfc0244fbbeb4e6a19ed0e2
child 535136 f818841dc6945c9d65d04fd9603567bd961f3eb8
push id37498
push userapavel@mozilla.com
push dateFri, 12 Jun 2020 03:05:25 +0000
treeherdermozilla-central@a5cb1a40413e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1643896
milestone79.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 1643896 - Convert sync onMessage listener exceptions into async rejections r=mixedpuppy When there are multiple runtime.onMessage listeners in the same context, if the first one sends a response, but the second one throws, the exception wins the race because we convert all responses into async promises. Converting the sync exception into an async promise rejection ensures we process them in the original order. Differential Revision: https://phabricator.services.mozilla.com/D79006
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -190,17 +190,22 @@ class MessageEvent extends SimpleEventAP
     let response, sendResponse;
     let promise = new Promise(resolve => {
       sendResponse = Cu.exportFunction(value => {
         resolve(value);
         response = promise;
       }, this.context.cloneScope);
     });
 
-    let result = fire.raw(message, sender, sendResponse);
+    let result;
+    try {
+      result = fire.raw(message, sender, sendResponse);
+    } catch (e) {
+      return Promise.reject(e);
+    }
     if (
       result &&
       typeof result === "object" &&
       Cu.getClassName(result, true) === "Promise" &&
       this.context.principal.subsumes(Cu.getObjectPrincipal(result))
     ) {
       return StrongPromise.wrap(result, fire.location);
     } else if (result === true) {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js
@@ -36,16 +36,26 @@ add_task(async function runtimeSendMessa
     browser.runtime.onMessage.addListener((msg, sender, respond) => {
       if (msg == "respond-now") {
         respond("hello");
       } else if (msg == "respond-now-2") {
         respond(msg);
       }
     });
 
+    browser.runtime.onMessage.addListener((msg, sender, respond) => {
+      if (msg == "respond-now") {
+        // If a response from another listener is received first, this
+        // exception should be ignored.  Test fails if it is not.
+
+        // All this is of course stupid, but some extensions depend on it.
+        msg.blah.this.throws();
+      }
+    });
+
     let childFrame = document.createElement("iframe");
     childFrame.src = "extensionpage.html";
     document.body.appendChild(childFrame);
   }
 
   function senderScript() {
     Promise.all([
       browser.runtime.sendMessage("respond-now"),