Ensure restarted animations don't clobber start time
The bug here occurs because of a race. Blink Animations can be
invalidated for all sorts of reasons and they require cancelling the
current cc::Animation and starting a new one. When this happens, the new
cc::Animation will use the start time that was received for the original
Animation.
The race condition occurs when the Blink Animation is invalidated before
a start time is received. In that case, the Animation starts playing on
the impl thread but the new Animation is created without a start time,
leading to the Animation starting from the beginning, causing a stutter
effect. Additionally, animation events generated on the impl thread when
Blink invalidates an animation are dropped since the main thread
counterpart is gone by the time of the events arrive.
This CL fixes the issue by making Blink recreate the compositor
animation with the same id and setting a bit on it to make it reuse
the start time from the compositor animation when it is pushed. The
animation also reuses the compositor group id on a restarted animation
so that animation events are routed back to the blink::Animation
correctly.
Bug: 1445090
Change-Id: I8e91bdbf8e22b00cd6025d21aa2a982ad4742d31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4605129
Commit-Queue: David Bokan <bokan@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1218221}
diff --git a/web-animations/timing-model/animations/invalidating-animation-before-start-time-synced.html b/web-animations/timing-model/animations/invalidating-animation-before-start-time-synced.html
new file mode 100644
index 0000000..653e469
--- /dev/null
+++ b/web-animations/timing-model/animations/invalidating-animation-before-start-time-synced.html
@@ -0,0 +1,71 @@
+<!DOCTYPE html>
+<link rel="author" href="mailto:bokan@chromium.org">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+.target {
+ width: 100px;
+ height: 100px;
+ background-color: dodgerblue;
+}
+</style>
+<script>
+function jank() {
+ let start = performance.now();
+ let x = 0;
+ while(performance.now() - start < 100) {
+ x++;
+ }
+}
+
+function target1() { return document.querySelector('#target1'); }
+function target2() { return document.querySelector('#target2'); }
+function spinner() { return document.querySelector('#spinner'); }
+
+function firstFrame() {
+ target1().animate([{transform: 'translateX(400px)'}], {id: "target1", duration: 10000});
+ target2().animate([{transform: 'translateX(400px)'}], {id: "target2", duration: 10000});
+ requestAnimationFrame(secondFrame);
+
+ // Simulate some jank so that, if the above animations are started
+ // asynchronously, the next rendering opportunity is likely to start
+ // immediately after this one and without the animations having started yet.
+ jank();
+}
+
+function secondFrame() {
+ // Modify the style to invalidate the starting keyframe on the first target
+ // only.
+ target1().style.transform = `translateY(-1px)`;
+
+ // The spinner is used to avoid a specific Chrome behavior (bug?). It ensures
+ // a new animation is pushed to the compositor in this frame and prevents the
+ // #target1 animation being started from the main thread in
+ // PendingAnimations::Update when `started_synchronized_on_compositor` is
+ // false.
+ spinner().animate([{transform: 'rotateZ(90deg)'}], {id: 'spinner', duration: 1000});
+
+ requestAnimationFrame(finishTestCb);
+}
+
+let finishTestCb = null;
+const finishTest = new Promise(resolve => { finishTestCb = resolve; });
+
+promise_test(async (t) => {
+ onload = () => requestAnimationFrame(firstFrame);
+ await finishTest;
+
+ anim1 = target1().getAnimations()[0];
+ anim2 = target2().getAnimations()[0];
+
+ await Promise.all([anim1.ready, anim2.ready]);
+ assert_not_equals(anim1.startTime, 0);
+ assert_equals(anim1.startTime, anim2.startTime);
+}, "Animation invalidated before startTime is set doesn't affect startTime");
+</script>
+<!-- This text is necessary in Chrome in order to trigger a first contentful
+paint which unblocks compositor commits. -->
+The blue boxes below should stay aligned in the x-axis.
+<div id="target1" class="target"></div>
+<div id="target2" class="target"></div>
+<div id="spinner" class="target" style="background:limegreen"></div>
