Bug 1453385 - Throw error when RDP message can't be serialized in message manager. r=jryans draft
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 12 Apr 2018 04:44:12 -0700
changeset 781941 49b570f02f1d2bd7d4eccd2fd8e691be83524fe8
parent 781940 b0709b05690ca59c34c9bbb76f12a370c4660862
child 781942 a7903d7cc2357419d24c39ba1dca767fdba3f5d4
push id106451
push userbmo:poirot.alex@gmail.com
push dateFri, 13 Apr 2018 20:48:33 +0000
reviewersjryans
bugs1453385
milestone61.0a1
Bug 1453385 - Throw error when RDP message can't be serialized in message manager. r=jryans If the packet contains a function or anything that StructureClone doesn't support, Message manager is going to stringify and parse the packet via JSON API. This can be a performance issue as it will duplicate the object. MozReview-Commit-ID: EZC1BU1Ps7Y
devtools/shared/Loader.jsm
devtools/shared/builtin-modules.js
devtools/shared/transport/transport.js
--- a/devtools/shared/Loader.jsm
+++ b/devtools/shared/Loader.jsm
@@ -9,17 +9,19 @@
  */
 
 var { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm", {});
 var { Loader, Require, resolveURI, unload } =
   ChromeUtils.import("resource://devtools/shared/base-loader.js", {});
 var { requireRawId } = ChromeUtils.import("resource://devtools/shared/loader-plugin-raw.jsm", {});
 
 this.EXPORTED_SYMBOLS = ["DevToolsLoader", "devtools", "BuiltinProvider",
-                         "require", "loader"];
+                         "require", "loader",
+                         // Export StructuredCloneHolder for its use from builtin-modules
+                         "StructuredCloneHolder"];
 
 /**
  * Providers are different strategies for loading the devtools.
  */
 
 /**
  * Used when the tools should be loaded from the Firefox package itself.
  * This is the default case.
--- a/devtools/shared/builtin-modules.js
+++ b/devtools/shared/builtin-modules.js
@@ -10,22 +10,23 @@
  *
  * As it does so, the module itself doesn't have access to these globals,
  * nor the pseudo modules. Be careful to avoid loading any other js module as
  * they would also miss them.
  */
 
 const { Cu, CC, Cc, Ci } = require("chrome");
 const promise = require("resource://gre/modules/Promise.jsm").Promise;
-const jsmScope = require("resource://gre/modules/Services.jsm");
+const jsmScope = require("resource://devtools/shared/Loader.jsm");
 const { Services } = jsmScope;
 // Steal various globals only available in JSM scope (and not Sandbox one)
 const {
   console,
   HeapSnapshot,
+  StructuredCloneHolder,
 } = Cu.getGlobalForObject(jsmScope);
 
 // Create a single Sandbox to access global properties needed in this module.
 // Sandbox are memory expensive, so we should create as little as possible.
 const {
   atob,
   btoa,
   ChromeUtils,
@@ -270,16 +271,17 @@ exports.globals = {
     lazyImporter: defineLazyModuleGetter,
     lazyServiceGetter: defineLazyServiceGetter,
     lazyRequireGetter: lazyRequireGetter,
     // Defined by Loader.jsm
     id: null
   },
   Node: Ci.nsIDOMNode,
   reportError: Cu.reportError,
+  StructuredCloneHolder,
   TextDecoder,
   TextEncoder,
   URL,
   XMLHttpRequest,
 };
 // DevTools loader copy globals property descriptors on each module global
 // object so that we have to memoize them from here in order to instantiate each
 // global only once.
--- a/devtools/shared/transport/transport.js
+++ b/devtools/shared/transport/transport.js
@@ -755,18 +755,53 @@
       this.hooks.onClosed();
     },
 
     receiveMessage: function({data}) {
       this.emit("packet", data);
       this.hooks.onPacket(data);
     },
 
+    /**
+     * Helper method to ensure a given `object` can be sent across message manager
+     * without being serialized to JSON.
+     * See https://searchfox.org/mozilla-central/rev/6bfadf95b4a6aaa8bb3b2a166d6c3545983e179a/dom/base/nsFrameMessageManager.cpp#458-469
+     */
+    _canBeSerialized: function(object) {
+      try {
+        let holder = new StructuredCloneHolder(object);
+        holder.deserialize(this);
+      } catch (e) {
+        return false;
+      }
+      return true;
+    },
+
+    pathToUnserializable: function(object) {
+      for (let key in object) {
+        let value = object[key];
+        if (!this._canBeSerialized(value)) {
+          if (typeof value == "object") {
+            return [key].concat(this.pathToUnserializable(value));
+          }
+          return [key];
+        }
+      }
+      return [];
+    },
+
     send: function(packet) {
       this.emit("send", packet);
+      if (flags.testing && !this._canBeSerialized(packet)) {
+        let attributes = this.pathToUnserializable(packet);
+        let msg = "Following packet can't be serialized: " + JSON.stringify(packet);
+        msg += "\nBecause of attributes: " + attributes.join(", ") + "\n";
+        msg += "Did you pass a function or an XPCOM object in it?";
+        throw new Error(msg);
+      }
       try {
         this._mm.sendAsyncMessage(this._messageName, packet);
       } catch (e) {
         if (e.result != Cr.NS_ERROR_NULL_POINTER) {
           throw e;
         }
         // In some cases, especially when using messageManagers in non-e10s mode, we reach
         // this point with a dead messageManager which only throws errors but does not