Bug 1532237 - Use a Debugger loaded in a distinct compartment when debugging chrome in the event collector codebase. r=miker
authorAlexandre Poirot <poirot.alex@gmail.com>
Tue, 05 Mar 2019 14:58:05 +0000
changeset 520359 c7cfa9240f33da062f3e072b2cbc23c6377ec387
parent 520358 6013e1f3b6486837ce4ec98678afb6a39f37b10c
child 520360 6890499eb5d12fe8bce5ce1289ecb052ceb89343
push id10862
push userffxbld-merge
push dateMon, 11 Mar 2019 13:01:11 +0000
treeherdermozilla-beta@a2e7f5c935da [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiker
bugs1532237
milestone67.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 1532237 - Use a Debugger loaded in a distinct compartment when debugging chrome in the event collector codebase. r=miker Now that the server is by default loaded in the shared system compartment, we have to special cases the code debugging system compartments. When devtools.chrome.enabled is turned on, the event collector is inspecting the system compartment's event listener. In this case, we are using a special Debugger instance loaded in a sandbox flagged with invisibleToDebugger=true. This forces loading it in a distinct compartment and helps the debugger know about the boundaries between debugger and debuggee code. It should be safe to only load the Debugger and not the whole server here as event collector is only inspecting and doesn't register any callback on the Debugger API. Differential Revision: https://phabricator.services.mozilla.com/D22070
devtools/server/actors/inspector/event-collector.js
devtools/shared/builtin-modules.js
--- a/devtools/server/actors/inspector/event-collector.js
+++ b/devtools/server/actors/inspector/event-collector.js
@@ -9,16 +9,17 @@
 
 const { Cu } = require("chrome");
 const Services = require("Services");
 const {
   isAfterPseudoElement,
   isBeforePseudoElement,
   isNativeAnonymous,
 } = require("devtools/shared/layout/utils");
+const Debugger = require("Debugger");
 
 // eslint-disable-next-line
 const JQUERY_LIVE_REGEX = /return typeof \w+.*.event\.triggered[\s\S]*\.event\.(dispatch|handle).*arguments/;
 
 const REACT_EVENT_NAMES = [
   "onAbort",
   "onAnimationEnd",
   "onAnimationIteration",
@@ -739,16 +740,28 @@ class EventCollector {
         return true;
       }
     }
 
     return false;
   }
 
   /**
+   * We allow displaying chrome events if the page is chrome or if
+   * `devtools.chrome.enabled = true`.
+   */
+  get chromeEnabled() {
+    if (typeof this._chromeEnabled === "undefined") {
+      this._chromeEnabled = Services.prefs.getBoolPref("devtools.chrome.enabled");
+    }
+
+    return this._chromeEnabled;
+  }
+
+  /**
    *
    * @param  {DOMNode} node
    *         The node for which events are to be gathered.
    * @return {Array}
    *         An array containing objects in the following format:
    *         {
    *           type: type,        // e.g. "click"
    *           handler: handler,  // The function called when event is triggered.
@@ -757,17 +770,28 @@ class EventCollector {
    *           hide: {            // Flags for hiding certain properties.
    *             capturing: true,
    *             dom0: true,
    *           }
    *         }
    */
   getEventListeners(node) {
     const listenerArray = [];
-    const dbg = new Debugger();
+    let dbg;
+    if (!this.chromeEnabled) {
+      dbg = new Debugger();
+    } else {
+      // When the chrome pref is turned on, we may try to debug system compartments.
+      // But since bug 1517210, the server is also loaded using the system principal
+      // and so here, we have to ensure using a special Debugger instance, loaded
+      // in a compartment flagged with invisibleToDebugger=true. This helps the Debugger
+      // know about the precise boundary between debuggee and debugger code.
+      const ChromeDebugger = require("ChromeDebugger");
+      dbg = new ChromeDebugger();
+    }
 
     for (const collector of this.eventCollectors) {
       const listeners = collector.getListeners(node);
 
       if (!listeners) {
         continue;
       }
 
--- a/devtools/shared/builtin-modules.js
+++ b/devtools/shared/builtin-modules.js
@@ -8,55 +8,46 @@
  * This module defines custom globals injected in all our modules and also
  * pseudo modules that aren't separate files but just dynamically set values.
  *
  * 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 { Cu, Cc, Ci } = require("chrome");
 const promise = require("resource://gre/modules/Promise.jsm").Promise;
 const jsmScope = require("resource://devtools/shared/Loader.jsm");
 const { Services } = require("resource://gre/modules/Services.jsm");
+
+const systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
+
 // Steal various globals only available in JSM scope (and not Sandbox one)
 const {
   console,
   DOMPoint,
   DOMQuad,
   DOMRect,
   HeapSnapshot,
   NamedNodeMap,
   NodeFilter,
   StructuredCloneHolder,
   TelemetryStopwatch,
 } = 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,
-  Blob,
-  ChromeUtils,
-  CSS,
-  CSSRule,
-  DOMParser,
-  Element,
-  Event,
-  FileReader,
-  FormData,
-  indexedDB,
-  InspectorUtils,
-  Node,
-  TextDecoder,
-  TextEncoder,
-  URL,
-  XMLHttpRequest,
-} = Cu.Sandbox(CC("@mozilla.org/systemprincipal;1", "nsIPrincipal")(), {
+const debuggerSandbox = Cu.Sandbox(systemPrincipal, {
+  // This sandbox is also reused for ChromeDebugger implementation.
+  // As we want to load the `Debugger` API for debugging chrome contexts,
+  // we have to ensure loading it in a distinct compartment from its debuggee.
+  // invisibleToDebugger does that and helps the Debugger API identify the boundaries
+  // between debuggee and debugger code.
+  invisibleToDebugger: true,
+
   wantGlobalProperties: [
     "atob",
     "btoa",
     "Blob",
     "ChromeUtils",
     "CSS",
     "CSSRule",
     "DOMParser",
@@ -69,16 +60,37 @@ const {
     "Node",
     "TextDecoder",
     "TextEncoder",
     "URL",
     "XMLHttpRequest",
   ],
 });
 
+const {
+  atob,
+  btoa,
+  Blob,
+  ChromeUtils,
+  CSS,
+  CSSRule,
+  DOMParser,
+  Element,
+  Event,
+  FileReader,
+  FormData,
+  indexedDB,
+  InspectorUtils,
+  Node,
+  TextDecoder,
+  TextEncoder,
+  URL,
+  XMLHttpRequest,
+} = debuggerSandbox;
+
 /**
  * Defines a getter on a specified object that will be created upon first use.
  *
  * @param object
  *        The object to define the lazy getter on.
  * @param name
  *        The name of the getter to define on object.
  * @param lambda
@@ -234,16 +246,22 @@ defineLazyGetter(exports.modules, "Debug
   if (global.Debugger) {
     return global.Debugger;
   }
   const { addDebuggerToGlobal } = ChromeUtils.import("resource://gre/modules/jsdebugger.jsm");
   addDebuggerToGlobal(global);
   return global.Debugger;
 });
 
+defineLazyGetter(exports.modules, "ChromeDebugger", () => {
+  const { addDebuggerToGlobal } = ChromeUtils.import("resource://gre/modules/jsdebugger.jsm");
+  addDebuggerToGlobal(debuggerSandbox);
+  return debuggerSandbox.Debugger;
+});
+
 defineLazyGetter(exports.modules, "RecordReplayControl", () => {
   // addDebuggerToGlobal also adds the RecordReplayControl object.
   const global = Cu.getGlobalForObject(this);
   // RecordReplayControl may already have been added by Debugger getter
   if (global.RecordReplayControl) {
     return global.RecordReplayControl;
   }
   const { addDebuggerToGlobal } = ChromeUtils.import("resource://gre/modules/jsdebugger.jsm");