style: Simplify selector flags setup even more

In my investigation for bug 1766439, I am digging into why selector
matching regressed.

It doesn't help that the selector-matching code is instantiated a
gazillion times (so there's a ton of copies of the relevant functions).

This was needed in the past because we had different ways of setting the
selector flags on elements, but I unified that recently and now we only
need to either set them or not. That is the kind of thing that
MatchingContext is really good for, so pass that instead on
MatchingContext creation.

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

components/selectors/context.rs

@@ -68,6 +68,14 @@ impl VisitedHandlingMode {
     }
 }
 
+/// Whether we need to set selector invalidation flags on elements for this
+/// match request.
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub enum NeedsSelectorFlags {
+    No,
+    Yes,
+}
+
 /// Which quirks mode is this document in.
 ///
 /// See: https://quirks.spec.whatwg.org/
@@ -140,6 +148,7 @@ where
     pub extra_data: Impl::ExtraMatchingData,
 
     quirks_mode: QuirksMode,
+    needs_selector_flags: NeedsSelectorFlags,
     classes_and_ids_case_sensitivity: CaseSensitivity,
     _impl: ::std::marker::PhantomData<Impl>,
 }
@@ -154,6 +163,7 @@ where
         bloom_filter: Option<&'a BloomFilter>,
         nth_index_cache: Option<&'a mut NthIndexCache>,
         quirks_mode: QuirksMode,
+        needs_selector_flags: NeedsSelectorFlags,
     ) -> Self {
         Self::new_for_visited(
             matching_mode,
@@ -161,6 +171,7 @@ where
             nth_index_cache,
             VisitedHandlingMode::AllLinksUnvisited,
             quirks_mode,
+            needs_selector_flags,
         )
     }
 
@@ -171,6 +182,7 @@ where
         nth_index_cache: Option<&'a mut NthIndexCache>,
         visited_handling: VisitedHandlingMode,
         quirks_mode: QuirksMode,
+        needs_selector_flags: NeedsSelectorFlags,
     ) -> Self {
         Self {
             matching_mode,
@@ -179,6 +191,7 @@ where
             nth_index_cache,
             quirks_mode,
             classes_and_ids_case_sensitivity: quirks_mode.classes_and_ids_case_sensitivity(),
+            needs_selector_flags,
             scope_element: None,
             current_host: None,
             nesting_level: 0,
@@ -213,6 +226,12 @@ where
         self.matching_mode
     }
 
+    /// Whether we need to set selector flags.
+    #[inline]
+    pub fn needs_selector_flags(&self) -> bool {
+        self.needs_selector_flags == NeedsSelectorFlags::Yes
+    }
+
     /// The case-sensitivity for class and ID selectors
     #[inline]
     pub fn classes_and_ids_case_sensitivity(&self) -> CaseSensitivity {

components/selectors/matching.rs

@@ -78,8 +78,7 @@ where
     // This is pretty much any(..) but manually inlined because the compiler
     // refuses to do so from querySelector / querySelectorAll.
     for selector in &selector_list.0 {
-        let matches = matches_selector(selector, 0, None, element, context, &mut |_, _| {});
+        let matches = matches_selector(selector, 0, None, element, context);
-
         if matches {
             return true;
         }
@@ -184,17 +183,15 @@ enum MatchesHoverAndActiveQuirk {
 /// unncessary cache miss for cases when we can fast-reject with AncestorHashes
 /// (which the caller can store inline with the selector pointer).
 #[inline(always)]
-pub fn matches_selector<E, F>(
+pub fn matches_selector<E>(
     selector: &Selector<E::Impl>,
     offset: usize,
     hashes: Option<&AncestorHashes>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    flags_setter: &mut F,
 ) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     // Use the bloom filter to fast-reject.
     if let Some(hashes) = hashes {
@@ -205,7 +202,7 @@ where
         }
     }
 
-    matches_complex_selector(selector.iter_from(offset), element, context, flags_setter)
+    matches_complex_selector(selector.iter_from(offset), element, context)
 }
 
 /// Whether a compound selector matched, and whether it was the rightmost
@@ -277,7 +274,7 @@ where
     );
 
     for component in iter {
-        if !matches_simple_selector(component, element, &mut local_context, &mut |_, _| {}) {
+        if !matches_simple_selector(component, element, &mut local_context) {
             return CompoundSelectorMatchingResult::NotMatched;
         }
     }
@@ -293,15 +290,13 @@ where
 
 /// Matches a complex selector.
 #[inline(always)]
-pub fn matches_complex_selector<E, F>(
+pub fn matches_complex_selector<E>(
     mut iter: SelectorIter<E::Impl>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    flags_setter: &mut F,
 ) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     // If this is the special pseudo-element mode, consume the ::pseudo-element
     // before proceeding, since the caller has already handled that part.
@@ -334,7 +329,7 @@ where
     }
 
     let result =
-        matches_complex_selector_internal(iter, element, context, flags_setter, Rightmost::Yes);
+        matches_complex_selector_internal(iter, element, context, Rightmost::Yes);
 
     matches!(result, SelectorMatchingResult::Matched)
 }
@@ -458,16 +453,14 @@ where
     }
 }
 
