servo: Merge #16183 - Replace ParsedDeclaration::expand with non-generic method (from servo:expand-diet); r=emilio
authorSimon Sapin <simon.sapin@exyr.org>
Wed, 29 Mar 2017 13:43:06 -0500
changeset 350513 26639f254814396c159011b7f71e0a202bba008c
parent 350512 8664b032a33028ee909c15adf164a742d294e863
child 350514 142baaabe917da059d400648c9455ad82d8595d8
push id88655
push userryanvm@gmail.com
push dateThu, 30 Mar 2017 19:21:42 +0000
treeherdermozilla-inbound@23e4fd04033b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #16183 - Replace ParsedDeclaration::expand with non-generic method (from servo:expand-diet); r=emilio ... to reduce its code size impact. https://bugzilla.mozilla.org/show_bug.cgi?id=1351737 Source-Repo: https://github.com/servo/servo Source-Revision: f3a5ad2f497a13aff0faf7873ce98f0868e56101
servo/components/script/dom/cssstyledeclaration.rs
servo/components/style/keyframes.rs
servo/components/style/properties/declaration_block.rs
servo/components/style/properties/properties.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/script/dom/cssstyledeclaration.rs
+++ b/servo/components/script/dom/cssstyledeclaration.rs
@@ -267,20 +267,17 @@ impl CSSStyleDeclaration {
                 Err(_) => {
                     *changed = false;
                     return Ok(());
                 }
             };
 
             // Step 8
             // Step 9
-            *changed = false;
-            parsed.expand(|declaration| {
-                *changed |= pdb.set_parsed_declaration(declaration, importance);
-            });
+            *changed = parsed.expand_set_into(pdb, importance);
 
             Ok(())
         })
     }
 }
 
 impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     // https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-length
