style: Implement parsing / selector-matching for :is() and :where().

This implements the easy / straight-forward parts of the :where / :is
selectors.

The biggest missing piece is to handle properly invalidation when there
are combinators present inside the :where. That's the hard part of this,
actually.

But this is probably worth landing in the interim. This fixes some of
the visitors that were easy to fix.

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

components/malloc_size_of/lib.rs

@@ -748,6 +748,9 @@ where
             Component::Slotted(ref selector) | Component::Host(Some(ref selector)) => {
                 selector.size_of(ops)
             },
+            Component::Is(ref list) | Component::Where(ref list) => {
+                list.size_of(ops)
+            },
             Component::PseudoElement(ref pseudo) => (*pseudo).size_of(ops),
             Component::Combinator(..) |
             Component::ExplicitAnyNamespace |

components/selectors/builder.rs

@@ -330,6 +330,19 @@ where
                     specificity.class_like_selectors += 1;
                 }
             },
+            Component::Is(ref list) => {
+                // https://drafts.csswg.org/selectors/#specificity-rules:
+                //
+                //     The specificity of an :is() pseudo-class is replaced by the
+                //     specificity of the most specific complex selector in its
+                //     selector list argument.
+                let mut max = 0;
+                for selector in &**list {
+                    max = std::cmp::max(selector.specificity(), max);
+                }
+                *specificity += Specificity::from(max);
+            },
+            Component::Where(..) |
             Component::ExplicitUniversalType |
             Component::ExplicitAnyNamespace |
             Component::ExplicitNoNamespace |

components/selectors/matching.rs

@@ -851,6 +851,14 @@ where
             matches_generic_nth_child(element, context, 0, 1, true, false, flags_setter) &&
                 matches_generic_nth_child(element, context, 0, 1, true, true, flags_setter)
         },
