Auto merge of #17439 - bholley:compound_left_to_right, r=SimonSapin

Match compound selectors left-to-right (second try)

https://bugzilla.mozilla.org/show_bug.cgi?id=1373800

<!-- 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/17439)
<!-- Reviewable:end -->

components/script/dom/element.rs

@@ -89,6 +89,7 @@ use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivit
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext, MatchingMode};
 use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
 use selectors::matching::{RelevantLinkStatus, matches_selector_list};
+use selectors::sink::Push;
 use servo_atoms::Atom;
 use std::ascii::AsciiExt;
 use std::borrow::Cow;
@@ -109,7 +110,6 @@ use style::rule_tree::CascadeLevel;
 use style::selector_parser::{NonTSPseudoClass, PseudoElement, RestyleDamage, SelectorImpl, SelectorParser};
 use style::selector_parser::extended_filtering;
 use style::shared_lock::{SharedRwLock, Locked};
-use style::sink::Push;
 use style::stylearc::Arc;
 use style::thread_state;
 use style::values::{CSSFloat, Either};

components/script/layout_wrapper.rs

@@ -52,6 +52,7 @@ use script_layout_interface::wrapper_traits::{PseudoElementType, ThreadSafeLayou
 use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext, RelevantLinkStatus};
 use selectors::matching::VisitedHandlingMode;
+use selectors::sink::Push;
 use servo_atoms::Atom;
 use servo_url::ServoUrl;
 use std::fmt;
@@ -75,7 +76,6 @@ use style::properties::{ComputedValues, PropertyDeclarationBlock};
 use style::selector_parser::{AttrValue as SelectorAttrValue, NonTSPseudoClass, PseudoClassStringArg};
 use style::selector_parser::{PseudoElement, SelectorImpl, extended_filtering};
 use style::shared_lock::{SharedRwLock as StyleSharedRwLock, Locked as StyleLocked};
-use style::sink::Push;
 use style::str::is_whitespace;
 use style::stylearc::Arc;
 

components/selectors/builder.rs

