Auto merge of #19005 - emilio:less-mako-more-joy, r=jdm

style: Move animation-name and animation-iteration-count outside of mako.

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

components/style/animation.rs

@@ -13,7 +13,6 @@ use font_metrics::FontMetricsProvider;
 use properties::{self, CascadeFlags, ComputedValues, LonghandId};
 use properties::animated_properties::{AnimatedProperty, TransitionProperty};
 use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
-use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
 use properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState;
 use rule_tree::CascadeLevel;
 use servo_arc::Arc;
@@ -22,6 +21,7 @@ use stylesheets::keyframes_rule::{KeyframesStep, KeyframesStepValue};
 use timer::Timer;
 use values::computed::Time;
 use values::computed::transform::TimingFunction;
+use values::generics::box_::AnimationIterationCount;
 use values::generics::transform::{StepPosition, TimingFunction as GenericTimingFunction};
 
 /// This structure represents a keyframes animation current iteration state.

components/style/properties/gecko.mako.rs

@@ -3261,15 +3261,16 @@ fn static_assert() {
                              data.longhands_by_name["animation-play-state"].keyword)}
 
     pub fn set_animation_iteration_count<I>(&mut self, v: I)
-        where I: IntoIterator<Item = longhands::animation_iteration_count::computed_value::single_value::T>,
+    where
+        I: IntoIterator<Item = values::computed::AnimationIterationCount>,
         I::IntoIter: ExactSizeIterator + Clone
     {
         use std::f32;
-        use properties::longhands::animation_iteration_count::single_value::SpecifiedValue as AnimationIterationCount;
+        use values::generics::box_::AnimationIterationCount;
 
         let v = v.into_iter();
 
-        debug_assert!(v.len() != 0);
+        debug_assert_ne!(v.len(), 0);
         let input_len = v.len();
         unsafe { self.gecko.mAnimations.ensure_len(input_len) };
 
@@ -3281,10 +3282,12 @@ fn static_assert() {
             }
         }
     }
