1524232 - Dispatch synthetic mouse moves via APZ

Status: RESOLVED FIXED

(Core :: Web Painting, enhancement, P3)

Description

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

Dispatch synthetic mouse moves (from bug 655267) through APZ so that the events get a LayersId and matrix attached to them by the code from bug 1524229.

Comment 1

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

This shouldn't wait for bug 1524229, since bug 1528725 took priority.

In the present architecture, it seems a bit weird to me that the synthesis code is in PresShell instead of being closer nsBaseWidget.

Comment 2

[Kartikaya Gupta (email:[e-mail])]

I was looking at the code involved in this event dispatch. From comment 0 it sounds like we want to route this input event to the compositor for APZ to stamp it, but I think that's actually unnecessary. The synthetic event is created in a content process and dispatched within that process itself, because we construct it already knowing exactly where we want it to go.

In the common case that destination would be within the same content document that is doing the dispatching, so we could just stamp with the layers id for the "current document" but I think there might be some cases where we end up dispatching to an iframe that is a child of the current document. But still, we should be able to figure that out based on the view being targeted and then get the layers id for that without having to go to APZ.

Comment 3

[Kartikaya Gupta (email:[e-mail])]

I guess there's the additional problem that this code dispatches directly to the presShell, which means it doesn't actually go through any cross-process codepaths. So really it's been broken since e10s happened. And that's ignoring the fact that the view bounds it uses for hit-testing doesn't include CSS transforms, so if you have a transformed iframe the synthesized mouse event might end up getting dispatched to the wrong view/presshell.

This latter problem has been a bug since the code was written, I think, so I'm not going to address it here. For now I'm just planning on sending the mouse event over PBrowserBridge if it's intended for an OOP iframe so that it gets dispatched in the correct process.

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1524232 - Add a mechanism for dispatching synthesized mousemove events to an OOP iframe. r?hsivonen

Comment 5

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1524232 - Dispatch synthesized mousemoves to OOP iframes if that's where they land. r?tnikkel

Depends on D29731

Comment 6

[Timothy Nikkel (:tnikkel)]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #3)

I guess there's the additional problem that this code dispatches directly to
the presShell, which means it doesn't actually go through any cross-process
codepaths. So really it's been broken since e10s happened.

Why is that a problem?

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Aren't there scenarios where say the parent process does a reflow, the position of the content shifts, and then the synthesized mouse event is dispatched from the parent process? In this scenario the mouse event would not get dispatched to the content because it's OOP and so the last-known mouse location in the content process would be wrong.

Comment 8

[Kartikaya Gupta (email:[e-mail])]

Also here's the try push for these patches: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=aa30fb1c6583a2af3b5dd0b7ca0664c7e2e2cb81

Comment 9

[Henri Sivonen (:hsivonen) (temporarily away from Bugzilla)]

The synthetic event is created in a content process and dispatched within that process itself, because we construct it already knowing exactly where we want it to go.

This is surprising. I'd have expected APZ to be the source of truth regarding the current global mouse position.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

(In reply to Henri Sivonen (:hsivonen) from comment #9)

This is surprising. I'd have expected APZ to be the source of truth regarding the current global mouse position.

The source of truth would be the widget code rather than APZ, and yeah, we could probably ask the widget to synthesize a mouse event based on that source of truth. However in thinking about this I came up with a question: if a mouse is over an iframe A, and then teleports to be over iframe B, is A supposed to get a "mouseout" type of event? Or is it ok that the last event delivered to A was a mousemove? It seems to me that with the current code, in this scenario, we'd deliver the mousemove directly to iframe B after the teleport, and A would never get notified that the mouse left. IIRC there's some process-global state in EventStateManager that records the element the mouse is currently over, so if A and B are in the same process then A would automagically get the mouseout event when B got the mouseover/mousemove event, but if A and B are in different processes then that won't happen. I could be misremembering though.

Comment 11

[Kartikaya Gupta (email:[e-mail])]

At any rate I'd like to land these patches for now, and consider dispatching this event from the widget code in another bug, as it's a larger change that could subtly impact other scenarios, would also solve bug 1548560. Perhaps writing some mochitests to enforce the desired behaviour would also be good.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/676cf3a452c6
Add a mechanism for dispatching synthesized mousemove events to an OOP iframe. r=hsivonen
https://hg.mozilla.org/integration/autoland/rev/b3417a0b019f
Dispatch synthesized mousemoves to OOP iframes if that's where they land. r=tnikkel

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1549355 for that.

Comment 14

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/676cf3a452c6
https://hg.mozilla.org/mozilla-central/rev/b3417a0b019f