Bug 1515290 - Instantiate DebuggerServer in dedicated loader when debugging chrome tabs. r=yulia,jdescottes
authorAlexandre Poirot <poirot.alex@gmail.com>
Wed, 30 Jan 2019 08:04:50 +0000
changeset 516916 820e7e98a4f7beee52772f1ef6b3de38b920927b
parent 516915 4ca7df7a6ab5a8d1471a9c0517ea0cc71d6e161f
child 516917 35df2ad0974a3225053cea204a1a57764a5da7fa
push id2032
push userffxbld-merge
push dateMon, 13 May 2019 09:36:57 +0000
treeherdermozilla-release@455c1065dcbe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyulia, jdescottes
bugs1515290
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 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
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_target_server_compartment.js
devtools/client/framework/test/test_chrome_page.html
devtools/client/webide/test/test_autoconnect_runtime.html
devtools/client/webide/test/test_autoselect_project.html
devtools/client/webide/test/test_runtime.html
devtools/server/startup/frame.js
devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -40,16 +40,17 @@ support-files =
   doc_theme.css
   doc_viewsource.html
   browser_toolbox_options_enable_serviceworkers_testing_frame_script.js
   browser_toolbox_options_enable_serviceworkers_testing.html
   serviceworker.js
   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
   !/devtools/client/shared/test/frame-script-utils.js
   !/devtools/client/shared/test/shared-head.js
   !/devtools/client/shared/test/shared-redux-head.js
   !/devtools/client/shared/test/telemetry-test-helpers.js
 
@@ -77,16 +78,17 @@ skip-if = os == 'win' || debug # Bug 128
 [browser_source_map-late-script.js]
 [browser_target_from_url.js]
 [browser_target_cached-front.js]
 [browser_target_events.js]
 [browser_target_remote.js]
 [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]
 [browser_toolbox_highlight.js]
 [browser_toolbox_hosts.js]
 [browser_toolbox_hosts_size.js]
 [browser_toolbox_hosts_telemetry.js]
 [browser_toolbox_keyboard_navigation.js]
new file mode 100644
--- /dev/null
+++ b/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();
+}
new file mode 100644
--- /dev/null
+++ b/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>
--- a/devtools/client/webide/test/test_autoconnect_runtime.html
+++ b/devtools/client/webide/test/test_autoconnect_runtime.html
@@ -44,49 +44,49 @@
           };
           win.AppManager.runtimeList.usb.push(fakeRuntime);
           win.AppManager.update("runtime-list");
 
           const panelNode = docRuntime.querySelector("#runtime-panel");
           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"),
             "UI is busy");
           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);
 
           await connectionsChanged;
           is(Object.keys(DebuggerServer._connections).length, 0, "Disconnected");
 
           // The DebuggerServer is destroyed when closing WebIDE, so re-initialize it.
           DebuggerServer.init();
           DebuggerServer.registerAllActors();
 
-          connectionsChanged = waitForConnectionChange("opened", 2);
+          connectionsChanged = waitForConnectionChange("opened");
 
           win = await openWebIDE();
 
           win.AppManager.runtimeList.usb.push(fakeRuntime);
           win.AppManager.update("runtime-list");
 
           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();
 
           await closeWebIDE(win);
 
           DebuggerServer.destroy();
 
           SimpleTest.finish();
--- a/devtools/client/webide/test/test_autoselect_project.html
+++ b/devtools/client/webide/test/test_autoselect_project.html
@@ -26,68 +26,68 @@
           let docRuntime = getRuntimeDocument(win);
           const docProject = getProjectDocument(win);
 
           let panelNode = docRuntime.querySelector("#runtime-panel");
           let items = panelNode.querySelectorAll(".runtime-panel-item-other");
           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");
 
           // Select main process
           await win.Cmds.showProjectPanel();
           await waitForUpdate(win, "runtime-targets");
           SimpleTest.executeSoon(() => {
             docProject.querySelectorAll("#project-panel-runtimeapps .panel-item")[0].click();
           });
 
           await waitForUpdate(win, "project");
 
           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);
 
           await connectionsChanged;
           is(Object.keys(DebuggerServer._connections).length, 0, "Disconnected");
 
           // The DebuggerServer is destroyed when closing WebIDE, so re-initialize it.
           DebuggerServer.init();
           DebuggerServer.registerAllActors();
 
