Only cascade at a priority level rules that have declarations of that…

… priority.

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<PropertyDeclaration>,
                   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);
-
-                '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;
+            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;
+                            }
                         }
+                        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(),
             });
         }
 

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<Vec<(PropertyDeclaration, Importance)>>,
+
+    /// 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,13 +564,20 @@ 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,
     };
     let mut iter = DeclarationListParser::new(input, parser);
     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]

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);

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);

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<PropertyDeclaration>) -> 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()

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 {