2023721 - test_scroll_behavior.html fails standalone due to missing smooth scroll animation initialization tick

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Florian Quèze [:florian]]

test_scroll_behavior.html fails when run standalone but passes when run as part of the full layout/generic/test/ folder.

The root cause is that the MSD (Mass-Spring-Damper) smooth scroll animation needs at least two refresh driver ticks to produce position changes — the first tick initializes the animation state, and subsequent ticks advance it. When running after other tests in the folder, the refresh driver has already been cycling through advanceTimeAndRefresh/restoreNormalRefresh from preceding tests (particularly test_scroll_animation_restore.html), so the animation initialization happens implicitly. When running standalone, the first advanceTimeAndRefresh(100) after requesting a smooth scroll is only a single tick, and the scroll position doesn't move.

The fix adds an extra advanceTimeAndRefresh(0) tick after initiating smooth scrolls in two places:

1. In testScrollBehaviorInterruption(), after calling window.scrollTo({behavior: 'smooth'}), tick once before the 100ms advance.
2. In sampleAnimation(), after initiating the smooth scroll, tick once before the sampling loop begins.

Open question: is the fact that the first refresh driver tick after a smooth scroll request doesn't advance the animation actually a bug in the scroll animation implementation itself? If so, the right fix might be in the engine rather than the test. But it might also be a test-only thing caused by artificial time changes.

Comment 1

[Florian Quèze [:florian]]

Attachment: Bug 2023721 - Fix test_scroll_behavior.html to pass standalone. r=hiro

Comment 2

[Hiroyuki Ikezoe (:hiro)]

