Auto merge of #26407 - mrobinson:animation-restyle-model, r=jdm

Have animations more closely match the HTML spec

These two commits do two major things:

**Have animations ticks trigger a restyle**: This corrects synchronization issues with animations,
where the values used in layout are out of sync with what is returned by `getComputedStyle`.

**Tick the animation timer in script according to spec**: This greatly reduces the flakiness of
animation and transitions tests.

Fixes #13865.

<!-- Please describe your changes on the following line: -->

---
<!-- 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 #13865

<!-- Either: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because ___

<!-- 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. -->

1 files changed, 64 insertions, 49 deletions

diff --git a/components/script/script_thread.rs b/components/script/script_thread.rs
index 165e526fa76..008274ded47 100644
--- a/components/script/script_thread.rs
+++ b/components/script/script_thread.rs
@@ -136,16 +136,17 @@ use script_traits::CompositorEvent::{
     WheelEvent,
 };
 use script_traits::{
-    CompositorEvent, ConstellationControlMsg, DiscardBrowsingContext, DocumentActivity,
-    EventResult, HistoryEntryReplacement, InitialScriptState, JsEvalResult, LayoutMsg, LoadData,
-    LoadOrigin, MediaSessionActionType, MouseButton, MouseEventType, NewLayoutInfo, Painter,
-    ProgressiveWebMetricType, ScriptMsg, ScriptThreadFactory, ScriptToConstellationChan,
-    StructuredSerializedData, TimerSchedulerMsg, TouchEventType, TouchId,
-    TransitionOrAnimationEventType, UntrustedNodeAddress, UpdatePipelineIdReason,
-    WebrenderIpcSender, WheelDelta, WindowSizeData, WindowSizeType,
+    AnimationTickType, CompositorEvent, ConstellationControlMsg, DiscardBrowsingContext,
+    DocumentActivity, EventResult, HistoryEntryReplacement, InitialScriptState, JsEvalResult,
+    LayoutMsg, LoadData, LoadOrigin, MediaSessionActionType, MouseButton, MouseEventType,
+    NewLayoutInfo, Painter, ProgressiveWebMetricType, ScriptMsg, ScriptThreadFactory,
+    ScriptToConstellationChan, StructuredSerializedData, TimerSchedulerMsg, TouchEventType,
+    TouchId, TransitionOrAnimationEvent, TransitionOrAnimationEventType, UntrustedNodeAddress,
+    UpdatePipelineIdReason, WebrenderIpcSender, WheelDelta, WindowSizeData, WindowSizeType,
 };
 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;
@@ -1497,8 +1498,7 @@ impl ScriptThread {
                         self.handle_set_scroll_state(id, &scroll_state);
                     })
                 },
