Auto merge of #14899 - canaltinova:transition-timing, r=Manishearth

Fix serialization of timing-function

<!-- Please describe your changes on the following line: -->
`transition-timing-function` and `animation-timing-function` properties corrected to reflect [the spec](https://drafts.csswg.org/css-transitions/#serializing-a-timing-function). It was converting function keywords to `cubic-bezier` or `steps` functions directly. But we needed to store it to use in the serialization step.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #14822 (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/14899)
<!-- Reviewable:end -->

components/style/properties/longhand/box.mako.rs

@@ -495,60 +495,60 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
                    need_index="True"
                    animatable="False"
                    spec="https://drafts.csswg.org/css-transitions/#propdef-transition-timing-function">
-    use self::computed_value::{StartEnd, TransitionTimingFunction};
+    use self::computed_value::StartEnd;
+    use self::computed_value::TransitionTimingFunction as ComputedTransitionTimingFunction;
 
     use euclid::point::{Point2D, TypedPoint2D};
+    use std::fmt;
     use std::marker::PhantomData;
-    use values::computed::ComputedValueAsSpecified;
+    use style_traits::ToCss;
 
-    pub use self::computed_value::SingleComputedValue as SingleSpecifiedValue;
+    pub use self::TransitionTimingFunction as SingleSpecifiedValue;
-    pub use self::computed_value::T as SpecifiedValue;
 
     // FIXME: This could use static variables and const functions when they are available.
     #[inline(always)]
-    fn ease() -> TransitionTimingFunction {
+    fn ease() -> ComputedTransitionTimingFunction {
-        TransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.25, 0.1),
+        ComputedTransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.25, 0.1),
                                               TypedPoint2D::new(0.25, 1.0))
     }
 
     #[inline(always)]
-    fn linear() -> TransitionTimingFunction {
+    fn linear() -> ComputedTransitionTimingFunction {
-        TransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.0, 0.0),
+        ComputedTransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.0, 0.0),
                                               TypedPoint2D::new(1.0, 1.0))
     }
 
     #[inline(always)]
-    fn ease_in() -> TransitionTimingFunction {
+    fn ease_in() -> ComputedTransitionTimingFunction {
-        TransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.42, 0.0),
+        ComputedTransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.42, 0.0),
                                               TypedPoint2D::new(1.0, 1.0))
     }
 
     #[inline(always)]
-    fn ease_out() -> TransitionTimingFunction {
+    fn ease_out() -> ComputedTransitionTimingFunction {
-        TransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.0, 0.0),
+        ComputedTransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.0, 0.0),
                                               TypedPoint2D::new(0.58, 1.0))
     }
 
     #[inline(always)]
-    fn ease_in_out() -> TransitionTimingFunction {
+    fn ease_in_out() -> ComputedTransitionTimingFunction {
-        TransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.42, 0.0),
+        ComputedTransitionTimingFunction::CubicBezier(TypedPoint2D::new(0.42, 0.0),
                                               TypedPoint2D::new(0.58, 1.0))
     }
 
-    static STEP_START: TransitionTimingFunction =
+    static STEP_START: ComputedTransitionTimingFunction =
-        TransitionTimingFunction::Steps(1, StartEnd::Start);
+        ComputedTransitionTimingFunction::Steps(1, StartEnd::Start);
-    static STEP_END: TransitionTimingFunction =
+    static STEP_END: ComputedTransitionTimingFunction =
-        TransitionTimingFunction::Steps(1, StartEnd::End);
+        ComputedTransitionTimingFunction::Steps(1, StartEnd::End);
 
     pub mod computed_value {
         use euclid::point::Point2D;
         use parser::{Parse, ParserContext};
         use std::fmt;
         use style_traits::ToCss;
-        use values::specified;
-        use values::computed::ComputedValueAsSpecified;
 
         pub use self::TransitionTimingFunction as SingleComputedValue;
+        pub use super::parse;
 
         #[derive(Copy, Clone, Debug, PartialEq)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@@ -557,58 +557,6 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
             Steps(u32, StartEnd),
         }
 