+        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) {
+                    return true;
+                }
+            }
+            false
+        }),
         Component::Negation(ref negated) => context.shared.nest_for_negation(|context| {
             let mut local_context = LocalMatchingContext {
                 matches_hover_and_active_quirk: MatchesHoverAndActiveQuirk::No,

components/selectors/parser.rs

@@ -9,7 +9,7 @@ use crate::bloom::BLOOM_HASH_MASK;
 use crate::builder::{SelectorBuilder, SelectorFlags, SpecificityAndFlags};
 use crate::context::QuirksMode;
 use crate::sink::Push;
-pub use crate::visitor::{SelectorVisitor, Visit};
+pub use crate::visitor::SelectorVisitor;
 use cssparser::{parse_nth, serialize_identifier};
 use cssparser::{BasicParseError, BasicParseErrorKind, ParseError, ParseErrorKind};
 use cssparser::{CowRcStr, Delimiter, SourceLocation};
@@ -55,6 +55,13 @@ pub trait NonTSPseudoClass: Sized + ToCss {
 
     /// Whether this pseudo-class has zero specificity.
     fn has_zero_specificity(&self) -> bool;
+
+    fn visit<V>(&self, _visitor: &mut V) -> bool
+    where
+        V: SelectorVisitor<Impl = Self::Impl>,
+    {
+        true
+    }
 }
 
 /// Returns a Cow::Borrowed if `s` is already ASCII lowercase, and a
@@ -147,6 +154,7 @@ pub enum SelectorParseErrorKind<'i> {
     NonCompoundSelector,
     NonPseudoElementAfterSlotted,
     InvalidPseudoElementAfterSlotted,
+    InvalidPseudoElementInsideWhere,
     InvalidState,
     UnexpectedTokenInAttributeSelector(Token<'i>),
     PseudoElementExpectedColon(Token<'i>),
@@ -226,6 +234,11 @@ pub trait Parser<'i> {
         false
     }
 
+    /// Whether to parse the `:where` pseudo-class.
+    fn parse_is_and_where(&self) -> bool {
+        false
+    }
+
     /// Whether to parse the `:host` pseudo-class.
     fn parse_host(&self) -> bool {
         false
@@ -441,128 +454,6 @@ impl AncestorHashes {
     }
 }
 
-impl<Impl: SelectorImpl> Visit for Selector<Impl>
-where
-    Impl::NonTSPseudoClass: Visit<Impl = Impl>,
-{
-    type Impl = Impl;
-
-    fn visit<V>(&self, visitor: &mut V) -> bool
-    where
-        V: SelectorVisitor<Impl = Impl>,
-    {
-        let mut current = self.iter();
-        let mut combinator = None;
-        loop {
-            if !visitor.visit_complex_selector(combinator) {
-                return false;
-            }
-
-            for selector in &mut current {
-                if !selector.visit(visitor) {
-                    return false;
-                }
-            }
-
-            combinator = current.next_sequence();
-            if combinator.is_none() {
-                break;
-            }
-        }
-
-        true
-    }
-}
-
-impl<Impl: SelectorImpl> Visit for Component<Impl>
-where
-    Impl::NonTSPseudoClass: Visit<Impl = Impl>,
-{
-    type Impl = Impl;
-
-    fn visit<V>(&self, visitor: &mut V) -> bool
-    where
-        V: SelectorVisitor<Impl = Impl>,
-    {
-        use self::Component::*;
-        if !visitor.visit_simple_selector(self) {
-            return false;
-        }
-
-        match *self {
-            Slotted(ref selector) => {
-                if !selector.visit(visitor) {
-                    return false;
-                }
-            },
-            Host(Some(ref selector)) => {
-                if !selector.visit(visitor) {
-                    return false;
-                }
-            },
-            Negation(ref negated) => {
-                for component in negated.iter() {
-                    if !component.visit(visitor) {
-                        return false;
-                    }
-                }
-            },
-
-            AttributeInNoNamespaceExists {
-                ref local_name,
-                ref local_name_lower,
-            } => {
-                if !visitor.visit_attribute_selector(
-                    &NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()),
-                    local_name,
-                    local_name_lower,
-                ) {
-                    return false;
-                }
-            },
-            AttributeInNoNamespace {
-                ref local_name,
-                never_matches,
-                ..
-            } if !never_matches => {
-                if !visitor.visit_attribute_selector(
-                    &NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()),
-                    local_name,
-                    local_name,
-                ) {
-                    return false;
-                }
-            },
-            AttributeOther(ref attr_selector) if !attr_selector.never_matches => {
-                let empty_string;
-                let namespace = match attr_selector.namespace() {
-                    Some(ns) => ns,
-                    None => {
-                        empty_string = crate::parser::namespace_empty_string::<Impl>();
-                        NamespaceConstraint::Specific(&empty_string)
-                    },
-                };
-                if !visitor.visit_attribute_selector(
-                    &namespace,
-                    &attr_selector.local_name,
-                    &attr_selector.local_name_lower,
-                ) {
-                    return false;
-                }
-            },
-
-            NonTSPseudoClass(ref pseudo_class) => {
-                if !pseudo_class.visit(visitor) {
-                    return false;
-                }
-            },
-            _ => {},
-        }
-
-        true
-    }
-}
-
 pub fn namespace_empty_string<Impl: SelectorImpl>() -> Impl::NamespaceUrl {
     // Rust type’s default, not default namespace
     Impl::NamespaceUrl::default()
@@ -781,6 +672,50 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     pub fn thin_arc_heap_ptr(&self) -> *const ::std::os::raw::c_void {
         self.0.heap_ptr()
     }
+
+    /// Traverse selector components inside `self`.
+    ///
+    /// Implementations of this method should call `SelectorVisitor` methods
+    /// or other impls of `Visit` as appropriate based on the fields of `Self`.
+    ///
+    /// A return value of `false` indicates terminating the traversal.
+    /// It should be propagated with an early return.
+    /// On the contrary, `true` indicates that all fields of `self` have been traversed:
+    ///
+    /// ```rust,ignore
+    /// if !visitor.visit_simple_selector(&self.some_simple_selector) {
+    ///     return false;
+    /// }
+    /// if !self.some_component.visit(visitor) {
+    ///     return false;
+    /// }
+    /// true
+    /// ```
+    pub fn visit<V>(&self, visitor: &mut V) -> bool
+    where
+        V: SelectorVisitor<Impl = Impl>,
+    {
+        let mut current = self.iter();
+        let mut combinator = None;
+        loop {
+            if !visitor.visit_complex_selector(combinator) {
+                return false;
+            }
+
+            for selector in &mut current {
+                if !selector.visit(visitor) {
+                    return false;
+                }
+            }
+
+            combinator = current.next_sequence();
+            if combinator.is_none() {
+                break;
+            }
+        }
+
+        true
+    }
 }
 
 #[derive(Clone)]
@@ -1027,6 +962,20 @@ pub enum Component<Impl: SelectorImpl> {
     ///
     /// See https://github.com/w3c/csswg-drafts/issues/2158
     Host(Option<Selector<Impl>>),
+    /// The `:where` pseudo-class.
+    ///
+    /// https://drafts.csswg.org/selectors/#zero-matches
+    ///
+    /// The inner argument is conceptually a SelectorList, but we move the
+    /// selectors to the heap to keep Component small.
+    Where(Box<[Selector<Impl>]>),
+    /// The `:is` pseudo-class.
+    ///
+    /// https://drafts.csswg.org/selectors/#matches-pseudo
+    ///
+    /// Same comment as above re. the argument.
+    Is(Box<[Selector<Impl>]>),
+    /// An implementation-dependent pseudo-element selector.
     PseudoElement(#[shmem(field_bound)] Impl::PseudoElement),
 }
 
@@ -1080,6 +1029,94 @@ impl<Impl: SelectorImpl> Component<Impl> {
             _ => None,
         }
     }
+
+    pub fn visit<V>(&self, visitor: &mut V) -> bool
+    where
+        V: SelectorVisitor<Impl = Impl>,
+    {
+        use self::Component::*;
+        if !visitor.visit_simple_selector(self) {
+            return false;
+        }
+
+        match *self {
+            Slotted(ref selector) => {
+                if !selector.visit(visitor) {
+                    return false;
+                }
+            },
+            Host(Some(ref selector)) => {
+                if !selector.visit(visitor) {
+                    return false;
+                }
+            },
+            Negation(ref negated) => {
+                for component in negated.iter() {
+                    if !component.visit(visitor) {
+                        return false;
+                    }
+                }
+            },
+
+            AttributeInNoNamespaceExists {
+                ref local_name,
+                ref local_name_lower,
+            } => {
+                if !visitor.visit_attribute_selector(
+                    &NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()),
+                    local_name,
+                    local_name_lower,
+                ) {
+                    return false;
+                }
+            },
+            AttributeInNoNamespace {
+                ref local_name,
+                never_matches,
+                ..
+            } if !never_matches => {
+                if !visitor.visit_attribute_selector(
+                    &NamespaceConstraint::Specific(&namespace_empty_string::<Impl>()),
+                    local_name,
+                    local_name,
+                ) {
+                    return false;
+                }
+            },
+            AttributeOther(ref attr_selector) if !attr_selector.never_matches => {
+                let empty_string;
+                let namespace = match attr_selector.namespace() {
+                    Some(ns) => ns,
+                    None => {
+                        empty_string = crate::parser::namespace_empty_string::<Impl>();
+                        NamespaceConstraint::Specific(&empty_string)
+                    },
+                };
+                if !visitor.visit_attribute_selector(
+                    &namespace,
+                    &attr_selector.local_name,
+                    &attr_selector.local_name_lower,
+                ) {
+                    return false;
+                }
+            },
+
+            NonTSPseudoClass(ref pseudo_class) => {
+                if !pseudo_class.visit(visitor) {
+                    return false;
+                }
+            },
+
+            Is(ref list) | Where(ref list) => {
+                if !visitor.visit_selector_list(&list) {
+                    return false;
+                }
+            },
+            _ => {},
+        }
+
+        true
+    }
 }
 
 #[derive(Clone, Eq, PartialEq, ToShmem)]
