Cleanup handling of animation events triggered by web-animations API

calls

Various web-animations API calls can trigger changes that would not be
experienced during the normal play of a CSS animation. For example,
seek operations can trigger a restart on a finished animation.

Bug: 1059968
Change-Id: I05bf78a3706aad06c014aa0d1dae03de5f18236a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2115880Reviewed-by: Majid Valipour <majidvp@chromium.org>
Commit-Queue: Kevin Ellis <kevers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753624}

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

@@ -223,6 +223,37 @@ AnimationTimeDelta StartTimeFromDelay(double start_delay) {
   return AnimationTimeDelta::FromSecondsD(start_delay < 0 ? -start_delay : 0);
 }
 
+// Timing functions for computing elapsed time of an event.
+
+AnimationTimeDelta IntervalStart(const AnimationEffect& effect) {
+  const double start_delay = effect.SpecifiedTiming().start_delay;
+  const double active_duration = effect.SpecifiedTiming().ActiveDuration();
+  return AnimationTimeDelta::FromSecondsD(
+      std::fmax(std::fmin(-start_delay, active_duration), 0.0));
+}
+
+AnimationTimeDelta IntervalEnd(const AnimationEffect& effect) {
+  const double start_delay = effect.SpecifiedTiming().start_delay;
+  const double end_delay = effect.SpecifiedTiming().end_delay;
+  const double active_duration = effect.SpecifiedTiming().ActiveDuration();
+  const double target_effect_end =
+      std::max(start_delay + active_duration + end_delay, 0.0);
+  return AnimationTimeDelta::FromSecondsD(std::max(
+      std::min(target_effect_end - start_delay, active_duration), 0.0));
+}
+
+AnimationTimeDelta IterationElapsedTime(const AnimationEffect& effect,
+                                        double previous_iteration) {
+  const double current_iteration = effect.CurrentIteration().value();
+  const double iteration_boundary = (previous_iteration > current_iteration)
+                                        ? current_iteration + 1
+                                        : current_iteration;
+  const double iteration_start = effect.SpecifiedTiming().iteration_start;
+  const AnimationTimeDelta iteration_duration =
+      effect.SpecifiedTiming().iteration_duration.value();
+  return iteration_duration * (iteration_boundary - iteration_start);
+}
+
 }  // namespace
 
 CSSAnimations::CSSAnimations() = default;
