Auto merge of #19817 - emilio:matching-context-visited, r=nox

style: Track the visited-handling-mode on the MatchingContext.

This fixes bugs where we're not passing the value around correctly, like from
::-moz-any.

This is a fix for https://bugzilla.mozilla.org/show_bug.cgi?id=1431539.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19817)
<!-- Reviewable:end -->

components/layout_thread/dom_wrapper.rs

@@ -713,7 +713,6 @@ impl<'le> ::selectors::Element for ServoLayoutElement<'le> {
         &self,
         pseudo_class: &NonTSPseudoClass,
         _: &mut MatchingContext<Self::Impl>,
-        _: VisitedHandlingMode,
         _: &mut F,
     ) -> bool
     where
@@ -1221,7 +1220,6 @@ impl<'le> ::selectors::Element for ServoThreadSafeLayoutElement<'le> {
         &self,
         _: &NonTSPseudoClass,
         _: &mut MatchingContext<Self::Impl>,
-        _: VisitedHandlingMode,
         _: &mut F,
     ) -> bool
     where

components/script/dom/element.rs

@@ -89,7 +89,6 @@ use script_layout_interface::message::ReflowGoal;
 use script_thread::ScriptThread;
 use selectors::Element as SelectorsElement;
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
-use selectors::context::VisitedHandlingMode;
 use selectors::matching::{ElementSelectorFlags, MatchingContext};
 use selectors::sink::Push;
 use servo_arc::Arc;
@@ -2636,7 +2635,6 @@ impl<'a> SelectorsElement for DomRoot<Element> {
         &self,
         pseudo_class: &NonTSPseudoClass,
         _: &mut MatchingContext<Self::Impl>,
-        _: VisitedHandlingMode,
         _: &mut F,
     ) -> bool
     where

components/selectors/context.rs

@@ -105,8 +105,6 @@ where
     pub bloom_filter: Option<&'a BloomFilter>,
     /// An optional cache to speed up nth-index-like selectors.
     pub nth_index_cache: Option<&'a mut NthIndexCache>,
-    /// Input that controls how matching for links is handled.
-    pub visited_handling: VisitedHandlingMode,
     /// The element which is going to match :scope pseudo-class. It can be
     /// either one :scope element, or the scoping element.
     ///
@@ -120,11 +118,14 @@ where
     /// See https://drafts.csswg.org/selectors-4/#scope-pseudo
     pub scope_element: Option<OpaqueElement>,
 
+    /// Controls how matching for links is handled.
+    visited_handling: VisitedHandlingMode,
+
     /// The current nesting level of selectors that we're matching.
     ///
-    /// FIXME(emilio): Move this somewhere else and make MatchingContext
+    /// FIXME(emilio): Consider putting the mutable stuff in a Cell, then make
-    /// immutable again.
+    /// MatchingContext immutable again.
-    pub nesting_level: usize,
+    nesting_level: usize,
 
     /// An optional hook function for checking whether a pseudo-element
     /// should match when matching_mode is ForStatelessPseudoElement.
@@ -181,6 +182,12 @@ where
         }
     }
 
+    /// Whether we're matching a nested selector.
+    #[inline]
+    pub fn is_nested(&self) -> bool {
+        self.nesting_level != 0
+    }
+
     /// The quirks mode of the document.
     #[inline]
     pub fn quirks_mode(&self) -> QuirksMode {
@@ -192,4 +199,38 @@ where
     pub fn classes_and_ids_case_sensitivity(&self) -> CaseSensitivity {
         self.classes_and_ids_case_sensitivity
     }
+
+    /// Runs F with a deeper nesting level.
+    #[inline]
+    pub fn nest<F, R>(&mut self, f: F) -> R
+    where
+        F: FnOnce(&mut Self) -> R,
+    {
+        self.nesting_level += 1;
+        let result = f(self);
+        self.nesting_level -= 1;
+        result
+    }
+
+    #[inline]
+    pub fn visited_handling(&self) -> VisitedHandlingMode {
+        self.visited_handling
+    }
+
+    /// Runs F with a different VisitedHandlingMode.
+    #[inline]
+    pub fn with_visited_handling_mode<F, R>(
+        &mut self,
+        handling_mode: VisitedHandlingMode,
+        f: F,
+    ) -> R
+    where
+        F: FnOnce(&mut Self) -> R,
+    {
+        let original_handling_mode = self.visited_handling;
+        self.visited_handling = handling_mode;
+        let result = f(self);
+        self.visited_handling = original_handling_mode;
+        result
+    }
 }

