1177651 - test_moz_mouse_pixel_scroll_event.html fails with APZ enabled

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

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

test_moz_mouse_pixel_scroll_event.html (in m-e10s-1) fails with APZ enabled on Linux. I was able to reproduce locally and debug using rr. The first problem turned out to be that the MozMousePixelScroll event that the test waits for becomes asynchronous with APZ enabled, and so the test needs to be updated to yield after synthesizing the wheel event.

Even with that fixed though there appears to be a race somewhere (which I *can't* reproduce locally) that causes some of the test assertions to fail on try.

Comment 1

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

Attachment: Patch

This fixes the issue I identified above, but the test isn't fully green yet.

Comment 2

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

Attachment: Part 1 - Yield after synthesizing wheel events

Comment 3

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

Attachment: Part 2 - MozMousePixelScroll events are APZ-aware too

Doing a preventDefault on a MozMousePixelScroll event is supposed to cancel the scrolling, so APZ needs to wait for the response, and therefore layout needs to put areas covered by these events in the dispatch-to-content region.

Comment 4

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

Attachment: Part 3 - Extend content response timeout to deal with the test running slowly in automation

We may or may not need this also. It was one of my earlier attempts at fixing this problem (before I figured out part 2 and the dependent bug).

I did a try push without this patch at https://treeherder.mozilla.org/#/jobs?repo=try&revision=5b0b2ea9d345, if that's green then we shouldn't need this.

Comment 5

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6d2c22105132 is better, with less logging.

Comment 6

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

The test is still failing but with a lower frequency it appears. I did another push with the part 3 patch at https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6f1df7f8cc5, but I also discovered another potential problem - the AddEventListener call in the test doesn't trigger a new layers transaction, so the event regions on the APZ side are stale. We might need to trigger a layout flush of some sort if an APZ-aware listener is added or removed.

Comment 7

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

Attachment: Part 1 - Yield after synthesizing wheel events

Comment 8

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

Attachment: Part 2 - MozMousePixelScroll events are APZ-aware too

Comment 9

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

Attachment: Part 3 - Extend the content response timeout

Comment 10

[David Anderson [:dvander] - inactive, e-mail if emergency]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #6)
> The test is still failing but with a lower frequency it appears. I did
> another push with the part 3 patch at
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=a6f1df7f8cc5, but I
> also discovered another potential problem - the AddEventListener call in the
> test doesn't trigger a new layers transaction, so the event regions on the
> APZ side are stale. We might need to trigger a layout flush of some sort if
> an APZ-aware listener is added or removed.

Yuck. That could explain why other tests are intermittently failing.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bf2dc08fe83a
https://hg.mozilla.org/integration/mozilla-inbound/rev/55a2ebc33984
https://hg.mozilla.org/integration/mozilla-inbound/rev/e8ea2a9493da

Comment 12

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

(In reply to David Anderson [:dvander] from comment #10)
> Yuck. That could explain why other tests are intermittently failing.

Possibly, yeah. I filed bug 1178743 for it.

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/bf2dc08fe83a
https://hg.mozilla.org/mozilla-central/rev/55a2ebc33984
https://hg.mozilla.org/mozilla-central/rev/e8ea2a9493da