-        impl Parse for TransitionTimingFunction {
-            fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<Self, ()> {
-                if let Ok(function_name) = input.try(|input| input.expect_function()) {
-                    return match_ignore_ascii_case! { function_name,
-                        "cubic-bezier" => {
-                            let (mut p1x, mut p1y, mut p2x, mut p2y) = (0.0, 0.0, 0.0, 0.0);
-                            try!(input.parse_nested_block(|input| {
-                                p1x = try!(specified::parse_number(input));
-                                try!(input.expect_comma());
-                                p1y = try!(specified::parse_number(input));
-                                try!(input.expect_comma());
-                                p2x = try!(specified::parse_number(input));
-                                try!(input.expect_comma());
-                                p2y = try!(specified::parse_number(input));
-                                Ok(())
-                            }));
-                            let (p1, p2) = (Point2D::new(p1x, p1y), Point2D::new(p2x, p2y));
-                            Ok(TransitionTimingFunction::CubicBezier(p1, p2))
-                        },
-                        "steps" => {
-                            let (mut step_count, mut start_end) = (0, StartEnd::End);
-                            try!(input.parse_nested_block(|input| {
-                                step_count = try!(specified::parse_integer(input));
-                                if input.try(|input| input.expect_comma()).is_ok() {
-                                    start_end = try!(match_ignore_ascii_case! {
-                                        try!(input.expect_ident()),
-                                        "start" => Ok(StartEnd::Start),
-                                        "end" => Ok(StartEnd::End),
-                                        _ => Err(())
-                                    });
-                                }
-                                Ok(())
-                            }));
-                            Ok(TransitionTimingFunction::Steps(step_count as u32, start_end))
-                        },
-                        _ => Err(())
-                    }
-                }
-                match_ignore_ascii_case! {
-                    try!(input.expect_ident()),
-                    "ease" => Ok(super::ease()),
-                    "linear" => Ok(super::linear()),
-                    "ease-in" => Ok(super::ease_in()),
-                    "ease-out" => Ok(super::ease_out()),
-                    "ease-in-out" => Ok(super::ease_in_out()),
-                    "step-start" => Ok(super::STEP_START),
-                    "step-end" => Ok(super::STEP_END),
-                    _ => Err(())
-                }
-            }
-        }
-
         impl ToCss for TransitionTimingFunction {
             fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
                 match *self {
@@ -624,11 +572,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
                         dest.write_str(")")
                     }
                     TransitionTimingFunction::Steps(steps, start_end) => {
-                        try!(dest.write_str("steps("));
+                        super::serialize_steps(dest, steps, start_end)
-                        try!(steps.to_css(dest));
-                        try!(dest.write_str(", "));
-                        try!(start_end.to_css(dest));
-                        dest.write_str(")")
                     }
                 }
             }
@@ -670,14 +614,192 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
         }
     }
 
