diff --git a/components/style/matching.rs b/components/style/matching.rs index 0e1f94c9327..f8fbfd493c9 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -26,6 +26,7 @@ use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl}; use selectors::bloom::BloomFilter; use selectors::matching::{ElementSelectorFlags, StyleRelations}; use selectors::matching::AFFECTED_BY_PSEUDO_ELEMENTS; +use shared_lock::StylesheetGuards; use sink::ForgetfulSink; use stylearc::Arc; use stylist::ApplicableDeclarationBlock; @@ -858,11 +859,12 @@ trait PrivateMatchMethods: TElement { } fn compute_rule_node(rule_tree: &RuleTree, - applicable_declarations: &mut Vec) + applicable_declarations: &mut Vec, + guards: &StylesheetGuards) -> StrongRuleNode { let rules = applicable_declarations.drain(..).map(|d| (d.source, d.level)); - let rule_node = rule_tree.insert_ordered_rules(rules); + let rule_node = rule_tree.insert_ordered_rules_with_important(rules, guards); rule_node } @@ -1007,12 +1009,13 @@ pub trait MatchMethods : TElement { smil_override, animation_rules, implemented_pseudo.as_ref(), - &context.shared.guards, &mut applicable_declarations, &mut set_selector_flags); let primary_rule_node = - compute_rule_node::(&stylist.rule_tree, &mut applicable_declarations); + compute_rule_node::(&stylist.rule_tree, + &mut applicable_declarations, + &context.shared.guards); return data.set_primary_rules(primary_rule_node); } @@ -1061,13 +1064,14 @@ pub trait MatchMethods : TElement { None, AnimationRules(None, None), Some(&pseudo), - &guards, &mut applicable_declarations, &mut set_selector_flags); if !applicable_declarations.is_empty() { let new_rules = - compute_rule_node::(rule_tree, &mut applicable_declarations); + compute_rule_node::(rule_tree, + &mut applicable_declarations, + &guards); if pseudos.has(&pseudo) { rule_nodes_changed = pseudos.set_rules(&pseudo, new_rules); } else { diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 904bf0bb1a0..8c5c5166077 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -10,6 +10,7 @@ use heapsize::HeapSizeOf; use properties::{Importance, PropertyDeclarationBlock}; use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard}; +use smallvec::SmallVec; use std::io::{self, Write}; use std::ptr; use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering}; @@ -124,6 +125,96 @@ impl RuleTree { self.dump(guards, &mut stdout); } + /// Inserts the given rules, that must be in proper order by specifity, and + /// returns the corresponding rule node representing the last inserted one. + /// + /// !important rules are detected and inserted into the appropriate position + /// in the rule tree. This allows selector matching to ignore importance, + /// while still maintaining the appropriate cascade order in the rule tree. + pub fn insert_ordered_rules_with_important<'a, I>(&self, + iter: I, + guards: &StylesheetGuards) + -> StrongRuleNode + where I: Iterator, + { + use self::CascadeLevel::*; + let mut current = self.root.clone(); + let mut last_level = current.get().level; + + let mut found_important = false; + let mut important_style_attr = None; + let mut important_author = SmallVec::<[StyleSource; 4]>::new(); + let mut important_user = SmallVec::<[StyleSource; 4]>::new(); + let mut important_ua = SmallVec::<[StyleSource; 4]>::new(); + let mut transition = None; + + for (source, level) in iter { + debug_assert!(last_level <= level, "Not really ordered"); + debug_assert!(!level.is_important(), "Important levels handled internally"); + let (any_normal, any_important) = { + let pdb = source.read(level.guard(guards)); + (pdb.any_normal(), pdb.any_important()) + }; + if any_important { + found_important = true; + match level { + AuthorNormal => important_author.push(source.clone()), + UANormal => important_ua.push(source.clone()), + UserNormal => important_user.push(source.clone()), + StyleAttributeNormal => { + debug_assert!(important_style_attr.is_none()); + important_style_attr = Some(source.clone()); + }, + _ => {}, + }; + } + if any_normal { + if matches!(level, Transitions) && found_important { + // There can be at most one transition, and it will come at + // the end of the iterator. Stash it and apply it after + // !important rules. + debug_assert!(transition.is_none()); + transition = Some(source); + } else { + current = current.ensure_child(self.root.downgrade(), source, level); + } + } + last_level = level; + } + + // Early-return in the common case of no !important declarations. + if !found_important { + return current; + } + + // + // Insert important declarations, in order of increasing importance, + // followed by any transition rule. + // + + for source in important_author.into_iter() { + current = current.ensure_child(self.root.downgrade(), source, AuthorImportant); + } + + if let Some(source) = important_style_attr { + current = current.ensure_child(self.root.downgrade(), source, StyleAttributeImportant); + } + + for source in important_user.into_iter() { + current = current.ensure_child(self.root.downgrade(), source, UserImportant); + } + + for source in important_ua.into_iter() { + current = current.ensure_child(self.root.downgrade(), source, UAImportant); + } + + if let Some(source) = transition { + current = current.ensure_child(self.root.downgrade(), source, Transitions); + } + + current + } + /// Insert the given rules, that must be in proper order by specifity, and /// return the corresponding rule node representing the last inserted one. pub fn insert_ordered_rules<'a, I>(&self, iter: I) -> StrongRuleNode diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 4abc363200c..2bdcfbb31b3 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -327,10 +327,7 @@ impl Stylist { SelectorImpl::each_precomputed_pseudo_element(|pseudo| { if let Some(map) = self.pseudos_map.remove(&pseudo) { - let declarations = - map.user_agent.get_universal_rules( - guards.ua_or_user, CascadeLevel::UANormal, CascadeLevel::UAImportant - ); + let declarations = map.user_agent.get_universal_rules(CascadeLevel::UANormal); self.precomputed_pseudo_element_decls.insert(pseudo, declarations); } }); @@ -376,7 +373,7 @@ impl Stylist { self.num_declarations += style_rule.block.read_with(&guard).len(); for selector in &style_rule.selectors.0 { self.num_selectors += 1; - self.add_rule_to_map(guard, selector, locked, stylesheet); + self.add_rule_to_map(selector, locked, stylesheet); self.dependencies.note_selector(selector); self.note_for_revalidation(selector); } @@ -412,7 +409,6 @@ impl Stylist { #[inline] fn add_rule_to_map(&mut self, - guard: &SharedRwLockReadGuard, selector: &Selector, rule: &Arc>, stylesheet: &Stylesheet) @@ -426,8 +422,7 @@ impl Stylist { self.element_map.borrow_for_origin(&stylesheet.origin) }; - map.insert(Rule::new(guard, - selector.inner.clone(), + map.insert(Rule::new(selector.inner.clone(), rule.clone(), self.rules_source_order, selector.specificity)); @@ -460,8 +455,9 @@ impl Stylist { Some(declarations) => { // FIXME(emilio): When we've taken rid of the cascade we can just // use into_iter. - self.rule_tree.insert_ordered_rules( - declarations.into_iter().map(|a| (a.source.clone(), a.level))) + self.rule_tree.insert_ordered_rules_with_important( + declarations.into_iter().map(|a| (a.source.clone(), a.level)), + guards) } None => self.rule_tree.root(), }; @@ -622,17 +618,21 @@ impl Stylist { None, AnimationRules(None, None), Some(pseudo), - guards, &mut declarations, &mut set_selector_flags); if declarations.is_empty() { return None } - let rule_node = self.rule_tree.insert_ordered_rules(declarations.into_iter().map(|a| { - (a.source, a.level) - })); - Some(rule_node) + let rule_node = + self.rule_tree.insert_ordered_rules_with_important( + declarations.into_iter().map(|a| (a.source, a.level)), + guards); + if rule_node == self.rule_tree.root() { + None + } else { + Some(rule_node) + } } /// Set a given device, which may change the styles that apply to the @@ -740,7 +740,6 @@ impl Stylist { smil_override: Option<&Arc>>, animation_rules: AnimationRules, pseudo_element: Option<&PseudoElement>, - guards: &StylesheetGuards, applicable_declarations: &mut V, flags_setter: &mut F) -> StyleRelations @@ -810,13 +809,11 @@ impl Stylist { // Step 4: Normal style attributes. if let Some(sa) = style_attribute { - if sa.read_with(guards.author).any_normal() { - relations |= AFFECTED_BY_STYLE_ATTRIBUTE; - Push::push( - applicable_declarations, - ApplicableDeclarationBlock::from_declarations(sa.clone(), - CascadeLevel::StyleAttributeNormal)); - } + relations |= AFFECTED_BY_STYLE_ATTRIBUTE; + Push::push( + applicable_declarations, + ApplicableDeclarationBlock::from_declarations(sa.clone(), + CascadeLevel::StyleAttributeNormal)); } debug!("style attr: {:?}", relations); @@ -841,52 +838,14 @@ impl Stylist { CascadeLevel::Animations)); } debug!("animation: {:?}", relations); - - // Step 7: Author-supplied `!important` rules. - map.author.get_all_matching_rules(element, - parent_bf, - applicable_declarations, - &mut relations, - flags_setter, - CascadeLevel::AuthorImportant); - - debug!("author important: {:?}", relations); - - // Step 8: `!important` style attributes. - if let Some(sa) = style_attribute { - if sa.read_with(guards.author).any_important() { - relations |= AFFECTED_BY_STYLE_ATTRIBUTE; - Push::push( - applicable_declarations, - ApplicableDeclarationBlock::from_declarations(sa.clone(), - CascadeLevel::StyleAttributeImportant)); - } - } - - debug!("style attr important: {:?}", relations); - - // Step 9: User `!important` rules. - map.user.get_all_matching_rules(element, - parent_bf, - applicable_declarations, - &mut relations, - flags_setter, - CascadeLevel::UserImportant); - - debug!("user important: {:?}", relations); } else { debug!("skipping non-agent rules"); } - // Step 10: UA `!important` rules. - map.user_agent.get_all_matching_rules(element, - parent_bf, - applicable_declarations, - &mut relations, - flags_setter, - CascadeLevel::UAImportant); - - debug!("UA important: {:?}", relations); + // + // Steps 7-10 correspond to !important rules, and are handled during + // rule tree insertion. + // // Step 11: Transitions. // The transitions sheet (CSS transitions that are tied to CSS markup) @@ -1256,45 +1215,24 @@ impl SelectorMap { /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in /// `self` sorted by specificity and source order. pub fn get_universal_rules(&self, - guard: &SharedRwLockReadGuard, - cascade_level: CascadeLevel, - important_cascade_level: CascadeLevel) + cascade_level: CascadeLevel) -> Vec { debug_assert!(!cascade_level.is_important()); - debug_assert!(important_cascade_level.is_important()); if self.is_empty() { return vec![]; } - let mut matching_rules_list = vec![]; - - // 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![]; + let mut rules_list = vec![]; 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); - if block.any_normal() { - matching_rules_list.push( - rule.to_applicable_declaration_block(cascade_level)); - } - if block.any_important() { - important.push( - rule.to_applicable_declaration_block(important_cascade_level)); - } + rules_list.push(rule.to_applicable_declaration_block(cascade_level)); } } - let normal_len = matching_rules_list.len(); - matching_rules_list.extend(important.into_iter()); - - sort_by_key(&mut matching_rules_list[0..normal_len], - |block| (block.specificity, block.source_order)); - sort_by_key(&mut matching_rules_list[normal_len..], + sort_by_key(&mut rules_list, |block| (block.specificity, block.source_order)); - matching_rules_list + rules_list } fn get_matching_rules_from_hash( @@ -1336,13 +1274,7 @@ impl SelectorMap { F: FnMut(&E, ElementSelectorFlags), { for rule in rules.iter() { - let any_declaration_for_importance = if cascade_level.is_important() { - rule.any_important_declarations() - } else { - rule.any_normal_declarations() - }; - if any_declaration_for_importance && - matches_selector(&rule.selector, element, parent_bf, + if matches_selector(&rule.selector, element, parent_bf, relations, flags_setter) { matching_rules.push( rule.to_applicable_declaration_block(cascade_level)); @@ -1556,10 +1488,11 @@ pub struct Rule { pub style_rule: Arc>, /// The source order this style rule appears in. pub source_order: usize, - /// Bottom 30 bits: The specificity of the rule this selector represents. - /// 31st bit: Whether the rule's declaration block has any important declarations. - /// 32nd bit: Whether the rule's declaration block has any normal declarations. - specificity_and_bits: u32, + /// The specificity of the rule this selector represents. + /// + /// Note: The top two bits of this are unused, and could be used to store + /// flags. + specificity: u32, } impl Borrow> for Rule { @@ -1568,23 +1501,10 @@ impl Borrow> for Rule { } } -/// Masks for specificity_and_bits. -const SPECIFICITY_MASK: u32 = 0x3fffffff; -const ANY_IMPORTANT_DECLARATIONS_BIT: u32 = 1 << 30; -const ANY_NORMAL_DECLARATIONS_BIT: u32 = 1 << 31; - impl Rule { /// Returns the specificity of the rule. pub fn specificity(&self) -> u32 { - self.specificity_and_bits & SPECIFICITY_MASK - } - - fn any_important_declarations(&self) -> bool { - (self.specificity_and_bits & ANY_IMPORTANT_DECLARATIONS_BIT) != 0 - } - - fn any_normal_declarations(&self) -> bool { - (self.specificity_and_bits & ANY_NORMAL_DECLARATIONS_BIT) != 0 + self.specificity } fn to_applicable_declaration_block(&self, level: CascadeLevel) -> ApplicableDeclarationBlock { @@ -1597,31 +1517,17 @@ impl Rule { } /// Creates a new Rule. - pub fn new(guard: &SharedRwLockReadGuard, - selector: SelectorInner, + pub fn new(selector: SelectorInner, style_rule: Arc>, source_order: usize, specificity: u32) -> Self { - let (any_important, any_normal) = { - let block = style_rule.read_with(guard).block.read_with(guard); - (block.any_important(), block.any_normal()) - }; - debug_assert!(specificity & (ANY_IMPORTANT_DECLARATIONS_BIT | ANY_NORMAL_DECLARATIONS_BIT) == 0); - let mut specificity_and_bits = specificity; - if any_important { - specificity_and_bits |= ANY_IMPORTANT_DECLARATIONS_BIT; - } - if any_normal { - specificity_and_bits |= ANY_NORMAL_DECLARATIONS_BIT; - } - Rule { selector: selector, style_rule: style_rule, source_order: source_order, - specificity_and_bits: specificity_and_bits, + specificity: specificity, } } } diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs index 965805cd5c9..baba229865a 100644 --- a/tests/unit/style/stylist.rs +++ b/tests/unit/style/stylist.rs @@ -37,8 +37,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(&guard, - s.inner.clone(), + Rule::new(s.inner.clone(), locked.clone(), i, s.specificity) @@ -213,9 +212,7 @@ fn test_get_universal_rules() { thread_state::initialize(thread_state::LAYOUT); let (map, shared_lock) = get_mock_map(&["*|*", "#foo > *|*", ".klass", "#id"]); - let guard = shared_lock.read(); - let decls = map.get_universal_rules( - &guard, CascadeLevel::UserNormal, CascadeLevel::UserImportant); + let decls = map.get_universal_rules(CascadeLevel::UserNormal); assert_eq!(decls.len(), 1); }