Bug 1515290 - Instantiate DebuggerServer in dedicated loader when debugging chrome tabs. r=yulia,jdescottes

When debugging contexts running from the system compartment, the debugger has
to be loaded in a dedicated Loader, with invisibleToDebugger flag turned on.
This ensures that the Debugger API is going to be used from a distinct system
compartment. Otherwise it may be used from the same compartment than the page
we are debugging.

Differential Revision: https://phabricator.services.mozilla.com/D14957

--HG--
extra : moz-landing-system : lando

devtools/client/framework/test/browser.ini

@@ -45,6 +45,7 @@ support-files =
   sjs_code_reload.sjs
   sjs_code_bundle_reload_map.sjs
   test_browser_toolbox_debugger.js
+  test_chrome_page.html
   !/devtools/client/debugger/new/test/mochitest/head.js
   !/devtools/client/debugger/new/test/mochitest/helpers.js
   !/devtools/client/debugger/new/test/mochitest/helpers/context.js
@@ -82,6 +83,7 @@ skip-if = os == 'win' || debug # Bug 1282269, 1448084
 [browser_target_support.js]
 [browser_target_get-front.js]
 [browser_target_listeners.js]
+[browser_target_server_compartment.js]
 [browser_toolbox_custom_host.js]
 [browser_toolbox_dynamic_registration.js]
 [browser_toolbox_getpanelwhenready.js]

devtools/client/framework/test/browser_target_server_compartment.js

@@ -0,0 +1,126 @@
+/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
+/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// Bug 1515290 - Ensure that DebuggerServer runs in its own compartment when debugging
+// chrome context. If not, Debugger API's addGlobal will throw when trying to attach
+// to chrome scripts as debugger actor's module and the chrome script will be in the same
+// compartment. Debugger and debuggee can't be running in the same compartment.
+
+const CHROME_PAGE = "chrome://mochitests/content/browser/devtools/client/framework/" +
+  "test/test_chrome_page.html";
+
+add_task(async function() {
+  await testChromeTab();
+  await testMainProcess();
+});
+
+// Test that Tab Target can debug chrome pages
+async function testChromeTab() {
+  const tab = await addTab(CHROME_PAGE);
+  const browser = tab.linkedBrowser;
+  ok(!browser.isRemoteBrowser, "chrome page is not remote");
+  ok(browser.contentWindow.document.nodePrincipal.isSystemPrincipal,
+    "chrome page is a privileged document");
+
+  const onThreadActorInstantiated = new Promise(resolve => {
+    const observe = function(subject, topic, data) {
+      if (topic === "devtools-thread-instantiated") {
+        Services.obs.removeObserver(observe, "devtools-thread-instantiated");
+        const threadActor = subject.wrappedJSObject;
+        resolve(threadActor);
+      }
+    };
+    Services.obs.addObserver(observe, "devtools-thread-instantiated");
+  });
+
+  const target = await TargetFactory.forTab(tab);
+  await target.attach();
+
+  const [, threadClient] = await target.activeTab.attachThread();
+  await threadClient.resume();
+
+  const { sources } = await threadClient.getSources();
+  ok(sources.find(s => s.url == CHROME_PAGE),
+    "The thread actor is able to attach to the chrome page and its sources");
+
+  const threadActor = await onThreadActorInstantiated;
+  const serverGlobal = Cu.getGlobalForObject(threadActor);
+  isnot(loader.id, serverGlobal.loader.id,
+    "The actors are loaded in a distinct loader in order for the actors to use its very own compartment");
+
+  const onDedicatedLoaderDestroy = new Promise(resolve => {
+    const observe = function(subject, topic, data) {
+      if (topic === "devtools:loader:destroy") {
+        Services.obs.removeObserver(observe, "devtools:loader:destroy");
+        resolve();
+      }
+    };
+    Services.obs.addObserver(observe, "devtools:loader:destroy");
+  });
+
+  await target.destroy();
+
+  // Wait for the dedicated loader used for DebuggerServer to be destroyed
+  // in order to prevent leak reports on try
+  await onDedicatedLoaderDestroy;
+}
+
+// Test that Main process Target can debug chrome scripts
+async function testMainProcess() {
+  const { DevToolsLoader } =
+    ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
+  const customLoader = new DevToolsLoader();
+  customLoader.invisibleToDebugger = true;
+  const { DebuggerServer } = customLoader.require("devtools/server/main");
+  const { DebuggerClient } = require("devtools/shared/client/debugger-client");
+
+  DebuggerServer.init();
+  DebuggerServer.registerAllActors();
+  DebuggerServer.allowChromeProcess = true;
+
+  const client = new DebuggerClient(DebuggerServer.connectPipe());
+  await client.connect();
+
+  const onThreadActorInstantiated = new Promise(resolve => {
+    const observe = function(subject, topic, data) {
+      if (topic === "devtools-thread-instantiated") {
+        Services.obs.removeObserver(observe, "devtools-thread-instantiated");
+        const threadActor = subject.wrappedJSObject;
+        resolve(threadActor);
+      }
+    };
+    Services.obs.addObserver(observe, "devtools-thread-instantiated");
+  });
+
+  const targetFront = await client.mainRoot.getMainProcess();
+  const target = await TargetFactory.forRemoteTab({
+    client,
+    activeTab: targetFront,
+    chrome: true,
+  });
+  await target.attach();
+
+  const [, threadClient] = await target.activeTab.attachThread();
+  await threadClient.resume();
+  const { sources } = await threadClient.getSources();
+  ok(sources.find(s => s.url == "resource://devtools/client/framework/devtools.js"),
+    "The thread actor is able to attach to the chrome script, like client modules");
+
+  const threadActor = await onThreadActorInstantiated;
+  const serverGlobal = Cu.getGlobalForObject(threadActor);
+  isnot(loader.id, serverGlobal.loader.id,
+    "The actors are loaded in a distinct loader in order for the actors to use its very own compartment");
+
+  await target.destroy();
+
+  // As this target is remote (i.e. isn't a local tab) calling Target.destroy won't close
+  // the client. So do it manually here in order to ensure cleaning up the DebuggerServer
+  // spawn for this main process actor.
+  await client.close();
+
+  // As we create the loader and server manually, we also destroy them manually here:
+  await DebuggerServer.destroy();
+  await customLoader.destroy();
+}