-fn matches_complex_selector_internal<E, F>(
+fn matches_complex_selector_internal<E>(
     mut selector_iter: SelectorIter<E::Impl>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    flags_setter: &mut F,
     rightmost: Rightmost,
 ) -> SelectorMatchingResult
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     debug!(
         "Matching complex selector {:?} for {:?}",
@@ -478,16 +471,16 @@ where
         &mut selector_iter,
         element,
         context,
-        flags_setter,
         rightmost,
     );
 
     let combinator = selector_iter.next_sequence();
     if combinator.map_or(false, |c| c.is_sibling()) {
-        flags_setter(
+        if context.needs_selector_flags() {
-            element,
+            element.apply_selector_flags(
-            ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS,
+                ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS
-        );
+            );
+        }
     }
 
     if !matches_compound_selector {
@@ -532,7 +525,6 @@ where
                 selector_iter.clone(),
                 &element,
                 context,
-                flags_setter,
                 Rightmost::No,
             )
         });
@@ -595,16 +587,14 @@ where
 
 /// Determines whether the given element matches the given compound selector.
 #[inline]
-fn matches_compound_selector<E, F>(
+fn matches_compound_selector<E>(
     selector_iter: &mut SelectorIter<E::Impl>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    flags_setter: &mut F,
     rightmost: Rightmost,
 ) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     let matches_hover_and_active_quirk =
         matches_hover_and_active_quirk(&selector_iter, context, rightmost);
@@ -643,19 +633,17 @@ where
     };
     iter::once(selector)
         .chain(selector_iter)
-        .all(|simple| matches_simple_selector(simple, element, &mut local_context, flags_setter))
+        .all(|simple| matches_simple_selector(simple, element, &mut local_context))
 }
 
 /// Determines whether the given element matches the given single selector.
-fn matches_simple_selector<E, F>(
+fn matches_simple_selector<E>(
     selector: &Component<E::Impl>,
     element: &E,
     context: &mut LocalMatchingContext<E::Impl>,
-    flags_setter: &mut F,
 ) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     debug_assert!(context.shared.is_nested() || !context.shared.in_negation());
 
@@ -703,7 +691,7 @@ where
             // <slots> are never flattened tree slottables.
             !element.is_html_slot_element() &&
                 context.shared.nest(|context| {
-                    matches_complex_selector(selector.iter(), element, context, flags_setter)
+                    matches_complex_selector(selector.iter(), element, context)
                 })
         },
         Component::PseudoElement(ref pseudo) => {
@@ -795,16 +783,18 @@ where
                 return false;
             }
 
-            element.match_non_ts_pseudo_class(pc, &mut context.shared, flags_setter)
+            element.match_non_ts_pseudo_class(pc, &mut context.shared)
         },
-        Component::FirstChild => matches_first_child(element, flags_setter),
+        Component::FirstChild => matches_first_child(element, context.shared),
-        Component::LastChild => matches_last_child(element, flags_setter),
+        Component::LastChild => matches_last_child(element, context.shared),
         Component::OnlyChild => {
-            matches_first_child(element, flags_setter) && matches_last_child(element, flags_setter)
+            matches_first_child(element, context.shared) && matches_last_child(element, context.shared)
         },
         Component::Root => element.is_root(),
         Component::Empty => {
-            flags_setter(element, ElementSelectorFlags::HAS_EMPTY_SELECTOR);
+            if context.shared.needs_selector_flags() {
+                element.apply_selector_flags(ElementSelectorFlags::HAS_EMPTY_SELECTOR);
+            }
             element.is_empty()
         },
         Component::Host(ref selector) => {
@@ -814,7 +804,7 @@ where
                 .map_or(false, |host| host == element.opaque()) &&
                 selector.as_ref().map_or(true, |selector| {
                     context.shared.nest(|context| {
-                        matches_complex_selector(selector.iter(), element, context, flags_setter)
+                        matches_complex_selector(selector.iter(), element, context)
                     })
                 })
         },
