Bug 132109

Summary: Web Inspector: Expect to see top level JavaScript function profiler details for event handlers in timeline
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Timothy Hatcher <timothy>
Severity: Normal CC: commit-queue, graouts, joepeck, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Description Flags
Patch none

Description Joseph Pecoraro 2014-04-23 22:37:03 PDT
If a JS event handler A calls function B then B shows up in the profile, but not A. This means that event handler functions themselves are not showing up with timeline profiler details.

1. Inspect <http://timothy.hatcher.name/tetris/>
2. Start a timeline
3. Press keys 'a', 's', 'd', 'f' in the inspected page
4. Stop timeline
5. Select the range with keydown events

- Notice that key events for 'a', 'd', and 'f' have no profiler information, despite the fact that they ran the "onKeyDown" function.
- The 's' handler does have profiler information for calling into "save game" code, but the top level "onKeyDown" function is not shown
Comment 1 Radar WebKit Bug Importer 2014-04-23 22:37:18 PDT
Comment 2 Joseph Pecoraro 2014-08-01 14:18:36 PDT
This was really bugging me in recent Timeline work.
Comment 3 Timothy Hatcher 2014-08-01 16:07:53 PDT
Was this fixed by recompiling the functions during timeline recording?
Comment 4 Joseph Pecoraro 2014-08-01 17:47:24 PDT
(In reply to comment #3)
> Was this fixed by recompiling the functions during timeline recording?

Nope, that just shows the proper functions underneath. The issue is with showing the top level function. So given:

    1. function myHandler(event) {
    2.     foo();
    3. }
    5. elem.addEventListener("click", myHandler, false);

I would expect to see:

    Click Event (line 5)
      -> myHandler (line 1)
        -> foo

But what we currently see is:

    Click Event (line 5)
      -> foo

Essentially the "myHandler" time is rolled up into the Click Event record. It would be good to see both (1) where the event was attached and (2) the handler being run.

Also, I don't know what happens if there are multiple click handlers. Does that mean we produce two "Click Event" records, or one? It might be nice to have 1, with all of the event listeners it executed nested under it.
Comment 5 Joseph Pecoraro 2014-08-01 17:48:25 PDT
> But what we currently see is:
>     Click Event (line 5)
>       -> foo

Correction: Click Event (line 1)
Comment 6 Timothy Hatcher 2014-08-01 18:14:29 PDT
Oh! That…

This is explicit and all done on the UI side. If there is only one listener, it becomes the top level record.

See ScriptTimelineView.js:208.

                // If there is only one node, promote its children. The TimelineRecordTreeElement already reflects the root
                // node in this case (e.g. a "Load Event Dispatched" record with an "onload" root profile node).
                // FIXME: Only do this for the top-down mode. Doing this for bottom-up would be incorrect.
                if (rootNodes.length === 1)
                    rootNodes = rootNodes[0].childNodes;
Comment 7 Timothy Hatcher 2014-08-01 18:17:46 PDT
Right now where the event was registered is a separate record, not attached to the firing records.
Comment 8 Timothy Hatcher 2014-08-01 22:01:44 PDT
This does have an effect on filtering. You can't filter for the entry function name. Either the entry function name needs to be included in the top level record, or it needs to be nested under it so filtering works.
Comment 9 Timothy Hatcher 2014-08-05 11:37:21 PDT
Lets just remove those lines of code and revisit this later if we have better ideas.
Comment 10 Timothy Hatcher 2014-08-11 08:37:21 PDT
Created attachment 236368 [details]
Comment 11 Joseph Pecoraro 2014-08-11 21:52:58 PDT
Comment on attachment 236368 [details]

r=me. Yeah, I just played with this on the tetris page and I like it much better!
Comment 12 WebKit Commit Bot 2014-08-12 00:57:15 PDT
Comment on attachment 236368 [details]

Clearing flags on attachment: 236368

Committed r172432: <http://trac.webkit.org/changeset/172432>
Comment 13 WebKit Commit Bot 2014-08-12 00:57:18 PDT
All reviewed patches have been landed.  Closing bug.