@@ -0,0 +1,311 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+//! Helper module to build up a selector safely and efficiently.
+//!
+//! Our selector representation is designed to optimize matching, and has
+//! several requirements:
+//! * All simple selectors and combinators are stored inline in the same buffer
+//!   as Component instances.
+//! * We store the top-level compound selectors from right to left, i.e. in
+//!   matching order.
+//! * We store the simple selectors for each combinator from left to right, so
+//!   that we match the cheaper simple selectors first.
+//!
+//! Meeting all these constraints without extra memmove traffic during parsing
+//! is non-trivial. This module encapsulates those details and presents an
+//! easy-to-use API for the parser.
+
+use parser::{Combinator, Component, SelectorImpl};
+use servo_arc::{Arc, HeaderWithLength, ThinArc};
+use sink::Push;
+use smallvec::{self, SmallVec};
+use std::cmp;
+use std::iter;
+use std::ops::Add;
+use std::ptr;
+use std::slice;
+
+/// Top-level SelectorBuilder struct. This should be stack-allocated by the
+/// consumer and never moved (because it contains a lot of inline data that
+/// would be slow to memmov).
+///
+/// After instantation, callers may call the push_simple_selector() and
+/// push_combinator() methods to append selector data as it is encountered
+/// (from left to right). Once the process is complete, callers should invoke
+/// build(), which transforms the contents of the SelectorBuilder into a heap-
+/// allocated Selector and leaves the builder in a drained state.
+pub struct SelectorBuilder<Impl: SelectorImpl> {
+    /// The entire sequence of simple selectors, from left to right, without combinators.
+    ///
+    /// We make this large because the result of parsing a selector is fed into a new
+    /// Arc-ed allocation, so any spilled vec would be a wasted allocation. Also,
+    /// Components are large enough that we don't have much cache locality benefit
+    /// from reserving stack space for fewer of them.
+    simple_selectors: SmallVec<[Component<Impl>; 32]>,
+    /// The combinators, and the length of the compound selector to their left.
+    combinators: SmallVec<[(Combinator, usize); 16]>,
+    /// The length of the current compount selector.
+    current_len: usize,
+}
+
+impl<Impl: SelectorImpl> Default for SelectorBuilder<Impl> {
+    #[inline(always)]
+    fn default() -> Self {
+        SelectorBuilder {
+            simple_selectors: SmallVec::new(),
+            combinators: SmallVec::new(),
+            current_len: 0,
+        }
+    }
+}
+
+impl<Impl: SelectorImpl> Push<Component<Impl>> for SelectorBuilder<Impl> {
+    fn push(&mut self, value: Component<Impl>) {
+        self.push_simple_selector(value);
+    }
+}
+
+impl<Impl: SelectorImpl> SelectorBuilder<Impl> {
+    /// Pushes a simple selector onto the current compound selector.
+    #[inline(always)]
+    pub fn push_simple_selector(&mut self, ss: Component<Impl>) {
+        debug_assert!(!ss.is_combinator());
+        self.simple_selectors.push(ss);
+        self.current_len += 1;
+    }
+
+    /// Completes the current compound selector and starts a new one, delimited
+    /// by the given combinator.
+    #[inline(always)]
+    pub fn push_combinator(&mut self, c: Combinator) {
+        self.combinators.push((c, self.current_len));
+        self.current_len = 0;
+    }
+
+    /// Returns true if no simple selectors have ever been pushed to this builder.
+    #[inline(always)]
+    pub fn is_empty(&self) -> bool {
+        self.simple_selectors.is_empty()
+    }
+
+    /// Consumes the builder, producing a Selector.
+    #[inline(always)]
+    pub fn build(&mut self, parsed_pseudo: bool) -> ThinArc<SpecificityAndFlags, Component<Impl>> {
+        // Compute the specificity and flags.
+        let mut spec = SpecificityAndFlags(specificity(self.simple_selectors.iter()));
+        if parsed_pseudo {
+            spec.0 |= HAS_PSEUDO_BIT;
+        }
+
+        self.build_with_specificity_and_flags(spec)
+    }
+
+
+    /// Builds with an explicit SpecificityAndFlags. This is separated from build() so
+    /// that unit tests can pass an explicit specificity.
+    #[inline(always)]
+    pub fn build_with_specificity_and_flags(&mut self, spec: SpecificityAndFlags)
+                                            -> ThinArc<SpecificityAndFlags, Component<Impl>> {
+        // First, compute the total number of Components we'll need to allocate
+        // space for.
+        let full_len = self.simple_selectors.len() + self.combinators.len();
+
+        // Create the header.
+        let header = HeaderWithLength::new(spec, full_len);
+
+        // Create the Arc using an iterator that drains our buffers.
+
+        // Use a raw pointer to be able to call set_len despite "borrowing" the slice.
+        // This is similar to SmallVec::drain, but we use a slice here because
+        // we’re gonna traverse it non-linearly.
+        let raw_simple_selectors: *const [Component<Impl>] = &*self.simple_selectors;
+        unsafe {
+            // Panic-safety: if SelectorBuilderIter is not iterated to the end,
+            // some simple selectors will safely leak.
+            self.simple_selectors.set_len(0)
+        }
+        let (rest, current) = split_from_end(unsafe { &*raw_simple_selectors }, self.current_len);
+        let iter = SelectorBuilderIter {
+            current_simple_selectors: current.iter(),
+            rest_of_simple_selectors: rest,
+            combinators: self.combinators.drain().rev(),
+        };
+
+        Arc::into_thin(Arc::from_header_and_iter(header, iter))
+    }
+}
+
+struct SelectorBuilderIter<'a, Impl: SelectorImpl> {
+    current_simple_selectors: slice::Iter<'a, Component<Impl>>,
+    rest_of_simple_selectors: &'a [Component<Impl>],
+    combinators: iter::Rev<smallvec::Drain<'a, (Combinator, usize)>>,
+}
+
+impl<'a, Impl: SelectorImpl> ExactSizeIterator for SelectorBuilderIter<'a, Impl> {
+    fn len(&self) -> usize {
+        self.current_simple_selectors.len() +
+        self.rest_of_simple_selectors.len() +
+        self.combinators.len()
+    }
+}
+
+impl<'a, Impl: SelectorImpl> Iterator for SelectorBuilderIter<'a, Impl> {
+    type Item = Component<Impl>;
+    #[inline(always)]
+    fn next(&mut self) -> Option<Self::Item> {
+        if let Some(simple_selector_ref) = self.current_simple_selectors.next() {
+            // Move a simple selector out of this slice iterator.
+            // This is safe because we’ve called SmallVec::set_len(0) above,
+            // so SmallVec::drop won’t drop this simple selector.
+            unsafe {
+                Some(ptr::read(simple_selector_ref))
+            }
+        } else {
+            self.combinators.next().map(|(combinator, len)| {
+                let (rest, current) = split_from_end(self.rest_of_simple_selectors, len);
+                self.rest_of_simple_selectors = rest;
+                self.current_simple_selectors = current.iter();
+                Component::Combinator(combinator)
+            })
+        }
+    }
+
+    fn size_hint(&self) -> (usize, Option<usize>) {
+        (self.len(), Some(self.len()))
+    }
+}
+
+fn split_from_end<T>(s: &[T], at: usize) -> (&[T], &[T]) {
+    s.split_at(s.len() - at)
+}
+
+pub const HAS_PSEUDO_BIT: u32 = 1 << 30;
+
+#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+pub struct SpecificityAndFlags(pub u32);
+
+impl SpecificityAndFlags {
+    pub fn specificity(&self) -> u32 {
+        self.0 & !HAS_PSEUDO_BIT
+    }
+
+    pub fn has_pseudo_element(&self) -> bool {
+        (self.0 & HAS_PSEUDO_BIT) != 0
+    }
+}
+
+const MAX_10BIT: u32 = (1u32 << 10) - 1;
+
+#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)]
+struct Specificity {
+    id_selectors: u32,
+    class_like_selectors: u32,
+    element_selectors: u32,
+}
+
+impl Add for Specificity {
+    type Output = Specificity;
+
+    fn add(self, rhs: Specificity) -> Specificity {
+        Specificity {
+            id_selectors: self.id_selectors + rhs.id_selectors,
+            class_like_selectors:
+                self.class_like_selectors + rhs.class_like_selectors,
+            element_selectors:
+                self.element_selectors + rhs.element_selectors,
+        }
+    }
+}
+
+impl Default for Specificity {
+    fn default() -> Specificity {
+        Specificity {
+            id_selectors: 0,
+            class_like_selectors: 0,
+            element_selectors: 0,
+        }
+    }
+}
+
+impl From<u32> for Specificity {
+    fn from(value: u32) -> Specificity {
+        assert!(value <= MAX_10BIT << 20 | MAX_10BIT << 10 | MAX_10BIT);
+        Specificity {
+            id_selectors: value >> 20,
+            class_like_selectors: (value >> 10) & MAX_10BIT,
+            element_selectors: value & MAX_10BIT,
+        }
+    }
+}
+
+impl From<Specificity> for u32 {
+    fn from(specificity: Specificity) -> u32 {
+        cmp::min(specificity.id_selectors, MAX_10BIT) << 20
+        | cmp::min(specificity.class_like_selectors, MAX_10BIT) << 10
+        | cmp::min(specificity.element_selectors, MAX_10BIT)
+    }
+}
+
+fn specificity<Impl>(iter: slice::Iter<Component<Impl>>) -> u32
+    where Impl: SelectorImpl
+{
+    complex_selector_specificity(iter).into()
+}
+
+fn complex_selector_specificity<Impl>(mut iter: slice::Iter<Component<Impl>>)
+                                      -> Specificity
+    where Impl: SelectorImpl
+{
+    fn simple_selector_specificity<Impl>(simple_selector: &Component<Impl>,
+                                         specificity: &mut Specificity)
+        where Impl: SelectorImpl
+    {
+        match *simple_selector {
+            Component::Combinator(..) => unreachable!(),
+            Component::PseudoElement(..) |
+            Component::LocalName(..) => {
+                specificity.element_selectors += 1
+            }
+            Component::ID(..) => {
+                specificity.id_selectors += 1
+            }
+            Component::Class(..) |
+            Component::AttributeInNoNamespace { .. } |
+            Component::AttributeInNoNamespaceExists { .. } |
+            Component::AttributeOther(..) |
+
+            Component::FirstChild | Component::LastChild |
+            Component::OnlyChild | Component::Root |
+            Component::Empty |
+            Component::NthChild(..) |
+            Component::NthLastChild(..) |
+            Component::NthOfType(..) |
+            Component::NthLastOfType(..) |
+            Component::FirstOfType | Component::LastOfType |
+            Component::OnlyOfType |
+            Component::NonTSPseudoClass(..) => {
+                specificity.class_like_selectors += 1
+            }
+            Component::ExplicitUniversalType |
+            Component::ExplicitAnyNamespace |
+            Component::ExplicitNoNamespace |
+            Component::DefaultNamespace(..) |
+            Component::Namespace(..) => {
+                // Does not affect specificity
+            }
+            Component::Negation(ref negated) => {
+                for ss in negated.iter() {
+                    simple_selector_specificity(&ss, specificity);
+                }
+            }
+        }
+    }
+
+    let mut specificity = Default::default();
+    for simple_selector in &mut iter {
+        simple_selector_specificity(&simple_selector, &mut specificity);
+    }
+    specificity
+}

