Bug 1295473 - Fix return type of {tabs,runtime}.sendMessage draft
authorRob Wu <rob@robwu.nl>
Mon, 15 Aug 2016 23:53:24 -0700
changeset 401076 54f9f38a534617bd9608616035768f7b5ac5fddc
parent 400444 6e191a55c3d23e83e6a2e72e4e80c1dc21516493
child 528387 8fb1636d6ff7a482735e69383df83186a5f1cb5b
push id26353
push userbmo:rob@robwu.nl
push dateTue, 16 Aug 2016 06:58:17 +0000
bugs1295473
milestone51.0a1
Bug 1295473 - Fix return type of {tabs,runtime}.sendMessage The tabs.sendMessage and runtime.sendMessage implementations behave like an async function: They take a callback parameter and return a promise. So they should be handled by |callAsyncFunction|, not |callFunctionNoReturn|. This fixes the issue for background pages, but not for content scripts because sendMessage is not implemented as a schema at the moment. This will also be fixed once content script APIs are generated via Schemas. MozReview-Commit-ID: 9p1hvOP0KSm
browser/components/extensions/schemas/tabs.json
toolkit/components/extensions/Schemas.jsm
toolkit/components/extensions/schemas/runtime.json
toolkit/components/extensions/test/mochitest/mochitest.ini
toolkit/components/extensions/test/mochitest/test_ext_sendmessage_no_receiver.html
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_no_receiver.js
--- a/browser/components/extensions/schemas/tabs.json
+++ b/browser/components/extensions/schemas/tabs.json
@@ -250,16 +250,17 @@
             ]
           }
         ]
       },
       {
         "name": "sendMessage",
         "type": "function",
         "description": "Sends a single message to the content script(s) in the specified tab, with an optional callback to run when a response is sent back.  The $(ref:runtime.onMessage) event is fired in each content script running in the specified tab for the current extension.",
+        "async": "sendResponse",
         "parameters": [
           {
             "type": "integer",
             "name": "tabId",
             "minimum": 0
           },
           {
             "type": "any",
--- a/toolkit/components/extensions/Schemas.jsm
+++ b/toolkit/components/extensions/Schemas.jsm
@@ -1117,16 +1117,21 @@ class CallEntry extends Entry {
       }
 
       return check(parameterIndex + 1, argIndex + 1);
     };
 
     if (this.allowAmbiguousOptionalArguments) {
       // When this option is set, it's up to the implementation to
       // parse arguments.
+      // The last argument for asynchronous methods is either a function or null.
+      // This is specifically done for runtime.sendMessage.
+      if (this.hasAsyncCallback && typeof(args[args.length - 1]) != "function") {
+        args.push(null);
+      }
       return args;
     }
     let success = check(0, 0);
     if (!success) {
       this.throwError(context, "Incorrect argument types");
     }
 
     // Now we normalize (and fully type check) all non-omitted arguments.
@@ -1419,18 +1424,21 @@ this.Schemas = {
         }
       }
 
       let hasAsyncCallback = false;
       if (isAsync) {
         if (parameters && parameters.length && parameters[parameters.length - 1].name == type.async) {
           hasAsyncCallback = true;
         }
-        if (type.returns || type.allowAmbiguousOptionalArguments) {
-          throw new Error(`Internal error: Async functions must not have return values or ambiguous arguments.`);
+        if (type.returns) {
+          throw new Error("Internal error: Async functions must not have return values.");
+        }
+        if (type.allowAmbiguousOptionalArguments && !hasAsyncCallback) {
+          throw new Error("Internal error: Async functions with ambiguous arguments must declare the callback as the last parameter");
         }
       }
 
       checkTypeProperties("parameters", "async", "returns");
       return new FunctionType(type, parameters, isAsync, hasAsyncCallback);
     } else if (type.type == "any") {
       // Need to see what minimum and maximum are supposed to do here.
       checkTypeProperties("minimum", "maximum");
--- a/toolkit/components/extensions/schemas/runtime.json
+++ b/toolkit/components/extensions/schemas/runtime.json
@@ -291,16 +291,17 @@
           "description": "Port through which messages can be sent and received with the application"
         }
       },
       {
         "name": "sendMessage",
         "type": "function",
         "allowAmbiguousOptionalArguments": true,
         "description": "Sends a single message to event listeners within your extension/app or a different extension/app. Similar to $(ref:runtime.connect) but only sends a single message, with an optional response. If sending to your extension, the $(ref:runtime.onMessage) event will be fired in each page, or $(ref:runtime.onMessageExternal), if a different extension. Note that extensions cannot send messages to content scripts using this method. To send messages to content scripts, use $(ref:tabs.sendMessage).",
+        "async": "responseCallback",
         "parameters": [
           {"type": "string", "name": "extensionId", "optional": true, "description": "The ID of the extension/app to send the message to. If omitted, the message will be sent to your own extension/app. Required if sending messages from a web page for $(topic:manifest/externally_connectable)[web messaging]."},
           { "type": "any", "name": "message" },
           {
             "type": "object",
             "name": "options",
             "properties": {
               "includeTlsChannelId": { "type": "boolean", "optional": true, "description": "Whether the TLS channel ID will be passed into onMessageExternal for processes that are listening for the connection event." }
--- a/toolkit/components/extensions/test/mochitest/mochitest.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest.ini
@@ -69,16 +69,17 @@ skip-if = (os == 'android' || buildapp =
 [test_ext_runtime_id.html]
 [test_ext_sandbox_var.html]
 [test_ext_sendmessage_reply.html]
 skip-if = (os == 'android' || buildapp == 'b2g') # sender.tab is undefined on b2g. Bug 1258975 on android.
 [test_ext_sendmessage_reply2.html]
 skip-if = (os == 'android' || buildapp == 'b2g') # sender.tab is undefined on b2g. Bug 1258975 on android.
 [test_ext_sendmessage_doublereply.html]
 skip-if = (os == 'android' || buildapp == 'b2g') # sender.tab is undefined on b2g. Bug 1258975 on android.
+[test_ext_sendmessage_no_receiver.html]
 [test_ext_storage_content.html]
 [test_ext_storage_tab.html]
 skip-if = os == 'android' # Android does not currently support tabs.
 [test_ext_cookies.html]
 [test_ext_background_api_injection.html]
 [test_ext_background_generated_url.html]
 [test_ext_background_teardown.html]
 [test_ext_i18n.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_sendmessage_no_receiver.html
@@ -0,0 +1,85 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <title>WebExtension test</title>
+  <meta charset="utf-8">
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css">
+</head>
+<body>
+<script>
+"use strict";
+
+function loadContentScriptExtension(contentScript) {
+  let extensionData = {
+    manifest: {
+      "content_scripts": [{
+        "js": ["contentscript.js"],
+        "matches": ["http://mochi.test/*/file_sample.html"],
+      }],
+    },
+    files: {
+      "contentscript.js": `(${contentScript})();`,
+    },
+  };
+  return ExtensionTestUtils.loadExtension(extensionData);
+}
+
+add_task(function* test_content_script_sendMessage_without_listener() {
+  function contentScript() {
+    browser.runtime.sendMessage("msg").then(reply => {
+      browser.test.assertEq(undefined, reply);
+      browser.test.notifyFail("Did not expect a reply to sendMessage");
+    }, error => {
+      browser.test.assertEq("Could not establish connection. Receiving end does not exist.", error.message);
+      browser.test.notifyPass("sendMessage callback was invoked");
+    });
+  }
+
+  let extension = loadContentScriptExtension(contentScript);
+  yield extension.startup();
+
+  let win = window.open("file_sample.html");
+  yield extension.awaitFinish("sendMessage callback was invoked");
+  win.close();
+
+  yield extension.unload();
+});
+
+add_task(function* test_content_script_chrome_sendMessage_without_listener() {
+  function contentScript() {
+    /* globals chrome */
+    browser.test.assertEq(null, chrome.runtime.lastError, "no lastError before call");
+    let retval = chrome.runtime.sendMessage("msg");
+    browser.test.assertEq(null, chrome.runtime.lastError, "no lastError after call");
+    // TODO(robwu): Fix the implementation and uncomment the next expectation.
+    // When content script APIs are schema-based (bugzil.la/1287007) this bug will be fixed for free.
+    // browser.test.assertEq(undefined, retval, "return value of chrome.runtime.sendMessage without callback");
+    browser.test.assertTrue(retval instanceof Promise, "TODO: chrome.runtime.sendMessage should return undefined, not a promise");
+
+    let isAsyncCall = false;
+    retval = chrome.runtime.sendMessage("msg", reply => {
+      browser.test.assertEq(undefined, reply, "no reply");
+      browser.test.assertTrue(isAsyncCall, "chrome.runtime.sendMessage's callback must be called asynchronously");
+      browser.test.assertEq(undefined, retval, "return value of chrome.runtime.sendMessage with callback");
+      browser.test.assertEq("Could not establish connection. Receiving end does not exist.", chrome.runtime.lastError.message);
+      browser.test.notifyPass("finished chrome.runtime.sendMessage");
+    });
+    isAsyncCall = true;
+  }
+
+  let extension = loadContentScriptExtension(contentScript);
+  yield extension.startup();
+
+  let win = window.open("file_sample.html");
+  yield extension.awaitFinish("finished chrome.runtime.sendMessage");
+  win.close();
+
+  yield extension.unload();
+});
+</script>
+</body>
+</html>
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_no_receiver.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_no_receiver.js
@@ -18,8 +18,38 @@ add_task(function* test_sendMessage_with
 
   let extension = ExtensionTestUtils.loadExtension(extensionData);
   yield extension.startup();
 
   yield extension.awaitFinish("sendMessage callback was invoked");
 
   yield extension.unload();
 });
+
+add_task(function* test_chrome_sendMessage_without_listener() {
+  function background() {
+    /* globals chrome */
+    browser.test.assertEq(null, chrome.runtime.lastError, "no lastError before call");
+    let retval = chrome.runtime.sendMessage("msg");
+    browser.test.assertEq(null, chrome.runtime.lastError, "no lastError after call");
+    browser.test.assertEq(undefined, retval, "return value of chrome.runtime.sendMessage without callback");
+
+    let isAsyncCall = false;
+    retval = chrome.runtime.sendMessage("msg", reply => {
+      browser.test.assertEq(undefined, reply, "no reply");
+      browser.test.assertTrue(isAsyncCall, "chrome.runtime.sendMessage's callback must be called asynchronously");
+      browser.test.assertEq(undefined, retval, "return value of chrome.runtime.sendMessage with callback");
+      browser.test.assertEq("Could not establish connection. Receiving end does not exist.", chrome.runtime.lastError.message);
+      browser.test.notifyPass("finished chrome.runtime.sendMessage");
+    });
+    isAsyncCall = true;
+  }
+  let extensionData = {
+    background,
+  };
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  yield extension.startup();
+
+  yield extension.awaitFinish("finished chrome.runtime.sendMessage");
+
+  yield extension.unload();
+});