Auto merge of #20049 - emilio:cleanup-serialization-a-bit, r=SimonSapin

style: Cleanup serialization a bit.

Mostly minor cleanups, in preparation for more refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20049)
<!-- Reviewable:end -->

components/style/properties/declaration_block.rs

@@ -304,17 +304,14 @@ impl PropertyDeclarationBlock {
         })
     }
 
-    /// Find the value of the given property in this block and serialize it
+    fn shorthand_to_css(
-    ///
+        &self,
-    /// <https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertyvalue>
+        shorthand: ShorthandId,
-    pub fn property_value_to_css(&self, property: &PropertyId, dest: &mut CssStringWriter) -> fmt::Result {
+        dest: &mut CssStringWriter,
-        // Step 1.1: done when parsing a string to PropertyId
+    ) -> fmt::Result {
-
+        // Step 1.2.1 of
-        // Step 1.2
+        // https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue
-        match property.as_shorthand() {
+        let mut list = SmallVec::<[&_; 10]>::new();
-            Ok(shorthand) => {
-                // Step 1.2.1
-                let mut list = Vec::new();
         let mut important_count = 0;
 
         // Step 1.2.2
@@ -343,13 +340,26 @@ impl PropertyDeclarationBlock {
         // Step 1.2.3
         // We don't print !important when serializing individual properties,
         // so we treat this as a normal-importance property
-                match shorthand.get_shorthand_appendable_value(list) {
+        match shorthand.get_shorthand_appendable_value(list.iter().cloned()) {
-                    Some(appendable_value) =>
+            Some(appendable_value) => {
-                        append_declaration_value(dest, appendable_value),
+                append_declaration_value(dest, appendable_value)
+            }
             None => return Ok(()),
         }
     }
-            Err(longhand_or_custom) => {
+
+    /// Find the value of the given property in this block and serialize it
+    ///
+    /// <https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue>
+    pub fn property_value_to_css(&self, property: &PropertyId, dest: &mut CssStringWriter) -> fmt::Result {
+        // Step 1.1: done when parsing a string to PropertyId
+
+        // Step 1.2
+        let longhand_or_custom = match property.as_shorthand() {
+            Ok(shorthand) => return self.shorthand_to_css(shorthand, dest),
+            Err(longhand_or_custom) => longhand_or_custom,
+        };
+
         if let Some((value, _importance)) = self.get(longhand_or_custom) {
             // Step 2
             value.to_css(dest)
@@ -358,10 +368,8 @@ impl PropertyDeclarationBlock {
             Ok(())
         }
     }
-        }
-    }
 
-    /// <https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-getpropertypriority>
+    /// <https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertypriority>
     pub fn property_priority(&self, property: &PropertyId) -> Importance {
         // Step 1: done when parsing a string to PropertyId
 
@@ -575,7 +583,7 @@ impl PropertyDeclarationBlock {
         updated_at_least_one
     }
 
-    /// <https://dev.w3.org/csswg/cssom/#dom-cssstyledeclaration-removeproperty>
+    /// <https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-removeproperty>
     ///
     /// Returns whether any declaration was actually removed.
     pub fn remove_property(&mut self, property: &PropertyId) -> bool {
@@ -617,18 +625,24 @@ impl PropertyDeclarationBlock {
         dest: &mut CssStringWriter,
         computed_values: Option<&ComputedValues>,
         custom_properties_block: Option<&PropertyDeclarationBlock>,
-    ) -> fmt::Result
+    ) -> fmt::Result {
-    {
+        if let Ok(shorthand) = property.as_shorthand() {
-        match property.as_shorthand() {
+            return self.shorthand_to_css(shorthand, dest);
-            Err(_longhand_or_custom) => {
+        }
-                if self.declarations.len() == 1 {
+
-                    let declaration = &self.declarations[0];
+        // FIXME(emilio): Should this assert, or assert that the declaration is
+        // the property we expect?
+        let declaration = match self.declarations.get(0) {
+            Some(d) => d,
+            None => return Err(fmt::Error),
+        };
+
         let custom_properties = if let Some(cv) = computed_values {
-                        // If there are extra custom properties for this
+            // If there are extra custom properties for this declaration block,
-                        // declaration block, factor them in too.
+            // factor them in too.
             if let Some(block) = custom_properties_block {
-                            // FIXME(emilio): This is not super-efficient
+                // FIXME(emilio): This is not super-efficient here, and all this
-                            // here...
+                // feels like a hack anyway...
                 block.cascade_custom_properties(cv.custom_properties())
             } else {
                 cv.custom_properties().cloned()
@@ -638,9 +652,11 @@ impl PropertyDeclarationBlock {
         };
 
         match (declaration, computed_values) {
-                        // If we have a longhand declaration with variables, those variables will be
+            // If we have a longhand declaration with variables, those variables
-                        // stored as unparsed values. As a temporary measure to produce sensible results
+            // will be stored as unparsed values.
-                        // in Gecko's getKeyframes() implementation for CSS animations, if
+            //
+            // As a temporary measure to produce sensible results in Gecko's
+            // getKeyframes() implementation for CSS animations, if
             // |computed_values| is supplied, we use it to expand such variable
             // declarations. This will be fixed properly in Gecko bug 1391537.
             (
@@ -655,26 +671,6 @@ impl PropertyDeclarationBlock {
             },
             (ref d, _) => d.to_css(dest),
         }
-                } else {
-                    Err(fmt::Error)
-                }
-            }
-            Ok(shorthand) => {
-                if !self.declarations.iter().all(|decl| decl.shorthands().contains(&shorthand)) {
-                    return Err(fmt::Error)
-                }
-                let iter = self.declarations.iter();
-                match shorthand.get_shorthand_appendable_value(iter) {
-                    Some(AppendableValue::Css { css, .. }) => {
-                        css.append_to(dest)
-                    },
-                    Some(AppendableValue::DeclarationsForShorthand(_, decls)) => {
-                        shorthand.longhands_to_css(decls, &mut CssWriter::new(dest))
-                    }
-                    _ => Ok(())
-                }
-            }
-        }
     }
 
     /// Convert AnimationValueMap to PropertyDeclarationBlock.
@@ -765,13 +761,11 @@ impl PropertyDeclarationBlock {
             }
 
             // Step 3.3
-            let shorthands = declaration.shorthands();
-            if !shorthands.is_empty() {
             // Step 3.3.1 is done by checking already_serialized while
             // iterating below.
 
             // Step 3.3.2
-                for &shorthand in shorthands {
+            for &shorthand in declaration.shorthands() {
                 let properties = shorthand.longhands();
 
                 // Substep 2 & 3
@@ -902,7 +896,6 @@ impl PropertyDeclarationBlock {
                 // preferred order.
                 break;
             }
-            }
 
             // Step 3.3.4
             if already_serialized.contains(property) {