Bug 136805 - Web Inspector: FunctionCall timeline records omit profile data if the debugger has paused
Summary: Web Inspector: FunctionCall timeline records omit profile data if the debugge...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Brian Burg
URL: http://www.nihilogic.dk/labs/tetris/
Keywords: InRadar
Depends on:
Blocks: 136806
  Show dependency treegraph
 
Reported: 2014-09-13 23:00 PDT by Brian Burg
Modified: 2014-09-26 10:37 PDT (History)
4 users (show)

See Also:


Attachments
Proposed Fix (4.77 KB, patch)
2014-09-14 01:12 PDT, Brian Burg
no flags Details | Formatted Diff | Diff
Patch (12.92 KB, patch)
2014-09-25 13:48 PDT, Brian Burg
timothy: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.