Don't take snapshot in CSSScroll/ViewTimeline constructor

CSSScrolltimeline and CSSViewTime objects are created as part of style recalc. However, they currently takes scroll offset snapshot when constructed, which is not spec-compliant (spec says taking snapshot once per frame update before style recalc) and violates pipeline stages. Hence, this patch removes snapshot taking from the constructors. The behavior change is that if a scroll timeline is created due to a style change, it won't be activated in an immediate getComputedStyle() call, but need to wait until the next frame update. This aligns with scroll offset changes. As a result: - Many web tests add waitForNextFrame() before getComputedStyle() - A unit test is changed into web test, because snapshotting is not part of LocalFrameView::UpdateLifecyclePhases(), but current part of update animation steps. Bug: 1371217 Change-Id: Idb0397f241ae579b580c71b01889e92fb7471bde Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3935343 Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org> Reviewed-by: Kevin Ellis <kevers@chromium.org> Cr-Commit-Position: refs/heads/main@{#1058924}
web-platform-tests · Aug 2, 2023 · 1193482 · 1193482
1 parent c62beb0
commit 1193482
Show file tree

Hide file tree

Showing 10 changed files with 52 additions and 3 deletions.
diff --git a/scroll-animations/css/animation-timeline-named-scroll-progress-timeline.tentative.html b/scroll-animations/css/animation-timeline-named-scroll-progress-timeline.tentative.html
@@ -257,6 +257,7 @@
   // Ensure that #main (an ancestor of the scroller) needs style recalc.
   main.style.background = 'lightgray';
   sibling.style.scrollTimelineName = 'timeline';
+  await waitForNextFrame();
   assert_equals(getComputedStyle(target).translate, '100px');
 
   main.remove();
@@ -301,6 +302,7 @@
 
   scroller.style.scrollTimelineName = 'timeline';
   target.style.animation = 'anim 10s linear timeline';
+  await waitForNextFrame();
 
   assert_equals(getComputedStyle(target).translate, '100px');
 
@@ -351,6 +353,7 @@
 
   scroller.style.scrollTimelineName = 'timeline';
   target.style.animation = 'anim 10s linear timeline';
+  await waitForNextFrame();
 
   assert_equals(getComputedStyle(target).translate, '100px');
 

diff --git a/scroll-animations/css/progress-based-animation-animation-longhand-properties.tentative.html b/scroll-animations/css/progress-based-animation-animation-longhand-properties.tentative.html
@@ -104,10 +104,12 @@
 
   // Let animation become 50% in the 1st iteration.
   target.style.animationIterationCount = '2';
+  await waitForNextFrame();
   assert_equals(getComputedStyle(target).translate, '50px');
 
   // Let animation become 0% in the 2nd iteration.
   target.style.animationIterationCount = '4';
+  await waitForNextFrame();
   assert_equals(getComputedStyle(target).translate, '0px');
 }, 'animation-iteration-count');
 
@@ -217,6 +219,7 @@
 
   await scrollTop(scroller, 20); // [0, 100].
   target.style.animationDelay = '-5s';
+  await waitForNextFrame();
   assert_equals(getComputedStyle(target).translate, '60px');
 }, 'animation-delay with a negative value');
 
@@ -234,6 +237,7 @@
   assert_equals(getComputedStyle(target).translate, 'none');
 
   target.style.animationFillMode = 'backwards';
+  await waitForNextFrame();
   assert_equals(getComputedStyle(target).translate, '0px');
 }, 'animation-fill-mode');
 

diff --git a/scroll-animations/css/scroll-timeline-default-iframe-print.html b/scroll-animations/css/scroll-timeline-default-iframe-print.html
@@ -5,6 +5,7 @@
 <link rel="help" href="https://drafts.csswg.org/css-animations-2/#animation-timeline">
 <meta name="assert" content="CSS animation correctly updates values when using the default scroll() timeline">
 <link rel="match" href="scroll-timeline-default-iframe-ref.html">
+<meta name="fuzzy" content="25;100">
 
 <iframe id="target" width="400" height="400" srcdoc='
   <html>

diff --git a/scroll-animations/css/scroll-timeline-document-scroller-quirks.html b/scroll-animations/css/scroll-timeline-document-scroller-quirks.html
@@ -0,0 +1,26 @@
+<!-- Quirks mode -->
+<title>Tests the document scroller in quirks mode</title>
+<link rel="help" href="https://drafts.csswg.org/scroll-animations-1/#scroll-notation">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1180575">
+<link rel="author" href="mailto:andruud@chromium.org">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/scroll-animations/scroll-timelines/testcommon.js"></script>
+<script src="/css/css-animations/support/testcommon.js"></script>
+<style>
+  @keyframes anim {
+    from { z-index: 100; }
+    to { z-index: 100; }
+  }
+  #element {
+    animation: anim forwards scroll(root);
+  }
+</style>
+<div id=element></div>
+
+<script>
+promise_test(async () => {
+  await waitForNextFrame();
+  assert_equals(getComputedStyle(element).zIndex, "100");
+});
+</script>
diff --git a/scroll-animations/css/scroll-timeline-dynamic.tentative.html b/scroll-animations/css/scroll-timeline-dynamic.tentative.html
@@ -71,6 +71,7 @@
     // Verify that the computed style is as expected immediately after the
     // rule change took place.
     instantiate(async (element, expected) => {
+      await waitForNextFrame();
       assert_equals(getComputedStyle(element).width, expected);
     }, description + ' [immediate]');
 

