Bug 1234020: Part 1 - [webext] Add initial binding-level promise<->callback support. r=billm

--HG--
extra : commitid : LmqVSqXGkKa
extra : rebase_source : 1e28a81fd74c920822bb5eed0aff8841bd628271
extra : histedit_source : 9acf96c0593f1271a753507c6630765394b88f3c%2Cf19e421dfed99dd65482ba935ac50fffa1208a6d

browser/components/extensions/ext-tabs.js

@@ -507,18 +507,15 @@ extensions.registerSchemaAPI("tabs", null, (extension, context) => {
           options.run_at = details.runAt;
         }
 
-        return context.sendMessage(mm, "Extension:Execute", { options }, recipient)
-                      .then(value => [value]);
+        return context.sendMessage(mm, "Extension:Execute", { options }, recipient);
       },
 
-      executeScript: function(tabId, details, callback) {
-        return context.wrapPromise(self.tabs._execute(tabId, details, "js"),
-                                   callback);
+      executeScript: function(tabId, details) {
+        return self.tabs._execute(tabId, details, "js");
       },
 
-      insertCSS: function(tabId, details, callback) {
-        return context.wrapPromise(self.tabs._execute(tabId, details, "css"),
-                                   callback);
+      insertCSS: function(tabId, details) {
+        return self.tabs._execute(tabId, details, "css");
       },
 
       connect: function(tabId, connectInfo) {

browser/components/extensions/schemas/tabs.json

@@ -771,6 +771,7 @@
         "name": "executeScript",
         "type": "function",
         "description": "Injects JavaScript code into a page. For details, see the $(topic:content_scripts)[programmatic injection] section of the content scripts doc.",
+        "async": "callback",
         "parameters": [
           {"type": "integer", "name": "tabId", "minimum": 0, "optional": true, "description": "The ID of the tab in which to run the script; defaults to the active tab of the current window."},
           {
@@ -799,6 +800,7 @@
         "name": "insertCSS",
         "type": "function",
         "description": "Injects CSS into a page. For details, see the $(topic:content_scripts)[programmatic injection] section of the content scripts doc.",
+        "async": "callback",
         "parameters": [
           {"type": "integer", "name": "tabId", "minimum": 0, "optional": true, "description": "The ID of the tab in which to insert the CSS; defaults to the active tab of the current window."},
           {

toolkit/components/extensions/Extension.jsm

@@ -351,36 +351,73 @@ GlobalManager = {
 
   observe(contentWindow, topic, data) {
     let inject = (extension, context) => {
-      let chromeObj = Cu.createObjectIn(contentWindow, {defineAs: "browser"});
-      contentWindow.wrappedJSObject.chrome = contentWindow.wrappedJSObject.browser;
-      let api = Management.generateAPIs(extension, context, Management.apis);
-      injectAPI(api, chromeObj);
+      // We create two separate sets of bindings, one for the `chrome`
+      // global, and one for the `browser` global. The latter returns
+      // Promise objects if a callback is not passed, while the former
+      // does not.
+      let injectObject = (name, defaultCallback) => {
+        let browserObj = Cu.createObjectIn(contentWindow, {defineAs: name});
 
-      let schemaApi = Management.generateAPIs(extension, context, Management.schemaApis);
-      let schemaWrapper = {
-        callFunction(ns, name, args) {
-          return schemaApi[ns][name].apply(null, args);
-        },
+        let api = Management.generateAPIs(extension, context, Management.apis);
+        injectAPI(api, browserObj);
 
-        getProperty(ns, name) {
-          return schemaApi[ns][name];
-        },
+        let schemaApi = Management.generateAPIs(extension, context, Management.schemaApis);
+        let schemaWrapper = {
+          callFunction(ns, name, args) {
+            return schemaApi[ns][name](...args);
+          },
 
-        setProperty(ns, name, value) {
-          schemaApi[ns][name] = value;
-        },
+          callAsyncFunction(ns, name, args, callback) {
+            // We pass an empty stub function as a default callback for
+            // the `chrome` API, so promise objects are not returned,
+            // and lastError values are reported immediately.
+            if (callback === null) {
+              callback = defaultCallback;
+            }
 
-        addListener(ns, name, listener, args) {
-          return schemaApi[ns][name].addListener.call(null, listener, ...args);
-        },
-        removeListener(ns, name, listener) {
-          return schemaApi[ns][name].removeListener.call(null, listener);
-        },
-        hasListener(ns, name, listener) {
-          return schemaApi[ns][name].hasListener.call(null, listener);
-        },
+            let promise;
+            try {
+              // TODO: Stop passing the callback once all APIs return
+              // promises.
+              promise = schemaApi[ns][name](...args, callback);
+            } catch (e) {
+              promise = Promise.reject(e);
+              // TODO: Certain tests are still expecting API methods to
+              // throw errors.
+              throw e;
+            }
+
+            // TODO: This check should no longer be necessary
+            // once all async methods return promises.
+            if (promise) {
+              return context.wrapPromise(promise, callback);
+            }
+            return undefined;
+          },
+
+          getProperty(ns, name) {
+            return schemaApi[ns][name];
+          },
+
+          setProperty(ns, name, value) {
+            schemaApi[ns][name] = value;
+          },
+
+          addListener(ns, name, listener, args) {
+            return schemaApi[ns][name].addListener.call(null, listener, ...args);
+          },
+          removeListener(ns, name, listener) {
+            return schemaApi[ns][name].removeListener.call(null, listener);
+          },
+          hasListener(ns, name, listener) {
+            return schemaApi[ns][name].hasListener.call(null, listener);
+          },
+        };
+        Schemas.inject(browserObj, schemaWrapper);
       };
-      Schemas.inject(chromeObj, schemaWrapper);
+
+      injectObject("browser", null);
+      injectObject("chrome", () => {});
     };
 
     let id = ExtensionManagement.getAddonIdForWindow(contentWindow);

toolkit/components/extensions/ExtensionUtils.jsm

@@ -112,6 +112,13 @@ DefaultWeakMap.prototype = {
   },
 };
 
+class SpreadArgs extends Array {
+  constructor(args) {
+    super();
+    this.push(...args);
+  }
+}
+
 class BaseContext {
   constructor() {
     this.onClose = new Set();
@@ -202,8 +209,8 @@ class BaseContext {
    * to the console.
    *
    * @param {Promise} promise The promise with which to wrap the
-   *     callback. Must resolve to an array, or other iterable, each
-   *     element of which will be passed as an argument to the callback.
+   *     callback. May resolve to a `SpreadArgs` instance, in which case
+   *     each element will be used as a separate argument.
    *
    * @param {function} [callback] The callback function to wrap
    *
@@ -214,7 +221,11 @@ class BaseContext {
     if (callback) {
       promise.then(
         args => {
-          runSafeSync(this, callback, ...args);
+          if (args instanceof SpreadArgs) {
+            runSafeSync(this, callback, ...args);
+          } else {
+            runSafeSync(this, callback, args);
+          }
         },
         error => {
           this.withLastError(error, () => {
@@ -925,6 +936,7 @@ this.ExtensionUtils = {
   injectAPI,
   MessageBroker,
   Messenger,
+  SpreadArgs,
   extend,
   flushJarCache,
   instanceOf,

toolkit/components/extensions/Schemas.jsm

@@ -84,7 +84,7 @@ class Context {
 
     this.path = [];
 
-    let props = ["addListener", "callFunction",
+    let props = ["addListener", "callFunction", "callAsyncFunction",
                  "hasListener", "removeListener",
                  "getProperty", "setProperty"];
     for (let prop of props) {
@@ -613,9 +613,10 @@ class ArrayType extends Type {
 }
 
 class FunctionType extends Type {
-  constructor(parameters) {
+  constructor(parameters, isAsync) {
     super();
     this.parameters = parameters;
+    this.isAsync = isAsync;
   }
 
   normalize(value, context) {
@@ -779,9 +780,12 @@ class CallEntry extends Entry {
 
 // Represents a "function" defined in a schema namespace.
 class FunctionEntry extends CallEntry {
-  constructor(namespaceName, name, type, unsupported, allowAmbiguousOptionalArguments) {
+  constructor(namespaceName, name, type, unsupported, allowAmbiguousOptionalArguments, returns) {
     super(namespaceName, name, type.parameters, allowAmbiguousOptionalArguments);
     this.unsupported = unsupported;
+    this.returns = returns;
+
+    this.isAsync = type.isAsync;
   }
 
   inject(name, dest, wrapperFuncs) {
@@ -789,10 +793,20 @@ class FunctionEntry extends CallEntry {
       return;
     }
 
-    let stub = (...args) => {
-      let actuals = this.checkParameters(args, dest, new Context(wrapperFuncs));
-      return wrapperFuncs.callFunction(this.namespaceName, name, actuals);
-    };
+    let context = new Context(wrapperFuncs);
+    let stub;
+    if (this.isAsync) {
+      stub = (...args) => {
+        let actuals = this.checkParameters(args, dest, context);
+        let callback = actuals.pop();
+        return wrapperFuncs.callAsyncFunction(this.namespaceName, name, actuals, callback);
+      };
+    } else {
+      stub = (...args) => {
+        let actuals = this.checkParameters(args, dest, context);
+        return wrapperFuncs.callFunction(this.namespaceName, name, actuals);
+      };
+    }
     Cu.exportFunction(stub, dest, {defineAs: name});
   }
 }
@@ -986,20 +1000,35 @@ this.Schemas = {
       checkTypeProperties();
       return new BooleanType();
     } else if (type.type == "function") {
+      let isAsync = typeof(type.async) == "string";
+
       let parameters = null;
       if ("parameters" in type) {
         parameters = [];
         for (let param of type.parameters) {
+          // Callbacks default to optional for now, because of promise
+          // handling.
+          let isCallback = isAsync && param.name == type.async;
+
           parameters.push({
             type: this.parseType(namespaceName, param, ["name", "optional"]),
             name: param.name,
-            optional: param.optional || false,
+            optional: param.optional == null ? isCallback : param.optional,
           });
         }
       }
 
-      checkTypeProperties("parameters");
-      return new FunctionType(parameters);
+      if (isAsync) {
+        if (!parameters || !parameters.length || parameters[parameters.length - 1].name != type.async) {
+          throw new Error(`Internal error: "async" property must name the last parameter of the function.`);
+        }
+        if (type.returns || type.allowAmbiguousOptionalArguments) {
+          throw new Error(`Internal error: Async functions must not have return values or ambiguous arguments.`);
+        }
+      }
+
+      checkTypeProperties("parameters", "async", "returns");
+      return new FunctionType(parameters, isAsync);
     } else if (type.type == "any") {
       // Need to see what minimum and maximum are supposed to do here.
       checkTypeProperties("minimum", "maximum");
@@ -1051,15 +1080,13 @@ this.Schemas = {
   },
 
   loadFunction(namespaceName, fun) {
-    // We ignore this property for now.
-    let returns = fun.returns;  // eslint-disable-line no-unused-vars
-
     let f = new FunctionEntry(namespaceName, fun.name,
                               this.parseType(namespaceName, fun,
                                              ["name", "unsupported", "deprecated", "returns",
                                               "allowAmbiguousOptionalArguments"]),
                               fun.unsupported || false,
-                              fun.allowAmbiguousOptionalArguments || false);
+                              fun.allowAmbiguousOptionalArguments || false,
+                              fun.returns || null);
     this.register(namespaceName, fun.name, f);
   },
 

toolkit/components/extensions/schemas/extension.json

@@ -101,6 +101,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Retrieves the state of the extension's access to Incognito-mode (as determined by the user-controlled 'Allowed in Incognito' checkbox.",
+        "async": "callback",
         "parameters": [
           {
             "type": "function",
@@ -120,6 +121,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Retrieves the state of the extension's access to the 'file://' scheme (as determined by the user-controlled 'Allow access to File URLs' checkbox.",
+        "async": "callback",
         "parameters": [
           {
             "type": "function",

toolkit/components/extensions/schemas/i18n.json

@@ -33,6 +33,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Gets the accept-languages of the browser. This is different from the locale used by the browser; to get the locale, use $(ref:i18n.getUILanguage).",
+        "async": "callback",
         "parameters": [
           {
             "type": "function",
@@ -81,6 +82,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Detects the language of the provided text using CLD.",
+        "async": "callback",
         "parameters": [
           {
             "type": "string",

toolkit/components/extensions/schemas/web_navigation.json

@@ -39,6 +39,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Retrieves information about the given frame. A frame refers to an <iframe> or a <frame> of a web page and is identified by a tab ID and a frame ID.",
+        "async": "callback",
         "parameters": [
           {
             "type": "object",
@@ -51,7 +52,9 @@
             }
           },
           {
-            "type": "function", "name": "callback", "parameters": [
+            "type": "function",
+            "name": "callback",
+            "parameters": [
               {
                 "type": "object",
                 "name": "details",
@@ -81,6 +84,7 @@
         "unsupported": true,
         "type": "function",
         "description": "Retrieves information about all frames of a given tab.",
+        "async": "callback",
         "parameters": [
           {
             "type": "object",
@@ -91,7 +95,9 @@
             }
           },
           {
-            "type": "function", "name": "callback", "parameters": [
+            "type": "function",
+            "name": "callback",
+            "parameters": [
               {
                 "name": "details",
                 "type": "array",

toolkit/components/extensions/schemas/web_request.json

@@ -190,8 +190,14 @@
         "name": "handlerBehaviorChanged",
         "type": "function",
         "description": "Needs to be called when the behavior of the webRequest handlers has changed to prevent incorrect handling due to caching. This function call is expensive. Don't call it often.",
+        "async": "callback",
         "parameters": [
-          {"type": "function", "name": "callback", "optional": true, "parameters": []}
+          {
+            "type": "function",
+            "name": "callback",
+            "optional": true,
+            "parameters": []
+          }
         ]
       }
     ],