Bug 1845095: Make sure caching and filtering for relative selector is…

…n't used in invalidation. r=emilio

Unify the invalidation flag used for nth index cache for this purpose.

Differential Revision: https://phabricator.services.mozilla.com/D184526

selectors/context.rs

@@ -78,10 +78,9 @@ pub enum NeedsSelectorFlags {
     Yes,
 }
 
-/// Whether we need to ignore nth child selectors for this match request (only expected during
-/// invalidation).
+/// Whether we're matching in the contect of invalidation.
 #[derive(PartialEq)]
-pub enum IgnoreNthChildForInvalidation {
+pub enum MatchingForInvalidation {
     No,
     Yes,
 }
@@ -204,9 +203,8 @@ where
     quirks_mode: QuirksMode,
     needs_selector_flags: NeedsSelectorFlags,
 
-    /// Whether this match request should ignore nth child selectors (only expected during
-    /// invalidation).
-    ignores_nth_child_selectors_for_invalidation: IgnoreNthChildForInvalidation,
+    /// Whether we're matching in the contect of invalidation.
+    matching_for_invalidation: MatchingForInvalidation,
 
     /// Caches to speed up expensive selector matches.
     pub selector_caches: &'a mut SelectorCaches,
@@ -226,7 +224,7 @@ where
         selector_caches: &'a mut SelectorCaches,
         quirks_mode: QuirksMode,
         needs_selector_flags: NeedsSelectorFlags,
-        ignores_nth_child_selectors_for_invalidation: IgnoreNthChildForInvalidation,
+        matching_for_invalidation: MatchingForInvalidation,
     ) -> Self {
         Self::new_for_visited(
             matching_mode,
@@ -235,7 +233,7 @@ where
             VisitedHandlingMode::AllLinksUnvisited,
             quirks_mode,
             needs_selector_flags,
-            ignores_nth_child_selectors_for_invalidation,
+            matching_for_invalidation,
         )
     }
 
@@ -247,7 +245,7 @@ where
         visited_handling: VisitedHandlingMode,
         quirks_mode: QuirksMode,
         needs_selector_flags: NeedsSelectorFlags,