diff --git a/scroll-animations/css/scroll-timeline-in-container-query.html b/scroll-animations/css/scroll-timeline-in-container-query.html
@@ -60,8 +60,9 @@
     assert_equals(getComputedStyle(element).backgroundColor, 'rgb(100, 100, 100)');
     // This causes the timeline to be created.
     outer.style.width = '250px';
-    // Check value with getComputedStyle immediately.
-    assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');
+    // Check value with getComputedStyle immediately, which is the unanimated
+    // value since the scroll timeline is inactive before the next frame.
+    assert_equals(getComputedStyle(element).backgroundColor, 'rgb(0, 0, 0)');
     // Also check value after one frame.
     await waitForNextFrame();
     assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');

diff --git a/scroll-animations/css/scroll-timeline-paused-animations.html b/scroll-animations/css/scroll-timeline-paused-animations.html
@@ -36,6 +36,8 @@
   t.add_cleanup(resetScrollPosition);
 
   div.style.animation = 'anim 100s linear paused scroll(root)';
+  await waitForNextFrame();
+
   const anim = div.getAnimations()[0];
   await anim.ready;
   assert_percents_equal(anim.currentTime, 0, 'timeline time reset');
@@ -55,6 +57,8 @@
   await waitForNextFrame();
 
   div.style.animation = 'anim 100s linear forwards scroll(root)';
+  await waitForNextFrame();
+
   const anim = div.getAnimations()[0];
   await anim.ready;
   assert_percents_equal(anim.currentTime, 0, 'timeline time reset');

diff --git a/scroll-animations/css/scroll-timeline-responsiveness-from-endpoint.html b/scroll-animations/css/scroll-timeline-responsiveness-from-endpoint.html
@@ -32,6 +32,8 @@
   await waitForNextFrame();
 
   div.style.animation = 'anim 100s linear scroll(root)';
+  await waitForNextFrame();
+
   const anim = div.getAnimations()[0];
   await anim.ready;
   assert_percents_equal(anim.timeline.currentTime, 0,

diff --git a/scroll-animations/css/scroll-timeline-sibling-gcs.html b/scroll-animations/css/scroll-timeline-sibling-gcs.html
@@ -42,6 +42,7 @@
     // Unknown timeline, time held at zero.
     assert_equals(getComputedStyle(element).backgroundColor, 'rgb(100, 100, 100)');
     scroller.style.scrollTimeline = 'timeline';
+    await waitForNextFrame();
     assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');
   }, 'Timelines appearing on preceding siblings are visible to getComputedStyle');
 </script>
diff --git a/scroll-animations/css/view-timeline-dynamic.html b/scroll-animations/css/view-timeline-dynamic.html
@@ -63,14 +63,17 @@
 
     // scrollTop=50 is 75% for div75.
     div75.classList.add('timeline');
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '75');
 
     // scrollTop=50 is 25% for div25.
     div25.classList.add('timeline');
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '25');
 
     // scrollTop=50 is before the timeline start for div_before.
     div_before.classList.add('timeline');
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '-1');
     // Scroll to 25% (for div_before) to verify that we're linked to that
     // timeline.
@@ -80,6 +83,7 @@
     // Now we should be back to div25's timeline, although with the new
     // scrollTop=150, it's actually at 75%.
     div_before.classList.remove('timeline');
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '75');
   }, 'Dynamically changing view-timeline-name');
 </script>
@@ -110,6 +114,7 @@
 
     assert_equals(getComputedStyle(target).zIndex, '25');
     timeline.style.viewTimelineAxis = 'horizontal';
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '10');
   }, 'Dynamically changing view-timeline-axis');
 </script>
@@ -139,6 +144,7 @@
 
     assert_equals(getComputedStyle(target).zIndex, '25');
     timeline.style.viewTimelineInset = '0px 50px';
+    await waitForNextFrame();
     assert_equals(getComputedStyle(target).zIndex, '0');
   }, 'Dynamically changing view-timeline-inset');
 </script>
@@ -167,4 +173,4 @@
     timeline.style.display = 'none';
     assert_equals(getComputedStyle(target).zIndex, '-1');
   }, 'Element with view-timeline becoming display:none');
-</script>
+</script>