Bug 1295473 - Fix return type of {tabs,runtime}.sendMessage r=billm
authorRob Wu <rob@robwu.nl>
Mon, 15 Aug 2016 23:53:24 -0700
changeset 310108 c50cb95d1f42420df51b190835aa82e2ec7fd552
parent 310106 ba84914a7e70aa4d61aa1b8ca4796fae7172dfa7
child 310109 30da32629a3c6ef74b8c66f819c08b1106f6da91
push id80771
push userkwierso@gmail.com
push dateThu, 18 Aug 2016 23:33:08 +0000
treeherdermozilla-inbound@cb1295738c37 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1295473
milestone51.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1295473 - Fix return type of {tabs,runtime}.sendMessage r=billm 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.
@@ -1425,18 +1430,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
@@ -290,16 +290,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_tab_teardown.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();
+});