1492395 - Make event markers in profiler timeline per event, not per event listener

Status: RESOLVED FIXED

(Core :: DOM: Events, defect, P2)

Description

[Olli Pettay [:smaug][[e-mail]]]

Attachment: dispatcher_prof.diff

Currently it is easy to interpret the profiler timeline marker so that "DOMEvent" is one event, when it is actually one event listener and there can be several listeners for an event.
I think per event marker makes more sense.


remote: View your change here:
remote:   https://hg.mozilla.org/try/rev/1d7bcfd647ce414dea148d309156b22d5fce7472
remote: 
remote: Follow the progress of your build on Treeherder:
remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=1d7bcfd647ce414dea148d309156b22d5fce7472
remote: recorded changegroup in replication log in 0.015s

Comment 1

[Markus Stange [:mstange]]

Comment on attachment 9010172 [details] [diff] [review]
dispatcher_prof.diff

Review of attachment 9010172 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks! This will be a lot less confusing.

::: dom/events/EventDispatcher.cpp
@@ +1127,5 @@
> +            RefPtr<Event> event = EventDispatcher::CreateEvent(et, aPresContext,
> +                                                               aEvent,
> +                                                               EmptyString());
> +            event.swap(postVisitor.mDOMEvent);
> +          }

I'm a bit surprised about this if { ... } block, and about the fact that it's only executed in the profiler branch. What is its purpose? Is it just so that we can call postVisitor.mDOMEvent->GetType(typeStr)? Is it doing something earlier that usually would be delayed until EventListenerManager::HandleEventInternal, because this is the least-overhead way of getting the typeStr in that case?

Comment 2

[Olli Pettay [:smaug][[e-mail]]]

We create DOM events on demand from widget level events. And yes, we need that for the type string.
This is just the easiest way to get the type. Normally C++ code doesn't need to deal with strings as types, but JS does.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29a00d4e81c1
Make event markers in profiler timeline per event, not per event listener, r=mstange

Comment 4

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/29a00d4e81c1