Auto merge of #16382 - nox:calc-clamping, r=emilio

Don't reject out of bound calc() values at parsing-time

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

components/style/gecko_bindings/sugar/ns_timing_function.rs

@@ -71,8 +71,8 @@ impl From<SpecifiedTimingFunction> for nsTimingFunction {
             },
             SpecifiedTimingFunction::CubicBezier(p1, p2) => {
                 tf.set_as_bezier(nsTimingFunction_Type::CubicBezier,
-                                 Point2D::new(p1.x.value, p1.y.value),
-                                 Point2D::new(p2.x.value, p2.y.value));
+                                 Point2D::new(p1.x.get(), p1.y.get()),
+                                 Point2D::new(p2.x.get(), p2.y.get()));
             },
             SpecifiedTimingFunction::Keyword(keyword) => {
                 match keyword.to_computed_value() {

components/style/properties/gecko.mako.rs

@@ -3160,11 +3160,11 @@ fn static_assert() {
                 self.gecko.mInitialLetterSink = 0;
             },
             T::Specified(size, sink) => {
-                self.gecko.mInitialLetterSize = size.value;
+                self.gecko.mInitialLetterSize = size.get();
                 if let Some(sink) = sink {
                     self.gecko.mInitialLetterSink = sink.value();
                 } else {
-                    self.gecko.mInitialLetterSink = size.value.floor() as i32;
+                    self.gecko.mInitialLetterSink = size.get().floor() as i32;
                 }
             }
         }

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

@@ -574,8 +574,8 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
                             p2y = try!(specified::parse_number(context, input));
                             Ok(())
                         }));
-                        if p1x.value < 0.0 || p1x.value > 1.0 ||
-                           p2x.value < 0.0 || p2x.value > 1.0 {
+                        if p1x.get() < 0.0 || p1x.get() > 1.0 ||
+                           p2x.get() < 0.0 || p2x.get() > 1.0 {
                             return Err(())
                         }
 

components/style/values/specified/length.rs

@@ -15,7 +15,7 @@ use std::{cmp, fmt, mem};
 use std::ascii::AsciiExt;
 use std::ops::Mul;
 use style_traits::ToCss;
-use style_traits::values::specified::AllowedNumericType;
+use style_traits::values::specified::AllowedLengthType;
 use stylesheets::CssRuleType;
 use super::{Angle, Number, SimplifiedValueNode, SimplifiedSumNode, Time, ToComputedValue};
 use values::{Auto, CSSFloat, Either, FONT_MEDIUM_PX, HasViewportPercentage, None_, Normal};
@@ -465,7 +465,7 @@ pub enum Length {
     /// A calc expression.
     ///
     /// https://drafts.csswg.org/css-values/#calc-notation
-    Calc(Box<CalcLengthOrPercentage>, AllowedNumericType),
+    Calc(Box<CalcLengthOrPercentage>, AllowedLengthType),
 }
 
 impl From<NoCalcLength> for Length {
@@ -546,7 +546,7 @@ impl Length {
     }
 
     #[inline]
-    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedNumericType)
+    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedLengthType)
                       -> Result<Length, ()> {
         match try!(input.next()) {
             Token::Dimension(ref value, ref unit) if num_context.is_ok(value.value) =>
@@ -563,7 +563,7 @@ impl Length {
     /// Parse a non-negative length
     #[inline]
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<Length, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::NonNegative)
+        Self::parse_internal(context, input, AllowedLengthType::NonNegative)
     }
 
     /// Get an absolute length from a px value.
@@ -583,7 +583,7 @@ impl Length {
 
 impl Parse for Length {
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::All)
+        Self::parse_internal(context, input, AllowedLengthType::All)
     }
 }
 
@@ -605,7 +605,7 @@ impl Either<Length, Normal> {
         if input.try(|input| Normal::parse(context, input)).is_ok() {
             return Ok(Either::Second(Normal));
         }
-        Length::parse_internal(context, input, AllowedNumericType::NonNegative).map(Either::First)
+        Length::parse_internal(context, input, AllowedLengthType::NonNegative).map(Either::First)
     }
 }
 
@@ -616,7 +616,7 @@ impl Either<Length, Auto> {
         if input.try(|input| Auto::parse(context, input)).is_ok() {
             return Ok(Either::Second(Auto));
         }
-        Length::parse_internal(context, input, AllowedNumericType::NonNegative).map(Either::First)
+        Length::parse_internal(context, input, AllowedLengthType::NonNegative).map(Either::First)
     }
 }
 
