style: Refactor the traversal so it's more easy to read and straight-forward.

components/style/data.rs

@@ -9,7 +9,7 @@
 use dom::TElement;
 use properties::ComputedValues;
 use properties::longhands::display::computed_value as display;
-use restyle_hints::{RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RESTYLE_STYLE_ATTRIBUTE, RestyleHint};
+use restyle_hints::{RESTYLE_LATER_SIBLINGS, RESTYLE_SELF, RestyleHint};
 use rule_tree::StrongRuleNode;
 use selector_parser::{PseudoElement, RestyleDamage, Snapshot};
 use std::collections::HashMap;
@@ -168,7 +168,7 @@ pub struct StoredRestyleHint {
     ///
     /// This hint is stripped down, and only contains hints that are a subset of
     /// RestyleHint::for_single_element().
-    pub for_self: RestyleHint,
+    pub self_: RestyleHint,
     /// Whether the descendants of this element need to be restyled.
     pub descendants: DescendantRestyleHint,
 }
@@ -177,7 +177,7 @@ impl StoredRestyleHint {
     /// Propagates this restyle hint to a child element.
     pub fn propagate(&self) -> Self {
         StoredRestyleHint {
-            for_self: if self.descendants == DescendantRestyleHint::Empty {
+            self_: if self.descendants == DescendantRestyleHint::Empty {
                 RestyleHint::empty()
             } else {
                 RESTYLE_SELF
@@ -189,7 +189,7 @@ impl StoredRestyleHint {
     /// Creates an empty `StoredRestyleHint`.
     pub fn empty() -> Self {
         StoredRestyleHint {
-            for_self: RestyleHint::empty(),
+            self_: RestyleHint::empty(),
             descendants: DescendantRestyleHint::Empty,
         }
     }
@@ -198,24 +198,25 @@ impl StoredRestyleHint {
     /// including the element.
     pub fn subtree() -> Self {
         StoredRestyleHint {
-            for_self: RESTYLE_SELF,
+            self_: RESTYLE_SELF,
             descendants: DescendantRestyleHint::Descendants,
         }
     }
 
-    /// Whether the restyle hint is empty (nothing requires to be restyled).
+    /// Returns true if the hint indicates that our style may be invalidated.
-    pub fn is_empty(&self) -> bool {
+    pub fn has_self_invalidations(&self) -> bool {
-        !self.needs_restyle_self() && self.descendants == DescendantRestyleHint::Empty
+        !self.self_.is_empty()
     }
 
-    /// Whether the element holding the hint needs to be restyled on some way.
+    /// Whether the restyle hint is empty (nothing requires to be restyled).
-    pub fn needs_restyle_self(&self) -> bool {
+    pub fn is_empty(&self) -> bool {
-        !self.for_self.is_empty()
+        !self.has_self_invalidations() &&
+            self.descendants == DescendantRestyleHint::Empty
     }
 
     /// Insert another restyle hint, effectively resulting in the union of both.
     pub fn insert(&mut self, other: &Self) {
-        self.for_self |= other.for_self;
+        self.self_ |= other.self_;
         self.descendants = self.descendants.union(other.descendants);
     }
 }
@@ -232,7 +233,7 @@ impl From<RestyleHint> for StoredRestyleHint {
         use self::DescendantRestyleHint::*;
         debug_assert!(!hint.contains(RESTYLE_LATER_SIBLINGS), "Caller should apply sibling hints");
         StoredRestyleHint {
-            for_self: hint & RestyleHint::for_single_element(),
+            self_: hint & RestyleHint::for_self(),
             descendants: if hint.contains(RESTYLE_DESCENDANTS) { Descendants } else { Empty },
         }
     }
@@ -348,7 +349,7 @@ impl RestyleData {
 
     /// Returns true if this RestyleData might invalidate the current style.
     pub fn has_invalidations(&self) -> bool {
-        self.hint.needs_restyle_self() ||
+        self.hint.has_self_invalidations() ||
             self.recascade ||
             self.snapshot.is_some()
     }
@@ -369,6 +370,19 @@ pub struct ElementData {
     restyle: Option<Box<RestyleData>>,
 }
 
+/// The kind of restyle that a single element should do.
+pub enum RestyleKind {
+    /// We need to run selector matching plus re-cascade, that is, a full
+    /// restyle.
+    MatchAndCascade,
+    /// We need to recascade with some replacement rule, such as the style
+    /// attribute, or animation rules.
+    CascadeWithReplacements(RestyleHint),
+    /// We only need to recascade, for example, because only inherited
+    /// properties in the parent changed.
+    CascadeOnly,
+}
+
 impl ElementData {
     /// Trivially construct an ElementData.
     pub fn new(existing: Option<ElementStyles>) -> Self {
@@ -390,13 +404,28 @@ impl ElementData {
             self.restyle.as_ref().map_or(true, |r| !r.has_invalidations())
     }
 
-    /// Returns true if this element and its pseudo elements do not need
+    /// Returns the kind of restyling that we're going to need to do on this
-    /// selector matching, and can just go on with a style attribute update in
+    /// element, based of the stored restyle hint.
-    /// the rule tree plus re-cascade.
+    pub fn restyle_kind(&self) -> RestyleKind {
-    pub fn needs_only_style_attribute_update(&self) -> bool {
         debug_assert!(!self.has_current_styles(), "Should've stopped earlier");
-        self.has_styles() &&
+        if !self.has_styles() {
-            self.restyle.as_ref().map_or(false, |r| r.hint.for_self == RESTYLE_STYLE_ATTRIBUTE)
+            return RestyleKind::MatchAndCascade;
+        }
+
+        debug_assert!(self.restyle.is_some());
+        let restyle_data = self.restyle.as_ref().unwrap();
+        let hint = restyle_data.hint.self_;
+        if hint.contains(RESTYLE_SELF) {
+            return RestyleKind::MatchAndCascade;
+        }
+
+        if !hint.is_empty() {
+            return RestyleKind::CascadeWithReplacements(hint);
+        }
+
+        debug_assert!(restyle_data.recascade,
+                      "We definitely need to do something!");
+        return RestyleKind::CascadeOnly;
     }
 
     /// Gets the element styles, if any.

components/style/matching.rs

@@ -17,6 +17,7 @@ use data::{ComputedStyle, ElementData, ElementStyles, PseudoRuleNodes, PseudoSty
 use dom::{SendElement, TElement, TNode};
 use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
 use properties::longhands::display::computed_value as display;
+use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RestyleHint};
 use rule_tree::{CascadeLevel, StrongRuleNode};
 use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl};
 use selectors::MatchAttr;
@@ -627,31 +628,50 @@ pub trait MatchMethods : TElement {
         }
     }
 
-    /// Updates the style attribute rule nodes without re-running selector
+    /// Get the appropriate MatchResults from the current styles, to perform a
-    /// matching, using just the rule tree.
+    /// recascade.
-    fn update_style_attribute(&self,
+    ///
+    /// TODO(emilio): Stop using `MachResults`, use an enum, or something.
+    fn match_results_from_current_style(&self, data: &ElementData) -> MatchResults {
+        let rule_node = data.styles().primary.rules.clone();
+        MatchResults {
+            primary: rule_node,
+            // FIXME(emilio): Same concern as below.
+            relations: StyleRelations::empty(),
+            // The per-pseudo rule-nodes haven't changed, but still need to be
+            // recascaded.
+            per_pseudo: data.styles().pseudos.get_rules(),
+        }
+
+    }
+
+    /// Updates the rule nodes without re-running selector matching, using just
+    /// the rule tree.
+    fn cascade_with_replacements(&self,
+                                 hint: RestyleHint,
                                  context: &StyleContext<Self>,
                                  data: &mut AtomicRefMut<ElementData>)
                                  -> MatchResults {
+        let mut rule_node = data.styles().primary.rules.clone();
+
+        if hint.contains(RESTYLE_STYLE_ATTRIBUTE) {
             let style_attribute = self.style_attribute();
 
-        let mut new_rule_node = data.styles().primary.rules.clone();
+            rule_node = context.shared.stylist.rule_tree
-
+                .update_rule_at_level(CascadeLevel::StyleAttributeNormal,
-        new_rule_node = context.shared.stylist.rule_tree
-            .replace_rule_at_level_if_applicable(CascadeLevel::StyleAttributeNormal,
                                       style_attribute,
-                                                 new_rule_node);
+                                      rule_node);
 
-        new_rule_node = context.shared.stylist.rule_tree
+            rule_node = context.shared.stylist.rule_tree
-            .replace_rule_at_level_if_applicable(CascadeLevel::StyleAttributeImportant,
+                .update_rule_at_level(CascadeLevel::StyleAttributeImportant,
                                       style_attribute,
-                                                 new_rule_node);
+                                      rule_node);
+        }
 
         MatchResults {
-            primary: new_rule_node,
+            primary: rule_node,
-            // FIXME(emilio): This is ok, for now at least, because it disables
+            // FIXME(emilio): This is ok, for now, we shouldn't need to fake
-            // style sharing. That being said, it's not ideal, probably should
+            // this.
-            // be optional?
             relations: AFFECTED_BY_STYLE_ATTRIBUTE,
             // The per-pseudo rule-nodes haven't changed, but still need to be
             // recomputed.
@@ -675,6 +695,10 @@ pub trait MatchMethods : TElement {
             return StyleSharingResult::CannotShare
         }
 
+        if self.parent_element().is_none() {
+            return StyleSharingResult::CannotShare
+        }
+
         if self.style_attribute().is_some() {
             return StyleSharingResult::CannotShare
         }

components/style/restyle_hints.rs

@@ -53,7 +53,7 @@ bitflags! {
 impl RestyleHint {
     /// The subset hints that affect the styling of a single element during the
     /// traversal.
-    pub fn for_single_element() -> Self {
+    pub fn for_self() -> Self {
         RESTYLE_SELF | RESTYLE_STYLE_ATTRIBUTE
     }
 }

components/style/rule_tree/mod.rs

@@ -184,9 +184,7 @@ impl RuleTree {
     /// Replaces a rule in a given level (if present) for another rule.
     ///
     /// Returns the resulting node that represents the new path.
-    ///
+    pub fn update_rule_at_level(&self,
-    /// TODO(emilio): Maybe this should be in `StrongRuleNode`?
-    pub fn replace_rule_at_level_if_applicable(&self,
                                 level: CascadeLevel,
                                 pdb: Option<&Arc<RwLock<PropertyDeclarationBlock>>>,
                                 path: StrongRuleNode)

components/style/traversal.rs

@@ -8,7 +8,7 @@
 
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
 use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext};
-use data::{ElementData, ElementStyles, StoredRestyleHint};
+use data::{ElementData, ElementStyles, RestyleKind, StoredRestyleHint};
 use dom::{NodeInfo, TElement, TNode};
 use matching::{MatchMethods, StyleSharingResult};
 use restyle_hints::{RESTYLE_DESCENDANTS, RESTYLE_SELF};
@@ -453,9 +453,6 @@ pub fn recalc_style_at<E, D>(traversal: &D,
 
 // Computes style, returning true if the inherited styles changed for this
 // element.
-//
-// FIXME(bholley): This should differentiate between matching and cascading,
-// since we have separate bits for each now.
 fn compute_style<E, D>(_traversal: &D,
                        traversal_data: &mut PerLevelTraversalData,
                        context: &mut StyleContext<E>,
@@ -467,56 +464,60 @@ fn compute_style<E, D>(_traversal: &D,
     context.thread_local.statistics.elements_styled += 1;
     let shared_context = context.shared;
 
+    // TODO(emilio): Make cascade_input less expensive to compute in the cases
+    // we don't need to run selector matching.
+    let cascade_input = match data.restyle_kind() {
+        RestyleKind::MatchAndCascade => {
+            // Check to see whether we can share a style with someone.
+            let sharing_result = unsafe {
+                element.share_style_if_possible(&mut context.thread_local.style_sharing_candidate_cache,
+                                                shared_context,
+                                                &mut data)
+            };
+
+            match sharing_result {
+                StyleSharingResult::StyleWasShared(index) => {
+                    context.thread_local.statistics.styles_shared += 1;
+                    context.thread_local.style_sharing_candidate_cache.touch(index);
+                    None
+                }
+                StyleSharingResult::CannotShare => {
                     // Ensure the bloom filter is up to date.
-    //
+                    let dom_depth =
-    // TODO(emilio): In theory we could avoid a bit of this work in the style
+                        context.thread_local.bloom_filter
-    // attribute case.
+                               .insert_parents_recovering(element,
-    let dom_depth = context.thread_local.bloom_filter
+                                                          traversal_data.current_dom_depth);
-                           .insert_parents_recovering(element, traversal_data.current_dom_depth);
 
                     // Update the dom depth with the up-to-date dom depth.
                     //
-    // Note that this is always the same than the pre-existing depth, but it can
+                    // Note that this is always the same than the pre-existing depth,
-    // change from unknown to known at this step.
+                    // but it can change from unknown to known at this step.
                     traversal_data.current_dom_depth = Some(dom_depth);
 
                     context.thread_local.bloom_filter.assert_complete(element);
 
-    // Check to see whether we can share a style with someone.
-    let sharing_result = if element.parent_element().is_none() {
-        StyleSharingResult::CannotShare
-    } else {
-        unsafe { element.share_style_if_possible(&mut context.thread_local.style_sharing_candidate_cache,
-                                                 shared_context, &mut data) }
-    };
-
-    // Otherwise, match and cascade selectors.
-    match sharing_result {
-        StyleSharingResult::CannotShare => {
-            let match_results;
-            let shareable_element = {
                     // Perform the CSS selector matching.
                     context.thread_local.statistics.elements_matched += 1;
 
-                // TODO(emilio): Here we'll need to support animation-only hints
-                // and similar.
-                match_results = if data.needs_only_style_attribute_update() {
-                    element.update_style_attribute(context, data)
-                } else {
                     let filter = context.thread_local.bloom_filter.filter();
-                    element.match_element(context, Some(filter))
+                    Some(element.match_element(context, Some(filter)))
-                };
+                }
-                if match_results.primary_is_shareable() {
+            }
-                    Some(element)
+        }
-                } else {
+        RestyleKind::CascadeWithReplacements(hint) => {
-                    None
+            Some(element.cascade_with_replacements(hint, context, &mut data))
+        }
+        RestyleKind::CascadeOnly => {
+            // TODO(emilio): Stop doing this work, and teach cascade_node about
+            // the current style instead.
+            Some(element.match_results_from_current_style(&*data))
         }
     };
-            let relations = match_results.relations;
 
+    if let Some(match_results) = cascade_input {
         // Perform the CSS cascade.
-            unsafe {
         let shareable = match_results.primary_is_shareable();
+        unsafe {
             element.cascade_node(context, &mut data,
                                  element.parent_element(),
                                  match_results.primary,
@@ -524,16 +525,13 @@ fn compute_style<E, D>(_traversal: &D,
                                  shareable);
         }
 
+        if shareable {
             // Add ourselves to the LRU cache.
-            if let Some(element) = shareable_element {
             context.thread_local
                    .style_sharing_candidate_cache
-                       .insert_if_possible(&element, &data.styles().primary.values, relations);
+                   .insert_if_possible(&element,
-            }
+                                       &data.styles().primary.values,
-        }
+                                       match_results.relations);
-        StyleSharingResult::StyleWasShared(index) => {
-            context.thread_local.statistics.styles_shared += 1;
-            context.thread_local.style_sharing_candidate_cache.touch(index);
         }
     }