@@ -1114,21 +1151,29 @@ impl<Impl: SelectorImpl> Debug for LocalName<Impl> {
     }
 }
 
+fn serialize_selector_list<'a, Impl, I, W>(mut iter: I, dest: &mut W) -> fmt::Result
+where
+    Impl: SelectorImpl,
+    I: Iterator<Item = &'a Selector<Impl>>,
+    W: fmt::Write,
+{
+    let first = iter
+        .next()
+        .expect("Empty SelectorList, should contain at least one selector");
+    first.to_css(dest)?;
+    for selector in iter {
+        dest.write_str(", ")?;
+        selector.to_css(dest)?;
+    }
+    Ok(())
+}
+
 impl<Impl: SelectorImpl> ToCss for SelectorList<Impl> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
     where
         W: fmt::Write,
     {
-        let mut iter = self.0.iter();
+        serialize_selector_list(self.0.iter(), dest)
-        let first = iter
-            .next()
-            .expect("Empty SelectorList, should contain at least one selector");
-        first.to_css(dest)?;
-        for selector in iter {
-            dest.write_str(", ")?;
-            selector.to_css(dest)?;
-        }
-        Ok(())
     }
 }
 
@@ -1401,6 +1446,15 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
                 write_affine(dest, a, b)?;
                 dest.write_char(')')
             },
