Skip to content

Commit 1193482

Browse files
xiaochenghpull[bot]
xiaochengh
authored and
pull[bot]
committed
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}
1 parent c62beb0 commit 1193482

10 files changed

Lines changed: 52 additions & 3 deletions

‎scroll-animations/css/animation-timeline-named-scroll-progress-timeline.tentative.html‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@
257257
// Ensure that #main (an ancestor of the scroller) needs style recalc.
258258
main.style.background = 'lightgray';
259259
sibling.style.scrollTimelineName = 'timeline';
260+
await waitForNextFrame();
260261
assert_equals(getComputedStyle(target).translate, '100px');
261262

262263
main.remove();
@@ -301,6 +302,7 @@
301302

302303
scroller.style.scrollTimelineName = 'timeline';
303304
target.style.animation = 'anim 10s linear timeline';
305+
await waitForNextFrame();
304306

305307
assert_equals(getComputedStyle(target).translate, '100px');
306308

@@ -351,6 +353,7 @@
351353

352354
scroller.style.scrollTimelineName = 'timeline';
353355
target.style.animation = 'anim 10s linear timeline';
356+
await waitForNextFrame();
354357

355358
assert_equals(getComputedStyle(target).translate, '100px');
356359

‎scroll-animations/css/progress-based-animation-animation-longhand-properties.tentative.html‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,12 @@
104104

105105
// Let animation become 50% in the 1st iteration.
106106
target.style.animationIterationCount = '2';
107+
await waitForNextFrame();
107108
assert_equals(getComputedStyle(target).translate, '50px');
108109

109110
// Let animation become 0% in the 2nd iteration.
110111
target.style.animationIterationCount = '4';
112+
await waitForNextFrame();
111113
assert_equals(getComputedStyle(target).translate, '0px');
112114
}, 'animation-iteration-count');
113115

@@ -217,6 +219,7 @@
217219

218220
await scrollTop(scroller, 20); // [0, 100].
219221
target.style.animationDelay = '-5s';
222+
await waitForNextFrame();
220223
assert_equals(getComputedStyle(target).translate, '60px');
221224
}, 'animation-delay with a negative value');
222225

@@ -234,6 +237,7 @@
234237
assert_equals(getComputedStyle(target).translate, 'none');
235238

236239
target.style.animationFillMode = 'backwards';
240+
await waitForNextFrame();
237241
assert_equals(getComputedStyle(target).translate, '0px');
238242
}, 'animation-fill-mode');
239243

‎scroll-animations/css/scroll-timeline-default-iframe-print.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
<link rel="help" href="https://drafts.csswg.org/css-animations-2/#animation-timeline">
66
<meta name="assert" content="CSS animation correctly updates values when using the default scroll() timeline">
77
<link rel="match" href="scroll-timeline-default-iframe-ref.html">
8+
<meta name="fuzzy" content="25;100">
89

910
<iframe id="target" width="400" height="400" srcdoc='
1011
<html>

‎scroll-animations/css/scroll-timeline-document-scroller-quirks.html‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<!-- Quirks mode -->
2+
<title>Tests the document scroller in quirks mode</title>
3+
<link rel="help" href="https://drafts.csswg.org/scroll-animations-1/#scroll-notation">
4+
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1180575">
5+
<link rel="author" href="mailto:andruud@chromium.org">
6+
<script src="/resources/testharness.js"></script>
7+
<script src="/resources/testharnessreport.js"></script>
8+
<script src="/scroll-animations/scroll-timelines/testcommon.js"></script>
9+
<script src="/css/css-animations/support/testcommon.js"></script>
10+
<style>
11+
@keyframes anim {
12+
from { z-index: 100; }
13+
to { z-index: 100; }
14+
}
15+
#element {
16+
animation: anim forwards scroll(root);
17+
}
18+
</style>
19+
<div id=element></div>
20+
21+
<script>
22+
promise_test(async () => {
23+
await waitForNextFrame();
24+
assert_equals(getComputedStyle(element).zIndex, "100");
25+
});
26+
</script>

‎scroll-animations/css/scroll-timeline-dynamic.tentative.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
// Verify that the computed style is as expected immediately after the
7272
// rule change took place.
7373
instantiate(async (element, expected) => {
74+
await waitForNextFrame();
7475
assert_equals(getComputedStyle(element).width, expected);
7576
}, description + ' [immediate]');
7677

