Bug 1356546: Part 2 - Use StructuredCloneHolder as transport for MessageManager messages. r=aswan draft
authorKris Maglione <maglione.k@gmail.com>
Wed, 10 May 2017 17:51:20 -0700
changeset 582062 af66552457b99e1955ba30d3105bdcfc0af17333
parent 582061 9447019c097e0d4cde261b2c784b87f8e90c0a60
child 582063 53fa565137d7f676c9da7ef6a3e992f7a515f168
push id59956
push usermaglione.k@gmail.com
push dateSat, 20 May 2017 22:13:12 +0000
reviewersaswan
bugs1356546
milestone55.0a1
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
--- 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,
     // Specific to WebExtensions:
     "AppConstants": true,
     "Extension": true,
     "ExtensionAPI": true,
     "ExtensionManagement": true,
     "ExtensionUtils": true,
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -114,25 +114,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;
       },
     };
@@ -201,17 +202,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);
@@ -306,17 +309,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});
         }
       });
 
@@ -335,31 +340,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;
--- a/toolkit/components/extensions/NativeMessaging.jsm
+++ b/toolkit/components/extensions/NativeMessaging.jsm
@@ -56,16 +56,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") {
@@ -233,17 +235,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.
    */
@@ -331,20 +333,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;
 
@@ -409,24 +412,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(function* test_sendMessage_erro
       [[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]);
     }