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 Inspector | Assignee: | Timothy Hatcher <timothy> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | commit-queue, graouts, joepeck, timothy, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | All | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Joseph Pecoraro
2014-04-23 22:37:03 PDT
This was really bugging me in recent Timeline work. Was this fixed by recompiling the functions during timeline recording? (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. > But what we currently see is:
>
> Click Event (line 5)
> -> foo
Correction: Click Event (line 1)
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; Right now where the event was registered is a separate record, not attached to the firing records. 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. Lets just remove those lines of code and revisit this later if we have better ideas. Created attachment 236368 [details]
Patch
Comment on attachment 236368 [details]
Patch
r=me. Yeah, I just played with this on the tetris page and I like it much better!
Comment on attachment 236368 [details] Patch Clearing flags on attachment: 236368 Committed r172432: <http://trac.webkit.org/changeset/172432> All reviewed patches have been landed. Closing bug. |