If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

dom/events/test/test_bug607464.html fails on M-e10s with APZ enabled

RESOLVED FIXED in Firefox 42

Status

()

Core
Panning and Zooming
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: kats, Assigned: kats)

Tracking

unspecified
mozilla42
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(e10s-, firefox42 fixed)

Details

Attachments

(1 attachment)

A recent try run with APZ enabled on Linux showed that this test was failing. I was able to reproduce locally by running the test in chaos mode, and grabbed an rr recording. The intention of the test is to synthesize a bunch of wheel events that should trigger "instant" scrolling despite smooth-scrolling being enabled. The test checks for this by using requestAnimationFrame callbacks to ensure the scrolling has taken effect by the desired deadline.

With APZ enabled, however, the wheel events get sent to the parent process and handled by APZ. The APZ does handle these wheel events "instantly" (i.e. going through the instant scroll codepath in AsyncPanZoomController) but the scroll offset in the child process doesn't get updated until the repaint requests make it back and get processed. This process is asynchronous relative to the requestAnimationFrame calls in the child process, and so this test is racy and fails intermittently.

The only thing I can think of to salvage this test is to change the way the check is done. Instead of using rAF, perhaps the test should listen for scroll events and make sure that each scroll event happens at a multiple of 3 pixels (i.e. none of the wheel events trigger a scroll of less than 3 pixels, which would happen if there were a smooth scroll animation). I think that should still exercise the thing the test is trying to exercise, but I'm not 100% sure.
Your suggestion sounds fine to me. Let's not make this too complicated, it's not a very valuable test.
Created attachment 8637710 [details] [diff] [review]
Patch

Thanks for the quick response! I updated the test to implement what I said and it seems to work as expected. I also increased the scroll amount from 3px to 30px just to make sure any partial scrolls would be more likely to get caught.
Assignee: nobody → bugmail.mozilla
Attachment #8637710 - Flags: review?(mstange)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Comment hidden (Treeherder Robot)
Attachment #8637710 - Flags: review?(mstange) → review+

Comment 7

2 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/cac48b8498ac
tracking-e10s: --- → -
https://hg.mozilla.org/mozilla-central/rev/cac48b8498ac
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox42: --- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
See Also: → bug 1208072
You need to log in before you can comment on or make changes to this bug.