Issue 4579 - Promisify APIs

ext/common.js

Index: ext/common.js
===================================================================
--- a/ext/common.js
+++ b/ext/common.js
@@ -20,16 +20,89 @@
 (function()
 {
   // Both Edge and Mozilla Web Extensions use the namespace
   // 'browser' instead of 'chrome'. Edge has chrome namespace defined,
   // in some cases, but only with one property: 'app'.
   if (typeof chrome == "undefined" || typeof chrome.extension == "undefined")
     window.chrome = window.browser;
 
+  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, object)
+  {
+    if (!object)
+      return;
+
+    let [root, ...subPath] = api.split(".");

[Sebastian Noack, 2017/10/13 04:04:19]

Would this logic here potentially somewhat more light-weight if asyncAPIs would
be an array of arrays?

[Manish Jethani, 2017/10/13 07:17:46]

It would be more efficient if it were were an array of arrays, but then it would
be a little harder to maintain. I don't think we would gain much from
"normalizing" that list.

I have modified the code now to use a loop instead of recursion.

+
+    if (subPath.length > 0)
+    {
+      wrapAPI(subPath.join("."), object[root]);
+      return;
+    }
+
+    let func = object[root];
+    object[root] = function(...args)
+    {
+      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
+      // 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 = chrome.runtime.lastError;
+          if (error)
+            reject(error);
+          else
+            resolve(result);
+        });
+      });
+    };
+  }
+
+  if (typeof browser == "undefined" ||

[Manish Jethani, 2017/10/11 21:33:55]

I was going for runtime.getPlatformInfo, but it's not available in content
scripts.

storage.local.get with an empty list of keys should be a no-op.

+      !(browser.storage.local.get([]) instanceof Promise))

[Sebastian Noack, 2017/10/13 04:04:19]

Wouldn't you get an error here on Chrome, because a function is expected as
second argument? I think we need to do something like this:
https://github.com/mozilla/webextension-polyfill/issues/3#issuecomment-259425339

[Manish Jethani, 2017/10/13 07:17:46]

It would be Edge on which the error might be thrown. I've wrapped it in a
try...catch for now, but we'd have to confirm that it's needed.

+  {
+    for (let api of asyncAPIs)
+      wrapAPI(api, chrome);
+  }
+
   window.ext = {};
 
   let EventTarget = ext._EventTarget = function()
   {
     this._listeners = new Set();
   };
   EventTarget.prototype = {
     addListener(listener)