devtools/client/framework/test/test_chrome_page.html

@@ -0,0 +1,9 @@
+<!doctype html>
+<meta charset=utf-8>
+<title>Chrome page</title>
+<script>
+// eslint-disable-next-line no-unused-vars
+function inlineScript() {
+  console.log("foo");
+}
+</script>

devtools/client/webide/test/test_autoconnect_runtime.html

@@ -49,7 +49,7 @@
           const items = panelNode.querySelectorAll(".runtime-panel-item-usb");
           is(items.length, 1, "Found one runtime button");
 
-          let connectionsChanged = waitForConnectionChange("opened", 2);
+          let connectionsChanged = waitForConnectionChange("opened");
           items[0].click();
 
           ok(win.document.querySelector("window").classList.contains("busy"),
@@ -57,9 +57,9 @@
           await win.UI._busyPromise;
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Connected");
 
-          connectionsChanged = waitForConnectionChange("closed", 2);
+          connectionsChanged = waitForConnectionChange("closed");
 
           await nextTick();
           await closeWebIDE(win);
@@ -71,7 +71,7 @@
           DebuggerServer.init();
           DebuggerServer.registerAllActors();
 
-          connectionsChanged = waitForConnectionChange("opened", 2);
+          connectionsChanged = waitForConnectionChange("opened");
 
           win = await openWebIDE();
 
@@ -81,7 +81,7 @@
           await waitForUpdate(win, "runtime-targets");
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Automatically reconnected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Automatically reconnected");
 
           await win.Cmds.disconnectRuntime();
 

devtools/client/webide/test/test_autoselect_project.html

@@ -31,13 +31,13 @@
           is(items.length, 2, "Found 2 runtime buttons");
 
           // Connect to local runtime
-          let connectionsChanged = waitForConnectionChange("opened", 2);
+          let connectionsChanged = waitForConnectionChange("opened");
           items[1].click();
 
           await waitForUpdate(win, "runtime-targets");
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Locally connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Locally connected");
 
           ok(win.AppManager.isMainProcessDebuggable(), "Main process available");
 
@@ -53,7 +53,7 @@
           const lastProject = Services.prefs.getCharPref("devtools.webide.lastSelectedProject");
           is(lastProject, "mainProcess:", "Last project is main process");
 
-          connectionsChanged = waitForConnectionChange("closed", 2);
+          connectionsChanged = waitForConnectionChange("closed");
 
           await nextTick();
           await closeWebIDE(win);
@@ -65,7 +65,7 @@
           DebuggerServer.init();
           DebuggerServer.registerAllActors();
 
-          connectionsChanged = waitForConnectionChange("opened", 2);
+          connectionsChanged = waitForConnectionChange("opened");
 
           // Re-open, should reselect main process after connection
           win = await openWebIDE();
@@ -82,7 +82,7 @@
           await waitForUpdate(win, "runtime-targets");
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Locally connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Locally connected");
           ok(win.AppManager.isMainProcessDebuggable(), "Main process available");
           is(win.AppManager.selectedProject.type, "mainProcess", "Main process reselected");
 

devtools/client/webide/test/test_runtime.html

@@ -98,7 +98,7 @@
 
           let onGlobalActors = waitForUpdate(win, "runtime-global-actors");
 
-          let connectionsChanged = waitForConnectionChange("opened", 2);
+          let connectionsChanged = waitForConnectionChange("opened");
           items[0].click();
 
           ok(win.document.querySelector("window").classList.contains("busy"),
@@ -106,7 +106,7 @@
           await win.UI._busyPromise;
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Connected");
 
           await onGlobalActors;
 
@@ -128,7 +128,7 @@
           ok(!isPlayActive(), "play button is enabled");
           ok(!isStopActive(), "stop button is disabled");
 
-          connectionsChanged = waitForConnectionChange("closed", 2);
+          connectionsChanged = waitForConnectionChange("closed");
           await win.Cmds.disconnectRuntime();
 
           await connectionsChanged;
@@ -138,7 +138,7 @@
           ok(!isPlayActive(), "play button is disabled");
           ok(!isStopActive(), "stop button is disabled");
 
-          connectionsChanged = waitForConnectionChange("opened", 2);
+          connectionsChanged = waitForConnectionChange("opened");
           onGlobalActors = waitForUpdate(win, "runtime-global-actors");
           const onRuntimeTargets = waitForUpdate(win, "runtime-targets");
           docRuntime.querySelectorAll(".runtime-panel-item-other")[1].click();
@@ -146,7 +146,7 @@
           await onGlobalActors;
           await onRuntimeTargets;
 
-          is(Object.keys(DebuggerServer._connections).length, 2, "Locally connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Locally connected");
 
           ok(win.AppManager.isMainProcessDebuggable(), "Main process available");
 

devtools/server/startup/frame.js

@@ -4,7 +4,8 @@
 
 "use strict";
 
-/* global addEventListener, addMessageListener, removeMessageListener, sendAsyncMessage */
+/* global content, addEventListener, addMessageListener, removeMessageListener,
+  sendAsyncMessage */
 
 /*
  * Frame script that listens for requests to start a `DebuggerServer` for a frame in a
@@ -17,7 +18,23 @@ try {
 
   // Encapsulate in its own scope to allows loading this frame script more than once.
   (function() {
-    const { require } = ChromeUtils.import("resource://devtools/shared/Loader.jsm");
+    // In most cases, we are debugging a tab in content process, without chrome
+    // privileges. But in some tests, we are attaching to privileged document.
+    // Because the debugger can't be running in the same compartment than its debuggee,
+    // we have to load the server in a dedicated Loader, flagged with
+    // invisibleToDebugger, which will force it to be loaded in another compartment.
+    let loader, customLoader = false;
+    if (content.document.nodePrincipal.isSystemPrincipal) {
+      const { DevToolsLoader } =
+        ChromeUtils.import("resource://devtools/shared/Loader.jsm");
+      loader = new DevToolsLoader();
+      loader.invisibleToDebugger = true;
+      customLoader = true;
+    } else {
+      // Otherwise, use the shared loader.
+      loader = ChromeUtils.import("resource://devtools/shared/Loader.jsm");
+    }
+    const { require } = loader;
 
     const DevToolsUtils = require("devtools/shared/DevToolsUtils");
     const { dumpn } = DevToolsUtils;
@@ -133,6 +150,11 @@ try {
       }
       DebuggerServer.off("connectionchange", destroyServer);
       DebuggerServer.destroy();
+
+      // When debugging chrome pages, we initialized a dedicated loader, also destroy it
+      if (customLoader) {
+        loader.destroy();
+      }
     }
     DebuggerServer.on("connectionchange", destroyServer);
   })();

devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js

@@ -22,6 +22,16 @@ const STACK_DATA = [
 add_task(async function test() {
   requestLongerTimeout(10);
 
+  // Start the DebuggerServer in a distinct loader as we want to debug system
+  // compartments. The actors have to be in a distinct compartment than the
+  // context they are debugging. `invisibleToDebugger` force loading modules in
+  // a distinct compartments.
+  const { DevToolsLoader } =
+    ChromeUtils.import("resource://devtools/shared/Loader.jsm", {});
+  const customLoader = new DevToolsLoader();
+  customLoader.invisibleToDebugger = true;
+  const { DebuggerServer } = customLoader.require("devtools/server/main");
+
   DebuggerServer.init();
   DebuggerServer.registerAllActors();
   DebuggerServer.allowChromeProcess = true;