-        ignores_nth_child_selectors_for_invalidation: IgnoreNthChildForInvalidation,
+        matching_for_invalidation: MatchingForInvalidation,
     ) -> Self {
         Self {
             matching_mode,
@@ -256,7 +254,7 @@ where
             quirks_mode,
             classes_and_ids_case_sensitivity: quirks_mode.classes_and_ids_case_sensitivity(),
             needs_selector_flags,
-            ignores_nth_child_selectors_for_invalidation,
+            matching_for_invalidation,
             scope_element: None,
             current_host: None,
             nesting_level: 0,
@@ -313,10 +311,10 @@ where
         self.needs_selector_flags == NeedsSelectorFlags::Yes
     }
 
-    /// Whether we need to ignore nth child selectors (only expected during invalidation).
+    /// Whether or not we're matching to invalidate.
     #[inline]
-    pub fn ignores_nth_child_selectors_for_invalidation(&self) -> bool {
-        self.ignores_nth_child_selectors_for_invalidation == IgnoreNthChildForInvalidation::Yes
+    pub fn matching_for_invalidation(&self) -> bool {
+        self.matching_for_invalidation == MatchingForInvalidation::Yes
     }
 
     /// The case-sensitivity for class and ID selectors

selectors/matching.rs

@@ -429,6 +429,46 @@ fn matches_relative_selector<E: Element>(
     return false;
 }
 
+fn relative_selector_match_early<E: Element>(
+    selector: &RelativeSelector<E::Impl>,
+    element: &E,
+    context: &mut MatchingContext<E::Impl>,
+) -> Option<bool> {
+    if context.matching_for_invalidation() {
+        // In the context of invalidation, we can't use caching/filtering due to
+        // now/then matches. DOM structure also may have changed, so just pretend
+        // that we always match.
+        return Some(!context.in_negation());
+    }
+    // See if we can return a cached result.
+    if let Some(cached) = context
+        .selector_caches
+        .relative_selector
+        .lookup(element.opaque(), selector)
+    {
+        return Some(cached.matched());
+    }
+    // See if we can fast-reject.
+    if context
+        .selector_caches
+        .relative_selector_filter_map
+        .fast_reject(
+            element,
+            selector,
+            context.quirks_mode(),
+        )
+    {
+        // Alright, add as unmatched to cache.
+        context.selector_caches.relative_selector.add(
+            element.opaque(),
+            selector,
+            RelativeSelectorCachedMatch::NotMatched,
+        );
+        return Some(false);
+    }
+    None
+}
+
 /// Matches a relative selector in a list of relative selectors.
 fn matches_relative_selectors<E: Element>(
     selectors: &[RelativeSelector<E::Impl>],
@@ -446,35 +486,11 @@ fn matches_relative_selectors<E: Element>(
     }
 
     for relative_selector in selectors.iter() {
-        // See if we can return a cached result.
-        if let Some(cached) = context
-            .selector_caches
-            .relative_selector
-            .lookup(element.opaque(), &relative_selector)
-        {
-            if cached.matched() {
+        if let Some(result) = relative_selector_match_early(relative_selector, element, context) {
+            if result {
                 return true;
             }
-            // Did not match, continue on.
-            continue;
-        }
-        // See if we can fast-reject.
-        if context
-            .selector_caches
-            .relative_selector_filter_map
-            .fast_reject(
-                element,
-                relative_selector,
-                context.quirks_mode(),
-            )
-        {
-            // Alright, add as unmatched to cache.
-            context.selector_caches.relative_selector.add(
-                element.opaque(),
-                relative_selector,
-                RelativeSelectorCachedMatch::NotMatched,
-            );
-            // Then continue on.
+            // Early return indicates no match, continue to next selector.
             continue;
         }
 
@@ -1051,7 +1067,8 @@ where
     let has_selectors = !selectors.is_empty();
     let selectors_match =
         !has_selectors || matches_complex_selector_list(selectors, element, context, rightmost);
-    if context.ignores_nth_child_selectors_for_invalidation() {
+    if context.matching_for_invalidation() {
+        // Skip expensive indexing math in invalidation.
         return selectors_match && !context.in_negation();
     }
 

style/dom_apis.rs

@@ -14,7 +14,7 @@ use crate::selector_parser::SelectorImpl;
 use crate::values::AtomIdent;
 use selectors::attr::CaseSensitivity;
 use selectors::matching::{
-    self, IgnoreNthChildForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags,
+    self, MatchingForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags,
     SelectorCaches,
 };
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint};
@@ -39,7 +39,7 @@ where
         &mut selector_caches,
         quirks_mode,
         NeedsSelectorFlags::No,
-        IgnoreNthChildForInvalidation::No,
+        MatchingForInvalidation::No,
     );
     context.scope_element = Some(element.opaque());
     context.current_host = element.containing_shadow_host().map(|e| e.opaque());
@@ -63,7 +63,7 @@ where
         &mut selector_caches,
         quirks_mode,
         NeedsSelectorFlags::No,
-        IgnoreNthChildForInvalidation::No,
+        MatchingForInvalidation::No,
     );
     context.scope_element = Some(element.opaque());
     context.current_host = element.containing_shadow_host().map(|e| e.opaque());
@@ -746,7 +746,7 @@ pub fn query_selector<E, Q>(
         &mut selector_caches,
         quirks_mode,
         NeedsSelectorFlags::No,
-        IgnoreNthChildForInvalidation::No,
+        MatchingForInvalidation::No,
     );
     let root_element = root.as_element();
     matching_context.scope_element = root_element.map(|e| e.opaque());

style/invalidation/element/document_state.rs

@@ -12,7 +12,7 @@ use crate::invalidation::element::state_and_attributes;
 use crate::stylist::CascadeData;
 use dom::DocumentState;
 use selectors::matching::{
-    IgnoreNthChildForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags, QuirksMode,
+    MatchingForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags, QuirksMode,
     SelectorCaches, VisitedHandlingMode,
 };
 
@@ -57,7 +57,7 @@ impl<'a, E: TElement, I> DocumentStateInvalidationProcessor<'a, E, I> {
             VisitedHandlingMode::AllLinksVisitedAndUnvisited,
             quirks_mode,
             NeedsSelectorFlags::No,
-            IgnoreNthChildForInvalidation::No,
+            MatchingForInvalidation::No,
         );
 
         matching_context.extra_data.invalidation_data.document_state = document_states_changed;

style/invalidation/element/state_and_attributes.rs

@@ -20,7 +20,7 @@ use crate::{Atom, WeakAtom};
 use dom::ElementState;
 use selectors::attr::CaseSensitivity;
 use selectors::matching::{
-    matches_selector, IgnoreNthChildForInvalidation, MatchingContext, MatchingMode,
+    matches_selector, MatchingForInvalidation, MatchingContext, MatchingMode,
     NeedsSelectorFlags, SelectorCaches, VisitedHandlingMode,
 };
 use smallvec::SmallVec;
@@ -69,7 +69,7 @@ impl<'a, 'b: 'a, E: TElement + 'b> StateAndAttrInvalidationProcessor<'a, 'b, E>
             VisitedHandlingMode::AllLinksVisitedAndUnvisited,
             shared_context.quirks_mode(),
             NeedsSelectorFlags::No,
-            IgnoreNthChildForInvalidation::Yes,
+            MatchingForInvalidation::Yes,
         );
 
         Self {

style/style_resolver.rs

@@ -17,7 +17,7 @@ use crate::selector_parser::{PseudoElement, SelectorImpl};
 use crate::stylist::RuleInclusion;
 use log::Level::Trace;
 use selectors::matching::{
-    IgnoreNthChildForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags,
+    MatchingForInvalidation, MatchingContext, MatchingMode, NeedsSelectorFlags,
     RelativeSelectorMatchingState, VisitedHandlingMode,
 };
 use servo_arc::Arc;
@@ -473,7 +473,7 @@ where
             visited_handling,
             self.context.shared.quirks_mode(),
             NeedsSelectorFlags::Yes,
-            IgnoreNthChildForInvalidation::No,
+            MatchingForInvalidation::No,
         );
 
         let stylist = &self.context.shared.stylist;
@@ -568,7 +568,7 @@ where
             visited_handling,
             self.context.shared.quirks_mode(),
             NeedsSelectorFlags::Yes,
-            IgnoreNthChildForInvalidation::No,
+            MatchingForInvalidation::No,
         );
         matching_context.extra_data.originating_element_style = Some(originating_element_style);
 

style/stylist.rs

@@ -53,7 +53,7 @@ use selectors::bloom::BloomFilter;
 use selectors::matching::{
     matches_selector, MatchingContext, MatchingMode, NeedsSelectorFlags, SelectorCaches,
 };
-use selectors::matching::{IgnoreNthChildForInvalidation, VisitedHandlingMode};
+use selectors::matching::{MatchingForInvalidation, VisitedHandlingMode};
 use selectors::parser::{
     AncestorHashes, Combinator, Component, Selector, SelectorIter, SelectorList,
 };
@@ -1140,7 +1140,7 @@ impl Stylist {
             &mut selector_caches,
             self.quirks_mode,
             needs_selector_flags,
-            IgnoreNthChildForInvalidation::No,
+            MatchingForInvalidation::No,
         );
 
         matching_context.pseudo_element_matching_fn = matching_fn;
@@ -1175,7 +1175,7 @@ impl Stylist {
                 VisitedHandlingMode::RelevantLinkVisited,
                 self.quirks_mode,
                 needs_selector_flags,
-                IgnoreNthChildForInvalidation::No,
+                MatchingForInvalidation::No,
             );
             matching_context.pseudo_element_matching_fn = matching_fn;
             matching_context.extra_data.originating_element_style = Some(originating_element_style);
@@ -1394,7 +1394,7 @@ impl Stylist {
             selector_caches,
             self.quirks_mode,
             needs_selector_flags,
-            IgnoreNthChildForInvalidation::No,
+            MatchingForInvalidation::No,
         );
 
         // Note that, by the time we're revalidating, we're guaranteed that the