Auto merge of #26594 - mrobinson:animationiteration, r=jdm

Implement animationiteration event

This event is triggered when an animation iterates. This change also
moves iteration out of style calculation to an "update animations" which
is the next part of having animation event handling match the HTML spec.

This change causes a few more tests to pass. Some of the other tests which
exercise this functionality require `animationstart` events.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #___ (GitHub issue number if applicable)
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

12 files changed, 93 insertions, 80 deletions

diff --git a/components/script/animations.rs b/components/script/animations.rs
index 91946853f22..0c3f1c68206 100644
--- a/components/script/animations.rs
+++ b/components/script/animations.rs
@@ -35,6 +35,30 @@ impl Animations {
         }
     }
 
+    pub(crate) fn update_for_new_timeline_value(
+        &mut self,
+        window: &Window,
+        now: f64,
+    ) -> AnimationsUpdate {
+        let mut update = AnimationsUpdate::new(window.pipeline_id());
+        let mut sets = self.sets.write();
+
+        for set in sets.values_mut() {
+            // When necessary, iterate our running animations to the next iteration.
+            for animation in set.animations.iter_mut() {
+                if animation.iterate_if_necessary(now) {
+                    update.add_event(
+                        animation.node,
+                        animation.name.to_string(),
+                        TransitionOrAnimationEventType::AnimationIteration,
+                        animation.active_duration(),
+                    );
+                }
+            }
+        }
+        update
+    }
+
     /// Processes any new animations that were discovered after reflow. Collect messages
     /// that trigger events for any animations that changed state.
     /// TODO(mrobinson): The specification dictates that this should happen before reflow.