-    pub fn animation_iteration_count_at(&self, index: usize)
-        -> longhands::animation_iteration_count::computed_value::SingleComputedValue {
-        use properties::longhands::animation_iteration_count::single_value::computed_value::T
-            as AnimationIterationCount;
+
+    pub fn animation_iteration_count_at(
+        &self,
+        index: usize,
+    ) -> values::computed::AnimationIterationCount {
+        use values::generics::box_::AnimationIterationCount;
 
         if self.gecko.mAnimations[index].mIterationCount.is_infinite() {
             AnimationIterationCount::Infinite
@@ -3292,6 +3295,7 @@ fn static_assert() {
             AnimationIterationCount::Number(self.gecko.mAnimations[index].mIterationCount)
         }
     }
+
     ${impl_animation_count('iteration_count', 'IterationCount')}
     ${impl_copy_animation_value('iteration_count', 'IterationCount')}
 

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

@@ -444,78 +444,19 @@ ${helpers.predefined_type("transition-delay",
                           extra_prefixes="moz webkit",
                           spec="https://drafts.csswg.org/css-transitions/#propdef-transition-delay")}
 
-<%helpers:vector_longhand name="animation-name"
-                          need_index="True"
+
+${helpers.predefined_type(
+    "animation-name",
+    "AnimationName",
+    "computed::AnimationName::none()",
+    initial_specified_value="specified::AnimationName::none()",
+    vector=True,
+    need_index=True,
     animation_value_type="none",
-                          extra_prefixes="moz webkit"
-                          allowed_in_keyframe_block="False"
-                          spec="https://drafts.csswg.org/css-animations/#propdef-animation-name">
-    use Atom;
-    use std::fmt;
-    use style_traits::ToCss;
-    use values::KeyframesName;
-
-    pub mod computed_value {
-        pub use super::SpecifiedValue as T;
-    }
-
-    #[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue)]
-    pub struct SpecifiedValue(pub Option<KeyframesName>);
-
-    impl SpecifiedValue {
-        /// As an Atom
-        pub fn as_atom(&self) -> Option< &Atom> {
-            self.0.as_ref().map(|n| n.as_atom())
-        }
-    }
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        get_initial_specified_value()
-    }
-
-    #[inline]
-    pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue(None)
-    }
-
-    impl fmt::Display for SpecifiedValue {
-        fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            self.to_css(f)
-        }
-    }
-
-    impl ToCss for SpecifiedValue {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            if let Some(ref name) = self.0 {
-                name.to_css(dest)
-            } else {
-                dest.write_str("none")
-            }
-        }
-    }
-
-    impl Parse for SpecifiedValue {
-        fn parse<'i, 't>(
-            context: &ParserContext,
-            input: &mut ::cssparser::Parser<'i, 't>
-        ) -> Result<Self, ParseError<'i>> {
-            if let Ok(name) = input.try(|input| KeyframesName::parse(context, input)) {
-                Ok(SpecifiedValue(Some(name)))
-            } else {
-                input.expect_ident_matching("none").map(|_| SpecifiedValue(None)).map_err(|e| e.into())
-            }
-        }
-    }
-
-    pub fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<SpecifiedValue,ParseError<'i>> {
-        SpecifiedValue::parse(context, input)
-    }
-
-</%helpers:vector_longhand>
+    extra_prefixes="moz webkit",
+    allowed_in_keyframe_block=False,
+    spec="https://drafts.csswg.org/css-animations/#propdef-animation-name",
+)}
 
 ${helpers.predefined_type("animation-duration",
                           "Time",
@@ -541,58 +482,18 @@ ${helpers.predefined_type("animation-timing-function",
                           allowed_in_keyframe_block=True,
                           spec="https://drafts.csswg.org/css-transitions/#propdef-animation-timing-function")}
 
-<%helpers:vector_longhand name="animation-iteration-count"
-                          need_index="True"
+${helpers.predefined_type(
+    "animation-iteration-count",
+    "AnimationIterationCount",
+    "computed::AnimationIterationCount::one()",
+    initial_specified_value="specified::AnimationIterationCount::one()",
+    vector=True,
+    need_index=True,
     animation_value_type="none",
-                          extra_prefixes="moz webkit"
+    extra_prefixes="moz webkit",
+    allowed_in_keyframe_block=False,
     spec="https://drafts.csswg.org/css-animations/#propdef-animation-iteration-count",
-                          allowed_in_keyframe_block="False">
-    pub mod computed_value {
-        pub use super::SpecifiedValue as T;
-    }
-
-    // https://drafts.csswg.org/css-animations/#animation-iteration-count
-    #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss, ToComputedValue)]
-    pub enum SpecifiedValue {
-        Number(f32),
-        Infinite,
-    }
-
-    impl Parse for SpecifiedValue {
-        fn parse<'i, 't>(_context: &ParserContext, input: &mut ::cssparser::Parser<'i, 't>)
-                         -> Result<Self, ParseError<'i>> {
-            if input.try(|input| input.expect_ident_matching("infinite")).is_ok() {
-                return Ok(SpecifiedValue::Infinite)
-            }
-
-            let number = input.expect_number()?;
-            if number < 0.0 {
-                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-            }
-
-            Ok(SpecifiedValue::Number(number))
-        }
-    }
-
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        get_initial_specified_value()
-    }
-
-    #[inline]
-    pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue::Number(1.0)
-    }
-
-    #[inline]
-    pub fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<SpecifiedValue, ParseError<'i>> {
-        SpecifiedValue::parse(context, input)
-    }
-</%helpers:vector_longhand>
+)}
 
 <% animation_direction_custom_consts = { "alternate-reverse": "Alternate_reverse" } %>
 ${helpers.single_keyword("animation-direction",

components/style/values/computed/box.rs

@@ -4,8 +4,23 @@
 
 //! Computed types for box properties.
 
+use values::computed::Number;
 use values::computed::length::LengthOrPercentage;
+use values::generics::box_::AnimationIterationCount as GenericAnimationIterationCount;
 use values::generics::box_::VerticalAlign as GenericVerticalAlign;
 
+pub use values::specified::box_::AnimationName;
+
 /// A computed value for the `vertical-align` property.
 pub type VerticalAlign = GenericVerticalAlign<LengthOrPercentage>;
+
+/// A computed value for the `animation-iteration-count` property.
+pub type AnimationIterationCount = GenericAnimationIterationCount<Number>;
+
+impl AnimationIterationCount {
+    /// Returns the value `1.0`.
+    #[inline]
+    pub fn one() -> Self {
+        GenericAnimationIterationCount::Number(1.0)
+    }
+}

components/style/values/computed/mod.rs

@@ -36,7 +36,7 @@ pub use self::angle::Angle;
 pub use self::background::{BackgroundSize, BackgroundRepeat};
 pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageSideWidth};
 pub use self::border::{BorderRadius, BorderCornerRadius, BorderSpacing};
-pub use self::box_::VerticalAlign;
+pub use self::box_::{AnimationIterationCount, AnimationName, VerticalAlign};
 pub use self::color::{Color, ColorPropertyValue, RGBAColor};
 pub use self::effects::{BoxShadow, Filter, SimpleShadow};
 pub use self::flex::FlexBasis;

components/style/values/generics/box.rs

@@ -46,3 +46,12 @@ impl<L> ToAnimatedZero for VerticalAlign<L> {
         Err(())
     }
 }
+
+/// https://drafts.csswg.org/css-animations/#animation-iteration-count
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
+pub enum AnimationIterationCount<Number> {
+    /// A `<number>` value.
+    Number(Number),
+    /// The `infinite` keyword.
+    Infinite,
+}

components/style/values/specified/box.rs

@@ -4,11 +4,15 @@
 
 //! Specified types for box properties.
 
+use Atom;
 use cssparser::Parser;
 use parser::{Parse, ParserContext};
-use style_traits::ParseError;
+use std::fmt;
+use style_traits::{ParseError, ToCss};
+use values::KeyframesName;
+use values::generics::box_::AnimationIterationCount as GenericAnimationIterationCount;
 use values::generics::box_::VerticalAlign as GenericVerticalAlign;