components/selectors/matching.rs

@@ -60,8 +60,7 @@ impl ElementSelectorFlags {
 /// Holds per-compound-selector data.
 struct LocalMatchingContext<'a, 'b: 'a, Impl: SelectorImpl> {
     shared: &'a mut MatchingContext<'b, Impl>,
-    matches_hover_and_active_quirk: bool,
+    matches_hover_and_active_quirk: MatchesHoverAndActiveQuirk,
-    visited_handling: VisitedHandlingMode,
 }
 
 #[inline(always)]
@@ -171,6 +170,15 @@ enum SelectorMatchingResult {
     NotMatchedGlobally,
 }
 
+/// Whether the :hover and :active quirk applies.
+///
+/// https://quirks.spec.whatwg.org/#the-active-and-hover-quirk
+#[derive(Clone, Copy, Debug, PartialEq)]
+enum MatchesHoverAndActiveQuirk {
+    Yes,
+    No,
+}
+
 /// Matches a selector, fast-rejecting against a bloom filter.
 ///
 /// We accept an offset to allow consumers to represent and match against
@@ -236,11 +244,9 @@ where
         selector.combinator_at_parse_order(from_offset - 1); // This asserts.
     }
 
-    let visited_handling = context.visited_handling;
     let mut local_context = LocalMatchingContext {
         shared: context,
-        visited_handling,
+        matches_hover_and_active_quirk: MatchesHoverAndActiveQuirk::No,
-        matches_hover_and_active_quirk: false,
     };
 
     for component in selector.iter_raw_parse_order_from(from_offset) {
@@ -280,7 +286,7 @@ where
     // If this is the special pseudo-element mode, consume the ::pseudo-element
     // before proceeding, since the caller has already handled that part.
     if context.matching_mode == MatchingMode::ForStatelessPseudoElement &&
-        context.nesting_level == 0 {
+        !context.is_nested() {
         // Consume the pseudo.
         match *iter.next().unwrap() {
             Component::PseudoElement(ref pseudo) => {
@@ -312,12 +318,10 @@ where
         }
     }
 
-    let visited_handling = context.visited_handling;
     let result = matches_complex_selector_internal(
         iter,
         element,
         context,
-        visited_handling,
         flags_setter,
         Rightmost::Yes,
     );
@@ -333,23 +337,23 @@ fn matches_hover_and_active_quirk<Impl: SelectorImpl>(
     selector_iter: &SelectorIter<Impl>,
     context: &MatchingContext<Impl>,
     rightmost: Rightmost,
-) -> bool {
+) -> MatchesHoverAndActiveQuirk {
     if context.quirks_mode() != QuirksMode::Quirks {
-        return false;
+        return MatchesHoverAndActiveQuirk::No;
     }
 
-    if context.nesting_level != 0 {
+    if context.is_nested() {
-        return false;
+        return MatchesHoverAndActiveQuirk::No;
     }
 
     // This compound selector had a pseudo-element to the right that we
     // intentionally skipped.
-    if matches!(rightmost, Rightmost::Yes) &&
+    if rightmost == Rightmost::Yes &&
         context.matching_mode == MatchingMode::ForStatelessPseudoElement {
-        return false;
+        return MatchesHoverAndActiveQuirk::No;
     }
 
-    selector_iter.clone().all(|simple| {
+    let all_match = selector_iter.clone().all(|simple| {
         match *simple {
             Component::LocalName(_) |
             Component::AttributeInNoNamespaceExists { .. } |
@@ -375,9 +379,16 @@ fn matches_hover_and_active_quirk<Impl: SelectorImpl>(
             },
             _ => true,
         }
-    })
+    });
+
+    if all_match {
+        MatchesHoverAndActiveQuirk::Yes
+    } else {
+        MatchesHoverAndActiveQuirk::No
+    }
 }
 
+#[derive(Clone, Copy, PartialEq)]
 enum Rightmost {
     Yes,
     No,
@@ -417,7 +428,6 @@ fn matches_complex_selector_internal<E, F>(
     mut selector_iter: SelectorIter<E::Impl>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    visited_handling: VisitedHandlingMode,
     flags_setter: &mut F,
     rightmost: Rightmost,
 ) -> SelectorMatchingResult
@@ -431,7 +441,6 @@ where
         &mut selector_iter,
         element,
         context,
-        visited_handling,
         flags_setter,
         rightmost
     );
@@ -467,11 +476,10 @@ where
 
     // Stop matching :visited as soon as we find a link, or a combinator for
     // something that isn't an ancestor.
-    let mut visited_handling =
+    let mut visited_handling = if element.is_link() || combinator.is_sibling() {
-        if element.is_link() || combinator.is_sibling() {
         VisitedHandlingMode::AllLinksUnvisited
     } else {
-            visited_handling
+        context.visited_handling()
     };
 
     loop {
@@ -480,14 +488,16 @@ where
             Some(next_element) => next_element,
         };
 
-        let result = matches_complex_selector_internal(
+        let result =
+            context.with_visited_handling_mode(visited_handling, |context| {
+                matches_complex_selector_internal(
                     selector_iter.clone(),
                     &element,
                     context,
-            visited_handling,
                     flags_setter,
                     Rightmost::No,
-        );
+                )
+            });
 
         match (result, combinator) {
             // Return the status immediately.
@@ -521,12 +531,9 @@ where
             _ => {},
         }
 
-        visited_handling =
         if element.is_link() || combinator.is_sibling() {
-                VisitedHandlingMode::AllLinksUnvisited
+            visited_handling = VisitedHandlingMode::AllLinksUnvisited;
-            } else {
+        }
-                visited_handling
-            };
 
         next_element = next_element_for_combinator(&element, combinator);
     }
@@ -554,7 +561,6 @@ fn matches_compound_selector<E, F>(
     selector_iter: &mut SelectorIter<E::Impl>,
     element: &E,
     context: &mut MatchingContext<E::Impl>,
-    visited_handling: VisitedHandlingMode,
     flags_setter: &mut F,
     rightmost: Rightmost,
 ) -> bool
@@ -596,7 +602,6 @@ where
     let mut local_context =
         LocalMatchingContext {
             shared: context,
-            visited_handling,
             matches_hover_and_active_quirk,
         };
     iter::once(selector).chain(selector_iter).all(|simple| {
@@ -623,17 +628,15 @@ where
     match *selector {
         Component::Combinator(_) => unreachable!(),
         Component::Slotted(ref selector) => {
-            context.shared.nesting_level += 1;
+            context.shared.nest(|context| {
-            let result =
                 element.assigned_slot().is_some() &&
                 matches_complex_selector(
                     selector.iter(),
                     element,
-                    context.shared,
+                    context,
                     flags_setter,
-                );
+                )
-            context.shared.nesting_level -= 1;
+            })
-            result
         }
         Component::PseudoElement(ref pseudo) => {
             element.match_pseudo_element(pseudo, context.shared)
@@ -714,17 +717,17 @@ where
             )
         }
         Component::NonTSPseudoClass(ref pc) => {
-            if context.matches_hover_and_active_quirk &&
+            if context.matches_hover_and_active_quirk == MatchesHoverAndActiveQuirk::Yes &&
-               context.shared.nesting_level == 0 &&
+               !context.shared.is_nested() &&
                E::Impl::is_active_or_hover(pc) &&
-               !element.is_link() {
+               !element.is_link()
+            {
                 return false;
             }
 
             element.match_non_ts_pseudo_class(
                 pc,
                 &mut context.shared,
-                context.visited_handling,
                 flags_setter
             )
         }
@@ -774,17 +777,20 @@ where
             matches_generic_nth_child(element, context, 0, 1, true, true, flags_setter)
         }
         Component::Negation(ref negated) => {
-            context.shared.nesting_level += 1;
+            context.shared.nest(|context| {
-            let result = !negated.iter().all(|ss| {
+                let mut local_context = LocalMatchingContext {
+                    matches_hover_and_active_quirk: MatchesHoverAndActiveQuirk::No,
+                    shared: context,
+                };
+                !negated.iter().all(|ss| {
                     matches_simple_selector(
                         ss,
                         element,
-                    context,
+                        &mut local_context,
                         flags_setter,
                     )
-            });
+                })
-            context.shared.nesting_level -= 1;
+            })
-            result
         }
     }
 }

components/selectors/tree.rs

@@ -6,7 +6,6 @@
 //! between layout and style.
 
 use attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
-use context::VisitedHandlingMode;
 use matching::{ElementSelectorFlags, MatchingContext};
 use parser::SelectorImpl;
 use servo_arc::NonZeroPtrMut;
@@ -69,7 +68,6 @@ pub trait Element: Sized + Clone + Debug {
         &self,
         pc: &<Self::Impl as SelectorImpl>::NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        visited_handling: VisitedHandlingMode,
         flags_setter: &mut F,
     ) -> bool
     where

components/style/gecko/wrapper.rs

@@ -1997,7 +1997,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
         &self,
         pseudo_class: &NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        visited_handling: VisitedHandlingMode,
         flags_setter: &mut F,
     ) -> bool
     where
@@ -2057,10 +2056,10 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
             },
             NonTSPseudoClass::AnyLink => self.is_link(),
             NonTSPseudoClass::Link => {
-                self.is_link() && visited_handling.matches_unvisited()
+                self.is_link() && context.visited_handling().matches_unvisited()
             }
             NonTSPseudoClass::Visited => {
-                self.is_link() && visited_handling.matches_visited()
+                self.is_link() && context.visited_handling().matches_visited()
             }
             NonTSPseudoClass::MozFirstNode => {
                 flags_setter(self, ElementSelectorFlags::HAS_EDGE_CHILD_SELECTOR);
@@ -2119,12 +2118,11 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
             }
             NonTSPseudoClass::MozPlaceholder => false,
             NonTSPseudoClass::MozAny(ref sels) => {
-                context.nesting_level += 1;
+                context.nest(|context| {
-                let result = sels.iter().any(|s| {
+                    sels.iter().any(|s| {
                         matches_complex_selector(s.iter(), self, context, flags_setter)
-                });
+                    })
-                context.nesting_level -= 1;
+                })
-                result
             }
             NonTSPseudoClass::Lang(ref lang_arg) => {
                 self.match_element_lang(None, lang_arg)

components/style/invalidation/element/element_wrapper.rs

@@ -11,7 +11,6 @@ use element_state::ElementState;
 use selector_parser::{NonTSPseudoClass, PseudoElement, SelectorImpl, Snapshot, SnapshotMap, AttrValue};
 use selectors::{Element, OpaqueElement};
 use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
-use selectors::context::VisitedHandlingMode;
 use selectors::matching::{ElementSelectorFlags, MatchingContext};
 use std::cell::Cell;
 use std::fmt;
@@ -150,7 +149,6 @@ impl<'a, E> Element for ElementWrapper<'a, E>
         &self,
         pseudo_class: &NonTSPseudoClass,
         context: &mut MatchingContext<Self::Impl>,
-        visited_handling: VisitedHandlingMode,
         _setter: &mut F,
     ) -> bool
     where
