Bug 1374233 - Part 5: Use NonNegativeLength and NonNegativeAu for border-spacing. draft
authorBoris Chiou <boris.chiou@gmail.com>
Fri, 21 Jul 2017 13:35:06 +0800
changeset 614996 2d07c4faed1715015d4a1cd94812f3ee4d96d22c
parent 614995 88d1a5c41b977f7e0b99b53f72b9ae1e9be517d3
child 614997 8a2d21f3348c00e86ce2149f5240b13e4b0afe1d
push id70205
push userbmo:boris.chiou@gmail.com
push dateTue, 25 Jul 2017 08:53:17 +0000
bugs1374233
milestone56.0a1
Bug 1374233 - Part 5: Use NonNegativeLength and NonNegativeAu for border-spacing. We already have NonNegativeLength and NonNegativeAu, so we can re-use it to define the specified value and the computed value of border-spacing. And then implement ToAnimatedValue for it. MozReview-Commit-ID: CLckpKMYVXU
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/longhand/inherited_table.mako.rs
servo/components/style/values/specified/length.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3993,29 +3993,29 @@ fn static_assert() {
         }
     }
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="InheritedTable"
                   skip_longhands="border-spacing">
 
     pub fn set_border_spacing(&mut self, v: longhands::border_spacing::computed_value::T) {
-        self.gecko.mBorderSpacingCol = v.horizontal.0;
-        self.gecko.mBorderSpacingRow = v.vertical.0;
+        self.gecko.mBorderSpacingCol = v.horizontal.value();
+        self.gecko.mBorderSpacingRow = v.vertical.value();
     }
 
     pub fn copy_border_spacing_from(&mut self, other: &Self) {
         self.gecko.mBorderSpacingCol = other.gecko.mBorderSpacingCol;
         self.gecko.mBorderSpacingRow = other.gecko.mBorderSpacingRow;
     }
 
     pub fn clone_border_spacing(&self) -> longhands::border_spacing::computed_value::T {
         longhands::border_spacing::computed_value::T {
-            horizontal: Au(self.gecko.mBorderSpacingCol),
-            vertical: Au(self.gecko.mBorderSpacingRow)
+            horizontal: NonNegativeAu(Au(self.gecko.mBorderSpacingCol)),
+            vertical: NonNegativeAu(Au(self.gecko.mBorderSpacingRow))
         }
     }
 </%self:impl_trait>
 
 
 <%self:impl_trait style_struct_name="InheritedText"
                   skip_longhands="text-align text-emphasis-style text-shadow line-height letter-spacing word-spacing
                                   -webkit-text-stroke-width text-emphasis-position -moz-tab-size">
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -11,16 +11,17 @@ use cssparser::{Parser, RGBA};
 use euclid::{Point2D, Size2D};
 #[cfg(feature = "gecko")] use gecko_bindings::bindings::RawServoAnimationValueMap;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::RawGeckoGfxMatrix4x4;
 #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID;
 #[cfg(feature = "gecko")] use gecko_bindings::sugar::ownership::{HasFFI, HasSimpleFFI};
 #[cfg(feature = "gecko")] use gecko_string_cache::Atom;
 use properties::{CSSWideKeyword, PropertyDeclaration};
 use properties::longhands;
+use properties::longhands::border_spacing::computed_value::T as BorderSpacing;
 use properties::longhands::font_weight::computed_value::T as FontWeight;
 use properties::longhands::font_size_adjust::computed_value::T as FontSizeAdjust;
 use properties::longhands::font_stretch::computed_value::T as FontStretch;
 use properties::longhands::transform::computed_value::ComputedMatrix;
 use properties::longhands::transform::computed_value::ComputedOperation as TransformOperation;
 use properties::longhands::transform::computed_value::T as TransformList;
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
--- a/servo/components/style/properties/longhand/inherited_table.mako.rs
+++ b/servo/components/style/properties/longhand/inherited_table.mako.rs
@@ -15,31 +15,31 @@
                          animation_value_type="discrete",
                          spec="https://drafts.csswg.org/css-tables/#propdef-empty-cells")}
 ${helpers.single_keyword("caption-side", "top bottom",
                          extra_gecko_values="right left top-outside bottom-outside",
                          needs_conversion="True",
                          animation_value_type="discrete",
                          spec="https://drafts.csswg.org/css-tables/#propdef-caption-side")}
 