@@ -1181,54 +1212,68 @@ void CSSAnimations::AnimationEventDelegate::OnEventCondition(
   const base::Optional<double> current_iteration =
       animation_node.CurrentIteration();
 
-  if (previous_phase_ != current_phase &&
-      (current_phase == Timing::kPhaseActive ||
-       current_phase == Timing::kPhaseAfter) &&
-      (previous_phase_ == Timing::kPhaseNone ||
-       previous_phase_ == Timing::kPhaseBefore)) {
-    const double start_delay = animation_node.SpecifiedTiming().start_delay;
-    const auto& elapsed_time =
-        AnimationTimeDelta::FromSecondsD(start_delay < 0 ? -start_delay : 0);
+  // See http://drafts.csswg.org/css-animations-2/#event-dispatch
+  // When multiple events are dispatched for a single phase transition,
+  // the animationstart event is to be dispatched before the animationend
+  // event.
+
+  // The following phase transitions trigger an animationstart event:
+  //   idle or before --> active or after
+  //   after --> active or before
+  const bool phase_change = previous_phase_ != current_phase;
+  const bool was_idle_or_before = (previous_phase_ == Timing::kPhaseNone ||
+                                   previous_phase_ == Timing::kPhaseBefore);
+  const bool is_active_or_after = (current_phase == Timing::kPhaseActive ||
+                                   current_phase == Timing::kPhaseAfter);
+  const bool is_active_or_before = (current_phase == Timing::kPhaseActive ||
+                                    current_phase == Timing::kPhaseBefore);
+  const bool was_after = (previous_phase_ == Timing::kPhaseAfter);
+  if (phase_change && ((was_idle_or_before && is_active_or_after) ||
+                       (was_after && is_active_or_before))) {
+    AnimationTimeDelta elapsed_time =
+        was_after ? IntervalEnd(animation_node) : IntervalStart(animation_node);
     MaybeDispatch(Document::kAnimationStartListener,
                   event_type_names::kAnimationstart, elapsed_time);
   }
 
-  if (current_phase == Timing::kPhaseActive &&
-      previous_phase_ == current_phase &&
+  // The following phase transitions trigger an animationend event:
+  //   idle, before or active--> after
+  //   active or after--> before
+  const bool was_active_or_after = (previous_phase_ == Timing::kPhaseActive ||
+                                    previous_phase_ == Timing::kPhaseAfter);
+  const bool is_after = (current_phase == Timing::kPhaseAfter);
+  const bool is_before = (current_phase == Timing::kPhaseBefore);
+  if (phase_change && (is_after || (was_active_or_after && is_before))) {
+    AnimationTimeDelta elapsed_time =
+        is_after ? IntervalEnd(animation_node) : IntervalStart(animation_node);
+    MaybeDispatch(Document::kAnimationEndListener,
+                  event_type_names::kAnimationend, elapsed_time);
+  }
+
+  if (phase_change && current_phase == Timing::kPhaseNone) {
+    // TODO(crbug.com/1059968): Determine if animation direction or playback
+    // rate factor into the calculation of the elapsed time.
+    double cancel_time = animation_node.GetCancelTime();
+    MaybeDispatch(Document::kAnimationCancelListener,
+                  event_type_names::kAnimationcancel,
+                  AnimationTimeDelta::FromSecondsD(cancel_time));
+  }
+
+  if (!phase_change && current_phase == Timing::kPhaseActive &&
       previous_iteration_ != current_iteration) {
     // We fire only a single event for all iterations that terminate
     // between a single pair of samples. See http://crbug.com/275263. For
     // compatibility with the existing implementation, this event uses
     // the elapsedTime for the first iteration in question.
-    DCHECK(previous_iteration_);
+    DCHECK(previous_iteration_ && current_iteration);
     const AnimationTimeDelta elapsed_time =
-        animation_node.SpecifiedTiming().iteration_duration.value() *
-        (previous_iteration_.value() + 1);
+        IterationElapsedTime(animation_node, previous_iteration_.value());
     MaybeDispatch(Document::kAnimationIterationListener,
                   event_type_names::kAnimationiteration, elapsed_time);
   }
 
   previous_iteration_ = current_iteration;
-
-  if (previous_phase_ == current_phase)
-    return;
   previous_phase_ = current_phase;
-
-  if (current_phase == Timing::kPhaseAfter) {
-    MaybeDispatch(Document::kAnimationEndListener,
-                  event_type_names::kAnimationend,
-                  AnimationTimeDelta::FromSecondsD(
-                      animation_node.SpecifiedTiming().ActiveDuration()));
-  }
-
-  if (current_phase == Timing::kPhaseNone) {
-    // TODO(crbug.com/1059968): Determine if animation direction or playback
-    // rate factor into the calculation of the elapsed time.
-    double cancel_time = animation_node.GetCancelTime();
-    MaybeDispatch(Document::kAnimationCancelListener,
-                  event_type_names::kAnimationcancel,
-                  AnimationTimeDelta::FromSecondsD(cancel_time));
-  }
 }
 
 void CSSAnimations::AnimationEventDelegate::Trace(Visitor* visitor) {

third_party/blink/web_tests/TestExpectations

@@ -3668,8 +3668,6 @@ crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftest
 crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-flex-basis-content-002b.html [ Failure ]
 crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/flexbox/flexbox-flex-basis-content-001b.html [ Failure ]
 crbug.com/626703 [ Win7 ] external/wpt/css/css-fonts/variations/font-opentype-collections.html [ Timeout ]
-crbug.com/626703 external/wpt/css/css-animations/animation-delay-010.html [ Failure ]
-crbug.com/626703 virtual/threaded/external/wpt/css/css-animations/animation-delay-010.html [ Failure ]
 crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/counter-styles-3/system-numeric.html [ Failure ]
 crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/counter-styles-3/descriptor-suffix-invalid.html [ Failure ]
 crbug.com/626703 external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/counter-styles-3/descriptor-range.html [ Failure ]

third_party/blink/web_tests/animations/events/negative-delay-events.html

@@ -31,7 +31,8 @@
 async_test(t => {
   var count = 0;
   document.addEventListener('animationstart', t.step_func((event) => {
-      var pass = event.elapsedTime === [0, 0.05, 0.15][count++];
+      // Elapsed time caps at the animation duration.
+      var pass = event.elapsedTime === [0, 0.05, 0.1][count++];
       assert_true(pass, event.target.id + ': Start event: elapsedTime=' + event.elapsedTime);
   }));
 

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
-FAIL After replacing a finished animation's effect with a longer one it fires an animationstart event assert_true: Timed out waiting for animationstart expected true got false
-PASS Replacing the effect of a CSSAnimation causes subsequent changes to corresponding animation-* properties to be ignored
-Harness: the test ran to completion.
-

third_party/blink/web_tests/external/wpt/css/css-animations/event-dispatch.tentative-expected.txt

@@ -6,14 +6,14 @@ PASS Before -> After
 PASS Active -> Idle, display: none
 FAIL Active -> Idle, setting Animation.timeline = null promise_test: Unhandled rejection with value: object "TypeError: Cannot assign to read only property 'timeline' of object '#<CSSAnimation>'"
 PASS Active -> Idle, calling Animation.cancel()
-FAIL Active -> Before assert_true: Timed out waiting for animationend expected true got false
+PASS Active -> Before
 PASS Active -> After
-FAIL After -> Before assert_true: Timed out waiting for animationstart, animationend expected true got false
-FAIL After -> Active assert_true: Timed out waiting for animationstart expected true got false
-FAIL Active -> Active (forwards) assert_equals: expected 200 but got 100
-FAIL Active -> Active (backwards) assert_true: Timed out waiting for animationstart expected true got false
+PASS After -> Before
+PASS After -> Active
+PASS Active -> Active (forwards)
+PASS Active -> Active (backwards)
 PASS Active -> Idle -> Active: animationstart is fired by restarting animation
-FAIL Negative playbackRate sanity test(Before -> Active -> Before) assert_true: Timed out waiting for animationstart expected true got false
+PASS Negative playbackRate sanity test(Before -> Active -> Before)
 PASS Redundant change, before -> active, then back
 PASS Redundant change, before -> after, then back
 PASS Redundant change, active -> before, then back