Bug 132109 – Web Inspector: Expect to see top level JavaScript function profiler details for event handlers in timeline

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

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.

* STEPS TO REPRODUCE
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

* RESULTS
- 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

[Radar WebKit Bug Importer]

<rdar://problem/16709018>

[Joseph Pecoraro]

This was really bugging me in recent Timeline work.

[Timothy Hatcher]

Was this fixed by recompiling the functions during timeline recording?

[Joseph Pecoraro]

(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. }
    4.
    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.

[Joseph Pecoraro]

> But what we currently see is:
> 
>     Click Event (line 5)
>       -> foo

Correction: Click Event (line 1)

[Timothy Hatcher]

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;

[Timothy Hatcher]

Right now where the event was registered is a separate record, not attached to the firing records.

[Timothy Hatcher]

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.

[Timothy Hatcher]

Lets just remove those lines of code and revisit this later if we have better ideas.

[Timothy Hatcher]

Created attachment 236368 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 236368 [details]
Patch

r=me. Yeah, I just played with this on the tetris page and I like it much better!

[WebKit Commit Bot]

Comment on attachment 236368 [details]
Patch

Clearing flags on attachment: 236368

Committed r172432: <http://trac.webkit.org/changeset/172432>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.