components/selectors/lib.rs

@@ -15,11 +15,13 @@ extern crate smallvec;
 
 pub mod attr;
 pub mod bloom;
+mod builder;
 pub mod context;
 pub mod matching;
 pub mod parser;
 #[cfg(test)] mod size_of_tests;
 #[cfg(any(test, feature = "gecko_like_types"))] pub mod gecko_like_types;
+pub mod sink;
 mod tree;
 pub mod visitor;
 

components/selectors/matching.rs

@@ -419,7 +419,7 @@ where
     }
 
     let mut local_context = LocalMatchingContext::new(context, selector);
-    for component in selector.iter_raw_rev_from(from_offset - 1) {
+    for component in selector.iter_raw_parse_order_from(from_offset - 1) {
         if matches!(*component, Component::Combinator(..)) {
             return CompoundSelectorMatchingResult::Matched {
                 next_combinator_offset: from_offset - 1,
@@ -460,24 +460,27 @@ pub fn matches_complex_selector<E, F>(mut iter: SelectorIter<E::Impl>,
         }
     }
 
+    // If this is the special pseudo-element mode, consume the ::pseudo-element
+    // before proceeding, since the caller has already handled that part.
     if context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
-        match *iter.next().unwrap() {
+        // Consume the pseudo.
-            // Stateful pseudo, just don't match.
+        let pseudo = iter.next().unwrap();
-            Component::NonTSPseudoClass(..) => return false,
+        debug_assert!(matches!(*pseudo, Component::PseudoElement(..)),
-            Component::PseudoElement(..) => {
+                      "Used MatchingMode::ForStatelessPseudoElement in a non-pseudo selector");
-                // Pseudo, just eat the whole sequence.
-                let next = iter.next();
-                debug_assert!(next.is_none(),
-                              "Someone messed up pseudo-element parsing?");
 
-                if iter.next_sequence().is_none() {
+        // The only other parser-allowed Component in this sequence is a state
-                    return true;
+        // class. We just don't match in that case.
-                }
+        if let Some(s) = iter.next() {
-                // Inform the context that the we've advanced to the next compound selector.
+            debug_assert!(matches!(*s, Component::NonTSPseudoClass(..)),
-                context.note_next_sequence(&mut iter);
+                          "Someone messed up pseudo-element parsing");
-            }
+            return false;
-            _ => panic!("Used MatchingMode::ForStatelessPseudoElement in a non-pseudo selector"),
         }
+
+        // Advance to the non-pseudo-element part of the selector, and inform the context.
+        if iter.next_sequence().is_none() {
+            return true;
+        }
+        context.note_next_sequence(&mut iter);
     }
 
     match matches_complex_selector_internal(iter,

components/selectors/parser.rs

@@ -5,17 +5,17 @@
 use attr::{AttrSelectorWithNamespace, ParsedAttrSelectorOperation, AttrSelectorOperator};
 use attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE, NamespaceConstraint};
 use bloom::BLOOM_HASH_MASK;
+use builder::{SelectorBuilder, SpecificityAndFlags};
 use cssparser::{ParseError, BasicParseError, CompactCowStr};
 use cssparser::{Token, Parser as CssParser, parse_nth, ToCss, serialize_identifier, CssStringWriter};
 use precomputed_hash::PrecomputedHash;
-use servo_arc::{Arc, HeaderWithLength, ThinArc};
+use servo_arc::ThinArc;
+use sink::Push;
 use smallvec::SmallVec;
 use std::ascii::AsciiExt;
 use std::borrow::{Borrow, Cow};
-use std::cmp;
 use std::fmt::{self, Display, Debug, Write};
 use std::iter::Rev;
-use std::ops::Add;
 use std::slice;
 use visitor::SelectorVisitor;
 
@@ -265,8 +265,6 @@ impl AncestorHashes {
     }
 }
 
-const HAS_PSEUDO_BIT: u32 = 1 << 30;
-
 pub trait SelectorMethods {
     type Impl: SelectorImpl;
 
@@ -369,27 +367,20 @@ pub fn namespace_empty_string<Impl: SelectorImpl>() -> Impl::NamespaceUrl {
     Impl::NamespaceUrl::default()
 }
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
-struct SpecificityAndFlags(u32);
-
-impl SpecificityAndFlags {
-    fn specificity(&self) -> u32 {
-        self.0 & !HAS_PSEUDO_BIT
-    }
-
-    fn has_pseudo_element(&self) -> bool {
-        (self.0 & HAS_PSEUDO_BIT) != 0
-    }
-}
-
 /// A Selector stores a sequence of simple selectors and combinators. The
 /// iterator classes allow callers to iterate at either the raw sequence level or
 /// at the level of sequences of simple selectors separated by combinators. Most
 /// callers want the higher-level iterator.
 ///
-/// We store selectors internally left-to-right (in parsing order), but the
+/// We store compound selectors internally right-to-left (in matching order).
-/// canonical iteration order is right-to-left (selector matching order). The
+/// Additionally, we invert the order of top-level compound selectors so that
-/// iterators abstract over these details.
+/// each one matches left-to-right. This is because matching namespace, local name,
+/// id, and class are all relatively cheap, whereas matching pseudo-classes might
+/// be expensive (depending on the pseudo-class). Since authors tend to put the
+/// pseudo-classes on the right, it's faster to start matching on the left.
+///
+/// This reordering doesn't change the semantics of selector matching, and we
+/// handle it in to_css to make it invisible to serialization.
 #[derive(Clone, Eq, PartialEq)]
 pub struct Selector<Impl: SelectorImpl>(ThinArc<SpecificityAndFlags, Component<Impl>>);
 
@@ -421,7 +412,7 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     ///
     /// Used for "pre-computed" pseudo-elements in components/style/stylist.rs
     pub fn is_universal(&self) -> bool {
-        self.iter_raw().all(|c| matches!(*c,
+        self.iter_raw_match_order().all(|c| matches!(*c,
             Component::ExplicitUniversalType |
             Component::ExplicitAnyNamespace |
             Component::Combinator(Combinator::PseudoElement) |
@@ -430,29 +421,32 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     }
 
 
-    /// Returns an iterator over the next sequence of simple selectors. When
+    /// Returns an iterator over this selector in matching order (right-to-left).
-    /// a combinator is reached, the iterator will return None, and
+    /// When a combinator is reached, the iterator will return None, and
     /// next_sequence() may be called to continue to the next sequence.
     pub fn iter(&self) -> SelectorIter<Impl> {
         SelectorIter {
-            iter: self.iter_raw(),
+            iter: self.iter_raw_match_order(),
             next_combinator: None,
         }
     }
 
+    /// Returns an iterator over this selector in matching order (right-to-left),
+    /// skipping the rightmost |offset| Components.
     pub fn iter_from(&self, offset: usize) -> SelectorIter<Impl> {
-        // Note: selectors are stored left-to-right but logical order is right-to-left.
+        let iter = self.0.slice[offset..].iter();
-        let iter = self.0.slice[..(self.len() - offset)].iter().rev();
         SelectorIter {
             iter: iter,
             next_combinator: None,
         }
     }
 
-    /// Returns the combinator at index `index`, or panics if the component is
+    /// Returns the combinator at index `index` (one-indexed from the right),
-    /// not a combinator.
+    /// or panics if the component is not a combinator.
+    ///
+    /// FIXME(bholley): Use more intuitive indexing.
     pub fn combinator_at(&self, index: usize) -> Combinator {
-        match self.0.slice[self.0.slice.len() - index] {
+        match self.0.slice[index - 1] {
             Component::Combinator(c) => c,
             ref other => {
                 panic!("Not a combinator: {:?}, {:?}, index: {}",
@@ -462,27 +456,35 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     }
 
     /// Returns an iterator over the entire sequence of simple selectors and
-    /// combinators, from right to left.
+    /// combinators, in matching order (from right to left).
-    pub fn iter_raw(&self) -> Rev<slice::Iter<Component<Impl>>> {
+    pub fn iter_raw_match_order(&self) -> 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.
-    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.
+    /// combinators, in parse order (from left to right), _starting_
-    pub fn iter_raw_rev_from(&self, offset: usize) -> slice::Iter<Component<Impl>> {
+    /// 'offset_from_right' entries from the past-the-end sentinel on
-        self.0.slice[(self.0.slice.len() - offset)..].iter()
+    /// the right. So "0" panics,. "1" iterates nothing, and "len"
+    /// iterates the entire sequence.
+    ///
+    /// 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. Used in tests.
+    /// Creates a Selector from a vec of Components, specified in parse order. 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());
+        let mut builder = SelectorBuilder::default();
-        Selector(Arc::into_thin(Arc::from_header_and_iter(header, vec.into_iter())))
+        for component in vec.into_iter() {
+            if let Some(combinator) = component.as_combinator() {
+                builder.push_combinator(combinator);
+            } else {
+                builder.push_simple_selector(component);
+            }
+        }
+        let spec = SpecificityAndFlags(specificity_and_flags);
+        Selector(builder.build_with_specificity_and_flags(spec))
     }
 
     /// Returns count of simple selectors and combinators in the Selector.
@@ -493,7 +495,7 @@ impl<Impl: SelectorImpl> Selector<Impl> {
 
 #[derive(Clone)]
 pub struct SelectorIter<'a, Impl: 'a + SelectorImpl> {
-    iter: Rev<slice::Iter<'a, Component<Impl>>>,
+    iter: slice::Iter<'a, Component<Impl>>,
     next_combinator: Option<Combinator>,
 }
 
@@ -757,9 +759,31 @@ impl<Impl: SelectorImpl> ToCss for SelectorList<Impl> {
 
 impl<Impl: SelectorImpl> ToCss for Selector<Impl> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-       for item in self.iter_raw_rev() {
+        // Compound selectors invert the order of their contents, so we need to
-           item.to_css(dest)?;
+        // undo that during serialization.
-       }
+        //
+        // This two-iterator strategy involves walking over the selector twice.
+        // We could do something more clever, but selector serialization probably
+        // isn't hot enough to justify it, and the stringification likely
+        // dominates anyway.
+        //
+        // NB: A parse-order iterator is a Rev<>, which doesn't expose as_slice(),
+        // which we need for |split|. So we split by combinators on a match-order
+        // sequence and then reverse.
+        let mut combinators = self.iter_raw_match_order().rev().filter(|x| x.is_combinator());
+        let compound_selectors = self.iter_raw_match_order().as_slice().split(|x| x.is_combinator()).rev();
+
+        let mut combinators_exhausted = false;
+        for compound in compound_selectors {
+            debug_assert!(!combinators_exhausted);
+            for item in compound.iter() {
+                item.to_css(dest)?;
+            }
+            match combinators.next() {
+                Some(c) => c.to_css(dest)?,
+                None => combinators_exhausted = true,
+            };
+        }
 
        Ok(())
     }
@@ -830,7 +854,6 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
             // Pseudo-classes
             Negation(ref arg) => {
                 dest.write_str(":not(")?;
-                debug_assert!(single_simple_selector(arg));
                 for component in arg.iter() {
                     component.to_css(dest)?;
                 }
@@ -912,131 +935,6 @@ fn display_to_css_identifier<T: Display, W: fmt::Write>(x: &T, dest: &mut W) ->
     serialize_identifier(&string, dest)
 }
 
-const MAX_10BIT: u32 = (1u32 << 10) - 1;
-
-#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)]
-struct Specificity {
-    id_selectors: u32,
-    class_like_selectors: u32,
-    element_selectors: u32,
-}
-
-impl Add for Specificity {
-    type Output = Specificity;
-
-    fn add(self, rhs: Specificity) -> Specificity {
-        Specificity {
-            id_selectors: self.id_selectors + rhs.id_selectors,
-            class_like_selectors:
-                self.class_like_selectors + rhs.class_like_selectors,
-            element_selectors:
-                self.element_selectors + rhs.element_selectors,
-        }
-    }
-}
-
-impl Default for Specificity {
-    fn default() -> Specificity {
-        Specificity {
-            id_selectors: 0,
-            class_like_selectors: 0,
-            element_selectors: 0,
-        }
-    }
-}
-
-impl From<u32> for Specificity {
-    fn from(value: u32) -> Specificity {
-        assert!(value <= MAX_10BIT << 20 | MAX_10BIT << 10 | MAX_10BIT);
-        Specificity {
-            id_selectors: value >> 20,
-            class_like_selectors: (value >> 10) & MAX_10BIT,
-            element_selectors: value & MAX_10BIT,
-        }
-    }
-}
-
-impl From<Specificity> for u32 {
-    fn from(specificity: Specificity) -> u32 {
-        cmp::min(specificity.id_selectors, MAX_10BIT) << 20
-        | cmp::min(specificity.class_like_selectors, MAX_10BIT) << 10
-        | cmp::min(specificity.element_selectors, MAX_10BIT)
-    }
-}
-
-fn specificity<Impl>(iter: SelectorIter<Impl>) -> u32
-    where Impl: SelectorImpl
-{
-    complex_selector_specificity(iter).into()
-}
-
-fn complex_selector_specificity<Impl>(mut iter: SelectorIter<Impl>)
-                                      -> Specificity
-    where Impl: SelectorImpl
-{
-    fn simple_selector_specificity<Impl>(simple_selector: &Component<Impl>,
-                                         specificity: &mut Specificity)
-        where Impl: SelectorImpl
-    {
-        match *simple_selector {
-            Component::Combinator(..) => unreachable!(),
-            Component::PseudoElement(..) |
-            Component::LocalName(..) => {
-                specificity.element_selectors += 1
-            }
-            Component::ID(..) => {
-                specificity.id_selectors += 1
-            }
-            Component::Class(..) |
-            Component::AttributeInNoNamespace { .. } |
-            Component::AttributeInNoNamespaceExists { .. } |
-            Component::AttributeOther(..) |
-
-            Component::FirstChild | Component::LastChild |
-            Component::OnlyChild | Component::Root |
-            Component::Empty |
-            Component::NthChild(..) |
-            Component::NthLastChild(..) |
-            Component::NthOfType(..) |
-            Component::NthLastOfType(..) |
-            Component::FirstOfType | Component::LastOfType |
-            Component::OnlyOfType |
-            Component::NonTSPseudoClass(..) => {
-                specificity.class_like_selectors += 1
-            }
-            Component::ExplicitUniversalType |
-            Component::ExplicitAnyNamespace |
-            Component::ExplicitNoNamespace |
-            Component::DefaultNamespace(..) |
-            Component::Namespace(..) => {
-                // Does not affect specificity
-            }
-            Component::Negation(ref negated) => {
-                for ss in negated.iter() {
-                    simple_selector_specificity(&ss, specificity);
-                }
-            }
-        }
-    }
-
-    let mut specificity = Default::default();
-    loop {
-        for simple_selector in &mut iter {
-            simple_selector_specificity(&simple_selector, &mut specificity);
-        }
-        if iter.next_sequence().is_none() {
-            break;
-        }
-    }
-    specificity
-}
-
-/// We make this large because the result of parsing a selector is fed into a new
-/// Arc-ed allocation, so any spilled vec would be a wasted allocation. Also,
-/// Components are large enough that we don't have much cache locality benefit
-/// from reserving stack space for fewer of them.
-type ParseVec<Impl> = SmallVec<[Component<Impl>; 32]>;
-
 /// Build up a Selector.
 /// selector : simple_selector_sequence [ combinator simple_selector_sequence ]* ;
 ///
@@ -1047,13 +945,12 @@ fn parse_selector<'i, 't, P, E, Impl>(
         -> Result<Selector<Impl>, ParseError<'i, SelectorParseError<'i, E>>>
     where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
 {
-    let mut sequence = ParseVec::new();
+    let mut builder = SelectorBuilder::default();
+
     let mut parsed_pseudo_element;
     'outer_loop: loop {
         // Parse a sequence of simple selectors.
-        parsed_pseudo_element =
+        parsed_pseudo_element = parse_compound_selector(parser, input, &mut builder)?;
-            parse_compound_selector(parser, input, &mut sequence,
-                                    /* inside_negation = */ false)?;
         if parsed_pseudo_element {
             break;
         }
@@ -1089,20 +986,10 @@ fn parse_selector<'i, 't, P, E, Impl>(
                 }
             }
         }
-        sequence.push(Component::Combinator(combinator));
+        builder.push_combinator(combinator);
     }
 
-    let mut spec = SpecificityAndFlags(specificity(SelectorIter {
+    Ok(Selector(builder.build(parsed_pseudo_element)))
-        iter: sequence.iter().rev(),
-        next_combinator: None,
-    }));
-    if parsed_pseudo_element {
-        spec.0 |= HAS_PSEUDO_BIT;
-    }
-
-    let header = HeaderWithLength::new(spec, sequence.len());
-    let complex = Selector(Arc::into_thin(Arc::from_header_and_iter(header, sequence.into_iter())));
-    Ok(complex)
 }
 
 impl<Impl: SelectorImpl> Selector<Impl> {
@@ -1120,12 +1007,13 @@ impl<Impl: SelectorImpl> Selector<Impl> {
 }
 
 /// * `Err(())`: Invalid selector, abort
-/// * `Ok(None)`: Not a type selector, could be something else. `input` was not consumed.
+/// * `Ok(false)`: Not a type selector, could be something else. `input` was not consumed.
-/// * `Ok(Some(vec))`: Length 0 (`*|*`), 1 (`*|E` or `ns|*`) or 2 (`|E` or `ns|E`)
+/// * `Ok(true)`: Length 0 (`*|*`), 1 (`*|E` or `ns|*`) or 2 (`|E` or `ns|E`)
-fn parse_type_selector<'i, 't, P, E, Impl>(parser: &P, input: &mut CssParser<'i, 't>,
+fn parse_type_selector<'i, 't, P, E, Impl, S>(parser: &P, input: &mut CssParser<'i, 't>, sink: &mut S)
-                                           sequence: &mut ParseVec<Impl>)
+                                              -> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
-                                           -> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
+    where P: Parser<'i, Impl=Impl, Error=E>,
-    where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
+          Impl: SelectorImpl,
+          S: Push<Component<Impl>>,
 {
     match parse_qualified_name(parser, input, /* in_attr_selector = */ false)? {
         None => Ok(false),
@@ -1133,16 +1021,16 @@ fn parse_type_selector<'i, 't, P, E, Impl>(parser: &P, input: &mut CssParser<'i,
             match namespace {
                 QNamePrefix::ImplicitAnyNamespace => {}
                 QNamePrefix::ImplicitDefaultNamespace(url) => {
-                    sequence.push(Component::DefaultNamespace(url))
+                    sink.push(Component::DefaultNamespace(url))
                 }
                 QNamePrefix::ExplicitNamespace(prefix, url) => {
-                    sequence.push(Component::Namespace(prefix, url))
+                    sink.push(Component::Namespace(prefix, url))
                 }
                 QNamePrefix::ExplicitNoNamespace => {
-                    sequence.push(Component::ExplicitNoNamespace)
+                    sink.push(Component::ExplicitNoNamespace)
                 }
                 QNamePrefix::ExplicitAnyNamespace => {
-                    sequence.push(Component::ExplicitAnyNamespace)
+                    sink.push(Component::ExplicitAnyNamespace)
                 }
                 QNamePrefix::ImplicitNoNamespace => {
                     unreachable!()  // Not returned with in_attr_selector = false
@@ -1150,13 +1038,13 @@ fn parse_type_selector<'i, 't, P, E, Impl>(parser: &P, input: &mut CssParser<'i,
             }
             match local_name {
                 Some(name) => {
-                    sequence.push(Component::LocalName(LocalName {
+                    sink.push(Component::LocalName(LocalName {
                         lower_name: from_cow_str(to_ascii_lowercase(&name)),
                         name: from_cow_str(name.into()),
                     }))
                 }
                 None => {
-                    sequence.push(Component::ExplicitUniversalType)
+                    sink.push(Component::ExplicitUniversalType)
                 }
             }
             Ok(true)
@@ -1423,34 +1311,36 @@ fn parse_negation<'i, 't, P, E, Impl>(parser: &P,
                                                 ParseError<'i, SelectorParseError<'i, E>>>
     where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
 {
-    let mut v = ParseVec::new();
+    // We use a sequence because a type selector may be represented as two Components.
-    parse_compound_selector(parser, input, &mut v, /* inside_negation = */ true)?;
+    let mut sequence = SmallVec::<[Component<Impl>; 2]>::new();
 
-    if single_simple_selector(&v) {
+    // Consume any leading whitespace.
-        Ok(Component::Negation(v.into_vec().into_boxed_slice()))
+    loop {
-    } else {
+        let position = input.position();
-        Err(ParseError::Custom(SelectorParseError::NonSimpleSelectorInNegation))
+        if !matches!(input.next_including_whitespace(), Ok(Token::WhiteSpace(_))) {
-    }
+            input.reset(position);
-}
+            break
-
-// A single type selector can be represented as two components
-fn single_simple_selector<Impl: SelectorImpl>(v: &[Component<Impl>]) -> bool {
-    v.len() == 1 || (
-        v.len() == 2 &&
-        match v[1] {
-            Component::LocalName(_) | Component::ExplicitUniversalType => {
-                debug_assert!(matches!(v[0],
-                    Component::ExplicitAnyNamespace |
-                    Component::ExplicitNoNamespace |
-                    Component::DefaultNamespace(_) |
-                    Component::Namespace(..)
-                ));
-                true
-            }
-            _ => false,
         }
-    )
+    }
 
+    // Get exactly one simple selector. The parse logic in the caller will verify
+    // that there are no trailing tokens after we're done.
+    if !parse_type_selector(parser, input, &mut sequence)? {
+        match parse_one_simple_selector(parser, input, /* inside_negation = */ true)? {
+            Some(SimpleSelectorParseResult::SimpleSelector(s)) => {
+                sequence.push(s);
+            },
+            None => {
+                return Err(ParseError::Custom(SelectorParseError::EmptySelector));
+            },
+            Some(SimpleSelectorParseResult::PseudoElement(_)) => {
+                return Err(ParseError::Custom(SelectorParseError::NonSimpleSelectorInNegation));
+            }
+        }
+    }
+
+    // Success.
+    Ok(Component::Negation(sequence.into_vec().into_boxed_slice()))
 }
 
 /// simple_selector_sequence
@@ -1463,8 +1353,7 @@ fn single_simple_selector<Impl: SelectorImpl>(v: &[Component<Impl>]) -> bool {
 fn parse_compound_selector<'i, 't, P, E, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
-    mut sequence: &mut ParseVec<Impl>,
+    mut builder: &mut SelectorBuilder<Impl>)
-    inside_negation: bool)
     -> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
     where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
 {
@@ -1477,16 +1366,12 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
         }
     }
     let mut empty = true;
-    if !parse_type_selector(parser, input, &mut sequence)? {
+    if !parse_type_selector(parser, input, builder)? {
         if let Some(url) = parser.default_namespace() {
             // If there was no explicit type selector, but there is a
             // default namespace, there is an implicit "<defaultns>|*" type
             // selector.
-            //
+            builder.push_simple_selector(Component::DefaultNamespace(url))
-            // Note that this doesn't apply to :not() and :matches() per spec.
-            if !inside_negation {
-                sequence.push(Component::DefaultNamespace(url))
-            }
         }
     } else {
         empty = false;
@@ -1494,15 +1379,16 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
 
     let mut pseudo = false;
     loop {
-        match parse_one_simple_selector(parser, input, inside_negation)? {
+        match parse_one_simple_selector(parser, input, /* inside_negation = */ false)? {
             None => break,
             Some(SimpleSelectorParseResult::SimpleSelector(s)) => {
-                sequence.push(s);
+                builder.push_simple_selector(s);
                 empty = false
             }
             Some(SimpleSelectorParseResult::PseudoElement(p)) => {
-                // Try to parse state to its right.
+                // Try to parse state to its right. There are only 3 allowable
-                let mut state_selectors = ParseVec::new();
+                // state selectors that can go on pseudo-elements.
+                let mut state_selectors = SmallVec::<[Component<Impl>; 3]>::new();
 
                 loop {
                     match input.next_including_whitespace() {
@@ -1526,13 +1412,13 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
                     state_selectors.push(Component::NonTSPseudoClass(pseudo_class));
                 }
 
-                if !sequence.is_empty() {
+                if !builder.is_empty() {
-                    sequence.push(Component::Combinator(Combinator::PseudoElement));
+                    builder.push_combinator(Combinator::PseudoElement);
                 }
 
-                sequence.push(Component::PseudoElement(p));
+                builder.push_simple_selector(Component::PseudoElement(p));
-                for state_selector in state_selectors {
+                for state_selector in state_selectors.into_iter() {
-                    sequence.push(state_selector);
+                    builder.push_simple_selector(state_selector);
                 }
 
                 pseudo = true;
@@ -1683,6 +1569,7 @@ fn parse_simple_pseudo_class<'i, P, E, Impl>(parser: &P, name: CompactCowStr<'i>
 // NB: pub module in order to access the DummyParser
 #[cfg(test)]
 pub mod tests {
+    use builder::HAS_PSEUDO_BIT;
     use cssparser::{Parser as CssParser, ToCss, serialize_identifier, ParserInput};
     use parser;
     use std::borrow::Cow;

components/selectors/sink.rs

@@ -6,7 +6,6 @@
 #![deny(missing_docs)]
 
 use smallvec::{Array, SmallVec};
-use std::marker::PhantomData;
 
 /// A trait to abstract over a `push` method that may be implemented for
 /// different kind of types.
@@ -30,24 +29,3 @@ impl<A: Array> Push<A::Item> for SmallVec<A> {
         SmallVec::push(self, value);
     }
 }
-
-/// A struct that implements `Push`, but only stores whether it's empty.
-pub struct ForgetfulSink<T>(bool, PhantomData<T>);
-
-impl<T> ForgetfulSink<T> {
-    /// Trivially construct a new `ForgetfulSink`.
-    pub fn new() -> Self {
-        ForgetfulSink(true, PhantomData)
-    }
-
-    /// Whether this sink is empty or not.
-    pub fn is_empty(&self) -> bool {
-        self.0
-    }
-}
-
-impl<T> Push<T> for ForgetfulSink<T> {
-    fn push(&mut self, _value: T) {
-        self.0 = false;
-    }
-}

components/style/dom.rs

@@ -21,8 +21,8 @@ use rule_tree::CascadeLevel;
 use selector_parser::{AttrValue, ElementExt, PreExistingComputedValues};
 use selector_parser::{PseudoClassStringArg, PseudoElement};
 use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
+use selectors::sink::Push;
 use shared_lock::Locked;
-use sink::Push;
 use smallvec::VecLike;
 use std::fmt;
 #[cfg(feature = "gecko")] use std::collections::HashMap;

components/style/gecko/selector_parser.rs

@@ -310,7 +310,7 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> {
                             Selector::parse(self, input)
                         })?;
                         // Selectors inside `:-moz-any` may not include combinators.
-                        if selectors.iter().flat_map(|x| x.iter_raw()).any(|s| s.is_combinator()) {
+                        if selectors.iter().flat_map(|x| x.iter_raw_match_order()).any(|s| s.is_combinator()) {
                             return Err(SelectorParseError::UnexpectedIdent("-moz-any".into()).into())
                         }
                         NonTSPseudoClass::MozAny(selectors.into_boxed_slice())

components/style/gecko/wrapper.rs

@@ -76,8 +76,8 @@ use selectors::Element;
 use selectors::attr::{AttrSelectorOperation, AttrSelectorOperator, CaseSensitivity, NamespaceConstraint};
 use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext};
 use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode};
+use selectors::sink::Push;
 use shared_lock::Locked;
-use sink::Push;
 use smallvec::VecLike;
 use std::cell::RefCell;
 use std::collections::HashMap;

components/style/invalidation/element/invalidator.rs

@@ -53,7 +53,7 @@ impl fmt::Debug for Invalidation {
         use cssparser::ToCss;
 
         f.write_str("Invalidation(")?;
-        for component in self.selector.iter_raw_rev_from(self.offset - 1) {
+        for component in self.selector.iter_raw_parse_order_from(self.offset - 1) {
             if matches!(*component, Component::Combinator(..)) {
                 break;
             }
@@ -568,7 +568,7 @@ impl<'a, 'b: 'a, E> TreeStyleInvalidator<'a, 'b, E>
                 if matches!(next_combinator, Combinator::PseudoElement) {
                     let pseudo_selector =
                         invalidation.selector
-                            .iter_raw_rev_from(next_combinator_offset - 1)
+                            .iter_raw_parse_order_from(next_combinator_offset - 1)
                             .next()
                             .unwrap();
                     let pseudo = match *pseudo_selector {

components/style/lib.rs

@@ -126,7 +126,6 @@ pub mod sharing;
 pub mod stylist;
 #[cfg(feature = "servo")] #[allow(unsafe_code)] pub mod servo;
 pub mod sequential;
-pub mod sink;
 pub mod str;
 pub mod style_adjuster;
 pub mod stylesheet_set;

components/style/selector_map.rs

@@ -369,15 +369,12 @@ impl<T: SelectorMapEntry> SelectorMap<T> {
     }
 }
 
-/// Searches the selector from right to left, beginning to the left of the
+/// Searches a compound selector from left to right. If the compound selector
-/// ::pseudo-element (if any), and ending at the first combinator.
+/// is a pseudo-element, it's ignored.
 ///
 /// The first non-None value returned from |f| is returned.
-///
-/// Effectively, pseudo-elements are ignored, given only state pseudo-classes
-/// may appear before them.
 #[inline(always)]
-fn find_from_right<F, R>(mut iter: SelectorIter<SelectorImpl>,
+fn find_from_left<F, R>(mut iter: SelectorIter<SelectorImpl>,
                          mut f: F)
                          -> Option<R>
     where F: FnMut(&Component<SelectorImpl>) -> Option<R>,
@@ -388,6 +385,8 @@ fn find_from_right<F, R>(mut iter: SelectorIter<SelectorImpl>,
         }
     }
 
+    // Effectively, pseudo-elements are ignored, given only state pseudo-classes
+    // may appear before them.
     if iter.next_sequence() == Some(Combinator::PseudoElement) {
         for ss in &mut iter {
             if let Some(r) = f(ss) {
@@ -403,7 +402,7 @@ fn find_from_right<F, R>(mut iter: SelectorIter<SelectorImpl>,
 #[inline(always)]
 pub fn get_id_name(iter: SelectorIter<SelectorImpl>)
                    -> Option<Atom> {
-    find_from_right(iter, |ss| {
+    find_from_left(iter, |ss| {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::ID(ref id) = *ss {
@@ -417,7 +416,7 @@ pub fn get_id_name(iter: SelectorIter<SelectorImpl>)
 #[inline(always)]
 pub fn get_class_name(iter: SelectorIter<SelectorImpl>)
                       -> Option<Atom> {
-    find_from_right(iter, |ss| {
+    find_from_left(iter, |ss| {
         // TODO(pradeep): Implement case-sensitivity based on the
         // document type and quirks mode.
         if let Component::Class(ref class) = *ss {
@@ -431,7 +430,7 @@ pub fn get_class_name(iter: SelectorIter<SelectorImpl>)
 #[inline(always)]
 pub fn get_local_name(iter: SelectorIter<SelectorImpl>)
                       -> Option<LocalNameSelector<SelectorImpl>> {
-    find_from_right(iter, |ss| {
+    find_from_left(iter, |ss| {
         if let Component::LocalName(ref n) = *ss {
             return Some(LocalNameSelector {
                 name: n.name.clone(),

components/style/stylist.rs

@@ -31,9 +31,9 @@ use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContex
 use selectors::matching::AFFECTED_BY_PRESENTATIONAL_HINTS;
 use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorAndHashes};
 use selectors::parser::{SelectorIter, SelectorMethods};
+use selectors::sink::Push;
 use selectors::visitor::SelectorVisitor;
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
-use sink::Push;
 use smallvec::VecLike;
 use std::fmt::Debug;
 #[cfg(feature = "servo")]

tests/unit/style/stylist.rs

@@ -194,7 +194,7 @@ fn test_get_id_name() {
 #[test]
 fn test_get_class_name() {
     let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]);
-    assert_eq!(selector_map::get_class_name(rules_list[0][0].selector.iter()), Some(Atom::from("foo")));
+    assert_eq!(selector_map::get_class_name(rules_list[0][0].selector.iter()), Some(Atom::from("intro")));
     assert_eq!(selector_map::get_class_name(rules_list[1][0].selector.iter()), None);
 }
 
@@ -220,8 +220,8 @@ fn test_insert() {
     selector_map.insert(rules_list[1][0].clone(), QuirksMode::NoQuirks);
     assert_eq!(1, selector_map.id_hash.get(&Atom::from("top"), QuirksMode::NoQuirks).unwrap()[0].source_order);
     selector_map.insert(rules_list[0][0].clone(), QuirksMode::NoQuirks);
-    assert_eq!(0, selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).unwrap()[0].source_order);
+    assert_eq!(0, selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).unwrap()[0].source_order);
-    assert!(selector_map.class_hash.get(&Atom::from("intro"), QuirksMode::NoQuirks).is_none());
+    assert!(selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).is_none());
 }
 
 #[test]