Bug 153455

Summary: Web Inspector: Include SamplingProfiler's expression-level data for stack frames in the protocol
Product: WebKit Reporter: Saam Barati <saam>
Component: Web InspectorAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, buildbot, commit-queue, graouts, joepeck, keith_miller, mark.lam, mattbaker, msaboff, nvasilyev, rniwa, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 152629    
Bug Blocks:    
Attachments:
Description Flags
patch
joepeck: review+, buildbot: commit-queue-
Archive of layout-test-results from ews116 for mac-yosemite
none
patch for landing.
buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-yosemite
none
patch for landing. none

Saam Barati
Reported 2016-01-25 15:21:52 PST
....
Attachments
patch (11.85 KB, patch)
2016-02-04 14:27 PST, Saam Barati
joepeck: review+
buildbot: commit-queue-
Archive of layout-test-results from ews116 for mac-yosemite (839.56 KB, application/zip)
2016-02-04 15:22 PST, Build Bot
no flags
patch for landing. (11.76 KB, patch)
2016-02-05 13:59 PST, Saam Barati
buildbot: commit-queue-
Archive of layout-test-results from ews103 for mac-yosemite (901.33 KB, application/zip)
2016-02-05 14:28 PST, Build Bot
no flags
patch for landing. (11.74 KB, patch)
2016-02-05 14:42 PST, Saam Barati
no flags
Radar WebKit Bug Importer
Comment 1 2016-01-25 15:22:10 PST
Saam Barati
Comment 2 2016-02-04 14:27:00 PST
Joseph Pecoraro
Comment 3 2016-02-04 14:52:45 PST
Comment on attachment 270696 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=270696&action=review r=me, but address renames > Source/JavaScriptCore/inspector/protocol/ScriptProfiler.json:20 > + "id": "Location", Location is getting to be a heavily overloaded term. I think "ExpressionLocation" would be better. > Source/JavaScriptCore/inspector/protocol/ScriptProfiler.json:24 > + { "name": "lineNumber", "type": "integer" }, > + { "name": "columnNumber", "type": "integer" } Perhaps we should shorten these to "line" and "column" like ScriptProfiler.StackFrame (below). Whatever we choose should be consistent. I think short is fine, especially for objects like this which will show many many times. > LayoutTests/inspector/sampling-profiler/expression-location-info.html:49 > + let lineNumber = parseInt(lineColumnHashedString.split(":")[0]); > + let columnNumber = parseInt(lineColumnHashedString.split(":")[1]); Avoid doing the split twice. Up to you if you want to do the map or just pull them out individually. let [lineNumber, columnNumber] = lineColumnHashedString.split(":").map(Number);
Build Bot
Comment 4 2016-02-04 15:22:34 PST
Comment on attachment 270696 [details] patch Attachment 270696 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/783383 New failing tests: inspector/sampling-profiler/call-frame-with-dom-functions.html
Build Bot
Comment 5 2016-02-04 15:22:38 PST
Created attachment 270698 [details] Archive of layout-test-results from ews116 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews116 Port: mac-yosemite Platform: Mac OS X 10.10.5
Saam Barati
Comment 6 2016-02-05 13:59:04 PST
Created attachment 270767 [details] patch for landing. going to let EWS run tests
Build Bot
Comment 7 2016-02-05 14:28:03 PST
Comment on attachment 270767 [details] patch for landing. Attachment 270767 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/787840 New failing tests: inspector/sampling-profiler/expression-location-info.html
Build Bot
Comment 8 2016-02-05 14:28:06 PST
Created attachment 270772 [details] Archive of layout-test-results from ews103 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews103 Port: mac-yosemite Platform: Mac OS X 10.10.5
Saam Barati
Comment 9 2016-02-05 14:42:48 PST
Created attachment 270773 [details] patch for landing. lets give EWS another try.
Saam Barati
Comment 10 2016-02-05 15:25:43 PST
Note You need to log in before you can comment on or make changes to this bug.