@@ -162,12 +160,11 @@ impl<'a, E> Element for ElementWrapper<'a, E>
             #[cfg(feature = "gecko")]
             NonTSPseudoClass::MozAny(ref selectors) => {
                 use selectors::matching::matches_complex_selector;
-                context.nesting_level += 1;
+                return context.nest(|context| {
-                let result = selectors.iter().any(|s| {
+                    selectors.iter().any(|s| {
                         matches_complex_selector(s.iter(), self, context, _setter)
+                    })
                 });
-                context.nesting_level -= 1;
-                return result
             }
 
             // :dir is implemented in terms of state flags, but which state flag
@@ -199,10 +196,10 @@ impl<'a, E> Element for ElementWrapper<'a, E>
             // Instead, we use the `visited_handling` to determine if they
             // match.
             NonTSPseudoClass::Link => {
-                return self.is_link() && visited_handling.matches_unvisited()
+                return self.is_link() && context.visited_handling().matches_unvisited()
             }
             NonTSPseudoClass::Visited => {
-                return self.is_link() && visited_handling.matches_visited()
+                return self.is_link() && context.visited_handling().matches_visited()
             }
 
             #[cfg(feature = "gecko")]
@@ -237,7 +234,6 @@ impl<'a, E> Element for ElementWrapper<'a, E>
             return self.element.match_non_ts_pseudo_class(
                 pseudo_class,
                 context,
-                visited_handling,
                 &mut |_, _| {},
             )
         }
@@ -247,7 +243,6 @@ impl<'a, E> Element for ElementWrapper<'a, E>
                 self.element.match_non_ts_pseudo_class(
                     pseudo_class,
                     context,
-                    visited_handling,
                     &mut |_, _| {},
                 )
             }

components/style/stylist.rs

@@ -1246,7 +1246,7 @@ impl Stylist {
             // Step 2: Presentational hints.
             let length_before_preshints = applicable_declarations.len();
             element.synthesize_presentational_hints_for_legacy_attributes(
-                context.visited_handling,
+                context.visited_handling(),
                 applicable_declarations
             );
             if applicable_declarations.len() != length_before_preshints {