Bug 1299411 - Pass port parameter to port.onMessage draft
authorRob Wu <rob@robwu.nl>
Sat, 24 Sep 2016 11:34:26 +0200
changeset 429204 b73635f95a04da1fe572fade4a2e00322c7bb521
parent 429203 488311924964a6a0de79665371ebe5fdc95de261
child 429205 79017d9c912697630d4354e748c8328b8c0b7737
push id33512
push userbmo:rob@robwu.nl
push dateTue, 25 Oct 2016 14:01:58 +0000
bugs1299411, 1298810
milestone52.0a1
Bug 1299411 - Pass port parameter to port.onMessage This should have been a part of bug 1298810, but that only set the argument for native messaging ports, which does not use Port from ExtensionUtils. The port parameter must also be included in runtime's Port.onMessage to avoid regressions when the port implementations are unified and native messaging starts using runtime's Port. Note that starting from this commit, multiple onMessage listeners receive the same (cloned) message instead of a new clone per listener. This is a side effect of using `fire.withoutClone` instead of `fire`: `fire` clones all parameters, but ports are not cloneable so we have to use `fire.withoutClone` instead. This change with regards to message cloning is fully compatible with Chrome's messaging API (which also passes the same message object to all `port.onMessage` calls). MozReview-Commit-ID: AUDuUKHkXCM
browser/components/extensions/test/browser/browser_ext_contentscript_connect.js
toolkit/components/extensions/ExtensionUtils.jsm
--- a/browser/components/extensions/test/browser/browser_ext_contentscript_connect.js
+++ b/browser/components/extensions/test/browser/browser_ext_contentscript_connect.js
@@ -15,31 +15,33 @@ add_task(function* () {
       let port_messages_received = 0;
 
       browser.runtime.onConnect.addListener((port) => {
         browser.test.assertTrue(!!port, "port1 received");
 
         ports_received++;
         browser.test.assertEq(1, ports_received, "1 port received");
 
-        port.onMessage.addListener((msg, sender) => {
+        port.onMessage.addListener((msg, msgPort) => {
           browser.test.assertEq("port message", msg, "listener1 port message received");
+          browser.test.assertEq(port, msgPort, "onMessage should receive port as second argument");
 
           port_messages_received++;
           browser.test.assertEq(1, port_messages_received, "1 port message received");
         });
       });
       browser.runtime.onConnect.addListener((port) => {
         browser.test.assertTrue(!!port, "port2 received");
 
         ports_received++;
         browser.test.assertEq(2, ports_received, "2 ports received");
 
-        port.onMessage.addListener((msg, sender) => {
+        port.onMessage.addListener((msg, msgPort) => {
           browser.test.assertEq("port message", msg, "listener2 port message received");
+          browser.test.assertEq(port, msgPort, "onMessage should receive port as second argument");
 
           port_messages_received++;
           browser.test.assertEq(2, port_messages_received, "2 port messages received");
 
           browser.test.notifyPass("contentscript_connect.pass");
         });
       });
 
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -1257,17 +1257,18 @@ Port.prototype = {
       postMessage: json => {
         this.postMessage(json);
       },
       onDisconnect: new EventManager(this.context, "Port.onDisconnect", fire => {
         return this.registerOnDisconnect(() => fire.withoutClone(portObj));
       }).api(),
       onMessage: new EventManager(this.context, "Port.onMessage", fire => {
         return this.registerOnMessage(msg => {
-          fire(msg);
+          msg = Cu.cloneInto(msg, this.context.cloneScope);
+          fire.withoutClone(msg, portObj);
         });
       }).api(),
     };
 
     if (this.sender) {
       publicAPI.sender = this.sender;
     }