style: Use left-to-right indices in the invalidator.

This will make easier to create external invalidations, and also makes reasoning
about the invalidator a bit easier.

components/selectors/matching.rs

@@ -299,11 +299,10 @@ where
 /// Whether a compound selector matched, and whether it was the rightmost
 /// selector inside the complex selector.
 pub enum CompoundSelectorMatchingResult {
+    /// The selector was fully matched.
+    FullyMatched,
     /// 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,
@@ -325,8 +324,9 @@ pub fn matches_compound_selector<E>(
 where
     E: Element
 {
+    debug_assert_ne!(from_offset, 0);
     if cfg!(debug_assertions) {
-        selector.combinator_at(from_offset); // This asserts.
+        selector.combinator_at_parse_order(from_offset - 1); // This asserts.
     }
 
     let mut local_context = LocalMatchingContext {
@@ -334,10 +334,11 @@ where
         matches_hover_and_active_quirk: false,
     };
 
-    for component in selector.iter_raw_parse_order_from(from_offset - 1) {
+    for component in selector.iter_raw_parse_order_from(from_offset) {
         if matches!(*component, Component::Combinator(..)) {
+            debug_assert_ne!(from_offset, 0, "Selector started with a combinator?");
             return CompoundSelectorMatchingResult::Matched {
-                next_combinator_offset: from_offset - 1,
+                next_combinator_offset: from_offset,
             }
         }
 
@@ -350,12 +351,10 @@ where
             return CompoundSelectorMatchingResult::NotMatched;
         }
 
-        from_offset -= 1;
+        from_offset += 1;
     }
 
-    return CompoundSelectorMatchingResult::Matched {
+    CompoundSelectorMatchingResult::FullyMatched
-        next_combinator_offset: 0,
-    }
 }
 
 /// Matches a complex selector.

components/selectors/parser.rs

@@ -440,12 +440,11 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         }
     }
 
-    /// Returns the combinator at index `index` (one-indexed from the right),
+    /// Returns the combinator at index `index` (zero-indexed from the right),
     /// or panics if the component is not a combinator.
