Bug 1368669 - Support explicit null callback for runtime.sendMessage r?aswan draft
authorTomislav Jovanovic <tomica@gmail.com>
Wed, 07 Jun 2017 23:43:25 +0200
changeset 591876 177908f1829b57138a772d55c273cbcad94e579c
parent 589682 f781899dd0257002f806927a3474261aae3deafb
child 632649 9a7e07501165bdbb7a305d5955e19d1021d38505
push id63200
push userbmo:tomica@gmail.com
push dateFri, 09 Jun 2017 17:07:43 +0000
reviewersaswan
bugs1368669
milestone55.0a1
Bug 1368669 - Support explicit null callback for runtime.sendMessage r?aswan MozReview-Commit-ID: Lauyk877pIQ
toolkit/components/extensions/ext-c-runtime.js
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_args.js
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
--- a/toolkit/components/extensions/ext-c-runtime.js
+++ b/toolkit/components/extensions/ext-c-runtime.js
@@ -19,16 +19,17 @@ this.runtime = class extends ExtensionAP
           extensionId = extensionId || extension.id;
           let recipient = {extensionId};
 
           return context.messenger.connect(context.messageManager, name, recipient);
         },
 
         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"];
@@ -50,28 +51,28 @@ this.runtime = class extends ExtensionAP
           }
 
           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) {
             // With two optional arguments, this is the ambiguous case,
-            // particularly sendMessage("string", {});
+            // particularly sendMessage("string", {} or null)
             // 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.
+            // like valid options object, or is null/undefined.
             let [validOpts] = checkOptions(args[1]);
-            if (validOpts) {
+            if (validOpts || args[1] == null) {
               [message, options] = args;
             } else {
               [extensionId, message] = args;
             }
-          } else if (args.length === 3) {
+          } else if (args.length === 3 || (args.length === 4 && args[3] == null)) {
             [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"});
           }
 
           if (extensionId != null && typeof extensionId !== "string") {
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_args.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_args.js
@@ -54,29 +54,45 @@ add_task(async function() {
 
   // 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.
 
+  // A single null or undefined argument is allowed, and represents the message
+  extension1.sendMessage(null);
+  await checkLocalMessage(null);
+
   // 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)
+  // And these should be (message, options)
   extension1.sendMessage("message", {});
   await checkLocalMessage("message");
 
+  // or (message, non-callback), pick your poison
+  extension1.sendMessage("message", undefined);
+  await checkLocalMessage("message");
+
   // With three arguments, we send a cross-extension message
   extension1.sendMessage(ID2, "message", {});
   await checkRemoteMessage("message");
 
+  // Even when the last one is null or undefined
+  extension1.sendMessage(ID2, "message", undefined);
+  await checkRemoteMessage("message");
+
+  // The four params case is unambigous, so we allow null as a (non-) callback
+  extension1.sendMessage(ID2, "message", {}, null);
+  await checkRemoteMessage("message");
+
   await Promise.all([extension1.unload(), extension2.unload()]);
 });
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
@@ -4,17 +4,17 @@
 
 add_task(async function test_sendMessage_error() {
   async function background() {
     let circ = {};
     circ.circ = circ;
     let testCases = [
       // [arguments, expected error string],
       [[], "runtime.sendMessage's message argument is missing"],
-      [[null, null, null, null], "runtime.sendMessage's last argument is not a function"],
+      [[null, null, null, 42], "runtime.sendMessage's last argument is not a function"],
       [[null, null, 1], "runtime.sendMessage's options argument is invalid"],
       [[1, null, null], "runtime.sendMessage's extensionId argument is invalid"],
       [[null, null, null, null, null], "runtime.sendMessage received too many arguments"],
 
       // Even when the parameters are accepted, we still expect an error
       // because there is no onMessage listener.
       [[null, null, null], "Could not establish connection. Receiving end does not exist."],