(In reply to Florian Quèze [:florian] from comment #0)

Open question: is the fact that the first refresh driver tick after a smooth scroll request doesn't advance the animation actually a bug in the scroll animation implementation itself? If so, the right fix might be in the engine rather than the test. But it might also be a test-only thing caused by artificial time changes.

I don't think it's a bug. Also I don't think adding the extra advanceTimeAndRefresh(0) is the right approach to fix the failure. The MSD scroll animation gets sampled on the sampler thread, so that when the test fails, the sampler thread hasn't yet sampled the animation, I think. IIRC advanceTimeAndRefresh doesn't ensure that the sampler thread samples the animations. Also the extra advanceTimeAndRefesh with 0 implies it since with 0 argument advanceTimeAndRefresh doesn't change the animation's time at all.

So, I would say that the right approach to fix the failure is to make sure the sampler thread samples the animation (unless the analysis is incorrect).

Comment 3

[Hiroyuki Ikezoe (:hiro)]

Note that even with applying D288069, the test sill fails on my Linux/Wayland machine, FWIW.

Comment 4

[Florian Quèze [:florian]]

Thanks for the feedback! Would calling advanceTimeAndRefesh with a non-0 value fix it for you?
Eg doing this change on top of the patch on phabricator:

diff --git a/layout/generic/test/test_scroll_behavior.html b/layout/generic/test/test_scroll_behavior.html
index 9566c00a5a95..1cf4993e20ca 100644
--- a/layout/generic/test/test_scroll_behavior.html
+++ b/layout/generic/test/test_scroll_behavior.html
@@ -56,12 +56,10 @@
     ok(window.scrollX == 15 && window.scrollY == 16,
       "smooth scroll-behavior must be asynchronous");
 
-    // Tick once to let the smooth scroll animation initialize, then advance
-    // the real test interval. The MSD simulation needs an initial tick to
-    // establish state before it can produce position changes.
-    SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(0);
+    // Two composites are needed: the first sends the smooth scroll request
+    // to APZ and creates the animation, the second samples it.
+    SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(100);
     await promiseApzFlushedRepaints();
-
     SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(100);
     await promiseApzFlushedRepaints();
 
@@ -220,11 +218,12 @@
     window.scrollTo(startX, startY);
     window.scrollTo({left: endX, top: endY, behavior: 'smooth'});
 
-    // Tick once to let the smooth scroll animation initialize.
-    SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(0);
+    // Advance one frame to send the smooth scroll request to APZ and
+    // create the animation, so subsequent frames can sample it.
+    SpecialPowers.DOMWindowUtils.advanceTimeAndRefresh(timeStep);
     await promiseApzFlushedRepaints();
 
-    var currentTime = 0; // ms
+    var currentTime = timeStep; // ms
 
     while (currentTime < maxTime && frameCountAtDestination < 15) {
       positionSamples.push([window.scrollX, window.scrollY]);

If that doesn't fix it on your machine, can you give a hint about what APIs to use to "make sure the sampler thread samples the animation"? (Also, feel free to take over the bug if it's easier to just do it than to explain).

Comment 5

[Hiroyuki Ikezoe (:hiro)]

The additional change doesn't fix the failure at all.

Error Summary
-------------
layout/generic/test/test_scroll_behavior.html
  FAIL layout/generic/test/test_scroll_behavior.html - instant scroll-behavior must be synchronous when setting initial position
    SimpleTest.ok@SimpleTest/SimpleTest.js:427:16
    testScrollBehaviorInterruption@layout/generic/test/test_scroll_behavior.html:48:7
    async*@layout/generic/test/test_scroll_behavior.html:36:11
    EventListener.handleEvent*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - instant scroll-behavior must be synchronous when setting new position
    SimpleTest.ok@SimpleTest/SimpleTest.js:427:16
    testScrollBehaviorInterruption@layout/generic/test/test_scroll_behavior.html:52:7
    async*@layout/generic/test/test_scroll_behavior.html:36:11
    EventListener.handleEvent*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - smooth scroll-behavior must be asynchronous
    SimpleTest.ok@SimpleTest/SimpleTest.js:427:16
    testScrollBehaviorInterruption@layout/generic/test/test_scroll_behavior.html:56:7
    async*@layout/generic/test/test_scroll_behavior.html:36:11
    EventListener.handleEvent*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Smooth scrolls must always end at their destination unless they are interrupted, at deltaX: 1, deltaY: 100 - didn't expect +0, but got it
    SimpleTest.isnot@SimpleTest/SimpleTest.js:529:14
    sampleAnimation@layout/generic/test/test_scroll_behavior.html:241:10
    async*testScrollBehaviorFramerate@layout/generic/test/test_scroll_behavior.html:127:30
    async*@layout/generic/test/test_scroll_behavior.html:37:11
    async*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Smooth scrolls must always end at their destination unless they are interrupted, at deltaX: 1, deltaY: 100 - didn't expect +0, but got it
    SimpleTest.isnot@SimpleTest/SimpleTest.js:529:14
    sampleAnimation@layout/generic/test/test_scroll_behavior.html:241:10
    async*testScrollBehaviorFramerate@layout/generic/test/test_scroll_behavior.html:137:33
    async*@layout/generic/test/test_scroll_behavior.html:37:11
    async*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Smooth scroll animation duration must not be framerate dependent at deltaX: 1, deltaY: 100, frameRate: 13fps - got 912, expected +0
    SimpleTest.is@SimpleTest/SimpleTest.js:510:14
    testScrollBehaviorFramerate@layout/generic/test/test_scroll_behavior.html:144:11
    async*@layout/generic/test/test_scroll_behavior.html:37:11
    async*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Smooth scrolls must always end at their destination unless they are interrupted, at deltaX: 1, deltaY: 100 - didn't expect +0, but got it
    SimpleTest.isnot@SimpleTest/SimpleTest.js:529:14
    sampleAnimation@layout/generic/test/test_scroll_behavior.html:241:10
    async*testScrollBehaviorFramerate@layout/generic/test/test_scroll_behavior.html:137:33
    async*@layout/generic/test/test_scroll_behavior.html:37:11
    async*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Smooth scroll animation duration must not be framerate dependent at deltaX: 1, deltaY: 100, frameRate: 60fps - got 104, expected +0
    SimpleTest.is@SimpleTest/SimpleTest.js:510:14
    testScrollBehaviorFramerate@layout/generic/test/test_scroll_behavior.html:144:11
    async*@layout/generic/test/test_scroll_behavior.html:37:11
    async*@layout/generic/test/test_scroll_behavior.html:32:10
  FAIL layout/generic/test/test_scroll_behavior.html - Finished in 5953ms

(In reply to Florian Quèze [:florian] from comment #4)

If that doesn't fix it on your machine, can you give a hint about what APIs to use to "make sure the sampler thread samples the animation"? (Also, feel free to take over the bug if it's easier to just do it than to explain).

As of now there's no API to force the sampler thread to do a sample once, we need to add a such test only API. A relevant function call is this FlushRendering call in SetTestSampleTime. I may be missing something, though.

Comment 6

[Florian Quèze [:florian]]

Attachment: Bug 2023721 - Flush pending scene builds before updating test sample time in SetTestSampleTime. r=hiro

Comment 7

[Hiroyuki Ikezoe (:hiro)]

Filed bug 2024516 for most failures in comment 5.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Hey, Florian. I am curious that the analysis that the MSD animation needs two ticks was done by Claude or you.

Comment 9

[Florian Quèze [:florian]]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

Hey, Florian. I am curious that the analysis that the MSD animation needs two ticks was done by Claude or you.

By Claude (but I did ask it to add some debug logging to figure out what was happening). Sorry if this ended up wasting more of your time than debugging yourself from the beginning. I'm far outside of my area of expertize here, so if I had to fix this alone I would probably just have added a skip-if. I'm glad you figured out the correct more simple fix, thanks for debugging!

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Florian Quèze [:florian] from comment #9)

(In reply to Hiroyuki Ikezoe (:hiro) from comment #8)

Hey, Florian. I am curious that the analysis that the MSD animation needs two ticks was done by Claude or you.

By Claude (but I did ask it to add some debug logging to figure out what was happening). Sorry if this ended up wasting more of your time than debugging yourself from the beginning. I'm far outside of my area of expertize here, so if I had to fix this alone I would probably just have added a skip-if. I'm glad you figured out the correct more simple fix, thanks for debugging!

Thanks for the clarification. No problem, this experience has been a valuable lesson for me, something like Claude gives us very plausible solutions but we should investigate by ourselves.

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/3ad8f97266a2
https://hg.mozilla.org/integration/autoland/rev/11e1b54cd96a
Fix test_scroll_behavior.html to pass standalone. r=layout-reviewers,emilio,hiro

Comment 13

[amarc]

https://hg.mozilla.org/mozilla-central/rev/11e1b54cd96a

Comment 14

[Intermittent Failures Robot]

35 failures were associated with this bug yesterday.

Repository breakdown:

• autoland: 35

Table

	no_variant
linux2404-x86_64/debug	10
linux2404-x86_64/opt	10
linux2404-x86_64/tsan	9
macosx1470-x86_64/debug	1
macosx1470-x86_64/opt	1
windows11-x86/debug	1
windows11-x86/opt	1
windows11-x86_64/debug	1
windows11-x86_64/opt	1

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=2023721&startday=2026-03-19&endday=2026-03-19&tree=all

Comment 15

[Intermittent Failures Robot]

35 failures were associated with this bug in the last 7 days.

This failure happened more than 30 times this week! Resolving this bug is a high priority.

Try to resolve this bug as soon as possible. If unresolved for 2 weeks, the affected test(s) may be disabled.

Repository breakdown:

• autoland: 35

Table

	no_variant
linux2404-x86_64/debug	10
linux2404-x86_64/opt	10
linux2404-x86_64/tsan	9
macosx1470-x86_64/debug	1
macosx1470-x86_64/opt	1
windows11-x86/debug	1
windows11-x86/opt	1
windows11-x86_64/debug	1
windows11-x86_64/opt	1

For more details, see:

https://treeherder.mozilla.org/intermittent-failures/bugdetails?bug=2023721&startday=2026-03-16&endday=2026-03-22&tree=all