style: Parse and serialize NaN and infinity times correctly

Times like NaN and infinity are now parsed and serialized correctly.

Rewrote time to use `calc_clamping_mode` instead of `was_calc` to
stop some parse-time clamping which broke these values.

Adjusted WPT test expectations, 19 newly pass 🎉

Also added a new WPT test to cover a missing edge case:
`calc(1<unit> * NaN)` -> `calc(NaN * 1<canonical_unit>)`

The very similar angle tests do this already for angle's units.

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

components/style/values/specified/calc.rs

@@ -245,7 +245,7 @@ impl generic::CalcNodeLeaf for Leaf {
                 *a = Angle::from_calc(a.degrees() * scalar);
             },
             Self::Time(ref mut t) => {
-                *t = Time::from_calc(t.seconds() * scalar);
+                *t = Time::from_seconds(t.seconds() * scalar);
             },
             Self::Percentage(ref mut p) => {
                 *p *= scalar;
@@ -334,7 +334,7 @@ impl generic::CalcNodeLeaf for Leaf {
                 *one = specified::Angle::from_calc(one.degrees() + other.degrees());
             },
             (&mut Time(ref mut one), &Time(ref other)) => {
-                *one = specified::Time::from_calc(one.seconds() + other.seconds());
+                *one = specified::Time::from_seconds(one.seconds() + other.seconds());
             },
             (&mut Length(ref mut one), &Length(ref other)) => {
                 *one = one.try_op(other, std::ops::Add::add)?;
@@ -376,7 +376,7 @@ impl generic::CalcNodeLeaf for Leaf {
                 ))));
             },
             (&Time(ref one), &Time(ref other)) => {
-                return Ok(Leaf::Time(specified::Time::from_calc(op(
+                return Ok(Leaf::Time(specified::Time::from_seconds(op(
                     one.seconds(),
                     other.seconds(),
                 ))));
@@ -428,7 +428,7 @@ impl CalcNode {
                     }
                 }
                 if allowed_units.intersects(CalcUnits::TIME) {
-                    if let Ok(t) = Time::parse_dimension(value, unit, /* from_calc = */ true) {
+                    if let Ok(t) = Time::parse_dimension(value, unit) {
                         return Ok(CalcNode::Leaf(Leaf::Time(t)));
                     }
                 }
@@ -596,8 +596,8 @@ impl CalcNode {
                             return Ok(a.atan2(b));
                         }
 
-                        if let Ok(a) = a.to_time() {
-                            let b = b.to_time()?;
+                        if let Ok(a) = a.to_time(None) {
+                            let b = b.to_time(None)?;
                             return Ok(a.seconds().atan2(b.seconds()));
                         }
 
@@ -822,12 +822,21 @@ impl CalcNode {
     }
 
     /// Tries to simplify this expression into a `<time>` value.
-    fn to_time(&self) -> Result<Time, ()> {
+    fn to_time(
+        &self,
+        clamping_mode: Option<AllowedNumericType>
+    ) -> Result<Time, ()> {
         let seconds = self.resolve(|leaf| match *leaf {
-            Leaf::Time(ref t) => Ok(t.seconds()),
+            Leaf::Time(ref time) => Ok(time.seconds()),
             _ => Err(()),
         })?;
-        Ok(Time::from_calc(crate::values::normalize(seconds)))
+
+        let result = Time::from_seconds_with_calc_clamping_mode(if nan_inf_enabled() {
+            seconds
+        } else {
+            crate::values::normalize(seconds)
+        }, clamping_mode);
+        Ok(result)
     }
 
     /// Tries to simplify this expression into an `Angle` value.
@@ -967,10 +976,11 @@ impl CalcNode {
     pub fn parse_time<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
+        clamping_mode: AllowedNumericType,
         function: MathFunction,
     ) -> Result<Time, ParseError<'i>> {
         Self::parse(context, input, function, CalcUnits::TIME)?
-            .to_time()
+            .to_time(Some(clamping_mode))
             .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 

components/style/values/specified/time.rs

@@ -19,7 +19,7 @@ use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKin
 pub struct Time {
     seconds: CSSFloat,
     unit: TimeUnit,
-    was_calc: bool,
+    calc_clamping_mode: Option<AllowedNumericType>,
 }
 
 /// A time unit.
@@ -33,14 +33,19 @@ pub enum TimeUnit {
 
 impl Time {
     /// Returns a time value that represents `seconds` seconds.
-    pub fn from_seconds(seconds: CSSFloat) -> Self {
+    pub fn from_seconds_with_calc_clamping_mode(seconds: CSSFloat, calc_clamping_mode: Option<AllowedNumericType>) -> Self {
         Time {
             seconds,
             unit: TimeUnit::Second,
-            was_calc: false,
+            calc_clamping_mode,
         }
     }
 
+    /// Returns a time value that represents `seconds` seconds.
+    pub fn from_seconds(seconds: CSSFloat) -> Self {
+        Self::from_seconds_with_calc_clamping_mode(seconds, None)
+    }
+
     /// Returns `0s`.
     pub fn zero() -> Self {
         Self::from_seconds(0.0)
@@ -51,8 +56,25 @@ impl Time {
         self.seconds
     }
 
+    /// Returns the unit of the time.
+    #[inline]
+    pub fn unit(&self) -> &'static str {
+        match self.unit {
+            TimeUnit::Second => "s",
+            TimeUnit::Millisecond => "ms"
+        }
+    }
+
+    #[inline]
+    fn unitless_value(&self) -> CSSFloat {
+        match self.unit {
+            TimeUnit::Second => self.seconds,
+            TimeUnit::Millisecond => self.seconds * 1000.
+        }
+    }
+
     /// Parses a time according to CSS-VALUES § 6.2.
-    pub fn parse_dimension(value: CSSFloat, unit: &str, was_calc: bool) -> Result<Time, ()> {
+    pub fn parse_dimension(value: CSSFloat, unit: &str) -> Result<Time, ()> {
         let (seconds, unit) = match_ignore_ascii_case! { unit,
             "s" => (value, TimeUnit::Second),
             "ms" => (value / 1000.0, TimeUnit::Millisecond),
@@ -62,19 +84,10 @@ impl Time {
         Ok(Time {
             seconds,
             unit,
-            was_calc,
+            calc_clamping_mode: None,
         })
     }
 
-    /// Returns a `Time` value from a CSS `calc()` expression.
-    pub fn from_calc(seconds: CSSFloat) -> Self {
-        Time {
-            seconds,
-            unit: TimeUnit::Second,
-            was_calc: true,
-        }
-    }
-
     fn parse_with_clamping_mode<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
@@ -92,23 +105,12 @@ impl Time {
             Token::Dimension {
                 value, ref unit, ..
             } if clamping_mode.is_ok(ParsingMode::DEFAULT, value) => {
-                Time::parse_dimension(value, unit, /* from_calc = */ false)
+                Time::parse_dimension(value, unit)
                     .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             },
             Token::Function(ref name) => {
                 let function = CalcNode::math_function(name, location)?;
-                let time = CalcNode::parse_time(context, input, function)?;
-
-                // FIXME(emilio): Rejecting calc() at parse time is wrong,
-                // was_calc should probably be replaced by calc_clamping_mode or
-                // something like we do for numbers, or we should do the
-                // clamping here instead (simpler, but technically incorrect,
-                // though still more correct than this!).
-                if !clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                }
-
-                Ok(time)
+                CalcNode::parse_time(context, input, clamping_mode, function)
             },
             ref t => return Err(location.new_unexpected_token_error(t.clone())),
         }
@@ -127,14 +129,17 @@ impl ToComputedValue for Time {
     type ComputedValue = ComputedTime;
 
     fn to_computed_value(&self, _context: &Context) -> Self::ComputedValue {
-        ComputedTime::from_seconds(self.seconds())
+        let seconds = self.calc_clamping_mode
+            .map_or(self.seconds(), |mode| mode.clamp(self.seconds()));
+
+        ComputedTime::from_seconds(crate::values::normalize(seconds))
     }
 
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
         Time {
             seconds: computed.seconds(),
             unit: TimeUnit::Second,
-            was_calc: false,
+            calc_clamping_mode: None,
         }
     }
 }
@@ -153,23 +158,7 @@ impl ToCss for Time {
     where
         W: Write,
     {
-        if self.was_calc {
-            dest.write_str("calc(")?;
-        }
-        match self.unit {
-            TimeUnit::Second => {
-                self.seconds.to_css(dest)?;
-                dest.write_char('s')?;
-            },
-            TimeUnit::Millisecond => {
-                (self.seconds * 1000.).to_css(dest)?;
-                dest.write_str("ms")?;
-            },
-        }
-        if self.was_calc {
-            dest.write_char(')')?;
-        }
-        Ok(())
+        crate::values::serialize_specified_dimension(self.unitless_value(), self.unit(), self.calc_clamping_mode.is_some(), dest)
     }
 }