From 9524c5cb57673b1db32b57b10107703b8d241bfd Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Sat, 15 Apr 2017 15:03:46 -0700 Subject: [PATCH] Introduce SelectorInner and use it for top-level matching. MozReview-Commit-ID: DxG6USsPIkh --- components/script/dom/element.rs | 6 +-- components/script/dom/node.rs | 6 +-- components/selectors/matching.rs | 63 ++++++++++++++--------- components/selectors/parser.rs | 85 +++++++++++++++++++------------ components/style/gecko/wrapper.rs | 6 +-- components/style/restyle_hints.rs | 27 +++++----- components/style/stylist.rs | 36 ++++++------- tests/unit/style/stylesheets.rs | 16 +++--- tests/unit/style/stylist.rs | 2 +- 9 files changed, 138 insertions(+), 109 deletions(-) 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 c6255605468..9abfa2d4612 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -3,7 +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 parser::{SimpleSelector, Selector, SelectorInner}; use precomputed_hash::PrecomputedHash; use std::borrow::Borrow; use tree::Element; @@ -92,27 +92,28 @@ 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, { + let mut selector = &*sel.complex; // See if the bloom filter can exclude any of the descendant selectors, and // reject if we can. loop { @@ -159,28 +160,24 @@ fn may_match(mut selector: &ComplexSelector, } /// 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 => true, - _ => false - } + // Match the selector. + matches_complex_selector(&selector.complex, element, relations, flags_setter) } /// A result of selector matching, includes 3 failure types, @@ -233,6 +230,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..d31be6c0f78 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -121,9 +121,27 @@ impl SelectorList { } } +/// 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 { + pub complex: Arc>, +} + +impl SelectorInner { + pub fn new(c: Arc>) -> Self { + SelectorInner { + complex: c, + } + } +} + #[derive(PartialEq, Eq, Hash, Clone)] pub struct Selector { - pub complex_selector: Arc>, + pub inner: SelectorInner, pub pseudo_element: Option, pub specificity: u32, } @@ -141,7 +159,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) } } @@ -321,6 +339,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 +374,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 +693,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 +1512,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 +1534,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,