From 94946f8488afdfb6f186826d29359879ea1fcb34 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Sat, 26 Sep 2015 06:21:09 +0200 Subject: [PATCH] Bug 1196947 - Performance tools should display a message in private browsing, r=jsantell --- .../chrome/browser/devtools/performance.dtd | 7 +- devtools/client/performance/events.js | 3 + .../performance/performance-controller.js | 54 ++++++++++- .../client/performance/performance-view.js | 67 ++++++++----- devtools/client/performance/performance.xul | 24 +++++ devtools/client/performance/test/browser.ini | 4 +- .../test/browser_perf-console-record-01.js | 2 +- .../test/browser_perf-console-record-02.js | 6 +- .../test/browser_perf-console-record-03.js | 4 +- .../test/browser_perf-highlighted.js | 2 +- .../test/browser_perf-legacy-front-06.js | 2 +- .../test/browser_perf-private-browsing.js | 93 +++++++++++++++++++ devtools/client/performance/test/head.js | 29 +++++- devtools/server/actors/performance.js | 19 +++- devtools/shared/performance/recorder.js | 19 ++++ devtools/shared/shared/profiler.js | 57 +++++++++--- tools/profiler/gecko/nsIProfiler.idl | 5 +- tools/profiler/gecko/nsProfiler.cpp | 7 ++ 18 files changed, 344 insertions(+), 60 deletions(-) create mode 100644 devtools/client/performance/test/browser_perf-private-browsing.js diff --git a/browser/locales/en-US/chrome/browser/devtools/performance.dtd b/browser/locales/en-US/chrome/browser/devtools/performance.dtd index 9be465a6bb0..e1f210a5a95 100644 --- a/browser/locales/en-US/chrome/browser/devtools/performance.dtd +++ b/browser/locales/en-US/chrome/browser/devtools/performance.dtd @@ -35,7 +35,12 @@ + - in the details view while the profiler is unavailable, for example, while + - in Private Browsing mode. --> + + + + + + + + + + + + + + PerformanceController.getRecordings().length === 2); @@ -39,6 +38,7 @@ function* spawnTest() { let profileEnd = once(front, "recording-stopped"); console.profileEnd("rust"); yield profileEnd; + profileEnd = once(front, "recording-stopped"); console.profileEnd("rust2"); yield profileEnd; diff --git a/devtools/client/performance/test/browser_perf-console-record-03.js b/devtools/client/performance/test/browser_perf-console-record-03.js index ce39180d570..8cc2731c301 100644 --- a/devtools/client/performance/test/browser_perf-console-record-03.js +++ b/devtools/client/performance/test/browser_perf-console-record-03.js @@ -6,8 +6,6 @@ * also console recordings that have finished before it was opened. */ -var WAIT_TIME = 10; - function* spawnTest() { let { target, toolbox, console } = yield initConsole(SIMPLE_URL); let front = toolbox.performance; @@ -25,7 +23,7 @@ function* spawnTest() { yield profileStart; yield gDevTools.showToolbox(target, "performance"); - let panel = toolbox.getCurrentPanel(); + let panel = yield toolbox.getCurrentPanel().open(); let { panelWin: { PerformanceController, RecordingsView }} = panel; yield waitUntil(() => PerformanceController.getRecordings().length === 2); diff --git a/devtools/client/performance/test/browser_perf-highlighted.js b/devtools/client/performance/test/browser_perf-highlighted.js index 7f9e98f93b6..2b9ab932714 100644 --- a/devtools/client/performance/test/browser_perf-highlighted.js +++ b/devtools/client/performance/test/browser_perf-highlighted.js @@ -28,7 +28,7 @@ function* spawnTest() { "performance tab is no longer highlighted when console.profile recording finishes"); yield gDevTools.showToolbox(target, "performance"); - let panel = toolbox.getCurrentPanel(); + let panel = yield toolbox.getCurrentPanel().open(); let { panelWin: { PerformanceController, RecordingsView }} = panel; yield startRecording(panel); diff --git a/devtools/client/performance/test/browser_perf-legacy-front-06.js b/devtools/client/performance/test/browser_perf-legacy-front-06.js index 9c89e584fcc..6339f203420 100644 --- a/devtools/client/performance/test/browser_perf-legacy-front-06.js +++ b/devtools/client/performance/test/browser_perf-legacy-front-06.js @@ -20,7 +20,7 @@ function* spawnTest() { yield profileStart; yield gDevTools.showToolbox(target, "performance"); - let panel = toolbox.getCurrentPanel(); + let panel = yield toolbox.getCurrentPanel().open(); let { panelWin: { PerformanceController, RecordingsView }} = panel; yield waitUntil(() => PerformanceController.getRecordings().length === 2); diff --git a/devtools/client/performance/test/browser_perf-private-browsing.js b/devtools/client/performance/test/browser_perf-private-browsing.js new file mode 100644 index 00000000000..e7ee16c6414 --- /dev/null +++ b/devtools/client/performance/test/browser_perf-private-browsing.js @@ -0,0 +1,93 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests that disables the frontend when in private browsing mode. + */ + +let gPanelWinTuples = []; + +function* spawnTest() { + yield testNormalWindow(); + yield testPrivateWindow(); + yield testRecordingFailingInWindow(0); + yield testRecordingFailingInWindow(1); + yield teardownPerfInWindow(1); + yield testRecordingSucceedingInWindow(0); + yield teardownPerfInWindow(0); + + gPanelWinTuples = null; + finish(); +} + +function* createPanelInWindow(options) { + let win = yield addWindow(options); + let tab = yield addTab(SIMPLE_URL, win); + let target = TargetFactory.forTab(tab); + yield target.makeRemote(); + + let toolbox = yield gDevTools.showToolbox(target, "performance"); + yield toolbox.initPerformance(); + + let panel = yield toolbox.getCurrentPanel().open(); + gPanelWinTuples.push({ panel, win }); + + return { panel, win }; +} + +function* testNormalWindow() { + let { panel } = yield createPanelInWindow({ private: false }); + let { PerformanceView } = panel.panelWin; + + is(PerformanceView.getState(), "empty", + "The initial state of the performance panel view is correct (1)."); +} + +function* testPrivateWindow() { + let { panel } = yield createPanelInWindow({ private: true }); + let { PerformanceView } = panel.panelWin; + + is(PerformanceView.getState(), "unavailable", + "The initial state of the performance panel view is correct (2)."); +} + +function* testRecordingFailingInWindow(index) { + let { panel } = gPanelWinTuples[index]; + let { EVENTS, PerformanceController } = panel.panelWin; + + let onRecordingStarted = () => { + ok(false, "Recording should not start while a private window is present."); + }; + + PerformanceController.on(EVENTS.RECORDING_STARTED, onRecordingStarted); + + let whenFailed = once(PerformanceController, EVENTS.NEW_RECORDING_FAILED); + PerformanceController.startRecording(); + yield whenFailed; + ok(true, "Recording has failed."); + + PerformanceController.off(EVENTS.RECORDING_STARTED, onRecordingStarted); +} + +function* testRecordingSucceedingInWindow(index) { + let { panel } = gPanelWinTuples[index]; + let { EVENTS, PerformanceController } = panel.panelWin; + + let onRecordingFailed = () => { + ok(false, "Recording should start while now private windows are present."); + }; + + PerformanceController.on(EVENTS.NEW_RECORDING_FAILED, onRecordingFailed); + + yield startRecording(panel); + yield stopRecording(panel); + ok(true, "Recording has succeeded."); + + PerformanceController.off(EVENTS.RECORDING_STARTED, onRecordingFailed); +} + +function* teardownPerfInWindow(index) { + let { panel, win } = gPanelWinTuples[index]; + yield teardown(panel, win); + win.close(); +} diff --git a/devtools/client/performance/test/head.js b/devtools/client/performance/test/head.js index 1a00c87a473..9d623359541 100644 --- a/devtools/client/performance/test/head.js +++ b/devtools/client/performance/test/head.js @@ -105,6 +105,29 @@ registerCleanupFunction(() => { Cu.forceGC(); }); + +function whenDelayedStartupFinished(aWindow, aCallback) { + Services.obs.addObserver(function observer(aSubject, aTopic) { + if (aWindow == aSubject) { + Services.obs.removeObserver(observer, aTopic); + executeSoon(aCallback); + } + }, "browser-delayed-startup-finished", false); +} + +function addWindow(windowOptions) { + let deferred = Promise.defer(); + let win = OpenBrowserWindow(windowOptions); + + whenDelayedStartupFinished(win, () => { + executeSoon(() => { + deferred.resolve(win); + }); + }); + + return deferred.promise; +} + function addTab(aUrl, aWindow) { info("Adding tab: " + aUrl); @@ -233,6 +256,8 @@ function initPerformance(aUrl, tool="performance", targetOps={}) { // Wait for the performance tool to be spun up yield toolbox.initPerformance(); + // Panel is already initialized after `showToolbox` and `initPerformance`, + // no need to wait for `open` here. let panel = toolbox.getCurrentPanel(); return { target, panel, toolbox }; }); @@ -276,12 +301,12 @@ function consoleExecute (console, method, val) { return promise; } -function* teardown(panel) { +function* teardown(panel, win = window) { info("Destroying the performance tool."); let tab = panel.target.tab; yield panel._toolbox.destroy(); - yield removeTab(tab); + yield removeTab(tab, win); } function idleWait(time) { diff --git a/devtools/server/actors/performance.js b/devtools/server/actors/performance.js index 0f4486d13f3..f45c26beb12 100644 --- a/devtools/server/actors/performance.js +++ b/devtools/server/actors/performance.js @@ -107,10 +107,19 @@ var PerformanceActor = exports.PerformanceActor = protocol.ActorClass({ response: RetVal("json") }), + canCurrentlyRecord: method(function() { + return this.bridge.canCurrentlyRecord(); + }, { + response: { value: RetVal("json") } + }), + startRecording: method(Task.async(function *(options={}) { + if (!this.bridge.canCurrentlyRecord().success) { + return null; + } + let normalizedOptions = normalizePerformanceFeatures(options, this.traits.features); let recording = yield this.bridge.startRecording(normalizedOptions); - this.manage(recording); return recording; @@ -118,14 +127,18 @@ var PerformanceActor = exports.PerformanceActor = protocol.ActorClass({ request: { options: Arg(0, "nullable:json"), }, - response: RetVal("performance-recording"), + response: { + recording: RetVal("nullable:performance-recording") + } }), stopRecording: actorBridge("stopRecording", { request: { options: Arg(0, "performance-recording"), }, - response: RetVal("performance-recording"), + response: { + recording: RetVal("performance-recording") + } }), isRecording: actorBridge("isRecording", { diff --git a/devtools/shared/performance/recorder.js b/devtools/shared/performance/recorder.js index 43e4ba82082..1e244eca9e2 100644 --- a/devtools/shared/performance/recorder.js +++ b/devtools/shared/performance/recorder.js @@ -282,6 +282,25 @@ const PerformanceRecorder = exports.PerformanceRecorder = Class({ } }, + /** + * Checks whether or not recording is currently supported. At the moment, + * this is only influenced by private browsing mode and the profiler. + */ + canCurrentlyRecord: function() { + let success = true; + let reasons = []; + + if (!Profiler.canProfile()) { + success = false, + reasons.push("profiler-unavailable"); + } + + // Check other factors that will affect the possibility of successfully + // starting a recording here. + + return { success, reasons }; + }, + /** * Begins a recording session * diff --git a/devtools/shared/shared/profiler.js b/devtools/shared/shared/profiler.js index 97c8059dce1..585737096da 100644 --- a/devtools/shared/shared/profiler.js +++ b/devtools/shared/shared/profiler.js @@ -55,7 +55,10 @@ const ProfilerManager = (function () { * The nsIProfiler is target agnostic and interacts with the whole platform. * Therefore, special care needs to be given to make sure different profiler * consumers (i.e. "toolboxes") don't interfere with each other. Register - * the instance here. + * the profiler actor instances here. + * + * @param Profiler instance + * A profiler actor class. */ addInstance: function (instance) { consumers.add(instance); @@ -64,6 +67,12 @@ const ProfilerManager = (function () { this.registerEventListeners(); }, + /** + * Remove the profiler actor instances here. + * + * @param Profiler instance + * A profiler actor class. + */ removeInstance: function (instance) { consumers.delete(instance); @@ -101,25 +110,36 @@ const ProfilerManager = (function () { // interested in. let currentTime = nsIProfilerModule.getElapsedTime(); - nsIProfilerModule.StartProfiler( - config.entries, - config.interval, - config.features, - config.features.length, - config.threadFilters, - config.threadFilters.length - ); - let { position, totalSize, generation } = this.getBufferInfo(); + try { + nsIProfilerModule.StartProfiler( + config.entries, + config.interval, + config.features, + config.features.length, + config.threadFilters, + config.threadFilters.length + ); + } catch (e) { + // For some reason, the profiler couldn't be started. This could happen, + // for example, when in private browsing mode. + Cu.reportError(`Could not start the profiler module: ${e.message}`); + return { started: false, reason: e, currentTime }; + } this._updateProfilerStatusPolling(); + + let { position, totalSize, generation } = this.getBufferInfo(); return { started: true, position, totalSize, generation, currentTime }; }, + /** + * Attempts to stop the nsIProfiler module. + */ stop: function () { // Actually stop the profiler only if the last client has stopped profiling. - // Since this is used as a root actor, and the profiler module interacts with the - // whole platform, we need to avoid a case in which the profiler is stopped - // when there might be other clients still profiling. + // Since this is used as a root actor, and the profiler module interacts + // with the whole platform, we need to avoid a case in which the profiler + // is stopped when there might be other clients still profiling. if (this.length <= 1) { nsIProfilerModule.StopProfiler(); } @@ -306,7 +326,8 @@ const ProfilerManager = (function () { */ unregisterEventListeners: function () { if (this._eventsRegistered) { - PROFILER_SYSTEM_EVENTS.forEach(eventName => Services.obs.removeObserver(this, eventName)); + PROFILER_SYSTEM_EVENTS.forEach(eventName => + Services.obs.removeObserver(this, eventName)); this._eventsRegistered = false; } }, @@ -482,6 +503,14 @@ var Profiler = exports.Profiler = Class({ }, }); +/** + * Checks whether or not the profiler module can currently run. + * @return boolean + */ +Profiler.canProfile = function() { + return nsIProfilerModule.CanProfile(); +}; + /** * JSON.stringify callback used in Profiler.prototype.observe. */ diff --git a/tools/profiler/gecko/nsIProfiler.idl b/tools/profiler/gecko/nsIProfiler.idl index 07806c96433..bb019adaf24 100644 --- a/tools/profiler/gecko/nsIProfiler.idl +++ b/tools/profiler/gecko/nsIProfiler.idl @@ -2,7 +2,7 @@ /* 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/. */ - + #include "nsISupports.idl" %{C++ @@ -12,9 +12,10 @@ class nsCString; [ref] native StringArrayRef(const nsTArray); -[scriptable, uuid(921e1223-b1ea-4906-bb26-a846e6b6835b)] +[scriptable, uuid(ff398a14-df1c-4966-9ab2-772ea6a6da6c)] interface nsIProfiler : nsISupports { + boolean CanProfile(); void StartProfiler(in uint32_t aEntries, in double aInterval, [array, size_is(aFeatureCount)] in string aFeatures, in uint32_t aFeatureCount, diff --git a/tools/profiler/gecko/nsProfiler.cpp b/tools/profiler/gecko/nsProfiler.cpp index 95558696dd9..959c8bda2d4 100644 --- a/tools/profiler/gecko/nsProfiler.cpp +++ b/tools/profiler/gecko/nsProfiler.cpp @@ -70,6 +70,13 @@ nsProfiler::Observe(nsISupports *aSubject, return NS_OK; } +NS_IMETHODIMP +nsProfiler::CanProfile(bool *aCanProfile) +{ + *aCanProfile = !mLockedForPrivateBrowsing; + return NS_OK; +} + NS_IMETHODIMP nsProfiler::StartProfiler(uint32_t aEntries, double aInterval, const char** aFeatures, uint32_t aFeatureCount,