Have animations more closely match the HTML spec

components/compositing/compositor.rs

@@ -1042,20 +1042,22 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
         let animation_callbacks_running = self
             .pipeline_details(pipeline_id)
             .animation_callbacks_running;
-        if animation_callbacks_running {
-            let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Script);
-            if let Err(e) = self.constellation_chan.send(msg) {
-                warn!("Sending tick to constellation failed ({:?}).", e);
-            }
+        let animations_running = self.pipeline_details(pipeline_id).animations_running;
+        if !animation_callbacks_running && !animations_running {
+            return;
         }
 
-        // We may need to tick animations in layout. (See #12749.)
-        let animations_running = self.pipeline_details(pipeline_id).animations_running;
+        let mut tick_type = AnimationTickType::empty();
         if animations_running {
-            let msg = ConstellationMsg::TickAnimation(pipeline_id, AnimationTickType::Layout);
-            if let Err(e) = self.constellation_chan.send(msg) {
-                warn!("Sending tick to constellation failed ({:?}).", e);
-            }
+            tick_type.insert(AnimationTickType::CSS_ANIMATIONS_AND_TRANSITIONS);
+        }
+        if animation_callbacks_running {
+            tick_type.insert(AnimationTickType::REQUEST_ANIMATION_FRAME);
+        }
+
+        let msg = ConstellationMsg::TickAnimation(pipeline_id, tick_type);
+        if let Err(e) = self.constellation_chan.send(msg) {
+            warn!("Sending tick to constellation failed ({:?}).", e);
         }
     }
 

components/constellation/constellation.rs

