From fc5fb970e7d5f65de74955da91d521a6b07be630 Mon Sep 17 00:00:00 2001 From: Alexandre Poirot Date: Wed, 30 Jan 2019 08:04:50 +0000 Subject: [PATCH] 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 | 2 + .../test/browser_target_server_compartment.js | 126 ++++++++++++++++++ .../framework/test/test_chrome_page.html | 9 ++ .../webide/test/test_autoconnect_runtime.html | 10 +- .../webide/test/test_autoselect_project.html | 10 +- devtools/client/webide/test/test_runtime.html | 10 +- devtools/server/startup/frame.js | 26 +++- ...er_dbg_promises-chrome-allocation-stack.js | 10 ++ 8 files changed, 186 insertions(+), 17 deletions(-) create mode 100644 devtools/client/framework/test/browser_target_server_compartment.js create mode 100644 devtools/client/framework/test/test_chrome_page.html diff --git a/devtools/client/framework/test/browser.ini b/devtools/client/framework/test/browser.ini index 5a07d361c133..8928fbdaf9ef 100644 --- a/devtools/client/framework/test/browser.ini +++ b/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] diff --git a/devtools/client/framework/test/browser_target_server_compartment.js b/devtools/client/framework/test/browser_target_server_compartment.js new file mode 100644 index 000000000000..620b6df317a0 --- /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(); +} diff --git a/devtools/client/framework/test/test_chrome_page.html b/devtools/client/framework/test/test_chrome_page.html new file mode 100644 index 000000000000..688b9de1d684 --- /dev/null +++ b/devtools/client/framework/test/test_chrome_page.html @@ -0,0 +1,9 @@ + + +Chrome page + diff --git a/devtools/client/webide/test/test_autoconnect_runtime.html b/devtools/client/webide/test/test_autoconnect_runtime.html index f708d3804f5b..72680e3a7c9a 100644 --- a/devtools/client/webide/test/test_autoconnect_runtime.html +++ b/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(); diff --git a/devtools/client/webide/test/test_autoselect_project.html b/devtools/client/webide/test/test_autoselect_project.html index 4bdfa38bcc5f..858b4b68dfb8 100644 --- a/devtools/client/webide/test/test_autoselect_project.html +++ b/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"); diff --git a/devtools/client/webide/test/test_runtime.html b/devtools/client/webide/test/test_runtime.html index de5166914fd2..1b4bbedc5a70 100644 --- a/devtools/client/webide/test/test_runtime.html +++ b/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"); diff --git a/devtools/server/startup/frame.js b/devtools/server/startup/frame.js index 20018bcaf763..8241b5d79a9e 100644 --- a/devtools/server/startup/frame.js +++ b/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); })(); diff --git a/devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js b/devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js index 91fa049e0975..a38945528234 100644 --- a/devtools/server/tests/browser/browser_dbg_promises-chrome-allocation-stack.js +++ b/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;