From 5ddabef636fa91c2d0d0a9d9c90d2a7c71874ca6 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 1 Jun 2017 15:38:33 -0700 Subject: [PATCH] Collapse Selector, SelectorInner, and ComplexSelector into a single Selector. The refcounting is still internal. We'll fix that up next. MozReview-Commit-ID: CTxZNaR3Qgj --- components/selectors/matching.rs | 14 +- components/selectors/parser.rs | 169 ++++++------------- components/style/gecko/selector_parser.rs | 6 +- components/style/invalidation/stylesheets.rs | 2 +- components/style/restyle_hints.rs | 21 +-- components/style/selector_map.rs | 4 +- components/style/stylist.rs | 2 +- 7 files changed, 71 insertions(+), 147 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 094da6b3ca7..133e3232d04 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -4,8 +4,8 @@ use attr::{ParsedAttrSelectorOperation, AttrSelectorOperation, NamespaceConstraint}; use bloom::BloomFilter; -use parser::{AncestorHashes, Combinator, ComplexSelector, Component, LocalName}; -use parser::{SelectorInner, SelectorIter, SelectorList}; +use parser::{AncestorHashes, Combinator, Component, LocalName}; +use parser::{Selector, SelectorIter, SelectorList}; use std::borrow::Borrow; use tree::Element; @@ -159,7 +159,7 @@ pub fn matches_selector_list(selector_list: &SelectorList, where E: Element { selector_list.0.iter().any(|selector_and_hashes| { - matches_selector(&selector_and_hashes.selector.inner, + matches_selector(&selector_and_hashes.selector, &selector_and_hashes.hashes, element, context, @@ -334,7 +334,7 @@ enum SelectorMatchingResult { /// Matches a selector, fast-rejecting against a bloom filter. #[inline(always)] -pub fn matches_selector(selector: &SelectorInner, +pub fn matches_selector(selector: &Selector, hashes: &AncestorHashes, element: &E, context: &mut MatchingContext, @@ -350,13 +350,11 @@ pub fn matches_selector(selector: &SelectorInner, } } - matches_complex_selector(&selector.complex, element, context, flags_setter) + matches_complex_selector(selector, element, context, flags_setter) } /// Matches a complex selector. -/// -/// Use `matches_selector` if you need to skip pseudos. -pub fn matches_complex_selector(complex_selector: &ComplexSelector, +pub fn matches_complex_selector(complex_selector: &Selector, element: &E, context: &mut MatchingContext, flags_setter: &mut F) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index e083ecdab75..c442d24ab9d 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -176,10 +176,10 @@ const NUM_ANCESTOR_HASHES: usize = 4; pub struct AncestorHashes(pub [u32; NUM_ANCESTOR_HASHES]); impl AncestorHashes { - pub fn new(c: &Selector) -> Self { + pub fn new(s: &Selector) -> Self { let mut hashes = [0; NUM_ANCESTOR_HASHES]; // Compute ancestor hashes for the bloom filter. - let mut hash_iter = c.inner.complex.iter_ancestors() + let mut hash_iter = s.iter_ancestors() .map(|x| x.ancestor_hash()) .filter(|x| x.is_some()) .map(|x| x.unwrap()); @@ -194,81 +194,8 @@ impl AncestorHashes { } } -/// The cores parts of a selector used for matching. This exists to make that -/// information accessibly separately from the specificity and pseudo-element -/// information that lives on |Selector| proper. We may want to refactor things -/// and move that information elsewhere, at which point we could rename this -/// to |Selector|. -#[derive(PartialEq, Eq, Clone)] -pub struct SelectorInner { - /// The selector data. - pub complex: ComplexSelector, -} - -impl SelectorInner { - pub fn new(c: ComplexSelector) -> Self { - SelectorInner { - complex: c, - } - } - - /// Creates a SelectorInner from a Vec of Components. Used in tests. - pub fn from_vec(vec: Vec>, specificity_and_flags: u32) -> Self { - let complex = ComplexSelector::from_vec(vec, specificity_and_flags); - Self::new(complex) - } -} - -#[derive(PartialEq, Eq, Clone)] -pub struct Selector { - pub inner: SelectorInner, -} - -impl ::std::borrow::Borrow> for Selector { - fn borrow(&self) -> &SelectorInner { - &self.inner - } -} - const HAS_PSEUDO_BIT: u32 = 1 << 30; -impl Selector { - pub fn specificity(&self) -> u32 { - self.inner.complex.specificity() - } - - pub fn pseudo_element(&self) -> Option<&Impl::PseudoElement> { - if !self.has_pseudo_element() { - return None - } - - for component in self.inner.complex.iter() { - if let Component::PseudoElement(ref pseudo) = *component { - return Some(pseudo) - } - } - - debug_assert!(false, "has_pseudo_element lied!"); - None - } - - pub fn has_pseudo_element(&self) -> bool { - self.inner.complex.has_pseudo_element() - } - - /// Whether this selector (pseudo-element part excluded) matches every element. - /// - /// Used for "pre-computed" pseudo-elements in components/style/stylist.rs - pub fn is_universal(&self) -> bool { - self.inner.complex.iter_raw().all(|c| matches!(*c, - Component::ExplicitUniversalType | - Component::ExplicitAnyNamespace | - Component::Combinator(Combinator::PseudoElement) | - Component::PseudoElement(..) - )) - } -} - pub trait SelectorMethods { type Impl: SelectorImpl; @@ -279,16 +206,6 @@ pub trait SelectorMethods { impl SelectorMethods for Selector { type Impl = Impl; - fn visit(&self, visitor: &mut V) -> bool - where V: SelectorVisitor, - { - self.inner.complex.visit(visitor) - } -} - -impl SelectorMethods for ComplexSelector { - type Impl = Impl; - fn visit(&self, visitor: &mut V) -> bool where V: SelectorVisitor, { @@ -381,7 +298,7 @@ pub fn namespace_empty_string() -> Impl::NamespaceUrl { Impl::NamespaceUrl::default() } -/// A ComplexSelectors stores a sequence of simple selectors and combinators. The +/// 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. @@ -390,9 +307,9 @@ pub fn namespace_empty_string() -> Impl::NamespaceUrl { /// canonical iteration order is right-to-left (selector matching order). The /// iterators abstract over these details. #[derive(Clone, Eq, PartialEq)] -pub struct ComplexSelector(ArcSlice>, u32); +pub struct Selector(ArcSlice>, u32); -impl ComplexSelector { +impl Selector { pub fn specificity(&self) -> u32 { self.1 & !HAS_PSEUDO_BIT } @@ -401,6 +318,34 @@ impl ComplexSelector { (self.1 & HAS_PSEUDO_BIT) != 0 } + pub fn pseudo_element(&self) -> Option<&Impl::PseudoElement> { + if !self.has_pseudo_element() { + return None + } + + for component in self.iter() { + if let Component::PseudoElement(ref pseudo) = *component { + return Some(pseudo) + } + } + + debug_assert!(false, "has_pseudo_element lied!"); + None + } + + /// Whether this selector (pseudo-element part excluded) matches every element. + /// + /// Used for "pre-computed" pseudo-elements in components/style/stylist.rs + pub fn is_universal(&self) -> bool { + self.iter_raw().all(|c| matches!(*c, + Component::ExplicitUniversalType | + Component::ExplicitAnyNamespace | + Component::Combinator(Combinator::PseudoElement) | + Component::PseudoElement(..) + )) + } + + /// Returns an iterator over the next sequence of simple selectors. When /// a combinator is reached, the iterator will return None, and /// next_sequence() may be called to continue to the next sequence. @@ -429,25 +374,25 @@ impl ComplexSelector { AncestorIter::new(self.iter()) } - /// Returns a ComplexSelector identical to |self| but with the rightmost |index| - /// entries removed. + /// Returns a Selector identical to |self| but with the rightmost |index| entries + /// removed. pub fn slice_from(&self, index: usize) -> Self { // Note that we convert the slice_from to slice_to because selectors are // stored left-to-right but logical order is right-to-left. - ComplexSelector(self.0.clone().slice_to(self.0.len() - index), self.1) + Selector(self.0.clone().slice_to(self.0.len() - index), self.1) } - /// Returns a ComplexSelector identical to |self| but with the leftmost - /// |len() - index| entries removed. + /// Returns a Selector identical to |self| but with the leftmost |len() - index| + /// entries removed. pub fn slice_to(&self, index: usize) -> Self { // Note that we convert the slice_to to slice_from because selectors are // stored left-to-right but logical order is right-to-left. - ComplexSelector(self.0.clone().slice_from(self.0.len() - index), self.1) + Selector(self.0.clone().slice_from(self.0.len() - index), self.1) } - /// Creates a ComplexSelector from a vec of Components. Used in tests. + /// Creates a Selector from a vec of Components. Used in tests. pub fn from_vec(vec: Vec>, specificity_and_flags: u32) -> Self { - ComplexSelector(ArcSlice::new(vec.into_boxed_slice()), specificity_and_flags) + Selector(ArcSlice::new(vec.into_boxed_slice()), specificity_and_flags) } } @@ -671,12 +616,6 @@ impl Debug for Selector { } } -impl Debug for SelectorInner { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.complex.to_css(f) } -} -impl Debug for ComplexSelector { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } -} impl Debug for Component { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } } @@ -702,12 +641,6 @@ impl ToCss for SelectorList { } impl ToCss for Selector { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.inner.complex.to_css(dest) - } -} - -impl ToCss for ComplexSelector { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { for item in self.iter_raw_rev() { item.to_css(dest)?; @@ -916,13 +849,13 @@ impl From for u32 { } } -fn specificity(complex_selector: &ComplexSelector) -> u32 +fn specificity(complex_selector: &Selector) -> u32 where Impl: SelectorImpl { complex_selector_specificity(complex_selector).into() } -fn complex_selector_specificity(selector: &ComplexSelector) +fn complex_selector_specificity(selector: &Selector) -> Specificity where Impl: SelectorImpl { @@ -992,15 +925,13 @@ fn complex_selector_specificity(selector: &ComplexSelector) fn parse_selector(parser: &P, input: &mut CssParser) -> Result, ()> where P: Parser, Impl: SelectorImpl { - let (mut complex, has_pseudo_element) = parse_complex_selector(parser, input)?; - let mut specificity = specificity(&complex); + let (mut selector, has_pseudo_element) = parse_complex_selector(parser, input)?; + let mut specificity = specificity(&selector); if has_pseudo_element { specificity |= HAS_PSEUDO_BIT; } - complex.1 = specificity; - Ok(Selector { - inner: SelectorInner::new(complex), - }) + selector.1 = specificity; + Ok(selector) } /// We use a SmallVec for parsing to avoid extra reallocs compared to using a Vec @@ -1021,7 +952,7 @@ type ParseVec = SmallVec<[Component; 8]>; fn parse_complex_selector( parser: &P, input: &mut CssParser) - -> Result<(ComplexSelector, bool), ()> + -> Result<(Selector, bool), ()> where P: Parser, Impl: SelectorImpl { let mut sequence = ParseVec::new(); @@ -1069,11 +1000,11 @@ fn parse_complex_selector( sequence.push(Component::Combinator(combinator)); } - let complex = ComplexSelector(ArcSlice::new(sequence.into_vec().into_boxed_slice()), 0); + let complex = Selector(ArcSlice::new(sequence.into_vec().into_boxed_slice()), 0); Ok((complex, parsed_pseudo_element)) } -impl ComplexSelector { +impl Selector { /// Parse a complex selector, without any pseudo-element. pub fn parse

