Bug 1646811 - servo: Include animations and transitions in the cascade.
authorMartin Robinson <mrobinson@igalia.com>
Thu, 18 Jun 2020 18:12:14 +0000
changeset 536347 bdbb1eb6acf15ad43f0a979e37a89b4d86b8d037
parent 536346 f6e4935b184b705639042d91e25042807c717c8d
child 536348 08aa02d53ba51711c80c252a4010593b3e090809
push id37520
push userdluca@mozilla.com
push dateFri, 19 Jun 2020 04:04:08 +0000
treeherdermozilla-central@d1a4f9157858 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1646811
milestone79.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1646811 - servo: Include animations and transitions in the cascade. Instead of applying animations and transitions to styled elements, include them in the cascade. This allows them to interact properly with things like font-size and !important rules. Depends on D80234 Differential Revision: https://phabricator.services.mozilla.com/D80235
servo/components/style/animation.rs
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/matching.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/sharing/mod.rs
servo/components/style/style_resolver.rs
servo/components/style/traversal.rs
--- a/servo/components/style/animation.rs
+++ b/servo/components/style/animation.rs
@@ -5,17 +5,17 @@
 //! CSS transitions and animations.
 
 // NOTE(emilio): This code isn't really executed in Gecko, but we don't want to
 // compile it out so that people remember it exists.
 
 use crate::bezier::Bezier;
 use crate::context::{CascadeInputs, SharedStyleContext};
 use crate::dom::{OpaqueNode, TDocument, TElement, TNode};
-use crate::properties::animated_properties::AnimationValue;
+use crate::properties::animated_properties::{AnimationValue, AnimationValueMap};
 use crate::properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use crate::properties::longhands::animation_fill_mode::computed_value::single_value::T as AnimationFillMode;
 use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
 use crate::properties::{
     ComputedValues, Importance, LonghandId, LonghandIdSet, PropertyDeclarationBlock,
     PropertyDeclarationId,
 };
 use crate::rule_tree::CascadeLevel;
@@ -122,23 +122,21 @@ impl PropertyAnimation {
             GenericTimingFunction::Keyword(keyword) => {
                 let (x1, x2, y1, y2) = keyword.to_bezier();
                 Bezier::new(x1, x2, y1, y2).solve(progress, epsilon)
             },
         }
     }
 
     /// Update the given animation at a given point of progress.
-    fn update(&self, style: &mut ComputedValues, progress: f64) {
+    fn calculate_value(&self, progress: f64) -> Result<AnimationValue, ()> {
         let procedure = Procedure::Interpolate {
             progress: self.timing_function_output(progress),
         };
-        if let Ok(new_value) = self.from.animate(&self.to, procedure) {
-            new_value.set_in_style_for_servo(style);
-        }
+        self.from.animate(&self.to, procedure)
     }
 }
 
 /// This structure represents the state of an animation.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq)]
 pub enum AnimationState {
     /// The animation has been created, but is not running yet. This state
     /// is also used when an animation is still in the first delay phase.
@@ -593,26 +591,24 @@ impl Animation {
 
         // Try to detect when we should skip straight to the running phase to
         // avoid sending multiple animationstart events.
         if self.state == Pending && self.started_at <= now && old_state != Pending {
             self.state = Running;
         }
     }
 
-    /// Update the given style to reflect the values specified by this `Animation`
-    /// at the time provided by the given `SharedStyleContext`.
-    fn update_style(&self, context: &SharedStyleContext, style: &mut Arc<ComputedValues>) {
+    /// Fill in an `AnimationValueMap` with values calculated from this animation at
+    /// the given time value.
+    fn get_property_declaration_at_time(&self, now: f64, map: &mut AnimationValueMap) {
         let duration = self.duration;
         let started_at = self.started_at;
 
         let now = match self.state {
-            AnimationState::Running | AnimationState::Pending | AnimationState::Finished => {
-                context.current_time_for_animations
-            },
+            AnimationState::Running | AnimationState::Pending | AnimationState::Finished => now,
             AnimationState::Paused(progress) => started_at + duration * progress,
             AnimationState::Canceled => return,
         };
 
         debug_assert!(!self.computed_steps.is_empty());
 
         let mut total_progress = (now - started_at) / duration;
         if total_progress < 0. &&
@@ -661,40 +657,39 @@ impl Animation {
                         }
                     })
                     .unwrap_or(num_steps - 1)
             },
             _ => unreachable!(),
         }
 
         debug!(
-            "Animation::update_style: keyframe from {:?} to {:?}",
+            "Animation::get_property_declaration_at_time: keyframe from {:?} to {:?}",
             prev_keyframe_index, next_keyframe_index
         );
 
         let prev_keyframe = &self.computed_steps[prev_keyframe_index];
         let next_keyframe = match next_keyframe_index {
             Some(index) => &self.computed_steps[index],
             None => return,
         };
 
