style: Record attribute dependencies within the selector list of :nth-child(... of <selector list>)

There are separate filters for IDs, classes, attribute local names, and
element state.

Also, we invalidate siblings of elements matched against the selector
list of :nth-child(... of <selector list>) by marking matched elements
with NODE_HAS_SLOW_SELECTOR_NTH_OF.

The only remaining invalidation case invalidation case is
`:nth-child(An+B of :has())` (bug 1818155), which should not block
shipping `layout.css.nth-child-of.enabled`, because :has(...) is still
being implemented (bug 418039).

Depends on D172352

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

components/selectors/matching.rs

@@ -38,14 +38,24 @@ bitflags! {
         /// :first-of-type, or :nth-of-type.
         const HAS_SLOW_SELECTOR_LATER_SIBLINGS = 1 << 1;
 
+        /// When a DOM mutation occurs on a child that might be matched by
+        /// :nth-last-child(.. of <selector list>), earlier children must be
+        /// restyled, and HAS_SLOW_SELECTOR will be set (which normally
+        /// indicates that all children will be restyled).
+        ///
+        /// Similarly, when a DOM mutation occurs on a child that might be
+        /// matched by :nth-child(.. of <selector list>), later children must be
+        /// restyled, and HAS_SLOW_SELECTOR_LATER_SIBLINGS will be set.
+        const HAS_SLOW_SELECTOR_NTH_OF = 1 << 2;
+
         /// When a child is added or removed from the parent, the first and
         /// last children must be restyled, because they may match :first-child,
         /// :last-child, or :only-child.
-        const HAS_EDGE_CHILD_SELECTOR = 1 << 2;
+        const HAS_EDGE_CHILD_SELECTOR = 1 << 3;
 
         /// The element has an empty selector, so when a child is appended we
         /// might need to restyle the parent completely.
-        const HAS_EMPTY_SELECTOR = 1 << 3;
+        const HAS_EMPTY_SELECTOR = 1 << 4;
     }
 }
 
@@ -59,6 +69,7 @@ impl ElementSelectorFlags {
     pub fn for_parent(self) -> ElementSelectorFlags {
         self & (ElementSelectorFlags::HAS_SLOW_SELECTOR |
             ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS |
+            ElementSelectorFlags::HAS_SLOW_SELECTOR_NTH_OF |
             ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR)
     }
 }
@@ -939,13 +950,17 @@ where
     let is_edge_child_selector = a == 0 && b == 1 && !is_of_type && selectors.is_empty();
 
     if context.needs_selector_flags() {
-        element.apply_selector_flags(if is_edge_child_selector {
+        let mut flags = if is_edge_child_selector {
             ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR
         } else if is_from_end {
             ElementSelectorFlags::HAS_SLOW_SELECTOR
         } else {
             ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS
-        });
+        };
+        if !selectors.is_empty() {
+            flags |= ElementSelectorFlags::HAS_SLOW_SELECTOR_NTH_OF;
+        }
+        element.apply_selector_flags(flags);
     }
 
     if !selectors.is_empty() && !list_matches_complex_selector(selectors, element, context) {

components/selectors/parser.rs

@@ -12,6 +12,7 @@ use crate::builder::{
 };
 use crate::context::QuirksMode;
 use crate::sink::Push;
+use crate::visitor::SelectorListKind;
 pub use crate::visitor::SelectorVisitor;
 use bitflags::bitflags;
 use cssparser::{match_ignore_ascii_case, parse_nth, *};
@@ -1625,14 +1626,15 @@ impl<Impl: SelectorImpl> Component<Impl> {
                     return false;
                 }
             },
