Bug 1259944 Fix runtime.sendMessage() handling of 2 arguments r=zombie

MozReview-Commit-ID: AefmoEfy12j

--HG--
extra : rebase_source : 56c3f6b05199bc82dc27808e9ae598cb69e69ed0

toolkit/components/extensions/ext-c-runtime.js

@@ -22,21 +22,49 @@ this.runtime = class extends ExtensionAPI {
           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]) {
-              [extensionId, message] = args;
-            } else {
+            // 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 if (args.length === 3) {
             [extensionId, message, options] = args;
@@ -54,14 +82,11 @@ this.runtime = class extends ExtensionAPI {
           let recipient = {extensionId};
 
           if (options != null) {
-            if (typeof options !== "object") {
-              return Promise.reject({message: "runtime.sendMessage's options argument is invalid"});
-            }
-            if (typeof options.toProxyScript === "boolean") {
-              recipient.toProxyScript = options.toProxyScript;
-            } else {
-              return Promise.reject({message: "runtime.sendMessage's options.toProxyScript argument is invalid"});
+            let [valid, arg] = checkOptions(options);
+            if (!valid) {
+              return Promise.reject({message: arg});
             }
+            Object.assign(recipient, arg);
           }
 
           return context.messenger.sendMessage(context.messageManager, message, recipient, responseCallback);

toolkit/components/extensions/schemas/runtime.json

@@ -341,7 +341,7 @@
             "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

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()]);
+});

toolkit/components/extensions/test/xpcshell/xpcshell.ini

@@ -61,6 +61,7 @@ skip-if = "android" # Bug 1350559
 [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]