Bug 1485766 - Drop the manually implementation of ToCSS for BasicShape::Polygon. r=emilio

The implementation of ToCSS for Polygon has some rule, and we could use skip_if
to handle and serialization of fill-rule. However, we should derive ToCSS for
the pair of LengthOrPercentages, so define a new type for it.

Differential Revision: https://phabricator.services.mozilla.com/D4153

--HG--
extra : moz-landing-system : lando

servo/components/style/gecko/conversions.rs

@@ -670,7 +670,7 @@ pub mod basic_shape {
     use values::computed::position;
     use values::computed::url::ComputedUrl;
     use values::generics::basic_shape::{BasicShape as GenericBasicShape, InsetRect, Polygon};
-    use values::generics::basic_shape::{Circle, Ellipse, FillRule};
+    use values::generics::basic_shape::{Circle, Ellipse, FillRule, PolygonCoord};
     use values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource};
     use values::generics::border::BorderRadius as GenericBorderRadius;
     use values::generics::rect::Rect;
@@ -800,11 +800,14 @@ pub mod basic_shape {
                     for i in 0..(other.mCoordinates.len() / 2) {
                         let x = 2 * i;
                         let y = x + 1;
-                        coords.push((LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[x])
-                                    .expect("polygon() coordinate should be a length, percentage, or calc value"),
-                                LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[y])
-                                    .expect("polygon() coordinate should be a length, percentage, or calc value")
-                            ))
+                        coords.push(PolygonCoord(
+                            LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[x])
+                                .expect("polygon() coordinate should be a length, percentage, \
+                                        or calc value"),
+                            LengthOrPercentage::from_gecko_style_coord(&other.mCoordinates[y])
+                                .expect("polygon() coordinate should be a length, percentage, \
+                                        or calc value")
+                        ))
                     }
                     GenericBasicShape::Polygon(Polygon {
                         fill: fill_rule,

servo/components/style/values/generics/basic_shape.rs

@@ -113,16 +113,23 @@ pub enum ShapeRadius<LengthOrPercentage> {
 /// A generic type for representing the `polygon()` function
 ///
 /// <https://drafts.csswg.org/css-shapes/#funcdef-polygon>
-#[css(function)]
+#[css(comma, function)]
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo,
-         ToComputedValue)]
+         ToComputedValue, ToCss)]
 pub struct Polygon<LengthOrPercentage> {
     /// The filling rule for a polygon.
+    #[css(skip_if = "fill_is_default")]
     pub fill: FillRule,
     /// A collection of (x, y) coordinates to draw the polygon.
-    pub coordinates: Vec<(LengthOrPercentage, LengthOrPercentage)>,
+    #[css(iterable)]
+    pub coordinates: Vec<PolygonCoord<LengthOrPercentage>>,
 }
 
+/// Coordinates for Polygon.
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo,
+         ToComputedValue, ToCss)]
+pub struct PolygonCoord<LengthOrPercentage>(pub LengthOrPercentage, pub LengthOrPercentage);
+
 // https://drafts.csswg.org/css-shapes/#typedef-fill-rule
 // NOTE: Basic shapes spec says that these are the only two values, however
 // https://www.w3.org/TR/SVG/painting.html#FillRuleProperty
@@ -203,7 +210,7 @@ where
             .iter()
             .zip(other.coordinates.iter())
             .map(|(this, other)| {
-                Ok((
+                Ok(PolygonCoord(
                     this.0.animate(&other.0, procedure)?,
                     this.1.animate(&other.1, procedure)?,
                 ))
@@ -239,34 +246,14 @@ where
     }
 }
 
-impl<L: ToCss> ToCss for Polygon<L> {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str("polygon(")?;
-        if self.fill != FillRule::default() {
-            self.fill.to_css(dest)?;
-            dest.write_str(", ")?;
-        }
-
-        for (i, coord) in self.coordinates.iter().enumerate() {
-            if i > 0 {
-                dest.write_str(", ")?;
-            }
-
-            coord.0.to_css(dest)?;
-            dest.write_str(" ")?;
-            coord.1.to_css(dest)?;
-        }
-
-        dest.write_str(")")
-    }
-}
-
 impl Default for FillRule {
     #[inline]
     fn default() -> Self {
         FillRule::Nonzero
     }
 }
+
+#[inline]
+fn fill_is_default(fill: &FillRule) -> bool {
+    *fill == FillRule::default()
+}

servo/components/style/values/specified/basic_shape.rs

@@ -14,7 +14,7 @@ use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 use values::computed::Percentage;
 use values::generics::basic_shape as generic;
-use values::generics::basic_shape::{FillRule, GeometryBox, ShapeBox, ShapeSource};
+use values::generics::basic_shape::{FillRule, GeometryBox, PolygonCoord, ShapeBox, ShapeSource};
 use values::generics::rect::Rect;
 use values::specified::LengthOrPercentage;
 use values::specified::border::BorderRadius;
@@ -381,7 +381,7 @@ impl Polygon {
             .unwrap_or_default();
 
         let buf = input.parse_comma_separated(|i| {
-            Ok((
+            Ok(PolygonCoord(
                 LengthOrPercentage::parse(context, i)?,
                 LengthOrPercentage::parse(context, i)?,
             ))