@@ -255,6 +279,9 @@ pub enum TransitionOrAnimationEventType {
     TransitionCancel,
     /// "The animationend event occurs when the animation finishes"
     AnimationEnd,
+    /// "The animationiteration event occurs at the end of each iteration of an
+    /// animation, except when an animationend event would fire at the same time."
+    AnimationIteration,
 }
 
 impl TransitionOrAnimationEventType {
@@ -263,7 +290,7 @@ impl TransitionOrAnimationEventType {
     pub fn finalizes_transition_or_animation(&self) -> bool {
         match *self {
             Self::TransitionEnd | Self::TransitionCancel | Self::AnimationEnd => true,
-            Self::TransitionRun => false,
+            Self::TransitionRun | Self::AnimationIteration => false,
         }
     }
 
@@ -271,7 +298,7 @@ impl TransitionOrAnimationEventType {
     pub fn is_transition_event(&self) -> bool {
         match *self {
             Self::TransitionRun | Self::TransitionEnd | Self::TransitionCancel => true,
-            Self::AnimationEnd => false,
+            Self::AnimationEnd | Self::AnimationIteration => false,
         }
     }
 }
diff --git a/components/script/dom/document.rs b/components/script/dom/document.rs
index dfdfd53bd28..b0271e159de 100644
--- a/components/script/dom/document.rs
+++ b/components/script/dom/document.rs
@@ -3750,12 +3750,26 @@ impl Document {
             .collect()
     }
 
-    pub(crate) fn advance_animation_timeline_for_testing(&self, delta: f64) {
+    pub(crate) fn advance_animation_timeline_for_testing(&self, delta: f64) -> AnimationsUpdate {
         self.animation_timeline.borrow_mut().advance_specific(delta);
+        let current_timeline_value = self.current_animation_timeline_value();
+        self.animations
+            .borrow_mut()
+            .update_for_new_timeline_value(&self.window, current_timeline_value)
     }
 
-    pub(crate) fn update_animation_timeline(&self) {
-        self.animation_timeline.borrow_mut().update();
+    pub(crate) fn update_animation_timeline(&self) -> AnimationsUpdate {
+        // Only update the time if it isn't being managed by a test.
+        if !pref!(layout.animations.test.enabled) {
+            self.animation_timeline.borrow_mut().update();
+        }
+
+        // We still want to update the animations, because our timeline
+        // value might have been advanced previously via the TestBinding.
+        let current_timeline_value = self.current_animation_timeline_value();
+        self.animations
+            .borrow_mut()
+            .update_for_new_timeline_value(&self.window, current_timeline_value)
     }
 
     pub(crate) fn current_animation_timeline_value(&self) -> f64 {
@@ -3766,7 +3780,7 @@ impl Document {
         self.animations.borrow()
     }
 
-    pub(crate) fn update_animations(&self) -> AnimationsUpdate {
+    pub(crate) fn update_animations_post_reflow(&self) -> AnimationsUpdate {
         self.animations
             .borrow_mut()
             .do_post_reflow_update(&self.window, self.current_animation_timeline_value())
diff --git a/components/script/dom/macros.rs b/components/script/dom/macros.rs
index 871e831dc82..5a32b36996b 100644
--- a/components/script/dom/macros.rs
+++ b/components/script/dom/macros.rs
@@ -443,6 +443,7 @@ macro_rules! global_event_handlers(
     (NoOnload) => (
         event_handler!(abort, GetOnabort, SetOnabort);
         event_handler!(animationend, GetOnanimationend, SetOnanimationend);
+        event_handler!(animationiteration, GetOnanimationiteration, SetOnanimationiteration);
         event_handler!(cancel, GetOncancel, SetOncancel);
         event_handler!(canplay, GetOncanplay, SetOncanplay);
         event_handler!(canplaythrough, GetOncanplaythrough, SetOncanplaythrough);
diff --git a/components/script/dom/webidls/EventHandler.webidl b/components/script/dom/webidls/EventHandler.webidl
index b5617564fbc..57138967792 100644
--- a/components/script/dom/webidls/EventHandler.webidl
+++ b/components/script/dom/webidls/EventHandler.webidl
@@ -93,6 +93,7 @@ interface mixin GlobalEventHandlers {
 // https://drafts.csswg.org/css-animations/#interface-globaleventhandlers-idl
 partial interface mixin GlobalEventHandlers {
            attribute EventHandler onanimationend;
+           attribute EventHandler onanimationiteration;
 };
 
 // https://drafts.csswg.org/css-transitions/#interface-globaleventhandlers-idl
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs
index 946bcdcfea9..a6965155588 100644
--- a/components/script/dom/window.rs
+++ b/components/script/dom/window.rs
@@ -1578,10 +1578,15 @@ impl Window {
 
     /// Prepares to tick animations and then does a reflow which also advances the
     /// layout animation clock.
-    pub fn advance_animation_clock(&self, delta: i32) {
+    #[allow(unsafe_code)]
+    pub fn advance_animation_clock(&self, delta_ms: i32) {
         let pipeline_id = self.upcast::<GlobalScope>().pipeline_id();
-        self.Document()
-            .advance_animation_timeline_for_testing(delta as f64 / 1000.);
+        let update = self
+            .Document()
+            .advance_animation_timeline_for_testing(delta_ms as f64 / 1000.);
+        unsafe {
+            ScriptThread::process_animations_update(update);
+        }
         ScriptThread::handle_tick_all_animations_for_testing(pipeline_id);
     }
 
@@ -1747,7 +1752,7 @@ impl Window {
             }
         }
 
-        let update = document.update_animations();
+        let update = document.update_animations_post_reflow();
         unsafe {
             ScriptThread::process_animations_update(update);
         }
diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 2107cd070cd..d061dc7d32e 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -149,7 +149,6 @@ use script_traits::{
 };
 use servo_atoms::Atom;
 use servo_config::opts;
-use servo_config::pref;
 use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
 use std::borrow::Cow;
 use std::cell::Cell;
@@ -1631,19 +1630,17 @@ impl ScriptThread {
     // TODO(mrobinson): This should also update the current animations to conform to
     // the HTML specification.
     fn update_animations_and_send_events(&self) {
-        for (_, document) in self.documents.borrow().iter() {
-            // Only update the time if it isn't being managed by a test.
-            if !pref!(layout.animations.test.enabled) {
-                document.update_animation_timeline();
-            }
-        }
-
         // We remove the events because handling these events might trigger
         // a reflow which might want to add more events to the queue.
         let events = self.animation_events.replace(Vec::new());
         for event in events.into_iter() {
             self.handle_transition_or_animation_event(&event);
         }
+
+        for (_, document) in self.documents.borrow().iter() {
+            let update = document.update_animation_timeline();
+            unsafe { ScriptThread::process_animations_update(update) };
+        }
     }
 
     fn categorize_msg(&self, msg: &MixedMessage) -> ScriptThreadEventCategory {
@@ -2947,6 +2944,7 @@ impl ScriptThread {
 
         let event_atom = match event.event_type {
             TransitionOrAnimationEventType::AnimationEnd => atom!("animationend"),
+            TransitionOrAnimationEventType::AnimationIteration => atom!("animationiteration"),
             TransitionOrAnimationEventType::TransitionCancel => atom!("transitioncancel"),
             TransitionOrAnimationEventType::TransitionEnd => atom!("transitionend"),
             TransitionOrAnimationEventType::TransitionRun => atom!("transitionrun"),
diff --git a/components/style/animation.rs b/components/style/animation.rs
index 1ad126d341a..cc198e0491f 100644
--- a/components/style/animation.rs
+++ b/components/style/animation.rs
@@ -156,12 +156,10 @@ pub enum AnimationState {
 /// have to keep track the current iteration and the max iteration count.
 #[derive(Clone, Debug, MallocSizeOf)]
 pub enum KeyframesIterationState {
-    /// Infinite iterations, so no need to track a state.
-    Infinite,
+    /// Infinite iterations with the current iteration count.
+    Infinite(f64),
     /// Current and max iterations.
-    /// TODO: Everything else in this file uses f64, so perhaps this should
-    /// be as well.
-    Finite(f32, f32),
+    Finite(f64, f64),
 }
 
 /// A CSS Animation
@@ -227,26 +225,26 @@ impl Animation {
 
     /// Given the current time, advances this animation to the next iteration,
     /// updates times, and then toggles the direction if appropriate. Otherwise
-    /// does nothing.
-    pub fn iterate_if_necessary(&mut self, time: f64) {
+    /// does nothing. Returns true if this animation has iterated.
+    pub fn iterate_if_necessary(&mut self, time: f64) -> bool {
         if !self.iteration_over(time) {
-            return;
+            return false;
         }
 
         // Only iterate animations that are currently running.
         if self.state != AnimationState::Running {
-            return;
+            return false;
         }
 
         if let KeyframesIterationState::Finite(ref mut current, max) = self.iteration_state {
-            // If we are already on the final iteration, just exit now.
-            // NB: This prevent us from updating the direction, which might be
-            // needed for the correct handling of animation-fill-mode.
-            if (max - *current) <= 1.0 {
-                return;
+            // If we are already on the final iteration, just exit now. This prevents
+            // us from updating the direction, which might be needed for the correct
+            // handling of animation-fill-mode and also firing animationiteration events
+            // at the end of animations.
+            *current = (*current + 1.).min(max);
+            if *current == max {
+                return false;
             }
-
-            *current += 1.0;
         }
 
         // Update the next iteration direction if applicable.
@@ -262,6 +260,8 @@ impl Animation {
             },
             _ => {},
         }
+
+        true
     }
 
     fn iteration_over(&self, time: f64) -> bool {
@@ -285,8 +285,8 @@ impl Animation {
         // If we have a limited number of iterations and we cannot advance to another
         // iteration, then we have ended.
         return match self.iteration_state {
-            KeyframesIterationState::Finite(current, max) if (max - current) <= 1.0 => true,
-            KeyframesIterationState::Finite(..) | KeyframesIterationState::Infinite => false,
+            KeyframesIterationState::Finite(current, max) => max == current,
+            KeyframesIterationState::Infinite(..) => false,
         };
     }
 
@@ -347,13 +347,11 @@ impl Animation {
     }
 
     /// Calculate the active-duration of this animation according to
-    /// https://drafts.csswg.org/css-animations/#active-duration. active-duration
-    /// is not really meaningful for infinite animations so we just return 0
-    /// here in that case.
+    /// https://drafts.csswg.org/css-animations/#active-duration.
     pub fn active_duration(&self) -> f64 {
         match self.iteration_state {
-            KeyframesIterationState::Finite(_, max) => self.duration * (max as f64),
-            KeyframesIterationState::Infinite => 0.,
+            KeyframesIterationState::Finite(current, _) |
+            KeyframesIterationState::Infinite(current) => self.duration * current,
         }
     }
 
@@ -784,11 +782,6 @@ impl ElementAnimationSet {
         }
 
         maybe_start_animations(element, &context, &new_style, self);
-
-        // When necessary, iterate our running animations to the next iteration.
-        for animation in self.animations.iter_mut() {
-            animation.iterate_if_necessary(context.current_time_for_animations);
-        }
     }
 
     /// Update our transitions given a new style, canceling or starting new animations
@@ -1039,8 +1032,8 @@ pub fn maybe_start_animations<E>(
         let delay = box_style.animation_delay_mod(i).seconds();
         let animation_start = context.current_time_for_animations + delay as f64;
         let iteration_state = match box_style.animation_iteration_count_mod(i) {
-            AnimationIterationCount::Infinite => KeyframesIterationState::Infinite,
-            AnimationIterationCount::Number(n) => KeyframesIterationState::Finite(0.0, n),
+            AnimationIterationCount::Infinite => KeyframesIterationState::Infinite(0.0),
+            AnimationIterationCount::Number(n) => KeyframesIterationState::Finite(0.0, n.into()),
         };
 
         let animation_direction = box_style.animation_direction_mod(i);
diff --git a/tests/wpt/metadata-layout-2020/css/css-animations/animationevent-types.html.ini b/tests/wpt/metadata-layout-2020/css/css-animations/animationevent-types.html.ini
index d768d036d41..256ea2378e5 100644
--- a/tests/wpt/metadata-layout-2020/css/css-animations/animationevent-types.html.ini
+++ b/tests/wpt/metadata-layout-2020/css/css-animations/animationevent-types.html.ini
@@ -1,8 +1,5 @@
 [animationevent-types.html]
   expected: TIMEOUT
-  [animationiteration event is instanceof AnimationEvent]
-    expected: TIMEOUT
-
   [animationstart event is instanceof AnimationEvent]
     expected: TIMEOUT
 
diff --git a/tests/wpt/metadata-layout-2020/css/css-animations/idlharness.html.ini b/tests/wpt/metadata-layout-2020/css/css-animations/idlharness.html.ini
index 26b53e03eec..81fa027d83f 100644
--- a/tests/wpt/metadata-layout-2020/css/css-animations/idlharness.html.ini
+++ b/tests/wpt/metadata-layout-2020/css/css-animations/idlharness.html.ini
@@ -1,7 +1,4 @@
 [idlharness.html]
-  [HTMLElement interface: attribute onanimationiteration]
-    expected: FAIL
-
   [HTMLElement interface: attribute onanimationstart]
     expected: FAIL
 
@@ -20,18 +17,12 @@
   [Window interface: attribute onanimationstart]
     expected: FAIL
 
-  [Window interface: attribute onanimationiteration]
-    expected: FAIL
-
   [Document interface: attribute onanimationcancel]
     expected: FAIL
 
   [CSSKeyframeRule interface: attribute style]
     expected: FAIL
 
-  [Document interface: attribute onanimationiteration]
-    expected: FAIL
-
   [HTMLElement interface: attribute onanimationcancel]
     expected: FAIL
 
diff --git a/tests/wpt/metadata/css/css-animations/animationevent-types.html.ini b/tests/wpt/metadata/css/css-animations/animationevent-types.html.ini
index 70ed3998119..944fcee5779 100644
--- a/tests/wpt/metadata/css/css-animations/animationevent-types.html.ini
+++ b/tests/wpt/metadata/css/css-animations/animationevent-types.html.ini
@@ -1,9 +1,6 @@
 [animationevent-types.html]
   bug: https://github.com/servo/servo/issues/21564
   expected: TIMEOUT
-  [animationiteration event is instanceof AnimationEvent]
-    expected: TIMEOUT
-
   [animationstart event is instanceof AnimationEvent]
     expected: TIMEOUT
 
diff --git a/tests/wpt/metadata/css/css-animations/idlharness.html.ini b/tests/wpt/metadata/css/css-animations/idlharness.html.ini
index 6b8f7e14289..4eff1e56aa2 100644
--- a/tests/wpt/metadata/css/css-animations/idlharness.html.ini
+++ b/tests/wpt/metadata/css/css-animations/idlharness.html.ini
@@ -1,16 +1,10 @@
 [idlharness.html]
-  [Document interface: attribute onanimationiteration]
-    expected: FAIL
-
   [CSSKeyframeRule interface: attribute style]
     expected: FAIL
 
   [Document interface: attribute onanimationstart]
     expected: FAIL
 
-  [HTMLElement interface: attribute onanimationiteration]
-    expected: FAIL
-
   [Window interface: attribute onanimationcancel]
     expected: FAIL
 
@@ -29,9 +23,6 @@
   [HTMLElement interface: attribute onanimationcancel]
     expected: FAIL
 
-  [Window interface: attribute onanimationiteration]
-    expected: FAIL
-
   [CSSKeyframeRule interface: keyframes.cssRules[0\] must inherit property "keyText" with the proper type]
     expected: FAIL
 
diff --git a/tests/wpt/metadata/dom/events/webkit-animation-iteration-event.html.ini b/tests/wpt/metadata/dom/events/webkit-animation-iteration-event.html.ini
index 657029024b6..cc326be27bd 100644
--- a/tests/wpt/metadata/dom/events/webkit-animation-iteration-event.html.ini
+++ b/tests/wpt/metadata/dom/events/webkit-animation-iteration-event.html.ini
@@ -1,9 +1,7 @@
 [webkit-animation-iteration-event.html]
+  expected: TIMEOUT
   [webkitAnimationIteration event listener is case sensitive]
-    expected: FAIL
-
-  [dispatchEvent of a webkitAnimationIteration event does not trigger an unprefixed event handler or listener]
-    expected: FAIL
+    expected: NOTRUN
 
   [onwebkitanimationiteration event handler should trigger for an animation]
     expected: FAIL
@@ -12,10 +10,10 @@
     expected: FAIL
 
   [webkitAnimationIteration event listener should trigger for an animation]
-    expected: FAIL
+    expected: TIMEOUT
 
   [webkitAnimationIteration event listener should not trigger if an unprefixed listener also exists]
-    expected: FAIL
+    expected: NOTRUN
 
   [onwebkitanimationiteration event handler should not trigger if an unprefixed event handler also exists]
     expected: FAIL
@@ -24,10 +22,10 @@
     expected: FAIL
 
   [event types for prefixed and unprefixed animationiteration event listeners should be named appropriately]
-    expected: FAIL
+    expected: NOTRUN
 
   [webkitAnimationIteration event listener should not trigger if an unprefixed event handler also exists]
-    expected: FAIL
+    expected: NOTRUN
 
   [onanimationiteration and onwebkitanimationiteration are not aliases]
     expected: FAIL