Issue 4579 - Wrap runtime.onMessage

polyfill.js

Index: polyfill.js
===================================================================
--- a/polyfill.js
+++ b/polyfill.js
@@ -120,24 +120,40 @@
     {
     }
 
     return true;
   }
 
   if (shouldWrapAPIs())
   {
-    // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser" object,
-    // but provides the extension API through the "chrome" namespace
+    // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser"

[Manish Jethani, 2017/10/23 02:32:45]

Unrelated, but there's an ESLint error here because of line length.

+    // object, but provides the extension API through the "chrome" namespace
     // (non-standard).
     if (typeof browser == "undefined")
       window.browser = chrome;
 
     for (let api of asyncAPIs)
       wrapAPI(api);
+
+    let {addListener} = browser.runtime.onMessage;
+    browser.runtime.onMessage.addListener = function(listener)
+    {
+      addListener.call(browser.runtime.onMessage, (
+        message, sender, sendResponse
+      ) =>
+      {
+        let wait = listener(message, sender, sendResponse);
+
+        if (wait instanceof Promise)

[Manish Jethani, 2017/10/23 02:32:45]

There's no need to add a catch handler, Chrome (and probably Edge) handles this
automagically. The error will be logged to the console, the sender will get
undefined as a response.

Firefox actually doesn't send an undefined response to the sender, instead it
sends an "unexpected error" in the sender's catch handler. We cannot mimic this
part, unfortunately. We could probably override the Firefox version of the API
to behave like Chrome's (send an undefined response), but I don't know if it
matters so much.

[Wladimir Palant, 2017/10/23 08:53:26]

Not really, the message clearly says that not handling promise rejects is
deprecated. So you should handle rejections explicitly. In particular, you need
to make sure that a response is always sent and be it undefined.
We *could*, by adjusting the behavior of the sendMessage wrapper so that it
recognizes some special values as rejections. Probably not worth doing however.

[Manish Jethani, 2017/10/23 12:21:04]

Did you mean that Chrome will no longer do this automatically in future
versions?

Done.
Ack.

+          wait.then(sendResponse);
+
+        return !!wait;
+      });
+    };

[Wladimir Palant, 2017/10/23 08:53:26]

You have a problem here, removeListener will no longer work. The best solution
should be a WeakMap mapping wrapers to original listeners. You will have to wrap
removeListener then and remove the wrapper if you have it in the map. Ideally,
the same should be done with hasListener.

[Manish Jethani, 2017/10/23 12:21:04]

Done.

   }
 
   // Workaround since HTMLCollection, NodeList, StyleSheetList, and CSSRuleList
   // didn't have iterator support before Chrome 51.
   // https://bugs.chromium.org/p/chromium/issues/detail?id=401699
   for (let object of [HTMLCollection, NodeList, StyleSheetList, CSSRuleList])
   {
     if (!(Symbol.iterator in object.prototype))