scenario: developer made CPUProfile on a page. developer fixed the problem developer made another CPUProfile developer has to see the difference but he might be confused by the different colors associated with the same function in the different profiles.
Created attachment 195331 [details] Patch
Comment on attachment 195331 [details] Patch Attachment 195331 [details] did not pass win-ews (win): Output: http://webkit-commit-queue.appspot.com/results/17294365
Comment on attachment 195331 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195331&action=review > Source/WebCore/inspector/front-end/FlameChart.js:74 > + var profilesPanel = WebInspector.panels["profiles"]; FlameChart should not depend on the panel. > Source/WebCore/inspector/front-end/FlameChart.js:77 > + this._colorGenerator = profilesPanel.colorGenerator = new WebInspector.FlameChart.ColorGenerator(); A statement per line please.
Created attachment 195549 [details] Patch
(In reply to comment #3) > (From update of attachment 195331 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=195331&action=review > > > Source/WebCore/inspector/front-end/FlameChart.js:74 > > + var profilesPanel = WebInspector.panels["profiles"]; > > FlameChart should not depend on the panel. done > > > Source/WebCore/inspector/front-end/FlameChart.js:77 > > + this._colorGenerator = profilesPanel.colorGenerator = new WebInspector.FlameChart.ColorGenerator(); > > A statement per line please. done
Created attachment 195551 [details] screenshot
Comment on attachment 195549 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195549&action=review > Source/WebCore/inspector/front-end/FlameChart.js:75 > + this._colorGenerator = panel.colorGenerator; You can't declare properties on alien objects, just make it static! I.e. if (!WebInspector.FlameChart._colorGenerator)
Created attachment 195729 [details] Patch
Comment on attachment 195729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195729&action=review > Source/WebCore/inspector/front-end/FlameChart.js:74 > + this._colorGenerator = WebInspector.FlameChart._colorGenerator; Why do you need local field? > Source/WebCore/inspector/front-end/FlameChart.js:289 > + var colorGenerator = this._colorGenerator; Why do you need this variable? > Source/WebCore/inspector/front-end/FlameChart.js:348 > + var colorGenerator = this._colorGenerator; ditto
Comment on attachment 195729 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=195729&action=review >> Source/WebCore/inspector/front-end/FlameChart.js:289 >> + var colorGenerator = this._colorGenerator; > > Why do you need this variable? I don't like to access to member field in a cycle which runs huge number of times because usually it reduces performance.
Created attachment 195731 [details] Patch
Committed r147213: <http://trac.webkit.org/changeset/147213>