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

components/layout/table.rs

@@ -27,7 +27,7 @@ use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW};
 use style::values::CSSFloat;
-use style::values::computed::LengthOrPercentageOrAuto;
+use style::values::computed::{LengthOrPercentageOrAuto, NonNegativeAu};
 use table_row::{self, CellIntrinsicInlineSize, CollapsedBorder, CollapsedBorderProvenance};
 use table_row::TableRowFlow;
 use table_wrapper::TableLayout;
@@ -190,8 +190,8 @@ impl TableFlow {
             border_collapse::T::separate => style.get_inheritedtable().border_spacing,
             border_collapse::T::collapse => {
                 border_spacing::T {
-                    horizontal: Au(0),
+                    horizontal: NonNegativeAu::zero(),
-                    vertical: Au(0),
+                    vertical: NonNegativeAu::zero(),
                 }
             }
         }
@@ -202,7 +202,7 @@ impl TableFlow {
         if num_columns == 0 {
             return Au(0);
         }
-        self.spacing().horizontal * (num_columns as i32 + 1)
+        self.spacing().horizontal.0 * (num_columns as i32 + 1)
     }
 }
 
@@ -469,7 +469,7 @@ impl Flow for TableFlow {
 
     fn assign_block_size(&mut self, _: &LayoutContext) {
         debug!("assign_block_size: assigning block_size for table");
-        let vertical_spacing = self.spacing().vertical;
+        let vertical_spacing = self.spacing().vertical.0;
         self.block_flow.assign_block_size_for_table_like_flow(vertical_spacing)
     }
 

components/layout/table_row.rs

@@ -25,7 +25,7 @@ use style::computed_values::{border_collapse, border_spacing, border_top_style};
 use style::logical_geometry::{LogicalSize, PhysicalSide, WritingMode};
 use style::properties::ComputedValues;
 use style::servo::restyle_damage::{REFLOW, REFLOW_OUT_OF_FLOW};
-use style::values::computed::{Color, LengthOrPercentageOrAuto};
+use style::values::computed::{Color, LengthOrPercentageOrAuto, NonNegativeAu};
 use table::{ColumnComputedInlineSize, ColumnIntrinsicInlineSize, InternalTable, VecExt};
 use table_cell::{CollapsedBordersForCell, TableCellFlow};
 
@@ -93,8 +93,8 @@ impl TableRowFlow {
             column_computed_inline_sizes: Vec::new(),
             incoming_rowspan: Vec::new(),
             spacing: border_spacing::T {
-                horizontal: Au(0),
+                horizontal: NonNegativeAu::zero(),
-                vertical: Au(0),
+                vertical: NonNegativeAu::zero(),
             },
             table_writing_mode: writing_mode,
             preliminary_collapsed_borders: CollapsedBordersForRow::new(),
@@ -395,7 +395,7 @@ impl Flow for TableRowFlow {
                         None => break,
                     };
                 column_computed_inline_size.size = column_computed_inline_size.size +
-                    extra_column_computed_inline_size.size + self.spacing.horizontal;
+                    extra_column_computed_inline_size.size + self.spacing.horizontal.0;
                 col += 1;
             }
 
@@ -818,7 +818,7 @@ fn set_inline_position_of_child_flow(
         let column_inline_size = column_computed_inline_sizes[*column_index].size;
         let border_inline_size = match *border_collapse_info {
             Some(_) => Au(0), // FIXME: Make collapsed borders account for colspan/rowspan.
-            None => border_spacing.horizontal,
+            None => border_spacing.horizontal.0,
         };
         if reverse_column_order {
             *inline_end_margin_edge += column_inline_size + border_inline_size;
@@ -873,9 +873,9 @@ fn set_inline_position_of_child_flow(
         None => {
             // Take spacing into account.
             if reverse_column_order {
-                *inline_end_margin_edge += border_spacing.horizontal;
+                *inline_end_margin_edge += border_spacing.horizontal.0;
             } else {
-                *inline_start_margin_edge += border_spacing.horizontal;
+                *inline_start_margin_edge += border_spacing.horizontal.0;
             }
         }
     }

components/layout/table_rowgroup.rs

@@ -21,6 +21,7 @@ use std::iter::{IntoIterator, Iterator, Peekable};
 use style::computed_values::{border_collapse, border_spacing};
 use style::logical_geometry::LogicalSize;
 use style::properties::ComputedValues;
+use style::values::computed::NonNegativeAu;
 use table::{ColumnIntrinsicInlineSize, InternalTable, TableLikeFlow};
 
 /// A table formatting context.
@@ -55,8 +56,8 @@ impl TableRowGroupFlow {
             block_flow: BlockFlow::from_fragment(fragment),
             column_intrinsic_inline_sizes: Vec::new(),
             spacing: border_spacing::T {
-                horizontal: Au(0),
+                horizontal: NonNegativeAu::zero(),
-                vertical: Au(0),
+                vertical: NonNegativeAu::zero(),
             },
             collapsed_inline_direction_border_widths_for_table: Vec::new(),
             collapsed_block_direction_border_widths_for_table: Vec::new(),
@@ -161,7 +162,7 @@ impl Flow for TableRowGroupFlow {
 
     fn assign_block_size(&mut self, _: &LayoutContext) {
         debug!("assign_block_size: assigning block_size for table_rowgroup");
-        self.block_flow.assign_block_size_for_table_like_flow(self.spacing.vertical)
+        self.block_flow.assign_block_size_for_table_like_flow(self.spacing.vertical.0)
     }
 
     fn compute_absolute_position(&mut self, layout_context: &LayoutContext) {

components/script/dom/element.rs

@@ -548,7 +548,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
                 shared_lock,
                 PropertyDeclaration::BorderSpacing(
                     Box::new(border_spacing::SpecifiedValue {
-                        horizontal: width_value,
+                        horizontal: width_value.into(),
                         vertical: None,
                     }))));
         }

components/style/properties/gecko.mako.rs

@@ -4582,8 +4582,8 @@ fn static_assert() {
                   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.mBorderSpacingCol = v.horizontal.value();
-        self.gecko.mBorderSpacingRow = v.vertical.0;
+        self.gecko.mBorderSpacingRow = v.vertical.value();
     }
 
     pub fn copy_border_spacing_from(&mut self, other: &Self) {
@@ -4597,8 +4597,8 @@ fn static_assert() {
 
     pub fn clone_border_spacing(&self) -> longhands::border_spacing::computed_value::T {
         longhands::border_spacing::computed_value::T {
-            horizontal: Au(self.gecko.mBorderSpacingCol),
+            horizontal: Au(self.gecko.mBorderSpacingCol).into(),
-            vertical: Au(self.gecko.mBorderSpacingRow)
+            vertical: Au(self.gecko.mBorderSpacingRow).into()
         }
     }
 </%self:impl_trait>

components/style/properties/helpers/animated_properties.mako.rs

@@ -16,6 +16,7 @@ use euclid::{Point2D, Size2D};
 #[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_stretch::computed_value::T as FontStretch;
 use properties::longhands::transform::computed_value::ComputedMatrix;

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

@@ -20,21 +20,21 @@ ${helpers.single_keyword("caption-side", "top bottom",
                          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 horizontal: NonNegativeAu,
-            pub vertical: Au,
+            pub vertical: NonNegativeAu,
         }
 
         /// https://drafts.csswg.org/css-transitions/#animtype-simple-list
@@ -66,20 +66,38 @@ ${helpers.single_keyword("caption-side", "top bottom",
             #[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: ToAnimatedValue::from_animated_value(animated.horizontal),
+                    vertical: ToAnimatedValue::from_animated_value(animated.vertical)
+                }
+            }
+        }
     }
 
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     #[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToCss)]
     pub struct SpecifiedValue {
-        pub horizontal: Length,
+        pub horizontal: NonNegativeLength,
-        pub vertical: Option<Length>,
+        pub vertical: Option<NonNegativeLength>,
     }
 
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
+        use values::computed::NonNegativeAu;
         computed_value::T {
-            horizontal: Au(0),
+            horizontal: NonNegativeAu::zero(),
-            vertical: Au(0),
+            vertical: NonNegativeAu::zero(),
         }
     }
 
@@ -121,14 +139,14 @@ ${helpers.single_keyword("caption-side", "top bottom",
             (None, None) => Err(StyleParseError::UnspecifiedError.into()),
             (Some(length), None) => {
                 Ok(SpecifiedValue {
-                    horizontal: length,
+                    horizontal: length.into(),
                     vertical: None,
                 })
             }
             (Some(horizontal), Some(vertical)) => {
                 Ok(SpecifiedValue {
-                    horizontal: horizontal,
+                    horizontal: horizontal.into(),
-                    vertical: Some(vertical),
+                    vertical: Some(vertical.into()),
                 })
             }
             (None, Some(_)) => unreachable!(),

components/style/values/specified/length.rs

@@ -708,6 +708,20 @@ impl<T: Parse> Either<Length, T> {
 /// A wrapper of Length, whose value must be >= 0.
 pub type NonNegativeLength = NonNegative<Length>;
 
+impl From<NoCalcLength> for NonNegativeLength {
+    #[inline]
+    fn from(len: NoCalcLength) -> Self {
+        NonNegative::<Length>(Length::NoCalc(len))
+    }
+}
+
+impl From<Length> for NonNegativeLength {
+    #[inline]
+    fn from(len: Length) -> Self {
+        NonNegative::<Length>(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>> {