+    define_css_keyword_enum!(FunctionKeyword:
+                             "ease" => Ease,
+                             "linear" => Linear,
+                             "ease-in" => EaseIn,
+                             "ease-out" => EaseOut,
+                             "ease-in-out" => EaseInOut,
+                             "step-start" => StepStart,
+                             "step-end" => StepEnd);
+
+    #[derive(Copy, Clone, Debug, PartialEq)]
+    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+    pub enum TransitionTimingFunction {
+        CubicBezier(Point2D<f32>, Point2D<f32>),
+        Steps(u32, StartEnd),
+        Keyword(FunctionKeyword),
+    }
+
+    #[derive(Clone, Debug, PartialEq)]
+    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+    pub struct SpecifiedValue(pub Vec<TransitionTimingFunction>);
+
+    impl Parse for TransitionTimingFunction {
+        fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<Self, ()> {
+            if let Ok(function_name) = input.try(|input| input.expect_function()) {
+                return match_ignore_ascii_case! { function_name,
+                    "cubic-bezier" => {
+                        let (mut p1x, mut p1y, mut p2x, mut p2y) = (0.0, 0.0, 0.0, 0.0);
+                        try!(input.parse_nested_block(|input| {
+                            p1x = try!(specified::parse_number(input));
+                            try!(input.expect_comma());
+                            p1y = try!(specified::parse_number(input));
+                            try!(input.expect_comma());
+                            p2x = try!(specified::parse_number(input));
+                            try!(input.expect_comma());
+                            p2y = try!(specified::parse_number(input));
+                            Ok(())
+                        }));
+                        let (p1, p2) = (Point2D::new(p1x, p1y), Point2D::new(p2x, p2y));
+                        Ok(TransitionTimingFunction::CubicBezier(p1, p2))
+                    },
+                    "steps" => {
+                        let (mut step_count, mut start_end) = (0, StartEnd::End);
+                        try!(input.parse_nested_block(|input| {
+                            step_count = try!(specified::parse_integer(input));
+                            if input.try(|input| input.expect_comma()).is_ok() {
+                                start_end = try!(match_ignore_ascii_case! {
+                                    try!(input.expect_ident()),
+                                    "start" => Ok(StartEnd::Start),
+                                    "end" => Ok(StartEnd::End),
+                                    _ => Err(())
+                                });
+                            }
+                            Ok(())
+                        }));
+                        Ok(TransitionTimingFunction::Steps(step_count as u32, start_end))
+                    },
+                    _ => Err(())
+                }
+            }
+            Ok(TransitionTimingFunction::Keyword(try!(FunctionKeyword::parse(input))))
+        }
+    }
+
+    fn serialize_steps<W>(dest: &mut W, steps: u32,
+                          start_end: StartEnd) -> fmt::Result where W: fmt::Write {
+        try!(dest.write_str("steps("));
+        try!(steps.to_css(dest));
+        if let StartEnd::Start = start_end {
+            try!(dest.write_str(", start"));
+        }
+        dest.write_str(")")
+    }
+
+    // https://drafts.csswg.org/css-transitions/#serializing-a-timing-function
+    impl ToCss for TransitionTimingFunction {
+        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            match *self {
+                TransitionTimingFunction::CubicBezier(p1, p2) => {
+                    try!(dest.write_str("cubic-bezier("));
+                    try!(p1.x.to_css(dest));
+                    try!(dest.write_str(", "));
+                    try!(p1.y.to_css(dest));
+                    try!(dest.write_str(", "));
+                    try!(p2.x.to_css(dest));
+                    try!(dest.write_str(", "));
+                    try!(p2.y.to_css(dest));
+                    dest.write_str(")")
+                },
+                TransitionTimingFunction::Steps(steps, start_end) => {
+                    serialize_steps(dest, steps, start_end)
+                },
+                TransitionTimingFunction::Keyword(keyword) => {
+                    match keyword {
+                        FunctionKeyword::StepStart => {
+                            serialize_steps(dest, 1, StartEnd::Start)
+                        },
+                        FunctionKeyword::StepEnd => {
+                            serialize_steps(dest, 1, StartEnd::End)
+                        },
+                        _ => {
+                            keyword.to_css(dest)
+                        },
+                    }
+                },
+            }
+        }
+    }
+
+    impl ToComputedValue for TransitionTimingFunction {
+        type ComputedValue = ComputedTransitionTimingFunction;
+
+        #[inline]
+        fn to_computed_value(&self, _context: &Context) -> ComputedTransitionTimingFunction {
+            match *self {
+                TransitionTimingFunction::CubicBezier(p1, p2) => {
+                    ComputedTransitionTimingFunction::CubicBezier(p1, p2)
+                },
+                TransitionTimingFunction::Steps(count, start_end) => {
+                    ComputedTransitionTimingFunction::Steps(count, start_end)
+                },
+                TransitionTimingFunction::Keyword(keyword) => {
+                    match keyword {
+                        FunctionKeyword::Ease => ease(),
+                        FunctionKeyword::Linear => linear(),
+                        FunctionKeyword::EaseIn => ease_in(),
+                        FunctionKeyword::EaseOut => ease_out(),
+                        FunctionKeyword::EaseInOut => ease_in_out(),
+                        FunctionKeyword::StepStart => STEP_START,
+                        FunctionKeyword::StepEnd => STEP_END,
+                    }
+                },
+            }
+        }
+        #[inline]
+        fn from_computed_value(computed: &ComputedTransitionTimingFunction) -> Self {
+            match *computed {
+                computed_value::TransitionTimingFunction::CubicBezier(p1, p2) => {
+                    TransitionTimingFunction::CubicBezier(p1, p2)
+                },
+                computed_value::TransitionTimingFunction::Steps(count, start_end) => {
+                    TransitionTimingFunction::Steps(count, start_end)
+                },
+            }
+        }
+    }
+
+    impl ToCss for SpecifiedValue {
+        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            if self.0.is_empty() {
+                return dest.write_str("none")
+            }
+            for (i, value) in self.0.iter().enumerate() {
+                if i != 0 {
+                    try!(dest.write_str(", "))
+                }
+                try!(value.to_css(dest))
+            }
+            Ok(())
+        }
+    }
+
+    impl ToComputedValue for SpecifiedValue {
+        type ComputedValue = computed_value::T;
+
+        #[inline]
+        fn to_computed_value(&self, context: &Context) -> computed_value::T {
+            computed_value::T(self.0.iter().map(|f| f.to_computed_value(context)).collect())
+        }
+        #[inline]
+        fn from_computed_value(computed: &computed_value::T) -> Self {
+            SpecifiedValue(computed.0.iter().map(|f| ToComputedValue::from_computed_value(f))
+                                        .collect())
+        }
+    }
+
     use values::NoViewportPercentage;
     impl NoViewportPercentage for SpecifiedValue {}
 