--- a/servo/components/style/keyframes.rs
+++ b/servo/components/style/keyframes.rs
@@ -369,17 +369,17 @@ impl<'a> QualifiedRuleParser for Keyfram
                    -> Result<Self::QualifiedRule, ()> {
         let parser = KeyframeDeclarationParser {
             context: self.context,
         };
         let mut iter = DeclarationListParser::new(input, parser);
         let mut block = PropertyDeclarationBlock::new();
         while let Some(declaration) = iter.next() {
             match declaration {
-                Ok(parsed) => parsed.expand(|d| block.push(d, Importance::Normal)),
+                Ok(parsed) => parsed.expand_push_into(&mut block, Importance::Normal),
                 Err(range) => {
                     let pos = range.start;
                     let message = format!("Unsupported keyframe property declaration: '{}'",
                                           iter.input.slice(range));
                     log_css_error(iter.input, pos, &*message, self.context);
                 }
             }
             // `parse_important` is not called here, `!important` is not allowed in keyframe blocks.
--- a/servo/components/style/properties/declaration_block.rs
+++ b/servo/components/style/properties/declaration_block.rs
@@ -83,26 +83,26 @@ impl PropertyDeclarationBlock {
         }
     }
 
     /// The declarations in this block
     pub fn declarations(&self) -> &[(PropertyDeclaration, Importance)] {
         &self.declarations
     }
 
-    /// Returns wheather this block contains any declaration with `!important`.
+    /// Returns whether this block contains any declaration with `!important`.
     ///
     /// This is based on the `important_count` counter,
     /// which should be maintained whenever `declarations` is changed.
     // FIXME: make fields private and maintain it here in methods?
     pub fn any_important(&self) -> bool {
         self.important_count > 0
     }
 
-    /// Returns wheather this block contains any declaration without `!important`.
+    /// Returns whether this block contains any declaration without `!important`.
     ///
     /// This is based on the `important_count` counter,
     /// which should be maintained whenever `declarations` is changed.
     // FIXME: make fields private and maintain it here in methods?
     pub fn any_normal(&self) -> bool {
         self.declarations.len() > self.important_count
     }
 
@@ -197,26 +197,19 @@ impl PropertyDeclarationBlock {
     }
 
     /// Adds or overrides the declaration for a given property in this block,
     /// except if an existing declaration for the same property is more important.
     pub fn push(&mut self, declaration: PropertyDeclaration, importance: Importance) {
         self.push_common(declaration, importance, false);
     }
 
-    /// Adds or overrides the declaration for a given property in this block,
-    /// Returns whether the declaration block is actually changed.
-    pub fn set_parsed_declaration(&mut self,
-                                  declaration: PropertyDeclaration,
-                                  importance: Importance) -> bool {
-        self.push_common(declaration, importance, true)
-    }
-
-    fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance,
-                   overwrite_more_important: bool) -> bool {
+    /// Implementation detail of push and ParsedDeclaration::expand*
+    pub fn push_common(&mut self, declaration: PropertyDeclaration, importance: Importance,
+                       overwrite_more_important: bool) -> bool {
         let definitely_new = if let PropertyDeclarationId::Longhand(id) = declaration.id() {
             !self.longhands.contains(id)
         } else {
             false  // For custom properties, always scan
         };
 
         if !definitely_new {
             for slot in &mut *self.declarations {
@@ -692,17 +685,17 @@ pub fn parse_property_declaration_list(c
                                        -> PropertyDeclarationBlock {
     let mut block = PropertyDeclarationBlock::new();
     let parser = PropertyDeclarationParser {
         context: context,
     };
     let mut iter = DeclarationListParser::new(input, parser);
     while let Some(declaration) = iter.next() {
         match declaration {
-            Ok((parsed, importance)) => parsed.expand(|d| block.push(d, importance)),
+            Ok((parsed, importance)) => parsed.expand_push_into(&mut block, importance),
             Err(range) => {
                 let pos = range.start;
                 let message = format!("Unsupported property declaration: '{}'",
                                       iter.input.slice(range));
                 log_css_error(iter.input, pos, &*message, &context);
             }
         }
     }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -883,52 +883,100 @@ pub enum ParsedDeclaration {
 
     /// Not a shorthand
     LonghandOrCustom(PropertyDeclaration),
 }
 
 impl ParsedDeclaration {
     /// Transform this ParsedDeclaration into a sequence of PropertyDeclaration
     /// by expanding shorthand declarations into their corresponding longhands
-    pub fn expand<F>(self, mut push: F) where F: FnMut(PropertyDeclaration) {
+    ///
+    /// Adds or overrides exsting declarations in the given block,
+    /// except if existing declarations are more important.
+    #[inline]
+    pub fn expand_push_into(self, block: &mut PropertyDeclarationBlock,
+                            importance: Importance) {
+        self.expand_into(block, importance, false);
+    }
+
+    /// Transform this ParsedDeclaration into a sequence of PropertyDeclaration
+    /// by expanding shorthand declarations into their corresponding longhands
+    ///
+    /// Add or override existing declarations in the given block.
+    /// Return whether anything changed.
+    #[inline]
+    pub fn expand_set_into(self, block: &mut PropertyDeclarationBlock,
+                           importance: Importance) -> bool {
+        self.expand_into(block, importance, true)
+    }
+
+    fn expand_into(self, block: &mut PropertyDeclarationBlock,
+                   importance: Importance,
+                   overwrite_more_important: bool) -> bool {
         match self {
             % for shorthand in data.shorthands:
                 ParsedDeclaration::${shorthand.camel_case}(
                     shorthands::${shorthand.ident}::Longhands {
                         % for sub_property in shorthand.sub_properties:
                             ${sub_property.ident},
                         % endfor
                     }
                 ) => {
+                    let mut changed = false;
                     % for sub_property in shorthand.sub_properties:
-                        push(PropertyDeclaration::${sub_property.camel_case}(
-                            % if sub_property.boxed:
-                                Box::new(${sub_property.ident})
-                            % else:
-                                ${sub_property.ident}
-                            % endif
-                        ));
+                        changed |= block.push_common(
+                            PropertyDeclaration::${sub_property.camel_case}(
+                                % if sub_property.boxed:
+                                    Box::new(${sub_property.ident})
+                                % else:
+                                    ${sub_property.ident}
+                                % endif
+                            ),
+                            importance,
+                            overwrite_more_important,
+                        );
                     % endfor
+                    changed
                 },
                 ParsedDeclaration::${shorthand.camel_case}CSSWideKeyword(keyword) => {
+                    let mut changed = false;
                     % for sub_property in shorthand.sub_properties:
-                        push(PropertyDeclaration::CSSWideKeyword(LonghandId::${sub_property.camel_case}, keyword));
+                        changed |= block.push_common(
+                            PropertyDeclaration::CSSWideKeyword(
+                                LonghandId::${sub_property.camel_case},
+                                keyword,
+                            ),
+                            importance,
+                            overwrite_more_important,
+                        );
                     % endfor
+                    changed
                 },
                 ParsedDeclaration::${shorthand.camel_case}WithVariables(value) => {
                     debug_assert_eq!(
                         value.from_shorthand,
                         Some(ShorthandId::${shorthand.camel_case})
                     );
+                    let mut changed = false;
                     % for sub_property in shorthand.sub_properties:
-                        push(PropertyDeclaration::WithVariables(LonghandId::${sub_property.camel_case}, value.clone()));
+                        changed |= block.push_common(
+                            PropertyDeclaration::WithVariables(
+                                LonghandId::${sub_property.camel_case},
+                                value.clone()
+                            ),
+                            importance,
+                            overwrite_more_important,
+                        );
                     % endfor
+                    changed
                 }
             % endfor
-            ParsedDeclaration::LonghandOrCustom(declaration) => push(declaration),
+            ParsedDeclaration::LonghandOrCustom(declaration) => {
+                block.push_common(declaration, importance, overwrite_more_important)
+            }
         }
     }
 
     /// The `in_keyframe_block` parameter controls this:
     ///
     /// https://drafts.csswg.org/css-animations/#keyframes
     /// > The <declaration-list> inside of <keyframe-block> accepts any CSS property
     /// > except those defined in this specification,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -795,17 +795,17 @@ pub extern "C" fn Servo_ParseProperty(pr
                                                      &base_url,
                                                      &reporter,
                                                      extra_data);
 
     match ParsedDeclaration::parse(id, &context, &mut Parser::new(value), false) {
         Ok(parsed) => {
             let global_style_data = &*GLOBAL_STYLE_DATA;
             let mut block = PropertyDeclarationBlock::new();
-            parsed.expand(|d| block.push(d, Importance::Normal));
+            parsed.expand_push_into(&mut block, Importance::Normal);
             Arc::new(global_style_data.shared_lock.wrap(block)).into_strong()
         }
         Err(_) => RawServoDeclarationBlockStrong::null()
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_ParseEasing(easing: *const nsAString,
@@ -953,23 +953,19 @@ fn set_property(declarations: RawServoDe
                 value: *const nsACString, is_important: bool,
                 base: *const nsACString, data: *const structs::GeckoParserExtraData) -> bool {
     let value = unsafe { value.as_ref().unwrap().as_str_unchecked() };
 
     make_context!((base, data) => (base_url, extra_data));
     if let Ok(parsed) = parse_one_declaration(property_id, value, &base_url,
                                               &StdoutErrorReporter, extra_data) {
         let importance = if is_important { Importance::Important } else { Importance::Normal };
-        let mut changed = false;
         write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
-            parsed.expand(|decl| {
-                changed |= decls.set_parsed_declaration(decl, importance);
-            });
-        });
-        changed
+            parsed.expand_set_into(decls, importance)
+        })
     } else {
         false
     }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SetProperty(declarations: RawServoDeclarationBlockBorrowed,
                                                      property: *const nsACString, value: *const nsACString,