-    ///
+    #[inline]
-    /// FIXME(bholley): Use more intuitive indexing.
+    pub fn combinator_at_match_order(&self, index: usize) -> Combinator {
-    pub fn combinator_at(&self, index: usize) -> Combinator {
+        match self.0.slice[index] {
-        match self.0.slice[index - 1] {
             Component::Combinator(c) => c,
             ref other => {
                 panic!("Not a combinator: {:?}, {:?}, index: {}",
@@ -460,16 +459,24 @@ impl<Impl: SelectorImpl> Selector<Impl> {
         self.0.slice.iter()
     }
 
+    /// Returns the combinator at index `index` (zero-indexed from the left),
+    /// or panics if the component is not a combinator.
+    #[inline]
+    pub fn combinator_at_parse_order(&self, index: usize) -> Combinator {
+        match self.0.slice[self.len() - index - 1] {
+            Component::Combinator(c) => c,
+            ref other => {
+                panic!("Not a combinator: {:?}, {:?}, index: {}",
+                       other, self, index)
+            }
+        }
+    }
+
     /// Returns an iterator over the sequence of simple selectors and
-    /// combinators, in parse order (from left to right), _starting_
+    /// combinators, in parse order (from left to right), starting from
-    /// 'offset_from_right' entries from the past-the-end sentinel on
+    /// `offset`.
-    /// the right. So "0" panics,. "1" iterates nothing, and "len"
+    pub fn iter_raw_parse_order_from(&self, offset: usize) -> Rev<slice::Iter<Component<Impl>>> {
-    /// iterates the entire sequence.
+        self.0.slice[..self.len() - offset].iter().rev()
-    ///
-    /// FIXME(bholley): This API is rather unintuive, and should really
-    /// be changed to accept an offset from the left. Same for combinator_at.
-    pub fn iter_raw_parse_order_from(&self, offset_from_right: usize) -> Rev<slice::Iter<Component<Impl>>> {
-        self.0.slice[..offset_from_right].iter().rev()
     }
 
     /// Creates a Selector from a vec of Components, specified in parse order. Used in tests.

components/style/invalidation/element/collector.rs

@@ -463,16 +463,18 @@ where
 
         if dependency.affects_descendants() {
             debug_assert_ne!(dependency.selector_offset, 0);
+            debug_assert_ne!(dependency.selector_offset, dependency.selector.len());
             debug_assert!(!dependency.affects_later_siblings());
             self.descendant_invalidations.push(Invalidation::new(
                 dependency.selector.clone(),
-                dependency.selector_offset,
+                dependency.selector.len() - dependency.selector_offset + 1,
             ));
         } else if dependency.affects_later_siblings() {
             debug_assert_ne!(dependency.selector_offset, 0);
+            debug_assert_ne!(dependency.selector_offset, dependency.selector.len());
             self.sibling_invalidations.push(Invalidation::new(
                 dependency.selector.clone(),
-                dependency.selector_offset,
+                dependency.selector.len() - dependency.selector_offset + 1,
             ));
         }
     }

components/style/invalidation/element/invalidation_map.rs

@@ -79,7 +79,7 @@ impl Dependency {
             return None;
         }
 
-        Some(self.selector.combinator_at(self.selector_offset))
+        Some(self.selector.combinator_at_match_order(self.selector_offset - 1))
     }
 
     /// Whether this dependency affects the style of the element.

components/style/invalidation/element/invalidator.rs

@@ -114,14 +114,13 @@ enum InvalidationKind {
 
 /// An `Invalidation` is a complex selector that describes which elements,
 /// relative to a current element we are processing, must be restyled.
-///
-/// When `offset` points to the right-most compound selector in `selector`,
-/// then the Invalidation `represents` the fact that the current element
-/// must be restyled if the compound selector matches.  Otherwise, if
-/// describes which descendants (or later siblings) must be restyled.
 #[derive(Clone)]
 pub struct Invalidation {
     selector: Selector<SelectorImpl>,
+    /// The offset of the selector pointing to a compound selector.
+    ///
+    /// This order is a "parse order" offset, that is, zero is the leftmost part
+    /// of the selector written as parsed / serialized.
     offset: usize,
     /// Whether the invalidation was already matched by any previous sibling or
     /// ancestor.
@@ -149,7 +148,7 @@ impl Invalidation {
         // for the weird pseudos in <input type="number">.
         //
         // We should be able to do better here!
-        match self.selector.combinator_at(self.offset) {
+        match self.selector.combinator_at_parse_order(self.offset - 1) {
             Combinator::NextSibling |
             Combinator::Child => false,
             _ => true,
@@ -157,7 +156,7 @@ impl Invalidation {
     }
 
     fn kind(&self) -> InvalidationKind {
-        if self.selector.combinator_at(self.offset).is_ancestor() {
+        if self.selector.combinator_at_parse_order(self.offset - 1).is_ancestor() {
             InvalidationKind::Descendant
         } else {
             InvalidationKind::Sibling
@@ -170,7 +169,7 @@ impl fmt::Debug for Invalidation {
         use cssparser::ToCss;
 
         f.write_str("Invalidation(")?;
-        for component in self.selector.iter_raw_parse_order_from(self.offset - 1) {
+        for component in self.selector.iter_raw_parse_order_from(self.offset) {
             if matches!(*component, Component::Combinator(..)) {
                 break;
             }
@@ -624,14 +623,14 @@ where
         let mut invalidated_self = false;
         let mut matched = false;
         match matching_result {
-            CompoundSelectorMatchingResult::Matched { next_combinator_offset: 0 } => {
+            CompoundSelectorMatchingResult::FullyMatched => {
                 debug!(" > Invalidation matched completely");
                 matched = true;
                 invalidated_self = true;
             }
             CompoundSelectorMatchingResult::Matched { next_combinator_offset } => {
                 let next_combinator =
-                    invalidation.selector.combinator_at(next_combinator_offset);
+                    invalidation.selector.combinator_at_parse_order(next_combinator_offset);
                 matched = true;
 
                 if matches!(next_combinator, Combinator::PseudoElement) {
@@ -640,7 +639,7 @@ where
                     // around, so there could also be state pseudo-classes.
                     let pseudo_selector =
                         invalidation.selector
-                            .iter_raw_parse_order_from(next_combinator_offset - 1)
+                            .iter_raw_parse_order_from(next_combinator_offset + 1)
                             .skip_while(|c| matches!(**c, Component::NonTSPseudoClass(..)))
                             .next()
                             .unwrap();
@@ -681,7 +680,7 @@ where
 
                 let next_invalidation = Invalidation {
                     selector: invalidation.selector.clone(),
-                    offset: next_combinator_offset,
+                    offset: next_combinator_offset + 1,
                     matched_by_any_previous: false,
                 };