Bug 1368240: Add a way to match a single compound selector, and improve ergonomics of matches_complex_selector. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Fri, 09 Jun 2017 17:18:44 +0200
changeset 592543 84d97987e40e6b4b1b2b30e5dd3365514020b8e3
parent 592542 3f2c9b9eb92b2b9e4efaa78558cd994de5ca96e3
child 592544 7339e70bc8da18585067ca0b001bce6e6e4808ca
push id63430
push userbmo:emilio+bugs@crisal.io
push dateMon, 12 Jun 2017 12:30:48 +0000
reviewersheycam
bugs1368240
milestone55.0a1
Bug 1368240: Add a way to match a single compound selector, and improve ergonomics of matches_complex_selector. r?heycam MozReview-Commit-ID: 9DWDvyZmetM
servo/components/selectors/matching.rs
servo/components/selectors/parser.rs
servo/components/style/gecko/wrapper.rs
--- a/servo/components/selectors/matching.rs
+++ b/servo/components/selectors/matching.rs
@@ -437,19 +437,19 @@ enum SelectorMatchingResult {
     Matched,
     NotMatchedAndRestartFromClosestLaterSibling,
     NotMatchedAndRestartFromClosestDescendant,
     NotMatchedGlobally,
 }
 
 /// Matches a selector, fast-rejecting against a bloom filter.
 ///
-/// We accept an offset to allow consumers to represent and match against partial
-/// selectors (indexed from the right). We use this API design, rather than
-/// having the callers pass a SelectorIter, because creating a SelectorIter
+/// We accept an offset to allow consumers to represent and match against
+/// partial selectors (indexed from the right). We use this API design, rather
+/// than having the callers pass a SelectorIter, because creating a SelectorIter
 /// requires dereferencing the selector to get the length, which adds an
 /// 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>(selector: &Selector<E::Impl>,
                               offset: usize,
                               hashes: &AncestorHashes,
                               element: &E,
@@ -462,34 +462,91 @@ pub fn matches_selector<E, F>(selector: 
     // Use the bloom filter to fast-reject.
     if let Some(filter) = context.bloom_filter {
         if !may_match::<E>(hashes, filter) {
             return false;
         }
     }
 
     let mut local_context = LocalMatchingContext::new(context, selector);
-    matches_complex_selector(&selector, offset, element, &mut local_context, flags_setter)
+    let iter = if offset == 0 {
+        selector.iter()
+    } else {
+        selector.iter_from(offset)
+    };
+    matches_complex_selector(iter, element, &mut local_context, flags_setter)
+}
+
+/// Whether a compound selector matched, and whether it was the rightmost
+/// selector inside the complex selector.
+pub enum CompoundSelectorMatchingResult {
+    /// The compound selector matched, and the next combinator offset is
+    /// `next_combinator_offset`.
+    ///
+    /// If the next combinator offset is zero, it means that it's the rightmost
+    /// selector.
+    Matched { next_combinator_offset: usize, },
+    /// The selector didn't match.
+    NotMatched,
+}
+
+/// Matches a compound selector belonging to `selector`, starting at offset
+/// `from_offset`, matching left to right.
+///
+/// Requires that `from_offset` points to a `Combinator`.
+///
+/// NOTE(emilio): This doesn't allow to match in the leftmost sequence of the
+/// complex selector, but it happens to be the case we don't need it.
+pub fn matches_compound_selector<E>(
+    selector: &Selector<E::Impl>,
+    mut from_offset: usize,
+    context: &mut MatchingContext,
+    element: &E,
+) -> CompoundSelectorMatchingResult
+where
+    E: Element
+{
+    if cfg!(debug_assertions) {
+        selector.combinator_at(from_offset); // This asserts.
+    }
+
+    let mut local_context = LocalMatchingContext::new(context, selector);
+    for component in selector.iter_raw_rev_from(from_offset - 1) {
+        if matches!(*component, Component::Combinator(..)) {
+            return CompoundSelectorMatchingResult::Matched {
+                next_combinator_offset: from_offset - 1,
+            }
+        }
+
+        if !matches_simple_selector(
+            component,
+            element,
+            &mut local_context,
+            &RelevantLinkStatus::NotLooking,
+            &mut |_, _| {}) {
+            return CompoundSelectorMatchingResult::NotMatched;
+        }
+
+        from_offset -= 1;
+    }
+
+    return CompoundSelectorMatchingResult::Matched {
+        next_combinator_offset: 0,
+    }
 }
 
 /// Matches a complex selector.
