1886300 - Android mouse event handling skips wheel transaction handling and horizontalization

Status: NEW

(Core :: Panning and Zooming, defect, P3)

Description

[Botond Ballo [:botond]]

While doing code reading during the review of bug 1757352, I noticed an issue with the code for mouse event handling on Android.

APZInputBridge::ReceiveInputEvent has two overloads: one which takes an InputData and another which takes a WidgetInputEvent.

The WidgetInputEvent overload forwards to the InputData one, but it also does some extra processing:

• wheel transaction handling for scroll-wheel and other mouse events (e.g. having a mouse-move interrupt a wheel transaction)
• horizontalization
• a few other minor things such as handling some granular smooth-scroll prefs and handling user delta multipliers on scroll-wheel events

Desktop platforms all call the WidgetInputEvent overload, and so this extra processing happens there.

Android, however, calls the InputData overload directly, so this extra processing is bypassed.

Comment 1

[Botond Ballo [:botond]]

An additional note here is that the WidgetInputEvent overload does some filtering of mouse events and wheel events where, if it determines that APZ does not need to process an event at all, it skips calling the InputData overload altogether.

Note that, when using the GPU process, it's the implementation of the InputData overload which sends the event to the GPU process and waits for a response, so this filtering optimization has the non-trivial benefit of avoiding a round-trip to the GPU process for certain events.

By calling the InputData overload directly, Android does not benefit from this optimization.

Comment 2

[Botond Ballo [:botond]]

A few considerations to keep in mind when designing a fix for this:

• Four configurations need to be supported
  1. Desktop with no GPU process
  2. Desktop with GPU process
  3. Android with no GPU process
  4. Android with GPU process
• WidgetInputEvent can only be used on the main thread (this is the reason InputData exists in the first place)
• ReceiveInputEvent (either overload) must be called on the APZ controller thread
  • The APZ controller thread is the main thread on Desktop, but not on Android
  • In "GPU process" scenarios, we do have a notion of "APZ controller thread" both in the parent process and the GPU process
    • But it's unclear whether this constraint (that ReceiveInputEvent must be called on the controller thread) is important to observe in the parent process if APZ actually lives in the GPU process, or whether this could be relaxed

Comment 3

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #2)

it's unclear whether this constraint (that ReceiveInputEvent must be called on the controller thread) is important to observe in the parent process if APZ actually lives in the GPU process, or whether this could be relaxed

Thinking some more about this:

• The constraint is important in both the case where APZ lives in-process and the case where APZ lives out-of-process, though for different reasons.
  • If APZ lives in-process, the implementation of ReceiveInputEvent will access state that can only be accessed from the controller thread (it's not protected against concurrent access from other threads).
  • If APZ lives out-of-process, the IPC actor used to forward the events to the GPU process (PAPZInputBridgeChild) is tied to a particular thread, and it's important for event ordering purposes (and maybe concurrency reasons as well) that messages be sent from that thread.
• Moreover, relaxing this constraint in the out-of-process case would not actually be helpful for resolving this bug: while it might mean that Android code could use the WidgetInputEvent overload in the out-of-process case, we still need a solution in the in-process case.

Based on the above, I think the solution will have to involve implementing the wheel transaction etc. handling for InputData. (And then, to avoid code duplication, we should probably use that implementation in all cases, so that the WidgetInputEvent overload just converts the event to InputData and immediately calls the InputData overload which handles the wheel transaction etc. stuff.)

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #3)

Based on the above, I think the solution will have to involve implementing the wheel transaction etc. handling for InputData. (And then, to avoid code duplication, we should probably use that implementation in all cases, so that the WidgetInputEvent overload just converts the event to InputData and immediately calls the InputData overload which handles the wheel transaction etc. stuff.)

Of course, we still want to keep the optimization where events that don't need to be forwarded to the GPU process aren't, so we'll need to separate out (1) the operation on InputData that's pre-filtering and handles the wheel transaction etc. stuff, and (2) the operation that's post-filtering and involves actually forwarding the event to the GPU process.

Comment 5

[Botond Ballo [:botond]]

This is likely to also block the WebDriver Parent Process Events work.