‎scroll-animations/css/scroll-timeline-in-container-query.html‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,9 @@
6060
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(100, 100, 100)');
6161
// This causes the timeline to be created.
6262
outer.style.width = '250px';
63-
// Check value with getComputedStyle immediately.
64-
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');
63+
// Check value with getComputedStyle immediately, which is the unanimated
64+
// value since the scroll timeline is inactive before the next frame.
65+
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(0, 0, 0)');
6566
// Also check value after one frame.
6667
await waitForNextFrame();
6768
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');

‎scroll-animations/css/scroll-timeline-paused-animations.html‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
t.add_cleanup(resetScrollPosition);
3737

3838
div.style.animation = 'anim 100s linear paused scroll(root)';
39+
await waitForNextFrame();
40+
3941
const anim = div.getAnimations()[0];
4042
await anim.ready;
4143
assert_percents_equal(anim.currentTime, 0, 'timeline time reset');
@@ -55,6 +57,8 @@
5557
await waitForNextFrame();
5658

5759
div.style.animation = 'anim 100s linear forwards scroll(root)';
60+
await waitForNextFrame();
61+
5862
const anim = div.getAnimations()[0];
5963
await anim.ready;
6064
assert_percents_equal(anim.currentTime, 0, 'timeline time reset');

‎scroll-animations/css/scroll-timeline-responsiveness-from-endpoint.html‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@
3232
await waitForNextFrame();
3333

3434
div.style.animation = 'anim 100s linear scroll(root)';
35+
await waitForNextFrame();
36+
3537
const anim = div.getAnimations()[0];
3638
await anim.ready;
3739
assert_percents_equal(anim.timeline.currentTime, 0,

‎scroll-animations/css/scroll-timeline-sibling-gcs.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
// Unknown timeline, time held at zero.
4343
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(100, 100, 100)');
4444
scroller.style.scrollTimeline = 'timeline';
45+
await waitForNextFrame();
4546
assert_equals(getComputedStyle(element).backgroundColor, 'rgb(150, 150, 150)');
4647
}, 'Timelines appearing on preceding siblings are visible to getComputedStyle');
4748
</script>

‎scroll-animations/css/view-timeline-dynamic.html‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,17 @@
6363

6464
// scrollTop=50 is 75% for div75.
6565
div75.classList.add('timeline');
66+
await waitForNextFrame();
6667
assert_equals(getComputedStyle(target).zIndex, '75');
6768

6869
// scrollTop=50 is 25% for div25.
6970
div25.classList.add('timeline');
71+
await waitForNextFrame();
7072
assert_equals(getComputedStyle(target).zIndex, '25');
7173

7274
// scrollTop=50 is before the timeline start for div_before.
7375
div_before.classList.add('timeline');
76+
await waitForNextFrame();
7477
assert_equals(getComputedStyle(target).zIndex, '-1');
7578
// Scroll to 25% (for div_before) to verify that we're linked to that
7679
// timeline.
@@ -80,6 +83,7 @@
8083
// Now we should be back to div25's timeline, although with the new
8184
// scrollTop=150, it's actually at 75%.
8285
div_before.classList.remove('timeline');
86+
await waitForNextFrame();
8387
assert_equals(getComputedStyle(target).zIndex, '75');
8488
}, 'Dynamically changing view-timeline-name');
8589
</script>
@@ -110,6 +114,7 @@
110114

111115
assert_equals(getComputedStyle(target).zIndex, '25');
112116
timeline.style.viewTimelineAxis = 'horizontal';
117+
await waitForNextFrame();
113118
assert_equals(getComputedStyle(target).zIndex, '10');
114119
}, 'Dynamically changing view-timeline-axis');
115120
</script>
@@ -139,6 +144,7 @@
139144

140145
assert_equals(getComputedStyle(target).zIndex, '25');
141146
timeline.style.viewTimelineInset = '0px 50px';
147+
await waitForNextFrame();
142148
assert_equals(getComputedStyle(target).zIndex, '0');
143149
}, 'Dynamically changing view-timeline-inset');
144150
</script>
@@ -167,4 +173,4 @@
167173
timeline.style.display = 'none';
168174
assert_equals(getComputedStyle(target).zIndex, '-1');
169175
}, 'Element with view-timeline becoming display:none');
170-
</script>
176+
</script>

0 commit comments

Comments
 (0)