Bug 1356546: Part 2 - Use StructuredCloneHolder as transport for MessageManager messages. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Sun, 04 Jun 2017 20:46:38 -0700
changeset 412784 850918e6790b80ded3d5b70d4fdb845b6cb24408
parent 412783 f18f7ef1d81f273f6d0b9b4afd1e0980b65ae523
child 412785 fe952a030435f0690bd4f64d6299201b9734cb47
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1356546
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1356546: Part 2 - Use StructuredCloneHolder as transport for MessageManager messages. r=aswan MozReview-Commit-ID: 3z1uAAbsgTj
toolkit/components/extensions/.eslintrc.js
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/NativeMessaging.jsm
toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
toolkit/components/extensions/test/xpcshell/test_native_messaging.js
--- a/toolkit/components/extensions/.eslintrc.js
+++ b/toolkit/components/extensions/.eslintrc.js
@@ -2,16 +2,17 @@
 
 module.exports = {
 
   "globals": {
     "Cc": true,
     "Ci": true,
     "Cr": true,
     "Cu": true,
+    "StructuredCloneHolder": false,
     "TextDecoder": false,
     "TextEncoder": false,
 
     "MatchGlob": false,
     "MatchPattern": true,
     "MatchPatternSet": false,
     "WebExtensionContentScript": false,
     "WebExtensionPolicy": false,
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -129,25 +129,26 @@ class Port {
         this.disconnect();
       },
 
       postMessage: json => {
         this.postMessage(json);
       },
 
       onDisconnect: new SingletonEventManager(this.context, "Port.onDisconnect", fire => {
-        return this.registerOnDisconnect(error => {
+        return this.registerOnDisconnect(holder => {
+          let error = holder.deserialize(this.context.cloneScope);
           portError = error && this.context.normalizeError(error);
           fire.asyncWithoutClone(portObj);
         });
       }).api(),
 
       onMessage: new SingletonEventManager(this.context, "Port.onMessage", fire => {
-        return this.registerOnMessage(msg => {
-          msg = Cu.cloneInto(msg, this.context.cloneScope);
+        return this.registerOnMessage(holder => {
+          let msg = holder.deserialize(this.context.cloneScope);
           fire.asyncWithoutClone(msg, portObj);
         });
       }).api(),
 
       get error() {
         return portError;
       },
     };
@@ -216,17 +217,19 @@ class Port {
   }
 
   _sendMessage(message, data) {
     let options = {
       recipient: Object.assign({}, this.recipient, {portId: this.id}),
       responseType: MessageChannel.RESPONSE_NONE,
     };
 
-    return this.context.sendMessage(this.senderMM, message, data, options);
+    let holder = new StructuredCloneHolder(data);
+
+    return this.context.sendMessage(this.senderMM, message, holder, options);
   }
 
   handleDisconnection() {
     MessageChannel.removeListener(this.receiverMMs, "Extension:Port:Disconnect", this.disconnectHandler);
     for (let unregister of this.unregisterMessageFuncs) {
       unregister();
     }
     this.context.forgetOnClose(this);
@@ -321,17 +324,19 @@ class Messenger {
       sender: this.sender,
       responseType: MessageChannel.RESPONSE_FIRST,
     };
 
     return this.context.sendMessage(messageManager, message, data, options);
   }
 
   sendMessage(messageManager, msg, recipient, responseCallback) {
-    let promise = this._sendMessage(messageManager, "Extension:Message", msg, recipient)
+    let holder = new StructuredCloneHolder(msg);
+
+    let promise = this._sendMessage(messageManager, "Extension:Message", holder, recipient)
       .catch(error => {
         if (error.result == MessageChannel.RESULT_NO_HANDLER) {
           return Promise.reject({message: "Could not establish connection. Receiving end does not exist."});
         } else if (error.result != MessageChannel.RESULT_NO_RESPONSE) {
           return Promise.reject({message: error.message});
         }
       });
 
@@ -350,31 +355,31 @@ class Messenger {
         messageFilterStrict: this.filter,
 
         filterMessage: (sender, recipient) => {
           // Ignore the message if it was sent by this Messenger.
           return (sender.contextId !== this.context.contextId &&
                   filter(sender, recipient));
         },
 
-        receiveMessage: ({target, data: message, sender, recipient}) => {
+        receiveMessage: ({target, data: holder, sender, recipient}) => {
           if (!this.context.active) {
             return;
           }
 
           let sendResponse;
           let response = undefined;
           let promise = new Promise(resolve => {
             sendResponse = value => {
               resolve(value);
               response = promise;
             };
           });
 
-          message = Cu.cloneInto(message, this.context.cloneScope);
+          let message = holder.deserialize(this.context.cloneScope);
           sender = Cu.cloneInto(sender, this.context.cloneScope);
           sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
 
           // Note: We intentionally do not use runSafe here so that any
           // errors are propagated to the message sender.
           let result = fire.raw(message, sender, sendResponse);
           if (result instanceof this.context.cloneScope.Promise) {
             return result;
@@ -407,17 +412,17 @@ class Messenger {
     };
 
     this._sendMessage(messageManager, "Extension:Connect", msg, recipient).catch(error => {
       if (error.result === MessageChannel.RESULT_NO_HANDLER) {
         error = {message: "Could not establish connection. Receiving end does not exist."};
       } else if (error.result === MessageChannel.RESULT_DISCONNECTED) {
         error = null;
       }
-      port.disconnectByOtherEnd(error);
+      port.disconnectByOtherEnd(new StructuredCloneHolder(error));
     });
 
     return port.api();
   }
 
   connect(messageManager, name, recipient) {
     let portId = getUniqueId();
 
--- a/toolkit/components/extensions/NativeMessaging.jsm
+++ b/toolkit/components/extensions/NativeMessaging.jsm
@@ -54,16 +54,18 @@ const MAX_WRITE = 0xffffffff;
 
 // Preferences that can lower the message size limits above,
 // used for testing the limits.
 const PREF_MAX_READ = "webextensions.native-messaging.max-input-message-bytes";
 const PREF_MAX_WRITE = "webextensions.native-messaging.max-output-message-bytes";
 
 const REGPATH = "Software\\Mozilla\\NativeMessagingHosts";
 
+const global = this;
+
 this.HostManifestManager = {
   _initializePromise: null,
   _lookup: null,
 
   init() {
     if (!this._initializePromise) {
       let platform = AppConstants.platform;
       if (platform == "win") {
@@ -231,17 +233,17 @@ this.NativeApp = class extends EventEmit
     let app = new NativeApp(context, application);
     let port = new ExtensionChild.Port(context, messageManager, [Services.mm], "", portId, sender, sender);
     app.once("disconnect", (what, err) => port.disconnect(err));
 
     /* eslint-disable mozilla/balanced-listeners */
     app.on("message", (what, msg) => port.postMessage(msg));
     /* eslint-enable mozilla/balanced-listeners */
 
-    port.registerOnMessage(msg => app.send(msg));
+    port.registerOnMessage(holder => app.send(holder));
     port.registerOnDisconnect(msg => app.close());
   }
 
   /**
    * @param {BaseContext} context The scope from where `message` originates.
    * @param {*} message A message from the extension, meant for a native app.
    * @returns {ArrayBuffer} An ArrayBuffer that can be sent to the native app.
    */
@@ -329,20 +331,21 @@ this.NativeApp = class extends EventEmit
 
         for (let line of lines) {
           Services.console.logStringMessage(`stderr output from native app ${app}: ${line}`);
         }
       }
     })();
   }
 
-  send(msg) {
+  send(holder) {
     if (this._isDisconnected) {
       throw new this.context.cloneScope.Error("Attempt to postMessage on disconnected port");
     }
+    let msg = holder.deserialize(global);
     if (Cu.getClassName(msg, true) != "ArrayBuffer") {
       // This error cannot be triggered by extensions; it indicates an error in
       // our implementation.
       throw new Error("The message to the native messaging host is not an ArrayBuffer");
     }
 
     let buffer = msg;
 
@@ -407,24 +410,24 @@ this.NativeApp = class extends EventEmit
     }
   }
 
   // Called from Context when the extension is shut down.
   close() {
     this._cleanup();
   }
 
-  sendMessage(msg) {
+  sendMessage(holder) {
     let responsePromise = new Promise((resolve, reject) => {
       this.once("message", (what, msg) => { resolve(msg); });
       this.once("disconnect", (what, err) => { reject(err); });
     });
 
     let result = this.startupPromise.then(() => {
-      this.send(msg);
+      this.send(holder);
       return responsePromise;
     });
 
     result.then(() => {
       this._cleanup();
     }, () => {
       // Prevent the response promise from being reported as an
       // unchecked rejection if the startup promise fails.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage_errors.js
@@ -13,25 +13,19 @@ add_task(async function test_sendMessage
       [[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."],
 
-      // Structural cloning doesn't work with DOM but we fall back
-      // JSON serialization, so we don't expect another error.
-      [[null, location, null], "Could not establish connection. Receiving end does not exist."],
-
-      // Structured cloning supports cyclic self-references.
-      [[null, [circ, location], null], "cyclic object value"],
-      // JSON serialization does not support cyclic references.
-      [[null, circ, null], "Could not establish connection. Receiving end does not exist."],
-      // (the last two tests shows whether sendMessage is implemented as structured cloning).
+      // Structured cloning doesn't work with DOM objects
+      [[null, location, null], "The object could not be cloned."],
+      [[null, [circ, location], null], "The object could not be cloned."],
     ];
 
     // Repeat all tests with the undefined value instead of null.
     for (let [args, expectedError] of testCases.slice()) {
       args = args.map(arg => arg === null ? undefined : arg);
       testCases.push([args, expectedError]);
     }
 
--- a/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
+++ b/toolkit/components/extensions/test/xpcshell/test_native_messaging.js
@@ -282,17 +282,17 @@ while True:
       equal(msg, MSG, "Received test message");
       app.off("message", listener);
       resolve();
     };
     app.on("message", listener);
   });
 
   let buffer = NativeApp.encodeMessage(mockContext, MSG);
-  app.send(buffer);
+  app.send(new StructuredCloneHolder(buffer));
   await recvPromise;
 
   app._cleanup();
 
   do_print("waiting for async shutdown");
   Services.prefs.setBoolPref("toolkit.asyncshutdown.testing", true);
   AsyncShutdown.profileBeforeChange._trigger();
   Services.prefs.clearUserPref("toolkit.asyncshutdown.testing");