-<%helpers:longhand name="border-spacing" animation_value_type="ComputedValue" boxed="True"
+<%helpers:longhand name="border-spacing" animation_value_type="BorderSpacing" boxed="True"
                    spec="https://drafts.csswg.org/css-tables/#propdef-border-spacing">
-    use app_units::Au;
     use values::specified::{AllowQuirks, Length};
+    use values::specified::length::NonNegativeLength;
 
     pub mod computed_value {
-        use app_units::Au;
         use properties::animated_properties::Animatable;
-        use values::animated::ToAnimatedZero;
+        use values::animated::{ToAnimatedValue, ToAnimatedZero};
+        use values::computed::NonNegativeAu;
 
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
         #[derive(Clone, Copy, Debug, PartialEq, ToCss)]
         pub struct T {
-            pub horizontal: Au,
-            pub vertical: Au,
+            pub horizontal: NonNegativeAu,
+            pub vertical: NonNegativeAu,
         }
 
         /// https://drafts.csswg.org/css-transitions/#animtype-simple-list
         impl Animatable for T {
             #[inline]
             fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64)
                 -> Result<Self, ()> {
                 Ok(T {
@@ -61,30 +61,47 @@
                    self.vertical.compute_squared_distance(&other.vertical)?)
             }
         }
 
         impl ToAnimatedZero for T {
             #[inline]
             fn to_animated_zero(&self) -> Result<Self, ()> { Err(()) }
         }
+
+        impl ToAnimatedValue for T {
+            type AnimatedValue = Self;
+
+            #[inline]
+            fn to_animated_value(self) -> Self {
+                self
+            }
+
+            #[inline]
+            fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+                T { horizontal: NonNegativeAu::from_animated_value(animated.horizontal),
+                    vertical: NonNegativeAu::from_animated_value(animated.vertical) }
+            }
+        }
     }
 
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     #[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)]
     pub struct SpecifiedValue {
-        pub horizontal: Length,
-        pub vertical: Option<Length>,
+        pub horizontal: NonNegativeLength,
+        pub vertical: Option<NonNegativeLength>,
     }
 
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
+        use app_units::Au;
+        use values::computed::NonNegativeAu;
         computed_value::T {
-            horizontal: Au(0),
-            vertical: Au(0),
+            horizontal: NonNegativeAu(Au(0)),
+            vertical: NonNegativeAu(Au(0)),
         }
     }
 
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         #[inline]
         fn to_computed_value(&self, context: &Context) -> computed_value::T {
@@ -116,22 +133,22 @@
                     second = Some(len);
                 }
             }
         }
         match (first, second) {
             (None, None) => Err(StyleParseError::UnspecifiedError.into()),
             (Some(length), None) => {
                 Ok(SpecifiedValue {
-                    horizontal: length,
+                    horizontal: NonNegativeLength(length),
                     vertical: None,
                 })
             }
             (Some(horizontal), Some(vertical)) => {
                 Ok(SpecifiedValue {
-                    horizontal: horizontal,
-                    vertical: Some(vertical),
+                    horizontal: NonNegativeLength(horizontal),
+                    vertical: Some(NonNegativeLength(vertical)),
                 })
             }
             (None, Some(_)) => unreachable!(),
         }
     }
 </%helpers:longhand>
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -701,16 +701,23 @@ impl<T: Parse> Either<Length, T> {
     }
 }
 
 /// A wrapper of Length, whose value must be >= 0.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)]
 pub struct NonNegativeLength(pub Length);
 
+impl From<NoCalcLength> for NonNegativeLength {
+    #[inline]
+    fn from(len: NoCalcLength) -> Self {
+        NonNegativeLength(Length::NoCalc(len))
+    }
+}
+
 impl<T: Parse> Parse for Either<NonNegativeLength, T> {
     #[inline]
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         if let Ok(v) = input.try(|input| T::parse(context, input)) {
             return Ok(Either::Second(v));
         }
         Length::parse_internal(context, input, AllowedLengthType::NonNegative, AllowQuirks::No)
             .map(NonNegativeLength).map(Either::First)