-        let update_with_single_keyframe_style = |style, keyframe: &ComputedKeyframe| {
-            let mutable_style = Arc::make_mut(style);
+        let mut add_declarations_to_map = |keyframe: &ComputedKeyframe| {
             for value in keyframe.values.iter() {
-                value.set_in_style_for_servo(mutable_style);
+                map.insert(value.id(), value.clone());
             }
         };
 
         if total_progress <= 0.0 {
-            update_with_single_keyframe_style(style, &prev_keyframe);
+            add_declarations_to_map(&prev_keyframe);
             return;
         }
 
         if total_progress >= 1.0 {
-            update_with_single_keyframe_style(style, &next_keyframe);
+            add_declarations_to_map(&next_keyframe);
             return;
         }
 
         let relative_timespan =
             (next_keyframe.start_percentage - prev_keyframe.start_percentage).abs();
         let relative_duration = relative_timespan as f64 * duration;
         let last_keyframe_ended_at = match self.current_direction {
             AnimationDirection::Normal => {
@@ -702,28 +697,28 @@ impl Animation {
             },
             AnimationDirection::Reverse => {
                 self.started_at + (duration * (1. - prev_keyframe.start_percentage as f64))
             },
             _ => unreachable!(),
         };
 
         let relative_progress = (now - last_keyframe_ended_at) / relative_duration;
-        let mut new_style = (**style).clone();
         for (from, to) in prev_keyframe.values.iter().zip(next_keyframe.values.iter()) {
-            PropertyAnimation {
+            let animation = PropertyAnimation {
                 from: from.clone(),
                 to: to.clone(),
                 timing_function: prev_keyframe.timing_function,
                 duration: relative_duration as f64,
+            };
+
+            if let Ok(value) = animation.calculate_value(relative_progress) {
+                map.insert(value.id(), value);
             }
-            .update(&mut new_style, relative_progress);
         }
-
-        *Arc::make_mut(style) = new_style;
     }
 }
 
 impl fmt::Debug for Animation {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         f.debug_struct("Animation")
             .field("name", &self.name)
             .field("started_at", &self.started_at)
@@ -794,17 +789,20 @@ impl Transition {
         self.reversing_adjusted_start_value = replaced_animation.to.clone();
 
         // "* reversing shortening factor is the absolute value, clamped to the
         //    range [0, 1], of the sum of:
         //    1. the output of the timing function of the old transition at the
         //      time of the style change event, times the reversing shortening
         //      factor of the old transition
         //    2.  1 minus the reversing shortening factor of the old transition."
-        let transition_progress = replaced_transition.progress(now);
+        let transition_progress = ((now - replaced_transition.start_time) /
+            (replaced_transition.property_animation.duration))
+            .min(1.0)
+            .max(0.0);
         let timing_function_output = replaced_animation.timing_function_output(transition_progress);
         let old_reversing_shortening_factor = replaced_transition.reversing_shortening_factor;
         self.reversing_shortening_factor = ((timing_function_output *
             old_reversing_shortening_factor) +
             (1.0 - old_reversing_shortening_factor))
             .abs()
             .min(1.0)
             .max(0.0);
@@ -840,71 +838,81 @@ impl Transition {
 
     /// Whether or not this animation has ended at the provided time. This does
     /// not take into account canceling i.e. when an animation or transition is
     /// canceled due to changes in the style.
     pub fn has_ended(&self, time: f64) -> bool {
         time >= self.start_time + (self.property_animation.duration)
     }
 
-    /// Whether this animation has the same end value as another one.
-    #[inline]
-    fn progress(&self, now: f64) -> f64 {
-        let progress = (now - self.start_time) / (self.property_animation.duration);
-        progress.min(1.0)
-    }
-
-    /// Update a style to the value specified by this `Transition` given a `SharedStyleContext`.
-    fn update_style(&self, context: &SharedStyleContext, style: &mut Arc<ComputedValues>) {
-        // Never apply canceled transitions to a style.
-        if self.state == AnimationState::Canceled {
-            return;
+    /// Update the given animation at a given point of progress.
+    pub fn calculate_value(&self, time: f64) -> Option<AnimationValue> {
+        let progress = (time - self.start_time) / (self.property_animation.duration);
+        if progress < 0.0 {
+            return None;
         }
 
-        let progress = self.progress(context.current_time_for_animations);
-        if progress >= 0.0 {
-            self.property_animation
-                .update(Arc::make_mut(style), progress);
-        }
+        self.property_animation
+            .calculate_value(progress.min(1.0))
+            .ok()
     }
 }
 
 /// Holds the animation state for a particular element.
 #[derive(Debug, Default, MallocSizeOf)]
 pub struct ElementAnimationSet {
     /// The animations for this element.
     pub animations: Vec<Animation>,
 
     /// The transitions for this element.
     pub transitions: Vec<Transition>,
+
+    /// Whether or not this ElementAnimationSet has had animations or transitions
+    /// which have been added, removed, or had their state changed.
+    pub dirty: bool,
 }
 
 impl ElementAnimationSet {
     /// Cancel all animations in this `ElementAnimationSet`. This is typically called
     /// when the element has been removed from the DOM.
     pub fn cancel_all_animations(&mut self) {
+        self.dirty = !self.animations.is_empty();
         for animation in self.animations.iter_mut() {
             animation.state = AnimationState::Canceled;
         }
+        self.cancel_active_transitions();
+    }
+
+    fn cancel_active_transitions(&mut self) {
+        self.dirty = !self.transitions.is_empty();
+
         for transition in self.transitions.iter_mut() {
-            transition.state = AnimationState::Canceled;
+            if transition.state != AnimationState::Finished {
+                transition.state = AnimationState::Canceled;
+            }
         }
     }
 
     pub(crate) fn apply_active_animations(
         &mut self,
         context: &SharedStyleContext,
         style: &mut Arc<ComputedValues>,
     ) {
-        for animation in &self.animations {
-            animation.update_style(context, style);
+        let now = context.current_time_for_animations;
+        let mutable_style = Arc::make_mut(style);
+        if let Some(map) = self.get_value_map_for_active_animations(now) {
+            for value in map.values() {
+                value.set_in_style_for_servo(mutable_style);
+            }
         }
 
-        for transition in &self.transitions {
-            transition.update_style(context, style);
+        if let Some(map) = self.get_value_map_for_active_transitions(now) {
+            for value in map.values() {
+                value.set_in_style_for_servo(mutable_style);
+            }
         }
     }
 
     /// Clear all canceled animations and transitions from this `ElementAnimationSet`.
     pub fn clear_canceled_animations(&mut self) {
         self.animations
             .retain(|animation| animation.state != AnimationState::Canceled);
         self.transitions
@@ -973,34 +981,41 @@ impl ElementAnimationSet {
 
         maybe_start_animations(element, &context, &new_style, self, resolver);
     }
 
     /// Update our transitions given a new style, canceling or starting new animations
     /// when appropriate.
     pub fn update_transitions_for_new_style(
         &mut self,
+        might_need_transitions_update: bool,
         context: &SharedStyleContext,
         opaque_node: OpaqueNode,
         old_style: Option<&Arc<ComputedValues>>,
         after_change_style: &Arc<ComputedValues>,
     ) {
         // If this is the first style, we don't trigger any transitions and we assume
         // there were no previously triggered transitions.
         let mut before_change_style = match old_style {
             Some(old_style) => Arc::clone(old_style),
             None => return,
         };
 
+        // If the style of this element is display:none, then cancel all active transitions.
+        if after_change_style.get_box().clone_display().is_none() {
+            self.cancel_active_transitions();
+            return;
+        }
+
+        if !might_need_transitions_update {
+            return;
+        }
+
         // We convert old values into `before-change-style` here.
-        // See https://drafts.csswg.org/css-transitions/#starting. We need to clone the
-        // style because this might still be a reference to the original `old_style` and
-        // we want to preserve that so that we can later properly calculate restyle damage.
         if self.has_active_transition() || self.has_active_animation() {
-            before_change_style = before_change_style.clone();
             self.apply_active_animations(context, &mut before_change_style);
         }
 
         let transitioning_properties = start_transitions_if_applicable(
             context,
             opaque_node,
             &before_change_style,
             after_change_style,
@@ -1011,16 +1026,17 @@ impl ElementAnimationSet {
         for transition in self.transitions.iter_mut() {
             if transition.state == AnimationState::Finished {
                 continue;
             }
             if transitioning_properties.contains(transition.property_animation.property_id()) {
                 continue;
             }
             transition.state = AnimationState::Canceled;
+            self.dirty = true;
         }
     }
 
     fn start_transition_if_applicable(
         &mut self,
         context: &SharedStyleContext,
         opaque_node: OpaqueNode,
         longhand_id: LonghandId,
@@ -1081,35 +1097,67 @@ impl ElementAnimationSet {
             .find(|transition| transition.property_animation.property_id() == longhand_id)
         {
             // We always cancel any running transitions for the same property.
             old_transition.state = AnimationState::Canceled;
             new_transition.update_for_possibly_reversed_transition(old_transition, delay, now);
         }
 
         self.transitions.push(new_transition);
+        self.dirty = true;
+    }
+
+    /// Generate a `AnimationValueMap` for this `ElementAnimationSet`'s
+    /// active transitions at the given time value.
+    pub fn get_value_map_for_active_transitions(&self, now: f64) -> Option<AnimationValueMap> {
+        if !self.has_active_transition() {
+            return None;
+        }
+
+        let mut map =
+            AnimationValueMap::with_capacity_and_hasher(self.transitions.len(), Default::default());
+        for transition in &self.transitions {
+            if transition.state == AnimationState::Canceled {
+                continue;
+            }
+            let value = match transition.calculate_value(now) {
+                Some(value) => value,
+                None => continue,
+            };
+            map.insert(value.id(), value);
+        }
+
+        Some(map)
+    }
+
+    /// Generate a `AnimationValueMap` for this `ElementAnimationSet`'s
+    /// active animations at the given time value.
+    pub fn get_value_map_for_active_animations(&self, now: f64) -> Option<AnimationValueMap> {
+        if !self.has_active_animation() {
+            return None;
+        }
+
+        let mut map = Default::default();
+        for animation in &self.animations {
+            animation.get_property_declaration_at_time(now, &mut map);
+        }
+
+        Some(map)
     }
 }
 
 /// Kick off any new transitions for this node and return all of the properties that are
 /// transitioning. This is at the end of calculating style for a single node.
 pub fn start_transitions_if_applicable(
     context: &SharedStyleContext,
     opaque_node: OpaqueNode,
     old_style: &ComputedValues,
     new_style: &Arc<ComputedValues>,
     animation_state: &mut ElementAnimationSet,
 ) -> LonghandIdSet {
-    // If the style of this element is display:none, then we don't start any transitions
-    // and we cancel any currently running transitions by returning an empty LonghandIdSet.
-    let box_style = new_style.get_box();
-    if box_style.clone_display().is_none() {
-        return LonghandIdSet::new();
-    }
-
     let mut properties_that_transition = LonghandIdSet::new();
     for transition in new_style.transition_properties() {
         let physical_property = transition.longhand_id.to_physical(new_style.writing_mode);
         if properties_that_transition.contains(physical_property) {
             continue;
         }
 
         properties_that_transition.insert(physical_property);
@@ -1209,16 +1257,18 @@ pub fn maybe_start_animations<E>(
             iteration_state,
             state,
             direction: animation_direction,
             current_direction: initial_direction,
             cascade_style: new_style.clone(),
             is_new: true,
         };
 
+        animation_state.dirty = true;
+
         // If the animation was already present in the list for the node, just update its state.
         for existing_animation in animation_state.animations.iter_mut() {
             if existing_animation.state == AnimationState::Canceled {
                 continue;
             }
 
             if new_animation.name == existing_animation.name {
                 existing_animation
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -474,33 +474,35 @@ pub trait TElement:
     /// Get this element's SMIL override declarations.
     fn smil_override(&self) -> Option<ArcBorrow<Locked<PropertyDeclarationBlock>>> {
         None
     }
 
     /// Get the combined animation and transition rules.
     ///
     /// FIXME(emilio): Is this really useful?
-    fn animation_rules(&self) -> AnimationRules {
+    fn animation_rules(&self, context: &SharedStyleContext) -> AnimationRules {
         if !self.may_have_animations() {
             return AnimationRules(None, None);
         }
 
-        AnimationRules(self.animation_rule(), self.transition_rule())
+        AnimationRules(self.animation_rule(context), self.transition_rule(context))
     }
 
     /// Get this element's animation rule.
-    fn animation_rule(&self) -> Option<Arc<Locked<PropertyDeclarationBlock>>> {
-        None
-    }
+    fn animation_rule(
+        &self,
+        _: &SharedStyleContext,
+    ) -> Option<Arc<Locked<PropertyDeclarationBlock>>>;
 
     /// Get this element's transition rule.
-    fn transition_rule(&self) -> Option<Arc<Locked<PropertyDeclarationBlock>>> {
-        None
-    }
+    fn transition_rule(
+        &self,
+        context: &SharedStyleContext,
+    ) -> Option<Arc<Locked<PropertyDeclarationBlock>>>;
 
     /// Get this element's state, for non-tree-structural pseudos.
     fn state(&self) -> ElementState;
 
     /// Whether this element has an attribute with a given namespace.
     fn has_attr(&self, namespace: &Namespace, attr: &LocalName) -> bool;
 
     /// Returns whether this element has a `part` attribute.
@@ -724,43 +726,41 @@ pub trait TElement:
     unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags);
 
     /// Returns true if the element has all the specified selector flags.
     fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool;
 
     /// In Gecko, element has a flag that represents the element may have
     /// any type of animations or not to bail out animation stuff early.
     /// Whereas Servo doesn't have such flag.
-    fn may_have_animations(&self) -> bool {
-        false
-    }
+    fn may_have_animations(&self) -> bool;
 
     /// Creates a task to update various animation state on a given (pseudo-)element.
     #[cfg(feature = "gecko")]
     fn update_animations(
         &self,
         before_change_style: Option<Arc<ComputedValues>>,
         tasks: UpdateAnimationsTasks,
     );
 
     /// Creates a task to process post animation on a given element.
     #[cfg(feature = "gecko")]
     fn process_post_animation(&self, tasks: PostAnimationTasks);
 
     /// Returns true if the element has relevant animations. Relevant
     /// animations are those animations that are affecting the element's style
     /// or are scheduled to do so in the future.
-    fn has_animations(&self) -> bool;
+    fn has_animations(&self, context: &SharedStyleContext) -> bool;
 
     /// Returns true if the element has a CSS animation.
     fn has_css_animations(&self, context: &SharedStyleContext) -> bool;
 
     /// Returns true if the element has a CSS transition (including running transitions and
     /// completed transitions).
-    fn has_css_transitions(&self) -> bool;
+    fn has_css_transitions(&self, context: &SharedStyleContext) -> bool;
 
     /// Returns true if the element has animation restyle hints.
     fn has_animation_restyle_hints(&self) -> bool {
         let data = match self.borrow_data() {
             Some(d) => d,
             None => return false,
         };
         return data.hint.has_animation_hint();
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1519,42 +1519,20 @@ impl<'le> TElement for GeckoElement<'le>
     fn has_animations(&self) -> bool {
         self.may_have_animations() && unsafe { Gecko_ElementHasAnimations(self.0) }
     }
 
     fn has_css_animations(&self, _: &SharedStyleContext) -> bool {
         self.may_have_animations() && unsafe { Gecko_ElementHasCSSAnimations(self.0) }
     }
 
-    fn has_css_transitions(&self) -> bool {
+    fn has_css_transitions(&self, _: &SharedStyleContext) -> bool {
         self.may_have_animations() && unsafe { Gecko_ElementHasCSSTransitions(self.0) }
     }
 
-    fn might_need_transitions_update(
-        &self,
-        old_style: Option<&ComputedValues>,
-        new_style: &ComputedValues,
-    ) -> bool {
-        let old_style = match old_style {
-            Some(v) => v,
-            None => return false,
-        };
-
-        let new_box_style = new_style.get_box();
-        if !self.has_css_transitions() && !new_box_style.specifies_transitions() {
-            return false;
-        }
-
-        if new_box_style.clone_display().is_none() || old_style.clone_display().is_none() {
-            return false;
-        }
-
-        return true;
-    }
-
     // Detect if there are any changes that require us to update transitions.
     // This is used as a more thoroughgoing check than the, cheaper
     // might_need_transitions_update check.
     //
     // The following logic shadows the logic used on the Gecko side
     // (nsTransitionManager::DoUpdateTransitions) where we actually perform the
     // update.
     //
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -3,17 +3,17 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 //! High-level interface to CSS selector matching.
 
 #![allow(unsafe_code)]
 #![deny(missing_docs)]
 
 use crate::computed_value_flags::ComputedValueFlags;
-use crate::context::{ElementCascadeInputs, QuirksMode, SelectorFlagsMap};
+use crate::context::{CascadeInputs, ElementCascadeInputs, QuirksMode, SelectorFlagsMap};
 use crate::context::{SharedStyleContext, StyleContext};
 use crate::data::ElementData;
 use crate::dom::TElement;
 #[cfg(feature = "servo")]
 use crate::dom::TNode;
 use crate::invalidation::element::restyle_hints::RestyleHint;
 use crate::properties::longhands::display::computed_value::T as Display;
 use crate::properties::ComputedValues;
@@ -171,42 +171,43 @@ trait PrivateMatchMethods: TElement {
                     self.smil_override(),
                     primary_rules,
                 );
             }
 
             if replacements.contains(RestyleHint::RESTYLE_CSS_TRANSITIONS) {
                 replace_rule_node(
                     CascadeLevel::Transitions,
-                    self.transition_rule().as_ref().map(|a| a.borrow_arc()),
+                    self.transition_rule(&context.shared)
+                        .as_ref()
+                        .map(|a| a.borrow_arc()),
                     primary_rules,
                 );
             }
 
             if replacements.contains(RestyleHint::RESTYLE_CSS_ANIMATIONS) {
                 replace_rule_node(
                     CascadeLevel::Animations,
-                    self.animation_rule().as_ref().map(|a| a.borrow_arc()),
+                    self.animation_rule(&context.shared)
+                        .as_ref()
+                        .map(|a| a.borrow_arc()),
                     primary_rules,
                 );
             }
         }
 
         false
     }
 
     /// If there is no transition rule in the ComputedValues, it returns None.
-    #[cfg(feature = "gecko")]
     fn after_change_style(
         &self,
         context: &mut StyleContext<Self>,
         primary_style: &Arc<ComputedValues>,
     ) -> Option<Arc<ComputedValues>> {
-        use crate::context::CascadeInputs;
-
         let rule_node = primary_style.rules();
         let without_transition_rules = context
             .shared
             .stylist
             .rule_tree()
             .remove_transition_rule_if_applicable(rule_node);
         if without_transition_rules == *rule_node {
             // We don't have transition rule in this case, so return None to let
@@ -309,16 +310,39 @@ trait PrivateMatchMethods: TElement {
         // We may want to be more granular, but it's probably not worth it.
         if new_style.writing_mode != old_style.writing_mode {
             return has_animations;
         }
 
         false
     }
 
+    fn might_need_transitions_update(
+        &self,
+        context: &StyleContext<Self>,
+        old_style: Option<&ComputedValues>,
+        new_style: &ComputedValues,
+    ) -> bool {
+        let old_style = match old_style {
+            Some(v) => v,
+            None => return false,
+        };
+
+        let new_box_style = new_style.get_box();
+        if !self.has_css_transitions(context.shared) && !new_box_style.specifies_transitions() {
+            return false;
+        }
+
+        if new_box_style.clone_display().is_none() || old_style.clone_display().is_none() {
+            return false;
+        }
+
+        return true;
+    }
+
     /// Create a SequentialTask for resolving descendants in a SMIL display
     /// property animation if the display property changed from none.
     #[cfg(feature = "gecko")]
     fn handle_display_change_for_smil_if_needed(
         &self,
         context: &mut StyleContext<Self>,
         old_values: Option<&ComputedValues>,
         new_values: &ComputedValues,
@@ -369,20 +393,22 @@ trait PrivateMatchMethods: TElement {
         // Bug 868975: These steps should examine and update the visited styles
         // in addition to the unvisited styles.
 
         let mut tasks = UpdateAnimationsTasks::empty();
         if self.needs_animations_update(context, old_values.as_ref().map(|s| &**s), new_values) {
             tasks.insert(UpdateAnimationsTasks::CSS_ANIMATIONS);
         }
 
-        let before_change_style = if self
-            .might_need_transitions_update(old_values.as_ref().map(|s| &**s), new_values)
-        {
-            let after_change_style = if self.has_css_transitions() {
+        let before_change_style = if self.might_need_transitions_update(
+            context,
+            old_values.as_ref().map(|s| &**s),
+            new_values,
+        ) {
+            let after_change_style = if self.has_css_transitions(context.shared) {
                 self.after_change_style(context, new_values)
             } else {
                 None
             };
 
             // In order to avoid creating a SequentialTask for transitions which
             // may not be updated, we check it per property to make sure Gecko
             // side will really update transition.
@@ -440,16 +466,26 @@ trait PrivateMatchMethods: TElement {
         _important_rules_changed: bool,
     ) {
         use crate::animation::AnimationState;
 
         // We need to call this before accessing the `ElementAnimationSet` from the
         // map because this call will do a RwLock::read().
         let needs_animations_update =
             self.needs_animations_update(context, old_values.as_ref().map(|s| &**s), new_values);
+        let might_need_transitions_update = self.might_need_transitions_update(
+            context,
+            old_values.as_ref().map(|s| &**s),
+            new_values,
+        );
+
+        let mut after_change_style = None;
+        if might_need_transitions_update {
+            after_change_style = self.after_change_style(context, new_values);
+        }
 
         let this_opaque = self.as_node().opaque();
         let shared_context = context.shared;
         let mut animation_set = shared_context
             .animation_states
             .write()
             .remove(&this_opaque)
             .unwrap_or_default();
@@ -469,38 +505,53 @@ trait PrivateMatchMethods: TElement {
                 *self,
                 &shared_context,
                 &new_values,
                 &mut resolver,
             );
         }
 
         animation_set.update_transitions_for_new_style(
+            might_need_transitions_update,
             &shared_context,
             this_opaque,
             old_values.as_ref(),
-            new_values,
+            after_change_style.as_ref().unwrap_or(new_values),
         );
 
-        animation_set.apply_active_animations(shared_context, new_values);
-
         // We clear away any finished transitions, but retain animations, because they
-        // might still be used for proper calculation of `animation-fill-mode`.
+        // might still be used for proper calculation of `animation-fill-mode`. This
+        // should change the computed values in the style, so we don't need to mark
+        // this set as dirty.
         animation_set
             .transitions
             .retain(|transition| transition.state != AnimationState::Finished);
 
         // If the ElementAnimationSet is empty, and don't store it in order to
         // save memory and to avoid extra processing later.
+        let changed_animations = animation_set.dirty;
         if !animation_set.is_empty() {
+            animation_set.dirty = false;
             shared_context
                 .animation_states
                 .write()
                 .insert(this_opaque, animation_set);
         }
+
+        // If we have modified animation or transitions, we recascade style for this node.
+        if changed_animations {
+            let mut resolver = StyleResolverForElement::new(
+                *self,
+                context,
+                RuleInclusion::All,
+                PseudoElementResolution::IfApplicable,
+            );
+            let new_primary = resolver.resolve_style_with_default_parents();
+            *new_values = new_primary.primary.style.0;
+        }
     }
 
     /// Computes and applies non-redundant damage.
     fn accumulate_damage_for(
         &self,
         shared_context: &SharedStyleContext,
         damage: &mut RestyleDamage,
         old_values: &ComputedValues,
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2824,21 +2824,22 @@ pub mod style_structs {
             /// animation-name other than `none`.
             pub fn specifies_animations(&self) -> bool {
                 self.animation_name_iter().any(|name| name.0.is_some())
             }
 
             /// Returns whether there are any transitions specified.
             #[cfg(feature = "servo")]
             pub fn specifies_transitions(&self) -> bool {
-                // TODO(mrobinson): This should check the combined duration and not just
-                // the duration.
-                self.transition_duration_iter()
-                    .take(self.transition_property_count())
-                    .any(|t| t.seconds() > 0.)
+                (0..self.transition_property_count()).any(|index| {
+                    let combined_duration =
+                        self.transition_duration_mod(index).seconds().max(0.) +
+                        self.transition_delay_mod(index).seconds();
+                    combined_duration > 0.
+                })
             }
 
             /// Returns true if animation properties are equal between styles, but without
             /// considering keyframe data.
             #[cfg(feature = "servo")]
             pub fn animations_equals(&self, other: &Self) -> bool {
                 self.animation_name_iter().eq(other.animation_name_iter()) &&
                 self.animation_delay_iter().eq(other.animation_delay_iter()) &&
--- a/servo/components/style/sharing/mod.rs
+++ b/servo/components/style/sharing/mod.rs
@@ -582,16 +582,17 @@ impl<E: TElement> StyleSharingCache<E> {
     /// NB: We pass a source for the validation data, rather than the data itself,
     /// to avoid memmoving at each function call. See rust issue #42763.
     pub fn insert_if_possible(
         &mut self,
         element: &E,
         style: &PrimaryStyle,
         validation_data_holder: Option<&mut StyleSharingTarget<E>>,
         dom_depth: usize,
+        shared_context: &SharedStyleContext,
     ) {
         let parent = match element.traversal_parent() {
             Some(element) => element,
             None => {
                 debug!("Failing to insert to the cache: no parent element");
                 return;
             },
         };
@@ -614,17 +615,17 @@ impl<E: TElement> StyleSharingCache<E> {
         // If the element has running animations, we can't share style.
         //
         // This is distinct from the specifies_{animations,transitions} check below,
         // because:
         //   * Animations can be triggered directly via the Web Animations API.
         //   * Our computed style can still be affected by animations after we no
         //     longer match any animation rules, since removing animations involves
         //     a sequential task and an additional traversal.
-        if element.has_animations() {
+        if element.has_animations(shared_context) {
             debug!("Failing to insert to the cache: running animations");
             return;
         }
 
         // In addition to the above running animations check, we also need to
         // check CSS animation and transition styles since it's possible that
         // we are about to create CSS animations/transitions.
         //
@@ -695,27 +696,29 @@ impl<E: TElement> StyleSharingCache<E> {
         self.cache_mut().entries.lookup(|candidate| {
             Self::test_candidate(
                 target,
                 candidate,
                 &shared_context,
                 bloom_filter,
                 nth_index_cache,
                 selector_flags_map,
+                shared_context,
             )
         })
     }
 
     fn test_candidate(
         target: &mut StyleSharingTarget<E>,
         candidate: &mut StyleSharingCandidate<E>,
         shared: &SharedStyleContext,
         bloom: &StyleBloom<E>,
         nth_index_cache: &mut NthIndexCache,
         selector_flags_map: &mut SelectorFlagsMap<E>,
+        shared_context: &SharedStyleContext,
     ) -> Option<ResolvedElementStyles> {
         debug_assert!(!target.is_in_native_anonymous_subtree());
 
         // Check that we have the same parent, or at least that the parents
         // share styles and permit sharing across their children. The latter
         // check allows us to share style between cousins if the parents
         // shared style.
         if !checks::parents_allow_sharing(target, candidate) {
@@ -765,17 +768,17 @@ impl<E: TElement> StyleSharingCache<E> {
             return None;
         }
 
         if target.element.shadow_root().is_some() {
             trace!("Miss: Shadow host");
             return None;
         }
 
-        if target.element.has_animations() {
+        if target.element.has_animations(shared_context) {
             trace!("Miss: Has Animations");
             return None;
         }
 
         if target.matches_user_and_author_rules() !=
             candidate.element.matches_user_and_author_rules()
         {
             trace!("Miss: User and Author Rules");
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -428,17 +428,17 @@ where
             };
 
             // Compute the primary rule node.
             stylist.push_applicable_declarations(
                 self.element,
                 implemented_pseudo.as_ref(),
                 self.element.style_attribute(),
                 self.element.smil_override(),
-                self.element.animation_rules(),
+                self.element.animation_rules(self.context.shared),
                 self.rule_inclusion,
                 &mut applicable_declarations,
                 &mut matching_context,
                 &mut set_selector_flags,
             );
         }
 
         // FIXME(emilio): This is a hack for animations, and should go away.
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -608,16 +608,17 @@ where
                         resolver.resolve_style_with_default_parents()
                     };
 
                     context.thread_local.sharing_cache.insert_if_possible(
                         &element,
                         &new_styles.primary,
                         Some(&mut target),
                         traversal_data.current_dom_depth,
+                        &context.shared,
                     );
 
                     new_styles
                 },
             }
         },
         CascadeWithReplacements(flags) => {
             // Skipping full matching, load cascade inputs from previous values.
@@ -664,16 +665,17 @@ where
             // opportunities for skipping selector matching, which could hurt
             // performance.
             if !new_styles.primary.reused_via_rule_node {
                 context.thread_local.sharing_cache.insert_if_possible(
                     &element,
                     &new_styles.primary,
                     None,
                     traversal_data.current_dom_depth,
+                    &context.shared,
                 );
             }
 
             new_styles
         },
     };
 
     element.finish_restyle(context, data, new_styles, important_rules_changed)