-    impl ComputedValueAsSpecified for SpecifiedValue {}
+    #[inline]
+    pub fn get_initial_single_value() -> ComputedTransitionTimingFunction {
+        ease()
+    }
 
     #[inline]
-    pub fn get_initial_single_value() -> TransitionTimingFunction {
+    pub fn get_initial_specified_single_value() -> TransitionTimingFunction {
-        ease()
+        ToComputedValue::from_computed_value(&ease())
     }
 
     #[inline]
@@ -848,7 +970,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
                    spec="https://drafts.csswg.org/css-animations/#propdef-animation-timing-function",
                    allowed_in_keyframe_block="False">
     pub use super::transition_timing_function::computed_value;
-    pub use super::transition_timing_function::{get_initial_value, get_initial_single_value, parse};
+    pub use super::transition_timing_function::{get_initial_value, get_initial_single_value};
+    pub use super::transition_timing_function::{get_initial_specified_single_value, parse};
     pub use super::transition_timing_function::SpecifiedValue;
     pub use super::transition_timing_function::SingleSpecifiedValue;
 </%helpers:longhand>

components/style/properties/shorthand/box.mako.rs

@@ -76,7 +76,7 @@
 macro_rules! try_parse_one {
     ($input: expr, $var: ident, $prop_module: ident) => {
         if $var.is_none() {
-            if let Ok(value) = $input.try($prop_module::computed_value::SingleComputedValue::parse) {
+            if let Ok(value) = $input.try($prop_module::SingleSpecifiedValue::parse) {
                 $var = Some(value);
                 continue;
             }
@@ -85,7 +85,7 @@ macro_rules! try_parse_one {
     ($context: expr, $input: expr, $var: ident, $prop_module: ident) => {
         if $var.is_none() {
             if let Ok(value) = $input.try(|i| {
-                $prop_module::computed_value::SingleComputedValue::parse($context, i)
+                $prop_module::SingleSpecifiedValue::parse($context, i)
             }) {
                 $var = Some(value);
                 continue;
@@ -129,7 +129,7 @@ macro_rules! try_parse_one {
                     transition_duration:
                         duration.unwrap_or_else(transition_duration::get_initial_single_value),
                     transition_timing_function:
-                        timing_function.unwrap_or_else(transition_timing_function::get_initial_single_value),
+                        timing_function.unwrap_or_else(transition_timing_function::get_initial_specified_single_value),
                     transition_delay:
                         delay.unwrap_or_else(transition_delay::get_initial_single_value),
                 })
@@ -240,7 +240,7 @@ macro_rules! try_parse_one {
                     animation_duration:
                         duration.unwrap_or_else(animation_duration::get_initial_single_value),
                     animation_timing_function:
-                        timing_function.unwrap_or_else(animation_timing_function::get_initial_single_value),
+                        timing_function.unwrap_or_else(animation_timing_function::get_initial_specified_single_value),
                     animation_delay:
                         delay.unwrap_or_else(animation_delay::get_initial_single_value),
                     animation_iteration_count:

tests/unit/style/properties/serialization.rs

@@ -14,8 +14,8 @@ pub use style_traits::ToCss;
 
 #[test]
 fn property_declaration_block_should_serialize_correctly() {
-    use style::properties::longhands::overflow_x::computed_value::T as OverflowXValue;
+    use style::properties::longhands::overflow_x::SpecifiedValue as OverflowXValue;
-    use style::properties::longhands::overflow_y::computed_value::T as OverflowYContainer;
+    use style::properties::longhands::overflow_y::SpecifiedValue as OverflowYContainer;
 
     let declarations = vec![
         (PropertyDeclaration::Width(
@@ -72,8 +72,8 @@ mod shorthand_serialization {
 
     mod overflow {
         pub use super::*;
-        use style::properties::longhands::overflow_x::computed_value::T as OverflowXValue;
+        use style::properties::longhands::overflow_x::SpecifiedValue as OverflowXValue;
-        use style::properties::longhands::overflow_y::computed_value::T as OverflowYContainer;
+        use style::properties::longhands::overflow_y::SpecifiedValue as OverflowYContainer;
 
         #[test]
         fn equal_overflow_properties_should_serialize_to_single_value() {
@@ -418,8 +418,8 @@ mod shorthand_serialization {
     }
 
     mod list_style {
-        use style::properties::longhands::list_style_position::computed_value::T as ListStylePosition;
+        use style::properties::longhands::list_style_position::SpecifiedValue as ListStylePosition;
-        use style::properties::longhands::list_style_type::computed_value::T as ListStyleType;
+        use style::properties::longhands::list_style_type::SpecifiedValue as ListStyleType;
         use style::values::Either;
         use super::*;
 
@@ -536,10 +536,10 @@ mod shorthand_serialization {
     fn transition_should_serialize_all_available_properties() {
         use euclid::point::Point2D;
         use style::properties::animated_properties::TransitionProperty;
-        use style::properties::longhands::transition_duration::computed_value::T as DurationContainer;
+        use style::properties::longhands::transition_duration::SpecifiedValue as DurationContainer;
-        use style::properties::longhands::transition_property::computed_value::T as PropertyContainer;
+        use style::properties::longhands::transition_property::SpecifiedValue as PropertyContainer;
-        use style::properties::longhands::transition_timing_function::computed_value::T as TimingContainer;
+        use style::properties::longhands::transition_timing_function::SpecifiedValue as TimingContainer;
-        use style::properties::longhands::transition_timing_function::computed_value::TransitionTimingFunction;
+        use style::properties::longhands::transition_timing_function::TransitionTimingFunction;
         use style::values::specified::Time as TimeContainer;
 
         let property_name = DeclaredValue::Value(
@@ -594,8 +594,8 @@ mod shorthand_serialization {
 
     #[test]
     fn flex_flow_should_serialize_all_available_properties() {
-        use style::properties::longhands::flex_direction::computed_value::T as FlexDirection;
+        use style::properties::longhands::flex_direction::SpecifiedValue as FlexDirection;
-        use style::properties::longhands::flex_wrap::computed_value::T as FlexWrap;
+        use style::properties::longhands::flex_wrap::SpecifiedValue as FlexWrap;
 
         let mut properties = Vec::new();
 
@@ -1018,7 +1018,7 @@ mod shorthand_serialization {
 
     mod scroll_snap_type {
         pub use super::*;
-        use style::properties::longhands::scroll_snap_type_x::computed_value::T as ScrollSnapTypeXValue;
+        use style::properties::longhands::scroll_snap_type_x::SpecifiedValue as ScrollSnapTypeXValue;
 
         #[test]
         fn should_serialize_to_empty_string_if_sub_types_not_equal() {

tests/wpt/css-tests/css-transitions-1_dev/html/transition-timing-function-001.htm

@@ -58,15 +58,15 @@
                 'ease-out': 'cubic-bezier(0, 0, 0.58, 1)',
                 'ease-in-out': 'cubic-bezier(0.42, 0, 0.58, 1)',
                 'step-start': 'steps(1, start)',
-                'step-end': 'steps(1, end)',
+                'step-end': 'steps(1)',
                 // cubic bezier
                 'cubic-bezier(0.1, 0.2, 0.3, 0.4)': 'cubic-bezier(0.1, 0.2, 0.3, 0.4)',
                 'cubic-bezier(0.1, -0.2, 0.3, -0.4)': 'cubic-bezier(0.1, -0.2, 0.3, -0.4)',
                 'cubic-bezier(0.1, 1.2, 0.3, 1.4)': 'cubic-bezier(0.1, 1.2, 0.3, 1.4)',
                 // steps
                 'steps(3, start)': 'steps(3, start)',
-                'steps(3, end)': 'steps(3, end)',
+                'steps(3, end)': 'steps(3)',
-                'steps(3)': 'steps(3, end)',
+                'steps(3)': 'steps(3)',
                 // invalid
                 'cubic-bezier(foobar)': defaultValue,
                 'steps(foobar)': defaultValue,