Bug 136805

Summary: Web Inspector: FunctionCall timeline records omit profile data if the debugger has paused
Product: WebKit Reporter: Brian Burg <burg>
Component: Web InspectorAssignee: Brian Burg <burg>
Status: RESOLVED FIXED    
Severity: Normal CC: graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
URL: http://www.nihilogic.dk/labs/tetris/
Bug Depends on:    
Bug Blocks: 136806    
Attachments:
Description Flags
Proposed Fix
none
Patch timothy: review+

Description Brian Burg 2014-09-13 23:00:18 PDT
Steps to reproduce:

1. Set a breakpoint in rotateActivePiece
2. Start the game and rotate a piece
3. Continue the debugger

The timeline record for Key Down will not have any children, despite the onKeyDown handler having executed some code underneath.

It appears that the timeline records are getting munged improperly when the debugger is running. In particular, we have the following timeline record structure, which fails to create the correct child records with profile data (in TimelineManager.eventRecorded function processRecord)

> EventDispatch (data: {"type": "keydown"})
--> FunctionCall (data: {"scriptLine: 93, "scriptName": "..."}) // NOTE: normally has 'profile' property containing root nodes
----> FunctionCall [10+] (data: {"profile": ..., "scriptLine": 1, "scriptName": "InjectedScript"})

The first nested FunctionCall record contains the expected event handler in its profile. Subsequent nested FunctionCall records contain things that actually look related to InjectedScript, and these should not be displayed on the timeline.
Comment 1 Brian Burg 2014-09-14 01:12:13 PDT
Created attachment 238090 [details]
Proposed Fix

The patch fixes this bug (regression?) but I don't have time to write a test right now. Feel free to pick it up and write one, or wait a few weeks.
Comment 2 Radar WebKit Bug Importer 2014-09-14 01:12:19 PDT
<rdar://problem/18331304>
Comment 3 Timothy Hatcher 2014-09-24 20:53:25 PDT
This patch looks fine to me.
Comment 4 Timothy Hatcher 2014-09-24 20:53:59 PDT
Comment on attachment 238090 [details]
Proposed Fix

I thought it landed, but I guess not.
Comment 5 Brian Burg 2014-09-25 13:48:38 PDT
Created attachment 238670 [details]
Patch
Comment 6 Timothy Hatcher 2014-09-25 14:52:05 PDT
Comment on attachment 238670 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=238670&action=review

> LayoutTests/inspector/timeline/debugger-paused-while-recording.html:56
> +    WebInspector.debuggerManager.addEventListener(WebInspector.DebuggerManager.Event.Paused,
> +        function(event) {
> +            InspectorTest.addResult("Debugger paused; resuming...");
> +
> +            WebInspector.debuggerManager.resume().then(function() {
> +                InspectorTest.addResult("Debugger resumed; stopping timeline capture.");
> +                WebInspector.timelineManager.stopCapturing();
> +            })
> +
> +    });

function(event) { feels better on the previous line, like the other examples in this file.

Also extra new line.
Comment 7 Brian Burg 2014-09-25 19:29:55 PDT
Committed r173992: <http://trac.webkit.org/changeset/173992>
Comment 8 Alexey Proskuryakov 2014-09-25 23:58:25 PDT
inspector/timeline/debugger-paused-while-recording.html is very flaky, failing nearly every time:

https://build.webkit.org/results/Apple%20Mavericks%20Release%20WK2%20(Tests)/r173993%20(8220)/inspector/timeline/debugger-paused-while-recording-pretty-diff.html
Comment 9 Brian Burg 2014-09-26 10:37:08 PDT
(In reply to comment #8)
> inspector/timeline/debugger-paused-while-recording.html is very flaky, failing nearly every time:
> 
> https://build.webkit.org/results/Apple%20Mavericks%20Release%20WK2%20(Tests)/r173993%20(8220)/inspector/timeline/debugger-paused-while-recording-pretty-diff.html

I skipped the new inspector/timelines/ directory in <http://trac.webkit.org/changeset/173998>, it seems to suffer similar problems to the debugger and replay tests. I'll take a look at this later, since this flakiness is a big deterrent to writing any tests.