-          connectionsChanged = waitForConnectionChange("opened", 2);
+          connectionsChanged = waitForConnectionChange("opened");
 
           // Re-open, should reselect main process after connection
           win = await openWebIDE();
 
           docRuntime = getRuntimeDocument(win);
 
           panelNode = docRuntime.querySelector("#runtime-panel");
           items = panelNode.querySelectorAll(".runtime-panel-item-other");
           is(items.length, 2, "Found 2 runtime buttons");
 
           // Connect to local runtime
           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");
           is(win.AppManager.selectedProject.type, "mainProcess", "Main process reselected");
 
           // Wait for the toolbox to be fully loaded
           await win.UI.toolboxPromise;
 
           // If we happen to pass a project object targeting the same context,
           // here, the main process, the `selectedProject` attribute shouldn't be updated
--- a/devtools/client/webide/test/test_runtime.html
+++ b/devtools/client/webide/test/test_runtime.html
@@ -93,25 +93,25 @@
           win.AppManager.update("runtime-list");
 
           const panelNode = docRuntime.querySelector("#runtime-panel");
           const items = panelNode.querySelectorAll(".runtime-panel-item-usb");
           is(items.length, 3, "Found 3 runtime buttons");
 
           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"),
             "UI is busy");
           await win.UI._busyPromise;
 
           await connectionsChanged;
-          is(Object.keys(DebuggerServer._connections).length, 2, "Connected");
+          is(Object.keys(DebuggerServer._connections).length, 1, "Connected");
 
           await onGlobalActors;
 
           // Play button always disabled now, webapps actor removed
           ok(!isPlayActive(), "play button is disabled");
           ok(!isStopActive(), "stop button is disabled");
           const oldProject = win.AppManager.selectedProject;
           win.AppManager.selectedProject = null;
@@ -123,35 +123,35 @@
           win.AppManager._selectedProject = oldProject;
           win.UI.updateCommands();
 
           await nextTick();
 
           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;
           is(Object.keys(DebuggerServer._connections).length, 0, "Disconnected");
 
           ok(win.AppManager.selectedProject, "A project is still selected");
           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();
           await connectionsChanged;
           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");
 
           // Select main process
           SimpleTest.executeSoon(() => {
             docProject.querySelectorAll("#project-panel-runtimeapps .panel-item")[0].click();
           });
 
--- a/devtools/server/startup/frame.js
+++ b/devtools/server/startup/frame.js
@@ -1,28 +1,45 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "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
  * content process.  Loaded into content process frames by the main process during
  * `DebuggerServer.connectToFrame`.
  */
 
 try {
   var chromeGlobal = this;
 
   // 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;
     const { DebuggerServer } = require("devtools/server/main");
     const { ActorPool } = require("devtools/server/actors/common");
 
     DebuggerServer.init();
     // We want a special server without any root actor and only target-scoped actors.
@@ -128,14 +145,19 @@ try {
     function destroyServer() {
       // Only destroy the server if there is no more connections to it. It may be used
       // to debug another tab running in the same process.
       if (DebuggerServer.hasConnection()) {
         return;
       }
       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);
   })();
 } catch (e) {
   dump(`Exception in DevTools frame startup: ${e}\n`);
 }
--- a/devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js
+++ b/devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js
@@ -17,16 +17,26 @@ const ObjectClient = require("devtools/s
 const STACK_DATA = [
   { functionDisplayName: "test/<" },
   { functionDisplayName: "testGetAllocationStack" },
 ];
 
 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;
 
   const client = new DebuggerClient(DebuggerServer.connectPipe());
   await client.connect();
   const targetFront = await client.mainRoot.getMainProcess();
   const target = await TargetFactory.forRemoteTab({