+            Is(ref list) | Where(ref list) => {
+                match *self {
+                    Where(..) => dest.write_str(":where(")?,
+                    Is(..) => dest.write_str(":is(")?,
+                    _ => unreachable!(),
+                }
+                serialize_selector_list(list.iter(), dest)?;
+                dest.write_str(")")
+            },
             NonTSPseudoClass(ref pseudo) => pseudo.to_css(dest),
         }
     }
@@ -2086,6 +2140,28 @@ where
     }
 }
 
+fn parse_is_or_where<'i, 't, P, Impl>(
+    parser: &P,
+    input: &mut CssParser<'i, 't>,
+    component: impl FnOnce(Box<[Selector<Impl>]>) -> Component<Impl>,
+) -> Result<Component<Impl>, ParseError<'i, P::Error>>
+where
+    P: Parser<'i, Impl = Impl>,
+    Impl: SelectorImpl,
+{
+    debug_assert!(parser.parse_is_and_where());
+    let inner = SelectorList::parse(parser, input)?;
+    // https://drafts.csswg.org/selectors/#matches-pseudo:
+    //
+    //     Pseudo-elements cannot be represented by the matches-any
+    //     pseudo-class; they are not valid within :is().
+    //
+    if inner.0.iter().any(|i| i.has_pseudo_element()) {
+        return Err(input.new_custom_error(SelectorParseErrorKind::InvalidPseudoElementInsideWhere));
+    }
+    Ok(component(inner.0.into_vec().into_boxed_slice()))
+}
+
 fn parse_functional_pseudo_class<'i, 't, P, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
@@ -2105,6 +2181,8 @@ where
         "nth-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthOfType)?),
         "nth-last-child" => return Ok(parse_nth_pseudo_class(input, Component::NthLastChild)?),
         "nth-last-of-type" => return Ok(parse_nth_pseudo_class(input, Component::NthLastOfType)?),
