style: Remove Options from TransformOperation.

This may or may not be part of the plan to get rid of nsCSSValue ;)

Option is not usable via FFI, and they should not be needed (we should be
following the shortest serialization principle instead). These patches also do
that, which matches the other transform properties. I think that slight change
is fine, if we can make it work, and consistent with other properties.

Alternative is adding more TransformOperation variants or such, which I rather
not do.

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

6 files changed, 78 insertions, 100 deletions

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index 9ee8ab3c656..a22e745963e 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -855,13 +855,13 @@ def set_gecko_property(ffi_name, expr):
 transform_functions = [
     ("Matrix3D", "matrix3d", ["number"] * 16),
     ("Matrix", "matrix", ["number"] * 6),
-    ("Translate", "translate", ["lp", "optional_lp"]),
+    ("Translate", "translate", ["lp", "lp"]),
     ("Translate3D", "translate3d", ["lp", "lp", "length"]),
     ("TranslateX", "translatex", ["lp"]),
     ("TranslateY", "translatey", ["lp"]),
     ("TranslateZ", "translatez", ["length"]),
     ("Scale3D", "scale3d", ["number"] * 3),
-    ("Scale", "scale", ["number", "optional_number"]),
+    ("Scale", "scale", ["number", "number"]),
     ("ScaleX", "scalex", ["number"]),
     ("ScaleY", "scaley", ["number"]),
     ("ScaleZ", "scalez", ["number"]),
@@ -870,7 +870,7 @@ transform_functions = [
     ("RotateX", "rotatex", ["angle"]),
     ("RotateY", "rotatey", ["angle"]),
     ("RotateZ", "rotatez", ["angle"]),
-    ("Skew", "skew", ["angle", "optional_angle"]),
+    ("Skew", "skew", ["angle", "angle"]),
     ("SkewX", "skewx", ["angle"]),
     ("SkewY", "skewy", ["angle"]),
     ("Perspective", "perspective", ["length"]),
@@ -881,7 +881,6 @@ transform_functions = [
 
 <%def name="transform_function_arm(name, keyword, items)">
     <%
-        has_optional = items[-1].startswith("optional_")
         pattern = None
         if keyword == "matrix3d":
             # m11: number1, m12: number2, ..
@@ -919,36 +918,20 @@ transform_functions = [
         }
     %>
     crate::values::generics::transform::TransformOperation::${name}${pattern} => {
-        % if has_optional:
-            let optional_present = ${items[-1] + str(len(items))}.is_some();
-            let len = if optional_present {
-                ${len(items) + 1}
-            } else {
-                ${len(items)}
-            };
-        % else:
-            let len = ${len(items) + 1};
-        % endif
+        let len = ${len(items) + 1};
         bindings::Gecko_CSSValue_SetFunction(gecko_value, len);
         bindings::Gecko_CSSValue_SetKeyword(
             bindings::Gecko_CSSValue_GetArrayItem(gecko_value, 0),
             structs::nsCSSKeyword::eCSSKeyword_${keyword}
         );
         % for index, item in enumerate(items):
-            <% replaced_item = item.replace("optional_", "") %>
-            % if item.startswith("optional"):
-                if let Some(${replaced_item + str(index + 1)}) = ${item + str(index + 1)} {
-            % endif
             % if item == "list":
                 debug_assert!(!${item}${index + 1}.0.is_empty());
             % endif
-            ${css_value_setters[replaced_item] % (
+            ${css_value_setters[item] % (
                 "bindings::Gecko_CSSValue_GetArrayItem(gecko_value, %d)" % (index + 1),
-                replaced_item + str(index + 1)
+                item + str(index + 1)
             )};
-            % if item.startswith("optional"):
-                }
-            % endif
         % endfor
     }
 </%def>
@@ -959,8 +942,6 @@ transform_functions = [
         css_value_getters = {
             "length" : "Length::new(bindings::Gecko_CSSValue_GetNumber(%s))",
             "lp" : "%s.get_length_percentage()",
-            "lpon" : "Either::Second(%s.get_length_percentage())",
-            "lon" : "Either::First(%s.get_length())",
             "angle" : "%s.get_angle()",
             "number" : "bindings::Gecko_CSSValue_GetNumber(%s)",
             "percentage" : "Percentage(bindings::Gecko_CSSValue_GetPercentage(%s))",
@@ -998,20 +979,11 @@ transform_functions = [
                 ${field_names[index]}:
             % endif
             <%
-                getter = css_value_getters[item.replace("optional_", "")] % (
+                getter = css_value_getters[item] % (
                     "bindings::Gecko_CSSValue_GetArrayItemConst(gecko_value, %d)" % (index + 1)
                 )
             %>
-            % if item.startswith("optional_"):
-                if (**gecko_value.mValue.mArray.as_ref()).mCount == ${index + 1} {
-                    None
-                } else {
-                    Some(${getter})
-                }
-            % else:
-                ${getter}
-            % endif
-,
+            ${getter},
         % endfor
         ${post_symbols}
     },
diff --git a/components/style/values/animated/transform.rs b/components/style/values/animated/transform.rs
index 507c4ba4671..0c322d8fccc 100644
--- a/components/style/values/animated/transform.rs
+++ b/components/style/values/animated/transform.rs
@@ -1003,18 +1003,12 @@ impl Animate for ComputedTransformOperation {
             (&TransformOperation::Matrix(ref this), &TransformOperation::Matrix(ref other)) => {
                 Ok(TransformOperation::Matrix(this.animate(other, procedure)?))
             },
-            (&TransformOperation::Skew(ref fx, None), &TransformOperation::Skew(ref tx, None)) => {
-                Ok(TransformOperation::Skew(fx.animate(tx, procedure)?, None))
-            },
             (
                 &TransformOperation::Skew(ref fx, ref fy),
                 &TransformOperation::Skew(ref tx, ref ty),
             ) => Ok(TransformOperation::Skew(
                 fx.animate(tx, procedure)?,
-                Some(
-                    fy.unwrap_or(Angle::zero())
-                        .animate(&ty.unwrap_or(Angle::zero()), procedure)?,
-                ),
+                fy.animate(ty, procedure)?,
             )),
             (&TransformOperation::SkewX(ref f), &TransformOperation::SkewX(ref t)) => {
                 Ok(TransformOperation::SkewX(f.animate(t, procedure)?))
@@ -1031,21 +1025,11 @@ impl Animate for ComputedTransformOperation {
                 fz.animate(tz, procedure)?,
             )),
             (
-                &TransformOperation::Translate(ref fx, None),
-                &TransformOperation::Translate(ref tx, None),
-            ) => Ok(TransformOperation::Translate(
-                fx.animate(tx, procedure)?,
-                None,
-            )),
-            (
                 &TransformOperation::Translate(ref fx, ref fy),
                 &TransformOperation::Translate(ref tx, ref ty),
             ) => Ok(TransformOperation::Translate(
                 fx.animate(tx, procedure)?,
-                Some(
-                    fy.unwrap_or(LengthPercentage::zero())
-                        .animate(&ty.unwrap_or(LengthPercentage::zero()), procedure)?,
-                ),
+                fy.animate(ty, procedure)?,
             )),
             (&TransformOperation::TranslateX(ref f), &TransformOperation::TranslateX(ref t)) => {
                 Ok(TransformOperation::TranslateX(f.animate(t, procedure)?))
@@ -1073,22 +1057,12 @@ impl Animate for ComputedTransformOperation {
             (&TransformOperation::ScaleZ(ref f), &TransformOperation::ScaleZ(ref t)) => Ok(
                 TransformOperation::ScaleZ(animate_multiplicative_factor(*f, *t, procedure)?),
             ),
-            (&TransformOperation::Scale(ref f, None), &TransformOperation::Scale(ref t, None)) => {
-                Ok(TransformOperation::Scale(
-                    animate_multiplicative_factor(*f, *t, procedure)?,
-                    None,
-                ))
-            },
             (
                 &TransformOperation::Scale(ref fx, ref fy),
                 &TransformOperation::Scale(ref tx, ref ty),
             ) => Ok(TransformOperation::Scale(
                 animate_multiplicative_factor(*fx, *tx, procedure)?,
-                Some(animate_multiplicative_factor(
-                    fy.unwrap_or(*fx),
-                    ty.unwrap_or(*tx),
-                    procedure,
-                )?),
+                animate_multiplicative_factor(*fy, *ty, procedure)?,
             )),
             (
                 &TransformOperation::Rotate3D(fx, fy, fz, fa),
diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs
index f3b40c79f9e..cdb7fb39b47 100644
--- a/components/style/values/computed/transform.rs
+++ b/components/style/values/computed/transform.rs
@@ -371,15 +371,14 @@ impl TransformOperation {
     pub fn to_translate_3d(&self) -> Self {
         match *self {
             generic::TransformOperation::Translate3D(..) => self.clone(),
-            generic::TransformOperation::TranslateX(ref x) |
-            generic::TransformOperation::Translate(ref x, None) => {
+            generic::TransformOperation::TranslateX(ref x) => {
                 generic::TransformOperation::Translate3D(
                     x.clone(),
                     LengthPercentage::zero(),
                     Length::zero(),
                 )
             },
-            generic::TransformOperation::Translate(ref x, Some(ref y)) => {
+            generic::TransformOperation::Translate(ref x, ref y) => {
                 generic::TransformOperation::Translate3D(x.clone(), y.clone(), Length::zero())
             },
             generic::TransformOperation::TranslateY(ref y) => {
@@ -426,10 +425,7 @@ impl TransformOperation {
     pub fn to_scale_3d(&self) -> Self {
         match *self {
             generic::TransformOperation::Scale3D(..) => self.clone(),
-            generic::TransformOperation::Scale(s, None) => {
-                generic::TransformOperation::Scale3D(s, s, 1.)
-            },
-            generic::TransformOperation::Scale(x, Some(y)) => {
+            generic::TransformOperation::Scale(x, y) => {
                 generic::TransformOperation::Scale3D(x, y, 1.)
             },
             generic::TransformOperation::ScaleX(x) => {
@@ -494,7 +490,7 @@ impl ToAnimatedZero for TransformOperation {
                 Ok(generic::TransformOperation::Scale3D(1.0, 1.0, 1.0))
             },
             generic::TransformOperation::Scale(_, _) => {
-                Ok(generic::TransformOperation::Scale(1.0, Some(1.0)))
+                Ok(generic::TransformOperation::Scale(1.0, 1.0))
             },
             generic::TransformOperation::ScaleX(..) => Ok(generic::TransformOperation::ScaleX(1.0)),
             generic::TransformOperation::ScaleY(..) => Ok(generic::TransformOperation::ScaleY(1.0)),
@@ -585,7 +581,7 @@ impl Translate {
         match *self {
             generic::Translate::None => None,
             generic::Translate::Translate(tx, ty) => {
-                Some(generic::TransformOperation::Translate(tx, Some(ty)))
+                Some(generic::TransformOperation::Translate(tx, ty))
             },
             generic::Translate::Translate3D(tx, ty, tz) => {
                 Some(generic::TransformOperation::Translate3D(tx, ty, tz))
@@ -596,7 +592,7 @@ impl Translate {
     /// Convert Translate to TransformOperation.
     pub fn from_transform_operation(operation: &TransformOperation) -> Translate {
         match *operation {
-            generic::TransformOperation::Translate(tx, Some(ty)) => {
+            generic::TransformOperation::Translate(tx, ty) => {
                 generic::Translate::Translate(tx, ty)
             },
             generic::TransformOperation::Translate3D(tx, ty, tz) => {
@@ -615,7 +611,7 @@ impl Scale {
     pub fn to_transform_operation(&self) -> Option<TransformOperation> {
         match *self {
             generic::Scale::None => None,
-            generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, Some(sy))),
+            generic::Scale::Scale(sx, sy) => Some(generic::TransformOperation::Scale(sx, sy)),
             generic::Scale::Scale3D(sx, sy, sz) => {
                 Some(generic::TransformOperation::Scale3D(sx, sy, sz))
             },
@@ -625,8 +621,7 @@ impl Scale {
     /// Convert Scale to TransformOperation.
     pub fn from_transform_operation(operation: &TransformOperation) -> Scale {
         match *operation {
-            generic::TransformOperation::Scale(sx, Some(sy)) => generic::Scale::Scale(sx, sy),
-            generic::TransformOperation::Scale(sx, None) => generic::Scale::Scale(sx, sx),
+            generic::TransformOperation::Scale(sx, sy) => generic::Scale::Scale(sx, sy),
             generic::TransformOperation::Scale3D(sx, sy, sz) => generic::Scale::Scale3D(sx, sy, sz),
             _ => unreachable!("Found unexpected value for scale"),
         }
diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs
index b0a121fd6e9..15e0ea2d274 100644
--- a/components/style/values/generics/transform.rs
+++ b/components/style/values/generics/transform.rs
@@ -106,9 +106,18 @@ impl<H, V, D> TransformOrigin<H, V, D> {
     }
 }
 
+fn is_same<N: PartialEq>(x: &N, y: &N) -> bool {
+    x == y
+}
+
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss)]
 /// A single operation in the list of a `transform` value
-pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage> {
+pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage>
+where
+    Angle: Zero,
+    LengthPercentage: Zero,
+    Number: PartialEq,
+{
     /// Represents a 2D 2x3 matrix.
     Matrix(Matrix<Number>),
     /// Represents a 3D 4x4 matrix.
@@ -119,7 +128,7 @@ pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage> {
     ///
     /// Syntax can be skew(angle) or skew(angle, angle)
     #[css(comma, function)]
-    Skew(Angle, Option<Angle>),
+    Skew(Angle, #[css(skip_if = "Zero::is_zero")] Angle),
     /// skewX(angle)
     #[css(function = "skewX")]
     SkewX(Angle),
@@ -128,7 +137,7 @@ pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage> {
     SkewY(Angle),
     /// translate(x, y) or translate(x)
     #[css(comma, function)]
-    Translate(LengthPercentage, Option<LengthPercentage>),
+    Translate(LengthPercentage, #[css(skip_if = "Zero::is_zero")] LengthPercentage),
     /// translateX(x)
     #[css(function = "translateX")]
     TranslateX(LengthPercentage),
@@ -143,14 +152,9 @@ pub enum TransformOperation<Angle, Number, Length, Integer, LengthPercentage> {
     Translate3D(LengthPercentage, LengthPercentage, Length),
     /// A 2D scaling factor.
     ///
-    /// `scale(2)` is parsed as `Scale(Number::new(2.0), None)` and is equivalent to
-    /// writing `scale(2, 2)` (`Scale(Number::new(2.0), Some(Number::new(2.0)))`).
-    ///
-    /// Negative values are allowed and flip the element.
-    ///
     /// Syntax can be scale(factor) or scale(factor, factor)
     #[css(comma, function)]
-    Scale(Number, Option<Number>),
+    Scale(Number, #[css(contextual_skip_if = "is_same")] Number),
     /// scaleX(factor)
     #[css(function = "scaleX")]
     ScaleX(Number),
@@ -214,6 +218,10 @@ pub struct Transform<T>(#[css(if_empty = "none", iterable)] pub Vec<T>);
 
 impl<Angle, Number, Length, Integer, LengthPercentage>
     TransformOperation<Angle, Number, Length, Integer, LengthPercentage>
+where
+    Angle: Zero,
+    LengthPercentage: Zero,
+    Number: PartialEq,
 {
     /// Check if it is any rotate function.
     pub fn is_rotate(&self) -> bool {
@@ -333,10 +341,10 @@ impl ToRadians for SpecifiedAngle {
 impl<Angle, Number, Length, Integer, LoP> ToMatrix
     for TransformOperation<Angle, Number, Length, Integer, LoP>
 where
-    Angle: ToRadians + Copy,
-    Number: Copy + Into<f32> + Into<f64>,
+    Angle: Zero + ToRadians + Copy,
+    Number: PartialEq + Copy + Into<f32> + Into<f64>,
     Length: ToAbsoluteLength,
-    LoP: ToAbsoluteLength,
+    LoP: Zero + ToAbsoluteLength,
 {
     #[inline]
     fn is_3d(&self) -> bool {
@@ -389,7 +397,7 @@ where
                 m.cast()
             },
             Scale3D(sx, sy, sz) => Transform3D::create_scale(sx.into(), sy.into(), sz.into()),
-            Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.unwrap_or(sx).into(), 1.),
+            Scale(sx, sy) => Transform3D::create_scale(sx.into(), sy.into(), 1.),
             ScaleX(s) => Transform3D::create_scale(s.into(), 1., 1.),
             ScaleY(s) => Transform3D::create_scale(1., s.into(), 1.),
             ScaleZ(s) => Transform3D::create_scale(1., 1., s.into()),
@@ -398,12 +406,12 @@ where
                 let ty = ty.to_pixel_length(reference_height)? as f64;
                 Transform3D::create_translation(tx, ty, tz.to_pixel_length(None)? as f64)
             },
-            Translate(ref tx, Some(ref ty)) => {
+            Translate(ref tx, ref ty) => {
                 let tx = tx.to_pixel_length(reference_width)? as f64;
                 let ty = ty.to_pixel_length(reference_height)? as f64;
                 Transform3D::create_translation(tx, ty, 0.)
             },
-            TranslateX(ref t) | Translate(ref t, None) => {
+            TranslateX(ref t) => {
                 let t = t.to_pixel_length(reference_width)? as f64;
                 Transform3D::create_translation(t, 0., 0.)
             },
@@ -416,7 +424,7 @@ where
             },
             Skew(theta_x, theta_y) => Transform3D::create_skew(
                 euclid::Angle::radians(theta_x.radians64()),
-                euclid::Angle::radians(theta_y.map_or(0., |a| a.radians64())),
+                euclid::Angle::radians(theta_y.radians64()),
             ),
             SkewX(theta) => Transform3D::create_skew(
                 euclid::Angle::radians(theta.radians64()),
diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs
index 98233608ebb..01eb58368b9 100644
--- a/components/style/values/specified/angle.rs
+++ b/components/style/values/specified/angle.rs
@@ -9,6 +9,7 @@ use crate::values::computed::angle::Angle as ComputedAngle;
 use crate::values::computed::{Context, ToComputedValue};
 use crate::values::specified::calc::CalcNode;
 use crate::values::CSSFloat;
+use crate::Zero;
 use cssparser::{Parser, Token};
 use std::f32::consts::PI;
 use std::fmt::{self, Write};
@@ -32,6 +33,21 @@ pub enum AngleDimension {
     Turn(CSSFloat),
 }
 
+impl Zero for AngleDimension {
+    fn zero() -> Self {
+        AngleDimension::Deg(0.)
+    }
+
+    fn is_zero(&self) -> bool {
+        match *self {
+            AngleDimension::Deg(ref f) |
+            AngleDimension::Grad(ref f) |
+            AngleDimension::Rad(ref f) |
+            AngleDimension::Turn(ref f) => *f == 0.,
+        }
+    }
+}
+
 impl AngleDimension {
     /// Returns the amount of degrees this angle represents.
     #[inline]
@@ -58,6 +74,19 @@ pub struct Angle {
     was_calc: bool,
 }
 
+impl Zero for Angle {
+    fn zero() -> Self {
+        Self {
+            value: Zero::zero(),
+            was_calc: false,
+        }
+    }
+
+    fn is_zero(&self) -> bool {
+        self.value.is_zero()
+    }
+}
+
 impl ToCss for Angle {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs
index 4758a0c4c05..92ca953159f 100644
--- a/components/style/values/specified/transform.rs
+++ b/components/style/values/specified/transform.rs
@@ -109,9 +109,9 @@ impl Transform {
                         let sx = specified::LengthPercentage::parse(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let sy = specified::LengthPercentage::parse(context, input)?;
-                            Ok(generic::TransformOperation::Translate(sx, Some(sy)))
+                            Ok(generic::TransformOperation::Translate(sx, sy))
                         } else {
-                            Ok(generic::TransformOperation::Translate(sx, None))
+                            Ok(generic::TransformOperation::Translate(sx, Zero::zero()))
                         }
                     },
                     "translatex" => {
@@ -138,9 +138,9 @@ impl Transform {
                         let sx = Number::parse(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let sy = Number::parse(context, input)?;
-                            Ok(generic::TransformOperation::Scale(sx, Some(sy)))
+                            Ok(generic::TransformOperation::Scale(sx, sy))
                         } else {
-                            Ok(generic::TransformOperation::Scale(sx, None))
+                            Ok(generic::TransformOperation::Scale(sx, sx))
                         }
                     },
                     "scalex" => {
@@ -194,9 +194,9 @@ impl Transform {
                         let ax = specified::Angle::parse_with_unitless(context, input)?;
                         if input.try(|input| input.expect_comma()).is_ok() {
                             let ay = specified::Angle::parse_with_unitless(context, input)?;
-                            Ok(generic::TransformOperation::Skew(ax, Some(ay)))
+                            Ok(generic::TransformOperation::Skew(ax, ay))
                         } else {
-                            Ok(generic::TransformOperation::Skew(ax, None))
+                            Ok(generic::TransformOperation::Skew(ax, Zero::zero()))
                         }
                     },
                     "skewx" => {