Bug 1299411 - Pass port parameter to port.onMessage r=kmag
authorRob Wu <rob@robwu.nl>
Sat, 24 Sep 2016 11:34:26 +0200
changeset 320059 502aaf0691ccecb5988d719cf904fb52c992dce3
parent 320058 a41f871e2d1b37754bbd1001c36c075511b49342
child 320060 e08ee4c2b1e2a0f9020f9a02f5c0ded4b1aa4ab2
push id20749
push userryanvm@gmail.com
push dateSat, 29 Oct 2016 13:21:21 +0000
treeherderfx-team@1b170b39ed6b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1299411, 1298810
milestone52.0a1
Bug 1299411 - Pass port parameter to port.onMessage r=kmag 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;
     }