Issue 4579 - Promisify APIs

polyfill.js

Index: polyfill.js
===================================================================
--- a/polyfill.js
+++ b/polyfill.js
@@ -12,22 +12,111 @@
  * 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";
 
-// Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser" object,

[Manish Jethani, 2017/10/17 13:33:24]

It's best to move this inside, after the call to shouldWrapAPIs, see below.

-// but provides the extension API through the "chrome" namespace
-// (non-standard).
-if (typeof browser == "undefined")
-  window.browser = chrome;
+(function()

[Manish Jethani, 2017/10/17 13:33:24]

This all needs to be wrapped inside an anonymous function here so we don't
pollute the global namespace.

[Manish Jethani, 2017/10/17 17:35:45]

Actually this can just be an anonymous block.

Done.

[kzar, 2017/10/17 18:13:42]

IIRC Firefox got upset when declaring functions inside of a block like that.

[Manish Jethani, 2017/10/17 20:48:45]

You're right, the function declarations would be "hoisted" to the outer scope
here.

Fixed, by using arrow functions.

[Sebastian Noack, 2017/10/17 23:31:03]

Both of you are wrong. In strict mode, as of ES2015, and FF47+, function
declarations are scoped to the surrounding block. Hence declaring named
functions is totally fine, and IMO preferable, here.

However, what Dave is talking about is; before ES2015, function declarations
in nested blocks were non-standard, and the behavior varied across JavaScript
implementations, e.g. Firefox <47 threw an error.

[Manish Jethani, 2017/10/18 00:05:33]

Ah, of course.

Reverted.

+{
+  const asyncAPIs = [
+    "contextMenus.removeAll",
+    "devtools.panels.create",
+    "notifications.clear",
+    "notifications.create",
+    "runtime.openOptionsPage",
+    "runtime.sendMessage",
+    "runtime.setUninstallURL",
+    "storage.local.get",
+    "storage.local.remove",
+    "storage.local.set",
+    "storage.managed.get",
+    "tabs.create",
+    "tabs.get",
+    "tabs.insertCSS",
+    "tabs.query",
+    "tabs.reload",
+    "tabs.sendMessage",
+    "tabs.update",
+    "webNavigation.getAllFrames",
+    "webRequest.handlerBehaviorChanged",
+    "windows.create",
+    "windows.update"
+  ];
+
+  function wrapAPI(api)
+  {
+    let object = browser;

[Manish Jethani, 2017/10/17 13:33:23]

Note: Using browser now instead of chrome

+    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 (typeof args[args.length - 1] == "function")
+        return func.apply(object, args);
 
-// 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];
-}
+      // 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
+      // will cause an exception.
+      if (typeof args[args.length - 1] == "undefined")
+        args.pop();
+
+      return new Promise((resolve, reject) =>
+      {
+        func.call(object, ...args, result =>
+        {
+          let error = browser.runtime.lastError;
+          if (error)
+            reject(error);
+          else
+            resolve(result);
+        });
+      });
+    };
+  }
+
+  function shouldWrapAPIs()
+  {
+    try
+    {
+      return !(browser.storage.local.get([]) instanceof Promise);
+    }
+    catch (error)
+    {
+    }
+
+    return true;
+  }
+
+  if (shouldWrapAPIs())
+  {
+    // Unlike Firefox and Microsoft Edge, Chrome doesn't have a "browser" object,

[Manish Jethani, 2017/10/17 13:33:23]

After calling shouldWrapAPIs we can safely overwrite window.browser 

+    // 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);
+  }
+
+  // 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];
+  }
+}());