Bug 1259944 Fix runtime.sendMessage() handling of 2 arguments draft
authorAndrew Swan <aswan@mozilla.com>
Mon, 01 May 2017 10:08:56 -0700
changeset 573373 9f20d1b7fee36791dabc5a37d175e84da9f257fe
parent 571808 82c2d17e74ef9cdf38a5d5ac4eb3ae846ec30ba4
child 627283 611fe22c46e57dd8601baaf42542969f23fe8a61
push id57370
push useraswan@mozilla.com
push dateFri, 05 May 2017 17:08:30 +0000
bugs1259944
milestone55.0a1
Bug 1259944 Fix runtime.sendMessage() handling of 2 arguments MozReview-Commit-ID: AefmoEfy12j
toolkit/components/extensions/ext-c-runtime.js
toolkit/components/extensions/schemas/runtime.json
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_args.js
toolkit/components/extensions/test/xpcshell/xpcshell.ini
--- a/toolkit/components/extensions/ext-c-runtime.js
+++ b/toolkit/components/extensions/ext-c-runtime.js
@@ -17,31 +17,59 @@ this.runtime = class extends ExtensionAP
         connect: function(extensionId, connectInfo) {
           let name = (connectInfo !== null && connectInfo.name) || "";
           extensionId = extensionId || extension.id;
           let recipient = {extensionId};
 
           return context.messenger.connect(context.messageManager, name, recipient);
         },
 
