From fcf455a3171ca70d1d745133d0c9e6097bd71449 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Fri, 10 Apr 2015 13:58:28 -0400 Subject: [PATCH] Backed out changeset 881ef048e290 (bug 1147945) for causing spammy TypeErrors that make sheriffing real failures difficult. --- browser/app/profile/firefox.js | 2 - browser/devtools/framework/gDevTools.jsm | 15 +++---- browser/devtools/performance/modules/front.js | 15 +++---- .../performance/modules/recording-model.js | 4 +- .../performance/performance-controller.js | 8 +--- browser/devtools/performance/test/browser.ini | 1 - .../test/browser_perf-options-profiler.js | 26 ----------- browser/devtools/performance/test/head.js | 12 ++--- toolkit/devtools/server/actors/profiler.js | 45 +++++-------------- 9 files changed, 28 insertions(+), 100 deletions(-) delete mode 100644 browser/devtools/performance/test/browser_perf-options-profiler.js diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 376c6b661f9c..4156fc1a6ecc 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1432,8 +1432,6 @@ pref("devtools.performance.enabled", true); pref("devtools.performance.memory.sample-probability", "0.05"); pref("devtools.performance.memory.max-log-length", 2147483647); // Math.pow(2,31) - 1 pref("devtools.performance.timeline.hidden-markers", "[]"); -pref("devtools.performance.profiler.buffer-size", 10000000); -pref("devtools.performance.profiler.sample-frequency-khz", 1); pref("devtools.performance.ui.invert-call-tree", true); pref("devtools.performance.ui.invert-flame-graph", false); pref("devtools.performance.ui.flatten-tree-recursion", true); diff --git a/browser/devtools/framework/gDevTools.jsm b/browser/devtools/framework/gDevTools.jsm index d36c6a5ff7f8..413a99588a60 100644 --- a/browser/devtools/framework/gDevTools.jsm +++ b/browser/devtools/framework/gDevTools.jsm @@ -11,6 +11,7 @@ const { classes: Cc, interfaces: Ci, utils: Cu } = Components; Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://gre/modules/Services.jsm"); Cu.import("resource://gre/modules/devtools/Loader.jsm"); + XPCOMUtils.defineLazyModuleGetter(this, "promise", "resource://gre/modules/Promise.jsm", "Promise"); @@ -24,8 +25,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "DebuggerServer", XPCOMUtils.defineLazyModuleGetter(this, "DebuggerClient", "resource://gre/modules/devtools/dbg-client.jsm"); -XPCOMUtils.defineLazyModuleGetter(this, "Task", - "resource://gre/modules/Task.jsm"); const EventEmitter = devtools.require("devtools/toolkit/event-emitter"); const Telemetry = devtools.require("devtools/shared/telemetry"); @@ -1219,8 +1218,8 @@ let gDevToolsBrowser = { */ _connectToProfiler: function DT_connectToProfiler(event, toolbox) { let SharedPerformanceUtils = devtools.require("devtools/performance/front"); - this._performanceConnection = SharedPerformanceUtils.getPerformanceActorsConnection(toolbox.target); - this._performanceConnection.open(); + let connection = SharedPerformanceUtils.getPerformanceActorsConnection(toolbox.target); + connection.open(); }, /** @@ -1330,15 +1329,11 @@ let gDevToolsBrowser = { /** * All browser windows have been closed, tidy up remaining objects. */ - destroy: Task.async(function*() { + destroy: function() { gDevTools.off("toolbox-ready", gDevToolsBrowser._connectToProfiler); Services.prefs.removeObserver("devtools.", gDevToolsBrowser); Services.obs.removeObserver(gDevToolsBrowser.destroy, "quit-application"); - - if (this._performanceConnection) { - yield this._performanceConnection.destroy(); - } - }), + }, } this.gDevToolsBrowser = gDevToolsBrowser; diff --git a/browser/devtools/performance/modules/front.js b/browser/devtools/performance/modules/front.js index 4eb9d5c47e9e..221a05f6a749 100644 --- a/browser/devtools/performance/modules/front.js +++ b/browser/devtools/performance/modules/front.js @@ -247,7 +247,7 @@ PerformanceFront.prototype = { startRecording: Task.async(function*(options = {}) { // All actors are started asynchronously over the remote debugging protocol. // Get the corresponding start times from each one of them. - let profilerStartTime = yield this._startProfiler(options); + let profilerStartTime = yield this._startProfiler(); let timelineStartTime = yield this._startTimeline(options); let memoryStartTime = yield this._startMemory(options); @@ -286,7 +286,7 @@ PerformanceFront.prototype = { /** * Starts the profiler actor, if necessary. */ - _startProfiler: Task.async(function *(options={}) { + _startProfiler: Task.async(function *() { // Start the profiler only if it wasn't already active. The built-in // nsIPerformance module will be kept recording, because it's the same instance // for all targets and interacts with the whole platform, so we don't want @@ -297,13 +297,10 @@ PerformanceFront.prototype = { return profilerStatus.currentTime; } - // Translate options from the recording model into profiler-specific - // options for the nsIProfiler - let profilerOptions = { - entries: options.bufferSize, - interval: options.sampleFrequency ? (1000 / (options.sampleFrequency * 1000)) : void 0 - }; - + // If this._customProfilerOptions is defined, use those to pass in + // to the profiler actor. The profiler actor handles all the defaults + // now, so this should only be used for tests. + let profilerOptions = this._customProfilerOptions || {}; yield this._request("profiler", "startProfiler", profilerOptions); this.emit("profiler-activated"); diff --git a/browser/devtools/performance/modules/recording-model.js b/browser/devtools/performance/modules/recording-model.js index a246f356bfdb..9aa9002d85ad 100644 --- a/browser/devtools/performance/modules/recording-model.js +++ b/browser/devtools/performance/modules/recording-model.js @@ -26,9 +26,7 @@ const RecordingModel = function (options={}) { withMemory: options.withMemory || false, withAllocations: options.withAllocations || false, allocationsSampleProbability: options.allocationsSampleProbability || 0, - allocationsMaxLogLength: options.allocationsMaxLogLength || 0, - bufferSize: options.bufferSize || 0, - sampleFrequency: options.sampleFrequency || 1 + allocationsMaxLogLength: options.allocationsMaxLogLength || 0 }; }; diff --git a/browser/devtools/performance/performance-controller.js b/browser/devtools/performance/performance-controller.js index cc9642e70fdc..8dfe9b008ef4 100644 --- a/browser/devtools/performance/performance-controller.js +++ b/browser/devtools/performance/performance-controller.js @@ -199,9 +199,7 @@ let PerformanceController = { this._nonBooleanPrefs = new ViewHelpers.Prefs("devtools.performance", { "hidden-markers": ["Json", "timeline.hidden-markers"], "memory-sample-probability": ["Float", "memory.sample-probability"], - "memory-max-log-length": ["Int", "memory.max-log-length"], - "profiler-buffer-size": ["Int", "profiler.buffer-size"], - "profiler-sample-frequency": ["Int", "profiler.sample-frequency-khz"] + "memory-max-log-length": ["Int", "memory.max-log-length"] }); this._nonBooleanPrefs.registerObserver(); @@ -292,9 +290,7 @@ let PerformanceController = { withTicks: this.getOption("enable-framerate"), withAllocations: this.getOption("enable-memory"), allocationsSampleProbability: this.getPref("memory-sample-probability"), - allocationsMaxLogLength: this.getPref("memory-max-log-length"), - bufferSize: this.getPref("profiler-buffer-size"), - sampleFrequency: this.getPref("profiler-sample-frequency") + allocationsMaxLogLength: this.getPref("memory-max-log-length") }); this.emit(EVENTS.RECORDING_WILL_START, recording); diff --git a/browser/devtools/performance/test/browser.ini b/browser/devtools/performance/test/browser.ini index 3ed1e2fe9bd1..b01869a7edea 100644 --- a/browser/devtools/performance/test/browser.ini +++ b/browser/devtools/performance/test/browser.ini @@ -67,7 +67,6 @@ support-files = [browser_perf-options-enable-memory-02.js] [browser_perf-options-enable-framerate.js] [browser_perf-options-allocations.js] -[browser_perf-options-profiler.js] [browser_perf-overview-render-01.js] [browser_perf-overview-render-02.js] [browser_perf-overview-render-03.js] diff --git a/browser/devtools/performance/test/browser_perf-options-profiler.js b/browser/devtools/performance/test/browser_perf-options-profiler.js deleted file mode 100644 index 48d39685d255..000000000000 --- a/browser/devtools/performance/test/browser_perf-options-profiler.js +++ /dev/null @@ -1,26 +0,0 @@ -/* Any copyright is dedicated to the Public Domain. - http://creativecommons.org/publicdomain/zero/1.0/ */ - - -/** - * Tests that setting the `devtools.performance.profiler.` prefs propagate to the profiler actor. - */ -function spawnTest () { - let { panel } = yield initPerformance(SIMPLE_URL); - let { gFront } = panel.panelWin; - - Services.prefs.setIntPref(PROFILER_BUFFER_SIZE_PREF, 1000); - Services.prefs.setIntPref(PROFILER_SAMPLE_RATE_PREF, 2); - - yield startRecording(panel); - - let { entries, interval } = yield gFront._request("profiler", "getStartOptions"); - - yield stopRecording(panel); - - is(entries, 1000, "profiler entries option is set on profiler"); - is(interval, 0.5, "profiler interval option is set on profiler"); - - yield teardown(panel); - finish(); -} diff --git a/browser/devtools/performance/test/head.js b/browser/devtools/performance/test/head.js index f9de433f67eb..3075954941d4 100644 --- a/browser/devtools/performance/test/head.js +++ b/browser/devtools/performance/test/head.js @@ -5,7 +5,7 @@ const { classes: Cc, interfaces: Ci, utils: Cu, results: Cr } = Components; let { Services } = Cu.import("resource://gre/modules/Services.jsm", {}); -let { Preferences } = Cu.import("resource://gre/modules/Preferences.jsm", {}); + let { Task } = Cu.import("resource://gre/modules/Task.jsm", {}); let { Promise } = Cu.import("resource://gre/modules/Promise.jsm", {}); let { devtools } = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); @@ -25,8 +25,6 @@ const SIMPLE_URL = EXAMPLE_URL + "doc_simple-test.html"; const MEMORY_SAMPLE_PROB_PREF = "devtools.performance.memory.sample-probability"; const MEMORY_MAX_LOG_LEN_PREF = "devtools.performance.memory.max-log-length"; -const PROFILER_BUFFER_SIZE_PREF = "devtools.performance.profiler.buffer-size"; -const PROFILER_SAMPLE_RATE_PREF = "devtools.performance.profiler.sample-frequency-khz"; const FRAMERATE_PREF = "devtools.performance.ui.enable-framerate"; const MEMORY_PREF = "devtools.performance.ui.enable-memory"; @@ -52,12 +50,8 @@ let DEFAULT_PREFS = [ "devtools.performance.ui.enable-memory", "devtools.performance.ui.enable-framerate", "devtools.performance.ui.show-jit-optimizations", - "devtools.performance.memory.sample-probability", - "devtools.performance.memory.max-log-length", - "devtools.performance.profiler.buffer-size", - "devtools.performance.profiler.sample-frequency-khz", ].reduce((prefs, pref) => { - prefs[pref] = Preferences.get(pref); + prefs[pref] = Services.prefs.getBoolPref(pref); return prefs; }, {}); @@ -84,7 +78,7 @@ registerCleanupFunction(() => { // Rollback any pref changes Object.keys(DEFAULT_PREFS).forEach(pref => { - Preferences.set(pref, DEFAULT_PREFS[pref]); + Services.prefs.setBoolPref(pref, DEFAULT_PREFS[pref]); }); // Make sure the profiler module is stopped when the test finishes. diff --git a/toolkit/devtools/server/actors/profiler.js b/toolkit/devtools/server/actors/profiler.js index 80a2e808905e..8720ae12bbc9 100644 --- a/toolkit/devtools/server/actors/profiler.js +++ b/toolkit/devtools/server/actors/profiler.js @@ -7,16 +7,10 @@ const {Cc, Ci, Cu, Cr} = require("chrome"); const Services = require("Services"); const DevToolsUtils = require("devtools/toolkit/DevToolsUtils.js"); -let DEFAULT_PROFILER_OPTIONS = { - // When using the DevTools Performance Tools, this will be overridden - // by the pref `devtools.performance.profiler.buffer-size`. - entries: Math.pow(10, 7), - // When using the DevTools Performance Tools, this will be overridden - // by the pref `devtools.performance.profiler.sample-rate-khz`. - interval: 1, - features: ["js"], - threadFilters: ["GeckoMain"] -}; +let DEFAULT_PROFILER_ENTRIES = 10000000; +let DEFAULT_PROFILER_INTERVAL = 1; +let DEFAULT_PROFILER_FEATURES = ["js"]; +let DEFAULT_PROFILER_THREADFILTERS = ["GeckoMain"]; /** * The nsIProfiler is target agnostic and interacts with the whole platform. @@ -62,14 +56,6 @@ ProfilerActor.prototype = { return { features: nsIProfilerModule.GetFeatures([]) }; }, - /** - * Returns the configuration used that was originally passed in to start up the - * profiler. Used for tests, and does not account for others using nsIProfiler. - */ - onGetStartOptions: function() { - return this._profilerStartOptions || {}; - }, - /** * Starts the nsIProfiler module. Doing so will discard any samples * that might have been accumulated so far. @@ -80,21 +66,13 @@ ProfilerActor.prototype = { * @param array:string threadFilters [description] */ onStartProfiler: function(request = {}) { - let options = this._profilerStartOptions = { - entries: request.entries || DEFAULT_PROFILER_OPTIONS.entries, - interval: request.interval || DEFAULT_PROFILER_OPTIONS.interval, - features: request.features || DEFAULT_PROFILER_OPTIONS.features, - threadFilters: request.threadFilters || DEFAULT_PROFILER_OPTIONS.threadFilters, - }; - nsIProfilerModule.StartProfiler( - options.entries, - options.interval, - options.features, - options.features.length, - options.threadFilters, - options.threadFilters.length - ); + (request.entries || DEFAULT_PROFILER_ENTRIES), + (request.interval || DEFAULT_PROFILER_INTERVAL), + (request.features || DEFAULT_PROFILER_FEATURES), + (request.features || DEFAULT_PROFILER_FEATURES).length, + (request.threadFilters || DEFAULT_PROFILER_THREADFILTERS), + (request.threadFilters || DEFAULT_PROFILER_THREADFILTERS).length); return { started: true }; }, @@ -353,6 +331,5 @@ ProfilerActor.prototype.requestTypes = { "getSharedLibraryInformation": ProfilerActor.prototype.onGetSharedLibraryInformation, "getProfile": ProfilerActor.prototype.onGetProfile, "registerEventNotifications": ProfilerActor.prototype.onRegisterEventNotifications, - "unregisterEventNotifications": ProfilerActor.prototype.onUnregisterEventNotifications, - "getStartOptions": ProfilerActor.prototype.onGetStartOptions + "unregisterEventNotifications": ProfilerActor.prototype.onUnregisterEventNotifications };