Fix interaction between the KeyframeEffect API and CSS keyframes

Explicitly setting the keyframes via the KeyframeEffect API blocks
subsequent updates via CSS keyframe rules.

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

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

@@ -840,8 +840,11 @@ void Animation::setEffect(AnimationEffect* new_effect) {
   NotifyProbe();
 
   // The effect is no longer associated with CSS properties.
-  if (new_effect)
+  if (new_effect) {
     new_effect->SetIgnoreCssTimingProperties();
+    if (KeyframeEffect* keyframe_effect = DynamicTo<KeyframeEffect>(new_effect))
+      keyframe_effect->SetIgnoreCSSKeyframes();
+  }
 
   // The remaining steps are for handling CSS animation and transition events.
   // Both use an event delegate to dispatch events, which must be reattached to

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

@@ -575,7 +575,8 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
   for (const auto& entry : pending_update_.AnimationsWithUpdates()) {
     if (entry.animation->effect()) {
       auto* effect = To<KeyframeEffect>(entry.animation->effect());
-      effect->SetModel(entry.effect->Model());
+      if (!effect->GetIgnoreCSSKeyframes())
+        effect->SetModel(entry.effect->Model());
       effect->UpdateSpecifiedTiming(entry.effect->SpecifiedTiming());
     }
 

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

@@ -158,7 +158,8 @@ KeyframeEffect::KeyframeEffect(Element* target,
       target_pseudo_(),
       model_(model),
       sampled_effect_(nullptr),
-      priority_(priority) {
+      priority_(priority),
+      ignore_css_keyframes_(false) {
   DCHECK(model_);
 
   // fix target for css animations and transitions
@@ -229,6 +230,9 @@ void KeyframeEffect::setComposite(String composite_string) {
 
 HeapVector<ScriptValue> KeyframeEffect::getKeyframes(
     ScriptState* script_state) {
+  if (Animation* animation = GetAnimation())
+    animation->FlushPendingUpdates();
+
   HeapVector<ScriptValue> computed_keyframes;
   if (!model_->HasFrames())
     return computed_keyframes;
@@ -271,6 +275,7 @@ void KeyframeEffect::setKeyframes(ScriptState* script_state,
   if (exception_state.HadException())
     return;
 
+  ignore_css_keyframes_ = true;
   SetKeyframes(new_keyframes);
 }
 

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

@@ -135,6 +135,11 @@ class CORE_EXPORT KeyframeEffect final : public AnimationEffect {
 
   ActiveInterpolationsMap InterpolationsForCommitStyles();
 
+  // Explicitly setting the keyframes via KeyfrfameEffect.setFrames or
+  // Animation.effect block subseuqent changes via CSS keyframe rules.
+  bool GetIgnoreCSSKeyframes() { return ignore_css_keyframes_; }
+  void SetIgnoreCSSKeyframes() { ignore_css_keyframes_ = true; }
+
  private:
   EffectModel::CompositeOperation CompositeInternal() const;
 
@@ -164,6 +169,8 @@ class CORE_EXPORT KeyframeEffect final : public AnimationEffect {
   Priority priority_;
 
   Vector<int> compositor_keyframe_model_ids_;
+
+  bool ignore_css_keyframes_;
 };
 
 template <>

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

-This is a testharness.js-based test.
-PASS Setting a null effect on a running animation fires an animationend event
-PASS Replacing an animation's effect with an effect that targets a different property should update both properties
-PASS Replacing an animation's effect with a shorter one that should have already finished, the animation finishes immediately
-PASS A play-pending animation's effect whose effect is replaced still exits the pending state
-PASS CSS animation events are dispatched at the original element even after setting an effect with a different target element
-PASS After replacing a finished animation's effect with a longer one it fires an animationstart event
-FAIL Replacing the effect of a CSSAnimation causes subsequent changes to corresponding animation-* properties to be ignored assert_equals: keyframes should be the value set by the API expected (string) "200px" but got (undefined) undefined
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/css/css-animations/KeyframeEffect-setKeyframes.tentative-expected.txt

-This is a testharness.js-based test.
-PASS KeyframeEffect.setKeyframes() causes subsequent changes to @keyframes rules to be ignored
-FAIL KeyframeEffect.setKeyframes() causes subsequent changes to animation-timing-function to be ignored assert_equals: Easing should be the easing set by the API expected "linear" but got "ease-in"
-FAIL KeyframeEffect.setKeyframes() should NOT cause subsequent changes to @keyframes rules to be ignored if it threw assert_equals: value for 'left' on Keyframes reflect the value set via style should match expected "300px" but got "100px"
-Harness: the test ran to completion.
-