Bug 1453385 - Throw error when RDP message can't be serialized in message manager. r=jryans
authorAlexandre Poirot <poirot.alex@gmail.com>
Thu, 12 Apr 2018 04:44:12 -0700
changeset 413262 c611078b5add8c91a6df5f8a7dff2bedad4a11e2
parent 413261 565f6eac204da7596467c87ec7e25f4858735046
child 413263 49862a0820f94e96d07d3e20174b01f24871bf6e
push id62660
push userapoirot@mozilla.com
push dateFri, 13 Apr 2018 21:26:39 +0000
treeherderautoland@a7561379843f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjryans
bugs1453385
milestone61.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 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