Investigate Removing Gecko Hit Testing On Parent Process

RESOLVED INVALID

Status

()

Core
Event Handling
RESOLVED INVALID
4 years ago
4 years ago

People

(Reporter: mchang, Assigned: mchang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
This is probably crazy, but from bug 930939, we hit a perpetual regression. In the current master, touch events are processed by gecko on the main thread, then sent to APZ. With bug 930939, we reverse that where APZ processes touch events off main thread, then we dispatch the touch event to the main thread for gecko hit testing. This creates regressions. 

Currently, when a touch event is consumed by the parent process, so APZ never sees the touch event. With bug 930939, APZ sees the touch event, processes it, and the touch event is then consumed by gecko. APZ hit testing works most of the time, but corner cases will always come up. One example bug is 1053892.

We want to investigate removing this issue. First, we would like to send all touch events to APZ, then dispatch the touch event to JavaScript only on the parent process main thread. This is because the system app needs to listen to these touch events. Then APZ directly sends touch events to content bypassing Gecko hit testing on the parent process. I'm not sure this will work, but some initial questions kats and I had.

1) Does anything listen to touch events on the parent process other than the system app? I asked Fabrice, he said no. Kats could not think of anything either.

2) Can we send IPC messages off main thread in the parent process to the child process? - I asked Ben Turner, he said yes, but we should investigate looking at using PBackground instead of creating a new IPDL actor.

3) Can we move edge swipe detection from the system app or somehow do it in APZ / the compositor. 

If we can do this, we could delete lots of nsWindow::DispatchTouchEVent->nsView->nsViewManager->nsEventStateManager code paths that all touch events have to go through, greatly simplifying the touch dispatch code. Risky though as lots of things can break.
(Assignee)

Updated

4 years ago
Depends on: 1005815
Just to clarify, the way I was thinking about it is:

- input events come in on input thread
- input events get dispatched to APZ
- APZ figures out which process the event is targeting
- event gets sent to the appropriate process

Therefore events destined for content processes would not go through the root process event dispatching mechanism and vice-versa. We would still need to keep all that machinery because there will still be some events that use it, and other platforms still need it (also the same code machinery is used on parent and child processes). So unfortunately there won't be much in terms of actual code deletion.
Component: Graphics → DOM: Events

Updated

4 years ago
Component: DOM: Events → Event Handling

Comment 2

4 years ago
nsWindow::DispatchTouchEVent->nsView->nsViewManager->nsPresShell->nsEventStateManager
is the normal event dispatch, which Firefox and Mobile Firefox etc use, so deleting
that any time soon isn't possible.

Most of the touch events in b2g do already bypass the normal event dispatch machinery in
the parent process, since the event just go from widget to TabParent.
(Assignee)

Comment 3

4 years ago
(In reply to Olli Pettay [:smaug] from comment #2)
> nsWindow::DispatchTouchEVent->nsView->nsViewManager->nsPresShell-
> >nsEventStateManager
> is the normal event dispatch, which Firefox and Mobile Firefox etc use, so
> deleting
> that any time soon isn't possible.
> 
> Most of the touch events in b2g do already bypass the normal event dispatch
> machinery in
> the parent process, since the event just go from widget to TabParent.

Doh :(. Yes, most of the touch events after APZ knows that a scrollable layer exists bypass the machinery. However, the touch starts always go through nsWindow::DispatchTouchEvent and can get consumed, which causes the current problems if we switch the order. Can we bypass that machinery in all cases, at least on platforms with APZ?
(Assignee)

Comment 4

4 years ago
Created attachment 8473389 [details] [diff] [review]
Ignore Content Timeouts

I was thinking, I'm not sure if this will work, but it seems less risky than ignoring the gecko nsWindow::DispatchTouchEvent path. Currently, we have gonk->nsWindow->APZ -> Content. If the content doesn't respond with ContentReceivedTouch, we assume it is scrollable and start scrolling the content.

How about we invert that and never assume content is scrollable in a timeout? So then we can have gonk->APZ->nsWindow->Content. If content never responds, we clear out the touch events in APZ. Thus, if nsWindow decides to consume the event, the event is never sent to content, solving the rocket bar issue. When the timeout executes, we clear out the touch. If a new touch start occurs before the timeout, we already clear out old touches, so no problem there.

We have a few cases:

1) Normal case - Content responds in time and the timeout doesn't have to clear anything. Business as usual.
2) Touch is consumed by gecko - APZ processes it, sends a request to content. Content never sees the touch, so APZ::ContentTimeoutResponse fires and clears out the touch.
3) Touch is consumed by gecko, followed by another touch - APZ processes the touch, sends a request to content. Content never sees the touch, a new touch start occurs before ContentTimeoutResponse fires, we clear out the old touch event in APZ.
4) We create a tap - APZ creates a tap gesture, sees the touch start / move / ends, they are all consumed by gecko. Right now, we still send the gesture, we would have to change this to clear out gestures or only send out gestures AFTER AsyncPanZoomController::ContentReceivedTouch. This would solve the settings app problem, where we could scroll behind a menu pop up. Since gecko would consume this event, the ContentTimeoutResponse would fire and delete the touches and the gesture.

I have a small test that just resets APZ on ContentTimeoutResponse. It fixes the rocketbar issue and normal scrolling at the moment works. Would this work? What was the original reason for assuming content could scroll if we timeout? Thanks!
Flags: needinfo?(bugmail.mozilla)
Not sure if I understand correctly, but it sounds like you're suggesting that we don't scroll unless we hear back from stuff running on the main thread. That basically defeats the purpose of having APZ in the first place.
Flags: needinfo?(bugmail.mozilla)
(Assignee)

Comment 6

4 years ago
Resolving as the current plan moving forward is to get APZ to do proper hit testing on the parent process.
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.