Rollup merge of #17014 - hiikezoe:animation-rules-in-norma-restyle, r=emilio,birtles

Animation rules in norma restyle

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

This is a PR for https://bugzilla.mozilla.org/show_bug.cgi?id=1366631 and https://bugzilla.mozilla.org/show_bug.cgi?id=1367225

- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes do not require tests because it's for stylo

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17014)
<!-- Reviewable:end -->

components/style/data.rs

@@ -6,12 +6,12 @@
 
 use context::SharedStyleContext;
 use dom::TElement;
-use properties::ComputedValues;
+use properties::{AnimationRules, ComputedValues, PropertyDeclarationBlock};
 use properties::longhands::display::computed_value as display;
 use restyle_hints::{HintComputationContext, RestyleReplacements, RestyleHint};
 use rule_tree::StrongRuleNode;
 use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage};
-use shared_lock::StylesheetGuards;
+use shared_lock::{Locked, StylesheetGuards};
 use std::fmt;
 use stylearc::Arc;
 use traversal::TraversalFlags;
@@ -558,4 +558,29 @@ impl ElementData {
     pub fn restyle_mut(&mut self) -> &mut RestyleData {
         self.get_restyle_mut().expect("Calling restyle_mut without RestyleData")
     }
+
+    /// Returns SMIL overriden value if exists.
+    pub fn get_smil_override(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
+        if cfg!(feature = "servo") {
+            // Servo has no knowledge of a SMIL rule, so just avoid looking for it.
+            return None;
+        }
+
+        match self.get_styles() {
+            Some(s) => s.primary.rules.get_smil_animation_rule(),
+            None => None,
+        }
+    }
+
+    /// Returns AnimationRules that has processed during animation-only restyles.
+    pub fn get_animation_rules(&self) -> AnimationRules {
+        if cfg!(feature = "servo") {
+            return AnimationRules(None, None)
+        }
+
+        match self.get_styles() {
+            Some(s) => s.primary.rules.get_animation_rules(),
+            None => AnimationRules(None, None),
+        }
+    }
 }

components/style/dom.rs

@@ -274,20 +274,6 @@ pub trait PresentationalHintsSynthesizer {
         where V: Push<ApplicableDeclarationBlock>;
 }
 
-/// The animation rules.
-///
-/// The first one is for Animation cascade level, and the second one is for
-/// Transition cascade level.
-pub struct AnimationRules(pub Option<Arc<Locked<PropertyDeclarationBlock>>>,
-                          pub Option<Arc<Locked<PropertyDeclarationBlock>>>);
-
-impl AnimationRules {
-    /// Returns whether these animation rules represents an actual rule or not.
-    pub fn is_empty(&self) -> bool {
-        self.0.is_none() && self.1.is_none()
-    }
-}
-
 /// The element trait, the main abstraction the style crate acts over.
 pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
                      ElementExt + PresentationalHintsSynthesizer {
@@ -348,11 +334,6 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
         None
     }
 
-    /// Get this element's animation rules.
-    fn get_animation_rules(&self) -> AnimationRules {
-        AnimationRules(None, None)
-    }
-
     /// Get this element's animation rule by the cascade level.
     fn get_animation_rule_by_cascade(&self,
                                      _cascade_level: CascadeLevel)
@@ -525,6 +506,11 @@ pub trait TElement : Eq + PartialEq + Debug + Hash + Sized + Copy + Clone +
     /// 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 }