@@ -823,30 +813,30 @@ where
             None => element.is_root(),
         },
         Component::NthChild(a, b) => {
-            matches_generic_nth_child(element, context, a, b, false, false, flags_setter)
+            matches_generic_nth_child(element, context.shared, a, b, false, false)
         },
         Component::NthLastChild(a, b) => {
-            matches_generic_nth_child(element, context, a, b, false, true, flags_setter)
+            matches_generic_nth_child(element, context.shared, a, b, false, true)
         },
         Component::NthOfType(a, b) => {
-            matches_generic_nth_child(element, context, a, b, true, false, flags_setter)
+            matches_generic_nth_child(element, context.shared, a, b, true, false)
         },
         Component::NthLastOfType(a, b) => {
-            matches_generic_nth_child(element, context, a, b, true, true, flags_setter)
+            matches_generic_nth_child(element, context.shared, a, b, true, true)
         },
         Component::FirstOfType => {
-            matches_generic_nth_child(element, context, 0, 1, true, false, flags_setter)
+            matches_generic_nth_child(element, context.shared, 0, 1, true, false)
         },
         Component::LastOfType => {
-            matches_generic_nth_child(element, context, 0, 1, true, true, flags_setter)
+            matches_generic_nth_child(element, context.shared, 0, 1, true, true)
         },
         Component::OnlyOfType => {
-            matches_generic_nth_child(element, context, 0, 1, true, false, flags_setter) &&
+            matches_generic_nth_child(element, context.shared, 0, 1, true, false) &&
-                matches_generic_nth_child(element, context, 0, 1, true, true, flags_setter)
+                matches_generic_nth_child(element, context.shared, 0, 1, true, true)
         },
         Component::Is(ref list) | Component::Where(ref list) => context.shared.nest(|context| {
             for selector in &**list {
-                if matches_complex_selector(selector.iter(), element, context, flags_setter) {
+                if matches_complex_selector(selector.iter(), element, context) {
                     return true;
                 }
             }
@@ -854,7 +844,7 @@ where
         }),
         Component::Negation(ref list) => context.shared.nest_for_negation(|context| {
             for selector in &**list {
-                if matches_complex_selector(selector.iter(), element, context, flags_setter) {
+                if matches_complex_selector(selector.iter(), element, context) {
                     return false;
                 }
             }
@@ -873,35 +863,31 @@ fn select_name<'a, T>(is_html: bool, local_name: &'a T, local_name_lower: &'a T)
 }
 
 #[inline]
-fn matches_generic_nth_child<E, F>(
+fn matches_generic_nth_child<E>(
     element: &E,
-    context: &mut LocalMatchingContext<E::Impl>,
+    context: &mut MatchingContext<E::Impl>,
     a: i32,
     b: i32,
     is_of_type: bool,
     is_from_end: bool,
-    flags_setter: &mut F,
 ) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     if element.ignores_nth_child_selectors() {
         return false;
     }
 
-    flags_setter(
+    if context.needs_selector_flags() {
-        element,
+        element.apply_selector_flags(if is_from_end {
-        if is_from_end {
             ElementSelectorFlags::HAS_SLOW_SELECTOR
         } else {
             ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS
-        },
+        });
-    );
+    }
 
     // Grab a reference to the appropriate cache.
     let mut cache = context
-        .shared
         .nth_index_cache
         .as_mut()
         .map(|c| c.get(is_of_type, is_from_end));
@@ -992,21 +978,23 @@ where
 }
 
 #[inline]
-fn matches_first_child<E, F>(element: &E, flags_setter: &mut F) -> bool
+fn matches_first_child<E>(element: &E, context: &MatchingContext<E::Impl>) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
-    flags_setter(element, ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+    if context.needs_selector_flags() {
+        element.apply_selector_flags(ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+    }
     element.prev_sibling_element().is_none()
 }
 
 #[inline]
-fn matches_last_child<E, F>(element: &E, flags_setter: &mut F) -> bool
+fn matches_last_child<E>(element: &E, context: &MatchingContext<E::Impl>) -> bool
 where
     E: Element,
-    F: FnMut(&E, ElementSelectorFlags),
 {
-    flags_setter(element, ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+    if context.needs_selector_flags() {
+        element.apply_selector_flags(ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+    }
     element.next_sibling_element().is_none()
 }

components/selectors/tree.rs

@@ -77,14 +77,11 @@ pub trait Element: Sized + Clone + Debug {
         operation: &AttrSelectorOperation<&<Self::Impl as SelectorImpl>::AttrValue>,
     ) -> bool;
 
-    fn match_non_ts_pseudo_class<F>(
+    fn match_non_ts_pseudo_class(
         &self,
         pc: &<Self::Impl as SelectorImpl>::NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        flags_setter: &mut F,
+    ) -> bool;
-    ) -> bool
-    where
-        F: FnMut(&Self, ElementSelectorFlags);
 
     fn match_pseudo_element(
         &self,
@@ -92,6 +89,30 @@ pub trait Element: Sized + Clone + Debug {
         context: &mut MatchingContext<Self::Impl>,
     ) -> bool;
 
+    /// Sets selector flags, which indicate what kinds of selectors may have
+    /// matched on this element and therefore what kind of work may need to
+    /// be performed when DOM state changes.
+    ///
+    /// You probably don't want to use this directly and want to use
+    /// apply_selector_flags, since that sets flags on the parent as needed.
+    fn set_selector_flags(&self, flags: ElementSelectorFlags);
+
+    fn apply_selector_flags(&self, flags: ElementSelectorFlags) {
+        // Handle flags that apply to the element.
+        let self_flags = flags.for_self();
+        if !self_flags.is_empty() {
+            self.set_selector_flags(self_flags);
+        }
+
+        // Handle flags that apply to the parent.
+        let parent_flags = flags.for_parent();
+        if !parent_flags.is_empty() {
+            if let Some(p) = self.parent_element() {
+                p.set_selector_flags(parent_flags);
+            }
+        }
+    }
+
     /// Whether this element is a `link`.
     fn is_link(&self) -> bool;
 

components/style/dom.rs

@@ -22,7 +22,7 @@ use crate::traversal_flags::TraversalFlags;
 use crate::values::AtomIdent;
 use crate::{LocalName, Namespace, WeakAtom};
 use atomic_refcell::{AtomicRef, AtomicRefMut};
-use selectors::matching::{ElementSelectorFlags, QuirksMode, VisitedHandlingMode};
+use selectors::matching::{QuirksMode, VisitedHandlingMode};
 use selectors::sink::Push;
 use selectors::Element as SelectorsElement;
 use servo_arc::{Arc, ArcBorrow};
@@ -734,31 +734,6 @@ pub trait TElement:
     /// native anonymous content can opt out of this style fixup.)
     fn skip_item_display_fixup(&self) -> bool;
 
-    /// Sets selector flags, which indicate what kinds of selectors may have
-    /// matched on this element and therefore what kind of work may need to
-    /// be performed when DOM state changes.
-    ///
-    /// You probably don't want to use this directly and want to use
-    /// apply_selector_flags, since that sets flags on the parent as needed.
-    fn set_selector_flags(&self, flags: ElementSelectorFlags);
-
-    /// Applies selector flags to an element and its parent as needed.
-    fn apply_selector_flags(&self, flags: ElementSelectorFlags) {
-        // Handle flags that apply to the element.
-        let self_flags = flags.for_self();
-        if !self_flags.is_empty() {
-            self.set_selector_flags(self_flags);
-        }
-
-        // Handle flags that apply to the parent.
-        let parent_flags = flags.for_parent();
-        if !parent_flags.is_empty() {
-            if let Some(p) = self.parent_element() {
-                p.set_selector_flags(parent_flags);
-            }
-        }
-    }
-
     /// 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.

components/style/dom_apis.rs

@@ -12,7 +12,7 @@ use crate::invalidation::element::invalidator::{DescendantInvalidationLists, Inv
 use crate::invalidation::element::invalidator::{InvalidationProcessor, InvalidationVector};
 use crate::values::AtomIdent;
 use selectors::attr::CaseSensitivity;
-use selectors::matching::{self, MatchingContext, MatchingMode};
+use selectors::matching::{self, MatchingContext, MatchingMode, NeedsSelectorFlags};
 use selectors::parser::{Combinator, Component, LocalName, SelectorImpl};
 use selectors::{Element, NthIndexCache, SelectorList};
 use smallvec::SmallVec;
@@ -26,7 +26,13 @@ pub fn element_matches<E>(
 where
     E: Element,
 {
-    let mut context = MatchingContext::new(MatchingMode::Normal, None, None, quirks_mode);
+    let mut context = MatchingContext::new(
+        MatchingMode::Normal,
+        None,
+        None,
+        quirks_mode,
+        NeedsSelectorFlags::No,
+    );
     context.scope_element = Some(element.opaque());
     context.current_host = element.containing_shadow_host().map(|e| e.opaque());
     matching::matches_selector_list(selector_list, element, &mut context)
@@ -48,6 +54,7 @@ where
         None,
         Some(&mut nth_index_cache),
         quirks_mode,
+        NeedsSelectorFlags::No,
     );
     context.scope_element = Some(element.opaque());
     context.current_host = element.containing_shadow_host().map(|e| e.opaque());
@@ -618,8 +625,8 @@ pub fn query_selector<E, Q>(
         None,
         Some(&mut nth_index_cache),
         quirks_mode,
+        NeedsSelectorFlags::No,
     );
-
     let root_element = root.as_element();
     matching_context.scope_element = root_element.map(|e| e.opaque());
     matching_context.current_host = match root_element {

components/style/gecko/wrapper.rs

@@ -1393,11 +1393,6 @@ impl<'le> TElement for GeckoElement<'le> {
         self.is_root_of_native_anonymous_subtree()
     }
 
-    fn set_selector_flags(&self, flags: ElementSelectorFlags) {
-        debug_assert!(!flags.is_empty());
-        self.set_flags(selector_flags_to_node_flags(flags));
-    }
-
     #[inline]
     fn may_have_animations(&self) -> bool {
         if let Some(pseudo) = self.implemented_pseudo_element() {
@@ -1827,6 +1822,11 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
         None
     }
 
+    fn set_selector_flags(&self, flags: ElementSelectorFlags) {
+        debug_assert!(!flags.is_empty());
+        self.set_flags(selector_flags_to_node_flags(flags));
+    }
+
     fn attr_matches(
         &self,
         ns: &NamespaceConstraint<&Namespace>,
@@ -1939,15 +1939,11 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
         self.local_name() == other.local_name() && self.namespace() == other.namespace()
     }
 
-    fn match_non_ts_pseudo_class<F>(
+    fn match_non_ts_pseudo_class(
         &self,
         pseudo_class: &NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        flags_setter: &mut F,
+    ) -> bool {
-    ) -> bool
-    where
-        F: FnMut(&Self, ElementSelectorFlags),
-    {
         use selectors::matching::*;
         match *pseudo_class {
             NonTSPseudoClass::Autofill |
@@ -2003,7 +1999,9 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
                 self.is_link() && context.visited_handling().matches_visited()
             },
             NonTSPseudoClass::MozFirstNode => {
-                flags_setter(self, ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+                if context.needs_selector_flags() {
+                    self.apply_selector_flags(ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+                }
                 let mut elem = self.as_node();
                 while let Some(prev) = elem.prev_sibling() {
                     if prev.contains_non_whitespace_content() {
@@ -2014,7 +2012,9 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
                 true
             },
             NonTSPseudoClass::MozLastNode => {
-                flags_setter(self, ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+                if context.needs_selector_flags() {
+                    self.apply_selector_flags(ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
+                }
                 let mut elem = self.as_node();
                 while let Some(next) = elem.next_sibling() {
                     if next.contains_non_whitespace_content() {
@@ -2025,7 +2025,9 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
                 true
             },
             NonTSPseudoClass::MozOnlyWhitespace => {
-                flags_setter(self, ElementSelectorFlags::HAS_EMPTY_SELECTOR);
+                if context.needs_selector_flags() {
+                    self.apply_selector_flags(ElementSelectorFlags::HAS_EMPTY_SELECTOR);
+                }
                 if self
                     .as_node()
                     .dom_children()

components/style/invalidation/element/document_state.rs

@@ -11,7 +11,7 @@ use crate::invalidation::element::invalidator::{DescendantInvalidationLists, Inv
 use crate::invalidation::element::invalidator::{Invalidation, InvalidationProcessor};
 use crate::invalidation::element::state_and_attributes;
 use crate::stylist::CascadeData;
-use selectors::matching::{MatchingContext, MatchingMode, QuirksMode, VisitedHandlingMode};
+use selectors::matching::{MatchingContext, MatchingMode, QuirksMode, VisitedHandlingMode, NeedsSelectorFlags};
 
 /// A struct holding the members necessary to invalidate document state
 /// selectors.
@@ -47,6 +47,7 @@ impl<'a, E: TElement, I> DocumentStateInvalidationProcessor<'a, E, I> {
             None,
             VisitedHandlingMode::AllLinksVisitedAndUnvisited,
             quirks_mode,
+            NeedsSelectorFlags::No,
         );
 
         matching_context.extra_data = InvalidationMatchingData {

components/style/invalidation/element/element_wrapper.rs

@@ -166,15 +166,11 @@ where
 {
     type Impl = SelectorImpl;
 
-    fn match_non_ts_pseudo_class<F>(
+    fn match_non_ts_pseudo_class(
         &self,
         pseudo_class: &NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        _setter: &mut F,
+    ) -> bool {
-    ) -> bool
-    where
-        F: FnMut(&Self, ElementSelectorFlags),
-    {
         // Some pseudo-classes need special handling to evaluate them against
         // the snapshot.
         match *pseudo_class {
@@ -232,16 +228,20 @@ where
         if flag.is_empty() {
             return self
                 .element
-                .match_non_ts_pseudo_class(pseudo_class, context, &mut |_, _| {});
+                .match_non_ts_pseudo_class(pseudo_class, context);
         }
         match self.snapshot().and_then(|s| s.state()) {
             Some(snapshot_state) => snapshot_state.intersects(flag),
             None => self
                 .element
-                .match_non_ts_pseudo_class(pseudo_class, context, &mut |_, _| {}),
+                .match_non_ts_pseudo_class(pseudo_class, context),
         }
     }
 
+    fn set_selector_flags(&self, _flags: ElementSelectorFlags) {
+        debug_assert!(false, "Shouldn't need selector flags for invalidation");
+    }
+
     fn match_pseudo_element(
         &self,
         pseudo_element: &PseudoElement,

components/style/invalidation/element/state_and_attributes.rs

@@ -19,8 +19,7 @@ use crate::selector_parser::Snapshot;
 use crate::stylesheets::origin::OriginSet;
 use crate::{Atom, WeakAtom};
 use selectors::attr::CaseSensitivity;
-use selectors::matching::matches_selector;
+use selectors::matching::{matches_selector, MatchingContext, MatchingMode, VisitedHandlingMode, NeedsSelectorFlags};
-use selectors::matching::{MatchingContext, MatchingMode, VisitedHandlingMode};
 use selectors::NthIndexCache;
 use smallvec::SmallVec;
 
@@ -67,6 +66,7 @@ impl<'a, 'b: 'a, E: TElement + 'b> StateAndAttrInvalidationProcessor<'a, 'b, E>
             Some(nth_index_cache),
             VisitedHandlingMode::AllLinksVisitedAndUnvisited,
             shared_context.quirks_mode(),
+            NeedsSelectorFlags::No,
         );
 
         Self {
@@ -84,7 +84,7 @@ pub fn check_dependency<E, W>(
     dependency: &Dependency,
     element: &E,
     wrapper: &W,
-    mut context: &mut MatchingContext<'_, E::Impl>,
+    context: &mut MatchingContext<'_, E::Impl>,
 ) -> bool
 where
     E: TElement,
@@ -95,8 +95,7 @@ where
         dependency.selector_offset,
         None,
         element,
-        &mut context,
+        context,
-        &mut |_, _| {},
     );
 
     let matched_then = matches_selector(
@@ -104,8 +103,7 @@ where
         dependency.selector_offset,
         None,
         wrapper,
-        &mut context,
+        context,
-        &mut |_, _| {},
     );
 
     matched_then != matches_now

components/style/rule_collector.rs

@@ -13,7 +13,7 @@ use crate::selector_parser::PseudoElement;
 use crate::shared_lock::Locked;
 use crate::stylesheets::{layer_rule::LayerOrder, Origin};
 use crate::stylist::{AuthorStylesEnabled, CascadeData, Rule, RuleInclusion, Stylist};
-use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode};
+use selectors::matching::{MatchingContext, MatchingMode};
 use servo_arc::ArcBorrow;
 use smallvec::SmallVec;
 
@@ -59,7 +59,7 @@ pub fn containing_shadow_ignoring_svg_use<E: TElement>(
 ///
 /// This is done basically to be able to organize the cascade in smaller
 /// functions, and be able to reason about it easily.
-pub struct RuleCollector<'a, 'b: 'a, E, F: 'a>
+pub struct RuleCollector<'a, 'b: 'a, E>
 where
     E: TElement,
 {
@@ -73,16 +73,14 @@ where
     rule_inclusion: RuleInclusion,
     rules: &'a mut ApplicableDeclarationList,
     context: &'a mut MatchingContext<'b, E::Impl>,
-    flags_setter: &'a mut F,
     matches_user_and_author_rules: bool,
     matches_document_author_rules: bool,
     in_sort_scope: bool,
 }
 
-impl<'a, 'b: 'a, E, F: 'a> RuleCollector<'a, 'b, E, F>
+impl<'a, 'b: 'a, E> RuleCollector<'a, 'b, E>
 where
     E: TElement,
-    F: FnMut(&E, ElementSelectorFlags),
 {
     /// Trivially construct a new collector.
     pub fn new(
@@ -95,7 +93,6 @@ where
         rule_inclusion: RuleInclusion,
         rules: &'a mut ApplicableDeclarationList,
         context: &'a mut MatchingContext<'b, E::Impl>,
-        flags_setter: &'a mut F,
     ) -> Self {
         // When we're matching with matching_mode =
         // `ForStatelessPseudoeElement`, the "target" for the rule hash is the
@@ -125,7 +122,6 @@ where
             animation_declarations,
             rule_inclusion,
             context,
-            flags_setter,
             rules,
             matches_user_and_author_rules,
             matches_document_author_rules: matches_user_and_author_rules,
@@ -227,7 +223,6 @@ where
             part_rules,
             &mut self.rules,
             &mut self.context,
-            &mut self.flags_setter,
             cascade_level,
             cascade_data,
         );
@@ -246,7 +241,6 @@ where
             self.rule_hash_target,
             &mut self.rules,
             &mut self.context,
-            &mut self.flags_setter,
             cascade_level,
             cascade_data,
         );

components/style/selector_map.rs

@@ -14,7 +14,7 @@ use crate::stylist::{CascadeData, Rule};
 use crate::AllocErr;
 use crate::{Atom, LocalName, Namespace, ShrinkIfNeeded, WeakAtom};
 use precomputed_hash::PrecomputedHash;
-use selectors::matching::{matches_selector, ElementSelectorFlags, MatchingContext};
+use selectors::matching::{matches_selector, MatchingContext};
 use selectors::parser::{Combinator, Component, SelectorIter};
 use smallvec::SmallVec;
 use std::collections::hash_map;
@@ -186,18 +186,16 @@ impl SelectorMap<Rule> {
     ///
     /// Extract matching rules as per element's ID, classes, tag name, etc..
     /// Sort the Rules at the end to maintain cascading order.
-    pub fn get_all_matching_rules<E, F>(
+    pub fn get_all_matching_rules<E>(
         &self,
         element: E,
         rule_hash_target: E,
         matching_rules_list: &mut ApplicableDeclarationList,
         context: &mut MatchingContext<E::Impl>,
-        flags_setter: &mut F,
         cascade_level: CascadeLevel,
         cascade_data: &CascadeData,
     ) where
         E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
     {
         if self.is_empty() {
             return;
@@ -211,7 +209,6 @@ impl SelectorMap<Rule> {
                 &self.root,
                 matching_rules_list,
                 context,
-                flags_setter,
                 cascade_level,
                 cascade_data,
             );
@@ -224,7 +221,6 @@ impl SelectorMap<Rule> {
                     rules,
                     matching_rules_list,
                     context,
-                    flags_setter,
                     cascade_level,
                     cascade_data,
                 )
@@ -238,7 +234,6 @@ impl SelectorMap<Rule> {
                     rules,
                     matching_rules_list,
                     context,
-                    flags_setter,
                     cascade_level,
                     cascade_data,
                 )
@@ -253,7 +248,6 @@ impl SelectorMap<Rule> {
                         rules,
                         matching_rules_list,
                         context,
-                        flags_setter,
                         cascade_level,
                         cascade_data,
                     )
@@ -267,7 +261,6 @@ impl SelectorMap<Rule> {
                 rules,
                 matching_rules_list,
                 context,
-                flags_setter,
                 cascade_level,
                 cascade_data,
             )
@@ -279,7 +272,6 @@ impl SelectorMap<Rule> {
                 rules,
                 matching_rules_list,
                 context,
-                flags_setter,
                 cascade_level,
                 cascade_data,
             )
@@ -290,24 +282,21 @@ impl SelectorMap<Rule> {
             &self.other,
             matching_rules_list,
             context,
-            flags_setter,
             cascade_level,
             cascade_data,
         );
     }
 
     /// Adds rules in `rules` that match `element` to the `matching_rules` list.
-    pub(crate) fn get_matching_rules<E, F>(
+    pub(crate) fn get_matching_rules<E>(
         element: E,
         rules: &[Rule],
         matching_rules: &mut ApplicableDeclarationList,
         context: &mut MatchingContext<E::Impl>,
-        flags_setter: &mut F,
         cascade_level: CascadeLevel,
         cascade_data: &CascadeData,
     ) where
         E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
     {
         for rule in rules {
             if matches_selector(
@@ -316,7 +305,6 @@ impl SelectorMap<Rule> {
                 Some(&rule.hashes),
                 &element,
                 context,
-                flags_setter,
             ) {
                 matching_rules
                     .push(rule.to_applicable_declaration_block(cascade_level, cascade_data));

components/style/sharing/mod.rs

@@ -75,7 +75,7 @@ use crate::stylist::Stylist;
 use crate::values::AtomIdent;
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
 use owning_ref::OwningHandle;
-use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
+use selectors::matching::{VisitedHandlingMode, NeedsSelectorFlags};
 use selectors::NthIndexCache;
 use servo_arc::Arc;
 use smallbitvec::SmallBitVec;
@@ -222,18 +222,17 @@ impl ValidationData {
     /// Computes the revalidation results if needed, and returns it.
     /// Inline so we know at compile time what bloom_known_valid is.
     #[inline]
-    fn revalidation_match_results<E, F>(
+    fn revalidation_match_results<E>(
         &mut self,
         element: E,
         stylist: &Stylist,
         bloom: &StyleBloom<E>,
         nth_index_cache: &mut NthIndexCache,
         bloom_known_valid: bool,
-        flags_setter: &mut F,
+        needs_selector_flags: NeedsSelectorFlags,
     ) -> &SmallBitVec
     where
         E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
     {
         self.revalidation_match_results.get_or_insert_with(|| {
             // The bloom filter may already be set up for our element.
@@ -256,7 +255,7 @@ impl ValidationData {
                 element,
                 bloom_to_use,
                 nth_index_cache,
-                flags_setter,
+                needs_selector_flags,
             )
         })
     }
@@ -326,7 +325,9 @@ impl<E: TElement> StyleSharingCandidate<E> {
             bloom,
             nth_index_cache,
             /* bloom_known_valid = */ false,
-            &mut |_, _| {},
+            // The candidate must already have the right bits already, if
+            // needed.
+            NeedsSelectorFlags::No,
         )
     }
 }
@@ -399,17 +400,13 @@ impl<E: TElement> StyleSharingTarget<E> {
         // The style sharing cache will get a hit for the second span. When the
         // child span is subsequently removed from the DOM, missing selector
         // flags would cause us to miss the restyle on the second span.
-        let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| {
-            el.apply_selector_flags(flags);
-        };
-
         self.validation_data.revalidation_match_results(
             self.element,
             stylist,
             bloom,
             nth_index_cache,
             /* bloom_known_valid = */ true,
-            &mut set_selector_flags,
+            NeedsSelectorFlags::Yes,
         )
     }
 

components/style/style_resolver.rs

@@ -15,7 +15,7 @@ use crate::rule_tree::StrongRuleNode;
 use crate::selector_parser::{PseudoElement, SelectorImpl};
 use crate::stylist::RuleInclusion;
 use log::Level::Trace;
-use selectors::matching::{ElementSelectorFlags, MatchingContext};
+use selectors::matching::{NeedsSelectorFlags, MatchingContext};
 use selectors::matching::{MatchingMode, VisitedHandlingMode};
 use servo_arc::Arc;
 
@@ -459,28 +459,22 @@ where
             Some(nth_index_cache),
             visited_handling,
             self.context.shared.quirks_mode(),
+            NeedsSelectorFlags::Yes,
         );
 
         let stylist = &self.context.shared.stylist;
         let implemented_pseudo = self.element.implemented_pseudo_element();
-        {
+        // Compute the primary rule node.
-            let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
+        stylist.push_applicable_declarations(
-                element.apply_selector_flags(flags);
+            self.element,
-            };
+            implemented_pseudo.as_ref(),
-
+            self.element.style_attribute(),
-            // Compute the primary rule node.
+            self.element.smil_override(),
-            stylist.push_applicable_declarations(
+            self.element.animation_declarations(self.context.shared),
-                self.element,
+            self.rule_inclusion,
-                implemented_pseudo.as_ref(),
+            &mut applicable_declarations,
-                self.element.style_attribute(),
+            &mut matching_context,
-                self.element.smil_override(),
+        );
-                self.element.animation_declarations(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.
         self.element.unset_dirty_style_attribute();
@@ -538,12 +532,9 @@ where
             Some(nth_index_cache),
             visited_handling,
             self.context.shared.quirks_mode(),
+            NeedsSelectorFlags::Yes,
         );
 
-        let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
-            element.apply_selector_flags(flags);
-        };
-
         // NB: We handle animation rules for ::before and ::after when
         // traversing them.
         stylist.push_applicable_declarations(
@@ -555,7 +546,6 @@ where
             self.rule_inclusion,
             &mut applicable_declarations,
             &mut matching_context,
-            &mut set_selector_flags,
         );
 
         if applicable_declarations.is_empty() {

components/style/stylist.rs

@@ -48,7 +48,7 @@ use malloc_size_of::MallocUnconditionalShallowSizeOf;
 use selectors::attr::{CaseSensitivity, NamespaceConstraint};
 use selectors::bloom::BloomFilter;
 use selectors::matching::VisitedHandlingMode;
-use selectors::matching::{matches_selector, ElementSelectorFlags, MatchingContext, MatchingMode};
+use selectors::matching::{matches_selector, MatchingContext, MatchingMode, NeedsSelectorFlags};
 use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorIter};
 use selectors::visitor::SelectorVisitor;
 use selectors::NthIndexCache;
@@ -1072,22 +1072,12 @@ impl Stylist {
     {
         debug_assert!(pseudo.is_lazy());
 
-        // Apply the selector flags. We should be in sequential mode
+        // No need to bother setting the selector flags when we're computing
-        // already, so we can directly apply the parent flags.
+        // default styles.
-        let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
+        let needs_selector_flags = if rule_inclusion == RuleInclusion::DefaultOnly {
-            if cfg!(feature = "servo") {
+            NeedsSelectorFlags::No
-                // Servo calls this function from the worker, but only for internal
+        } else {
-                // pseudos, so we should never generate selector flags here.
+            NeedsSelectorFlags::Yes
-                unreachable!("internal pseudo generated slow selector flags?");
-            }
-
-            // No need to bother setting the selector flags when we're computing
-            // default styles.
-            if rule_inclusion == RuleInclusion::DefaultOnly {
-                return;
-            }
-
-            element.apply_selector_flags(flags);
         };
 
         let mut declarations = ApplicableDeclarationList::new();
@@ -1096,6 +1086,7 @@ impl Stylist {
             None,
             None,
             self.quirks_mode,
+            needs_selector_flags,
         );
 
         matching_context.pseudo_element_matching_fn = matching_fn;
@@ -1109,7 +1100,6 @@ impl Stylist {
             rule_inclusion,
             &mut declarations,
             &mut matching_context,
-            &mut set_selector_flags,
         );
 
         if declarations.is_empty() && is_probe {
@@ -1127,6 +1117,7 @@ impl Stylist {
                 None,
                 VisitedHandlingMode::RelevantLinkVisited,
                 self.quirks_mode,
+                needs_selector_flags,
             );
             matching_context.pseudo_element_matching_fn = matching_fn;
 
@@ -1139,7 +1130,6 @@ impl Stylist {
                 rule_inclusion,
                 &mut declarations,
                 &mut matching_context,
-                &mut set_selector_flags,
             );
             if !declarations.is_empty() {
                 let rule_node = self.rule_tree.insert_ordered_rules_with_important(
@@ -1252,7 +1242,7 @@ impl Stylist {
     }
 
     /// Returns the applicable CSS declarations for the given element.
-    pub fn push_applicable_declarations<E, F>(
+    pub fn push_applicable_declarations<E>(
         &self,
         element: E,
         pseudo_element: Option<&PseudoElement>,
@@ -1262,10 +1252,8 @@ impl Stylist {
         rule_inclusion: RuleInclusion,
         applicable_declarations: &mut ApplicableDeclarationList,
         context: &mut MatchingContext<E::Impl>,
-        flags_setter: &mut F,
     ) where
         E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
     {
         RuleCollector::new(
             self,
@@ -1277,7 +1265,6 @@ impl Stylist {
             rule_inclusion,
             applicable_declarations,
             context,
-            flags_setter,
         )
         .collect_all();
     }
@@ -1357,16 +1344,15 @@ impl Stylist {
 
     /// Computes the match results of a given element against the set of
     /// revalidation selectors.
-    pub fn match_revalidation_selectors<E, F>(
+    pub fn match_revalidation_selectors<E>(
         &self,
         element: E,
         bloom: Option<&BloomFilter>,
         nth_index_cache: &mut NthIndexCache,
-        flags_setter: &mut F,
+        needs_selector_flags: NeedsSelectorFlags,
     ) -> SmallBitVec
     where
         E: TElement,
-        F: FnMut(&E, ElementSelectorFlags),
     {
         // NB: `MatchingMode` doesn't really matter, given we don't share style
         // between pseudos.
@@ -1375,6 +1361,7 @@ impl Stylist {
             bloom,
             Some(nth_index_cache),
             self.quirks_mode,
+            needs_selector_flags,
         );
 
         // Note that, by the time we're revalidating, we're guaranteed that the
@@ -1397,7 +1384,6 @@ impl Stylist {
                                 Some(&selector_and_hashes.hashes),
                                 &element,
                                 matching_context,
-                                flags_setter,
                             ));
                             true
                         },
@@ -1420,7 +1406,6 @@ impl Stylist {
                         Some(&selector_and_hashes.hashes),
                         &element,
                         &mut matching_context,
-                        flags_setter,
                     ));
                     true
                 },