1583956 - Re-enable test_group_pointerevents.html for GeckoView

Status: RESOLVED FIXED

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

Description

[Botond Ballo [:botond]]

test_group_pointerevents.html was disabled for GeckoView in bug 1525959.

Now that we no longer run tests on Fennec, we should re-enable it so we don't lose test coverage.

Comment 1

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

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=32c4ac8c345452fd56797c232c61f698d40e969b

Comment 2

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

Retriggers on that try push are showing a couple of tests with intermittent failures, looks like pointer events aren't getting detected. I can run the mochitest locally (on emulator) and don't see the failure but I will dig into it a bit.

Comment 3

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

Logcat for helper_bug1285070.html shows that the missing events (pointerup, pointerout, pointerleave, touchend) are getting dispatched after the click. This seems like a bug in the product, not a bug in the test. The code here is supposed to prevent that from happening by delaying the click event until after the touchend (and related pointer events) are dispatched, but I guess for some reason that's not happening.

Comment 4

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

So the flow for the touchend events is:

1. Java UI thread (aka APZ controller thread) creates the events and sends through APZ
2. Java UI thread redispatches to gecko thread via nsAppShell::PostEvent here
3. Gecko thread sends it to content process and triggers listeners etc.

for the mouse/click events:

1. generated by processing of the touchend event on the Java UI thread (step 1 in the touchend flow above)
2. Java UI thread redispatches to Java UI thread via PostDelayedTask here so as to ensure that it goes to content after the touchend
3. (this is where the Java UI thread then sends the touchend to gecko thread, step 2 in the touchend flow above)
4. The Java UI thread dispatches to gecko main thread via NS_DispatchToMainThread here
5. Gecko thread sends it to content process and triggers listeners etc.

I think the problem is that the two mechanisms to go from the Java UI thread to the gecko thread are not the same (nsAppShell::PostEvent vs NS_DispatchToMainThread). And so sometimes the click events jump ahead of the touchend and most of the time they don't. Yay! In my local testing I always got the right result but I'll do a try push with logging to see if I can catch the misbehaviour in action to confirm.

Comment 5

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

Confirmed via try push logging: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=49bf4a01240894a720b0e3bf6757caeb4793557a

The logcat shows that the touch events are handled as expected on the Java UI thread, but the click event which is dispatched via NS_DispatchToMainThread actually runs before either the touchup or touchdown events posted to the main thread via nsAppShell::PostEvent.

Comment 6

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

With a fix: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=cac9502a8f1d10e481be120e6f0146ffcfbbf9e2
Also https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=2d040c0baf1ecac64f46b55de34e5c049a02000d

Comment 7

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

Try pushes are looking good, patches incoming.

Comment 8

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

Attachment: Bug 1583956 - Downgrade some APZC logging from debug to verbose. r?botond

Comment 9

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

Attachment: Bug 1583956 - Add additional APZ logging. r?botond

Depends on D64461

Comment 10

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

Attachment: Bug 1583956 - Dispatch tap events to Gecko on the same message loop that Android widget uses. r=snorp!,botond

Depends on D64462

Comment 11

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

Attachment: Bug 1583956 - Re-enable test_group_pointerevents on GeckoView. r?botond

Depends on D64463

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d0e2d6cfaeb
Downgrade some APZC logging from debug to verbose. r=botond
https://hg.mozilla.org/integration/autoland/rev/78dfbab51db6
Add additional APZ logging. r=botond
https://hg.mozilla.org/integration/autoland/rev/c8eadb4b6301
Dispatch tap events to Gecko on the same message loop that Android widget uses. r=snorp
https://hg.mozilla.org/integration/autoland/rev/f787e3e4a31c
Re-enable test_group_pointerevents on GeckoView. r=botond

Comment 13

[Dorel Luca [:dluca]]

Backed out 4 changesets (bug 1583956) for Build bustage in gfx/layers/apz/util/ChromeProcessController.cpp. CLOSED TREE

Log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=290748219&repo=autoland&lineNumber=48674

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=f787e3e4a31ca44b4ec281fe818a097a1b35a0d1

Backout:
https://hg.mozilla.org/integration/autoland/rev/859a66f9984802e66aedfc91bdd66179289a8d08

Comment 14

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

Thanks, should be an easy fix. Here's a try push to make sure: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=9adda25432cfe2a703f53366e7a6abe1a6a57814

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2dc80b6f0734
Downgrade some APZC logging from debug to verbose. r=botond
https://hg.mozilla.org/integration/autoland/rev/71b5445254cc
Add additional APZ logging. r=botond
https://hg.mozilla.org/integration/autoland/rev/b1a77364c6bd
Dispatch tap events to Gecko on the same message loop that Android widget uses. r=snorp
https://hg.mozilla.org/integration/autoland/rev/f143ac27a96f
Re-enable test_group_pointerevents on GeckoView. r=botond

Comment 16

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/2dc80b6f0734
https://hg.mozilla.org/mozilla-central/rev/71b5445254cc
https://hg.mozilla.org/mozilla-central/rev/b1a77364c6bd
https://hg.mozilla.org/mozilla-central/rev/f143ac27a96f