-pub fn matches_complex_selector<E, F>(complex_selector: &Selector<E::Impl>,
-                                      offset: usize,
+pub fn matches_complex_selector<E, F>(mut iter: SelectorIter<E::Impl>,
                                       element: &E,
                                       mut context: &mut LocalMatchingContext<E::Impl>,
                                       flags_setter: &mut F)
                                       -> bool
     where E: Element,
           F: FnMut(&E, ElementSelectorFlags),
 {
-    let mut iter = if offset == 0 {
-        complex_selector.iter()
-    } else {
-        complex_selector.iter_from(offset)
-    };
 
     if cfg!(debug_assertions) {
         if context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
             assert!(iter.clone().any(|c| {
                 matches!(*c, Component::PseudoElement(..))
             }));
         }
     }
--- a/servo/components/selectors/parser.rs
+++ b/servo/components/selectors/parser.rs
@@ -433,28 +433,46 @@ impl<Impl: SelectorImpl> Selector<Impl> 
         // Note: selectors are stored left-to-right but logical order is right-to-left.
         let iter = self.0.slice[..(self.len() - offset)].iter().rev();
         SelectorIter {
             iter: iter,
             next_combinator: None,
         }
     }
 
-    /// Returns an iterator over the entire sequence of simple selectors and combinators,
-    /// from right to left.
+    /// Returns the combinator at index `index`, or panics if the component is
+    /// not a combinator.
+    pub fn combinator_at(&self, index: usize) -> Combinator {
+        match self.0.slice[self.0.slice.len() - index] {
+            Component::Combinator(c) => c,
+            ref other => {
+                panic!("Not a combinator: {:?}, {:?}, index: {}",
+                       other, self, index)
+            }
+        }
+    }
+
+    /// Returns an iterator over the entire sequence of simple selectors and
+    /// combinators, from right to left.
     pub fn iter_raw(&self) -> Rev<slice::Iter<Component<Impl>>> {
         self.iter_raw_rev().rev()
     }
 
-    /// Returns an iterator over the entire sequence of simple selectors and combinators,
-    /// from left to right.
+    /// Returns an iterator over the entire sequence of simple selectors and
+    /// combinators, from left to right.
     pub fn iter_raw_rev(&self) -> slice::Iter<Component<Impl>> {
         self.0.slice.iter()
     }
 
+    /// Returns an iterator over the sequence of simple selectors and
+    /// combinators after `offset`, from left to right.
+    pub fn iter_raw_rev_from(&self, offset: usize) -> slice::Iter<Component<Impl>> {
+        self.0.slice[(self.0.slice.len() - offset)..].iter()
+    }
+
     /// Creates a Selector from a vec of Components. Used in tests.
     pub fn from_vec(vec: Vec<Component<Impl>>, specificity_and_flags: u32) -> Self {
         let header = HeaderWithLength::new(SpecificityAndFlags(specificity_and_flags), vec.len());
         Selector(Arc::into_thin(Arc::from_header_and_iter(header, vec.into_iter())))
     }
 
     /// Returns count of simple selectors and combinators in the Selector.
     pub fn len(&self) -> usize {
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1524,17 +1524,17 @@ impl<'le> ::selectors::Element for Gecko
             NonTSPseudoClass::MozIsHTML => {
                 self.is_html_element_in_html_document()
             }
             NonTSPseudoClass::MozPlaceholder => false,
             NonTSPseudoClass::MozAny(ref sels) => {
                 let old_value = context.within_functional_pseudo_class_argument;
                 context.within_functional_pseudo_class_argument = true;
                 let result = sels.iter().any(|s| {
-                    matches_complex_selector(s, 0, self, context, flags_setter)
+                    matches_complex_selector(s.iter(), self, context, flags_setter)
                 });
                 context.within_functional_pseudo_class_argument = old_value;
                 result
             }
             NonTSPseudoClass::Lang(ref lang_arg) => {
                 self.match_element_lang(None, lang_arg)
             }
             NonTSPseudoClass::MozSystemMetric(ref s) |