+        "is" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, Component::Is),
+        "where" if parser.parse_is_and_where() => return parse_is_or_where(parser, input, Component::Where),
         "host" => return Ok(Component::Host(Some(parse_inner_compound_selector(parser, input)?))),
         "not" => {
             if state.intersects(SelectorParsingState::INSIDE_NEGATION) {
@@ -2396,17 +2474,6 @@ pub mod tests {
         }
     }
 
-    impl Visit for PseudoClass {
-        type Impl = DummySelectorImpl;
-
-        fn visit<V>(&self, _visitor: &mut V) -> bool
-        where
-            V: SelectorVisitor<Impl = Self::Impl>,
-        {
-            true
-        }
-    }
-
     #[derive(Clone, Debug, PartialEq)]
     pub struct DummySelectorImpl;
 
@@ -2469,6 +2536,10 @@ pub mod tests {
             true
         }
 
+        fn parse_is_and_where(&self) -> bool {
+            true
+        }
+
         fn parse_part(&self) -> bool {
             true
         }
@@ -3100,6 +3171,10 @@ pub mod tests {
         assert!(parse("slot::slotted(div,foo)::first-line").is_err());
         assert!(parse("::slotted(div)::before").is_ok());
         assert!(parse("slot::slotted(div,foo)").is_err());
+
+        assert!(parse("foo:where()").is_err());
+        assert!(parse("foo:where(div, foo, .bar baz)").is_ok());
+        assert!(parse("foo:where(::before)").is_err());
     }
 
     #[test]

components/selectors/visitor.rs

@@ -7,13 +7,13 @@
 #![deny(missing_docs)]
 
 use crate::attr::NamespaceConstraint;
-use crate::parser::{Combinator, Component, SelectorImpl};
+use crate::parser::{Combinator, Component, Selector, SelectorImpl};
 
 /// A trait to visit selector properties.
 ///
 /// All the `visit_foo` methods return a boolean indicating whether the
 /// traversal should continue or not.
-pub trait SelectorVisitor {
+pub trait SelectorVisitor: Sized {
     /// The selector implementation this visitor wants to visit.
     type Impl: SelectorImpl;
 
@@ -34,6 +34,19 @@ pub trait SelectorVisitor {
         true
     }
 
+    /// Visit a nested selector list. The caller is responsible to call visit
+    /// into the internal selectors if / as needed.
+    ///
+    /// The default implementation does this.
+    fn visit_selector_list(&mut self, list: &[Selector<Self::Impl>]) -> bool {
+        for nested in list {
+            if !nested.visit(self) {
+                return false;
+            }
+        }
+        true
+    }
+
     /// Visits a complex selector.
     ///
     /// Gets the combinator to the right of the selector, or `None` if the
@@ -42,31 +55,3 @@ pub trait SelectorVisitor {
         true
     }
 }
-
-/// Enables traversing selector components stored in various types
-pub trait Visit {
-    /// The type parameter of selector component types.
-    type Impl: SelectorImpl;
-
-    /// Traverse selector components inside `self`.
-    ///
-    /// Implementations of this method should call `SelectorVisitor` methods
-    /// or other impls of `Visit` as appropriate based on the fields of `Self`.
-    ///
-    /// A return value of `false` indicates terminating the traversal.
-    /// It should be propagated with an early return.
-    /// On the contrary, `true` indicates that all fields of `self` have been traversed:
-    ///
-    /// ```rust,ignore
-    /// if !visitor.visit_simple_selector(&self.some_simple_selector) {
-    ///     return false;
-    /// }
-    /// if !self.some_component.visit(visitor) {
-    ///     return false;
-    /// }
-    /// true
-    /// ```
-    fn visit<V>(&self, visitor: &mut V) -> bool
-    where
-        V: SelectorVisitor<Impl = Self::Impl>;
-}

components/style/gecko/selector_parser.rs

@@ -15,7 +15,7 @@ use crate::values::serialize_atom_identifier;
 use cssparser::{BasicParseError, BasicParseErrorKind, Parser};
 use cssparser::{CowRcStr, SourceLocation, ToCss, Token};
 use selectors::parser::{self as selector_parser, Selector};
-use selectors::parser::{SelectorParseErrorKind, Visit};
+use selectors::parser::SelectorParseErrorKind;
 use selectors::visitor::SelectorVisitor;
 use selectors::SelectorList;
 use std::fmt;
@@ -109,25 +109,6 @@ impl ToCss for NonTSPseudoClass {
     }
 }
 
-impl Visit for NonTSPseudoClass {
-    type Impl = SelectorImpl;
-
-    fn visit<V>(&self, visitor: &mut V) -> bool
-    where
-        V: SelectorVisitor<Impl = Self::Impl>,
-    {
-        if let NonTSPseudoClass::MozAny(ref selectors) = *self {
-            for selector in selectors.iter() {
-                if !selector.visit(visitor) {
-                    return false;
-                }
-            }
-        }
-
-        true
-    }
-}
-
 impl NonTSPseudoClass {
     /// Parses the name and returns a non-ts-pseudo-class if succeeds.
     /// None otherwise. It doesn't check whether the pseudo-class is enabled
@@ -280,6 +261,21 @@ impl ::selectors::parser::NonTSPseudoClass for NonTSPseudoClass {
     fn has_zero_specificity(&self) -> bool {
         matches!(*self, NonTSPseudoClass::MozNativeAnonymousNoSpecificity)
     }
+
+    fn visit<V>(&self, visitor: &mut V) -> bool
+    where
+        V: SelectorVisitor<Impl = Self::Impl>,
+    {
+        if let NonTSPseudoClass::MozAny(ref selectors) = *self {
+            for selector in selectors.iter() {
+                if !selector.visit(visitor) {
+                    return false;
+                }
+            }
+        }
+
+        true
+    }
 }
 
 /// The dummy struct we use to implement our selector parsing.
@@ -354,6 +350,11 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> {
         true
     }
 
+    #[inline]
+    fn parse_is_and_where(&self) -> bool {
+        static_prefs::pref!("layout.css.is-where-selectors.enabled")
+    }
+
     #[inline]
     fn parse_part(&self) -> bool {
         self.chrome_rules_enabled() || static_prefs::pref!("layout.css.shadow-parts.enabled")

components/style/invalidation/element/invalidation_map.rs

@@ -13,7 +13,7 @@ use fallible::FallibleVec;
 use hashglobe::FailedAllocationError;
 use selectors::attr::NamespaceConstraint;
 use selectors::parser::{Combinator, Component};
-use selectors::parser::{Selector, SelectorIter, Visit};
+use selectors::parser::{Selector, SelectorIter};
 use selectors::visitor::SelectorVisitor;
 use smallvec::SmallVec;
 
@@ -322,6 +322,17 @@ impl InvalidationMap {
 }
 
 /// A struct that collects invalidations for a given compound selector.
+///
+/// FIXME(emilio, bug 1509418): :where is mishandled, figure out the right way
+/// to do invalidation for :where when combinators are inside.
+///
+/// Simplest feasible idea seems to be: For each :where branch, if there are
+/// combinators in the branch, treat as its own selector (basically, call
+/// `.note_selector` with the nested selector). That may over-invalidate, but
+/// not too much. If there are no combinators, then behave like we do today and
+/// use the outer selector as a whole. If we care a lot about that potential
+/// over-invalidation where combinators are present then we need more complex
+/// data-structures in `Dependency`.
 struct CompoundSelectorDependencyCollector<'a> {
     /// The state this compound selector is affected by.
     state: ElementState,

components/style/selector_map.rs

@@ -500,6 +500,15 @@ fn specific_bucket_for<'a>(component: &'a Component<SelectorImpl>) -> Bucket<'a>
         // match the slotted <span>.
         Component::Slotted(ref selector) => find_bucket(selector.iter()),
         Component::Host(Some(ref selector)) => find_bucket(selector.iter()),
+        Component::Is(ref list) | Component::Where(ref list) => {
+            if list.len() == 1 {
+                find_bucket(list[0].iter())
+            } else {
+                // TODO(emilio): We should handle the multiple element case by
+                // inserting multiple entries in the map.
+                Bucket::Universal
+            }
+        },
         _ => Bucket::Universal,
     }
 }

