1053892 - Cannot Close Rocketbar w/ Touch Events off Main Thread

Status: RESOLVED WORKSFORME

(Core Graveyard :: Widget: Gonk, defect, P1)

Description

[Mason Chang [Inactive] [:mchang]]

With bug 930939, when rocketbar is loaded and I push the close button, rocketbar goes out of view then reloads. This does not happen on master. Investigate and fix this issue.

Comment 1

[Mason Chang [Inactive] [:mchang]]

Attachment: Layer Tree on Homescreen w/ Rocketbar Open

Comment 2

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

Were you able to investigate this issue to figure out exactly what was going on? I think the flow we discussed before may have been wrong.

Comment 3

[Mason Chang [Inactive] [:mchang]]

I haven't followed it all the way through to gaia yet. I'll get to it today or tomorrow.

Comment 4

[Mason Chang [Inactive] [:mchang]]

So I think I tracked it down. We have a couple of things here. The system app, the vertical homescreen app, rocketbar, click events generated in gonk, and click events generated by APZ gestures.

In master, without bug 930939, consider the case on the vertical homescreen app and we tap the rocketbar search bar to open the rocketbar. The search bar listens for a 'click' event. Since the vertical homescreen uses APZ, APZ gesture detection generates a single tap and generates a series of mouse events in the TabChild (http://dxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp?from=TabChild.cpp&case=true#1828). The mouse events are generated into a click which is dispatched in the EventStateManager (http://dxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#4361). The Vertical Homescreen app listens to the click event, and then opens the rocketbar.

In master, the rocketbar listens to 'click' events to close the rocketbar in the system app. This is accomplished by pushing the close button or tapping an area of the screen between the keyboard and rocketbar. The touch events are sent through Gecko and consumed in the parent process in the system app, thus APZ never sees these touch events. Since APZ never sees the touch events, none are captured. Thus, all touch down, move, and ends, are not captured, and we send a mouse event to accompany each touch event at the gonk layer. The mouse events are translated as a click event, and we dispatch a click event to the system app. The system app dispatches the 'click' event to the rocketbar, which closes the rocketbar and reloads the vertical homescreen.

With bug 930939, since we already open the rocketbar with a single tap gesture in APZ in master, nothing changes in this case. The system process isn't listening for any click events during the normal vertical homescreen display and the rocketbar opens. However, once the rocketbar is open and when we tap close, two things happen. First, we generate the mouse events in gonk to dispatch a click event to the system app, which closes the rocketbar. Second, since APZ also saw the touches, the child receives the single tap gesture, which also becomes a click event in the homescreen app. The homescreen only gets the tap gesture, none of the touch events.

We actually have a race condition as a click event is dispatched to both the system app and the homescreen app. The system app listens to the click event generated in gonk to close the rocketbar and the homescreen app listens to the click event generated by APZ gesture detection to open the rocketbar. In practice, the system app usually wins, but sometimes it is possible to close the rocket bar if the homescreen app processed the click event prior to the system app getting the click event. Since we are dispatching two click events, we can close then reopen the rocketbar.

Comment 5

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

Ok, that makes sense. So I think the correct solution here is actually to notify the APZ that the events were consumed in the root process. In current master, the touch events never even get to the APZ. With bug 930939 what should happen is that the events go to the APZ, but get queued pending for touch listeners. Right now the ContentReceivedTouch call only happens from the child process, but we will need to add a call to that in the gonk code as well, to handle the case where the root process consumes the event. So the code in gonk would look something like this:

input thread:
  - dispatch MultiTouchInput to APZ
  - hand off MultiTouchInput and untransformed input to gecko thread
Gecko thread:
  - create WidgetAPZTouchEvent and dispatch into gecko
  - check response to see if event was consumed in root process, and call ContentReceivedTouch(false) on the APZ if it was

This will then cause the APZ to throw out those touch events sitting in the queue and so the click event will never get dispatched to the child process.

There is one other hiccup with this, which is that in order to catch all the cases we'll need to enable APZ in the root process, so that the right APZC instance gets the touch events. Part of the problem now is that the child process' APZC is getting the touch events when in fact it should be the root process APZC, because that's what is visible on-screen. The incorrect hit testing is a result of not having APZ enabled in the root process.

Doing the ContentReceivedTouch without the root-process APZ might still work, but won't always. What can happen is that if the child process has no touch listeners, the APZ won't even wait for the content response and so the ContentReceivedTouch will be too late. If the child process does have touch listeners then the APZ will wait and everything will be fine. But we shouldn't really be relying on that here, so we should try to enable root process APZ at the same time.

Comment 6

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

The new patches on bug 930939 don't have this problem.