@@ -843,7 +843,7 @@ impl CalcLengthOrPercentage {
 
     fn parse_length(context: &ParserContext,
                     input: &mut Parser,
-                    num_context: AllowedNumericType) -> Result<Length, ()> {
+                    num_context: AllowedLengthType) -> Result<Length, ()> {
         CalcLengthOrPercentage::parse(context, input, CalcUnit::Length).map(|calc| {
             Length::Calc(Box::new(calc), num_context)
         })
@@ -1068,7 +1068,7 @@ impl ToCss for Percentage {
 }
 
 impl Percentage {
-    fn parse_internal(input: &mut Parser, context: AllowedNumericType) -> Result<Self, ()> {
+    fn parse_internal(input: &mut Parser, context: AllowedLengthType) -> Result<Self, ()> {
         match try!(input.next()) {
             Token::Percentage(ref value) if context.is_ok(value.unit_value) => {
                 Ok(Percentage(value.unit_value))
@@ -1079,14 +1079,14 @@ impl Percentage {
 
     /// Parses a percentage token, but rejects it if it's negative.
     pub fn parse_non_negative(input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(input, AllowedNumericType::NonNegative)
+        Self::parse_internal(input, AllowedLengthType::NonNegative)
     }
 }
 
 impl Parse for Percentage {
     #[inline]
     fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(input, AllowedNumericType::All)
+        Self::parse_internal(input, AllowedLengthType::All)
     }
 }
 
@@ -1152,7 +1152,7 @@ impl LengthOrPercentage {
         LengthOrPercentage::Length(NoCalcLength::zero())
     }
 
-    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedNumericType)
+    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedLengthType)
                       -> Result<LengthOrPercentage, ()>
     {
         match try!(input.next()) {
@@ -1175,14 +1175,14 @@ impl LengthOrPercentage {
     /// Parse a non-negative length.
     #[inline]
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<LengthOrPercentage, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::NonNegative)
+        Self::parse_internal(context, input, AllowedLengthType::NonNegative)
     }
 
     /// Parse a length, treating dimensionless numbers as pixels
     ///
     /// https://www.w3.org/TR/SVG2/types.html#presentation-attribute-css-value
     pub fn parse_numbers_are_pixels(context: &ParserContext, input: &mut Parser) -> Result<LengthOrPercentage, ()> {
-        if let Ok(lop) = input.try(|i| Self::parse_internal(context, i, AllowedNumericType::All)) {
+        if let Ok(lop) = input.try(|i| Self::parse_internal(context, i, AllowedLengthType::All)) {
             return Ok(lop)
         }
 
@@ -1198,7 +1198,7 @@ impl LengthOrPercentage {
     pub fn parse_numbers_are_pixels_non_negative(context: &ParserContext,
                                                  input: &mut Parser)
                                                  -> Result<LengthOrPercentage, ()> {
-        if let Ok(lop) = input.try(|i| Self::parse_internal(context, i, AllowedNumericType::NonNegative)) {
+        if let Ok(lop) = input.try(|i| Self::parse_internal(context, i, AllowedLengthType::NonNegative)) {
             return Ok(lop)
         }
 
@@ -1224,7 +1224,7 @@ impl LengthOrPercentage {
 impl Parse for LengthOrPercentage {
     #[inline]
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::All)
+        Self::parse_internal(context, input, AllowedLengthType::All)
     }
 }
 
@@ -1277,7 +1277,7 @@ impl ToCss for LengthOrPercentageOrAuto {
 }
 
 impl LengthOrPercentageOrAuto {
-    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedNumericType)
+    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedLengthType)
                       -> Result<Self, ()> {
         match try!(input.next()) {
             Token::Dimension(ref value, ref unit) if num_context.is_ok(value.value) =>
@@ -1301,7 +1301,7 @@ impl LengthOrPercentageOrAuto {
     /// Parse a non-negative length, percentage, or auto.
     #[inline]
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<LengthOrPercentageOrAuto, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::NonNegative)
+        Self::parse_internal(context, input, AllowedLengthType::NonNegative)
     }
 
     /// Returns the `auto` value.
@@ -1318,7 +1318,7 @@ impl LengthOrPercentageOrAuto {
 impl Parse for LengthOrPercentageOrAuto {
     #[inline]
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::All)
+        Self::parse_internal(context, input, AllowedLengthType::All)
     }
 }
 
@@ -1355,7 +1355,7 @@ impl ToCss for LengthOrPercentageOrNone {
     }
 }
 impl LengthOrPercentageOrNone {
-    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedNumericType)
+    fn parse_internal(context: &ParserContext, input: &mut Parser, num_context: AllowedLengthType)
                       -> Result<LengthOrPercentageOrNone, ()>
     {
         match try!(input.next()) {
@@ -1379,14 +1379,14 @@ impl LengthOrPercentageOrNone {
     /// Parse a non-negative LengthOrPercentageOrNone.
     #[inline]
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::NonNegative)
+        Self::parse_internal(context, input, AllowedLengthType::NonNegative)
     }
 }
 
 impl Parse for LengthOrPercentageOrNone {
     #[inline]
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        Self::parse_internal(context, input, AllowedNumericType::All)
+        Self::parse_internal(context, input, AllowedLengthType::All)
     }
 }
 
@@ -1421,7 +1421,7 @@ pub enum LengthOrPercentageOrAutoOrContent {
 impl LengthOrPercentageOrAutoOrContent {
     /// Parse a non-negative LengthOrPercentageOrAutoOrContent.
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-        let num_context = AllowedNumericType::NonNegative;
+        let num_context = AllowedLengthType::NonNegative;
         match try!(input.next()) {
             Token::Dimension(ref value, ref unit) if num_context.is_ok(value.value) =>
                 NoCalcLength::parse_dimension(context, value.value, unit)

components/style/values/specified/mod.rs

@@ -18,6 +18,7 @@ use std::f32::consts::PI;
 use std::fmt;
 use std::ops::Mul;
 use style_traits::ToCss;
+use style_traits::values::specified::AllowedNumericType;
 use super::{Auto, CSSFloat, CSSInteger, HasViewportPercentage, Either, None_};
 use super::computed::{self, Context};
 use super::computed::{Shadow as ComputedShadow, ToComputedValue};
@@ -237,11 +238,19 @@ pub fn parse_integer(context: &ParserContext, input: &mut Parser) -> Result<Inte
 
 #[allow(missing_docs)]
 pub fn parse_number(context: &ParserContext, input: &mut Parser) -> Result<Number, ()> {
+    parse_number_with_clamping_mode(context, input, AllowedNumericType::All)
+}
+
+#[allow(missing_docs)]
+pub fn parse_number_with_clamping_mode(context: &ParserContext,
+                                       input: &mut Parser,
+                                       clamping_mode: AllowedNumericType)
+                                       -> Result<Number, ()> {
     match try!(input.next()) {
-        Token::Number(ref value) => {
+        Token::Number(ref value) if clamping_mode.is_ok(value.value) => {
             Ok(Number {
                 value: value.value.min(f32::MAX).max(f32::MIN),
-                was_calc: false,
+                calc_clamping_mode: None,
             })
         },
         Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => {
@@ -263,7 +272,7 @@ pub fn parse_number(context: &ParserContext, input: &mut Parser) -> Result<Numbe
                 Some(result) => {
                     Ok(Number {
                         value: result.min(f32::MAX).max(f32::MIN),
-                        was_calc: true,
+                        calc_clamping_mode: Some(clamping_mode),
                     })
                 },
                 _ => Err(())
@@ -692,11 +701,10 @@ impl ToCss for Time {
 #[allow(missing_docs)]
 pub struct Number {
     /// The numeric value itself.
-    pub value: CSSFloat,
-    /// Whether this came from a `calc()` expression. This is needed for
-    /// serialization purposes, since `calc(1)` should still serialize to
-    /// `calc(1)`, not just `1`.
-    was_calc: bool,
+    value: CSSFloat,
+    /// If this number came from a calc() expression, this tells how clamping
+    /// should be done on the value.
+    calc_clamping_mode: Option<AllowedNumericType>,
 }
 
 no_viewport_percentage!(Number);
@@ -708,29 +716,27 @@ impl Parse for Number {
 }
 
 impl Number {
-    fn parse_with_minimum(context: &ParserContext, input: &mut Parser, min: CSSFloat) -> Result<Number, ()> {
-        match parse_number(context, input) {
-            Ok(value) if value.value >= min => Ok(value),
-            _ => Err(()),
-        }
-    }
-
     /// Returns a new number with the value `val`.
     pub fn new(val: CSSFloat) -> Self {
         Number {
             value: val,
-            was_calc: false,
+            calc_clamping_mode: None,
         }
     }
 
+    /// Returns the numeric value, clamped if needed.
+    pub fn get(&self) -> f32 {
+        self.calc_clamping_mode.map_or(self.value, |mode| mode.clamp(self.value))
+    }
+
     #[allow(missing_docs)]
     pub fn parse_non_negative(context: &ParserContext, input: &mut Parser) -> Result<Number, ()> {
-        Number::parse_with_minimum(context, input, 0.0)
+        parse_number_with_clamping_mode(context, input, AllowedNumericType::NonNegative)
     }
 
     #[allow(missing_docs)]
     pub fn parse_at_least_one(context: &ParserContext, input: &mut Parser) -> Result<Number, ()> {
-        Number::parse_with_minimum(context, input, 1.0)
+        parse_number_with_clamping_mode(context, input, AllowedNumericType::AtLeastOne)
     }
 }
 
@@ -738,13 +744,13 @@ impl ToComputedValue for Number {
     type ComputedValue = CSSFloat;
 
     #[inline]
-    fn to_computed_value(&self, _: &Context) -> CSSFloat { self.value }
+    fn to_computed_value(&self, _: &Context) -> CSSFloat { self.get() }
 
     #[inline]
     fn from_computed_value(computed: &CSSFloat) -> Self {
         Number {
             value: *computed,
-            was_calc: false,
+            calc_clamping_mode: None,
         }
     }
 }
@@ -753,11 +759,11 @@ impl ToCss for Number {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
-        if self.was_calc {
+        if self.calc_clamping_mode.is_some() {
             dest.write_str("calc(")?;
         }
         self.value.to_css(dest)?;
-        if self.was_calc {
+        if self.calc_clamping_mode.is_some() {
             dest.write_str(")")?;
         }
         Ok(())

components/style_traits/values.rs

@@ -150,35 +150,70 @@ macro_rules! __define_css_keyword_enum__actual {
 /// Helper types for the handling of specified values.
 pub mod specified {
     use app_units::Au;
+    use std::cmp;
 
-    /// Whether to allow negative values or not.
+    /// Whether to allow negative lengths or not.
     #[repr(u8)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     #[derive(Clone, Copy, Debug, PartialEq, Eq)]
-    pub enum AllowedNumericType {
-        /// Allow all kind of numeric values.
+    pub enum AllowedLengthType {
+        /// Allow all kind of lengths.
         All,
-        /// Allow only non-negative values.
+        /// Allow only non-negative lengths.
         NonNegative
     }
 
-    impl AllowedNumericType {
-        /// Whether value is valid for this allowed numeric type.
+    impl AllowedLengthType {
+        /// Whether value is valid for this allowed length type.
         #[inline]
         pub fn is_ok(&self, value: f32) -> bool {
             match *self {
-                AllowedNumericType::All => true,
-                AllowedNumericType::NonNegative => value >= 0.,
+                AllowedLengthType::All => true,
+                AllowedLengthType::NonNegative => value >= 0.,
             }
         }
 
         /// Clamp the value following the rules of this numeric type.
         #[inline]
         pub fn clamp(&self, val: Au) -> Au {
-            use std::cmp;
             match *self {
-                AllowedNumericType::All => val,
-                AllowedNumericType::NonNegative => cmp::max(Au(0), val),
+                AllowedLengthType::All => val,
+                AllowedLengthType::NonNegative => cmp::max(Au(0), val),
+            }
+        }
+    }
+
+    /// Whether to allow negative lengths or not.
+    #[repr(u8)]
+    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+    #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)]
+    pub enum AllowedNumericType {
+        /// Allow all kind of numeric values.
+        All,
+        /// Allow only non-negative numeric values.
+        NonNegative,
+        /// Allow only numeric values greater or equal to 1.0.
+        AtLeastOne,
+    }
+
+    impl AllowedNumericType {
+        /// Whether the value fits the rules of this numeric type.
+        #[inline]
+        pub fn is_ok(&self, val: f32) -> bool {
+            match *self {
+                AllowedNumericType::All => true,
+                AllowedNumericType::NonNegative => val >= 0.0,
+                AllowedNumericType::AtLeastOne => val >= 1.0,
+            }
+        }
+
+        /// Clamp the value following the rules of this numeric type.
+        #[inline]
+        pub fn clamp(&self, val: f32) -> f32 {
+            match *self {
+                AllowedNumericType::NonNegative if val < 0. => 0.,
+                AllowedNumericType::AtLeastOne if val < 1. => 1.,
+                _ => val,
             }
         }
     }

components/style_traits/viewport.rs

@@ -9,7 +9,7 @@ use cssparser::{Parser, ToCss};
 use euclid::size::TypedSize2D;
 use std::ascii::AsciiExt;
 use std::fmt;
-use values::specified::AllowedNumericType;
+use values::specified::AllowedLengthType;
 
 define_css_keyword_enum!(UserZoom:
                          "zoom" => Zoom,
@@ -95,9 +95,9 @@ impl Zoom {
         use cssparser::Token;
 
         match try!(input.next()) {
-            Token::Percentage(ref value) if AllowedNumericType::NonNegative.is_ok(value.unit_value) =>
+            Token::Percentage(ref value) if AllowedLengthType::NonNegative.is_ok(value.unit_value) =>
                 Ok(Zoom::Percentage(value.unit_value)),
-            Token::Number(ref value) if AllowedNumericType::NonNegative.is_ok(value.value) =>
+            Token::Number(ref value) if AllowedLengthType::NonNegative.is_ok(value.value) =>
                 Ok(Zoom::Number(value.value)),
             Token::Ident(ref value) if value.eq_ignore_ascii_case("auto") =>
                 Ok(Zoom::Auto),