1959553 - Investigate underlying cause of intermittent 1px scroll offset in helper_click_interrupt_animation.html

Status: RESOLVED FIXED

(Core :: Panning and Zooming, task)

Description

[Julian Gaibler]

After landing bug 1955113, which disabled browser.theme.native-theme, we observed an increased rate of intermittent failures in helper_click_interrupt_animation.html, specifically:

Scroll position hasn't changed again - got 548, expected 549

An initial theory is that the 1px scroll position difference might be related to toolbar height changes triggered by the flipped pref, or potentially fractional scroll position rounding (like on 133% DPI).

Adding a short delay seems to mitigate the failure, but it's unclear why. (bug 1959374)

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Thank you, Julian for filing this bug.

Bug 1674687 is relevant, because of the bug scrollTop is rounded. But it doesn't mean it's the underlying issue. The underlying problem is, I think, the scroll position is unstable there, it looks like wobbling for a while.

Comment 2

[Julian Gaibler]

Looks like bug 1959374 did not fix the increase in failures :(

Comment 3

[Botond Ballo [:botond]]

I triggered the failure in Try with some logging enabled. An interesting log fragment is:

D/apz.controller 180509c00 processing scroll update { gen=20, type=PureRelative, mode=Smooth, origin=Pages, dst=(0,0), src=(0,0), delta=(0,832), triggered by script=0 }
D/apz.controller 180509c00 pure-relative smooth scrolling from (0,0) by (0,832)
D/apz.controller 180509c00(root scrollId=15): running CancelAnimation(0x0) in state NOTHING
D/apz.controller 180509c00(root scrollId=15): changing from state NOTHING to NOTHING
D/apz.controller 180509c00(root scrollId=15): changing from state NOTHING to SMOOTH_SCROLL
// XXX
D/apz.controller 16abaa700 flushing repaint for new input block
D/apz.controller 175193700 flushing repaint for new input block
D/apz.controller 180509c00 flushing repaint for new input block
D/apz.controller 180509c00(root scrollId=15): running CancelAnimation(0x3) in state SMOOTH_SCROLL
D/apz.controller 180509c00(root scrollId=15): changing from state SMOOTH_SCROLL to NOTHING
D/apz.controller 180509c00 scroll snapping near (0,548.5)
// YYY

At point "XXX", the main thread receives APZ scroll updates with the y offset increasing from y=0 to y=549.

At point "YYY", the main thread receives an APZ scroll update from y=549 to to y=548. I guess it's caused by the "scroll snapping near (0,548.5)".

Comment 4

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #3)

At point "YYY", the main thread receives an APZ scroll update from y=549 to to y=548. I guess it's caused by the "scroll snapping near (0,548.5)".

Further investigation suggests the scroll update is not actually caused by scroll snapping. The code that prints the "scroll snapping near" log message only changes the scroll offset itself if this branch is taken, which is not the case here.

However, the value printed in the log message, 548.5, provides a clue as to what's going on. It turns out, the scroll offset is not changing in the compositor at all: it's 548.5 both at the time of the main thread scroll update of y=548, and at the time of the main thread scroll update of y=549. I verified that the value that APZ sends with the repaint request is 548.5 both times.

The reason this results in a main thread update of y=548 one time and y=549 another time is that it gets aligned differently by the layer pixel alignment code.

I haven't dug into the details of why the scroll offset gets aligned differently; but since we're looking to disable pixel alignment by default anyways (bug 1946610), I think it would make sense to do so for this test. I've confirmed that this makes the test pass reliably.

Comment 5

[Botond Ballo [:botond]]

Attachment: Bug 1959553 - Run helper_click_interrupt_animation with pixel alignment disabled. r=hiro

This makes the subtest pass reliably on Mac, so re-enable it there.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bbbadc2bffa
Run helper_click_interrupt_animation with pixel alignment disabled. r=hiro

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/0bbbadc2bffa