Align stickiness rules for startTime and reverse with spec for CSSAnimations.

https://github.com/w3c/csswg-drafts/issues/4822

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

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

@@ -165,7 +165,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
 
   virtual void pause(ExceptionState& = ASSERT_NO_EXCEPTION);
   virtual void play(ExceptionState& = ASSERT_NO_EXCEPTION);
-  void reverse(ExceptionState& = ASSERT_NO_EXCEPTION);
+  virtual void reverse(ExceptionState& = ASSERT_NO_EXCEPTION);
   void finish(ExceptionState& = ASSERT_NO_EXCEPTION);
   void updatePlaybackRate(double playback_rate,
                           ExceptionState& = ASSERT_NO_EXCEPTION);
@@ -202,9 +202,9 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   double startTime(bool& is_null) const;
   base::Optional<double> startTime() const;
   base::Optional<double> StartTimeInternal() const { return start_time_; }
-  void setStartTime(double,
-                    bool is_null,
-                    ExceptionState& = ASSERT_NO_EXCEPTION);
+  virtual void setStartTime(double,
+                            bool is_null,
+                            ExceptionState& = ASSERT_NO_EXCEPTION);
 
   const AnimationEffect* effect() const { return content_.Get(); }
   AnimationEffect* effect() { return content_.Get(); }

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

@@ -15,7 +15,7 @@ CSSAnimation::CSSAnimation(ExecutionContext* execution_context,
                            const String& animation_name)
     : Animation(execution_context, timeline, content),
       animation_name_(animation_name),
-      sticky_play_state_(Animation::kUnset) {}
+      ignore_css_play_state_(false) {}
 
 String CSSAnimation::playState() const {
   FlushStyles();
@@ -28,13 +28,29 @@ bool CSSAnimation::pending() const {
 }
 
 void CSSAnimation::pause(ExceptionState& exception_state) {
-  sticky_play_state_ = kPaused;
   Animation::pause(exception_state);
+  if (exception_state.HadException())
+    return;
+  ignore_css_play_state_ = true;
 }
 
 void CSSAnimation::play(ExceptionState& exception_state) {
-  sticky_play_state_ = kRunning;
   Animation::play(exception_state);
+  if (exception_state.HadException())
+    return;
+  ignore_css_play_state_ = true;
+}
+
+void CSSAnimation::reverse(ExceptionState& exception_state) {
+  PlayStateTransitionScope scope(*this);
+  Animation::reverse(exception_state);
+}
+
+void CSSAnimation::setStartTime(double start_time,
+                                bool is_null,
+                                ExceptionState& exception_state) {
+  PlayStateTransitionScope scope(*this);
+  Animation::setStartTime(start_time, is_null, exception_state);
 }
 
 void CSSAnimation::FlushStyles() const {
@@ -44,4 +60,16 @@ void CSSAnimation::FlushStyles() const {
     GetDocument()->UpdateStyleAndLayoutTree();
 }
 
+CSSAnimation::PlayStateTransitionScope::PlayStateTransitionScope(
+    CSSAnimation& animation)
+    : animation_(animation) {
+  was_paused_ = animation_.Paused();
+}
+
+CSSAnimation::PlayStateTransitionScope::~PlayStateTransitionScope() {
+  bool is_paused = animation_.Paused();
+  if (was_paused_ != is_paused)
+    animation_.ignore_css_play_state_ = true;
+}
+
 }  // namespace blink

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

@@ -40,22 +40,37 @@ class CORE_EXPORT CSSAnimation : public Animation {
   // conditionally sticky and override the animation-play-state style.
   void pause(ExceptionState& = ASSERT_NO_EXCEPTION) override;
   void play(ExceptionState& = ASSERT_NO_EXCEPTION) override;
+  void reverse(ExceptionState& = ASSERT_NO_EXCEPTION) override;
+  void setStartTime(double,
+                    bool is_null,
+                    ExceptionState& = ASSERT_NO_EXCEPTION) override;
 
-  // When set, the override takes precedence over animation-play-state.
+  // When set, subsequent changes to animation-play-state no longer affect the
+  // play state.
   // https://drafts.csswg.org/css-animations-2/#interaction-between-animation-play-state-and-web-animations-API
-  AnimationPlayState getWebAnimationOverriddenPlayState() {
-    return sticky_play_state_;
-  }
-  void ResetWebAnimationOverriddenPlayState() { sticky_play_state_ = kUnset; }
+  bool getIgnoreCSSPlayState() { return ignore_css_play_state_; }
+  void resetIgnoreCSSPlayState() { ignore_css_play_state_ = false; }
 
  private:
   void FlushStyles() const;
 
+  class PlayStateTransitionScope {
+    STACK_ALLOCATED();
+
+   public:
+    explicit PlayStateTransitionScope(CSSAnimation& animation);
+    ~PlayStateTransitionScope();
+
+   private:
+    CSSAnimation& animation_;
+    bool was_paused_;
+  };
+
   String animation_name_;
 
   // When set, the web-animation API is overruling the animation-play-state
   // style.
-  AnimationPlayState sticky_play_state_;
+  bool ignore_css_play_state_;
 };
 
 template <>

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

@@ -399,17 +399,12 @@ void CSSAnimations::CalculateAnimationUpdate(CSSAnimationUpdate& update,
         // play state via the animation-play-state style. Ensure that the new
         // play state based on animation-play-state differs from the current
         // play state and that the change is not blocked by a sticky state.
-        Animation::AnimationPlayState sticky_override =
-            animation->getWebAnimationOverriddenPlayState();
         bool toggle_pause_state = false;
-        if (is_paused != was_paused) {
-          if (animation->Paused() && sticky_override != Animation::kPaused &&
-              !is_paused) {
+        if (is_paused != was_paused && !animation->getIgnoreCSSPlayState()) {
+          if (animation->Paused() && !is_paused)
             toggle_pause_state = true;
-          } else if (animation->Playing() &&
-                     sticky_override != Animation::kRunning && is_paused) {
+          else if (animation->Playing() && is_paused)
             toggle_pause_state = true;
-          }
         }
 
         if (keyframes_rule != existing_animation->style_rule ||
@@ -504,10 +499,10 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
 
     if (animation->Paused()) {
       animation->Unpause();
-      animation->ResetWebAnimationOverriddenPlayState();
+      animation->resetIgnoreCSSPlayState();
     } else {
       animation->pause();
-      animation->ResetWebAnimationOverriddenPlayState();
+      animation->resetIgnoreCSSPlayState();
     }
     if (animation->Outdated())
       animation->Update(kTimingUpdateOnDemand);
@@ -552,7 +547,7 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
     animation->play();
     if (inert_animation->Paused())
       animation->pause();
-    animation->ResetWebAnimationOverriddenPlayState();
+    animation->resetIgnoreCSSPlayState();
     animation->Update(kTimingUpdateOnDemand);
 
     running_animations_.push_back(

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

-This is a testharness.js-based test.
-PASS play() overrides animation-play-state
-PASS play() does NOT override the animation-play-state if there was an error
-PASS pause() overrides animation-play-state
-FAIL reverse() overrides animation-play-state when it starts playing the animation assert_equals: Should still be running even after flipping the animation-play-state expected "running" but got "paused"
-PASS reverse() does NOT override animation-play-state if the animation is already running
-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
-PASS Setting the current time completes a pending pause
-Harness: the test ran to completion.
-