-
             Negation(ref list) | Is(ref list) | Where(ref list) => {
-                if !visitor.visit_selector_list(&list) {
+                let list_kind = SelectorListKind::from_component(self);
+                debug_assert!(!list_kind.is_empty());
+                if !visitor.visit_selector_list(list_kind, &list) {
                     return false;
                 }
             },
             NthOf(ref nth_of_data) => {
-                if !visitor.visit_selector_list(nth_of_data.selectors()) {
+                if !visitor.visit_selector_list(SelectorListKind::NTH_OF, nth_of_data.selectors()) {
                     return false;
                 }
             },

components/selectors/visitor.rs

@@ -38,7 +38,11 @@ pub trait SelectorVisitor: Sized {
     /// into the internal selectors if / as needed.
     ///
     /// The default implementation does this.
-    fn visit_selector_list(&mut self, list: &[Selector<Self::Impl>]) -> bool {
+    fn visit_selector_list(
+        &mut self,
+        _list_kind: SelectorListKind,
+        list: &[Selector<Self::Impl>],
+    ) -> bool {
         for nested in list {
             if !nested.visit(self) {
                 return false;
@@ -55,3 +59,53 @@ pub trait SelectorVisitor: Sized {
         true
     }
 }
+
+bitflags! {
+    /// The kinds of components the visitor is visiting the selector list of, if any
+    #[derive(Default)]
+    pub struct SelectorListKind: u8 {
+        /// The visitor is inside :not(..)
+        const NEGATION = 1 << 0;
+        /// The visitor is inside :is(..)
+        const IS = 1 << 1;
+        /// The visitor is inside :where(..)
+        const WHERE = 1 << 2;
+        /// The visitor is inside :nth-child(.. of <selector list>) or
+        /// :nth-last-child(.. of <selector list>)
+        const NTH_OF = 1 << 3;
+    }
+}
+
+impl SelectorListKind {
+    /// Construct a SelectorListKind for the corresponding component.
+    pub fn from_component<Impl: SelectorImpl>(component: &Component<Impl>) -> Self {
+        match component {
+            Component::Negation(_) => SelectorListKind::NEGATION,
+            Component::Is(_) => SelectorListKind::IS,
+            Component::Where(_) => SelectorListKind::WHERE,
+            Component::NthOf(_) => SelectorListKind::NTH_OF,
+            _ => SelectorListKind::empty(),
+        }
+    }
+
+    /// Whether the visitor is inside :not(..)
+    pub fn in_negation(&self) -> bool {
+        self.intersects(SelectorListKind::NEGATION)
+    }
+
+    /// Whether the visitor is inside :is(..)
+    pub fn in_is(&self) -> bool {
+        self.intersects(SelectorListKind::IS)
+    }
+
+    /// Whether the visitor is inside :where(..)
+    pub fn in_where(&self) -> bool {
+        self.intersects(SelectorListKind::WHERE)
+    }
+
+    /// Whether the visitor is inside :nth-child(.. of <selector list>) or
+    /// :nth-last-child(.. of <selector list>)
+    pub fn in_nth_of(&self) -> bool {
+        self.intersects(SelectorListKind::NTH_OF)
+    }
+}

components/style/gecko/snapshot_helpers.rs

@@ -4,13 +4,17 @@
 
 //! Element an snapshot common logic.
 
+use crate::dom::TElement;
 use crate::gecko_bindings::bindings;
 use crate::gecko_bindings::structs::{self, nsAtom};
+use crate::invalidation::element::element_wrapper::ElementSnapshot;
+use crate::selector_parser::SnapshotMap;
 use crate::string_cache::WeakAtom;
 use crate::values::AtomIdent;
 use crate::Atom;
 use crate::CaseSensitivityExt;
 use selectors::attr::CaseSensitivity;
+use smallvec::SmallVec;
 
 /// A function that, given an element of type `T`, allows you to get a single
 /// class or a class list.
@@ -166,3 +170,27 @@ where
         }
     }
 }
+
+/// Returns a list of classes that were either added to or removed from the
+/// element since the snapshot.
+pub fn classes_changed<E: TElement>(element: &E, snapshots: &SnapshotMap) -> SmallVec<[Atom; 8]> {
+    debug_assert!(element.has_snapshot(), "Why bothering?");
+    let snapshot = snapshots.get(element).expect("has_snapshot lied");
+    if !snapshot.class_changed() {
+        return SmallVec::new();
+    }
+
+    let mut classes_changed = SmallVec::<[Atom; 8]>::new();
+    snapshot.each_class(|c| {
+        if !element.has_class(c, CaseSensitivity::CaseSensitive) {
+            classes_changed.push(c.0.clone());
+        }
+    });
+    element.each_class(|c| {
+        if !snapshot.has_class(c, CaseSensitivity::CaseSensitive) {
+            classes_changed.push(c.0.clone());
+        }
+    });
+
+    classes_changed
+}

components/style/gecko/wrapper.rs

@@ -910,6 +910,9 @@ fn selector_flags_to_node_flags(flags: ElementSelectorFlags) -> u32 {
     if flags.contains(ElementSelectorFlags::HAS_SLOW_SELECTOR_LATER_SIBLINGS) {
         gecko_flags |= NODE_HAS_SLOW_SELECTOR_LATER_SIBLINGS as u32;
     }
+    if flags.contains(ElementSelectorFlags::HAS_SLOW_SELECTOR_NTH_OF) {
+        gecko_flags |= NODE_HAS_SLOW_SELECTOR_NTH_OF as u32;
+    }
     if flags.contains(ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR) {
         gecko_flags |= NODE_HAS_EDGE_CHILD_SELECTOR as u32;
     }

components/style/invalidation/element/invalidation_map.rs

@@ -14,7 +14,6 @@ use crate::{Atom, LocalName, Namespace, ShrinkIfNeeded};
 use selectors::attr::NamespaceConstraint;
 use selectors::parser::{Combinator, Component};
 use selectors::parser::{Selector, SelectorIter};
-use selectors::visitor::SelectorVisitor;
 use smallvec::SmallVec;
 use style_traits::dom::{DocumentState, ElementState};
 
@@ -430,7 +429,11 @@ impl<'a> SelectorDependencyCollector<'a> {
 impl<'a> SelectorVisitor for SelectorDependencyCollector<'a> {
     type Impl = SelectorImpl;
 
-    fn visit_selector_list(&mut self, list: &[Selector<SelectorImpl>]) -> bool {
+    fn visit_selector_list(
+        &mut self,
+        _list_kind: SelectorListKind,
+        list: &[Selector<SelectorImpl>],
+    ) -> bool {
         for selector in list {
             // Here we cheat a bit: We can visit the rightmost compound with
             // the "outer" visitor, and it'd be fine. This reduces the amount of

components/style/stylist.rs

@@ -50,7 +50,7 @@ use selectors::bloom::BloomFilter;
 use selectors::matching::VisitedHandlingMode;
 use selectors::matching::{matches_selector, MatchingContext, MatchingMode, NeedsSelectorFlags};
 use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorIter};
-use selectors::visitor::SelectorVisitor;
+use selectors::visitor::{SelectorListKind, SelectorVisitor};
 use selectors::NthIndexCache;
 use servo_arc::{Arc, ArcBorrow};
 use smallbitvec::SmallBitVec;
@@ -1891,15 +1891,41 @@ struct StylistSelectorVisitor<'a> {
     /// Whether we've past the rightmost compound selector, not counting
     /// pseudo-elements.
     passed_rightmost_selector: bool,
+
     /// Whether the selector needs revalidation for the style sharing cache.
     needs_revalidation: &'a mut bool,
+
+    /// Flags for which selector list-containing components the visitor is
+    /// inside of, if any
+    in_selector_list_of: SelectorListKind,
+
     /// The filter with all the id's getting referenced from rightmost
     /// selectors.
     mapped_ids: &'a mut PrecomputedHashSet<Atom>,
+
+    /// The filter with the IDs getting referenced from the selector list of
+    /// :nth-child(... of <selector list>) selectors.
+    nth_of_mapped_ids: &'a mut PrecomputedHashSet<Atom>,
+
     /// The filter with the local names of attributes there are selectors for.
     attribute_dependencies: &'a mut PrecomputedHashSet<LocalName>,
+
+    /// The filter with the classes getting referenced from the selector list of
+    /// :nth-child(... of <selector list>) selectors.
+    nth_of_class_dependencies: &'a mut PrecomputedHashSet<Atom>,
+
+    /// The filter with the local names of attributes there are selectors for
+    /// within the selector list of :nth-child(... of <selector list>)
+    /// selectors.
+    nth_of_attribute_dependencies: &'a mut PrecomputedHashSet<LocalName>,
+
     /// All the states selectors in the page reference.
     state_dependencies: &'a mut ElementState,
+
+    /// All the state selectors in the page reference within the selector list
+    /// of :nth-child(... of <selector list>) selectors.
+    nth_of_state_dependencies: &'a mut ElementState,
+
     /// All the document states selectors in the page reference.
     document_state_dependencies: &'a mut DocumentState,
 }
@@ -1944,15 +1970,25 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
         true
     }
 
-    fn visit_selector_list(&mut self, list: &[Selector<Self::Impl>]) -> bool {
+    fn visit_selector_list(
+        &mut self,
+        list_kind: SelectorListKind,
+        list: &[Selector<Self::Impl>],
+    ) -> bool {
+        let in_selector_list_of = self.in_selector_list_of | list_kind;
         for selector in list {
             let mut nested = StylistSelectorVisitor {
                 passed_rightmost_selector: false,
                 needs_revalidation: &mut *self.needs_revalidation,
-                attribute_dependencies: &mut *self.attribute_dependencies,
-                state_dependencies: &mut *self.state_dependencies,
-                document_state_dependencies: &mut *self.document_state_dependencies,
+                in_selector_list_of,
                 mapped_ids: &mut *self.mapped_ids,
+                nth_of_mapped_ids: &mut *self.nth_of_mapped_ids,
+                attribute_dependencies: &mut *self.attribute_dependencies,
+                nth_of_class_dependencies: &mut *self.nth_of_class_dependencies,
+                nth_of_attribute_dependencies: &mut *self.nth_of_attribute_dependencies,
+                state_dependencies: &mut *self.state_dependencies,
+                nth_of_state_dependencies: &mut *self.nth_of_state_dependencies,
+                document_state_dependencies: &mut *self.document_state_dependencies,
             };
             let _ret = selector.visit(&mut nested);
             debug_assert!(_ret, "We never return false");
@@ -1966,8 +2002,15 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
         name: &LocalName,
         lower_name: &LocalName,
     ) -> bool {
+        if self.in_selector_list_of.in_nth_of() {
+            self.nth_of_attribute_dependencies.insert(name.clone());
+            self.nth_of_attribute_dependencies
+                .insert(lower_name.clone());
+        }
+
         self.attribute_dependencies.insert(name.clone());
         self.attribute_dependencies.insert(lower_name.clone());
+
         true
     }
 
@@ -1980,8 +2023,12 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
                 self.state_dependencies.insert(p.state_flag());
                 self.document_state_dependencies
                     .insert(p.document_state_flag());
+
+                if self.in_selector_list_of.in_nth_of() {
+                    self.nth_of_state_dependencies.insert(p.state_flag());
+                }
             },
-            Component::ID(ref id) if !self.passed_rightmost_selector => {
+            Component::ID(ref id) => {
                 // We want to stop storing mapped ids as soon as we've moved off
                 // the rightmost ComplexSelector that is not a pseudo-element.
                 //
@@ -1993,7 +2040,16 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
                 //
                 // NOTE(emilio): See the comment regarding on when this may
                 // break in visit_complex_selector.
+                if !self.passed_rightmost_selector {
                     self.mapped_ids.insert(id.0.clone());
+                }
+
+                if self.in_selector_list_of.in_nth_of() {
+                    self.nth_of_mapped_ids.insert(id.0.clone());
+                }
+            },
+            Component::Class(ref class) if self.in_selector_list_of.in_nth_of() => {
+                self.nth_of_class_dependencies.insert(class.0.clone());
             },
             _ => {},
         }
@@ -2181,11 +2237,25 @@ pub struct CascadeData {
     /// rare.)
     attribute_dependencies: PrecomputedHashSet<LocalName>,
 
+    /// The classes that appear in the selector list of
+    /// :nth-child(... of <selector list>). Used to avoid restyling siblings of
+    /// an element when an irrelevant class changes.
+    nth_of_class_dependencies: PrecomputedHashSet<Atom>,
+
+    /// The attributes that appear in the selector list of
+    /// :nth-child(... of <selector list>). Used to avoid restyling siblings of
+    /// an element when an irrelevant attribute changes.
+    nth_of_attribute_dependencies: PrecomputedHashSet<LocalName>,
+
     /// The element state bits that are relied on by selectors.  Like
     /// `attribute_dependencies`, this is used to avoid taking element snapshots
     /// when an irrelevant element state bit changes.
     state_dependencies: ElementState,
 
+    /// The element state bits that are relied on by selectors that appear in
+    /// the selector list of :nth-child(... of <selector list>).
+    nth_of_state_dependencies: ElementState,
+
     /// The document state bits that are relied on by selectors.  This is used
     /// to tell whether we need to restyle the entire document when a document
     /// state bit changes.
@@ -2197,6 +2267,11 @@ pub struct CascadeData {
     /// filter, and hence might be in one of our selector maps.
     mapped_ids: PrecomputedHashSet<Atom>,
 
+    /// The IDs that appear in the selector list of
+    /// :nth-child(... of <selector list>). Used to avoid restyling siblings
+    /// of an element when an irrelevant ID changes.
+    nth_of_mapped_ids: PrecomputedHashSet<Atom>,
+
     /// Selectors that require explicit cache revalidation (i.e. which depend
     /// on state that is not otherwise visible to the cache, like attributes or
     /// tree-structural state like child index and pseudos).
@@ -2242,6 +2317,10 @@ impl CascadeData {
             slotted_rules: None,
             part_rules: None,
             invalidation_map: InvalidationMap::new(),
+            nth_of_mapped_ids: PrecomputedHashSet::default(),
+            nth_of_class_dependencies: PrecomputedHashSet::default(),
+            nth_of_attribute_dependencies: PrecomputedHashSet::default(),
+            nth_of_state_dependencies: ElementState::empty(),
             attribute_dependencies: PrecomputedHashSet::default(),
             state_dependencies: ElementState::empty(),
             document_state_dependencies: DocumentState::empty(),
@@ -2322,6 +2401,13 @@ impl CascadeData {
         self.state_dependencies.intersects(state)
     }
 
+    /// Returns whether the given ElementState bit is relied upon by a selector
+    /// of some rule in the selector list of :nth-child(... of <selector list>).
+    #[inline]
+    pub fn has_nth_of_state_dependency(&self, state: ElementState) -> bool {
+        self.nth_of_state_dependencies.intersects(state)
+    }
+
     /// Returns whether the given attribute might appear in an attribute
     /// selector of some rule.
     #[inline]
@@ -2329,6 +2415,27 @@ impl CascadeData {
         self.attribute_dependencies.contains(local_name)
     }
 
+    /// Returns whether the given ID might appear in an ID selector in the
+    /// selector list of :nth-child(... of <selector list>).
+    #[inline]
+    pub fn might_have_nth_of_id_dependency(&self, id: &Atom) -> bool {
+        self.nth_of_mapped_ids.contains(id)
+    }
+
+    /// Returns whether the given class might appear in a class selector in the
+    /// selector list of :nth-child(... of <selector list>).
+    #[inline]
+    pub fn might_have_nth_of_class_dependency(&self, class: &Atom) -> bool {
+        self.nth_of_class_dependencies.contains(class)
+    }
+
+    /// Returns whether the given attribute might appear in an attribute
+    /// selector in the selector list of :nth-child(... of <selector list>).
+    #[inline]
+    pub fn might_have_nth_of_attribute_dependency(&self, local_name: &LocalName) -> bool {
+        self.nth_of_attribute_dependencies.contains(local_name)
+    }
+
     /// Returns the normal rule map for a given pseudo-element.
     #[inline]
     pub fn normal_rules(&self, pseudo: Option<&PseudoElement>) -> Option<&SelectorMap<Rule>> {
@@ -2419,6 +2526,9 @@ impl CascadeData {
         }
         self.invalidation_map.shrink_if_needed();
         self.attribute_dependencies.shrink_if_needed();
+        self.nth_of_attribute_dependencies.shrink_if_needed();
+        self.nth_of_class_dependencies.shrink_if_needed();
+        self.nth_of_mapped_ids.shrink_if_needed();
         self.mapped_ids.shrink_if_needed();
         self.layer_id.shrink_if_needed();
         self.selectors_for_cache_revalidation.shrink_if_needed();
@@ -2584,12 +2694,17 @@ impl CascadeData {
                             let mut visitor = StylistSelectorVisitor {
                                 needs_revalidation: &mut needs_revalidation,
                                 passed_rightmost_selector: false,
-                                attribute_dependencies: &mut self.attribute_dependencies,
-                                state_dependencies: &mut self.state_dependencies,
-                                document_state_dependencies: &mut self.document_state_dependencies,
+                                in_selector_list_of: SelectorListKind::default(),
                                 mapped_ids: &mut self.mapped_ids,
+                                nth_of_mapped_ids: &mut self.nth_of_mapped_ids,
+                                attribute_dependencies: &mut self.attribute_dependencies,
+                                nth_of_class_dependencies: &mut self.nth_of_class_dependencies,
+                                nth_of_attribute_dependencies: &mut self
+                                    .nth_of_attribute_dependencies,
+                                state_dependencies: &mut self.state_dependencies,
+                                nth_of_state_dependencies: &mut self.nth_of_state_dependencies,
+                                document_state_dependencies: &mut self.document_state_dependencies,
                             };
-
                             rule.selector.visit(&mut visitor);
 
                             if needs_revalidation {
@@ -3018,9 +3133,13 @@ impl CascadeData {
         self.clear_cascade_data();
         self.invalidation_map.clear();
         self.attribute_dependencies.clear();
+        self.nth_of_attribute_dependencies.clear();
+        self.nth_of_class_dependencies.clear();
         self.state_dependencies = ElementState::empty();
+        self.nth_of_state_dependencies = ElementState::empty();
         self.document_state_dependencies = DocumentState::empty();
         self.mapped_ids.clear();
+        self.nth_of_mapped_ids.clear();
         self.selectors_for_cache_revalidation.clear();
         self.effective_media_query_results.clear();
     }
@@ -3168,18 +3287,27 @@ size_of_test!(Rule, 40);
 
 /// A function to be able to test the revalidation stuff.
 pub fn needs_revalidation_for_testing(s: &Selector<SelectorImpl>) -> bool {
-    let mut attribute_dependencies = Default::default();
-    let mut mapped_ids = Default::default();
-    let mut state_dependencies = ElementState::empty();
-    let mut document_state_dependencies = DocumentState::empty();
     let mut needs_revalidation = false;
+    let mut mapped_ids = Default::default();
+    let mut nth_of_mapped_ids = Default::default();
+    let mut attribute_dependencies = Default::default();
+    let mut nth_of_class_dependencies = Default::default();
+    let mut nth_of_attribute_dependencies = Default::default();
+    let mut state_dependencies = ElementState::empty();
+    let mut nth_of_state_dependencies = ElementState::empty();
+    let mut document_state_dependencies = DocumentState::empty();
     let mut visitor = StylistSelectorVisitor {
         passed_rightmost_selector: false,
         needs_revalidation: &mut needs_revalidation,
-        attribute_dependencies: &mut attribute_dependencies,
-        state_dependencies: &mut state_dependencies,
-        document_state_dependencies: &mut document_state_dependencies,
+        in_selector_list_of: SelectorListKind::default(),
         mapped_ids: &mut mapped_ids,
+        nth_of_mapped_ids: &mut nth_of_mapped_ids,
+        attribute_dependencies: &mut attribute_dependencies,
+        nth_of_class_dependencies: &mut nth_of_class_dependencies,
+        nth_of_attribute_dependencies: &mut nth_of_attribute_dependencies,
+        state_dependencies: &mut state_dependencies,
+        nth_of_state_dependencies: &mut nth_of_state_dependencies,
+        document_state_dependencies: &mut document_state_dependencies,
     };
     s.visit(&mut visitor);
     needs_revalidation