components/style/servo/selector_parser.rs

@@ -19,7 +19,7 @@ use crate::{Atom, CaseSensitivityExt, LocalName, Namespace, Prefix};
 use cssparser::{serialize_identifier, CowRcStr, Parser as CssParser, SourceLocation, ToCss};
 use fxhash::FxHashMap;
 use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
-use selectors::parser::{SelectorParseErrorKind, Visit};
+use selectors::parser::SelectorParseErrorKind;
 use selectors::visitor::SelectorVisitor;
 use std::fmt;
 use std::mem;
@@ -315,8 +315,16 @@ impl ::selectors::parser::NonTSPseudoClass for NonTSPseudoClass {
     fn has_zero_specificity(&self) -> bool {
         false
     }
+
+    fn visit<V>(&self, _: &mut V) -> bool
+    where
+        V: SelectorVisitor<Impl = Self::Impl>,
+    {
+        true
+    }
 }
 
+
 impl ToCss for NonTSPseudoClass {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
     where
@@ -352,17 +360,6 @@ impl ToCss for NonTSPseudoClass {
     }
 }
 
-impl Visit for NonTSPseudoClass {
-    type Impl = SelectorImpl;
-
-    fn visit<V>(&self, _: &mut V) -> bool
-    where
-        V: SelectorVisitor<Impl = Self::Impl>,
-    {
-        true
-    }
-}
-
 impl NonTSPseudoClass {
     /// Gets a given state flag for this pseudo-class. This is used to do
     /// selector matching, and it's set from the DOM.

components/style/stylist.rs

@@ -42,8 +42,7 @@ use selectors::attr::{CaseSensitivity, NamespaceConstraint};
 use selectors::bloom::BloomFilter;
 use selectors::matching::VisitedHandlingMode;
 use selectors::matching::{matches_selector, ElementSelectorFlags, MatchingContext, MatchingMode};
-use selectors::parser::{AncestorHashes, Combinator, Component, Selector};
+use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorIter};
-use selectors::parser::{SelectorIter, Visit};
 use selectors::visitor::SelectorVisitor;
 use selectors::NthIndexCache;
 use servo_arc::{Arc, ArcBorrow};
