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

Keep track of the number of important and normal declarations in a block

Browse source 
… instead of the presence (`bool` flags) of each.

This allows removing `recalc_any` which iterated over the `Vec`.
This commit is contained in:
Simon Sapin 2016-08-21 03:20:29 +02:00
parent 5e4bdac2bd
commit f9150af936
5 changed files with 107 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

61
components/script/dom/element.rs
View file

@ -762,12 +762,21 @@ impl Element {
fn remove(element: &Element, property: &str) { fn remove(element: &Element, property: &str) {
let mut inline_declarations = element.style_attribute.borrow_mut(); let mut inline_declarations = element.style_attribute.borrow_mut();
if let &mut Some(ref mut declarations) = &mut *inline_declarations { if let &mut Some(ref mut declarations) = &mut *inline_declarations {
let index = declarations.declarations let mut importance = None;
.iter() let index = declarations.declarations.iter().position(|&(ref decl, i)| {
.position(|&(ref decl, _)| decl.matches(property)); let matching = decl.matches(property);
if matching {
importance = Some(i)
}
matching
});
if let Some(index) = index { if let Some(index) = index {
Arc::make_mut(&mut declarations.declarations).remove(index); Arc::make_mut(&mut declarations.declarations).remove(index);
declarations.recalc_any(); if importance.unwrap().important() {
declarations.important_count -= 1;
} else {
declarations.normal_count -= 1;
}
} }
} }
} }
@ -791,21 +800,42 @@ impl Element {
'outer: for incoming_declaration in declarations { 'outer: for incoming_declaration in declarations {
for existing_declaration in &mut *existing_declarations { for existing_declaration in &mut *existing_declarations {
if existing_declaration.0.name() == incoming_declaration.name() { if existing_declaration.0.name() == incoming_declaration.name() {
match (existing_declaration.1, importance) {
(Importance::Normal, Importance::Important) => {
declaration_block.normal_count -= 1;
declaration_block.important_count += 1;
}
(Importance::Important, Importance::Normal) => {
declaration_block.normal_count += 1;
declaration_block.important_count -= 1;
}
_ => {}
}
*existing_declaration = (incoming_declaration, importance); *existing_declaration = (incoming_declaration, importance);
continue 'outer; continue 'outer;
} }
} }
existing_declarations.push((incoming_declaration, importance)); existing_declarations.push((incoming_declaration, importance));
if importance.important() {
declaration_block.important_count += 1;
} else {
declaration_block.normal_count += 1;
}
} }
} }
declaration_block.recalc_any();
return; return;
} }
let (normal_count, important_count) = if importance.important() {
(0, declarations.len() as u32)
} else {
(declarations.len() as u32, 0)
};
*inline_declarations = Some(PropertyDeclarationBlock { *inline_declarations = Some(PropertyDeclarationBlock {
declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()), declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()),
any_important: importance.important(), normal_count: normal_count,
any_normal: !importance.important(), important_count: important_count,
}); });
} }
@ -818,13 +848,24 @@ impl Element {
new_importance: Importance) { new_importance: Importance) {
{ {
let mut inline_declarations = self.style_attribute().borrow_mut(); let mut inline_declarations = self.style_attribute().borrow_mut();
if let &mut Some(ref mut declarations) = &mut *inline_declarations { if let &mut Some(ref mut block) = &mut *inline_declarations {
// Usually, the reference counts of `from` and `to` will be 1 here. But transitions // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
// could make them greater than that. // could make them greater than that.
let declarations = Arc::make_mut(&mut declarations.declarations); let declarations = Arc::make_mut(&mut block.declarations);
for &mut (ref declaration, ref mut importance) in declarations { for &mut (ref declaration, ref mut importance) in declarations {
if properties.iter().any(|p| declaration.name() == **p) { if properties.iter().any(|p| declaration.name() == **p) {
*importance = new_importance match (*importance, new_importance) {
(Importance::Normal, Importance::Important) => {
block.normal_count -= 1;
block.important_count += 1;
}
(Importance::Important, Importance::Normal) => {
block.normal_count += 1;
block.important_count -= 1;
}
_ => {}
}
*importance = new_importance;
} }
} }
} }

64
components/style/properties/properties.mako.rs
View file

@ -288,27 +288,11 @@ pub struct PropertyDeclarationBlock {
#[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")]
pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>, pub declarations: Arc<Vec<(PropertyDeclaration, Importance)>>,
/// Whether `self.declaration` contains at least one declaration with `Importance::Normal` /// The number of entries in `self.declaration` with `Importance::Normal`
pub any_normal: bool, pub normal_count: u32,
/// Whether `self.declaration` contains at least one declaration with `Importance::Important` /// The number of entries in `self.declaration` with `Importance::Important`
pub any_important: bool, pub important_count: u32,
}
impl PropertyDeclarationBlock {
pub fn recalc_any(&mut self) {
let mut any_normal = false;
let mut any_important = false;
for &(_, importance) in &*self.declarations {
if importance.important() {
any_important = true
} else {
any_normal = true
}
}
self.any_normal = any_normal;
self.any_important = any_important;
}
} }
impl ToCss for PropertyDeclarationBlock { impl ToCss for PropertyDeclarationBlock {
@ -564,8 +548,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> {
pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser)
-> PropertyDeclarationBlock { -> PropertyDeclarationBlock {
let mut declarations = Vec::new(); let mut declarations = Vec::new();
let mut any_normal = false; let mut normal_count = 0;
let mut any_important = false; let mut important_count = 0;
let parser = PropertyDeclarationParser { let parser = PropertyDeclarationParser {
context: context, context: context,
}; };
@ -574,9 +558,9 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
match declaration { match declaration {
Ok((results, importance)) => { Ok((results, importance)) => {
if importance.important() { if importance.important() {
any_important = true important_count += results.len() as u32;
} else { } else {
any_normal = true normal_count += results.len() as u32;
} }
declarations.extend(results.into_iter().map(|d| (d, importance))) declarations.extend(results.into_iter().map(|d| (d, importance)))
} }
@ -588,50 +572,46 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars
} }
} }
} }
let (declarations, removed_any) = deduplicate_property_declarations(declarations);
let mut block = PropertyDeclarationBlock { let mut block = PropertyDeclarationBlock {
declarations: Arc::new(declarations), declarations: Arc::new(declarations),
any_normal: any_normal, normal_count: normal_count,
any_important: any_important, important_count: important_count,
}; };
if removed_any { deduplicate_property_declarations(&mut block);
block.recalc_any();
}
block block
} }
/// Only keep the "winning" declaration for any given property, by importance then source order. /// Only keep the "winning" declaration for any given property, by importance then source order.
/// The input and output are in source order /// The input and output are in source order
fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Importance)>) fn deduplicate_property_declarations(block: &mut PropertyDeclarationBlock) {
-> (Vec<(PropertyDeclaration, Importance)>, bool) {
let mut removed_any = false;
let mut deduplicated = Vec::new(); let mut deduplicated = Vec::new();
let mut seen_normal = PropertyBitField::new(); let mut seen_normal = PropertyBitField::new();
let mut seen_important = PropertyBitField::new(); let mut seen_important = PropertyBitField::new();
let mut seen_custom_normal = Vec::new(); let mut seen_custom_normal = Vec::new();
let mut seen_custom_important = Vec::new(); let mut seen_custom_important = Vec::new();
for (declaration, importance) in declarations.into_iter().rev() {
let declarations = Arc::get_mut(&mut block.declarations).unwrap();
for (declaration, importance) in declarations.drain(..).rev() {
match declaration { match declaration {
% for property in data.longhands: % for property in data.longhands:
PropertyDeclaration::${property.camel_case}(..) => { PropertyDeclaration::${property.camel_case}(..) => {
% if not property.derived_from: % if not property.derived_from:
if importance.important() { if importance.important() {
if seen_important.get_${property.ident}() { if seen_important.get_${property.ident}() {
removed_any = true; block.important_count -= 1;
continue continue
} }
if seen_normal.get_${property.ident}() { if seen_normal.get_${property.ident}() {
remove_one(&mut deduplicated, |d| { remove_one(&mut deduplicated, |d| {
matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _)) matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _))
}); });
removed_any = true; block.normal_count -= 1;
} }
seen_important.set_${property.ident}() seen_important.set_${property.ident}()
} else { } else {
if seen_normal.get_${property.ident}() || if seen_normal.get_${property.ident}() ||
seen_important.get_${property.ident}() { seen_important.get_${property.ident}() {
removed_any = true; block.normal_count -= 1;
continue continue
} }
seen_normal.set_${property.ident}() seen_normal.set_${property.ident}()
@ -644,20 +624,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
PropertyDeclaration::Custom(ref name, _) => { PropertyDeclaration::Custom(ref name, _) => {
if importance.important() { if importance.important() {
if seen_custom_important.contains(name) { if seen_custom_important.contains(name) {
removed_any = true; block.important_count -= 1;
continue continue
} }
if seen_custom_normal.contains(name) { if seen_custom_normal.contains(name) {
remove_one(&mut deduplicated, |d| { remove_one(&mut deduplicated, |d| {
matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name) matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name)
}); });
removed_any = true; block.normal_count -= 1;
} }
seen_custom_important.push(name.clone()) seen_custom_important.push(name.clone())
} else { } else {
if seen_custom_normal.contains(name) || if seen_custom_normal.contains(name) ||
seen_custom_important.contains(name) { seen_custom_important.contains(name) {
removed_any = true; block.normal_count -= 1;
continue continue
} }
seen_custom_normal.push(name.clone()) seen_custom_normal.push(name.clone())
@ -667,7 +647,7 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp
deduplicated.push((declaration, importance)) deduplicated.push((declaration, importance))
} }
deduplicated.reverse(); deduplicated.reverse();
(deduplicated, removed_any) *declarations = deduplicated;
} }
#[inline] #[inline]

12
components/style/selector_matching.rs
View file

@ -162,8 +162,8 @@ 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, $any: ident) => { ($style_rule: ident, $priority: ident, $importance: expr, $count: ident) => {
if $style_rule.declarations.$any { if $style_rule.declarations.$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
@ -191,8 +191,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) => {
append!(style_rule, normal, Importance::Normal, any_normal); append!(style_rule, normal, Importance::Normal, normal_count);
append!(style_rule, important, Importance::Important, any_important); 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 {
@ -394,7 +394,7 @@ impl Stylist {
// Step 4: Normal style attributes. // Step 4: Normal style attributes.
if let Some(ref sa) = style_attribute { if let Some(ref sa) = style_attribute {
if sa.any_normal { if sa.normal_count > 0 {
relations |= AFFECTED_BY_STYLE_ATTRIBUTE; relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
Push::push( Push::push(
applicable_declarations, applicable_declarations,
@ -416,7 +416,7 @@ impl Stylist {
// Step 6: `!important` style attributes. // Step 6: `!important` style attributes.
if let Some(ref sa) = style_attribute { if let Some(ref sa) = style_attribute {
if sa.any_important { if sa.important_count > 0 {
relations |= AFFECTED_BY_STYLE_ATTRIBUTE; relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
Push::push( Push::push(
applicable_declarations, applicable_declarations,

12
tests/unit/style/properties/serialization.rs
View file

@ -46,8 +46,10 @@ fn property_declaration_block_should_serialize_correctly() {
let block = PropertyDeclarationBlock { let block = PropertyDeclarationBlock {
declarations: Arc::new(declarations), declarations: Arc::new(declarations),
any_important: true,
any_normal: true, // Incorrect, but not used here:
normal_count: 0,
important_count: 0,
}; };
let css_string = block.to_css_string(); let css_string = block.to_css_string();
@ -64,8 +66,10 @@ mod shorthand_serialization {
pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String { pub fn shorthand_properties_to_string(properties: Vec<PropertyDeclaration>) -> String {
let block = PropertyDeclarationBlock { let block = PropertyDeclarationBlock {
declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()), declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()),
any_important: false,
any_normal: true, // Incorrect, but not used here:
normal_count: 0,
important_count: 0,
}; };
block.to_css_string() block.to_css_string()

28
tests/unit/style/stylesheets.rs
View file

@ -24,8 +24,18 @@ fn test_parse_stylesheet() {
let css = r" let css = r"
@namespace url(http://www.w3.org/1999/xhtml); @namespace url(http://www.w3.org/1999/xhtml);
/* FIXME: only if scripting is enabled */ /* FIXME: only if scripting is enabled */
input[type=hidden i] { display: none !important; } input[type=hidden i] {
html , body /**/ { display: block; } display: block !important;
display: none !important;
display: inline;
--a: b !important;
--a: inherit !important;
--a: c;
}
html , body /**/ {
display: none;
display: block;
}
#d1 > .ok { background: blue; } #d1 > .ok { background: blue; }
@keyframes foo { @keyframes foo {
from { width: 0% } from { width: 0% }
@ -92,9 +102,11 @@ fn test_parse_stylesheet() {
(PropertyDeclaration::Display(DeclaredValue::Value( (PropertyDeclaration::Display(DeclaredValue::Value(
longhands::display::SpecifiedValue::none)), longhands::display::SpecifiedValue::none)),
Importance::Important), Importance::Important),
(PropertyDeclaration::Custom(Atom::from("a"), DeclaredValue::Inherit),
Importance::Important),
]), ]),
any_normal: false, normal_count: 0,
any_important: true, important_count: 2,
}, },
}), }),
CSSRule::Style(StyleRule { CSSRule::Style(StyleRule {
@ -140,8 +152,8 @@ fn test_parse_stylesheet() {
longhands::display::SpecifiedValue::block)), longhands::display::SpecifiedValue::block)),
Importance::Normal), Importance::Normal),
]), ]),
any_normal: true, normal_count: 1,
any_important: false, important_count: 0,
}, },
}), }),
CSSRule::Style(StyleRule { CSSRule::Style(StyleRule {
@ -196,8 +208,8 @@ fn test_parse_stylesheet() {
(PropertyDeclaration::BackgroundClip(DeclaredValue::Initial), (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial),
Importance::Normal), Importance::Normal),
]), ]),
any_normal: true, normal_count: 8,
any_important: false, important_count: 0,
}, },
}), }),
CSSRule::Keyframes(KeyframesRule { CSSRule::Keyframes(KeyframesRule {