965351 - Raciness in scroll events with apz

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect)

Description

[[:fabrice] Fabrice Desré]

See https://bugzilla.mozilla.org/show_bug.cgi?id=950266#c49 :

> We think that turning on Nuwa in the emulator tests might have revealed some
> potential timing issues with APZ:
> 
> - (group 3) ERROR TEST-UNEXPECTED-FAIL |
> /tests/docshell/test/navigation/test_bug430723.html | Test timed out.
> 
> We found that the onscroll callback in the test case can only be fired when
> ScrollFrameHelper::AsyncScroll::WillRefresh() is called, that is, it is
> triggered by the refresh driver. If in
> ScrollFrameHelper::ScrollToWithOrigin(), mAsyncScroll haven't received
> callback from nsRefreshDriver, then the onscroll callback will never be
> fired. The test case will never finish until marionette times out.
> 
> We have a workaround of this problem that we call
> ScrollFrameHelper::AsyncScroll::WillRefresh() when it still waits for the
> refresh driver. Then this failure is gone and we will encounter the
> unexpected scroll offset like
> 
> - ERROR TEST-UNEXPECTED-FAIL | /tests/dom/events/test/test_bug946632.html |
> <input> horizontal line scroll container.scrollTop - got 20, expected 0
> 
> There are other unexpected failures like:
> 
> - ERROR TEST-UNEXPECTED-FAIL | /tests/dom/events/test/test_bug946632.html |
> <input> horizontal line scroll targets <input> - got [object
> HTMLDivElement], expected [object HTMLInputElement]

Comment 1

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

(In reply to Fabrice Desré [:fabrice] from comment #0)
> > - (group 3) ERROR TEST-UNEXPECTED-FAIL |
> > /tests/docshell/test/navigation/test_bug430723.html | Test timed out.
> > 
> > We found that the onscroll callback in the test case can only be fired when
> > ScrollFrameHelper::AsyncScroll::WillRefresh() is called, that is, it is
> > triggered by the refresh driver. If in
> > ScrollFrameHelper::ScrollToWithOrigin(), mAsyncScroll haven't received
> > callback from nsRefreshDriver, then the onscroll callback will never be
> > fired. The test case will never finish until marionette times out.

This one might be fixed by bug 961280 which just landed.

Comment 2

[[:fabrice] Fabrice Desré]

Unfortunately not, bug 961280 was merged on b2g-inbound and we're still failing.

Comment 3

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

I'm just gonna dupe this over.

Comment 4

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

Un-duping because fabrice already landed the patch to disable tests. I'll use this bug to re-enable the tests if bug 963278 sticks.

Comment 5

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

Attachment: Reenable tests

Try run is at https://tbpl.mozilla.org/?tree=Try&rev=e4f71f65253c. I accidentally ran everything but only the b2g runs actually matter here, and they're green.

Comment 6

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

https://hg.mozilla.org/integration/b2g-inbound/rev/96811be99c9e

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/96811be99c9e