@@ -3458,27 +3458,13 @@ where
     }
 
     fn handle_tick_animation(&mut self, pipeline_id: PipelineId, tick_type: AnimationTickType) {
-        let result = match tick_type {
-            AnimationTickType::Script => {
-                let msg = ConstellationControlMsg::TickAllAnimations(pipeline_id);
-                match self.pipelines.get(&pipeline_id) {
-                    Some(pipeline) => pipeline.event_loop.send(msg),
-                    None => {
-                        return warn!("Pipeline {:?} got script tick after closure.", pipeline_id);
-                    },
-                }
-            },
-            AnimationTickType::Layout => match self.pipelines.get(&pipeline_id) {
-                Some(pipeline) => {
-                    let msg = LayoutControlMsg::TickAnimations(pipeline.load_data.url.origin());
-                    pipeline.layout_chan.send(msg)
-                },
-                None => {
-                    return warn!("Pipeline {:?} got layout tick after closure.", pipeline_id);
-                },
-            },
+        let pipeline = match self.pipelines.get(&pipeline_id) {
+            Some(pipeline) => pipeline,
+            None => return warn!("Pipeline {:?} got script tick after closure.", pipeline_id),
         };
-        if let Err(e) = result {
+
+        let message = ConstellationControlMsg::TickAllAnimations(pipeline_id, tick_type);
+        if let Err(e) = pipeline.event_loop.send(message) {
             self.handle_send_error(pipeline_id, e);
         }
     }

components/layout/animation.rs

@@ -4,7 +4,6 @@
 
 //! CSS transitions and animations.
 
-use crate::context::LayoutContext;
 use crate::display_list::items::OpaqueNode;
 use crate::flow::{Flow, GetBaseFlow};
 use crate::opaque_node::OpaqueNodeMethods;
@@ -14,56 +13,29 @@ use msg::constellation_msg::PipelineId;
 use script_traits::UntrustedNodeAddress;
 use script_traits::{
     AnimationState, ConstellationControlMsg, LayoutMsg as ConstellationMsg,
-    TransitionOrAnimationEventType,
+    TransitionOrAnimationEvent, TransitionOrAnimationEventType,
 };
-use servo_arc::Arc;
-use style::animation::{update_style_for_animation, Animation, ElementAnimationState};
-use style::dom::TElement;
-use style::font_metrics::ServoMetricsProvider;
-use style::selector_parser::RestyleDamage;
-use style::timer::Timer;
+use style::animation::{Animation, ElementAnimationState};
 
-/// Collect newly animating nodes, which is used by the script process during
-/// forced, synchronous reflows to root DOM nodes for the duration of their
-/// animations or transitions.
-pub fn collect_newly_animating_nodes(
-    animation_states: &FxHashMap<OpaqueNode, ElementAnimationState>,
-    mut out: Option<&mut Vec<UntrustedNodeAddress>>,
-) {
-    // This extends the output vector with an iterator that contains a copy of the node
-    // address for every new animation. This is a bit goofy, but the script thread
-    // currently stores a rooted node for every property that is transitioning.
-    if let Some(ref mut out) = out {
-        out.extend(animation_states.iter().flat_map(|(node, state)| {
-            std::iter::repeat(node.to_untrusted_node_address()).take(state.new_animations.len())
-        }));
-    }
-}
-
-/// Processes any new animations that were discovered after style recalculation. Also
-/// finish any animations that have completed, inserting them into `finished_animations`.
-pub fn update_animation_states(
+/// Processes any new animations that were discovered after style recalculation and
+/// remove animations for any disconnected nodes. Send messages that trigger events
+/// for any events that changed state.
+pub fn do_post_style_animations_update(
     constellation_chan: &IpcSender<ConstellationMsg>,
     script_chan: &IpcSender<ConstellationControlMsg>,
     animation_states: &mut FxHashMap<OpaqueNode, ElementAnimationState>,
-    invalid_nodes: FxHashSet<OpaqueNode>,
     pipeline_id: PipelineId,
-    timer: &Timer,
+    now: f64,
+    out: &mut Vec<UntrustedNodeAddress>,
+    root_flow: &mut dyn Flow,
 ) {
     let had_running_animations = animation_states
         .values()
         .any(|state| !state.running_animations.is_empty());
 
-    // Cancel all animations on any invalid nodes. These entries will later
-    // be removed from the list of states, because their states will become
-    // empty.
-    for node in &invalid_nodes {
-        if let Some(mut state) = animation_states.remove(node) {
-            state.cancel_all_animations();
-        }
-    }
+    cancel_animations_for_disconnected_nodes(animation_states, root_flow);
+    collect_newly_animating_nodes(animation_states, out);
 
-    let now = timer.seconds();
     let mut have_running_animations = false;
     for (node, animation_state) in animation_states.iter_mut() {
         update_animation_state(script_chan, animation_state, pipeline_id, now, *node);
@@ -89,7 +61,50 @@ pub fn update_animation_states(
         .unwrap();
 }
 
-pub fn update_animation_state(
+/// Collect newly animating nodes, which is used by the script process during
+/// forced, synchronous reflows to root DOM nodes for the duration of their
+/// animations or transitions.
+pub fn collect_newly_animating_nodes(
+    animation_states: &FxHashMap<OpaqueNode, ElementAnimationState>,
+    out: &mut Vec<UntrustedNodeAddress>,
+) {
+    // This extends the output vector with an iterator that contains a copy of the node
+    // address for every new animation. This is a bit goofy, but the script thread
+    // currently stores a rooted node for every property that is transitioning.
+    out.extend(animation_states.iter().flat_map(|(node, state)| {
+        std::iter::repeat(node.to_untrusted_node_address()).take(state.new_animations.len())
+    }));
+}
+
+/// Cancel animations for any nodes which have been removed from the DOM or are display:none.
+/// We detect this by looking for nodes that are used in the flow tree.
+/// TODO(mrobinson): We should look into a way of doing this during flow tree construction.
+/// This also doesn't yet handles nodes that have been reparented.
+pub fn cancel_animations_for_disconnected_nodes(
+    animation_states: &mut FxHashMap<OpaqueNode, ElementAnimationState>,
+    root_flow: &mut dyn Flow,
+) {
+    // Assume all nodes have been removed until proven otherwise.
+    let mut invalid_nodes: FxHashSet<OpaqueNode> = animation_states.keys().cloned().collect();
+    fn traverse_flow(flow: &mut dyn Flow, invalid_nodes: &mut FxHashSet<OpaqueNode>) {
+        flow.mutate_fragments(&mut |fragment| {
+            invalid_nodes.remove(&fragment.node);
+        });
+        for kid in flow.mut_base().children.iter_mut() {
+            traverse_flow(kid, invalid_nodes)
+        }
+    }
+    traverse_flow(root_flow, &mut invalid_nodes);
+
+    // Cancel animations for any nodes that are no longer in the flow tree.
+    for node in &invalid_nodes {
+        if let Some(state) = animation_states.get_mut(node) {
+            state.cancel_all_animations();
+        }
+    }
+}
+
+fn update_animation_state(
     script_channel: &IpcSender<ConstellationControlMsg>,
     animation_state: &mut ElementAnimationState,
     pipeline_id: PipelineId,
@@ -105,13 +120,15 @@ pub fn update_animation_state(
         };
 
         script_channel
-            .send(ConstellationControlMsg::TransitionOrAnimationEvent {
-                pipeline_id,
-                event_type,
-                node: node.to_untrusted_node_address(),
-                property_or_animation_name,
-                elapsed_time,
-            })
+            .send(ConstellationControlMsg::TransitionOrAnimationEvent(
+                TransitionOrAnimationEvent {
+                    pipeline_id,
+                    event_type,
+                    node: node.to_untrusted_node_address(),
+                    property_or_animation_name,
+                    elapsed_time,
+                },
+            ))
             .unwrap()
     };
 
@@ -122,36 +139,28 @@ pub fn update_animation_state(
 
 /// Walk through the list of running animations and remove all of the ones that
 /// have ended.
-pub fn handle_running_animations(
+fn handle_running_animations(
     animation_state: &mut ElementAnimationState,
     now: f64,
     mut send_event: impl FnMut(&Animation, TransitionOrAnimationEventType, f64),
 ) {
+    if animation_state.running_animations.is_empty() {
+        return;
+    }
+
     let mut running_animations =
         std::mem::replace(&mut animation_state.running_animations, Vec::new());
-    for mut running_animation in running_animations.drain(..) {
-        let still_running = match running_animation {
-            Animation::Transition(_, started_at, ref property_animation) => {
-                now < started_at + (property_animation.duration)
-            },
-            Animation::Keyframes(_, _, _, ref mut state) => {
-                // This animation is still running, or we need to keep
-                // iterating.
-                now < state.started_at + state.duration || state.tick()
-            },
-        };
-
+    for running_animation in running_animations.drain(..) {
         // If the animation is still running, add it back to the list of running animations.
-        if still_running {
+        if !running_animation.has_ended(now) {
             animation_state.running_animations.push(running_animation);
         } else {
-            debug!("Finishing transition: {:?}", running_animation);
             let (event_type, elapsed_time) = match running_animation {
                 Animation::Transition(_, _, ref property_animation) => (
                     TransitionOrAnimationEventType::TransitionEnd,
                     property_animation.duration,
                 ),
-                Animation::Keyframes(_, _, _, ref mut state) => (
+                Animation::Keyframes(_, _, _, ref state) => (
                     TransitionOrAnimationEventType::AnimationEnd,
                     state.active_duration(),
                 ),
@@ -166,7 +175,7 @@ pub fn handle_running_animations(
 /// Send events for cancelled animations. Currently this only handles cancelled
 /// transitions, but eventually this should handle cancelled CSS animations as
 /// well.
-pub fn handle_cancelled_animations(
+fn handle_cancelled_animations(
     animation_state: &mut ElementAnimationState,
     now: f64,
     mut send_event: impl FnMut(&Animation, TransitionOrAnimationEventType, f64),
@@ -188,7 +197,7 @@ pub fn handle_cancelled_animations(
     }
 }
 
-pub fn handle_new_animations(
+fn handle_new_animations(
     animation_state: &mut ElementAnimationState,
     mut send_event: impl FnMut(&Animation, TransitionOrAnimationEventType, f64),
 ) {
@@ -208,55 +217,3 @@ pub fn handle_new_animations(
         animation_state.running_animations.push(animation);
     }
 }
-
-/// Recalculates style for a set of animations. This does *not* run with the DOM
-/// lock held. Returns a set of nodes associated with animations that are no longer
-/// valid.
-pub fn recalc_style_for_animations<E>(
-    context: &LayoutContext,
-    flow: &mut dyn Flow,
-    animation_states: &FxHashMap<OpaqueNode, ElementAnimationState>,
-) -> FxHashSet<OpaqueNode>
-where
-    E: TElement,
-{
-    let mut invalid_nodes = animation_states.keys().cloned().collect();
-    do_recalc_style_for_animations::<E>(context, flow, animation_states, &mut invalid_nodes);
-    invalid_nodes
-}
-
-fn do_recalc_style_for_animations<E>(
-    context: &LayoutContext,
-    flow: &mut dyn Flow,
-    animation_states: &FxHashMap<OpaqueNode, ElementAnimationState>,
-    invalid_nodes: &mut FxHashSet<OpaqueNode>,
-) where
-    E: TElement,
-{
-    let mut damage = RestyleDamage::empty();
-    flow.mutate_fragments(&mut |fragment| {
-        let animations = match animation_states.get(&fragment.node) {
-            Some(state) => &state.running_animations,
-            None => return,
-        };
-
-        invalid_nodes.remove(&fragment.node);
-        for animation in animations.iter() {
-            let old_style = fragment.style.clone();
-            update_style_for_animation::<E>(
-                &context.style_context,
-                animation,
-                Arc::make_mut(&mut fragment.style),
-                &ServoMetricsProvider,
-            );
-            let difference = RestyleDamage::compute_style_difference(&old_style, &fragment.style);
-            damage |= difference.damage;
-        }
-    });
-
-    let base = flow.mut_base();
-    base.restyle_damage.insert(damage);
-    for kid in base.children.iter_mut() {
-        do_recalc_style_for_animations::<E>(context, kid, animation_states, invalid_nodes)
-    }
-}