Fix synchronization of animation starts.

When there are composited animations in the process of being started,
we defer the start of main thread animations in order to synchronize
the start times. This process can lead to main thread animations getting
stranded in the pending state if composited animations are being
continuously generated. With this patch, main thread animations queued
up in a previous frame will no longer get blocked waiting to synchronize
with a fresh batch of composited animations.

Bug: 666710
Change-Id: I21eb1184bbf636230991e09691498a190ccf8f86
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2024476
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Reviewed-by: Robert Flack <flackr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737015}

third_party/blink/renderer/core/animation/animation_test.cc

@@ -1493,4 +1493,143 @@ TEST_F(AnimationAnimationTestNoCompositing,
   EXPECT_FALSE(animation->HasPendingActivity());
 }
 
+class AnimationPendingAnimationsTest : public RenderingTest {
+ public:
+  AnimationPendingAnimationsTest()
+      : RenderingTest(MakeGarbageCollected<SingleChildLocalFrameClient>()) {}
+
+  enum CompositingMode { kComposited, kNonComposited };
+
+  void SetUp() override {
+    EnableCompositing();
+    RenderingTest::SetUp();
+    GetDocument().GetAnimationClock().ResetTimeForTesting();
+    timeline = GetDocument().Timeline();
+    timeline->ResetForTesting();
+  }
+
+  Animation* MakeAnimation(const char* target, CompositingMode mode) {
+    Timing timing;
+    timing.iteration_duration = AnimationTimeDelta::FromSecondsD(30);
+    Persistent<StringKeyframe> start_keyframe =
+        MakeGarbageCollected<StringKeyframe>();
+    start_keyframe->SetCSSPropertyValue(CSSPropertyID::kOpacity, "1.0",
+                                        SecureContextMode::kInsecureContext,
+                                        nullptr);
+    Persistent<StringKeyframe> end_keyframe =
+        MakeGarbageCollected<StringKeyframe>();
+    end_keyframe->SetCSSPropertyValue(CSSPropertyID::kOpacity, "0.0",
+                                      SecureContextMode::kInsecureContext,
+                                      nullptr);
+
+    StringKeyframeVector keyframes;
+    keyframes.push_back(start_keyframe);
+    keyframes.push_back(end_keyframe);
+
+    Element* element = GetElementById(target);
+    auto* model = MakeGarbageCollected<StringKeyframeEffectModel>(keyframes);
+
+    Animation* animation = timeline->Play(
+        MakeGarbageCollected<KeyframeEffect>(element, model, timing));
+
+    if (mode == kNonComposited) {
+      // Having a playback rate of zero is one of several ways to force an
+      // animation to be non-composited.
+      animation->updatePlaybackRate(0);
+    }
+
+    return animation;
+  }
+
+  bool Update() {
+    UpdateAllLifecyclePhasesForTest();
+    GetDocument().GetAnimationClock().UpdateTime(base::TimeTicks());
+    return GetDocument().GetPendingAnimations().Update(nullptr, true);
+  }
+
+  void NotifyAnimationStarted(Animation* animation) {
+    animation->GetDocument()
+        ->GetPendingAnimations()
+        .NotifyCompositorAnimationStarted(0, animation->CompositorGroup());
+  }
+
+  void restartAnimation(Animation* animation) {
+    animation->cancel();
+    animation->play();
+  }
+
+  Persistent<DocumentTimeline> timeline;
+};
+
+TEST_F(AnimationPendingAnimationsTest, PendingAnimationStartSynchronization) {
+  RunDocumentLifecycle();
+  SetBodyInnerHTML("<div id='foo'></div><div id='bar'></div>");
+
+  Persistent<Animation> animA = MakeAnimation("foo", kComposited);
+  Persistent<Animation> animB = MakeAnimation("bar", kNonComposited);
+
+  // B's start time synchronized with A's start time.
+  EXPECT_TRUE(Update());
+  EXPECT_TRUE(animA->pending());
+  EXPECT_TRUE(animB->pending());
+  EXPECT_TRUE(animA->HasActiveAnimationsOnCompositor());
+  EXPECT_FALSE(animB->HasActiveAnimationsOnCompositor());
+  NotifyAnimationStarted(animA);
+  EXPECT_FALSE(animA->pending());
+  EXPECT_FALSE(animB->pending());
+}
+
+TEST_F(AnimationPendingAnimationsTest,
+       PendingAnimationCancelUnblocksSynchronizedStart) {
+  RunDocumentLifecycle();
+  SetBodyInnerHTML("<div id='foo'></div><div id='bar'></div>");
+
+  Persistent<Animation> animA = MakeAnimation("foo", kComposited);
+  Persistent<Animation> animB = MakeAnimation("bar", kNonComposited);
+
+  EXPECT_TRUE(Update());
+  EXPECT_TRUE(animA->pending());
+  EXPECT_TRUE(animB->pending());
+  animA->cancel();
+
+  // Animation A no longer blocks B from starting.
+  EXPECT_FALSE(Update());
+  EXPECT_FALSE(animB->pending());
+}
+
+TEST_F(AnimationPendingAnimationsTest,
+       PendingAnimationOnlySynchronizeStartsOfNewlyPendingAnimations) {
+  RunDocumentLifecycle();
+  SetBodyInnerHTML(
+      "<div id='foo'></div><div id='bar'></div><div id='baz'></div>");
+
+  Persistent<Animation> animA = MakeAnimation("foo", kComposited);
+  Persistent<Animation> animB = MakeAnimation("bar", kNonComposited);
+
+  // This test simulates the conditions in crbug.com/666710. The start of a
+  // non-composited animation is deferred in order to synchronize with a
+  // composited animation, which is canceled before it starts. Subsequent frames
+  // produce new composited animations which prevented the non-composited
+  // animation from ever starting. Non-composited animations should not be
+  // synchronize with new composited animations if queued up in a prior call to
+  // PendingAnimations::Update.
+  EXPECT_TRUE(Update());
+  EXPECT_TRUE(animA->pending());
+  EXPECT_TRUE(animB->pending());
+  animA->cancel();
+
+  Persistent<Animation> animC = MakeAnimation("baz", kComposited);
+  Persistent<Animation> animD = MakeAnimation("bar", kNonComposited);
+
+  EXPECT_TRUE(Update());
+  // B's is unblocked despite newly created composited animation.
+  EXPECT_FALSE(animB->pending());
+  EXPECT_TRUE(animC->pending());
+  // D's start time is synchronized with C's start.
+  EXPECT_TRUE(animD->pending());
+  NotifyAnimationStarted(animC);
+  EXPECT_FALSE(animC->pending());
+  EXPECT_FALSE(animD->pending());
+}
+
 }  // namespace blink