-        sendMessage: function(...args) {
-          let options; // eslint-disable-line no-unused-vars
-          let extensionId, message, responseCallback;
+        sendMessage(...args) {
+          let extensionId, message, options, responseCallback;
           if (typeof args[args.length - 1] === "function") {
             responseCallback = args.pop();
           }
+
+          function checkOptions(options) {
+            let toProxyScript = false;
+            if (typeof options !== "object") {
+              return [false, "runtime.sendMessage's options argument is invalid"];
+            }
+
+            for (let key of Object.keys(options)) {
+              if (key === "toProxyScript") {
+                let value = options[key];
+                if (typeof value !== "boolean") {
+                  return [false, "runtime.sendMessage's options.toProxyScript argument is invalid"];
+                }
+                toProxyScript = value;
+              } else {
+                return [false, `Unexpected property ${key}`];
+              }
+            }
+
+            return [true, {toProxyScript}];
+          }
+
           if (!args.length) {
             return Promise.reject({message: "runtime.sendMessage's message argument is missing"});
           } else if (args.length === 1) {
             message = args[0];
           } else if (args.length === 2) {
-            if (typeof args[0] === "string" && args[0]) {
+            // With two optional arguments, this is the ambiguous case,
+            // particularly sendMessage("string", {});
+            // Given that sending a message within the extension is generally
+            // more common than sending the empty object to another extension,
+            // we prefer that conclusion, as long as the second argument looks
+            // like valid options.
+            let [validOpts] = checkOptions(args[1]);
+            if (validOpts) {
+              [message, options] = args;
+            } else {
               [extensionId, message] = args;
-            } else {
-              [message, options] = args;
             }
           } else if (args.length === 3) {
             [extensionId, message, options] = args;
           } else if (args.length === 4 && !responseCallback) {
             return Promise.reject({message: "runtime.sendMessage's last argument is not a function"});
           } else {
             return Promise.reject({message: "runtime.sendMessage received too many arguments"});
           }
@@ -49,24 +77,21 @@ this.runtime = class extends ExtensionAP
           if (extensionId != null && typeof extensionId !== "string") {
             return Promise.reject({message: "runtime.sendMessage's extensionId argument is invalid"});
           }
 
           extensionId = extensionId || extension.id;
           let recipient = {extensionId};
 
           if (options != null) {
-            if (typeof options !== "object") {
-              return Promise.reject({message: "runtime.sendMessage's options argument is invalid"});
+            let [valid, arg] = checkOptions(options);
+            if (!valid) {
+              return Promise.reject({message: arg});
             }
-            if (typeof options.toProxyScript === "boolean") {
-              recipient.toProxyScript = options.toProxyScript;
-            } else {
-              return Promise.reject({message: "runtime.sendMessage's options.toProxyScript argument is invalid"});
-            }
+            Object.assign(recipient, arg);
           }
 
           return context.messenger.sendMessage(context.messageManager, message, recipient, responseCallback);
         },
 
         connectNative(application) {
           let recipient = {
             childId: context.childManager.id,
--- a/toolkit/components/extensions/schemas/runtime.json
+++ b/toolkit/components/extensions/schemas/runtime.json
@@ -336,17 +336,17 @@
         "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." },
+              "includeTlsChannelId": { "type": "boolean", "optional": true, "unsupported": true, "description": "Whether the TLS channel ID will be passed into onMessageExternal for processes that are listening for the connection event." },
               "toProxyScript": { "type": "boolean", "optional": true, "description": "If true, the message will be directed to the extension's proxy sandbox."}
             },
             "optional": true
           },
           {
             "type": "function",
             "name": "responseCallback",
             "optional": true,
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_args.js
@@ -0,0 +1,82 @@
+/* -*- Mode: indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set sts=2 sw=2 et tw=80: */
+"use strict";
+
+add_task(async function() {
+  const ID1 = "sendMessage1@tests.mozilla.org";
+  const ID2 = "sendMessage2@tests.mozilla.org";
+
+  let extension1 = ExtensionTestUtils.loadExtension({
+    background() {
+      browser.test.onMessage.addListener((...args) => {
+        browser.runtime.sendMessage(...args);
+      });
+
+      let frame = document.createElement("iframe");
+      frame.src = "page.html";
+      document.body.appendChild(frame);
+    },
+    manifest: {applications: {gecko: {id: ID1}}},
+    files: {
+      "page.js": function() {
+        browser.runtime.onMessage.addListener((msg, sender) => {
+          browser.test.sendMessage("received-page", {msg, sender});
+        });
+      },
+      "page.html": `<!DOCTYPE html><meta charset="utf-8"><script src="page.js"></script>`,
+    },
+  });
+
+  let extension2 = ExtensionTestUtils.loadExtension({
+    background() {
+      browser.runtime.onMessageExternal.addListener((msg, sender) => {
+        browser.test.sendMessage("received-external", {msg, sender});
+      });
+    },
+    manifest: {applications: {gecko: {id: ID2}}},
+  });
+
+  await Promise.all([extension1.startup(), extension2.startup()]);
+
+  // Check that a message was sent within extension1.
+  async function checkLocalMessage(msg) {
+    let result = await extension1.awaitMessage("received-page");
+    deepEqual(result.msg, msg, "Received internal message");
+    equal(result.sender.id, ID1, "Received correct sender id");
+  }
+
+  // Check that a message was sent from extension1 to extension2.
+  async function checkRemoteMessage(msg) {
+    let result = await extension2.awaitMessage("received-external");
+    deepEqual(result.msg, msg, "Received cross-extension message");
+    equal(result.sender.id, ID1, "Received correct sender id");
+  }
+
+  // sendMessage() takes 3 arguments:
+  //  optional extensionID
+  //  mandatory message
+  //  optional options
+  // Due to this insane design we parse its arguments manually.  This
+  // test is meant to cover all the combinations.
+
+  // With one argument, it must be just the message
+  extension1.sendMessage("message");
+  await checkLocalMessage("message");
+
+  // With two arguments, these cases should be treated as (extensionID, message)
+  extension1.sendMessage(ID2, "message");
+  await checkRemoteMessage("message");
+
+  extension1.sendMessage(ID2, {msg: "message"});
+  await checkRemoteMessage({msg: "message"});
+
+  // And this case should be (message, options)
+  extension1.sendMessage("message", {});
+  await checkLocalMessage("message");
+
+  // With three arguments, we send a cross-extension message
+  extension1.sendMessage(ID2, "message", {});
+  await checkRemoteMessage("message");
+
+  await Promise.all([extension1.unload(), extension2.unload()]);
+});
--- a/toolkit/components/extensions/test/xpcshell/xpcshell.ini
+++ b/toolkit/components/extensions/test/xpcshell/xpcshell.ini
@@ -56,16 +56,17 @@ skip-if = "android" # Bug 1350559
 [test_ext_privacy.js]
 [test_ext_privacy_disable.js]
 [test_ext_privacy_update.js]
 [test_ext_runtime_connect_no_receiver.js]
 [test_ext_runtime_getBrowserInfo.js]
 [test_ext_runtime_getPlatformInfo.js]
 [test_ext_runtime_onInstalled_and_onStartup.js]
 [test_ext_runtime_sendMessage.js]
+[test_ext_runtime_sendMessage_args.js]
 [test_ext_runtime_sendMessage_errors.js]
 [test_ext_runtime_sendMessage_no_receiver.js]
 [test_ext_runtime_sendMessage_self.js]
 [test_ext_schemas.js]
 [test_ext_schemas_async.js]
 [test_ext_schemas_allowed_contexts.js]
 [test_ext_schemas_revoke.js]
 [test_ext_shutdown_cleanup.js]