Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-07 14:35:33 +01:00
Code Issues Projects Releases Packages Wiki Activity

Prepare for interior mutability in PropertyDeclarationBlock

Browse source 
`Stylist` contains separate hashmaps for important and normal declarations,
but typically a given block only contains declarations of one importance.

Before this commit, there is an optimization where
a `PropertyDeclarationBlock` is only inserted in the corresponding map
if it has a non-zero number of declaration of a given importance.

With CSSOM, `PropertyDeclarationBlock` is gonna have interior mutability:
the importance (priority) of a declaration could change.
This optimization would become incorrect when the block is missing
in a hashmap where it should be.

This commits removes the original optimization, and replaces it with
a slightly weaker one: if a block doesn’t have any declaration
with the importance we’re cascading for, skip selector matching.
This commit is contained in:
Simon Sapin 2016-08-31 01:26:23 +02:00
parent ad3437b98a
commit 7ef4930472
1 changed files with 58 additions and 41 deletions
Download patch file Download diff file Expand all files Collapse all files

99
components/style/selector_matching.rs
View file

@ -165,28 +165,26 @@ impl Stylist {
// Take apart the StyleRule into individual Rules and insert // Take apart the StyleRule into individual Rules and insert
// them into the SelectorMap of that priority. // them into the SelectorMap of that priority.
macro_rules! append( macro_rules! append(
($style_rule: ident, $priority: ident, $importance: expr, $count: expr) => { ($style_rule: ident, $priority: ident, $importance: expr) => {
if $count > 0 { for selector in &$style_rule.selectors {
for selector in &$style_rule.selectors { let map = if let Some(ref pseudo) = selector.pseudo_element {
let map = if let Some(ref pseudo) = selector.pseudo_element { self.pseudos_map
self.pseudos_map .entry(pseudo.clone())
.entry(pseudo.clone()) .or_insert_with(PerPseudoElementSelectorMap::new)
.or_insert_with(PerPseudoElementSelectorMap::new) .borrow_for_origin(&stylesheet.origin)
.borrow_for_origin(&stylesheet.origin) } else {
} else { self.element_map.borrow_for_origin(&stylesheet.origin)
self.element_map.borrow_for_origin(&stylesheet.origin) };
};
map.$priority.insert(Rule { map.$priority.insert(Rule {
selector: selector.complex_selector.clone(), selector: selector.complex_selector.clone(),
declarations: DeclarationBlock { declarations: DeclarationBlock {
specificity: selector.specificity, specificity: selector.specificity,
mixed_declarations: $style_rule.declarations.clone(), mixed_declarations: $style_rule.declarations.clone(),
importance: $importance, importance: $importance,
source_order: rules_source_order, source_order: rules_source_order,
}, },
}); });
}
} }
}; };
); );
@ -194,10 +192,8 @@ impl Stylist {
for rule in stylesheet.effective_rules(&self.device) { for rule in stylesheet.effective_rules(&self.device) {
match *rule { match *rule {
CSSRule::Style(ref style_rule) => { CSSRule::Style(ref style_rule) => {
let important_count = style_rule.declarations.important_count; append!(style_rule, normal, Importance::Normal);
let normal_count = style_rule.declarations.declarations.len() as u32 - important_count; append!(style_rule, important, Importance::Important);
append!(style_rule, normal, Importance::Normal, normal_count);
append!(style_rule, important, Importance::Important, important_count);
rules_source_order += 1; rules_source_order += 1;
for selector in &style_rule.selectors { for selector in &style_rule.selectors {
@ -373,7 +369,8 @@ impl Stylist {
map.user_agent.normal.get_all_matching_rules(element, map.user_agent.normal.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Normal);
debug!("UA normal: {:?}", relations); debug!("UA normal: {:?}", relations);
// Step 2: Presentational hints. // Step 2: Presentational hints.
@ -389,17 +386,19 @@ impl Stylist {
map.user.normal.get_all_matching_rules(element, map.user.normal.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Normal);
debug!("user normal: {:?}", relations); debug!("user normal: {:?}", relations);
map.author.normal.get_all_matching_rules(element, map.author.normal.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Normal);
debug!("author normal: {:?}", relations); debug!("author normal: {:?}", relations);
// Step 4: Normal style attributes. // Step 4: Normal style attributes.
if let Some(sa) = style_attribute { if let Some(sa) = style_attribute {
if sa.declarations.len() as u32 - sa.important_count > 0 { if sa.declarations.len() > sa.important_count as usize {
relations |= AFFECTED_BY_STYLE_ATTRIBUTE; relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
Push::push( Push::push(
applicable_declarations, applicable_declarations,
@ -413,7 +412,8 @@ impl Stylist {
map.author.important.get_all_matching_rules(element, map.author.important.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Important);
debug!("author important: {:?}", relations); debug!("author important: {:?}", relations);
@ -433,14 +433,16 @@ impl Stylist {
map.user.important.get_all_matching_rules(element, map.user.important.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Important);
debug!("user important: {:?}", relations); debug!("user important: {:?}", relations);
map.user_agent.important.get_all_matching_rules(element, map.user_agent.important.get_all_matching_rules(element,
parent_bf, parent_bf,
applicable_declarations, applicable_declarations,
&mut relations); &mut relations,
Importance::Important);
debug!("UA important: {:?}", relations); debug!("UA important: {:?}", relations);
@ -647,7 +649,8 @@ impl SelectorMap {
element: &E, element: &E,
parent_bf: Option<&BloomFilter>, parent_bf: Option<&BloomFilter>,
matching_rules_list: &mut V, matching_rules_list: &mut V,
relations: &mut StyleRelations) relations: &mut StyleRelations,
importance: Importance)
where E: Element<Impl=TheSelectorImpl>, where E: Element<Impl=TheSelectorImpl>,
V: VecLike<DeclarationBlock> V: VecLike<DeclarationBlock>
{ {
@ -663,7 +666,8 @@ impl SelectorMap {
&self.id_hash, &self.id_hash,
&id, &id,
matching_rules_list, matching_rules_list,
relations) relations,
importance)
} }
element.each_class(|class| { element.each_class(|class| {
@ -672,7 +676,8 @@ impl SelectorMap {
&self.class_hash, &self.class_hash,
class, class,
matching_rules_list, matching_rules_list,
relations); relations,
importance);
}); });
let local_name_hash = if element.is_html_element_in_html_document() { let local_name_hash = if element.is_html_element_in_html_document() {
@ -685,13 +690,15 @@ impl SelectorMap {
local_name_hash, local_name_hash,
element.get_local_name(), element.get_local_name(),
matching_rules_list, matching_rules_list,
relations); relations,
importance);
SelectorMap::get_matching_rules(element, SelectorMap::get_matching_rules(element,
parent_bf, parent_bf,
&self.other_rules, &self.other_rules,
matching_rules_list, matching_rules_list,
relations); relations,
importance);
// Sort only the rules we just added. // Sort only the rules we just added.
sort_by_key(&mut matching_rules_list[init_len..], sort_by_key(&mut matching_rules_list[init_len..],
@ -727,7 +734,8 @@ impl SelectorMap {
hash: &FnvHashMap<Str, Vec<Rule>>, hash: &FnvHashMap<Str, Vec<Rule>>,
key: &BorrowedStr, key: &BorrowedStr,
matching_rules: &mut Vector, matching_rules: &mut Vector,
relations: &mut StyleRelations) relations: &mut StyleRelations,
importance: Importance)
where E: Element<Impl=TheSelectorImpl>, where E: Element<Impl=TheSelectorImpl>,
Str: Borrow<BorrowedStr> + Eq + Hash, Str: Borrow<BorrowedStr> + Eq + Hash,
BorrowedStr: Eq + Hash, BorrowedStr: Eq + Hash,
@ -738,7 +746,8 @@ impl SelectorMap {
parent_bf, parent_bf,
rules, rules,
matching_rules, matching_rules,
relations) relations,
importance)
} }
} }
@ -747,12 +756,20 @@ impl SelectorMap {
parent_bf: Option<&BloomFilter>, parent_bf: Option<&BloomFilter>,
rules: &[Rule], rules: &[Rule],
matching_rules: &mut V, matching_rules: &mut V,
relations: &mut StyleRelations) relations: &mut StyleRelations,
importance: Importance)
where E: Element<Impl=TheSelectorImpl>, where E: Element<Impl=TheSelectorImpl>,
V: VecLike<DeclarationBlock> V: VecLike<DeclarationBlock>
{ {
for rule in rules.iter() { for rule in rules.iter() {
if matches_complex_selector(&*rule.selector, let block = &rule.declarations.mixed_declarations;
let any_declaration_for_importance = if importance.important() {
block.important_count > 0
} else {
block.declarations.len() > block.important_count as usize
};
if any_declaration_for_importance &&
matches_complex_selector(&*rule.selector,
element, parent_bf, relations) { element, parent_bf, relations) {
matching_rules.push(rule.declarations.clone()); matching_rules.push(rule.declarations.clone());
} }