Issue 4579 - Wrap runtime.onMessage

polyfill.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 "use strict";
 
 {
   const asyncAPIs = [
     "contextMenus.removeAll",
     "devtools.panels.create",
     "notifications.clear",
     "notifications.create",
+    "runtime.getBrowserInfo",
     "runtime.openOptionsPage",
     "runtime.sendMessage",
     "runtime.setUninstallURL",
     "storage.local.get",
     "storage.local.remove",
     "storage.local.set",
     "storage.managed.get",
     "tabs.create",
     "tabs.get",
     "tabs.getCurrent",
@@ skipping 15 matching lines @@
     // Older versions of Chrome have a typo:
     // https://crrev.com/c33f51726eacdcc1a487b21a13611f7eab580d6d
     /^The message port closed before a res?ponse was received\.$/;
 
   // This is the error Firefox throws when a message listener is not a
   // function.
   const invalidMessageListenerError = "Invalid listener for runtime.onMessage.";
 
   let messageListeners = new WeakMap();
 
-  function wrapAPI(api)

[kzar, 2018/01/08 14:35:16]

Since we're now wrapping the message APIs as well perhaps we should rename this
to `wrapAsyncAPI` which would be consistent with the `asyncAPIs` variable name
as well?

[Manish Jethani, 2018/01/09 09:37:35]

Done.

+  function wrapAsyncAPI(api)
   {
     let object = browser;
     let path = api.split(".");
     let name = path.pop();
 
     for (let node of path)
     {
       object = object[node];
 
       if (!object)
         return;
     }
 
     let func = object[name];
-    object[name] = function(...args)
+    if (!func)
+      return;
+
+    let descriptor = Object.getOwnPropertyDescriptor(object, name);
+
+    delete descriptor["get"];
+    delete descriptor["set"];
+
+    descriptor.value = function(...args)
     {
       let callStack = new Error().stack;
 
       if (typeof args[args.length - 1] == "function")
         return func.apply(object, args);
 
       // If the last argument is undefined, we drop it from the list assuming
       // it stands for the optional callback. We must do this, because we have
       // to replace it with our own callback. If we simply append our own
       // callback to the list, it won't match the signature of the function and
@@ skipping 20 matching lines @@
 
             reject(error);
           }
           else
           {
             resolve(result);
           }
         });
       });
     };
-  }
-
-  function wrapMessageAPIs()

[kzar, 2018/01/08 14:35:16]

Nit: `wrapRuntimeOnMessage`?

[Manish Jethani, 2018/01/09 09:37:35]

Done.

+
+    Object.defineProperty(object, name, descriptor);
+  }
+
+  function wrapRuntimeOnMessage()
   {
     let {onMessage} = browser.runtime;
     let {addListener, removeListener, hasListener} = onMessage;
 
     onMessage.addListener = function(listener)
     {
       if (typeof listener != "function")

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

We must check the type here to be consistent with Firefox.

Note that typeof also works for Proxy instances.

[kzar, 2018/01/08 14:35:16]

Acknowledged.

         throw new Error(invalidMessageListenerError);
 
       // Don't add the same listener twice or we end up with multiple wrappers.
       if (messageListeners.has(listener))
         return;
 
       let wrapper = (message, sender, sendResponse) =>
       {
         let wait = listener(message, sender, sendResponse);
 
         if (wait instanceof Promise)
         {
           wait.then(sendResponse, reason =>
           {
             try
             {
               sendResponse();
             }
             finally
             {
               // sendResponse can throw if the internal port is closed; be sure

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

I haven't verified this, but in any case adding a finally here seems like a good
idea.

[kzar, 2018/01/08 14:35:16]

Acknowledged.

               // to throw the original error.
               throw reason;
             }
           });
         }
 
         return !!wait;
       };
 
       addListener.call(onMessage, wrapper);
@@ skipping 37 matching lines @@
 
   if (shouldWrapAPIs())
   {
     // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser"
     // 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);
-
-    wrapMessageAPIs();
+      wrapAsyncAPI(api);
+
+    wrapRuntimeOnMessage();
   }
 
   // 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))
       object.prototype[Symbol.iterator] = Array.prototype[Symbol.iterator];
   }
 }