diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 999f4aa100a..f6f8661689c 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -767,6 +767,7 @@ impl Element { .position(|&(ref decl, _)| decl.matches(property)); if let Some(index) = index { Arc::make_mut(&mut declarations.declarations).remove(index); + declarations.recalc_any(); } } } @@ -781,26 +782,30 @@ impl Element { fn update(element: &Element, declarations: Vec, importance: Importance) { let mut inline_declarations = element.style_attribute().borrow_mut(); - if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations { - // Usually, the reference count will be 1 here. But transitions could make it greater - // than that. - let existing_declarations = Arc::make_mut(&mut existing_declarations.declarations); + if let &mut Some(ref mut declaration_block) = &mut *inline_declarations { + { + // Usually, the reference count will be 1 here. But transitions could make it greater + // than that. + let existing_declarations = Arc::make_mut(&mut declaration_block.declarations); - 'outer: for incoming_declaration in declarations { - for existing_declaration in &mut *existing_declarations { - if existing_declaration.0.name() == incoming_declaration.name() { - *existing_declaration = (incoming_declaration, importance); - continue 'outer; + 'outer: for incoming_declaration in declarations { + for existing_declaration in &mut *existing_declarations { + if existing_declaration.0.name() == incoming_declaration.name() { + *existing_declaration = (incoming_declaration, importance); + continue 'outer; + } } + existing_declarations.push((incoming_declaration, importance)); } - existing_declarations.push((incoming_declaration, importance)); } - + declaration_block.recalc_any(); return; } *inline_declarations = Some(PropertyDeclarationBlock { declarations: Arc::new(declarations.into_iter().map(|d| (d, importance)).collect()), + any_important: importance.important(), + any_normal: !importance.important(), }); } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 45ee70ee3a1..539e03253c4 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -287,6 +287,28 @@ impl Importance { pub struct PropertyDeclarationBlock { #[cfg_attr(feature = "servo", ignore_heap_size_of = "#7038")] pub declarations: Arc>, + + /// Whether `self.declaration` contains at least one declaration with `Importance::Normal` + pub any_normal: bool, + + /// Whether `self.declaration` contains at least one declaration with `Importance::Important` + pub any_important: bool, +} + +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 { @@ -542,6 +564,8 @@ impl<'a, 'b> DeclarationParser for PropertyDeclarationParser<'a, 'b> { pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Parser) -> PropertyDeclarationBlock { let mut declarations = Vec::new(); + let mut any_normal = false; + let mut any_important = false; let parser = PropertyDeclarationParser { context: context, }; @@ -549,6 +573,11 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars while let Some(declaration) = iter.next() { match declaration { Ok((results, importance)) => { + if importance.important() { + any_important = true + } else { + any_normal = true + } declarations.extend(results.into_iter().map(|d| (d, importance))) } Err(range) => { @@ -559,16 +588,24 @@ pub fn parse_property_declaration_list(context: &ParserContext, input: &mut Pars } } } - PropertyDeclarationBlock { - declarations: Arc::new(deduplicate_property_declarations(declarations)), + let (declarations, removed_any) = deduplicate_property_declarations(declarations); + let mut block = PropertyDeclarationBlock { + declarations: Arc::new(declarations), + any_normal: any_normal, + any_important: any_important, + }; + if removed_any { + block.recalc_any(); } + block } /// Only keep the "winning" declaration for any given property, by importance then source order. /// The input and output are in source order fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Importance)>) - -> Vec<(PropertyDeclaration, Importance)> { + -> (Vec<(PropertyDeclaration, Importance)>, bool) { + let mut removed_any = false; let mut deduplicated = Vec::new(); let mut seen_normal = PropertyBitField::new(); let mut seen_important = PropertyBitField::new(); @@ -581,17 +618,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp % if not property.derived_from: if importance.important() { if seen_important.get_${property.ident}() { + removed_any = true; continue } if seen_normal.get_${property.ident}() { remove_one(&mut deduplicated, |d| { matches!(d, &(PropertyDeclaration::${property.camel_case}(..), _)) - }) + }); + removed_any = true; } seen_important.set_${property.ident}() } else { if seen_normal.get_${property.ident}() || seen_important.get_${property.ident}() { + removed_any = true; continue } seen_normal.set_${property.ident}() @@ -604,17 +644,20 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp PropertyDeclaration::Custom(ref name, _) => { if importance.important() { if seen_custom_important.contains(name) { + removed_any = true; continue } if seen_custom_normal.contains(name) { remove_one(&mut deduplicated, |d| { matches!(d, &(PropertyDeclaration::Custom(ref n, _), _) if n == name) - }) + }); + removed_any = true; } seen_custom_important.push(name.clone()) } else { if seen_custom_normal.contains(name) || seen_custom_important.contains(name) { + removed_any = true; continue } seen_custom_normal.push(name.clone()) @@ -624,7 +667,7 @@ fn deduplicate_property_declarations(declarations: Vec<(PropertyDeclaration, Imp deduplicated.push((declaration, importance)) } deduplicated.reverse(); - deduplicated + (deduplicated, removed_any) } #[inline] diff --git a/components/style/selector_matching.rs b/components/style/selector_matching.rs index c8f6dc089f1..3c6c1fe89f5 100644 --- a/components/style/selector_matching.rs +++ b/components/style/selector_matching.rs @@ -164,8 +164,8 @@ impl Stylist { // Take apart the StyleRule into individual Rules and insert // them into the SelectorMap of that priority. macro_rules! append( - ($style_rule: ident, $priority: ident, $importance: expr) => { - if !$style_rule.declarations.declarations.is_empty() { + ($style_rule: ident, $priority: ident, $importance: expr, $any: ident) => { + if $style_rule.declarations.$any { for selector in &$style_rule.selectors { let map = if let Some(ref pseudo) = selector.pseudo_element { self.pseudos_map @@ -193,8 +193,8 @@ impl Stylist { for rule in stylesheet.effective_rules(&self.device) { match *rule { CSSRule::Style(ref style_rule) => { - append!(style_rule, normal, Importance::Normal); - append!(style_rule, important, Importance::Important); + append!(style_rule, normal, Importance::Normal, any_normal); + append!(style_rule, important, Importance::Important, any_important); rules_source_order += 1; for selector in &style_rule.selectors { @@ -396,12 +396,14 @@ impl Stylist { // Step 4: Normal style attributes. if let Some(ref sa) = style_attribute { - relations |= AFFECTED_BY_STYLE_ATTRIBUTE; - Push::push( - applicable_declarations, - DeclarationBlock::from_declarations( - sa.declarations.clone(), - Importance::Normal)); + if sa.any_normal { + relations |= AFFECTED_BY_STYLE_ATTRIBUTE; + Push::push( + applicable_declarations, + DeclarationBlock::from_declarations( + sa.declarations.clone(), + Importance::Normal)); + } } debug!("style attr: {:?}", relations); @@ -416,11 +418,14 @@ impl Stylist { // Step 6: `!important` style attributes. if let Some(ref sa) = style_attribute { - Push::push( - applicable_declarations, - DeclarationBlock::from_declarations( - sa.declarations.clone(), - Importance::Important)); + if sa.any_important { + relations |= AFFECTED_BY_STYLE_ATTRIBUTE; + Push::push( + applicable_declarations, + DeclarationBlock::from_declarations( + sa.declarations.clone(), + Importance::Important)); + } } debug!("style attr important: {:?}", relations); diff --git a/tests/unit/script/size_of.rs b/tests/unit/script/size_of.rs index 472beb888c3..b30304d5dac 100644 --- a/tests/unit/script/size_of.rs +++ b/tests/unit/script/size_of.rs @@ -40,10 +40,10 @@ macro_rules! sizeof_checker ( // Update the sizes here sizeof_checker!(size_event_target, EventTarget, 40); sizeof_checker!(size_node, Node, 160); -sizeof_checker!(size_element, Element, 328); -sizeof_checker!(size_htmlelement, HTMLElement, 344); -sizeof_checker!(size_div, HTMLDivElement, 344); -sizeof_checker!(size_span, HTMLSpanElement, 344); +sizeof_checker!(size_element, Element, 336); +sizeof_checker!(size_htmlelement, HTMLElement, 352); +sizeof_checker!(size_div, HTMLDivElement, 352); +sizeof_checker!(size_span, HTMLSpanElement, 352); sizeof_checker!(size_text, Text, 192); sizeof_checker!(size_characterdata, CharacterData, 192); sizeof_checker!(size_servothreadsafelayoutnode, ServoThreadSafeLayoutNode, 16); diff --git a/tests/unit/style/properties/serialization.rs b/tests/unit/style/properties/serialization.rs index 2f2b5d39127..d22043b237b 100644 --- a/tests/unit/style/properties/serialization.rs +++ b/tests/unit/style/properties/serialization.rs @@ -46,6 +46,8 @@ fn property_declaration_block_should_serialize_correctly() { let block = PropertyDeclarationBlock { declarations: Arc::new(declarations), + any_important: true, + any_normal: true, }; let css_string = block.to_css_string(); @@ -62,6 +64,8 @@ mod shorthand_serialization { pub fn shorthand_properties_to_string(properties: Vec) -> String { let block = PropertyDeclarationBlock { declarations: Arc::new(properties.into_iter().map(|d| (d, Importance::Normal)).collect()), + any_important: false, + any_normal: true, }; block.to_css_string() diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 27d35c513cf..28b795d440e 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -93,6 +93,8 @@ fn test_parse_stylesheet() { longhands::display::SpecifiedValue::none)), Importance::Important), ]), + any_normal: false, + any_important: true, }, }), CSSRule::Style(StyleRule { @@ -138,6 +140,8 @@ fn test_parse_stylesheet() { longhands::display::SpecifiedValue::block)), Importance::Normal), ]), + any_normal: true, + any_important: false, }, }), CSSRule::Style(StyleRule { @@ -192,6 +196,8 @@ fn test_parse_stylesheet() { (PropertyDeclaration::BackgroundClip(DeclaredValue::Initial), Importance::Normal), ]), + any_normal: true, + any_important: false, }, }), CSSRule::Keyframes(KeyframesRule {