(parser: &P, input: &mut CssParser) -> Result where P: Parser diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index b6051f1f34c..90d503f58ab 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -8,7 +8,7 @@ use cssparser::{Parser, ToCss}; use element_state::ElementState; use gecko_bindings::structs::CSSPseudoClassType; use selector_parser::{SelectorParser, PseudoElementCascadeType}; -use selectors::parser::{ComplexSelector, SelectorMethods}; +use selectors::parser::{Selector, SelectorMethods}; use selectors::visitor::SelectorVisitor; use std::borrow::Cow; use std::fmt; @@ -47,7 +47,7 @@ macro_rules! pseudo_class_name { /// /// TODO(emilio): We disallow combinators and pseudos here, so we /// should use SimpleSelector instead - MozAny(Box<[ComplexSelector]>), + MozAny(Box<[Selector]>), } } } @@ -273,7 +273,7 @@ impl<'a> ::selectors::Parser for SelectorParser<'a> { }, )* "-moz-any" => { let selectors = parser.parse_comma_separated(|input| { - ComplexSelector::parse(self, input) + 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()) { diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index adfb4b97a59..a222b611d44 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/components/style/invalidation/stylesheets.rs @@ -218,7 +218,7 @@ impl StylesheetInvalidationSet { let mut scope: Option = None; let mut scan = true; - let mut iter = selector.inner.complex.iter(); + let mut iter = selector.iter(); loop { for component in &mut iter { diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 615445bef2a..1a65e821e86 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -23,7 +23,7 @@ use selectors::attr::{AttrSelectorOperation, NamespaceConstraint}; use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode}; use selectors::matching::{RelevantLinkStatus, VisitedHandlingMode, matches_selector}; use selectors::parser::{AncestorHashes, Combinator, Component}; -use selectors::parser::{Selector, SelectorAndHashes, SelectorInner, SelectorMethods}; +use selectors::parser::{Selector, SelectorAndHashes, SelectorMethods}; use selectors::visitor::SelectorVisitor; use smallvec::SmallVec; use std::cell::Cell; @@ -949,7 +949,7 @@ impl DependencySet { /// Adds a selector to this `DependencySet`. pub fn note_selector(&mut self, selector_and_hashes: &SelectorAndHashes) { let mut combinator = None; - let mut iter = selector_and_hashes.selector.inner.complex.iter(); + let mut iter = selector_and_hashes.selector.iter(); let mut index = 0; let mut child_combinators_seen = 0; let mut saw_descendant_combinator = false; @@ -1001,14 +1001,9 @@ impl DependencySet { let (dep_selector, hashes) = if sequence_start == 0 { // Reuse the bloom hashes if this is the base selector. - (selector_and_hashes.selector.clone(), - selector_and_hashes.hashes.clone()) - + (selector_and_hashes.selector.clone(), selector_and_hashes.hashes.clone()) } else { - let inner = SelectorInner::new(selector_and_hashes.selector - .inner - .complex.slice_from(sequence_start)); - let selector = Selector { inner: inner }; + let selector = selector_and_hashes.selector.slice_from(sequence_start); let hashes = AncestorHashes::new(&selector); (selector, hashes) }; @@ -1145,7 +1140,7 @@ impl DependencySet { MatchingContext::new_for_visited(MatchingMode::Normal, None, VisitedHandlingMode::AllLinksUnvisited); let matched_then = - matches_selector(&dep.selector.inner, + matches_selector(&dep.selector, &dep.hashes, &snapshot_el, &mut then_context, @@ -1154,7 +1149,7 @@ impl DependencySet { MatchingContext::new_for_visited(MatchingMode::Normal, bloom_filter, VisitedHandlingMode::AllLinksUnvisited); let matches_now = - matches_selector(&dep.selector.inner, + matches_selector(&dep.selector, &dep.hashes, el, &mut now_context, @@ -1181,14 +1176,14 @@ impl DependencySet { dep.sensitivities.states.intersects(IN_VISITED_OR_UNVISITED_STATE) { then_context.visited_handling = VisitedHandlingMode::RelevantLinkVisited; let matched_then = - matches_selector(&dep.selector.inner, + matches_selector(&dep.selector, &dep.hashes, &snapshot_el, &mut then_context, &mut |_, _| {}); now_context.visited_handling = VisitedHandlingMode::RelevantLinkVisited; let matches_now = - matches_selector(&dep.selector.inner, + matches_selector(&dep.selector, &dep.hashes, el, &mut now_context, diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 4120f000704..3a295bb2588 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -231,7 +231,7 @@ impl SelectorMap { F: FnMut(&E, ElementSelectorFlags), { for rule in rules { - if matches_selector(&rule.selector.inner, + if matches_selector(&rule.selector, &rule.hashes, element, context, @@ -403,7 +403,7 @@ fn find_from_right(selector: &Selector, -> Option where F: FnMut(&Component) -> Option, { - let mut iter = selector.inner.complex.iter(); + let mut iter = selector.iter(); for ss in &mut iter { if let Some(r) = f(ss) { return Some(r) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 010d37e1e53..b90e6fc42ab 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1133,7 +1133,7 @@ impl Stylist { // this in the caller by asserting that the bitvecs are same-length. let mut results = BitVec::new(); self.selectors_for_cache_revalidation.lookup(*element, &mut |selector_and_hashes| { - results.push(matches_selector(&selector_and_hashes.selector.inner, + results.push(matches_selector(&selector_and_hashes.selector, &selector_and_hashes.hashes, element, &mut matching_context,