@@ -1530,11 +1529,11 @@ impl SelectorMapEntry for RevalidationSelectorAndHashes {
 /// A selector visitor implementation that collects all the state the Stylist
 /// cares about a selector.
 struct StylistSelectorVisitor<'a> {
-    /// Whether the selector needs revalidation for the style sharing cache.
-    needs_revalidation: bool,
     /// 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,
     /// The filter with all the id's getting referenced from rightmost
     /// selectors.
     mapped_ids: &'a mut PrecomputedHashSet<Atom>,
@@ -1582,14 +1581,10 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
     type Impl = SelectorImpl;
 
     fn visit_complex_selector(&mut self, combinator: Option<Combinator>) -> bool {
-        self.needs_revalidation =
+        *self.needs_revalidation =
-            self.needs_revalidation || combinator.map_or(false, |c| c.is_sibling());
+            *self.needs_revalidation || combinator.map_or(false, |c| c.is_sibling());
 
-        // NOTE(emilio): This works properly right now because we can't store
+        // NOTE(emilio): this call happens before we visit any of the simple
-        // complex selectors in nested selectors, otherwise we may need to
-        // rethink this.
-        //
-        // Also, note that this call happens before we visit any of the simple
         // selectors in the next ComplexSelector, so we can use this to skip
         // looking at them.
         self.passed_rightmost_selector = self.passed_rightmost_selector ||
@@ -1598,6 +1593,22 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
         true
     }
 
+    fn visit_selector_list(&mut self, list: &[Selector<Self::Impl>]) -> bool {
+        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,
+                mapped_ids: &mut *self.mapped_ids,
+            };
+            let _ret = selector.visit(&mut nested);
+            debug_assert!(_ret, "We never return false");
+        }
+        true
+    }
+
     fn visit_attribute_selector(
         &mut self,
         _ns: &NamespaceConstraint<&Namespace>,
@@ -1610,7 +1621,7 @@ impl<'a> SelectorVisitor for StylistSelectorVisitor<'a> {
     }
 
     fn visit_simple_selector(&mut self, s: &Component<SelectorImpl>) -> bool {
-        self.needs_revalidation = self.needs_revalidation ||
+        *self.needs_revalidation = *self.needs_revalidation ||
             component_needs_revalidation(s, self.passed_rightmost_selector);
 
         match *s {
@@ -2022,8 +2033,9 @@ impl CascadeData {
 
                         if rebuild_kind.should_rebuild_invalidation() {
                             self.invalidation_map.note_selector(selector, quirks_mode)?;
+                            let mut needs_revalidation = false;
                             let mut visitor = StylistSelectorVisitor {
-                                needs_revalidation: false,
+                                needs_revalidation: &mut needs_revalidation,
                                 passed_rightmost_selector: false,
                                 attribute_dependencies: &mut self.attribute_dependencies,
                                 state_dependencies: &mut self.state_dependencies,
@@ -2033,7 +2045,7 @@ impl CascadeData {
 
                             rule.selector.visit(&mut visitor);
 
-                            if visitor.needs_revalidation {
+                            if needs_revalidation {
                                 self.selectors_for_cache_revalidation.insert(
                                     RevalidationSelectorAndHashes::new(
                                         rule.selector.clone(),
@@ -2365,14 +2377,15 @@ pub fn needs_revalidation_for_testing(s: &Selector<SelectorImpl>) -> bool {
     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 visitor = StylistSelectorVisitor {
-        needs_revalidation: false,
         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,
         mapped_ids: &mut mapped_ids,
     };
     s.visit(&mut visitor);
-    visitor.needs_revalidation
+    needs_revalidation
 }