1359502 - Should INPUT_EVENT_RESPONSE_MS and friends only run when aIsHandlingNativeEvent?

Status: REOPENED

(Core :: Widget, enhancement, P3)

Description

[Chris H-C :chutten|PTO (back Jan 5)]

From bug 1357457 comment 5, we should look into accumulating to INPUT_EVENT_RESPONSE_MS and related probes only when aIsHandlingNativeEvent.

:masayuki, could you expand on what aIsHandlingNativeEvent is and when it would be true/false (and any footguns therein)?

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

aIsHandlingNativeEvent is true when the event comes from widget. That means that the event represents a native event on the platform.

On the other hand, if aIsHandlingNativeEvent is false, it's synthesized event. For example, firing "click" event and "dblclick" event which are default action of "mousedown" and "mouseup" events. Another example is, firing "click" event when JS calls Element.click() or something similar API. So, the event is not related to user input if aIsHandlingNativeEvent is false.

Comment 2

[Chris H-C :chutten|PTO (back Jan 5)]

For what it's worth, JS calling Element.click() does not go all the way up to PresShell. It stays within the bounds of DOM dispatch (according to :smaug via email).

Synthesized events are interesting. I think we almost certainly want to measure the processing time of synthesized events like "click" (as they're the ones more likely to have handlers) in addition to native events like "mousedown" (as they're created by the OS and have to make their way to us).

I think we're going to have to continue recording both native and synthesized input events for these measurements to be meaningful.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Really? "click" event and "dblclick" event are fired during handling "mouseup" event. So, "mouseup" event's handling time may include "dblclick" event and/or "click" event time too but these nested events' time are also recorded separately.

And the Element.click()'s case is right. Element::DispatchClickEvent() is always called with aFullDispatch == false.
# I wonder, we can remove the argument.

Comment 4

[Chris H-C :chutten|PTO (back Jan 5)]

(In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> Really? "click" event and "dblclick" event are fired during handling
> "mouseup" event. So, "mouseup" event's handling time may include "dblclick"
> event and/or "click" event time too but these nested events' time are also
> recorded separately.
> 
> And the Element.click()'s case is right. Element::DispatchClickEvent() is
> always called with aFullDispatch == false.
> # I wonder, we can remove the argument.

They're nested? Now that is interesting. So

mousedown
mouseup
click

is more like

mousedown_start
mousedown_end
mouseup_start
click_start
click_end
mouseup_end

So for a given click interaction telemetry will be seeing three and only needs two? Are there cases when the synthesized events are not handled within the bounds of the triggering native event?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

(In reply to Chris H-C :chutten from comment #4)
> (In reply to Masayuki Nakano [:masayuki] (JST, +0900) from comment #3)
> > Really? "click" event and "dblclick" event are fired during handling
> > "mouseup" event. So, "mouseup" event's handling time may include "dblclick"
> > event and/or "click" event time too but these nested events' time are also
> > recorded separately.
> > 
> > And the Element.click()'s case is right. Element::DispatchClickEvent() is
> > always called with aFullDispatch == false.
> > # I wonder, we can remove the argument.
> 
> They're nested? Now that is interesting. So
> 
> mousedown
> mouseup
> click
> 
> is more like
> 
> mousedown_start
> mousedown_end
> mouseup_start
> click_start
> click_end
> mouseup_end

Yes, PresShell::HandleEventInternal() calls EventStateManager::PostHandleEvent(). PosthandleEvent() calls CheckForAndDispatchClick(). Then, it dispatches "click", "dblclick" and/or "auxclick" event(s) via PresShell::HandleEventWithTarget(). HandleEventWithTarget() calls HandeEventInternal() with aIsHandlingNativeEvent == false.

> Are there cases when the synthesized events are not handled
> within the bounds of the triggering native event?

If you meant that there are some cases which hasn't caused from native input event, it's possible.

PresShell::HandleEventWithTarget() is called by:
http://searchfox.org/mozilla-central/search?q=symbol:_ZN7mozilla9PresShell21HandleEventWithTargetEPNS_11WidgetEventEP8nsIFrameP10nsIContentP13nsEventStatus%2C_ZN12nsIPresShell21HandleEventWithTargetEPN7mozilla11WidgetEventEP8nsIFrameP10nsIContentP13nsEventStatus&redirect=false

nsTextEditorState uses it for dispatching "select" event.
http://searchfox.org/mozilla-central/rev/876c7dd30586f9c6f9c99ef7444f2d73c7acfe7c/dom/html/nsTextEditorState.cpp#849-851,863,865
SELECTALL_REASON can be triggered with JS. E.g., Selection.selectAllChildren().

EditorBase uses it for dispatching "input" event.
http://searchfox.org/mozilla-central/rev/876c7dd30586f9c6f9c99ef7444f2d73c7acfe7c/editor/libeditor/EditorBase.cpp#1828,1840,1861,1866
"input" event may be caused by JS. E.g., document.execCommand("undo", false).

Comment 6

[Chris H-C :chutten|PTO (back Jan 5)]

I was more specifically worried about cases that are caused by native input events, but don't happen within the native event's handler. For instance, it looks like EditorBase's "input" event is often caused by native input events but can (in the case that there's a script blocker) run completely asynchronously.

However, there is already an implicit assumption in the probes that asynchronous handling is unable to be captured. If using aIsHandlingNativeEvent will keep us from double-counting things (like mouseup+click), then I think it's a worthy change.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information