-use values::specified::AllowQuirks;
+use values::specified::{AllowQuirks, Number};
 use values::specified::length::LengthOrPercentage;
 
 /// A specified value for the `vertical-align` property.
@@ -39,3 +43,70 @@ impl Parse for VerticalAlign {
         }
     }
 }
+
+/// https://drafts.csswg.org/css-animations/#animation-iteration-count
+pub type AnimationIterationCount = GenericAnimationIterationCount<Number>;
+
+impl Parse for AnimationIterationCount {
+    fn parse<'i, 't>(
+        context: &ParserContext,
+        input: &mut ::cssparser::Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        if input.try(|input| input.expect_ident_matching("infinite")).is_ok() {
+            return Ok(GenericAnimationIterationCount::Infinite)
+        }
+
+        let number = Number::parse_non_negative(context, input)?;
+        Ok(GenericAnimationIterationCount::Number(number))
+    }
+}
+
+impl AnimationIterationCount {
+    /// Returns the value `1.0`.
+    #[inline]
+    pub fn one() -> Self {
+        GenericAnimationIterationCount::Number(Number::new(1.0))
+    }
+}
+
+/// A value for the `animation-name` property.
+#[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue)]
+pub struct AnimationName(pub Option<KeyframesName>);
+
+impl AnimationName {
+    /// Get the name of the animation as an `Atom`.
+    pub fn as_atom(&self) -> Option<&Atom> {
+        self.0.as_ref().map(|n| n.as_atom())
+    }
+
+    /// Returns the `none` value.
+    pub fn none() -> Self {
+        AnimationName(None)
+    }
+}
+
+impl ToCss for AnimationName {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        match self.0 {
+            Some(ref name) => name.to_css(dest),
+            None => dest.write_str("none"),
+        }
+    }
+}
+
+impl Parse for AnimationName {
+    fn parse<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>
+    ) -> Result<Self, ParseError<'i>> {
+        if let Ok(name) = input.try(|input| KeyframesName::parse(context, input)) {
+            return Ok(AnimationName(Some(name)));
+        }
+
+        input.expect_ident_matching("none")?;
+        Ok(AnimationName(None))
+    }
+}

components/style/values/specified/mod.rs

@@ -30,7 +30,7 @@ pub use self::align::{AlignItems, AlignJustifyContent, AlignJustifySelf, Justify
 pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth};
 pub use self::border::{BorderImageSideWidth, BorderRadius, BorderSideWidth, BorderSpacing};
-pub use self::box_::VerticalAlign;
+pub use self::box_::{AnimationIterationCount, AnimationName, VerticalAlign};
 pub use self::color::{Color, ColorPropertyValue, RGBAColor};
 pub use self::effects::{BoxShadow, Filter, SimpleShadow};
 pub use self::flex::FlexBasis;
@@ -176,8 +176,10 @@ impl BorderStyle {
     }
 }
 
+/// A CSS `<number>` specified value.
+///
+/// https://drafts.csswg.org/css-values-3/#number-value
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, PartialOrd)]
-#[allow(missing_docs)]
 pub struct Number {
     /// The numeric value itself.
     value: CSSFloat,

tests/unit/style/parsing/animation.rs

@@ -5,9 +5,9 @@
 use parsing::parse;
 use servo_atoms::Atom;
 use style::parser::Parse;
-use style::properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
 use style::properties::longhands::animation_name;
 use style::values::{KeyframesName, CustomIdent};
+use style::values::specified::AnimationIterationCount;
 use style_traits::ToCss;
 
 #[test]

tests/wpt/metadata/MANIFEST.json

@@ -306159,6 +306159,12 @@
      {}
     ]
    ],
+   "css/css-animations-1/animation-iteration-count-calc.html": [
+    [
+     "/css/css-animations-1/animation-iteration-count-calc.html",
+     {}
+    ]
+   ],
    "css/css-animations-1/animationevent-interface.html": [
     [
      "/css/css-animations-1/animationevent-interface.html",
@@ -474190,6 +474196,10 @@
    "812d8b618aaf90cf6d772b61d0eaed55848e4c20",
    "manual"
   ],
+  "css/css-animations-1/animation-iteration-count-calc.html": [
+   "e7558a046eb2856c666ba51c4208178b24706765",
+   "testharness"
+  ],
   "css/css-animations-1/animation-iteration-event.html": [
    "3aec5072660e45e332d5858c8870980d3a8db966",
    "manual"

tests/wpt/web-platform-tests/css/css-animations-1/animation-iteration-count-calc.html

@@ -0,0 +1,15 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Test: Animation count accepts calc()</title>
+<link rel="author" name="Emilio Cobos Álvarez" href="mailto:emilio@crisal.io">
+<link rel="help" href="https://drafts.csswg.org/css-values-3/#number-value">
+<link rel="help" href="https://drafts.csswg.org/css-animations/#animation-iteration-count">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="target" style="animation-iteration-count: calc(1 + 3)">
+<script>
+test(function() {
+  let target = document.getElementById("target");
+  assert_equals(getComputedStyle(target).animationIterationCount, "4")
+}, "calc() should be accepted in animation-iteration-count.");
+</script>