1062609 - Mochitests Are Unable to Read Realtime Scroll Position with APZ enabled

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[:kip (Kearwood Gilbert)]

Using nsDOMWindowUtils::AdvanceTimeAndRefresh is not effective in enabling framerate sensitive Mochitests to verify correct motion of APZ driven animation and scrolling.

TimeStamp::Now() is being used throughout AsyncPanZoomController.cpp (via GetFrameTime()) rather than respecting the time reported by the refresh driver while it is under test control.

Additionally, APZCTMController::RequestContentRepaint calls are not being followed by APZCTMController::RequestContentRepaintEvent::Run until after the refresh drive is released from test control as they are dependent on the main thread's message pump which may throttle events and/or be blocked by the javascript executing in the mochitest.

Functionality is required to enable mochitests to know with certainty that the main thread has received all frame metrics updates from the compositor before testing them and allowing the refresh driver to advance.

Comment 1

[:kip (Kearwood Gilbert)]

This is blocking the Mochitest added in Bug 1026023 from being enabled for APZ.  The smooth scrolling feature added in Bug 1026023 has been implemented for APZ in Bug 1022825.

Comment 2

[Botond Ballo [:botond]]

(In reply to :kip (Kearwood Gilbert) from comment #0)
> TimeStamp::Now() is being used throughout AsyncPanZoomController.cpp (via
> GetFrameTime()) rather than respecting the time reported by the refresh
> driver while it is under test control.

The APZ gtests use the static function AsyncPanZoomController::SetFrameTime() to override what GetFrameTime() returns [1]. Perhaps these tests could do the same?

[1] http://mxr.mozilla.org/mozilla-central/source/gfx/tests/gtest/TestAsyncPanZoomController.cpp?rev=9a507b307d1d#202

Comment 3

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

(In reply to :kip (Kearwood Gilbert) from comment #0)
> Using nsDOMWindowUtils::AdvanceTimeAndRefresh is not effective in enabling
> framerate sensitive Mochitests to verify correct motion of APZ driven
> animation and scrolling.

Yes, I think this is more or less by design. The APZ animation runs on the compositor thread and so is by design decoupled from main-thread control. It seems to me that a better approach might be to use the APZ test data framework (bug 961289 and friends). Botond can help you with that and/or augment the test data framework if needed to support your use cases.

Comment 4

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

This test is still broken with APZ enabled, and is basically not running because of the early-exit condition. I'll try and take a look at it.

Comment 5

[Botond Ballo [:botond]]

To clarify, is "this test" layout/generic/test/test_scroll_behavior.html?

Comment 6

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

Yeah, sorry I didn't realize this bug didn't actually refer to the test.

Comment 7

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

Replacing the waitForAllPaintsFlushed calls with waitForApzFlushedRepaints gets us most of the way there. It passes for me locally, and the try push just shows some intermittent failures on Linux. I'll try and debug that a bit.

Comment 8

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

Ah, when we start a smooth scroll animation the mLastSampleTime is initialized to TimeStamp::Now() here instead of using the test sample time in the CompositorBridgeParent. So in some cases the time delta of the animation's first sampling is shorter than what the test expects it to be.

Comment 9

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

Indeed, that's looking better: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=f1694e770a77c7f9efc381b614c3a56a9ce50b97

Comment 10

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

Attachment: Bug 1062609 - Ensure that APZCTreeManager::GetFrameTime respects the test sample time. r=botond!

Comment 11

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

Attachment: Bug 1062609 - Make scroll-behaviour test work with APZ enabled. r=botond!

The test itself is reasonable, except that it reads scroll positions
from the main thread. With APZ enabled we need to flush any pending APZ
repaints before reading these scroll positions. So changing the
waitForAllPaints to the APZ-flushing variant makes that happen, and the
test passes.

Depends on D29921

Comment 12

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

QR is green too: https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=d2910f9eec02c776e2d04fb8d5704e61a6bf881e

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dc4761caa7d4
Ensure that APZCTreeManager::GetFrameTime respects the test sample time. r=botond
https://hg.mozilla.org/integration/autoland/rev/c705a64ce83c
Make scroll-behaviour test work with APZ enabled. r=botond

Comment 14

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/dc4761caa7d4
https://hg.mozilla.org/mozilla-central/rev/c705a64ce83c