third_party/blink/renderer/core/animation/pending_animations.cc

@@ -73,7 +73,7 @@ bool PendingAnimations::Update(
     if (animation->PreCommit(animation->startTime() ? 1 : compositor_group,
                              paint_artifact_compositor, start_on_compositor)) {
       if (animation->HasActiveAnimationsOnCompositor() &&
-          !had_compositor_animation) {
+          !had_compositor_animation && !animation->startTime()) {
         started_synchronized_on_compositor = true;
       }
 
@@ -99,6 +99,7 @@ bool PendingAnimations::Update(
   // remaining synchronized animations need to wait for the synchronized
   // start time. Otherwise they may start immediately.
   if (started_synchronized_on_compositor) {
+    FlushWaitingNonCompositedAnimations();
     waiting_for_compositor_animation_start_.AppendVector(
         waiting_for_start_time);
   } else {
@@ -182,6 +183,28 @@ int PendingAnimations::NextCompositorGroup() {
   return compositor_group_;
 }
 
+void PendingAnimations::FlushWaitingNonCompositedAnimations() {
+  if (waiting_for_compositor_animation_start_.IsEmpty())
+    return;
+
+  // Start any main thread animations that were scheduled to wait on
+  // compositor synchronization from a previous frame. Otherwise, an
+  // continuous influx of new composited animations could delay the start
+  // of non-composited animations indefinitely (crbug.com/666710).
+  HeapVector<Member<Animation>> animations;
+  animations.swap(waiting_for_compositor_animation_start_);
+  for (auto& animation : animations) {
+    if (animation->HasActiveAnimationsOnCompositor()) {
+      waiting_for_compositor_animation_start_.push_back(animation);
+    } else {
+      // TODO(crbug.com/916117): Handle start time of scroll-linked
+      // animations.
+      animation->NotifyReady(
+          animation->timeline()->CurrentTimeSeconds().value_or(0));
+    }
+  }
+}
+
 void PendingAnimations::Trace(blink::Visitor* visitor) {
   visitor->Trace(pending_);
   visitor->Trace(waiting_for_compositor_animation_start_);

third_party/blink/renderer/core/animation/pending_animations.h

@@ -98,6 +98,7 @@ class CORE_EXPORT PendingAnimations final
  private:
   void TimerFired(TimerBase*) { Update(nullptr, false); }
   int NextCompositorGroup();
+  void FlushWaitingNonCompositedAnimations();
 
   HeapVector<Member<Animation>> pending_;
   HeapVector<Member<Animation>> waiting_for_compositor_animation_start_;