From 3dd24d960d5d0e12104f478fec2c26e448c8be16 Mon Sep 17 00:00:00 2001 From: Nick Fitzgerald Date: Thu, 24 Sep 2015 17:19:03 -0700 Subject: [PATCH] Bug 1207645 - Create HeapSnapshotFileActor; r=jryans This commit creates the HeapSnapshotFileActor and moves the transferHeapSnapshot method from MemoryActor to HeapSnapshotFileActor. This is necessary because child processes in e10s are sandboxed and do not have access to the file system, and because MemoryActor is in the sandboxed child process it cannot open the file and send it over the RDP. This complexity is hidden at the MemoryFront layer. Users of MemoryFront will still simply call its saveHeapSnapshot method, and do not need to worry about the inner workings of how heap snapshot files are transferred over the RDP. This required adding a third parameter to MemoryFront's initialize method: the listTabs response. --- devtools/client/framework/target.js | 18 +++++ devtools/client/framework/test/shared-head.js | 38 ++++++++-- devtools/client/memory/controller.js | 4 +- devtools/client/memory/moz.build | 1 + devtools/client/memory/panel.js | 13 +++- .../client/memory/test/browser/browser.ini | 7 ++ ...ser_memory_transferHeapSnapshot_e10s_01.js | 28 +++++++ devtools/client/memory/test/browser/head.js | 65 ++++++++++++++++ devtools/server/actors/heap-snapshot-file.js | 74 +++++++++++++++++++ devtools/server/actors/memory.js | 41 ++-------- devtools/server/actors/moz.build | 1 + devtools/server/main.js | 5 ++ .../server/tests/mochitest/memory-helpers.js | 2 +- devtools/server/tests/unit/head_dbg.js | 17 ++++- 14 files changed, 264 insertions(+), 50 deletions(-) create mode 100644 devtools/client/memory/test/browser/browser.ini create mode 100644 devtools/client/memory/test/browser/browser_memory_transferHeapSnapshot_e10s_01.js create mode 100644 devtools/client/memory/test/browser/head.js create mode 100644 devtools/server/actors/heap-snapshot-file.js diff --git a/devtools/client/framework/target.js b/devtools/client/framework/target.js index de8c574e5ad5..2ffd2f9e0397 100644 --- a/devtools/client/framework/target.js +++ b/devtools/client/framework/target.js @@ -317,10 +317,28 @@ TabTarget.prototype = { return this._form; }, + // Get a promise of the root form returned by a listTabs request. This promise + // is cached. get root() { + if (!this._root) { + this._root = this._getRoot(); + } return this._root; }, + _getRoot: function () { + return new Promise((resolve, reject) => { + this.client.listTabs(response => { + if (response.error) { + reject(new Error(response.error + ": " + response.message)); + return; + } + + resolve(response); + }); + }); + }, + get client() { return this._client; }, diff --git a/devtools/client/framework/test/shared-head.js b/devtools/client/framework/test/shared-head.js index 02c20a0f934a..09fd9479d90e 100644 --- a/devtools/client/framework/test/shared-head.js +++ b/devtools/client/framework/test/shared-head.js @@ -3,12 +3,21 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ // This shared-head.js file is used for multiple directories in devtools. + const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; -const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); -const {gDevTools} = Cu.import("resource:///modules/devtools/client/framework/gDevTools.jsm", {}); -const {console} = Cu.import("resource://gre/modules/devtools/shared/Console.jsm", {}); -const {ScratchpadManager} = Cu.import("resource:///modules/devtools/client/scratchpad/scratchpad-manager.jsm", {}); -const {require} = Cu.import("resource://gre/modules/devtools/shared/Loader.jsm", {}); + +function scopedCuImport(path) { + const scope = {}; + Cu.import(path, scope); + return scope; +} + +const {Services} = scopedCuImport("resource://gre/modules/Services.jsm"); +const {gDevTools} = scopedCuImport("resource:///modules/devtools/client/framework/gDevTools.jsm"); +const {console} = scopedCuImport("resource://gre/modules/devtools/shared/Console.jsm"); +const {ScratchpadManager} = scopedCuImport("resource:///modules/devtools/client/scratchpad/scratchpad-manager.jsm"); +const {require} = scopedCuImport("resource://gre/modules/devtools/shared/Loader.jsm"); + const {TargetFactory} = require("devtools/client/framework/target"); const DevToolsUtils = require("devtools/shared/DevToolsUtils"); const promise = require("promise"); @@ -69,6 +78,25 @@ function addTab(url) { return def.promise; } +/** + * Remove the given tab. + * @param {Object} tab The tab to be removed. + * @return Promise resolved when the tab is successfully removed. + */ +function removeTab(tab) { + info("Removing tab."); + return new Promise(resolve => { + let tabContainer = gBrowser.tabContainer; + tabContainer.addEventListener("TabClose", function onClose(aEvent) { + tabContainer.removeEventListener("TabClose", onClose, false); + info("Tab removed and finished closing."); + resolve(); + }, false); + + gBrowser.removeTab(tab); + }); +} + function synthesizeKeyFromKeyTag(aKeyId, document) { let key = document.getElementById(aKeyId); isnot(key, null, "Successfully retrieved the node"); diff --git a/devtools/client/memory/controller.js b/devtools/client/memory/controller.js index 4c98fd890339..8d672b945008 100644 --- a/devtools/client/memory/controller.js +++ b/devtools/client/memory/controller.js @@ -19,10 +19,10 @@ var gToolbox, gTarget, gFront; */ const MemoryController = { initialize: Task.async(function *() { - + yield gFront.attach(); }), destroy: Task.async(function *() { - + yield gFront.detach(); }) }; diff --git a/devtools/client/memory/moz.build b/devtools/client/memory/moz.build index 3b938fadc786..5b2981f72358 100644 --- a/devtools/client/memory/moz.build +++ b/devtools/client/memory/moz.build @@ -12,3 +12,4 @@ DevToolsModules( ) MOCHITEST_CHROME_MANIFESTS += ['test/mochitest/chrome.ini'] +BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini'] diff --git a/devtools/client/memory/panel.js b/devtools/client/memory/panel.js index 346c6e5425ea..9f4ed8cebff0 100644 --- a/devtools/client/memory/panel.js +++ b/devtools/client/memory/panel.js @@ -26,14 +26,19 @@ MemoryPanel.prototype = { this.panelWin.gToolbox = this._toolbox; this.panelWin.gTarget = this.target; - this.panelWin.gFront = new MemoryFront(this.target.client, this.target.form); + + const rootForm = yield this.target.root; + this.panelWin.gFront = new MemoryFront(this.target.client, + this.target.form, + rootForm); console.log(this.panelWin, this.panelWin.MemoryController); - return this._opening = this.panelWin.MemoryController.initialize().then(() => { + this._opening = this.panelWin.MemoryController.initialize().then(() => { this.isReady = true; this.emit("ready"); return this; }); + return this._opening; }), // DevToolPanel API @@ -48,12 +53,14 @@ MemoryPanel.prototype = { return this._destroyer; } - return this._destroyer = this.panelWin.MemoryController.destroy().then(() => { + this._destroyer = this.panelWin.MemoryController.destroy().then(() => { // Destroy front to ensure packet handler is removed from client this.panelWin.gFront.destroy(); + this.panelWin = null; this.emit("destroyed"); return this; }); + return this._destroyer; } }; diff --git a/devtools/client/memory/test/browser/browser.ini b/devtools/client/memory/test/browser/browser.ini new file mode 100644 index 000000000000..48a2e9e0c603 --- /dev/null +++ b/devtools/client/memory/test/browser/browser.ini @@ -0,0 +1,7 @@ +[DEFAULT] +tags = devtools +subsuite = devtools +support-files = + head.js + +[browser_memory_transferHeapSnapshot_e10s_01.js] diff --git a/devtools/client/memory/test/browser/browser_memory_transferHeapSnapshot_e10s_01.js b/devtools/client/memory/test/browser/browser_memory_transferHeapSnapshot_e10s_01.js new file mode 100644 index 000000000000..8ebb7622afc3 --- /dev/null +++ b/devtools/client/memory/test/browser/browser_memory_transferHeapSnapshot_e10s_01.js @@ -0,0 +1,28 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test that we can save a heap snapshot and transfer it over the RDP in e10s +// where the child process is sandboxed and so we have to use +// HeapSnapshotFileActor to get the heap snapshot file. + +"use strict"; + +const TEST_URL = "data:text/html,"; + +this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { + const memoryFront = panel.panelWin.gFront; + ok(memoryFront, "Should get the MemoryFront"); + + const snapshotFilePath = yield memoryFront.saveHeapSnapshot({ + // Force a copy so that we go through the HeapSnapshotFileActor's + // transferHeapSnapshot request and exercise this code path on e10s. + forceCopy: true + }); + + ok(!!(yield OS.File.stat(snapshotFilePath)), + "Should have the heap snapshot file"); + + const snapshot = ChromeUtils.readHeapSnapshot(snapshotFilePath); + ok(snapshot instanceof HeapSnapshot, + "And we should be able to read a HeapSnapshot instance from the file"); +}); diff --git a/devtools/client/memory/test/browser/head.js b/devtools/client/memory/test/browser/head.js new file mode 100644 index 000000000000..6b6d5267d1bd --- /dev/null +++ b/devtools/client/memory/test/browser/head.js @@ -0,0 +1,65 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Load the shared test helpers into this compartment. +Services.scriptloader.loadSubScript( + "chrome://mochitests/content/browser/devtools/client/framework/test/shared-head.js", + this); + +Services.prefs.setBoolPref("devtools.memory.enabled", true); + +/** + * Open the memory panel for the given tab. + */ +this.openMemoryPanel = Task.async(function* (tab) { + info("Opening memory panel."); + const target = TargetFactory.forTab(tab); + const toolbox = yield gDevTools.showToolbox(target, "memory"); + info("Memory panel shown successfully."); + let panel = toolbox.getCurrentPanel(); + return { tab, panel }; +}); + +/** + * Close the memory panel for the given tab. + */ +this.closeMemoryPanel = Task.async(function* (tab) { + info("Closing memory panel."); + const target = TargetFactory.forTab(tab); + const toolbox = gDevTools.getToolbox(target); + yield toolbox.destroy(); + info("Closed memory panel successfully."); +}); + +/** + * Return a test function that adds a tab with the given url, opens the memory + * panel, runs the given generator, closes the memory panel, removes the tab, + * and finishes. + * + * Example usage: + * + * this.test = makeMemoryTest(TEST_URL, function* ({ tab, panel }) { + * // Your tests go here... + * }); + */ +function makeMemoryTest(url, generator) { + return Task.async(function* () { + waitForExplicitFinish(); + + const tab = yield addTab(url); + const results = yield openMemoryPanel(tab); + + try { + yield* generator(results); + } catch (err) { + ok(false, "Got an error: " + DevToolsUtils.safeErrorString(err)); + } + + yield closeMemoryPanel(tab); + yield removeTab(tab); + + finish(); + }); +} diff --git a/devtools/server/actors/heap-snapshot-file.js b/devtools/server/actors/heap-snapshot-file.js new file mode 100644 index 000000000000..85f4bf38a0c9 --- /dev/null +++ b/devtools/server/actors/heap-snapshot-file.js @@ -0,0 +1,74 @@ +/* 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"; + +const protocol = require("devtools/server/protocol"); +const { method, Arg } = protocol; +const Services = require("Services"); + +loader.lazyRequireGetter(this, "DevToolsUtils", + "devtools/shared/DevToolsUtils"); +loader.lazyRequireGetter(this, "OS", "resource://gre/modules/osfile.jsm", true); +loader.lazyRequireGetter(this, "Task", "resource://gre/modules/Task.jsm", true); +loader.lazyRequireGetter(this, "HeapSnapshotFileUtils", + "devtools/shared/heapsnapshot/HeapSnapshotFileUtils"); + +/** + * The HeapSnapshotFileActor handles transferring heap snapshot files from the + * server to the client. This has to be a global actor in the parent process + * because child processes are sandboxed and do not have access to the file + * system. + */ +exports.HeapSnapshotFileActor = protocol.ActorClass({ + typeName: "heapSnapshotFile", + + initialize: function (conn, parent) { + if (Services.appInfo && + (Services.appInfo.processType !== + Services.appInfo.PROCESS_TYPE_DEFAULT)) { + const err = new Error("Attempt to create a HeapSnapshotFileActor in a " + + "child process! The HeapSnapshotFileActor *MUST* " + + "be in the parent process!"); + DevToolsUtils.reportException( + "HeapSnapshotFileActor.prototype.initialize", err); + return; + } + + protocol.Actor.prototype.initialize.call(this, conn, parent); + }, + + /** + * @see MemoryFront.prototype.transferHeapSnapshot + */ + transferHeapSnapshot: method(Task.async(function* (snapshotId) { + const snapshotFilePath = + HeapSnapshotFileUtils.getHeapSnapshotTempFilePath(snapshotId); + if (!snapshotFilePath) { + throw new Error(`No heap snapshot with id: ${snapshotId}`); + } + + const streamPromise = DevToolsUtils.openFileStream(snapshotFilePath); + + const { size } = yield OS.File.stat(snapshotFilePath); + const bulkPromise = this.conn.startBulkSend({ + actor: this.actorID, + type: "heap-snapshot", + length: size + }); + + const [bulk, stream] = yield Promise.all([bulkPromise, streamPromise]); + + try { + yield bulk.copyFrom(stream); + } finally { + stream.close(); + } + }), { + request: { + snapshotId: Arg(0, "string") + } + }), + +}); diff --git a/devtools/server/actors/memory.js b/devtools/server/actors/memory.js index d3be25184a05..ed3a957efbf9 100644 --- a/devtools/server/actors/memory.js +++ b/devtools/server/actors/memory.js @@ -5,7 +5,6 @@ "use strict"; const { Cc, Ci, Cu, components } = require("chrome"); -const { openFileStream } = require("devtools/shared/DevToolsUtils"); const protocol = require("devtools/server/protocol"); const { method, RetVal, Arg, types } = protocol; const { Memory } = require("devtools/shared/shared/memory"); @@ -17,7 +16,6 @@ loader.lazyRequireGetter(this, "FileUtils", "resource://gre/modules/FileUtils.jsm", true); loader.lazyRequireGetter(this, "NetUtil", "resource://gre/modules/NetUtil.jsm", true); loader.lazyRequireGetter(this, "Task", "resource://gre/modules/Task.jsm", true); -loader.lazyRequireGetter(this, "OS", "resource://gre/modules/osfile.jsm", true); loader.lazyRequireGetter(this, "HeapSnapshotFileUtils", "devtools/shared/heapsnapshot/HeapSnapshotFileUtils"); loader.lazyRequireGetter(this, "ThreadSafeChromeUtils"); @@ -115,35 +113,6 @@ var MemoryActor = exports.MemoryActor = protocol.ActorClass({ } }), - transferHeapSnapshot: method(Task.async(function* (snapshotId) { - const snapshotFilePath = - HeapSnapshotFileUtils.getHeapSnapshotTempFilePath(snapshotId); - if (!snapshotFilePath) { - throw new Error(`No heap snapshot with id: ${snapshotId}`); - } - - const streamPromise = openFileStream(snapshotFilePath); - - const { size } = yield OS.File.stat(snapshotFilePath); - const bulkPromise = this.conn.startBulkSend({ - actor: this.actorID, - type: "heap-snapshot", - length: size - }); - - const [bulk, stream] = yield Promise.all([bulkPromise, streamPromise]); - - try { - yield bulk.copyFrom(stream); - } finally { - stream.close(); - } - }), { - request: { - snapshotId: Arg(0, "string") - } - }), - takeCensus: actorBridge("takeCensus", { request: {}, response: RetVal("json") @@ -213,10 +182,11 @@ var MemoryActor = exports.MemoryActor = protocol.ActorClass({ }); exports.MemoryFront = protocol.FrontClass(MemoryActor, { - initialize: function(client, form) { + initialize: function(client, form, rootForm) { protocol.Front.prototype.initialize.call(this, client, form); this._client = client; this.actorID = form.memoryActor; + this.heapSnapshotFileActorID = rootForm.heapSnapshotFileActor; this.manage(this); }, @@ -225,9 +195,8 @@ exports.MemoryFront = protocol.FrontClass(MemoryActor, { * server and client do not share a file system, and return the local file * path to the heap snapshot. * - * NB: This will not work with sandboxed child processes, as they do not have - * access to the filesystem and the hep snapshot APIs do not support that use - * case yet. + * Note that this is safe to call for actors inside sandoxed child processes, + * as we jump through the correct IPDL hoops. * * @params Boolean options.forceCopy * Always force a bulk data copy of the saved heap snapshot, even when @@ -259,7 +228,7 @@ exports.MemoryFront = protocol.FrontClass(MemoryActor, { */ transferHeapSnapshot: protocol.custom(function (snapshotId) { const request = this._client.request({ - to: this.actorID, + to: this.heapSnapshotFileActorID, type: "transferHeapSnapshot", snapshotId }); diff --git a/devtools/server/actors/moz.build b/devtools/server/actors/moz.build index a15c1f3c9fe3..02fbefbaeed7 100644 --- a/devtools/server/actors/moz.build +++ b/devtools/server/actors/moz.build @@ -26,6 +26,7 @@ DevToolsModules( 'eventlooplag.js', 'framerate.js', 'gcli.js', + 'heap-snapshot-file.js', 'highlighters.css', 'highlighters.js', 'inspector.js', diff --git a/devtools/server/main.js b/devtools/server/main.js index ca70482e3a8d..048b8d2512b2 100644 --- a/devtools/server/main.js +++ b/devtools/server/main.js @@ -413,6 +413,11 @@ var DebuggerServer = { constructor: "DirectorRegistryActor", type: { global: true } }); + this.registerModule("devtools/server/actors/heap-snapshot-file", { + prefix: "heapSnapshotFile", + constructor: "HeapSnapshotFileActor", + type: { global: true } + }); }, /** diff --git a/devtools/server/tests/mochitest/memory-helpers.js b/devtools/server/tests/mochitest/memory-helpers.js index c484ffadb138..9b78361bdf3f 100644 --- a/devtools/server/tests/mochitest/memory-helpers.js +++ b/devtools/server/tests/mochitest/memory-helpers.js @@ -36,7 +36,7 @@ function startServerAndGetSelectedTabMemory() { } var form = response.tabs[response.selected]; - var memory = MemoryFront(client, form); + var memory = MemoryFront(client, form, response); resolve({ memory, client }); }); diff --git a/devtools/server/tests/unit/head_dbg.js b/devtools/server/tests/unit/head_dbg.js index 2d22d242e7df..f83e4a8395d2 100644 --- a/devtools/server/tests/unit/head_dbg.js +++ b/devtools/server/tests/unit/head_dbg.js @@ -52,10 +52,21 @@ function makeMemoryActorTest(testGeneratorFunction) { return function run_test() { do_test_pending(); startTestDebuggerServer(TEST_GLOBAL_NAME).then(client => { - getTestTab(client, TEST_GLOBAL_NAME, function (tabForm) { + DebuggerServer.registerModule("devtools/server/actors/heap-snapshot-file", { + prefix: "heapSnapshotFile", + constructor: "HeapSnapshotFileActor", + type: { global: true } + }); + + getTestTab(client, TEST_GLOBAL_NAME, function (tabForm, rootForm) { + if (!tabForm || !rootForm) { + ok(false, "Could not attach to test tab: " + TEST_GLOBAL_NAME); + return; + } + Task.spawn(function* () { try { - const memoryFront = new MemoryFront(client, tabForm); + const memoryFront = new MemoryFront(client, tabForm, rootForm); yield memoryFront.attach(); yield* testGeneratorFunction(client, memoryFront); yield memoryFront.detach(); @@ -294,7 +305,7 @@ function getTestTab(aClient, aTitle, aCallback) { aClient.listTabs(function (aResponse) { for (let tab of aResponse.tabs) { if (tab.title === aTitle) { - aCallback(tab); + aCallback(tab, aResponse); return; } }