+
     /// Creates a task to update various animation state on a given (pseudo-)element.
     #[cfg(feature = "gecko")]
     fn update_animations(&self,

components/style/gecko/wrapper.rs

@@ -18,7 +18,7 @@ use app_units::Au;
 use atomic_refcell::AtomicRefCell;
 use context::{QuirksMode, SharedStyleContext, UpdateAnimationsTasks};
 use data::ElementData;
-use dom::{self, AnimationRules, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
+use dom::{self, DescendantsBit, LayoutIterator, NodeInfo, TElement, TNode, UnsafeNode};
 use dom::{OpaqueNode, PresentationalHintsSynthesizer};
 use element_state::ElementState;
 use error_reporting::RustLogReporter;
@@ -422,11 +422,6 @@ impl<'le> GeckoElement<'le> {
         }
     }
 
-    #[inline]
-    fn may_have_animations(&self) -> bool {
-        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations)
-    }
-
     #[inline]
     fn has_id(&self) -> bool {
         self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasID)
@@ -613,11 +608,6 @@ impl<'le> TElement for GeckoElement<'le> {
         declarations.map(|s| s.as_arc_opt()).unwrap_or(None)
     }
 
-    fn get_animation_rules(&self) -> AnimationRules {
-        AnimationRules(self.get_animation_rule(),
-                       self.get_transition_rule())
-    }
-
     fn get_animation_rule_by_cascade(&self, cascade_level: ServoCascadeLevel)
                                      -> Option<Arc<Locked<PropertyDeclarationBlock>>> {
         match cascade_level {
@@ -782,6 +772,11 @@ impl<'le> TElement for GeckoElement<'le> {
         (self.flags() & node_flags) == node_flags
     }
 
+    #[inline]
+    fn may_have_animations(&self) -> bool {
+        self.as_node().get_bool_flag(nsINode_BooleanFlag::ElementHasAnimations)
+    }
+
     fn update_animations(&self,
                          before_change_style: Option<Arc<ComputedValues>>,
                          tasks: UpdateAnimationsTasks) {

components/style/matching.rs

@@ -10,10 +10,10 @@
 use cascade_info::CascadeInfo;
 use context::{SelectorFlagsMap, SharedStyleContext, StyleContext};
 use data::{ComputedStyle, ElementData, RestyleData};
-use dom::{AnimationRules, TElement, TNode};
+use dom::{TElement, TNode};
 use font_metrics::FontMetricsProvider;
 use log::LogLevel::Trace;
-use properties::{CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
+use properties::{AnimationRules, CascadeFlags, ComputedValues, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
 use properties::longhands::display::computed_value as display;
 use restyle_hints::{RESTYLE_CSS_ANIMATIONS, RESTYLE_CSS_TRANSITIONS, RestyleReplacements};
 use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RESTYLE_SMIL};
@@ -254,12 +254,15 @@ trait PrivateMatchMethods: TElement {
             // We could make that a bit better if the complexity cost is not too
             // big, but given further restyles are posted directly to
             // pseudo-elements, it doesn't seem worth the effort at a glance.
-            if pseudo.is_eager() && self.get_animation_rules().is_empty() {
+            if pseudo.is_eager() {
                 let parent = self.parent_element().unwrap();
-                let parent_data = parent.borrow_data().unwrap();
+                if !parent.may_have_animations() ||
-                let pseudo_style =
+                   primary_style.rules.get_animation_rules().is_empty() {
-                    parent_data.styles().pseudos.get(&pseudo).unwrap();
+                    let parent_data = parent.borrow_data().unwrap();
-                return pseudo_style.values().clone()
+                    let pseudo_style =
+                        parent_data.styles().pseudos.get(&pseudo).unwrap();
+                    return pseudo_style.values().clone()
+                }
             }
         }
 
@@ -694,38 +697,39 @@ pub trait MatchMethods : TElement {
                 let pseudo_style =
                     parent_data.styles().pseudos.get(&pseudo).unwrap();
                 let mut rules = pseudo_style.rules.clone();
-                let animation_rules = self.get_animation_rules();
+                if parent.may_have_animations() {
+                    let animation_rules = data.get_animation_rules();
 
-                // Handle animations here.
+                    // Handle animations here.
-                if let Some(animation_rule) = animation_rules.0 {
+                    if let Some(animation_rule) = animation_rules.0 {
-                    let animation_rule_node =
+                        let animation_rule_node =
-                        context.shared.stylist.rule_tree()
+                            context.shared.stylist.rule_tree()
-                            .update_rule_at_level(CascadeLevel::Animations,
+                                .update_rule_at_level(CascadeLevel::Animations,
-                                                  Some(&animation_rule),
+                                                      Some(&animation_rule),
-                                                  &mut rules,
+                                                      &mut rules,
-                                                  &context.shared.guards);
+                                                      &context.shared.guards);
-                    if let Some(node) = animation_rule_node {
+                        if let Some(node) = animation_rule_node {
-                        rules = node;
+                            rules = node;
+                        }
+                    }
+
+                    if let Some(animation_rule) = animation_rules.1 {
+                        let animation_rule_node =
+                            context.shared.stylist.rule_tree()
+                                .update_rule_at_level(CascadeLevel::Transitions,
+                                                      Some(&animation_rule),
+                                                      &mut rules,
+                                                      &context.shared.guards);
+                        if let Some(node) = animation_rule_node {
+                            rules = node;
+                        }
                     }
                 }
-
-                if let Some(animation_rule) = animation_rules.1 {
-                    let animation_rule_node =
-                        context.shared.stylist.rule_tree()
-                            .update_rule_at_level(CascadeLevel::Transitions,
-                                                  Some(&animation_rule),
-                                                  &mut rules,
-                                                  &context.shared.guards);
-                    if let Some(node) = animation_rule_node {
-                        rules = node;
-                    }
-                }
-
                 let important_rules_changed =
                     self.has_animations() &&
                     data.has_styles() &&
                     data.important_rules_are_different(&rules,
-                                                       &context.shared.guards);
+                                                   &context.shared.guards);
 
                 return RulesMatchedResult {
                     rule_nodes_changed: data.set_primary_rules(rules),
@@ -738,30 +742,35 @@ pub trait MatchMethods : TElement {
 
         let stylist = &context.shared.stylist;
         let style_attribute = self.style_attribute();
-        let smil_override = self.get_smil_override();
+        {
-        let animation_rules = self.get_animation_rules();
+            let smil_override = data.get_smil_override();
-        let bloom = context.thread_local.bloom_filter.filter();
+            let animation_rules = if self.may_have_animations() {
+                data.get_animation_rules()
+            } else {
+                AnimationRules(None, None)
+            };
+            let bloom = context.thread_local.bloom_filter.filter();
 
+            let map = &mut context.thread_local.selector_flags;
+            let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
+                self.apply_selector_flags(map, element, flags);
+            };
 
-        let map = &mut context.thread_local.selector_flags;
+            let mut matching_context =
-        let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
+                MatchingContext::new(MatchingMode::Normal, Some(bloom));
-            self.apply_selector_flags(map, element, flags);
-        };
 
-        let mut matching_context =
+            // Compute the primary rule node.
-            MatchingContext::new(MatchingMode::Normal, Some(bloom));
+            stylist.push_applicable_declarations(self,
+                                                 implemented_pseudo.as_ref(),
+                                                 style_attribute,
+                                                 smil_override,
+                                                 animation_rules,
+                                                 &mut applicable_declarations,
+                                                 &mut matching_context,
+                                                 &mut set_selector_flags);
 
-        // Compute the primary rule node.
+            *relations = matching_context.relations;
-        stylist.push_applicable_declarations(self,
+        }
-                                             implemented_pseudo.as_ref(),
-                                             style_attribute,
-                                             smil_override,
-                                             animation_rules,
-                                             &mut applicable_declarations,
-                                             &mut matching_context,
-                                             &mut set_selector_flags);
-
-        *relations = matching_context.relations;
 
         let primary_rule_node =
             compute_rule_node::<Self>(stylist.rule_tree(),
@@ -965,81 +974,82 @@ pub trait MatchMethods : TElement {
     }
 
     /// Updates the rule nodes without re-running selector matching, using just
-    /// the rule tree. Returns RulesChanged which indicates whether the rule nodes changed
+    /// the rule tree. Returns true if an !important rule was replaced.
-    /// and whether the important rules changed.
     fn replace_rules(&self,
                      replacements: RestyleReplacements,
                      context: &StyleContext<Self>,
                      data: &mut ElementData)
-                     -> RulesChanged {
+                     -> bool {
         use properties::PropertyDeclarationBlock;
         use shared_lock::Locked;
 
         let element_styles = &mut data.styles_mut();
         let primary_rules = &mut element_styles.primary.rules;
-        let mut result = RulesChanged::empty();
 
-        {
+        let replace_rule_node = |level: CascadeLevel,
-            let mut replace_rule_node = |level: CascadeLevel,
+                                 pdb: Option<&Arc<Locked<PropertyDeclarationBlock>>>,
-                                         pdb: Option<&Arc<Locked<PropertyDeclarationBlock>>>,
+                                 path: &mut StrongRuleNode| -> bool {
-                                         path: &mut StrongRuleNode| {
+            let new_node = context.shared.stylist.rule_tree()
-                let new_node = context.shared.stylist.rule_tree()
+                .update_rule_at_level(level, pdb, path, &context.shared.guards);
-                    .update_rule_at_level(level, pdb, path, &context.shared.guards);
+            match new_node {
-                if let Some(n) = new_node {
+                Some(n) => {
                     *path = n;
-                    if level.is_important() {
+                    level.is_important()
-                        result.insert(IMPORTANT_RULES_CHANGED);
+                },
-                    } else {
+                None => false,
-                        result.insert(NORMAL_RULES_CHANGED);
+            }
-                    }
+        };
-                }
+
+        if !context.shared.traversal_flags.for_animation_only() {
+            let mut result = false;
+            if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) {
+                let style_attribute = self.style_attribute();
+                result |= replace_rule_node(CascadeLevel::StyleAttributeNormal,
+                                            style_attribute,
+                                            primary_rules);
+                result |= replace_rule_node(CascadeLevel::StyleAttributeImportant,
+                                            style_attribute,
+                                            primary_rules);
+            }
+            return result;
+        }
+
+        // Animation restyle hints are processed prior to other restyle
+        // hints in the animation-only traversal.
+        //
+        // Non-animation restyle hints will be processed in a subsequent
+        // normal traversal.
+        if replacements.intersects(RestyleReplacements::for_animations()) {
+            debug_assert!(context.shared.traversal_flags.for_animation_only());
+
+            if replacements.contains(RESTYLE_SMIL) {
+                replace_rule_node(CascadeLevel::SMILOverride,
+                                  self.get_smil_override(),
+                                  primary_rules);
+            }
+
+            let replace_rule_node_for_animation = |level: CascadeLevel,
+                                                   primary_rules: &mut StrongRuleNode| {
+                let animation_rule = self.get_animation_rule_by_cascade(level);
+                replace_rule_node(level,
+                                  animation_rule.as_ref(),
+                                  primary_rules);
             };
 
-            // Animation restyle hints are processed prior to other restyle
+            // Apply Transition rules and Animation rules if the corresponding restyle hint
-            // hints in the animation-only traversal.
+            // is contained.
-            //
+            if replacements.contains(RESTYLE_CSS_TRANSITIONS) {
-            // Non-animation restyle hints will be processed in a subsequent
+                replace_rule_node_for_animation(CascadeLevel::Transitions,
-            // normal traversal.
+                                                primary_rules);
-            if replacements.intersects(RestyleReplacements::for_animations()) {
+            }
-                debug_assert!(context.shared.traversal_flags.for_animation_only());
 
-                if replacements.contains(RESTYLE_SMIL) {
+            if replacements.contains(RESTYLE_CSS_ANIMATIONS) {
-                    replace_rule_node(CascadeLevel::SMILOverride,
+                replace_rule_node_for_animation(CascadeLevel::Animations,
-                                      self.get_smil_override(),
+                                                primary_rules);
-                                      primary_rules);
-                }
-
-                let mut replace_rule_node_for_animation = |level: CascadeLevel,
-                                                           primary_rules: &mut StrongRuleNode| {
-                    let animation_rule = self.get_animation_rule_by_cascade(level);
-                    replace_rule_node(level,
-                                      animation_rule.as_ref(),
-                                      primary_rules);
-                };
-
-                // Apply Transition rules and Animation rules if the corresponding restyle hint
-                // is contained.
-                if replacements.contains(RESTYLE_CSS_TRANSITIONS) {
-                    replace_rule_node_for_animation(CascadeLevel::Transitions,
-                                                    primary_rules);
-                }
-
-                if replacements.contains(RESTYLE_CSS_ANIMATIONS) {
-                    replace_rule_node_for_animation(CascadeLevel::Animations,
-                                                    primary_rules);
-                }
-            } else if replacements.contains(RESTYLE_STYLE_ATTRIBUTE) {
-                let style_attribute = self.style_attribute();
-                replace_rule_node(CascadeLevel::StyleAttributeNormal,
-                                  style_attribute,
-                                  primary_rules);
-                replace_rule_node(CascadeLevel::StyleAttributeImportant,
-                                  style_attribute,
-                                  primary_rules);
             }
         }
 
-        result
+        false
     }
 
     /// Attempts to share a style with another node. This method is unsafe

components/style/properties/declaration_block.rs

@@ -11,6 +11,7 @@ use cssparser::{DeclarationListParser, parse_important};
 use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter};
 use error_reporting::ParseErrorReporter;
 use parser::{PARSING_MODE_DEFAULT, ParsingMode, ParserContext, log_css_error};
+use shared_lock::Locked;
 use std::fmt;
 use std::slice::Iter;
 use style_traits::ToCss;
@@ -18,6 +19,20 @@ use stylesheets::{CssRuleType, Origin, UrlExtraData};
 use super::*;
 #[cfg(feature = "gecko")] use properties::animated_properties::AnimationValueMap;
 
+/// The animation rules.
+///
+/// The first one is for Animation cascade level, and the second one is for
+/// Transition cascade level.
+pub struct AnimationRules<'a>(pub Option<&'a Arc<Locked<PropertyDeclarationBlock>>>,
+                              pub Option<&'a Arc<Locked<PropertyDeclarationBlock>>>);
+
+impl<'a> AnimationRules<'a> {
+    /// Returns whether these animation rules represents an actual rule or not.
+    pub fn is_empty(&self) -> bool {
+        self.0.is_none() && self.1.is_none()
+    }
+}
+
 /// A declaration [importance][importance].
 ///
 /// [importance]: https://drafts.csswg.org/css-cascade/#importance

components/style/rule_tree/mod.rs

@@ -8,7 +8,7 @@
 
 #[cfg(feature = "servo")]
 use heapsize::HeapSizeOf;
-use properties::{Importance, LonghandIdSet, PropertyDeclarationBlock};
+use properties::{AnimationRules, Importance, LonghandIdSet, PropertyDeclarationBlock};
 use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard};
 use smallvec::SmallVec;
 use std::io::{self, Write};
@@ -1142,6 +1142,56 @@ impl StrongRuleNode {
         }
         result
     }
+
+    /// Returns PropertyDeclarationBlock for this node.
+    /// This function must be called only for animation level node.
+    fn get_animation_style(&self) -> &Arc<Locked<PropertyDeclarationBlock>> {
+        debug_assert!(self.cascade_level().is_animation(),
+                      "The cascade level should be an animation level");
+        match *self.style_source().unwrap() {
+            StyleSource::Declarations(ref block) => block,
+            StyleSource::Style(_) => unreachable!("animating style should not be a style rule"),
+        }
+    }
+
+    /// Returns SMIL override declaration block if exists.
+    pub fn get_smil_animation_rule(&self) -> Option<&Arc<Locked<PropertyDeclarationBlock>>> {
+        if cfg!(feature = "servo") {
+            // Servo has no knowledge of a SMIL rule, so just avoid looking for it.
+            return None;
+        }
+
+        self.self_and_ancestors()
+            .take_while(|node| node.cascade_level() >= CascadeLevel::SMILOverride)
+            .find(|node| node.cascade_level() == CascadeLevel::SMILOverride)
+            .map(|node| node.get_animation_style())
+    }
+
+    /// Returns AnimationRules that has processed during animation-only restyles.
+    pub fn get_animation_rules(&self) -> AnimationRules {
+        if cfg!(feature = "servo") {
+            return AnimationRules(None, None);
+        }
+
+        let mut animation = None;
+        let mut transition = None;
+
+        for node in self.self_and_ancestors()
+                        .take_while(|node| node.cascade_level() >= CascadeLevel::Animations) {
+            match node.cascade_level() {
+                CascadeLevel::Animations => {
+                    debug_assert!(animation.is_none());
+                    animation = Some(node.get_animation_style())
+                },
+                CascadeLevel::Transitions => {
+                    debug_assert!(transition.is_none());
+                    transition = Some(node.get_animation_style())
+                },
+                _ => {},
+            }
+        }
+        AnimationRules(animation, transition)
+    }
 }
 
 /// An iterator over a rule node and its ancestors.

components/style/stylist.rs

@@ -8,7 +8,7 @@ use {Atom, LocalName, Namespace};
 use bit_vec::BitVec;
 use context::{QuirksMode, SharedStyleContext};
 use data::ComputedStyle;
-use dom::{AnimationRules, TElement};
+use dom::TElement;
 use element_state::ElementState;
 use error_reporting::RustLogReporter;
 use font_metrics::FontMetricsProvider;
@@ -17,9 +17,9 @@ use gecko_bindings::structs::nsIAtom;
 use keyframes::KeyframesAnimation;
 use media_queries::Device;
 use properties::{self, CascadeFlags, ComputedValues};
+use properties::{AnimationRules, PropertyDeclarationBlock};
 #[cfg(feature = "servo")]
 use properties::INHERIT_ALL;
-use properties::PropertyDeclarationBlock;
 use restyle_hints::{HintComputationContext, DependencySet, RestyleHint};
 use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
 use selector_map::{SelectorMap, SelectorMapEntry};
@@ -944,7 +944,7 @@ impl Stylist {
             if let Some(anim) = animation_rules.0 {
                 Push::push(
                     applicable_declarations,
-                    ApplicableDeclarationBlock::from_declarations(anim,
+                    ApplicableDeclarationBlock::from_declarations(anim.clone(),
                                                                   CascadeLevel::Animations));
             }
             debug!("animation: {:?}", context.relations);
@@ -962,7 +962,8 @@ impl Stylist {
         if let Some(anim) = animation_rules.1 {
             Push::push(
                 applicable_declarations,
-                ApplicableDeclarationBlock::from_declarations(anim, CascadeLevel::Transitions));
+                ApplicableDeclarationBlock::from_declarations(anim.clone(),
+                                                              CascadeLevel::Transitions));
         }
         debug!("transition: {:?}", context.relations);
         debug!("push_applicable_declarations: shareable: {:?}", context.relations);

components/style/traversal.rs

@@ -770,11 +770,11 @@ fn compute_style<E, D>(_traversal: &D,
             )
         }
         CascadeWithReplacements(flags) => {
-            let rules_changed = element.replace_rules(flags, context, data);
+            let important_rules_changed = element.replace_rules(flags, context, data);
             element.cascade_primary_and_pseudos(
                 context,
                 data,
-                rules_changed.important_rules_changed()
+                important_rules_changed
             )
         }
         CascadeOnly => {