diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 8baba9bb54d..f5ed8bef31a 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -86,7 +86,7 @@ use net_traits::request::CorsSettings; use ref_filter_map::ref_filter_map; use script_layout_interface::message::ReflowQueryType; use script_thread::Runnable; -use selectors::matching::{ElementSelectorFlags, StyleRelations, matches}; +use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_selector_list}; use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS}; use selectors::parser::{AttrSelector, NamespaceConstraint}; use servo_atoms::Atom; @@ -2050,7 +2050,7 @@ impl ElementMethods for Element { match SelectorParser::parse_author_origin_no_namespace(&selectors) { Err(()) => Err(Error::Syntax), Ok(selectors) => { - Ok(matches(&selectors.0, &Root::from_ref(self), None)) + Ok(matches_selector_list(&selectors.0, &Root::from_ref(self), None)) } } } @@ -2068,7 +2068,7 @@ impl ElementMethods for Element { let root = self.upcast::(); for element in root.inclusive_ancestors() { if let Some(element) = Root::downcast::(element) { - if matches(&selectors.0, &element, None) + if matches_selector_list(&selectors.0, &element, None) { return Ok(Some(element)); } diff --git a/components/script/dom/node.rs b/components/script/dom/node.rs index e21dfa2bee0..2e82d8b6a5a 100644 --- a/components/script/dom/node.rs +++ b/components/script/dom/node.rs @@ -68,7 +68,7 @@ use script_layout_interface::{LayoutElementType, LayoutNodeType, TrustedNodeAddr use script_layout_interface::message::Msg; use script_traits::DocumentActivity; use script_traits::UntrustedNodeAddress; -use selectors::matching::matches; +use selectors::matching::matches_selector_list; use selectors::parser::SelectorList; use servo_url::ServoUrl; use std::borrow::ToOwned; @@ -332,7 +332,7 @@ impl<'a> Iterator for QuerySelectorIterator { // (instead of passing `None`)? Probably. self.iterator.by_ref().filter_map(|node| { if let Some(element) = Root::downcast(node) { - if matches(selectors, &element, None) { + if matches_selector_list(selectors, &element, None) { return Some(Root::upcast(element)); } } @@ -695,7 +695,7 @@ impl Node { // Step 3. Ok(selectors) => { Ok(self.traverse_preorder().filter_map(Root::downcast).find(|element| { - matches(&selectors.0, element, None) + matches_selector_list(&selectors.0, element, None) })) } } diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 206e98a77d6..dcb904fdb08 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -3,8 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use bloom::BloomFilter; use parser::{CaseSensitivity, Combinator, ComplexSelector, LocalName}; -use parser::{SimpleSelector, Selector}; -use precomputed_hash::PrecomputedHash; +use parser::{SimpleSelector, Selector, SelectorInner}; use std::borrow::Borrow; use tree::Element; @@ -19,10 +18,6 @@ bitflags! { /// /// This is used to implement efficient sharing. pub flags StyleRelations: u16 { - /// Whether this element has matched any rule that is determined by a - /// sibling (when using the `+` or `~` combinators). - const AFFECTED_BY_SIBLINGS = 1 << 0, - /// Whether this element has matched any rule whose matching is /// determined by its position in the tree (i.e., first-child, /// nth-child, etc.). @@ -96,103 +91,61 @@ impl ElementSelectorFlags { } } -pub fn matches(selector_list: &[Selector], - element: &E, - parent_bf: Option<&BloomFilter>) - -> bool +pub fn matches_selector_list(selector_list: &[Selector], + element: &E, + parent_bf: Option<&BloomFilter>) + -> bool where E: Element { selector_list.iter().any(|selector| { selector.pseudo_element.is_none() && - matches_complex_selector(&*selector.complex_selector, - element, - parent_bf, - &mut StyleRelations::empty(), - &mut |_, _| {}) + matches_selector(&selector.inner, + element, + parent_bf, + &mut StyleRelations::empty(), + &mut |_, _| {}) }) } -fn may_match(mut selector: &ComplexSelector, +fn may_match(sel: &SelectorInner, bf: &BloomFilter) -> bool where E: Element, { - // See if the bloom filter can exclude any of the descendant selectors, and - // reject if we can. - loop { - match selector.next { - None => break, - Some((ref cs, Combinator::Child)) | - Some((ref cs, Combinator::Descendant)) => selector = &**cs, - Some((ref cs, _)) => { - selector = &**cs; - continue; - } - }; + // Check against the list of precomputed hashes. + for hash in sel.ancestor_hashes.iter() { + // If we hit the 0 sentinel hash, that means the rest are zero as well. + if *hash == 0 { + break; + } - for ss in selector.compound_selector.iter() { - match *ss { - SimpleSelector::LocalName(LocalName { ref name, ref lower_name }) => { - if !bf.might_contain_hash(name.precomputed_hash()) && - !bf.might_contain_hash(lower_name.precomputed_hash()) { - return false - } - }, - SimpleSelector::Namespace(ref namespace) => { - if !bf.might_contain_hash(namespace.url.precomputed_hash()) { - return false - } - }, - SimpleSelector::ID(ref id) => { - if !bf.might_contain_hash(id.precomputed_hash()) { - return false - } - }, - SimpleSelector::Class(ref class) => { - if !bf.might_contain_hash(class.precomputed_hash()) { - return false - } - }, - _ => {}, - } + if !bf.might_contain_hash(*hash) { + return false; } } - // If we haven't proven otherwise, it may match. true } /// Determines whether the given element matches the given complex selector. -pub fn matches_complex_selector(selector: &ComplexSelector, - element: &E, - parent_bf: Option<&BloomFilter>, - relations: &mut StyleRelations, - flags_setter: &mut F) - -> bool +pub fn matches_selector(selector: &SelectorInner, + element: &E, + parent_bf: Option<&BloomFilter>, + relations: &mut StyleRelations, + flags_setter: &mut F) + -> bool where E: Element, F: FnMut(&E, ElementSelectorFlags), { + // Use the bloom filter to fast-reject. if let Some(filter) = parent_bf { if !may_match::(selector, filter) { return false; } } - match matches_complex_selector_internal(selector, - element, - relations, - flags_setter) { - SelectorMatchingResult::Matched => { - match selector.next { - Some((_, Combinator::NextSibling)) | - Some((_, Combinator::LaterSibling)) => *relations |= AFFECTED_BY_SIBLINGS, - _ => {} - } - - true - } - _ => false - } + // Match the selector. + matches_complex_selector(&selector.complex, element, relations, flags_setter) } /// A result of selector matching, includes 3 failure types, @@ -245,6 +198,24 @@ enum SelectorMatchingResult { NotMatchedGlobally, } +/// Matches a complex selector. +pub fn matches_complex_selector(selector: &ComplexSelector, + element: &E, + relations: &mut StyleRelations, + flags_setter: &mut F) + -> bool + where E: Element, + F: FnMut(&E, ElementSelectorFlags), +{ + match matches_complex_selector_internal(selector, + element, + relations, + flags_setter) { + SelectorMatchingResult::Matched => true, + _ => false + } +} + fn matches_complex_selector_internal(selector: &ComplexSelector, element: &E, relations: &mut StyleRelations, diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index bff599600e2..feff5913434 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -121,9 +121,52 @@ impl SelectorList { } } +/// Copied from Gecko, where it was noted to be unmeasured. +const NUM_ANCESTOR_HASHES: usize = 4; + +/// 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, Hash, Clone)] +pub struct SelectorInner { + /// The selector data. + pub complex: Arc>, + /// Ancestor hashes for the bloom filter. We precompute these and store + /// them inline to optimize cache performance during selector matching. + /// This matters a lot. + pub ancestor_hashes: [u32; NUM_ANCESTOR_HASHES], +} + +impl SelectorInner { + pub fn new(c: Arc>) -> Self { + let mut hashes = [0; NUM_ANCESTOR_HASHES]; + { + // Compute ancestor hashes for the bloom filter. + let mut hash_iter = + iter_ancestors(&c).flat_map(|x| x.compound_selector.iter()) + .map(|x| x.ancestor_hash()) + .filter(|x| x.is_some()) + .map(|x| x.unwrap()); + for i in 0..NUM_ANCESTOR_HASHES { + hashes[i] = match hash_iter.next() { + Some(x) => x, + None => break, + } + } + } + + SelectorInner { + complex: c, + ancestor_hashes: hashes, + } + } +} + #[derive(PartialEq, Eq, Hash, Clone)] pub struct Selector { - pub complex_selector: Arc>, + pub inner: SelectorInner, pub pseudo_element: Option, pub specificity: u32, } @@ -141,7 +184,7 @@ impl SelectorMethods for Selector { fn visit(&self, visitor: &mut V) -> bool where V: SelectorVisitor, { - self.complex_selector.visit(visitor) + self.inner.complex.visit(visitor) } } @@ -225,6 +268,35 @@ pub struct ComplexSelector { pub next: Option<(Arc>, Combinator)>, // c.next is left of c } +struct AncestorIterator<'a, Impl: 'a + SelectorImpl> { + curr: Option<&'a Arc>>, +} + +impl<'a, Impl: SelectorImpl> Iterator for AncestorIterator<'a, Impl> { + type Item = &'a Arc>; + fn next(&mut self) -> Option { + while let Some(sel) = self.curr.take() { + let (next_sel, is_ancestor) = match sel.next { + None => (None, true), + Some((ref sel, comb)) => + (Some(sel), matches!(comb, Combinator::Child | Combinator::Descendant)), + }; + self.curr = next_sel; + if is_ancestor { + break; + } + } + + self.curr + } +} + +fn iter_ancestors(sel: &Arc>) -> AncestorIterator { + AncestorIterator { + curr: Some(sel) + } +} + #[derive(Eq, PartialEq, Clone, Copy, Debug, Hash)] pub enum Combinator { Child, // > @@ -270,6 +342,34 @@ pub enum SimpleSelector { // ... } +impl SimpleSelector { + /// Compute the ancestor hash to check against the bloom filter. + fn ancestor_hash(&self) -> Option { + match *self { + SimpleSelector::LocalName(LocalName { ref name, ref lower_name }) => { + // Only insert the local-name into the filter if it's all lowercase. + // Otherwise we would need to test both hashes, and our data structures + // aren't really set up for that. + if name == lower_name { + Some(name.precomputed_hash()) + } else { + None + } + }, + SimpleSelector::Namespace(ref namespace) => { + Some(namespace.url.precomputed_hash()) + }, + SimpleSelector::ID(ref id) => { + Some(id.precomputed_hash()) + }, + SimpleSelector::Class(ref class) => { + Some(class.precomputed_hash()) + }, + _ => None, + } + } +} + #[derive(Eq, PartialEq, Clone, Hash, Copy, Debug)] pub enum CaseSensitivity { CaseSensitive, // Selectors spec says language-defined, but HTML says sensitive. @@ -321,6 +421,9 @@ 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) } } @@ -353,7 +456,7 @@ impl ToCss for SelectorList { impl ToCss for Selector { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - self.complex_selector.to_css(dest)?; + self.inner.complex.to_css(dest)?; if let Some(ref pseudo) = self.pseudo_element { pseudo.to_css(dest)?; } @@ -672,7 +775,7 @@ fn parse_selector(parser: &P, input: &mut CssParser) -> Result .ok"), Ok(SelectorList(vec![Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec![ SimpleSelector::Class(DummyAtom::from("ok")), ], @@ -1491,12 +1594,12 @@ pub mod tests { ], next: None, }), Combinator::Child)), - }), + })), pseudo_element: None, specificity: (1 << 20) + (1 << 10) + (0 << 0), }]))); assert_eq!(parse(":not(.babybel, #provel.old)"), Ok(SelectorList(vec!(Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec!(SimpleSelector::Negation( vec!( Arc::new(ComplexSelector { @@ -1513,7 +1616,7 @@ pub mod tests { ) )), next: None, - }), + })), pseudo_element: None, specificity: specificity(1, 1, 0), })))); diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index f64703dfd9c..7dcb0fdfb71 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1150,11 +1150,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { }, NonTSPseudoClass::MozAny(ref sels) => { sels.iter().any(|s| { - matches_complex_selector(s, - self, - None, - relations, - flags_setter) + matches_complex_selector(s, self, relations, flags_setter) }) } NonTSPseudoClass::MozSystemMetric(ref s) | diff --git a/components/style/restyle_hints.rs b/components/style/restyle_hints.rs index 406de50b157..0ac298442ba 100644 --- a/components/style/restyle_hints.rs +++ b/components/style/restyle_hints.rs @@ -16,8 +16,8 @@ use heapsize::HeapSizeOf; use selector_parser::{AttrValue, NonTSPseudoClass, Snapshot, SelectorImpl}; use selectors::{Element, MatchAttr}; use selectors::matching::{ElementSelectorFlags, StyleRelations}; -use selectors::matching::matches_complex_selector; -use selectors::parser::{AttrSelector, Combinator, ComplexSelector, SelectorMethods, SimpleSelector}; +use selectors::matching::matches_selector; +use selectors::parser::{AttrSelector, Combinator, ComplexSelector, SelectorInner, SelectorMethods, SimpleSelector}; use selectors::visitor::SelectorVisitor; use std::clone::Clone; use std::sync::Arc; @@ -298,13 +298,10 @@ impl<'a, E> Element for ElementWrapper<'a, E> // snapshot. #[cfg(feature = "gecko")] { + use selectors::matching::matches_complex_selector; if let NonTSPseudoClass::MozAny(ref selectors) = *pseudo_class { return selectors.iter().any(|s| { - matches_complex_selector(s, - self, - None, - relations, - _setter) + matches_complex_selector(s, self, relations, _setter) }) } } @@ -493,7 +490,7 @@ impl Sensitivities { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] struct Dependency { #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - selector: Arc>, + selector: SelectorInner, hint: RestyleHint, sensitivities: Sensitivities, } @@ -603,7 +600,7 @@ impl DependencySet { self.add_dependency(Dependency { sensitivities: sensitivities, hint: hint, - selector: current.clone(), + selector: SelectorInner::new(current.clone()), }) } @@ -699,13 +696,13 @@ impl DependencySet { // We can ignore the selector flags, since they would have already been set during // original matching for any element that might change its matching behavior here. let matched_then = - matches_complex_selector(&dep.selector, snapshot, None, - &mut StyleRelations::empty(), - &mut |_, _| {}); + matches_selector(&dep.selector, snapshot, None, + &mut StyleRelations::empty(), + &mut |_, _| {}); let matches_now = - matches_complex_selector(&dep.selector, element, None, - &mut StyleRelations::empty(), - &mut |_, _| {}); + matches_selector(&dep.selector, element, None, + &mut StyleRelations::empty(), + &mut |_, _| {}); if matched_then != matches_now { hint.insert(dep.hint); } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index d91a4b6250d..76b12854b61 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -26,8 +26,8 @@ use selectors::Element; use selectors::bloom::BloomFilter; use selectors::matching::{AFFECTED_BY_ANIMATIONS, AFFECTED_BY_TRANSITIONS}; use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS}; -use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_complex_selector}; -use selectors::parser::{Selector, SimpleSelector, LocalName as LocalNameSelector, ComplexSelector}; +use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_selector}; +use selectors::parser::{Selector, SelectorInner, SimpleSelector, LocalName as LocalNameSelector}; use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards}; use sink::Push; use smallvec::VecLike; @@ -336,7 +336,7 @@ impl Stylist { }; map.insert(Rule { - selector: selector.complex_selector.clone(), + selector: selector.inner.clone(), style_rule: locked.clone(), specificity: selector.specificity, source_order: self.rules_source_order, @@ -346,7 +346,7 @@ impl Stylist { for selector in &style_rule.selectors.0 { let needs_cache_revalidation = - self.dependencies.note_selector(&selector.complex_selector); + self.dependencies.note_selector(&selector.inner.complex); if needs_cache_revalidation { self.selectors_for_cache_revalidation.push(selector.clone()); } @@ -826,18 +826,18 @@ impl Stylist { F: FnMut(&E, ElementSelectorFlags) { use selectors::matching::StyleRelations; - use selectors::matching::matches_complex_selector; + use selectors::matching::matches_selector; let len = self.selectors_for_cache_revalidation.len(); let mut results = BitVec::from_elem(len, false); for (i, ref selector) in self.selectors_for_cache_revalidation .iter().enumerate() { - results.set(i, matches_complex_selector(&selector.complex_selector, - element, - Some(bloom), - &mut StyleRelations::empty(), - flags_setter)); + results.set(i, matches_selector(&selector.inner, + element, + Some(bloom), + &mut StyleRelations::empty(), + flags_setter)); } results @@ -1081,8 +1081,8 @@ impl SelectorMap { // correct, and also to not trigger rule tree assertions. let mut important = vec![]; for rule in self.other_rules.iter() { - if rule.selector.compound_selector.is_empty() && - rule.selector.next.is_none() { + if rule.selector.complex.compound_selector.is_empty() && + rule.selector.complex.next.is_none() { let style_rule = rule.style_rule.read_with(guard); let block = style_rule.block.read_with(guard); if block.any_normal() { @@ -1157,8 +1157,8 @@ impl SelectorMap { block.any_normal() }; if any_declaration_for_importance && - matches_complex_selector(&*rule.selector, element, parent_bf, - relations, flags_setter) { + matches_selector(&rule.selector, element, parent_bf, + relations, flags_setter) { matching_rules.push( rule.to_applicable_declaration_block(cascade_level)); } @@ -1191,7 +1191,7 @@ impl SelectorMap { /// Retrieve the first ID name in Rule, or None otherwise. pub fn get_id_name(rule: &Rule) -> Option { - for ss in &rule.selector.compound_selector { + for ss in &rule.selector.complex.compound_selector { // TODO(pradeep): Implement case-sensitivity based on the // document type and quirks mode. if let SimpleSelector::ID(ref id) = *ss { @@ -1204,7 +1204,7 @@ impl SelectorMap { /// Retrieve the FIRST class name in Rule, or None otherwise. pub fn get_class_name(rule: &Rule) -> Option { - for ss in &rule.selector.compound_selector { + for ss in &rule.selector.complex.compound_selector { // TODO(pradeep): Implement case-sensitivity based on the // document type and quirks mode. if let SimpleSelector::Class(ref class) = *ss { @@ -1217,7 +1217,7 @@ impl SelectorMap { /// Retrieve the name if it is a type selector, or None otherwise. pub fn get_local_name(rule: &Rule) -> Option> { - for ss in &rule.selector.compound_selector { + for ss in &rule.selector.complex.compound_selector { if let SimpleSelector::LocalName(ref n) = *ss { return Some(LocalNameSelector { name: n.name.clone(), @@ -1245,7 +1245,7 @@ pub struct Rule { /// style rule was a generic parameter to it. It's not trivial though, due /// to the specificity. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] - pub selector: Arc>, + pub selector: SelectorInner, /// The actual style rule. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub style_rule: Arc>, diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 7935470f504..0d2bfe5f387 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -84,7 +84,7 @@ fn test_parse_stylesheet() { CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec![ SimpleSelector::Namespace(Namespace { prefix: None, @@ -104,7 +104,7 @@ fn test_parse_stylesheet() { }, "hidden".to_owned(), CaseSensitivity::CaseInsensitive) ], next: None, - }), + })), pseudo_element: None, specificity: (0 << 20) + (1 << 10) + (1 << 0), }, @@ -120,7 +120,7 @@ fn test_parse_stylesheet() { CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec![ SimpleSelector::Namespace(Namespace { prefix: None, @@ -132,12 +132,12 @@ fn test_parse_stylesheet() { }), ], next: None, - }), + })), pseudo_element: None, specificity: (0 << 20) + (0 << 10) + (1 << 0), }, Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec![ SimpleSelector::Namespace(Namespace { prefix: None, @@ -149,7 +149,7 @@ fn test_parse_stylesheet() { }), ], next: None, - }), + })), pseudo_element: None, specificity: (0 << 20) + (0 << 10) + (1 << 0), }, @@ -162,7 +162,7 @@ fn test_parse_stylesheet() { CssRule::Style(Arc::new(stylesheet.shared_lock.wrap(StyleRule { selectors: SelectorList(vec![ Selector { - complex_selector: Arc::new(ComplexSelector { + inner: SelectorInner::new(Arc::new(ComplexSelector { compound_selector: vec![ SimpleSelector::Namespace(Namespace { prefix: None, @@ -180,7 +180,7 @@ fn test_parse_stylesheet() { ], next: None, }), Combinator::Child)), - }), + })), pseudo_element: None, specificity: (1 << 20) + (1 << 10) + (0 << 0), }, diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 8f559703cbe..77791ebeedc 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -35,7 +35,7 @@ fn get_mock_rules(css_selectors: &[&str]) -> (Vec>, SharedRwLock) { let rule = locked.read_with(&guard); rule.selectors.0.iter().map(|s| { Rule { - selector: s.complex_selector.clone(), + selector: s.inner.clone(), style_rule: locked.clone(), specificity: s.specificity, source_order: i,