[scroll-animations]: update isCurrent calculation

Bug: 1356542 Change-Id: I31263f33a15fb3b98217aee84c0768403bd0eba7 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4054060 Commit-Queue: Kevin Ellis <kevers@chromium.org> Reviewed-by: Robert Flack <flackr@chromium.org> Cr-Commit-Position: refs/heads/main@{#1078224}
chromium · Dec 1, 2022 · 0804c5c · 0804c5c
1 parent 076bcd7
commit 0804c5c
Show file tree

Hide file tree

Showing 11 changed files with 114 additions and 12 deletions.
diff --git a/third_party/blink/renderer/core/animation/animation.cc b/third_party/blink/renderer/core/animation/animation.cc
@@ -2305,7 +2305,7 @@ bool Animation::Update(TimingUpdateReason reason) {
     }
 
     content_->UpdateInheritedTime(inherited_time, AtScrollTimelineBoundary(),
-                                  playback_rate_, reason);
+                                  idle, playback_rate_, reason);
 
     // After updating the animation time if the animation is no longer current
     // blink will no longer composite the element (see

diff --git a/third_party/blink/renderer/core/animation/animation_effect.cc b/third_party/blink/renderer/core/animation/animation_effect.cc
@@ -260,6 +260,7 @@ void AnimationEffect::updateTiming(OptionalEffectTiming* optional_timing,
 void AnimationEffect::UpdateInheritedTime(
     absl::optional<AnimationTimeDelta> inherited_time,
     bool at_progress_timeline_boundary,
+    bool is_idle,
     double inherited_playback_rate,
     TimingUpdateReason reason) const {
   const Timing::AnimationDirection direction =
@@ -269,9 +270,10 @@ void AnimationEffect::UpdateInheritedTime(
   bool needs_update =
       needs_update_ || last_update_time_ != inherited_time ||
       last_at_progress_timeline_boundary_ != at_progress_timeline_boundary ||
-      (owner_ && owner_->EffectSuppressed());
+      last_is_idle_ != is_idle || (owner_ && owner_->EffectSuppressed());
   needs_update_ = false;
   last_update_time_ = inherited_time;
+  last_is_idle_ = is_idle;
   // A finished animation saturates inherited time at 0 or effect end.
   // If we hit a progress timeline boundary and then enter the after phase
   // timeline time doesn't change. Thus, we need to track boundary transitions
@@ -280,8 +282,9 @@ void AnimationEffect::UpdateInheritedTime(
 
   if (needs_update) {
     Timing::CalculatedTiming calculated = SpecifiedTiming().CalculateTimings(
-        inherited_time, at_progress_timeline_boundary, NormalizedTiming(),
-        direction, IsA<KeyframeEffect>(this), inherited_playback_rate);
+        inherited_time, at_progress_timeline_boundary, is_idle,
+        NormalizedTiming(), direction, IsA<KeyframeEffect>(this),
+        inherited_playback_rate);
 
     const bool was_canceled = calculated.phase != calculated_.phase &&
                               calculated.phase == Timing::kPhaseNone;

diff --git a/third_party/blink/renderer/core/animation/animation_effect.h b/third_party/blink/renderer/core/animation/animation_effect.h
@@ -152,6 +152,7 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
   // UpdateChildrenAndEffects.
   void UpdateInheritedTime(absl::optional<AnimationTimeDelta> inherited_time,
                            bool at_progress_timeline_boundary,
+                           bool is_idle,
                            double inherited_playback_rate,
                            TimingUpdateReason) const;
   void Invalidate() const { needs_update_ = true; }
@@ -196,6 +197,7 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
   mutable bool needs_update_;
   mutable absl::optional<AnimationTimeDelta> last_update_time_;
   mutable bool last_at_progress_timeline_boundary_ = false;
+  mutable bool last_is_idle_ = false;
   AnimationTimeDelta cancel_time_;
   const Timing::CalculatedTiming& EnsureCalculated() const;
   void EnsureNormalizedTiming() const;

diff --git a/third_party/blink/renderer/core/animation/animation_effect_test.cc b/third_party/blink/renderer/core/animation/animation_effect_test.cc
@@ -128,6 +128,7 @@ class TestAnimationEffect : public AnimationEffect {
     AnimationEffect::UpdateInheritedTime(
         ANIMATION_TIME_DELTA_FROM_SECONDS(time),
         /* at_progress_timeline_boundary */ false,
+        /* is_idle */ false,
         /* inherited_playback_rate */ 1.0, reason);
   }
 

diff --git a/third_party/blink/renderer/core/animation/inert_effect.cc b/third_party/blink/renderer/core/animation/inert_effect.cc
@@ -49,7 +49,8 @@ InertEffect::InertEffect(KeyframeEffectModelBase* model,
 
 void InertEffect::Sample(HeapVector<Member<Interpolation>>& result) const {
   UpdateInheritedTime(inherited_time_, /* at_scroll_timeline_boundary */ false,
-                      playback_rate_, kTimingUpdateOnDemand);
+                      /* is_idle */ false, playback_rate_,
+                      kTimingUpdateOnDemand);
   if (!IsInEffect()) {
     result.clear();
     return;

diff --git a/third_party/blink/renderer/core/animation/timing.cc b/third_party/blink/renderer/core/animation/timing.cc
@@ -216,6 +216,7 @@ ComputedEffectTiming* Timing::getComputedTiming(
 Timing::CalculatedTiming Timing::CalculateTimings(
     absl::optional<AnimationTimeDelta> local_time,
     bool at_progress_timeline_boundary,
+    bool is_idle,
     const NormalizedTiming& normalized_timing,
     AnimationDirection animation_direction,
     bool is_keyframe_effect,
@@ -282,12 +283,15 @@ Timing::CalculatedTiming Timing::CalculateTimings(
   DCHECK(!calculated.is_in_effect ||
          (current_iteration.has_value() && progress.has_value()));
   calculated.is_in_play = calculated.phase == Timing::kPhaseActive;
+
   // https://w3.org/TR/web-animations-1/#current
   calculated.is_current = calculated.is_in_play ||
                           (playback_rate.has_value() && playback_rate > 0 &&
                            calculated.phase == Timing::kPhaseBefore) ||
                           (playback_rate.has_value() && playback_rate < 0 &&
-                           calculated.phase == Timing::kPhaseAfter);
+                           calculated.phase == Timing::kPhaseAfter) ||
+                          (!is_idle && normalized_timing.timeline_duration);
+
   calculated.local_time = local_time;
   calculated.time_to_next_iteration = time_to_next_iteration;
 

diff --git a/third_party/blink/renderer/core/animation/timing.h b/third_party/blink/renderer/core/animation/timing.h
@@ -213,7 +213,8 @@ struct CORE_EXPORT Timing {
   struct NormalizedTiming {
     DISALLOW_NEW();
     // Value used in normalization math. Stored so that we can convert back if
-    // needed.
+    // needed. At present, only scroll-linked animations have a timeline
+    // duration. If this changes, we need to update the is_current calculation.
     absl::optional<AnimationTimeDelta> timeline_duration;
     // Though timing delays may be expressed as either times or (phase,offset)
     // pairs, post normalization, delays is expressed in time.
@@ -226,9 +227,12 @@ struct CORE_EXPORT Timing {
     AnimationTimeDelta end_time;
   };
 
+  // TODO(crbug.com/1394434): Cleanup method signature by passing in
+  // AnimationEffectOwner.
   CalculatedTiming CalculateTimings(
       absl::optional<AnimationTimeDelta> local_time,
       bool at_progress_timeline_boundary,
+      bool is_idle,
       const NormalizedTiming& normalized_timing,
       AnimationDirection animation_direction,
       bool is_keyframe_effect,

diff --git a/third_party/blink/renderer/core/animation/timing_test.cc b/third_party/blink/renderer/core/animation/timing_test.cc
@@ -19,8 +19,9 @@ class AnimationTimingTest : public testing::Test {
                           : Timing::AnimationDirection::kForwards;
     return timing_.CalculateTimings(local_time,
                                     /* at_progress_timeline_boundary */ false,
-                                    normalized_timing_, animation_direction,
-                                    is_keyframe_effect, playback_rate);
+                                    /* is_idle */ false, normalized_timing_,
+                                    animation_direction, is_keyframe_effect,
+                                    playback_rate);
   }
   bool IsCurrent(absl::optional<double> local_time, double playback_rate) {
     absl::optional<AnimationTimeDelta> local_time_delta;

diff --git a/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc b/third_party/blink/renderer/modules/animationworklet/worklet_animation.cc
@@ -418,7 +418,8 @@ void WorkletAnimation::cancel() {
     for (auto& effect : effects_) {
       effect->UpdateInheritedTime(absl::nullopt,
                                   /* at_scroll_timeline_boundary */ false,
-                                  playback_rate_, kTimingUpdateOnDemand);
+                                  /* is_idle */ false, playback_rate_,
+                                  kTimingUpdateOnDemand);
     }
   }
   SetPlayState(Animation::kIdle);
@@ -504,7 +505,8 @@ void WorkletAnimation::Update(TimingUpdateReason reason) {
         local_times_[i]
             ? absl::make_optional(AnimationTimeDelta(local_times_[i].value()))
             : absl::nullopt,
-        /* at_scroll_timeline_boundary */ false, playback_rate_, reason);
+        /* at_scroll_timeline_boundary */ false,
+        /* is_idle */ false, playback_rate_, reason);
   }
 }
 

diff --git a/third_party/blink/renderer/modules/animationworklet/worklet_animation_effect.cc b/third_party/blink/renderer/modules/animationworklet/worklet_animation_effect.cc
@@ -38,7 +38,8 @@ ComputedEffectTiming* WorkletAnimationEffect::getComputedTiming() const {
       local_time = AnimationTimeDelta(local_time_.value());
     }
     calculated_ = specified_timing_.CalculateTimings(
-        local_time, /*at_progress_timeline_boundary*/ false, normalized_timing_,
+        local_time, /*at_progress_timeline_boundary*/ false,
+        /*is_idle*/ false, normalized_timing_,
         Timing::AnimationDirection::kForwards, false, playback_rate);
   }
 

diff --git a/.../blink/web_tests/external/wpt/scroll-animations/css/get-animations-inactive-timeline.html b/.../blink/web_tests/external/wpt/scroll-animations/css/get-animations-inactive-timeline.html
@@ -0,0 +1,83 @@
+<!DOCTYPE html>
+<html>
+<meta charset="utf-8">
+<title>getAnimations for scroll-linked animations</title>
+<link rel="help"
+ href="https://www.w3.org/TR/web-animations-1/#animation-effect-phases-and-states">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+  @keyframes slide {
+    from { transform: translateX(100px);  }
+    to   { transform: translateX(100px); }
+  }
+
+  #container {
+    border:  10px solid lightgray;
+    overflow-x: scroll;
+    height:  200px;
+    width: 200px;
+    scroll-timeline-name:  timeline;
+  }
+  #spacer {
+    height:  200vh;
+  }
+  #target {
+    background-color:  green;
+    height:  100px;
+    width:  100px;
+    animation:  slide 1s linear timeline;
+  }
+</style>
+<body>
+  <div id="container">
+    <div id="spacer"></div>
+    <div id="target"></div>
+  </div>
+</body>
+<script type="text/javascript">
+  promise_test(async t => {
+    // Newly created timeline is inactive,
+    let animations = document.getAnimations();
+    assert_equals(animations.length, 1,
+                  'Single running animation');
+    assert_true(animations[0].timeline instanceof ScrollTimeline,
+                'Animation associated with a scroll timeline');
+    assert_equals(animations[0].timeline.currentTime, null,
+                  'Timeline is initially inactive');
+
+    // Canceled animation is no longer current.
+    const anim = animations[0];
+    animations[0].cancel();
+
+    assert_equals(
+        document.getAnimations().length, 0,
+        'A canceled animation is no longer returned by getAnimations');
+
+    // Replaying an animation makes it current.
+    anim.play();
+    assert_equals(
+        document.getAnimations().length, 1,
+        'A play-pending animation is return by getAnimations');
+
+    // Animation effect is still current even if the timeline's source element
+    // cannot be scrolled.
+    spacer.style = 'display: none';
+    t.add_cleanup(() => {
+      spacer.style = '';
+    });
+
+    animations = document.getAnimations();
+    assert_equals(
+        animations.length, 1,
+        'Running animation is included in getAnimations list even if ' +
+        'currentTime is null');
+    assert_true(animations[0].timeline  instanceof ScrollTimeline,
+                'Animation has timeline associated with an element that ' +
+                'cannot be scrolled');
+    assert_equals(animations[0].timeline.currentTime, null,
+                  'Inactive timeline when timeline\'s source element cannot ' +
+                  'be scrolled');
+  }, 'getAnimations includes inactive scroll-linked animations that have not ' +
+     'been canceled');
+</script>