diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index 9fc5b0815a1..f65703d0578 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -164,10 +164,10 @@ pub fn matches_selector_list(selector_list: &SelectorList, -> bool where E: Element { - selector_list.0.iter().any(|selector_and_hashes| { - matches_selector(&selector_and_hashes.selector, + selector_list.0.iter().any(|selector| { + matches_selector(selector, 0, - &selector_and_hashes.hashes, + None, element, context, &mut |_, _| {}) @@ -371,7 +371,7 @@ enum SelectorMatchingResult { #[inline(always)] pub fn matches_selector(selector: &Selector, offset: usize, - hashes: &AncestorHashes, + hashes: Option<&AncestorHashes>, element: &E, context: &mut MatchingContext, flags_setter: &mut F) @@ -380,9 +380,11 @@ pub fn matches_selector(selector: &Selector, F: FnMut(&E, ElementSelectorFlags), { // Use the bloom filter to fast-reject. - if let Some(filter) = context.bloom_filter { - if !may_match::(hashes, filter) { - return false; + if let Some(hashes) = hashes { + if let Some(filter) = context.bloom_filter { + if !may_match::(hashes, filter) { + return false; + } } } diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 77d31741de6..0cb7c6e3f47 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -177,27 +177,7 @@ pub trait Parser<'i> { } #[derive(PartialEq, Eq, Clone, Debug)] -pub struct SelectorAndHashes { - pub selector: Selector, - pub hashes: AncestorHashes, -} - -impl SelectorAndHashes { - pub fn new(selector: Selector) -> Self { - let hashes = AncestorHashes::new(&selector); - Self::new_with_hashes(selector, hashes) - } - - pub fn new_with_hashes(selector: Selector, hashes: AncestorHashes) -> Self { - SelectorAndHashes { - selector: selector, - hashes: hashes, - } - } -} - -#[derive(PartialEq, Eq, Clone, Debug)] -pub struct SelectorList(pub Vec>); +pub struct SelectorList(pub Vec>); impl SelectorList { /// Parse a comma-separated list of Selectors. @@ -207,13 +187,13 @@ impl SelectorList { pub fn parse<'i, 't, P, E>(parser: &P, input: &mut CssParser<'i, 't>) -> Result>> where P: Parser<'i, Impl=Impl, Error=E> { - input.parse_comma_separated(|input| parse_selector(parser, input).map(SelectorAndHashes::new)) + input.parse_comma_separated(|input| parse_selector(parser, input)) .map(SelectorList) } /// Creates a SelectorList from a Vec of selectors. Used in tests. pub fn from_vec(v: Vec>) -> Self { - SelectorList(v.into_iter().map(SelectorAndHashes::new).collect()) + SelectorList(v) } } @@ -761,10 +741,10 @@ impl ToCss for SelectorList { let mut iter = self.0.iter(); let first = iter.next() .expect("Empty SelectorList, should contain at least one selector"); - first.selector.to_css(dest)?; - for selector_and_hashes in iter { + first.to_css(dest)?; + for selector in iter { dest.write_str(", ")?; - selector_and_hashes.selector.to_css(dest)?; + selector.to_css(dest)?; } Ok(()) } @@ -1761,7 +1741,7 @@ pub mod tests { let result = SelectorList::parse(parser, &mut CssParser::new(&mut parser_input)); if let Ok(ref selectors) = result { assert_eq!(selectors.0.len(), 1); - assert_eq!(selectors.0[0].selector.to_css_string(), input); + assert_eq!(selectors.0[0].to_css_string(), input); } result } @@ -2066,7 +2046,7 @@ pub mod tests { #[test] fn test_pseudo_iter() { - let selector = &parse("q::before").unwrap().0[0].selector; + let selector = &parse("q::before").unwrap().0[0]; assert!(!selector.is_universal()); let mut iter = selector.iter(); assert_eq!(iter.next(), Some(&Component::PseudoElement(PseudoElement::Before))); @@ -2080,13 +2060,13 @@ pub mod tests { #[test] fn test_universal() { - let selector = &parse("*|*::before").unwrap().0[0].selector; + let selector = &parse("*|*::before").unwrap().0[0]; assert!(selector.is_universal()); } #[test] fn test_empty_pseudo_iter() { - let selector = &parse("::before").unwrap().0[0].selector; + let selector = &parse("::before").unwrap().0[0]; assert!(selector.is_universal()); let mut iter = selector.iter(); assert_eq!(iter.next(), Some(&Component::PseudoElement(PseudoElement::Before))); @@ -2112,11 +2092,11 @@ pub mod tests { #[test] fn visitor() { let mut test_visitor = TestVisitor { seen: vec![], }; - parse(":not(:hover) ~ label").unwrap().0[0].selector.visit(&mut test_visitor); + parse(":not(:hover) ~ label").unwrap().0[0].visit(&mut test_visitor); assert!(test_visitor.seen.contains(&":hover".into())); let mut test_visitor = TestVisitor { seen: vec![], }; - parse("::before:hover").unwrap().0[0].selector.visit(&mut test_visitor); + parse("::before:hover").unwrap().0[0].visit(&mut test_visitor); assert!(test_visitor.seen.contains(&":hover".into())); } } diff --git a/components/style/invalidation/element/invalidation_map.rs b/components/style/invalidation/element/invalidation_map.rs index 8e4dfebec99..e5c6c63bfce 100644 --- a/components/style/invalidation/element/invalidation_map.rs +++ b/components/style/invalidation/element/invalidation_map.rs @@ -10,8 +10,8 @@ use element_state::ElementState; use selector_map::{MaybeCaseInsensitiveHashMap, SelectorMap, SelectorMapEntry}; use selector_parser::SelectorImpl; use selectors::attr::NamespaceConstraint; -use selectors::parser::{AncestorHashes, Combinator, Component}; -use selectors::parser::{Selector, SelectorAndHashes, SelectorIter, SelectorMethods}; +use selectors::parser::{Combinator, Component}; +use selectors::parser::{Selector, SelectorIter, SelectorMethods}; use selectors::visitor::SelectorVisitor; use smallvec::SmallVec; @@ -59,10 +59,6 @@ pub struct Dependency { /// The dependency selector. #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")] pub selector: Selector, - /// The ancestor hashes associated with the above selector at the given - /// offset. - #[cfg_attr(feature = "servo", ignore_heap_size_of = "No heap data")] - pub hashes: AncestorHashes, /// The offset into the selector that we should match on. pub selector_offset: usize, } @@ -109,10 +105,6 @@ impl SelectorMapEntry for Dependency { fn selector(&self) -> SelectorIter { self.selector.iter_from(self.selector_offset) } - - fn hashes(&self) -> &AncestorHashes { - &self.hashes - } } /// The same, but for state selectors, which can track more exactly what state @@ -128,11 +120,7 @@ pub struct StateDependency { impl SelectorMapEntry for StateDependency { fn selector(&self) -> SelectorIter { - self.dep.selector.iter_from(self.dep.selector_offset) - } - - fn hashes(&self) -> &AncestorHashes { - &self.dep.hashes + self.dep.selector() } } @@ -196,10 +184,10 @@ impl InvalidationMap { /// Adds a selector to this `InvalidationMap`. pub fn note_selector( &mut self, - selector_and_hashes: &SelectorAndHashes, + selector: &Selector, quirks_mode: QuirksMode) { - self.collect_invalidations_for(selector_and_hashes, quirks_mode) + self.collect_invalidations_for(selector, quirks_mode) } /// Clears this map, leaving it empty. @@ -214,13 +202,12 @@ impl InvalidationMap { fn collect_invalidations_for( &mut self, - selector_and_hashes: &SelectorAndHashes, + selector: &Selector, quirks_mode: QuirksMode) { - debug!("InvalidationMap::collect_invalidations_for({:?})", - selector_and_hashes.selector); + debug!("InvalidationMap::collect_invalidations_for({:?})", selector); - let mut iter = selector_and_hashes.selector.iter(); + let mut iter = selector.iter(); let mut combinator; let mut index = 0; @@ -249,22 +236,6 @@ impl InvalidationMap { index += 1; // Account for the simple selector. } - // Reuse the bloom hashes if this is the base selector. Otherwise, - // rebuild them. - let mut hashes = None; - - let mut get_hashes = || -> AncestorHashes { - if hashes.is_none() { - hashes = Some(if sequence_start == 0 { - selector_and_hashes.hashes.clone() - } else { - let seq_iter = selector_and_hashes.selector.iter_from(sequence_start); - AncestorHashes::from_iter(seq_iter) - }); - } - hashes.clone().unwrap() - }; - self.has_id_attribute_selectors |= compound_visitor.has_id_attribute_selectors; self.has_class_attribute_selectors |= compound_visitor.has_class_attribute_selectors; @@ -273,9 +244,8 @@ impl InvalidationMap { .entry(class, quirks_mode) .or_insert_with(SelectorMap::new) .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } @@ -284,9 +254,8 @@ impl InvalidationMap { .entry(id, quirks_mode) .or_insert_with(SelectorMap::new) .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } @@ -294,9 +263,8 @@ impl InvalidationMap { self.state_affecting_selectors .insert(StateDependency { dep: Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, state: compound_visitor.state, }, quirks_mode); @@ -305,9 +273,8 @@ impl InvalidationMap { if compound_visitor.other_attributes { self.other_attribute_affecting_selectors .insert(Dependency { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: sequence_start, - hashes: get_hashes(), }, quirks_mode); } diff --git a/components/style/invalidation/element/invalidator.rs b/components/style/invalidation/element/invalidator.rs index ec644f5e564..7c31f4a3c53 100644 --- a/components/style/invalidation/element/invalidator.rs +++ b/components/style/invalidation/element/invalidator.rs @@ -762,14 +762,14 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matched_then = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.wrapper, &mut then_context, &mut |_, _| {}); let matches_now = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.element, &mut now_context, &mut |_, _| {}); @@ -802,7 +802,7 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matched_then = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.wrapper, &mut then_context, &mut |_, _| {}); @@ -811,7 +811,7 @@ impl<'a, 'b: 'a, E> InvalidationCollector<'a, 'b, E> let matches_now = matches_selector(&dependency.selector, dependency.selector_offset, - &dependency.hashes, + None, &self.element, &mut now_context, &mut |_, _| {}); diff --git a/components/style/invalidation/stylesheets.rs b/components/style/invalidation/stylesheets.rs index 9540482786c..19c35405676 100644 --- a/components/style/invalidation/stylesheets.rs +++ b/components/style/invalidation/stylesheets.rs @@ -282,8 +282,8 @@ impl StylesheetInvalidationSet { match *rule { Style(ref lock) => { let style_rule = lock.read_with(guard); - for selector_and_hashes in &style_rule.selectors.0 { - self.collect_scopes(&selector_and_hashes.selector); + for selector in &style_rule.selectors.0 { + self.collect_scopes(selector); if self.fully_invalid { return; } diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index 77052990edc..0a7ebb65612 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -14,7 +14,7 @@ use pdqsort::sort_by; use rule_tree::CascadeLevel; use selector_parser::SelectorImpl; use selectors::matching::{matches_selector, MatchingContext, ElementSelectorFlags}; -use selectors::parser::{AncestorHashes, Component, Combinator, SelectorAndHashes, SelectorIter}; +use selectors::parser::{Component, Combinator, SelectorIter}; use selectors::parser::LocalName as LocalNameSelector; use smallvec::VecLike; use std::collections::HashMap; @@ -26,19 +26,6 @@ use stylist::Rule; pub trait SelectorMapEntry : Sized + Clone { /// Gets the selector we should use to index in the selector map. fn selector(&self) -> SelectorIter; - - /// Gets the ancestor hashes associated with the selector. - fn hashes(&self) -> &AncestorHashes; -} - -impl SelectorMapEntry for SelectorAndHashes { - fn selector(&self) -> SelectorIter { - self.selector.iter() - } - - fn hashes(&self) -> &AncestorHashes { - &self.hashes - } } /// Map element data to selector-providing objects for which the last simple @@ -65,7 +52,7 @@ impl SelectorMapEntry for SelectorAndHashes { /// TODO: Tune the initial capacity of the HashMap #[derive(Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct SelectorMap { +pub struct SelectorMap { /// A hash from an ID to rules which contain that ID selector. pub id_hash: MaybeCaseInsensitiveHashMap>, /// A hash from a class name to rules which contain that class selector. @@ -83,7 +70,7 @@ fn sort_by_key K, K: Ord>(v: &mut [T], f: F) { sort_by(v, |a, b| f(a).cmp(&f(b))) } -impl SelectorMap { +impl SelectorMap { /// Trivially constructs an empty `SelectorMap`. pub fn new() -> Self { SelectorMap { @@ -209,7 +196,7 @@ impl SelectorMap { for rule in rules { if matches_selector(&rule.selector, 0, - &rule.hashes, + Some(&rule.hashes), element, context, flags_setter) { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 6c429a427bf..0b50b6d82f1 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -27,7 +27,7 @@ use selectors::attr::NamespaceConstraint; use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, matches_selector, MatchingContext, MatchingMode}; use selectors::matching::VisitedHandlingMode; -use selectors::parser::{AncestorHashes, Combinator, Component, Selector, SelectorAndHashes}; +use selectors::parser::{AncestorHashes, Combinator, Component, Selector}; use selectors::parser::{SelectorIter, SelectorMethods}; use selectors::sink::Push; use selectors::visitor::SelectorVisitor; @@ -478,10 +478,10 @@ impl Stylist { CssRule::Style(ref locked) => { let style_rule = locked.read_with(&guard); self.num_declarations += style_rule.block.read_with(&guard).len(); - for selector_and_hashes in &style_rule.selectors.0 { + for selector in &style_rule.selectors.0 { self.num_selectors += 1; - let map = if let Some(pseudo) = selector_and_hashes.selector.pseudo_element() { + let map = if let Some(pseudo) = selector.pseudo_element() { self.pseudos_map .entry(pseudo.canonical()) .or_insert_with(PerPseudoElementSelectorMap::new) @@ -490,25 +490,26 @@ impl Stylist { self.element_map.borrow_for_origin(&origin) }; + let hashes = AncestorHashes::new(&selector); map.insert( - Rule::new(selector_and_hashes.selector.clone(), - selector_and_hashes.hashes.clone(), + Rule::new(selector.clone(), + hashes.clone(), locked.clone(), self.rules_source_order), self.quirks_mode); - self.invalidation_map.note_selector(selector_and_hashes, self.quirks_mode); - if needs_revalidation(&selector_and_hashes.selector) { + self.invalidation_map.note_selector(selector, self.quirks_mode); + if needs_revalidation(&selector) { self.selectors_for_cache_revalidation.insert( - RevalidationSelectorAndHashes::new(&selector_and_hashes), + RevalidationSelectorAndHashes::new(&selector, &hashes), self.quirks_mode); } - selector_and_hashes.selector.visit(&mut AttributeAndStateDependencyVisitor { + selector.visit(&mut AttributeAndStateDependencyVisitor { attribute_dependencies: &mut self.attribute_dependencies, style_attribute_dependency: &mut self.style_attribute_dependency, state_dependencies: &mut self.state_dependencies, }); - selector_and_hashes.selector.visit(&mut MappedIdVisitor { + selector.visit(&mut MappedIdVisitor { mapped_ids: &mut self.mapped_ids, }); } @@ -1302,7 +1303,7 @@ impl Stylist { *element, self.quirks_mode, &mut |selector_and_hashes| { results.push(matches_selector(&selector_and_hashes.selector, selector_and_hashes.selector_offset, - &selector_and_hashes.hashes, + Some(&selector_and_hashes.hashes), element, &mut matching_context, flags_setter)); @@ -1431,12 +1432,12 @@ struct RevalidationSelectorAndHashes { } impl RevalidationSelectorAndHashes { - fn new(selector_and_hashes: &SelectorAndHashes) -> Self { + fn new(selector: &Selector, hashes: &AncestorHashes) -> Self { // We basically want to check whether the first combinator is a // pseudo-element combinator. If it is, we want to use the offset one // past it. Otherwise, our offset is 0. let mut index = 0; - let mut iter = selector_and_hashes.selector.iter(); + let mut iter = selector.iter(); // First skip over the first ComplexSelector. We can't check what sort // of combinator we have until we do that. @@ -1450,9 +1451,9 @@ impl RevalidationSelectorAndHashes { }; RevalidationSelectorAndHashes { - selector: selector_and_hashes.selector.clone(), + selector: selector.clone(), selector_offset: offset, - hashes: selector_and_hashes.hashes.clone(), + hashes: hashes.clone(), } } } @@ -1461,10 +1462,6 @@ impl SelectorMapEntry for RevalidationSelectorAndHashes { fn selector(&self) -> SelectorIter { self.selector.iter_from(self.selector_offset) } - - fn hashes(&self) -> &AncestorHashes { - &self.hashes - } } /// Visitor to determine whether a selector requires cache revalidation. @@ -1609,10 +1606,6 @@ impl SelectorMapEntry for Rule { fn selector(&self) -> SelectorIter { self.selector.iter() } - - fn hashes(&self) -> &AncestorHashes { - &self.hashes - } } impl Rule { diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index baee4e4e490..b18b03f25d3 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -1263,7 +1263,7 @@ pub extern "C" fn Servo_StyleRule_GetSelectorTextAtIndex(rule: RawServoStyleRule if index >= rule.selectors.0.len() { return; } - rule.selectors.0[index].selector.to_css(unsafe { result.as_mut().unwrap() }).unwrap(); + rule.selectors.0[index].to_css(unsafe { result.as_mut().unwrap() }).unwrap(); }) } @@ -1287,7 +1287,7 @@ pub extern "C" fn Servo_StyleRule_GetSpecificityAtIndex( *specificity = 0; return; } - *specificity = rule.selectors.0[index].selector.specificity() as u64; + *specificity = rule.selectors.0[index].specificity() as u64; }) } @@ -1302,14 +1302,14 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule return false; } - let selector_and_hashes = &rule.selectors.0[index]; + let selector = &rule.selectors.0[index]; let mut matching_mode = MatchingMode::Normal; match PseudoElement::from_pseudo_type(pseudo_type) { Some(pseudo) => { // We need to make sure that the requested pseudo element type // matches the selector pseudo element type before proceeding. - match selector_and_hashes.selector.pseudo_element() { + match selector.pseudo_element() { Some(selector_pseudo) if *selector_pseudo == pseudo => { matching_mode = MatchingMode::ForStatelessPseudoElement }, @@ -1319,7 +1319,7 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule None => { // Do not attempt to match if a pseudo element is requested and // this is not a pseudo element selector, or vice versa. - if selector_and_hashes.selector.has_pseudo_element() { + if selector.has_pseudo_element() { return false; } }, @@ -1327,8 +1327,7 @@ pub extern "C" fn Servo_StyleRule_SelectorMatchesElement(rule: RawServoStyleRule let element = GeckoElement(element); let mut ctx = MatchingContext::new(matching_mode, None, element.owner_document_quirks_mode()); - matches_selector(&selector_and_hashes.selector, 0, &selector_and_hashes.hashes, - &element, &mut ctx, &mut |_, _| {}) + matches_selector(selector, 0, None, &element, &mut ctx, &mut |_, _| {}) }) } diff --git a/tests/unit/style/size_of.rs b/tests/unit/style/size_of.rs index 60d8c7b4ac0..486fecf4e25 100644 --- a/tests/unit/style/size_of.rs +++ b/tests/unit/style/size_of.rs @@ -2,8 +2,11 @@ * 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/. */ +use style::invalidation::element::invalidation_map::Dependency; use style::properties; +size_of_test!(test_size_of_dependency, Dependency, 16); + size_of_test!(test_size_of_property_declaration, properties::PropertyDeclaration, 32); // This is huge, but we allocate it on the stack and then never move it, diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 0f371967efb..13e9ae24f5c 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -5,8 +5,8 @@ use cssparser::SourceLocation; use euclid::TypedSize2D; use html5ever::LocalName; +use selectors::parser::{AncestorHashes, Selector}; use selectors::parser::LocalName as LocalNameSelector; -use selectors::parser::Selector; use servo_atoms::Atom; use style::context::QuirksMode; use style::media_queries::{Device, MediaType}; @@ -45,7 +45,7 @@ fn get_mock_rules(css_selectors: &[&str]) -> (Vec>, SharedRwLock) { let guard = shared_lock.read(); let rule = locked.read_with(&guard); rule.selectors.0.iter().map(|s| { - Rule::new(s.selector.clone(), s.hashes.clone(), locked.clone(), i as u32) + Rule::new(s.clone(), AncestorHashes::new(s), locked.clone(), i as u32) }).collect() }).collect(), shared_lock) } @@ -68,7 +68,7 @@ fn parse_selectors(selectors: &[&str]) -> Vec> { .map(|x| SelectorParser::parse_author_origin_no_namespace(x).unwrap().0 .into_iter() .nth(0) - .unwrap().selector) + .unwrap()) .collect() }