Bug 1965802 - CSS calc() expression in an integer context should not round until computed-value time. r=firefox-style-system-reviewers,emilio

The spec at https://drafts.csswg.org/css-values-4/#calc-simplification does not call for
rounding as part of specified-value calc() simplification when used an integer context.
Rounding (as well as clamping) happens at computed value time.

To support this, we let the specified::Integer type hold a floating-point value if it
comes from a calc() expression, and only round to integer when accessing the value() for
further use.

Tryserver seems to be happy with this:
https://treeherder.mozilla.org/jobs?repo=try&revision=9a6b1850d25078a6de128728ddf39e8387f615e4

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

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

@@ -17,7 +17,7 @@ use crate::values::generics::calc::{
 use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength};
 use crate::values::specified::length::{ContainerRelativeLength, ViewportPercentageLength};
 use crate::values::specified::{self, Angle, Resolution, Time};
-use crate::values::{serialize_number, serialize_percentage, CSSFloat, CSSInteger, DashedIdent};
+use crate::values::{serialize_number, serialize_percentage, CSSFloat, DashedIdent};
 use cssparser::{CowRcStr, Parser, Token};
 use smallvec::SmallVec;
 use std::cmp;
@@ -1158,15 +1158,6 @@ impl CalcNode {
         Ok(function)
     }
 
-    /// Convenience parsing function for integers.
-    pub fn parse_integer<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-        function: MathFunction,
-    ) -> Result<CSSInteger, ParseError<'i>> {
-        Self::parse_number(context, input, function).map(|n| (n + 0.5).floor() as CSSInteger)
-    }
-
     /// Convenience parsing function for `<length> | <percentage>`, and, optionally, `anchor()`.
     pub fn parse_length_or_percentage<'i, 't>(
         context: &ParserContext,

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

@@ -113,7 +113,7 @@ fn parse_counters<'i, 't>(
 
         let value = match input.try_parse(|input| Integer::parse(context, input)) {
             Ok(start) => {
-                if start.value == i32::min_value() {
+                if start.value() == i32::min_value() {
                     // The spec says that values must be clamped to the valid range,
                     // and we reserve i32::min_value() as an internal magic value.
                     // https://drafts.csswg.org/css-lists/#auto-numbering

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

@@ -589,13 +589,17 @@ impl ToComputedValue for Opacity {
     }
 }
 
-/// A specified `<integer>`, optionally coming from a `calc()` expression.
+/// A specified `<integer>`, either a simple integer value or a calc expression.
+/// Note that a calc expression may not actually be an integer; it will be rounded
+/// at computed-value time.
 ///
 /// <https://drafts.csswg.org/css-values/#integers>
-#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, PartialOrd, ToShmem)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, PartialOrd, ToShmem)]
-pub struct Integer {
+pub enum Integer {
-    value: CSSInteger,
+    /// A literal integer value.
-    was_calc: bool,
+    Literal(CSSInteger),
+    /// A calc expression, whose value will be rounded later if necessary.
+    Calc(CSSFloat),
 }
 
 impl Zero for Integer {
@@ -606,7 +610,7 @@ impl Zero for Integer {
 
     #[inline]
     fn is_zero(&self) -> bool {
-        self.value() == 0
+        *self == 0
     }
 }
 
@@ -618,7 +622,7 @@ impl One for Integer {
 
     #[inline]
     fn is_one(&self) -> bool {
-        self.value() == 1
+        *self == 1
     }
 }
 
@@ -631,23 +635,20 @@ impl PartialEq<i32> for Integer {
 impl Integer {
     /// Trivially constructs a new `Integer` value.
     pub fn new(val: CSSInteger) -> Self {
-        Integer {
+        Self::Literal(val)
-            value: val,
-            was_calc: false,
-        }
     }
 
-    /// Returns the integer value associated with this value.
+    /// Returns the (rounded) integer value associated with this value.
     pub fn value(&self) -> CSSInteger {
-        self.value
+        match *self {
+            Self::Literal(i) => i,
+            Self::Calc(n) => (n + 0.5).floor() as CSSInteger,
+        }
     }
 
     /// Trivially constructs a new integer value from a `calc()` expression.
-    fn from_calc(val: CSSInteger) -> Self {
+    fn from_calc(val: CSSFloat) -> Self {
-        Integer {
+        Self::Calc(val)
-            value: val,
-            was_calc: true,
-        }
     }
 }
 
@@ -663,7 +664,7 @@ impl Parse for Integer {
             } => Ok(Integer::new(v)),
             Token::Function(ref name) => {
                 let function = CalcNode::math_function(context, name, location)?;
-                let result = CalcNode::parse_integer(context, input, function)?;
+                let result = CalcNode::parse_number(context, input, function)?;
                 Ok(Integer::from_calc(result))
             },
             ref t => Err(location.new_unexpected_token_error(t.clone())),
@@ -712,7 +713,7 @@ impl ToComputedValue for Integer {
 
     #[inline]
     fn to_computed_value(&self, _: &Context) -> i32 {
-        self.value
+        self.value()
     }
 
     #[inline]
@@ -726,14 +727,14 @@ impl ToCss for Integer {
     where
         W: Write,
     {
-        if self.was_calc {
+        match *self {
+            Integer::Literal(i) => i.to_css(dest),
+            Integer::Calc(n) => {
                 dest.write_str("calc(")?;
+                n.to_css(dest)?;
+                dest.write_char(')')
             }
-        self.value.to_css(dest)?;
-        if self.was_calc {
-            dest.write_char(')')?;
         }
-        Ok(())
     }
 }