132109 2014-04-23 22:37:03 -0700 Web Inspector: Expect to see top level JavaScript function profiler details for event handlers in timeline 2014-08-12 00:57:18 -0700 1 1 1 Unclassified WebKit Web Inspector 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck timothy commit-queue graouts joepeck timothy webkit-bug-importer oldest_to_newest 1003544 0 joepeck 2014-04-23 22:37:03 -0700 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 1003545 1 webkit-bug-importer 2014-04-23 22:37:18 -0700 <rdar://problem/16709018> 1026332 2 joepeck 2014-08-01 14:18:36 -0700 This was really bugging me in recent Timeline work. 1026380 3 timothy 2014-08-01 16:07:53 -0700 Was this fixed by recompiling the functions during timeline recording? 1026410 4 joepeck 2014-08-01 17:47:24 -0700 (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. 1026411 5 joepeck 2014-08-01 17:48:25 -0700 > But what we currently see is: > > Click Event (line 5) > -> foo Correction: Click Event (line 1) 1026414 6 timothy 2014-08-01 18:14:29 -0700 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; 1026415 7 timothy 2014-08-01 18:17:46 -0700 Right now where the event was registered is a separate record, not attached to the firing records. 1026438 8 timothy 2014-08-01 22:01:44 -0700 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. 1026901 9 timothy 2014-08-05 11:37:21 -0700 Lets just remove those lines of code and revisit this later if we have better ideas. 1028186 10 236368 timothy 2014-08-11 08:37:21 -0700 Created attachment 236368 Patch 1028415 11 236368 joepeck 2014-08-11 21:52:58 -0700 Comment on attachment 236368 Patch r=me. Yeah, I just played with this on the tetris page and I like it much better! 1028448 12 236368 commit-queue 2014-08-12 00:57:15 -0700 Comment on attachment 236368 Patch Clearing flags on attachment: 236368 Committed r172432: <http://trac.webkit.org/changeset/172432> 1028449 13 commit-queue 2014-08-12 00:57:18 -0700 All reviewed patches have been landed. Closing bug. 236368 2014-08-11 08:37:21 -0700 2014-08-12 00:57:15 -0700 Patch bug-132109-20140811083710.patch text/plain 2061 timothy U3VidmVyc2lvbiBSZXZpc2lvbjogMTcyMzcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCA1 ZGQ1NDdhMDU0ZDUyMWNiNGViY2U2YTQ4Mzk3MDhjM2I0M2ZlYTA3Li45ZTZhZGYzMDMwNDBlYzJm ODA2MTU3YmQxNzkyMTIwZDI5ZTU4NzcxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNCBAQAorMjAxNC0wOC0xMSAgVGltb3RoeSBIYXRjaGVyICA8dGltb3RoeUBhcHBsZS5j b20+CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogRXhwZWN0IHRvIHNlZSB0b3AgbGV2ZWwgSmF2 YVNjcmlwdCBmdW5jdGlvbiBwcm9maWxlciBkZXRhaWxzIGZvciBldmVudCBoYW5kbGVycyBpbiB0 aW1lbGluZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 MTMyMTA5CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAg KiBVc2VySW50ZXJmYWNlL1ZpZXdzL1NjcmlwdFRpbWVsaW5lVmlldy5qczoKKyAgICAgICAgKFdl Ykluc3BlY3Rvci5TY3JpcHRUaW1lbGluZVZpZXcucHJvdG90eXBlLl9wcm9jZXNzUGVuZGluZ1Jl Y29yZHMpOiBEb24ndCBwcm9tb3RlIHJvb3Qgbm9kZSBjaGlsZHJlbiBpZgorICAgICAgICB0aGVy ZSBpcyBvbmx5IG9uZSBjaGlsZC4KKwogMjAxNC0wOC0xMCAgVGltb3RoeSBIYXRjaGVyICA8dGlt b3RoeUBhcHBsZS5jb20+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogRGF0YUdyaWQgYW5kIGl0 cyBjbGllbnRzIGFyZSBhIGNsdXN0ZXItY3VzcyBvZiBzdHlsZXMKZGlmZiAtLWdpdCBhL1NvdXJj ZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NjcmlwdFRpbWVsaW5lVmlldy5q cyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NjcmlwdFRpbWVs aW5lVmlldy5qcwppbmRleCA1OWI0NDUxYmM1ZjMyNWYwYWNiOWZlZmI1NDMxM2RlNTRiNmEzMjU2 Li5mNTZkZDkzM2U3NDZkZmY4MmZlMzkzZjk3MjVlM2U3OTlkODM2YTczIDEwMDY0NAotLS0gYS9T b3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9TY3JpcHRUaW1lbGluZVZp ZXcuanMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvU2Ny aXB0VGltZWxpbmVWaWV3LmpzCkBAIC0yMDUsMTIgKzIwNSw2IEBAIFdlYkluc3BlY3Rvci5TY3Jp cHRUaW1lbGluZVZpZXcucHJvdG90eXBlID0gewogICAgICAgICAgICAgaWYgKHNjcmlwdFRpbWVs aW5lUmVjb3JkLnByb2ZpbGUpIHsKICAgICAgICAgICAgICAgICAvLyBGSVhNRTogU3VwcG9ydCB1 c2luZyB0aGUgYm90dG9tLXVwIHRyZWUgb25jZSBpdCBpcyBpbXBsZW1lbnRlZC4KICAgICAgICAg ICAgICAgICByb290Tm9kZXMgPSBzY3JpcHRUaW1lbGluZVJlY29yZC5wcm9maWxlLnRvcERvd25S b290Tm9kZXM7Ci0KLSAgICAgICAgICAgICAgICAvLyBJZiB0aGVyZSBpcyBvbmx5IG9uZSBub2Rl LCBwcm9tb3RlIGl0cyBjaGlsZHJlbi4gVGhlIFRpbWVsaW5lUmVjb3JkVHJlZUVsZW1lbnQgYWxy ZWFkeSByZWZsZWN0cyB0aGUgcm9vdAotICAgICAgICAgICAgICAgIC8vIG5vZGUgaW4gdGhpcyBj YXNlIChlLmcuIGEgIkxvYWQgRXZlbnQgRGlzcGF0Y2hlZCIgcmVjb3JkIHdpdGggYW4gIm9ubG9h ZCIgcm9vdCBwcm9maWxlIG5vZGUpLgotICAgICAgICAgICAgICAgIC8vIEZJWE1FOiBPbmx5IGRv IHRoaXMgZm9yIHRoZSB0b3AtZG93biBtb2RlLiBEb2luZyB0aGlzIGZvciBib3R0b20tdXAgd291 bGQgYmUgaW5jb3JyZWN0LgotICAgICAgICAgICAgICAgIGlmIChyb290Tm9kZXMubGVuZ3RoID09 PSAxKQotICAgICAgICAgICAgICAgICAgICByb290Tm9kZXMgPSByb290Tm9kZXNbMF0uY2hpbGRO b2RlczsKICAgICAgICAgICAgIH0KIAogICAgICAgICAgICAgdmFyIHplcm9UaW1lID0gdGhpcy56 ZXJvVGltZTsK