diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5dc3e807374..91ea78a9abf 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1005,11 +1005,11 @@ pub fn needs_revalidation(selector: &Selector) -> bool { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] struct PerPseudoElementSelectorMap { /// Rules from user agent stylesheets - user_agent: SelectorMap, + user_agent: SelectorMap, /// Rules from author stylesheets - author: SelectorMap, + author: SelectorMap, /// Rules from user stylesheets - user: SelectorMap, + user: SelectorMap, } impl PerPseudoElementSelectorMap { @@ -1023,7 +1023,7 @@ impl PerPseudoElementSelectorMap { } #[inline] - fn borrow_for_origin(&mut self, origin: &Origin) -> &mut SelectorMap { + fn borrow_for_origin(&mut self, origin: &Origin) -> &mut SelectorMap { match *origin { Origin::UserAgent => &mut self.user_agent, Origin::Author => &mut self.author, @@ -1032,39 +1032,41 @@ impl PerPseudoElementSelectorMap { } } -/// Map element data to Rules whose last simple selector starts with them. +/// Map element data to selector-providing objects for which the last simple +/// selector starts with them. /// /// e.g., -/// "p > img" would go into the set of Rules corresponding to the +/// "p > img" would go into the set of selectors corresponding to the /// element "img" -/// "a .foo .bar.baz" would go into the set of Rules corresponding to +/// "a .foo .bar.baz" would go into the set of selectors corresponding to /// the class "bar" /// -/// Because we match Rules right-to-left (i.e., moving up the tree +/// Because we match selectors right-to-left (i.e., moving up the tree /// from an element), we need to compare the last simple selector in the -/// Rule with the element. +/// selector with the element. /// /// So, if an element has ID "id1" and classes "foo" and "bar", then all /// the rules it matches will have their last simple selector starting /// either with "#id1" or with ".foo" or with ".bar". /// /// Hence, the union of the rules keyed on each of element's classes, ID, -/// element name, etc. will contain the Rules that actually match that +/// element name, etc. will contain the Selectors that actually match that /// element. /// /// 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: FnvHashMap>, + pub id_hash: FnvHashMap>, /// A hash from a class name to rules which contain that class selector. - pub class_hash: FnvHashMap>, + pub class_hash: FnvHashMap>, /// A hash from local name to rules which contain that local name selector. - pub local_name_hash: FnvHashMap>, + pub local_name_hash: FnvHashMap>, /// Rules that don't have ID, class, or element selectors. - pub other_rules: Vec, - /// Whether this hash is empty. - pub empty: bool, + pub other: Vec, + /// The number of entries in this map. + pub count: usize, } #[inline] @@ -1072,18 +1074,30 @@ 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 where T: Clone + Borrow> { /// Trivially constructs an empty `SelectorMap`. pub fn new() -> Self { SelectorMap { id_hash: HashMap::default(), class_hash: HashMap::default(), local_name_hash: HashMap::default(), - other_rules: Vec::new(), - empty: true, + other: Vec::new(), + count: 0, } } + /// Returns whether there are any entries in the map. + pub fn is_empty(&self) -> bool { + self.count == 0 + } + + /// Returns the number of entries. + pub fn len(&self) -> usize { + self.count + } +} + +impl SelectorMap { /// Append to `rule_list` all Rules in `self` that match element. /// /// Extract matching rules as per element's ID, classes, tag name, etc.. @@ -1099,7 +1113,7 @@ impl SelectorMap { V: VecLike, F: FnMut(&E, ElementSelectorFlags), { - if self.empty { + if self.is_empty() { return } @@ -1138,7 +1152,7 @@ impl SelectorMap { SelectorMap::get_matching_rules(element, parent_bf, - &self.other_rules, + &self.other, matching_rules_list, relations, flags_setter, @@ -1158,7 +1172,7 @@ impl SelectorMap { -> Vec { debug_assert!(!cascade_level.is_important()); debug_assert!(important_cascade_level.is_important()); - if self.empty { + if self.is_empty() { return vec![]; } @@ -1167,7 +1181,7 @@ impl SelectorMap { // We need to insert important rules _after_ normal rules for this to be // correct, and also to not trigger rule tree assertions. let mut important = vec![]; - for rule in self.other_rules.iter() { + for rule in self.other.iter() { if rule.selector.complex.iter_raw().next().is_none() { let style_rule = rule.style_rule.read_with(guard); let block = style_rule.block.read_with(guard); @@ -1245,23 +1259,24 @@ impl SelectorMap { } } } +} - /// Insert rule into the correct hash. - /// Order in which to try: id_hash, class_hash, local_name_hash, other_rules. - pub fn insert(&mut self, rule: Rule) { - self.empty = false; +impl SelectorMap where T: Clone + Borrow> { + /// Inserts into the correct hash, trying id, class, and localname. + pub fn insert(&mut self, entry: T) { + self.count += 1; - if let Some(id_name) = SelectorMap::get_id_name(&rule) { - find_push(&mut self.id_hash, id_name, rule); + if let Some(id_name) = get_id_name(entry.borrow()) { + find_push(&mut self.id_hash, id_name, entry); return; } - if let Some(class_name) = SelectorMap::get_class_name(&rule) { - find_push(&mut self.class_hash, class_name, rule); + if let Some(class_name) = get_class_name(entry.borrow()) { + find_push(&mut self.class_hash, class_name, entry); return; } - if let Some(LocalNameSelector { name, lower_name }) = SelectorMap::get_local_name(&rule) { + if let Some(LocalNameSelector { name, lower_name }) = get_local_name(entry.borrow()) { // If the local name in the selector isn't lowercase, insert it into // the rule hash twice. This means that, during lookup, we can always // find the rules based on the local name of the element, regardless @@ -1274,55 +1289,165 @@ impl SelectorMap { // lookup may produce superfluous selectors, but the subsequent // selector matching work will filter them out. if name != lower_name { - find_push(&mut self.local_name_hash, lower_name, rule.clone()); + find_push(&mut self.local_name_hash, lower_name, entry.clone()); } - find_push(&mut self.local_name_hash, name, rule); + find_push(&mut self.local_name_hash, name, entry); return; } - self.other_rules.push(rule); + self.other.push(entry); } - /// Retrieve the first ID name in Rule, or None otherwise. - pub fn get_id_name(rule: &Rule) -> Option { - for ss in rule.selector.complex.iter() { - // TODO(pradeep): Implement case-sensitivity based on the - // document type and quirks mode. - if let Component::ID(ref id) = *ss { - return Some(id.clone()); + /// Looks up entries by id, class, local name, and other (in order). + /// + /// Each entry is passed to the callback, which returns true to continue + /// iterating entries, or false to terminate the lookup. + /// + /// Returns false if the callback ever returns false. + /// + /// FIXME(bholley) This overlaps with SelectorMap::get_all_matching_rules, + /// but that function is extremely hot and I'd rather not rearrange it. + #[inline] + pub fn lookup(&self, element: E, f: &mut F) -> bool + where E: TElement, + F: FnMut(&T) -> bool + { + // Id. + if let Some(id) = element.get_id() { + if let Some(v) = self.id_hash.get(&id) { + for entry in v.iter() { + if !f(&entry) { + return false; + } + } } } - None - } + // Class. + let mut done = false; + element.each_class(|class| { + if !done { + if let Some(v) = self.class_hash.get(class) { + for entry in v.iter() { + if !f(&entry) { + done = true; + return; + } + } + } + } + }); + if done { + return false; + } - /// Retrieve the FIRST class name in Rule, or None otherwise. - pub fn get_class_name(rule: &Rule) -> Option { - for ss in rule.selector.complex.iter() { - // TODO(pradeep): Implement case-sensitivity based on the - // document type and quirks mode. - if let Component::Class(ref class) = *ss { - return Some(class.clone()); + // Local name. + if let Some(v) = self.local_name_hash.get(element.get_local_name()) { + for entry in v.iter() { + if !f(&entry) { + return false; + } } } - None - } - - /// 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.complex.iter() { - if let Component::LocalName(ref n) = *ss { - return Some(LocalNameSelector { - name: n.name.clone(), - lower_name: n.lower_name.clone(), - }) + // Other. + for entry in self.other.iter() { + if !f(&entry) { + return false; } } - None + true } + + /// Performs a normal lookup, and also looks up entries for the passed-in + /// id and classes. + /// + /// Each entry is passed to the callback, which returns true to continue + /// iterating entries, or false to terminate the lookup. + /// + /// Returns false if the callback ever returns false. + #[inline] + pub fn lookup_with_additional(&self, + element: E, + additional_id: Option, + additional_classes: &[Atom], + f: &mut F) + -> bool + where E: TElement, + F: FnMut(&T) -> bool + { + // Do the normal lookup. + if !self.lookup(element, f) { + return false; + } + + // Check the additional id. + if let Some(id) = additional_id { + if let Some(v) = self.id_hash.get(&id) { + for entry in v.iter() { + if !f(&entry) { + return false; + } + } + } + } + + // Check the additional classes. + for class in additional_classes { + if let Some(v) = self.class_hash.get(class) { + for entry in v.iter() { + if !f(&entry) { + return false; + } + } + } + } + + true + } +} + +/// Retrieve the first ID name in the selector, or None otherwise. +pub fn get_id_name(selector: &SelectorInner) -> Option { + for ss in selector.complex.iter() { + // TODO(pradeep): Implement case-sensitivity based on the + // document type and quirks mode. + if let Component::ID(ref id) = *ss { + return Some(id.clone()); + } + } + + None +} + +/// Retrieve the FIRST class name in the selector, or None otherwise. +pub fn get_class_name(selector: &SelectorInner) -> Option { + for ss in selector.complex.iter() { + // TODO(pradeep): Implement case-sensitivity based on the + // document type and quirks mode. + if let Component::Class(ref class) = *ss { + return Some(class.clone()); + } + } + + None +} + +/// Retrieve the name if it is a type selector, or None otherwise. +pub fn get_local_name(selector: &SelectorInner) + -> Option> { + for ss in selector.complex.iter() { + if let Component::LocalName(ref n) = *ss { + return Some(LocalNameSelector { + name: n.name.clone(), + lower_name: n.lower_name.clone(), + }) + } + } + + None } /// A rule, that wraps a style rule, but represents a single selector of the @@ -1347,6 +1472,12 @@ pub struct Rule { specificity_and_bits: u32, } +impl Borrow> for Rule { + fn borrow(&self) -> &SelectorInner { + &self.selector + } +} + /// Masks for specificity_and_bits. const SPECIFICITY_MASK: u32 = 0x3fffffff; const ANY_IMPORTANT_DECLARATIONS_BIT: u32 = 1 << 30; @@ -1441,7 +1572,8 @@ impl ApplicableDeclarationBlock { } #[inline] -fn find_push(map: &mut FnvHashMap>, key: Str, - value: Rule) { +fn find_push(map: &mut FnvHashMap>, + key: Str, + value: V) { map.entry(key).or_insert_with(Vec::new).push(value) } diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 7d8c6741925..965805cd5c9 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -13,6 +13,7 @@ use style::selector_parser::{SelectorImpl, SelectorParser}; use style::shared_lock::SharedRwLock; use style::stylearc::Arc; use style::stylesheets::StyleRule; +use style::stylist; use style::stylist::{Rule, SelectorMap}; use style::stylist::needs_revalidation; use style::thread_state; @@ -45,8 +46,8 @@ fn get_mock_rules(css_selectors: &[&str]) -> (Vec>, SharedRwLock) { }).collect(), shared_lock) } -fn get_mock_map(selectors: &[&str]) -> (SelectorMap, SharedRwLock) { - let mut map = SelectorMap::new(); +fn get_mock_map(selectors: &[&str]) -> (SelectorMap, SharedRwLock) { + let mut map = SelectorMap::::new(); let (selector_rules, shared_lock) = get_mock_rules(selectors); for rules in selector_rules.into_iter() { @@ -170,22 +171,22 @@ fn test_rule_ordering_same_specificity() { #[test] fn test_get_id_name() { let (rules_list, _) = get_mock_rules(&[".intro", "#top"]); - assert_eq!(SelectorMap::get_id_name(&rules_list[0][0]), None); - assert_eq!(SelectorMap::get_id_name(&rules_list[1][0]), Some(Atom::from("top"))); + assert_eq!(stylist::get_id_name(&rules_list[0][0].selector), None); + assert_eq!(stylist::get_id_name(&rules_list[1][0].selector), Some(Atom::from("top"))); } #[test] fn test_get_class_name() { let (rules_list, _) = get_mock_rules(&[".intro.foo", "#top"]); - assert_eq!(SelectorMap::get_class_name(&rules_list[0][0]), Some(Atom::from("foo"))); - assert_eq!(SelectorMap::get_class_name(&rules_list[1][0]), None); + assert_eq!(stylist::get_class_name(&rules_list[0][0].selector), Some(Atom::from("foo"))); + assert_eq!(stylist::get_class_name(&rules_list[1][0].selector), None); } #[test] fn test_get_local_name() { let (rules_list, _) = get_mock_rules(&["img.foo", "#top", "IMG", "ImG"]); let check = |i: usize, names: Option<(&str, &str)>| { - assert!(SelectorMap::get_local_name(&rules_list[i][0]) + assert!(stylist::get_local_name(&rules_list[i][0].selector) == names.map(|(name, lower_name)| LocalNameSelector { name: LocalName::from(name), lower_name: LocalName::from(lower_name) }))