Cleanup of Animations timing function-related code

This is a code health CL which makes a number of changes:
  * Renames AnimationEffect::RepeatedDuration to ActiveDuration to align
    with the spec.
  * Adds a few links/descriptions to the AnimationEffect header.
  * Moves two functions in timing_calculations.h to the end, to properly
    group all spec-related functions and differentiate them from
    non-spec related.
  * Documents said moved functions.

No behavior changes should be seen from this CL, its purely renames +
comments.

Bug: 630915
Change-Id: I8c9618212722d650ea009f54af0fa910214d3452
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1678719Reviewed-by: Kevin Ellis <kevers@chromium.org>
Commit-Queue: Stephen McGruer <smcgruer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672630}

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

@@ -70,7 +70,7 @@ AnimationTimeDelta AnimationEffect::IterationDuration() const {
   return result;
 }
 
-double AnimationEffect::RepeatedDuration() const {
+double AnimationEffect::ActiveDuration() const {
   const double result =
       MultiplyZeroAlwaysGivesZero(IterationDuration(), timing_.iteration_count);
   DCHECK_GE(result, 0);
@@ -80,7 +80,7 @@ double AnimationEffect::RepeatedDuration() const {
 double AnimationEffect::EndTimeInternal() const {
   // Per the spec, the end time has a lower bound of 0.0:
   // https://drafts.csswg.org/web-animations-1/#end-time
-  return std::max(timing_.start_delay + RepeatedDuration() + timing_.end_delay,
+  return std::max(timing_.start_delay + ActiveDuration() + timing_.end_delay,
                   0.0);
 }
 
@@ -99,7 +99,7 @@ ComputedEffectTiming* AnimationEffect::getComputedTiming() const {
 
   // ComputedEffectTiming members.
   computed_timing->setEndTime(EndTimeInternal() * 1000);
-  computed_timing->setActiveDuration(RepeatedDuration() * 1000);
+  computed_timing->setActiveDuration(ActiveDuration() * 1000);
 
   if (IsNull(EnsureCalculated().local_time)) {
     computed_timing->setLocalTimeToNull();
@@ -159,7 +159,7 @@ void AnimationEffect::UpdateInheritedTime(double inherited_time,
   const double local_time = inherited_time;
   double time_to_next_iteration = std::numeric_limits<double>::infinity();
   if (needs_update) {
-    const double active_duration = RepeatedDuration();
+    const double active_duration = ActiveDuration();
     // TODO(yigu): Direction of WorkletAnimation is always forwards based on
     // the calculation. Need to unify the logic to handle it correctly.
     // https://crbug.com/896249.

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

@@ -120,8 +120,12 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
   }
   double LocalTime() const { return EnsureCalculated().local_time; }
 
+  // https://drafts.csswg.org/web-animations-1/#iteration-duration
   AnimationTimeDelta IterationDuration() const;
-  double RepeatedDuration() const;
+
+  // https://drafts.csswg.org/web-animations-1/#active-duration
+  double ActiveDuration() const;
+
   double EndTimeInternal() const;
 
   const Timing& SpecifiedTiming() const { return timing_; }
@@ -159,6 +163,11 @@ class CORE_EXPORT AnimationEffect : public ScriptWrappable {
   void ClearEventDelegate() { event_delegate_ = nullptr; }
 
   virtual void UpdateChildrenAndEffects() const = 0;
+
+  // This is the value of the iteration duration when it is specified as 'auto'.
+  // In web-animations-1, auto is treated as "the value zero for the purpose of
+  // timing model calculations and for the result of the duration member
+  // returned from getComputedTiming()".
   virtual AnimationTimeDelta IntrinsicIterationDuration() const {
     return AnimationTimeDelta();
   }

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

@@ -135,7 +135,7 @@ TEST(AnimationAnimationEffectTest, Sanity) {
   EXPECT_TRUE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(2, animation_node->RepeatedDuration());
+  EXPECT_EQ(2, animation_node->ActiveDuration());
   EXPECT_EQ(0, animation_node->Progress());
 
   animation_node->UpdateInheritedTime(1);
@@ -145,7 +145,7 @@ TEST(AnimationAnimationEffectTest, Sanity) {
   EXPECT_TRUE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(2, animation_node->RepeatedDuration());
+  EXPECT_EQ(2, animation_node->ActiveDuration());
   EXPECT_EQ(0.5, animation_node->Progress());
 
   animation_node->UpdateInheritedTime(2);
@@ -155,7 +155,7 @@ TEST(AnimationAnimationEffectTest, Sanity) {
   EXPECT_FALSE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(2, animation_node->RepeatedDuration());
+  EXPECT_EQ(2, animation_node->ActiveDuration());
   EXPECT_EQ(1, animation_node->Progress());
 
   animation_node->UpdateInheritedTime(3);
@@ -165,7 +165,7 @@ TEST(AnimationAnimationEffectTest, Sanity) {
   EXPECT_FALSE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(2, animation_node->RepeatedDuration());
+  EXPECT_EQ(2, animation_node->ActiveDuration());
   EXPECT_EQ(1, animation_node->Progress());
 }
 
@@ -245,12 +245,12 @@ TEST(AnimationAnimationEffectTest, ZeroIteration) {
   auto* animation_node = MakeGarbageCollected<TestAnimationEffect>(timing);
 
   animation_node->UpdateInheritedTime(-1);
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_TRUE(IsNull(animation_node->CurrentIteration()));
   EXPECT_FALSE(animation_node->Progress());
 
   animation_node->UpdateInheritedTime(0);
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_EQ(0, animation_node->CurrentIteration());
   EXPECT_EQ(0, animation_node->Progress());
 }
@@ -267,7 +267,7 @@ TEST(AnimationAnimationEffectTest, InfiniteIteration) {
   EXPECT_FALSE(animation_node->Progress());
 
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
-            animation_node->RepeatedDuration());
+            animation_node->ActiveDuration());
 
   animation_node->UpdateInheritedTime(0);
   EXPECT_EQ(0, animation_node->CurrentIteration());
@@ -373,7 +373,7 @@ TEST(AnimationAnimationEffectTest, ZeroDurationSanity) {
   EXPECT_FALSE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_EQ(1, animation_node->Progress());
 
   animation_node->UpdateInheritedTime(1);
@@ -383,7 +383,7 @@ TEST(AnimationAnimationEffectTest, ZeroDurationSanity) {
   EXPECT_FALSE(animation_node->IsCurrent());
   EXPECT_TRUE(animation_node->IsInEffect());
   EXPECT_EQ(0, animation_node->CurrentIteration());
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_EQ(1, animation_node->Progress());
 }
 
@@ -474,12 +474,12 @@ TEST(AnimationAnimationEffectTest, ZeroDurationInfiniteIteration) {
   auto* animation_node = MakeGarbageCollected<TestAnimationEffect>(timing);
 
   animation_node->UpdateInheritedTime(-1);
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_TRUE(IsNull(animation_node->CurrentIteration()));
   EXPECT_FALSE(animation_node->Progress());
 
   animation_node->UpdateInheritedTime(0);
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
             animation_node->CurrentIteration());
   EXPECT_EQ(1, animation_node->Progress());
@@ -573,7 +573,7 @@ TEST(AnimationAnimationEffectTest, InfiniteDurationSanity) {
   animation_node->UpdateInheritedTime(0);
 
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
-            animation_node->RepeatedDuration());
+            animation_node->ActiveDuration());
   EXPECT_EQ(AnimationEffect::kPhaseActive, animation_node->GetPhase());
   EXPECT_TRUE(animation_node->IsInPlay());
   EXPECT_TRUE(animation_node->IsCurrent());
@@ -584,7 +584,7 @@ TEST(AnimationAnimationEffectTest, InfiniteDurationSanity) {
   animation_node->UpdateInheritedTime(1);
 
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
-            animation_node->RepeatedDuration());
+            animation_node->ActiveDuration());
   EXPECT_EQ(AnimationEffect::kPhaseActive, animation_node->GetPhase());
   EXPECT_TRUE(animation_node->IsInPlay());
   EXPECT_TRUE(animation_node->IsCurrent());
@@ -602,7 +602,7 @@ TEST(AnimationAnimationEffectTest, InfiniteDurationZeroIterations) {
 
   animation_node->UpdateInheritedTime(0);
 
-  EXPECT_EQ(0, animation_node->RepeatedDuration());
+  EXPECT_EQ(0, animation_node->ActiveDuration());
   EXPECT_EQ(AnimationEffect::kPhaseAfter, animation_node->GetPhase());
   EXPECT_FALSE(animation_node->IsInPlay());
   EXPECT_FALSE(animation_node->IsCurrent());
@@ -630,7 +630,7 @@ TEST(AnimationAnimationEffectTest, InfiniteDurationInfiniteIterations) {
   animation_node->UpdateInheritedTime(0);
 
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
-            animation_node->RepeatedDuration());
+            animation_node->ActiveDuration());
   EXPECT_EQ(AnimationEffect::kPhaseActive, animation_node->GetPhase());
   EXPECT_TRUE(animation_node->IsInPlay());
   EXPECT_TRUE(animation_node->IsCurrent());
@@ -641,7 +641,7 @@ TEST(AnimationAnimationEffectTest, InfiniteDurationInfiniteIterations) {
   animation_node->UpdateInheritedTime(1);
 
   EXPECT_EQ(std::numeric_limits<double>::infinity(),
-            animation_node->RepeatedDuration());
+            animation_node->ActiveDuration());
   EXPECT_EQ(AnimationEffect::kPhaseActive, animation_node->GetPhase());
   EXPECT_TRUE(animation_node->IsInPlay());
   EXPECT_TRUE(animation_node->IsCurrent());

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

@@ -1175,7 +1175,7 @@ void CSSAnimations::AnimationEventDelegate::OnEventCondition(
       previous_phase_ != AnimationEffect::kPhaseAfter) {
     MaybeDispatch(Document::kAnimationEndListener,
                   event_type_names::kAnimationend,
-                  animation_node.RepeatedDuration());
+                  animation_node.ActiveDuration());
   }
 
   previous_phase_ = current_phase;
@@ -1249,7 +1249,7 @@ void CSSAnimations::TransitionEventDelegate::OnEventCondition(
       // "active time of the animation at the moment it was cancelled,
       // calculated using a fill mode of both".
       double cancel_active_time = CalculateActiveTime(
-          animation_node.RepeatedDuration(), Timing::FillMode::BOTH,
+          animation_node.ActiveDuration(), Timing::FillMode::BOTH,
           animation_node.LocalTime(), previous_phase_,
           animation_node.SpecifiedTiming());
       EnqueueEvent(event_type_names::kTransitioncancel, cancel_active_time);

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

@@ -436,7 +436,7 @@ double KeyframeEffect::CalculateTimeToEffectChange(
     double local_time,
     double time_to_next_iteration) const {
   const double start_time = SpecifiedTiming().start_delay;
-  const double end_time_minus_end_delay = start_time + RepeatedDuration();
+  const double end_time_minus_end_delay = start_time + ActiveDuration();
   const double end_time =
       end_time_minus_end_delay + SpecifiedTiming().end_delay;
   const double after_time = std::min(end_time_minus_end_delay, end_time);

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

@@ -48,6 +48,12 @@ inline bool IsWithinEpsilon(double a, double b) {
 inline bool LessThanOrEqualToWithinEpsilon(double a, double b) {
   return a <= b || IsWithinEpsilon(a, b);
 }
+
+inline bool EndsOnIterationBoundary(double iteration_count,
+                                    double iteration_start) {
+  DCHECK(std::isfinite(iteration_count));
+  return !fmod(iteration_count + iteration_start, 1);
+}
 }  // namespace
 
 static inline double MultiplyZeroAlwaysGivesZero(double x, double y) {
@@ -90,6 +96,7 @@ static inline AnimationEffect::Phase CalculatePhase(
   return AnimationEffect::kPhaseActive;
 }
 
+// https://drafts.csswg.org/web-animations/#calculating-the-active-time
 static inline double CalculateActiveTime(double active_duration,
                                          Timing::FillMode fill_mode,
                                          double local_time,
@@ -121,69 +128,6 @@ static inline double CalculateActiveTime(double active_duration,
   }
 }
 
-static inline double CalculateOffsetActiveTime(double active_duration,
-                                               double active_time,
-                                               double start_offset) {
-  DCHECK_GE(active_duration, 0);
-  DCHECK_GE(start_offset, 0);
-
-  if (IsNull(active_time))
-    return NullValue();
-
-  DCHECK(active_time >= 0 &&
-         LessThanOrEqualToWithinEpsilon(active_time, active_duration));
-
-  if (!std::isfinite(active_time))
-    return std::numeric_limits<double>::infinity();
-
-  return active_time + start_offset;
-}
-
-static inline bool EndsOnIterationBoundary(double iteration_count,
-                                           double iteration_start) {
-  DCHECK(std::isfinite(iteration_count));
-  return !fmod(iteration_count + iteration_start, 1);
-}
-
-// TODO(crbug.com/630915): Align this function with current Web Animations spec
-// text.
-static inline double CalculateIterationTime(double iteration_duration,
-                                            double repeated_duration,
-                                            double offset_active_time,
-                                            double start_offset,
-                                            AnimationEffect::Phase phase,
-                                            const Timing& specified) {
-  DCHECK_GT(iteration_duration, 0);
-  DCHECK_EQ(repeated_duration,
-            MultiplyZeroAlwaysGivesZero(iteration_duration,
-                                        specified.iteration_count));
-
-  if (IsNull(offset_active_time))
-    return NullValue();
-
-  DCHECK_GE(offset_active_time, 0);
-  DCHECK(LessThanOrEqualToWithinEpsilon(offset_active_time,
-                                        repeated_duration + start_offset));
-
-  if (!std::isfinite(offset_active_time) ||
-      (offset_active_time - start_offset == repeated_duration &&
-       specified.iteration_count &&
-       EndsOnIterationBoundary(specified.iteration_count,
-                               specified.iteration_start)))
-    return iteration_duration;
-
-  DCHECK(std::isfinite(offset_active_time));
-  double iteration_time = fmod(offset_active_time, iteration_duration);
-
-  // This implements step 3 of
-  // https://drafts.csswg.org/web-animations/#calculating-the-simple-iteration-progress
-  if (iteration_time == 0 && phase == AnimationEffect::kPhaseAfter &&
-      repeated_duration != 0 && offset_active_time != 0)
-    return iteration_duration;
-
-  return iteration_time;
-}
-
 // Calculates the overall progress, which describes the number of iterations
 // that have completed (including partial iterations).
 // https://drafts.csswg.org/web-animations/#calculating-the-overall-progress
@@ -358,6 +302,71 @@ static inline double CalculateTransformedProgress(
                                    AccuracyForDuration(iteration_duration));
 }
 
+// Offsets the active time by how far into the animation we start (i.e. the
+// product of the iteration start and iteration duration). This is not part of
+// the Web Animations spec; it is used for calculating the time until the next
+// iteration to optimize scheduling.
+static inline double CalculateOffsetActiveTime(double active_duration,
+                                               double active_time,
+                                               double start_offset) {
+  DCHECK_GE(active_duration, 0);
+  DCHECK_GE(start_offset, 0);
+
+  if (IsNull(active_time))
+    return NullValue();
+
+  DCHECK(active_time >= 0 &&
+         LessThanOrEqualToWithinEpsilon(active_time, active_duration));
+
+  if (!std::isfinite(active_time))
+    return std::numeric_limits<double>::infinity();
+
+  return active_time + start_offset;
+}
+
+// Maps the offset active time into 'iteration time space'[0], aka the offset
+// into the current iteration. This is not part of the Web Animations spec (note
+// that the section linked below is non-normative); it is used for calculating
+// the time until the next iteration to optimize scheduling.
+//
+// [0] https://drafts.csswg.org/web-animations-1/#iteration-time-space
+static inline double CalculateIterationTime(double iteration_duration,
+                                            double active_duration,
+                                            double offset_active_time,
+                                            double start_offset,
+                                            AnimationEffect::Phase phase,
+                                            const Timing& specified) {
+  DCHECK_GT(iteration_duration, 0);
+  DCHECK_EQ(active_duration,
+            MultiplyZeroAlwaysGivesZero(iteration_duration,
+                                        specified.iteration_count));
+
+  if (IsNull(offset_active_time))
+    return NullValue();
+
+  DCHECK_GE(offset_active_time, 0);
+  DCHECK(LessThanOrEqualToWithinEpsilon(offset_active_time,
+                                        active_duration + start_offset));
+
+  if (!std::isfinite(offset_active_time) ||
+      (offset_active_time - start_offset == active_duration &&
+       specified.iteration_count &&
+       EndsOnIterationBoundary(specified.iteration_count,
+                               specified.iteration_start)))
+    return iteration_duration;
+
+  DCHECK(std::isfinite(offset_active_time));
+  double iteration_time = fmod(offset_active_time, iteration_duration);
+
+  // This implements step 3 of
+  // https://drafts.csswg.org/web-animations/#calculating-the-simple-iteration-progress
+  if (iteration_time == 0 && phase == AnimationEffect::kPhaseAfter &&
+      active_duration != 0 && offset_active_time != 0)
+    return iteration_duration;
+
+  return iteration_time;
+}
+
 }  // namespace blink
 
 #endif

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

@@ -104,7 +104,7 @@ TEST(AnimationTimingCalculationsTest, IterationTime) {
   Timing timing;
 
   // calculateIterationTime(
-  //     iterationDuration, repeatedDuration, scaledActiveTime, startOffset,
+  //     iterationDuration, activeDuration, scaledActiveTime, startOffset,
   //     phase, timing)
 
   // if the scaled active time is null
@@ -125,14 +125,14 @@ TEST(AnimationTimingCalculationsTest, IterationTime) {
   EXPECT_EQ(8, CalculateIterationTime(12, 120, 20, 7,
                                       AnimationEffect::kPhaseActive, timing));
 
-  // Edge case for offset_active_time being within epsilon of (repeated_duration
+  // Edge case for offset_active_time being within epsilon of (active_duration
   // + start_offset). https://crbug.com/962138
   timing.iteration_count = 1;
   const double offset_active_time = 1.3435713716800004;
   const double iteration_duration = 1.3435713716800002;
-  const double repeated_duration = 1.3435713716800002;
+  const double active_duration = 1.3435713716800002;
   EXPECT_NEAR(2.22045e-16,
-              CalculateIterationTime(iteration_duration, repeated_duration,
+              CalculateIterationTime(iteration_duration, active_duration,
                                      offset_active_time, 0,
                                      AnimationEffect::kPhaseActive, timing),
               std::numeric_limits<float>::epsilon());