Check for empty when determining what selectors to use for revalidation.

components/style/matching.rs

@@ -154,9 +154,7 @@ fn element_matches_candidate<E: TElement>(element: &E,
         miss!(PresHints)
     }
 
-    if !match_same_sibling_affecting_rules(element,
+    if !revalidate(element, candidate_element, shared_context) {
-                                           candidate_element,
-                                           shared_context) {
         miss!(SiblingRules)
     }
 
@@ -212,10 +210,10 @@ fn match_same_style_affecting_attributes_rules<E: TElement>(element: &E,
 }
 
 #[inline]
-fn match_same_sibling_affecting_rules<E: TElement>(element: &E,
+fn revalidate<E: TElement>(element: &E,
-                                                   candidate: &E,
+                           candidate: &E,
-                                                   ctx: &SharedStyleContext) -> bool {
+                           ctx: &SharedStyleContext) -> bool {
-    ctx.stylist.match_same_sibling_affecting_rules(element, candidate)
+    ctx.stylist.revalidate_entry_for_cache(element, candidate)
 }
 
 static STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 8;

components/style/restyle_hints.rs

@@ -400,8 +400,15 @@ fn is_attr_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
     }
 }
 
-fn is_sibling_affecting_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
+/// Whether a selector containing this simple selector needs to be explicitly
+/// matched against both the style sharing cache entry and the candidate.
+///
+///
+/// We use this for selectors that can have different matching behavior between
+/// siblings that are otherwise identical as far as the cache is concerned.
+fn needs_cache_revalidation(sel: &SimpleSelector<SelectorImpl>) -> bool {
     match *sel {
+        SimpleSelector::Empty |
         SimpleSelector::FirstChild |
         SimpleSelector::LastChild |
         SimpleSelector::OnlyChild |
@@ -412,6 +419,7 @@ fn is_sibling_affecting_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
         SimpleSelector::FirstOfType |
         SimpleSelector::LastOfType |
         SimpleSelector::OnlyOfType => true,
+        SimpleSelector::NonTSPseudoClass(ref p) => p.state_flag().is_empty(),
         _ => false,
     }
 }
@@ -481,7 +489,7 @@ struct Dependency {
 /// selector.
 pub struct SelectorDependencyVisitor<'a> {
     dependency_set: &'a mut DependencySet,
-    affects_siblings: bool,
+    needs_cache_revalidation: bool,
     affected_by_attribute: bool,
 }
 
@@ -490,14 +498,15 @@ impl<'a> SelectorDependencyVisitor<'a> {
     pub fn new(dependency_set: &'a mut DependencySet) -> Self {
         SelectorDependencyVisitor {
             dependency_set: dependency_set,
-            affects_siblings: false,
+            needs_cache_revalidation: false,
             affected_by_attribute: false,
         }
     }
 
-    /// Returns whether this visitor has known of a sibling-dependent selector.
+    /// Returns whether this visitor has encountered a simple selector that needs
-    pub fn affects_siblings(&self) -> bool {
+    /// cache revalidation.
-        self.affects_siblings
+    pub fn needs_cache_revalidation(&self) -> bool {
+        self.needs_cache_revalidation
     }
 
     /// Returns whether this visitor has known of a attribute-dependent
@@ -518,8 +527,8 @@ impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> {
         let mut sensitivities = Sensitivities::new();
         for s in &selector.compound_selector {
             sensitivities.states.insert(selector_to_state(s));
-            if !self.affects_siblings {
+            if !self.needs_cache_revalidation {
-                self.affects_siblings = is_sibling_affecting_selector(s);
+                self.needs_cache_revalidation = needs_cache_revalidation(s);
             }
             if !sensitivities.attrs {
                 sensitivities.attrs = is_attr_selector(s);
@@ -529,7 +538,7 @@ impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> {
         let hint = combinator_to_restyle_hint(combinator);
 
         self.affected_by_attribute |= sensitivities.attrs;
-        self.affects_siblings |= hint.intersects(RESTYLE_LATER_SIBLINGS);
+        self.needs_cache_revalidation |= hint.intersects(RESTYLE_LATER_SIBLINGS);
 
         if !sensitivities.is_empty() {
             self.dependency_set.add_dependency(Dependency {

components/style/stylist.rs

@@ -111,9 +111,10 @@ pub struct Stylist {
     /// Selector dependencies used to compute restyle hints.
     state_deps: DependencySet,
 
-    /// Selectors in the page affecting siblings
+    /// Selectors that require explicit cache revalidation (i.e. which depend
+    /// on state that is not otherwise visible to the cache, like child index).
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
-    sibling_affecting_selectors: Vec<Selector<SelectorImpl>>,
+    selectors_for_cache_revalidation: Vec<Selector<SelectorImpl>>,
 
     /// Selectors in the page matching elements with non-common style-affecting
     /// attributes.
@@ -170,7 +171,7 @@ impl Stylist {
             rule_tree: RuleTree::new(),
             state_deps: DependencySet::new(),
 
-            sibling_affecting_selectors: vec![],
+            selectors_for_cache_revalidation: vec![],
             style_affecting_attributes_selectors: vec![]
         };
 
@@ -225,7 +226,7 @@ impl Stylist {
         self.state_deps.clear();
         self.animations.clear();
 
-        self.sibling_affecting_selectors.clear();
+        self.selectors_for_cache_revalidation.clear();
         self.style_affecting_attributes_selectors.clear();
 
         extra_data.clear_font_faces();
@@ -246,8 +247,8 @@ impl Stylist {
         }
 
         debug!("Stylist stats:");
-        debug!(" - Got {} sibling-affecting selectors",
+        debug!(" - Got {} selectors for cache revalidation",
-               self.sibling_affecting_selectors.len());
+               self.selectors_for_cache_revalidation.len());
         debug!(" - Got {} non-common-style-attribute-affecting selectors",
                self.style_affecting_attributes_selectors.len());
         debug!(" - Got {} deps for style-hint calculation",
@@ -304,8 +305,8 @@ impl Stylist {
                             SelectorDependencyVisitor::new(&mut self.state_deps);
                         selector.visit(&mut visitor);
 
-                        if visitor.affects_siblings() {
+                        if visitor.needs_cache_revalidation() {
-                            self.sibling_affecting_selectors.push(selector.clone());
+                            self.selectors_for_cache_revalidation.push(selector.clone());
                         }
 
                         if visitor.affected_by_attribute() {
@@ -813,12 +814,13 @@ impl Stylist {
         self.rule_tree.root()
     }
 
-    /// Returns whether two elements match the same sibling-affecting rules.
+    /// Returns whether two elements match the same set of revalidation-
+    /// requiring rules.
     ///
     /// This is also for the style sharing candidate cache.
-    pub fn match_same_sibling_affecting_rules<E>(&self,
+    pub fn revalidate_entry_for_cache<E>(&self,
-                                                 element: &E,
+                                         element: &E,
-                                                 candidate: &E) -> bool
+                                         candidate: &E) -> bool
         where E: TElement,
     {
         use selectors::matching::StyleRelations;
@@ -828,7 +830,7 @@ impl Stylist {
         //
         // TODO(emilio): Use the bloom filter, since they contain the element's
         // ancestor chain and it's correct for the candidate too.
-        for ref selector in self.sibling_affecting_selectors.iter() {
+        for ref selector in self.selectors_for_cache_revalidation.iter() {
             let element_matches =
                 matches_complex_selector(&selector.complex_selector, element,
                                          None, &mut StyleRelations::empty(),