-                FromConstellation(ConstellationControlMsg::TickAllAnimations(pipeline_id)) => {
-                    // step 7.8
+                FromConstellation(ConstellationControlMsg::TickAllAnimations(pipeline_id, _)) => {
                     if !animation_ticks.contains(&pipeline_id) {
                         animation_ticks.insert(pipeline_id);
                         sequential.push(event);
@@ -1544,6 +1544,9 @@ impl ScriptThread {
             }
         }
 
+        // Step 11.10 from https://html.spec.whatwg.org/multipage/#event-loops.
+        self.update_animations_and_send_events();
+
         // Process the gathered events.
         debug!("Processing events.");
         for msg in sequential {
@@ -1603,7 +1606,7 @@ impl ScriptThread {
 
             let pending_reflows = window.get_pending_reflow_count();
             if pending_reflows > 0 {
-                window.reflow(ReflowGoal::Full, ReflowReason::ImageLoaded);
+                window.reflow(ReflowGoal::Full, ReflowReason::PendingReflow);
             } else {
                 // Reflow currently happens when explicitly invoked by code that
                 // knows the document could have been modified. This should really
@@ -1616,6 +1619,19 @@ impl ScriptThread {
         true
     }
 
+    // Perform step 11.10 from https://html.spec.whatwg.org/multipage/#event-loops.
+    // TODO(mrobinson): This should also update the current animations and send events
+    // to conform to the HTML specification. This might mean having events for rooting
+    // DOM nodes and ultimately all animations living in script.
+    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();
+            }
+        }
+    }
+
     fn categorize_msg(&self, msg: &MixedMessage) -> ScriptThreadEventCategory {
         match *msg {
             MixedMessage::FromConstellation(ref inner_msg) => match *inner_msg {
@@ -1703,7 +1719,7 @@ impl ScriptThread {
                 RemoveHistoryStates(id, ..) => Some(id),
                 FocusIFrame(id, ..) => Some(id),
                 WebDriverScriptCommand(id, ..) => Some(id),
-                TickAllAnimations(id) => Some(id),
+                TickAllAnimations(id, ..) => Some(id),
                 TransitionOrAnimationEvent { .. } => None,
                 WebFontLoaded(id) => Some(id),
                 DispatchIFrameLoadEvent {
@@ -1902,23 +1918,11 @@ impl ScriptThread {
             ConstellationControlMsg::WebDriverScriptCommand(pipeline_id, msg) => {
                 self.handle_webdriver_msg(pipeline_id, msg)
             },
-            ConstellationControlMsg::TickAllAnimations(pipeline_id) => {
-                self.handle_tick_all_animations(pipeline_id)
+            ConstellationControlMsg::TickAllAnimations(pipeline_id, tick_type) => {
+                self.handle_tick_all_animations(pipeline_id, tick_type)
             },
-            ConstellationControlMsg::TransitionOrAnimationEvent {
-                pipeline_id,
-                event_type,
-                node,
-                property_or_animation_name,
-                elapsed_time,
-            } => {
-                self.handle_transition_or_animation_event(
-                    pipeline_id,
-                    event_type,
-                    node,
-                    property_or_animation_name,
-                    elapsed_time,
-                );
+            ConstellationControlMsg::TransitionOrAnimationEvent(ref event) => {
+                self.handle_transition_or_animation_event(event);
             },
             ConstellationControlMsg::WebFontLoaded(pipeline_id) => {
                 self.handle_web_font_loaded(pipeline_id)
@@ -2914,35 +2918,51 @@ impl ScriptThread {
         debug!("Exited script thread.");
     }
 
+    /// Handles animation tick requested during testing.
+    pub fn handle_tick_all_animations_for_testing(id: PipelineId) {
+        SCRIPT_THREAD_ROOT.with(|root| {
+            let script_thread = unsafe { &*root.get().unwrap() };
+            script_thread
+                .handle_tick_all_animations(id, AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS);
+        });
+    }
+
     /// Handles when layout thread finishes all animation in one tick
-    fn handle_tick_all_animations(&self, id: PipelineId) {
+    fn handle_tick_all_animations(&self, id: PipelineId, tick_type: AnimationTickType) {
         let document = match self.documents.borrow().find_document(id) {
             Some(document) => document,
             None => return warn!("Message sent to closed pipeline {}.", id),
         };
-        document.run_the_animation_frame_callbacks();
+        if tick_type.contains(AnimationTickType::REQUEST_ANIMATION_FRAME) {
+            document.run_the_animation_frame_callbacks();
+        }
+        if tick_type.contains(AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS) {
+            match self.animating_nodes.borrow().get(&id) {
+                Some(nodes) => {
+                    for node in nodes.iter() {
+                        node.dirty(NodeDamage::NodeStyleDamaged);
+                    }
+                },
+                None => return,
+            }
+
+            document.window().add_pending_reflow();
+        }
     }
 
     /// Handles firing of transition-related events.
     ///
     /// TODO(mrobinson): Add support for more events.
-    fn handle_transition_or_animation_event(
-        &self,
-        pipeline_id: PipelineId,
-        event_type: TransitionOrAnimationEventType,
-        unsafe_node: UntrustedNodeAddress,
-        property_or_animation_name: String,
-        elapsed_time: f64,
-    ) {
+    fn handle_transition_or_animation_event(&self, event: &TransitionOrAnimationEvent) {
         let js_runtime = self.js_runtime.rt();
-        let node = unsafe { from_untrusted_node_address(js_runtime, unsafe_node) };
+        let node = unsafe { from_untrusted_node_address(js_runtime, event.node) };
 
         // We limit the scope of the borrow here, so that we don't maintain this borrow
         // and then incidentally trigger another layout. That might result in a double
         // mutable borrow of `animating_nodes`.
         {
             let mut animating_nodes = self.animating_nodes.borrow_mut();
-            let nodes = match animating_nodes.get_mut(&pipeline_id) {
+            let nodes = match animating_nodes.get_mut(&event.pipeline_id) {
                 Some(nodes) => nodes,
                 None => {
                     return warn!(
@@ -2964,17 +2984,12 @@ impl ScriptThread {
                 },
             };
 
-            if event_type.finalizes_transition_or_animation() {
+            if event.event_type.finalizes_transition_or_animation() {
                 nodes.remove(node_index);
             }
         }
 
-        // Not quite the right thing - see #13865.
-        if event_type.finalizes_transition_or_animation() {
-            node.dirty(NodeDamage::NodeStyleDamaged);
-        }
-
-        let event_atom = match event_type {
+        let event_atom = match event.event_type {
             TransitionOrAnimationEventType::AnimationEnd => atom!("animationend"),
             TransitionOrAnimationEventType::TransitionCancel => atom!("transitioncancel"),
             TransitionOrAnimationEventType::TransitionEnd => atom!("transitionend"),
@@ -2986,11 +3001,11 @@ impl ScriptThread {
         };
 
         // TODO: Handle pseudo-elements properly
-        let property_or_animation_name = DOMString::from(property_or_animation_name);
-        let elapsed_time = Finite::new(elapsed_time as f32).unwrap();
+        let property_or_animation_name = DOMString::from(event.property_or_animation_name.clone());
+        let elapsed_time = Finite::new(event.elapsed_time as f32).unwrap();
         let window = window_from_node(&*node);
 
-        if event_type.is_transition_event() {
+        if event.event_type.is_transition_event() {
             let event_init = TransitionEventInit {
                 parent,
                 propertyName: property_or_animation_name,