Sync styles checking for a pending change to play state on a CSSAnimation.

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

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

@@ -732,7 +732,7 @@ void Animation::setStartTime(double start_time_ms,
 
   // 7. If animation has a pending play task or a pending pause task, cancel
   //    that task and resolve animation’s current ready promise with animation.
-  if (pending()) {
+  if (PendingInternal()) {
     pending_pause_ = false;
     pending_play_ = false;
     if (ready_promise_ &&
@@ -830,7 +830,7 @@ Animation::AnimationPlayState Animation::CalculateAnimationPlayState() const {
   //    * animation does not have either a pending play task or a pending pause
   //      task,
   //    then idle.
-  if (!CurrentTimeInternal() && !pending())
+  if (!CurrentTimeInternal() && !PendingInternal())
     return kIdle;
 
   // 2. Either of the following conditions are true:
@@ -854,15 +854,19 @@ Animation::AnimationPlayState Animation::CalculateAnimationPlayState() const {
   return kRunning;
 }
 
-bool Animation::pending() const {
+bool Animation::PendingInternal() const {
   return pending_pause_ || pending_play_;
 }
 
+bool Animation::pending() const {
+  return PendingInternal();
+}
+
 // https://drafts.csswg.org/web-animations-1/#reset-an-animations-pending-tasks.
 void Animation::ResetPendingTasks() {
   // 1. If animation does not have a pending play task or a pending pause task,
   //    abort this procedure.
-  if (!pending())
+  if (!PendingInternal())
     return;
 
   // 2. If animation has a pending play task, cancel that task.
@@ -1244,7 +1248,7 @@ void Animation::AsyncFinishMicrotask() {
   // transition was only temporary.
   if (pending_finish_notification_) {
     // A pending play or pause must resolve before the finish promise.
-    if (pending() && timeline_)
+    if (PendingInternal() && timeline_)
       NotifyReady(timeline_->CurrentTimeSeconds().value_or(0));
     CommitFinishNotification();
   }
@@ -1295,7 +1299,7 @@ void Animation::updatePlaybackRate(double playback_rate,
   //
   // 3a If animation has a pending play task or a pending pause task,
   //    Abort these steps.
-  if (pending())
+  if (PendingInternal())
     return;
 
   switch (play_state) {
@@ -1377,7 +1381,7 @@ ScriptPromise Animation::ready(ScriptState* script_state) {
   if (!ready_promise_) {
     ready_promise_ = MakeGarbageCollected<AnimationPromise>(
         ExecutionContext::From(script_state));
-    if (!pending())
+    if (!PendingInternal())
       ready_promise_->Resolve(this);
   }
   return ready_promise_->Promise(script_state->World());
@@ -1569,7 +1573,7 @@ void Animation::StartAnimationOnCompositor(
   // the playback rate preserve current time even if the start time is set.
   // Asynchronous updates have an associated pending play or pending pause
   // task associated with them.
-  if (start_time_ && !pending()) {
+  if (start_time_ && !PendingInternal()) {
     start_time = To<DocumentTimeline>(*timeline_)
                      .ZeroTime()
                      .since_origin()
@@ -1624,7 +1628,8 @@ void Animation::SetCompositorPending(bool effect_changed) {
   // sync them. This can happen if the blink side animation was started, the
   // compositor side hadn't started on its side yet, and then the blink side
   // start time was cleared (e.g. by setting current time).
-  if (pending() || !compositor_state_ || compositor_state_->effect_changed ||
+  if (PendingInternal() || !compositor_state_ ||
+      compositor_state_->effect_changed ||
       compositor_state_->playback_rate != EffectivePlaybackRate() ||
       compositor_state_->start_time != start_time_ ||
       !compositor_state_->start_time || !start_time_) {
@@ -1977,10 +1982,10 @@ void Animation::RejectAndResetPromiseMaybeAsync(AnimationPromise* promise) {
 void Animation::NotifyProbe() {
   AnimationPlayState old_play_state = reported_play_state_;
   AnimationPlayState new_play_state =
-      pending() ? kPending : CalculateAnimationPlayState();
+      PendingInternal() ? kPending : CalculateAnimationPlayState();
 
   if (old_play_state != new_play_state) {
-    if (!pending()) {
+    if (!PendingInternal()) {
       probe::AnimationPlayStateChanged(document_, this, old_play_state,
                                        new_play_state);
     }

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

@@ -151,13 +151,17 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   static const char* PlayStateString(AnimationPlayState);
   AnimationPlayState CalculateAnimationPlayState() const;
 
-  // Do not call this method directly except via the v8 bindings. Depending on
-  // the type of animation a style flush is required to ensure that pending
-  // style changes that affect play state are resolved. Instead call
-  // PlayStateString directly.
+  // As a web exposed API, playState must update style and layout if the play
+  // state may be affected by it (see CSSAnimation::playState), whereas
+  // PlayStateString can be used to query the current play state.
   virtual String playState() const;
 
-  bool pending() const;
+  bool PendingInternal() const;
+
+  // As a web exposed API, pending must update style and layout if the pending
+  // status may be affected by it (see CSSAnimation::pending), whereas
+  // PendingInternal can be used to query the current pending status.
+  virtual bool pending() const;
 
   virtual void pause(ExceptionState& = ASSERT_NO_EXCEPTION);
   virtual void play(ExceptionState& = ASSERT_NO_EXCEPTION);

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

@@ -74,7 +74,7 @@ bool ConsiderAnimationAsIncompatible(const Animation& animation,
   if (&animation == &animation_to_add)
     return false;
 
-  if (animation.pending())
+  if (animation.PendingInternal())
     return true;
 
   switch (animation.CalculateAnimationPlayState()) {

third_party/blink/renderer/core/animation/css/css_animation.cc

@@ -18,11 +18,15 @@ CSSAnimation::CSSAnimation(ExecutionContext* execution_context,
       sticky_play_state_(Animation::kUnset) {}
 
 String CSSAnimation::playState() const {
-  if (GetDocument())
-    GetDocument()->UpdateStyleAndLayoutTree();
+  FlushStyles();
   return Animation::playState();
 }
 
+bool CSSAnimation::pending() const {
+  FlushStyles();
+  return Animation::pending();
+}
+
 void CSSAnimation::pause(ExceptionState& exception_state) {
   sticky_play_state_ = kPaused;
   Animation::pause(exception_state);
@@ -33,4 +37,11 @@ void CSSAnimation::play(ExceptionState& exception_state) {
   Animation::play(exception_state);
 }
 
+void CSSAnimation::FlushStyles() const {
+  // TODO(1043778): Flush is likely not required once the CSSAnimation is
+  // disassociated from its owning element.
+  if (GetDocument())
+    GetDocument()->UpdateStyleAndLayoutTree();
+}
+
 }  // namespace blink

third_party/blink/renderer/core/animation/css/css_animation.h

@@ -34,6 +34,7 @@ class CORE_EXPORT CSSAnimation : public Animation {
   // to animation-play-state and display:none must update the play state.
   // https://drafts.csswg.org/css-animations-2/#requirements-on-pending-style-changes
   String playState() const override;
+  bool pending() const override;
 
   // Explicit calls to the web-animation API that update the play state are
   // conditionally sticky and override the animation-play-state style.
@@ -48,6 +49,8 @@ class CORE_EXPORT CSSAnimation : public Animation {
   void ResetWebAnimationOverriddenPlayState() { sticky_play_state_ = kUnset; }
 
  private:
+  void FlushStyles() const;
+
   String animation_name_;
 
   // When set, the web-animation API is overruling the animation-play-state

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

@@ -82,7 +82,7 @@ bool PendingAnimations::Update(
 
       if (animation->Playing() && !animation->startTime()) {
         waiting_for_start_time.push_back(animation.Get());
-      } else if (animation->pending()) {
+      } else if (animation->PendingInternal()) {
         // A pending animation that is not waiting on a start time does not need
         // to be synchronized with animations that are starting up. Nonetheless,
         // it needs to notify the animation to resolve the ready promise and
@@ -153,7 +153,7 @@ void PendingAnimations::NotifyCompositorAnimationStarted(
   animations.swap(waiting_for_compositor_animation_start_);
 
   for (auto animation : animations) {
-    if (animation->startTime() || !animation->pending() ||
+    if (animation->startTime() || !animation->PendingInternal() ||
         !animation->timeline() || !animation->timeline()->IsActive()) {
       // Already started or no longer relevant.
       continue;

third_party/blink/web_tests/external/wpt/css/css-animations/CSSAnimation-pausing.tentative-expected.txt

@@ -7,6 +7,6 @@ PASS reverse() does NOT override animation-play-state if the animation is alread
 FAIL Setting the startTime to null overrides animation-play-state if the animation is already running assert_equals: Should still be paused even after flipping the animation-play-state expected "paused" but got "running"
 FAIL Setting the startTime to non-null overrides animation-play-state if the animation is paused assert_equals: Should still be running even after flipping the animation-play-state expected "running" but got "paused"
 PASS Setting the startTime to non-null does NOT override the animation-play-state if the animation is already running
-FAIL Setting the current time completes a pending pause assert_true: Animation is pause-pending expected true got false
+PASS Setting the current time completes a pending pause
 Harness: the test ran to completion.