[scroll-animations] Ignore animation-timeline if replaced by JS

Similarly to how animation-play-state is ignored if play() is called
using the Web Animations API, the animation-timeline property should
probably be ignored if the timeline of a CSSAnimation has been replaced
using the Animation.timeline attribute.

Bug: 1074052
Change-Id: I5fcc16539e50ee2a6da6fe64c181f45086386e93
Fixed: 1141836
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2514210Reviewed-by: Kevin Ellis <kevers@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823481}

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

@@ -199,7 +199,7 @@ class CORE_EXPORT Animation : public EventTargetWithInlineData,
   double playbackRate() const;
   void setPlaybackRate(double, ExceptionState& = ASSERT_NO_EXCEPTION);
   AnimationTimeline* timeline() { return timeline_; }
-  void setTimeline(AnimationTimeline* timeline);
+  virtual void setTimeline(AnimationTimeline* timeline);
   Document* GetDocument() const;
 
   base::Optional<double> startTime() const;

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

@@ -18,7 +18,8 @@ CSSAnimation::CSSAnimation(ExecutionContext* execution_context,
     : Animation(execution_context, timeline, content),
       animation_index_(animation_index),
       animation_name_(animation_name),
-      ignore_css_play_state_(false) {
+      ignore_css_play_state_(false),
+      ignore_css_timeline_(false) {
   // The owning_element does not always equal to the target element of an
   // animation. The following spec gives an example:
   // https://drafts.csswg.org/css-animations-2/#owning-element-section
@@ -54,6 +55,11 @@ void CSSAnimation::reverse(ExceptionState& exception_state) {
   Animation::reverse(exception_state);
 }
 
+void CSSAnimation::setTimeline(AnimationTimeline* timeline) {
+  Animation::setTimeline(timeline);
+  ignore_css_timeline_ = true;
+}
+
 void CSSAnimation::setStartTime(base::Optional<double> start_time_ms,
                                 ExceptionState& exception_state) {
   PlayStateTransitionScope scope(*this);

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

@@ -50,6 +50,7 @@ class CORE_EXPORT CSSAnimation : public Animation {
   void pause(ExceptionState& = ASSERT_NO_EXCEPTION) override;
   void play(ExceptionState& = ASSERT_NO_EXCEPTION) override;
   void reverse(ExceptionState& = ASSERT_NO_EXCEPTION) override;
+  void setTimeline(AnimationTimeline*) override;
   void setStartTime(base::Optional<double>, ExceptionState&) override;
 
   // When set, subsequent changes to animation-play-state no longer affect the
@@ -57,6 +58,8 @@ class CORE_EXPORT CSSAnimation : public Animation {
   // https://drafts.csswg.org/css-animations-2/#interaction-between-animation-play-state-and-web-animations-API
   bool getIgnoreCSSPlayState() { return ignore_css_play_state_; }
   void resetIgnoreCSSPlayState() { ignore_css_play_state_ = false; }
+  bool GetIgnoreCSSTimeline() const { return ignore_css_timeline_; }
+  void ResetIgnoreCSSTimeline() { ignore_css_timeline_ = false; }
   void Trace(blink::Visitor* visitor) const override {
     Animation::Trace(visitor);
     visitor->Trace(owning_element_);
@@ -96,6 +99,8 @@ class CORE_EXPORT CSSAnimation : public Animation {
   // When set, the web-animation API is overruling the animation-play-state
   // style.
   bool ignore_css_play_state_;
+  // When set, changes to the 'animation-timeline' property will be ignored.
+  bool ignore_css_timeline_;
   // The owning element of an animation refers to the element or pseudo-element
   // whose animation-name property was applied that generated the animation
   // The spec: https://drafts.csswg.org/css-animations-2/#owning-element-section

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

@@ -452,14 +452,6 @@ AnimationTimeline* ComputeTimeline(Element* element,
                                    const StyleNameOrKeyword& timeline_name,
                                    StyleRuleScrollTimeline* rule,
                                    AnimationTimeline* existing_timeline) {
-  // TODO(crbug.com/1141836): Implement sticky timelines properly. For now just
-  // ignore animation-timeline whenever a regular (non-CSS) ScrollTimeline is
-  // present.
-  if (existing_timeline && existing_timeline->IsScrollTimeline() &&
-      !existing_timeline->IsCSSScrollTimeline()) {
-    return existing_timeline;
-  }
-
   if (timeline_name.IsKeyword()) {
     if (timeline_name.GetKeyword() == CSSValueID::kAuto)
       return &element->GetDocument().Timeline();
@@ -733,7 +725,7 @@ void CSSAnimations::CalculateAnimationUpdate(CSSAnimationUpdate& update,
             toggle_pause_state ? animation->Paused() : animation->Playing();
 
         AnimationTimeline* timeline = existing_animation->Timeline();
-        if (!is_animation_style_change) {
+        if (!is_animation_style_change && !animation->GetIgnoreCSSTimeline()) {
           timeline = ComputeTimeline(&element, timeline_name,
                                      scroll_timeline_rule, timeline);
         }
@@ -909,8 +901,10 @@ void CSSAnimations::MaybeApplyPendingUpdate(Element* element) {
         effect->SetModel(entry.effect->Model());
       effect->UpdateSpecifiedTiming(entry.effect->SpecifiedTiming());
     }
-    if (entry.animation->timeline() != entry.timeline)
+    if (entry.animation->timeline() != entry.timeline) {
       entry.animation->setTimeline(entry.timeline);
+      To<CSSAnimation>(*entry.animation).ResetIgnoreCSSTimeline();
+    }
 
     running_animations_[entry.index]->Update(entry);
   }

third_party/blink/web_tests/external/wpt/scroll-animations/css/at-scroll-timeline-ignored.tentative.html

+<!DOCTYPE html>
+<link rel="help" src="https://github.com/w3c/csswg-drafts/pull/5666">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="/web-animations/testcommon.js"></script>
+<style>
+  #scrollers {
+    overflow: hidden;
+    height: 0px;
+  }
+  #scrollers > div {
+    overflow: scroll;
+    width: 100px;
+    height: 100px;
+  }
+  #scrollers > div > div {
+    height: 200px;
+  }
+  @keyframes expand {
+    from { width: 100px; }
+    to { width: 200px; }
+  }
+  @scroll-timeline timeline1 {
+    source: selector(#scroller1);
+    orientation: auto;
+    time-range: 1e10s;
+    start: 0px;
+    end: 100px;
+  }
+  @scroll-timeline timeline2 {
+    source: selector(#scroller2);
+    orientation: auto;
+    time-range: 1e10s;
+    start: 0px;
+    end: 100px;
+  }
+  @scroll-timeline timeline3 {
+    source: selector(#scroller3);
+    orientation: auto;
+    time-range: 1e10s;
+    start: 0px;
+    end: 100px;
+  }
+  #element {
+    width: 0px;
+    height: 20px;
+    animation-name: expand;
+    animation-duration: 1e10s;
+    animation-timing-function: linear;
+    animation-timeline: timeline1;
+  }
+  /* Ensure stable expectations if feature is not supported */
+  @supports not (animation-timeline:foo) {
+    #element { animation-play-state: paused; }
+  }
+</style>
+<div id=scrollers>
+  <div id=scroller1><div></div></div>
+  <div id=scroller2><div></div></div>
+  <div id=scroller3><div></div></div>
+  <div id=scroller4><div></div></div>
+</div>
+<div id=container></div>
+<script>
+  // Force layout of scrollers.
+  scroller1.offsetTop;
+  scroller2.offsetTop;
+  scroller3.offsetTop;
+  scroller4.offsetTop;
+
+  scroller1.scrollTop = 20;
+  scroller2.scrollTop = 40;
+  scroller3.scrollTop = 60;
+  scroller4.scrollTop = 80;
+
+  // Create #element in #container, run |func|, then clean up afterwards.
+  function test_animation_timeline(func, description) {
+    promise_test(async () => {
+      try {
+        let element = document.createElement('element');
+        element.setAttribute('id', 'element');
+        container.append(element);
+        await func();
+      } finally {
+        while (container.firstChild)
+          container.firstChild.remove();
+      }
+    }, description);
+  }
+
+  test_animation_timeline(async () => {
+    await waitForNextFrame();
+    assert_equals(getComputedStyle(element).width, '120px');
+    element.style = 'animation-timeline:timeline2';
+    assert_equals(getComputedStyle(element).width, '140px');
+  }, 'Changing animation-timeline changes the timeline (sanity check)');
+
+  test_animation_timeline(async () => {
+    await waitForNextFrame();
+    assert_equals(getComputedStyle(element).width, '120px');
+
+    // Set a (non-CSS) ScrollTimeline on the CSSAnimation.
+    let timeline4 = new ScrollTimeline({
+      scrollSource: scroller4,
+      timeRange: 1e13,
+      startScrollOffset: CSS.px(0),
+      endScrollOffset: CSS.px(100)
+    });
+
+    element.getAnimations()[0].timeline = timeline4;
+    assert_equals(getComputedStyle(element).width, '180px');
+
+    // Changing the animation-timeline property should have no effect.
+    element.style = 'animation-timeline:timeline2';
+    assert_equals(getComputedStyle(element).width, '180px');
+  }, 'animation-timeline ignored after setting timeline with JS (ScrollTimeline from JS)');
+
+  test_animation_timeline(async () => {
+    await waitForNextFrame();
+    assert_equals(getComputedStyle(element).width, '120px');
+    let animation = element.getAnimations()[0];
+    let timeline1 = animation.timeline;
+
+    element.style = 'animation-timeline:timeline2';
+    assert_equals(getComputedStyle(element).width, '140px');
+
+    animation.timeline = timeline1;
+    assert_equals(getComputedStyle(element).width, '120px');
+
+    // Should have no effect.
+    element.style = 'animation-timeline:timeline3';
+    assert_equals(getComputedStyle(element).width, '120px');
+  }, 'animation-timeline ignored after setting timeline with JS (ScrollTimeline from CSS)');
+
+  test_animation_timeline(async () => {
+    await waitForNextFrame();
+    assert_equals(getComputedStyle(element).width, '120px');
+    element.getAnimations()[0].timeline = document.timeline;
+
+    // (The animation continues from where the previous timeline left it).
+    assert_equals(getComputedStyle(element).width, '120px');
+
+    // Changing the animation-timeline property should have no effect.
+    element.style = 'animation-timeline:timeline2';
+    assert_equals(getComputedStyle(element).width, '120px');
+  }, 'animation-timeline ignored after setting timeline with JS (document timeline)');
+
+  test_animation_timeline(async () => {
+    await waitForNextFrame();
+    assert_equals(getComputedStyle(element).width, '120px');
+    element.getAnimations()[0].timeline = null;
+    assert_equals(getComputedStyle(element).width, '0px');
+
+    // Changing the animation-timeline property should have no effect.
+    element.style = 'animation-timeline:timeline2';
+    assert_equals(getComputedStyle(element).width, '0px');
+  }, 'animation-timeline ignored after setting timeline with JS (null)');
+
+</script>