Auto merge of #20500 - emilio:1105111, r=xidorn

style: Add support for 'flex-basis:content' in the style system.

Bug: 1105111
Reviewed-by: xidorn
MozReview-Commit-ID: 5WhgHJJ0mDB

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

components/layout/flex.rs

@@ -75,28 +75,16 @@ impl AxisSize {
 fn from_flex_basis(
     flex_basis: FlexBasis,
     main_length: LengthOrPercentageOrAuto,
-    containing_length: Option<Au>
+    containing_length: Au,
 ) -> MaybeAuto {
-    match (flex_basis, containing_length) {
+    let width = match flex_basis {
-        (GenericFlexBasis::Length(LengthOrPercentage::Length(length)), _) =>
+        GenericFlexBasis::Content => return MaybeAuto::Auto,
-            MaybeAuto::Specified(Au::from(length)),
+        GenericFlexBasis::Width(width) => width,
-        (GenericFlexBasis::Length(LengthOrPercentage::Percentage(percent)), Some(size)) =>
+    };
-            MaybeAuto::Specified(size.scale_by(percent.0)),
+
-        (GenericFlexBasis::Length(LengthOrPercentage::Percentage(_)), None) =>
+    match width.0 {
-            MaybeAuto::Auto,
+        LengthOrPercentageOrAuto::Auto => MaybeAuto::from_style(main_length, containing_length),
-        (GenericFlexBasis::Length(LengthOrPercentage::Calc(calc)), _) =>
+        other => MaybeAuto::from_style(other, containing_length),
-            MaybeAuto::from_option(calc.to_used_value(containing_length)),
-        (GenericFlexBasis::Content, _) =>
-            MaybeAuto::Auto,
-        (GenericFlexBasis::Auto, Some(size)) =>
-            MaybeAuto::from_style(main_length, size),
-        (GenericFlexBasis::Auto, None) => {
-            if let LengthOrPercentageOrAuto::Length(length) = main_length {
-                MaybeAuto::Specified(Au::from(length))
-            } else {
-                MaybeAuto::Auto
-            }
-        }
     }
 }
 
@@ -161,7 +149,7 @@ impl FlexItem {
             Direction::Inline => {
                 let basis = from_flex_basis(block.fragment.style.get_position().flex_basis,
                                             block.fragment.style.content_inline_size(),
-                                            Some(containing_length));
+                                            containing_length);
 
                 // These methods compute auto margins to zero length, which is exactly what we want.
                 block.fragment.compute_border_and_padding(containing_length);
@@ -183,7 +171,7 @@ impl FlexItem {
             Direction::Block => {
                 let basis = from_flex_basis(block.fragment.style.get_position().flex_basis,
                                             block.fragment.style.content_block_size(),
-                                            Some(containing_length));
+                                            containing_length);
                 let content_size = block.fragment.border_box.size.block
                     - block.fragment.border_padding.block_start_end()
                     + block.fragment.box_sizing_boundary(direction);

components/style/gecko/generated/structs.rs

@@ -489,6 +489,7 @@ pub mod root {
     pub const NS_STYLE_WIDTH_MIN_CONTENT: u32 = 1;
     pub const NS_STYLE_WIDTH_FIT_CONTENT: u32 = 2;
     pub const NS_STYLE_WIDTH_AVAILABLE: u32 = 3;
+    pub const NS_STYLE_FLEX_BASIS_CONTENT: u32 = 4;
     pub const NS_STYLE_POSITION_STATIC: u32 = 0;
     pub const NS_STYLE_POSITION_RELATIVE: u32 = 1;
     pub const NS_STYLE_POSITION_ABSOLUTE: u32 = 2;

components/style/gecko/values.rs

@@ -10,7 +10,7 @@ use Atom;
 use app_units::Au;
 use counter_style::{Symbol, Symbols};
 use cssparser::RGBA;
-use gecko_bindings::structs::{CounterStylePtr, nsStyleCoord};
+use gecko_bindings::structs::{self, CounterStylePtr, nsStyleCoord};
 use gecko_bindings::structs::{StyleGridTrackBreadth, StyleShapeRadius};
 use gecko_bindings::sugar::ns_style_coord::{CoordData, CoordDataMut, CoordDataValue};
 use media_queries::Device;
@@ -21,10 +21,12 @@ use values::computed::{Angle, ExtremumLength, Length, LengthOrPercentage, Length
 use values::computed::{LengthOrPercentageOrNone, Number, NumberOrPercentage};
 use values::computed::{MaxLength, MozLength, Percentage};
 use values::computed::{NonNegativeLength, NonNegativeLengthOrPercentage, NonNegativeNumber};
+use values::computed::FlexBasis as ComputedFlexBasis;
 use values::computed::basic_shape::ShapeRadius as ComputedShapeRadius;
 use values::generics::{CounterStyleOrNone, NonNegative};
 use values::generics::basic_shape::ShapeRadius;
 use values::generics::box_::Perspective;
+use values::generics::flex::FlexBasis;
 use values::generics::gecko::ScrollSnapPoint;
 use values::generics::grid::{TrackBreadth, TrackKeyword};
 
@@ -59,6 +61,31 @@ impl<A: GeckoStyleCoordConvertible, B: GeckoStyleCoordConvertible> GeckoStyleCoo
     }
 }
 
+impl GeckoStyleCoordConvertible for ComputedFlexBasis {
+    fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
+        match *self {
+            FlexBasis::Content => {
+                coord.set_value(
+                    CoordDataValue::Enumerated(structs::NS_STYLE_FLEX_BASIS_CONTENT)
+                )
+            },
+            FlexBasis::Width(ref w) => w.to_gecko_style_coord(coord),
+        }
+    }
+
+    fn from_gecko_style_coord<T: CoordData>(coord: &T) -> Option<Self> {
+        if let Some(width) = MozLength::from_gecko_style_coord(coord) {
+            return Some(FlexBasis::Width(width))
+        }
+
+        if let CoordDataValue::Enumerated(structs::NS_STYLE_FLEX_BASIS_CONTENT) = coord.as_value() {
+            return Some(FlexBasis::Content)
+        }
+
+        None
+    }
+}
+
 impl GeckoStyleCoordConvertible for Number {
     fn to_gecko_style_coord<T: CoordDataMut>(&self, coord: &mut T) {
         coord.set_value(CoordDataValue::Factor(*self));

components/style/properties/gecko.mako.rs

@@ -1446,6 +1446,7 @@ impl Clone for ${style_struct.gecko_struct_name} {
         "length::LengthOrNormal": impl_style_coord,
         "length::NonNegativeLengthOrAuto": impl_style_coord,
         "length::NonNegativeLengthOrPercentageOrNormal": impl_style_coord,
+        "FlexBasis": impl_style_coord,
         "Length": impl_absolute_length,
         "LengthOrNormal": impl_style_coord,
         "LengthOrPercentage": impl_style_coord,
@@ -1748,14 +1749,14 @@ fn static_assert() {
 
 <% skip_position_longhands = " ".join(x.ident for x in SIDES + GRID_LINES) %>
 <%self:impl_trait style_struct_name="Position"
-                  skip_longhands="${skip_position_longhands} z-index order align-content
+                  skip_longhands="${skip_position_longhands} z-index order
-                                  justify-content align-self justify-self align-items
+                                  align-content justify-content align-self
-                                  justify-items grid-auto-rows grid-auto-columns grid-auto-flow
+                                  justify-self align-items justify-items
-                                  grid-template-areas grid-template-rows grid-template-columns">
+                                  grid-auto-rows grid-auto-columns
+                                  grid-auto-flow grid-template-areas
+                                  grid-template-rows grid-template-columns">
     % for side in SIDES:
-    <% impl_split_style_coord("%s" % side.ident,
+    <% impl_split_style_coord(side.ident, "mOffset", side.index) %>
-                              "mOffset",
-                              side.index) %>
     % endfor
 
     pub fn set_z_index(&mut self, v: longhands::z_index::computed_value::T) {

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

@@ -174,32 +174,16 @@ ${helpers.predefined_type("order", "Integer", "0",
                           spec="https://drafts.csswg.org/css-flexbox/#order-property",
                           servo_restyle_damage = "reflow")}
 
-% if product == "gecko":
+${helpers.predefined_type(
-    // FIXME: Gecko doesn't support content value yet.
+    "flex-basis",
-    //
+    "FlexBasis",
-    // FIXME(emilio): I suspect this property shouldn't allow quirks, and this
+    "computed::FlexBasis::auto()",
-    // was just a mistake, it's kind of justificable to support it given the
+    spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
-    // spec grammar is just `content | <width>`, but other browsers don't...
+    extra_prefixes="webkit",
-    ${helpers.predefined_type(
+    animation_value_type="FlexBasis",
-        "flex-basis",
+    servo_restyle_damage = "reflow"
-        "MozLength",
+)}
-        "computed::MozLength::auto()",
+
-        extra_prefixes="webkit",
-        animation_value_type="MozLength",
-        allow_quirks=True,
-        spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
-        servo_restyle_damage = "reflow"
-    )}
-% else:
-    // FIXME: This property should be animatable.
-    ${helpers.predefined_type("flex-basis",
-                              "FlexBasis",
-                              "computed::FlexBasis::auto()",
-                              spec="https://drafts.csswg.org/css-flexbox/#flex-basis-property",
-                              extra_prefixes="webkit",
-                              animation_value_type="none",
-                              servo_restyle_damage = "reflow")}
-% endif
 % for (size, logical) in ALL_SIZES:
     <%
         spec = "https://drafts.csswg.org/css-box/#propdef-%s"

components/style/values/computed/flex.rs

@@ -4,8 +4,23 @@
 
 //! Computed types for CSS values related to flexbox.
 
-use values::computed::length::LengthOrPercentage;
 use values::generics::flex::FlexBasis as GenericFlexBasis;
 
+/// The `width` value type.
+#[cfg(feature = "servo")]
+pub type Width = ::values::computed::NonNegativeLengthOrPercentageOrAuto;
+
+/// The `width` value type.
+#[cfg(feature = "gecko")]
+pub type Width = ::values::computed::MozLength;
+
 /// A computed value for the `flex-basis` property.
-pub type FlexBasis = GenericFlexBasis<LengthOrPercentage>;
+pub type FlexBasis = GenericFlexBasis<Width>;
+
+impl FlexBasis {
+    /// `auto`
+    #[inline]
+    pub fn auto() -> Self {
+        GenericFlexBasis::Width(Width::auto())
+    }
+}

components/style/values/computed/length.rs

@@ -427,10 +427,10 @@ impl LengthOrPercentage {
     pub fn clamp_to_non_negative(self) -> Self {
         match self {
             LengthOrPercentage::Length(length) => {
-                LengthOrPercentage::Length(Length::new(length.px().max(0.)))
+                LengthOrPercentage::Length(length.clamp_to_non_negative())
             },
             LengthOrPercentage::Percentage(percentage) => {
-                LengthOrPercentage::Percentage(Percentage(percentage.0.max(0.)))
+                LengthOrPercentage::Percentage(percentage.clamp_to_non_negative())
             },
             _ => self
         }
@@ -511,6 +511,31 @@ impl LengthOrPercentageOrAuto {
     }
 }
 
+/// A wrapper of LengthOrPercentageOrAuto, whose value must be >= 0.
+pub type NonNegativeLengthOrPercentageOrAuto = NonNegative<LengthOrPercentageOrAuto>;
+
+impl NonNegativeLengthOrPercentageOrAuto {
+    /// `auto`
+    #[inline]
+    pub fn auto() -> Self {
+        NonNegative(LengthOrPercentageOrAuto::Auto)
+    }
+}
+
+impl ToAnimatedValue for NonNegativeLengthOrPercentageOrAuto {
+    type AnimatedValue = LengthOrPercentageOrAuto;
+
+    #[inline]
+    fn to_animated_value(self) -> Self::AnimatedValue {
+        self.0
+    }
+
+    #[inline]
+    fn from_animated_value(animated: Self::AnimatedValue) -> Self {
+        NonNegative(animated.clamp_to_non_negative())
+    }
+}
+
 impl LengthOrPercentageOrAuto {
     /// Returns true if the computed value is absolute 0 or 0%.
     ///
@@ -524,6 +549,15 @@ impl LengthOrPercentageOrAuto {
             Calc(_) | Auto => false
         }
     }
+
+    fn clamp_to_non_negative(self) -> Self {
+        use self::LengthOrPercentageOrAuto::*;
+        match self {
+            Length(l) => Length(l.clamp_to_non_negative()),
+            Percentage(p) => Percentage(p.clamp_to_non_negative()),
+            _ => self,
+        }
+    }
 }
 
 impl ToComputedValue for specified::LengthOrPercentageOrAuto {
@@ -737,6 +771,11 @@ impl CSSPixelLength {
         self.0
     }
 
+    #[inline]
+    fn clamp_to_non_negative(self) -> Self {
+        Self::new(self.px().max(0.))
+    }
+
     /// Return the length with app_unit i32 type.
     #[inline]
     pub fn to_i32_au(&self) -> i32 {

components/style/values/computed/mod.rs

@@ -60,7 +60,8 @@ pub use super::{Auto, Either, None_};
 pub use super::specified::{BorderStyle, TextDecorationLine};
 pub use self::length::{CalcLengthOrPercentage, Length, LengthOrNumber, LengthOrPercentage};
 pub use self::length::{LengthOrPercentageOrAuto, LengthOrPercentageOrNone, MaxLength, MozLength};
-pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength, NonNegativeLengthOrPercentage};
+pub use self::length::{CSSPixelLength, ExtremumLength, NonNegativeLength};
+pub use self::length::{NonNegativeLengthOrPercentage, NonNegativeLengthOrPercentageOrAuto};
 pub use self::list::{ListStyleImage, Quotes};
 #[cfg(feature = "gecko")]
 pub use self::list::ListStyleType;

components/style/values/computed/percentage.rs

@@ -32,6 +32,12 @@ impl Percentage {
     pub fn abs(&self) -> Self {
         Percentage(self.0.abs())
     }
+
+    /// Clamps this percentage to a non-negative percentage.
+    #[inline]
+    pub fn clamp_to_non_negative(self) -> Self {
+        Percentage(self.0.max(0.))
+    }
 }
 
 impl ToCss for Percentage {

components/style/values/generics/flex.rs

@@ -4,34 +4,13 @@
 
 //! Generic types for CSS values related to flexbox.
 
-use values::computed::Percentage;
-
 /// A generic value for the `flex-basis` property.
 #[cfg_attr(feature = "servo", derive(MallocSizeOf))]
-#[derive(Clone, Copy, Debug, PartialEq, ToComputedValue, ToCss)]
+#[derive(Animate, Clone, ComputeSquaredDistance, Copy, Debug, PartialEq)]
-pub enum FlexBasis<LengthOrPercentage> {
+#[derive(ToAnimatedValue, ToAnimatedZero, ToComputedValue, ToCss)]
-    /// `auto`
+pub enum FlexBasis<Width> {
-    Auto,
     /// `content`
     Content,
-    /// `<length-percentage>`
+    /// `<width>`
-    Length(LengthOrPercentage),
+    Width(Width),
-}
-
-impl<L> FlexBasis<L> {
-    /// Returns `auto`.
-    #[inline]
-    pub fn auto() -> Self {
-        FlexBasis::Auto
-    }
-}
-
-impl<L> FlexBasis<L>
-where Percentage: Into<L>,
-{
-    /// Returns `0%`.
-    #[inline]
-    pub fn zero_percent() -> Self {
-        FlexBasis::Length(Percentage(0.).into())
-    }
 }

components/style/values/specified/flex.rs

@@ -8,22 +8,42 @@ use cssparser::Parser;
 use parser::{Parse, ParserContext};
 use style_traits::ParseError;
 use values::generics::flex::FlexBasis as GenericFlexBasis;
-use values::specified::length::LengthOrPercentage;
+
+/// The `width` value type.
+#[cfg(feature = "servo")]
+pub type Width = ::values::specified::NonNegativeLengthOrPercentageOrAuto;
+
+/// The `width` value type.
+#[cfg(feature = "gecko")]
+pub type Width = ::values::specified::MozLength;
 
 /// A specified value for the `flex-basis` property.
-pub type FlexBasis = GenericFlexBasis<LengthOrPercentage>;
+pub type FlexBasis = GenericFlexBasis<Width>;
 
 impl Parse for FlexBasis {
     fn parse<'i, 't>(
         context: &ParserContext,
-        input: &mut Parser<'i, 't>)
+        input: &mut Parser<'i, 't>,
-    -> Result<Self, ParseError<'i>> {
+    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(length) = input.try(|i| LengthOrPercentage::parse_non_negative(context, i)) {
+        if let Ok(width) = input.try(|i| Width::parse(context, i)) {
-            return Ok(GenericFlexBasis::Length(length));
+            return Ok(GenericFlexBasis::Width(width));
         }
         try_match_ident_ignore_ascii_case! { input,
-            "auto" => Ok(GenericFlexBasis::Auto),
             "content" => Ok(GenericFlexBasis::Content),
         }
     }
 }
+
+impl FlexBasis {
+    /// `auto`
+    #[inline]
+    pub fn auto() -> Self {
+        GenericFlexBasis::Width(Width::auto())
+    }
+
+    /// `0%`
+    #[inline]
+    pub fn zero_percent() -> Self {
+        GenericFlexBasis::Width(Width::zero_percent())
+    }
+}

components/style/values/specified/length.rs

@@ -914,6 +914,16 @@ impl LengthOrPercentageOrAuto {
     pub fn zero_percent() -> Self {
         LengthOrPercentageOrAuto::Percentage(computed::Percentage::zero())
     }
+
+    /// Parses, with quirks.
+    #[inline]
+    pub fn parse_quirky<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+        allow_quirks: AllowQuirks,
+    ) -> Result<Self, ParseError<'i>> {
+        Self::parse_internal(context, input, AllowedNumericType::All, allow_quirks)
+    }
 }
 
 impl Parse for LengthOrPercentageOrAuto {
@@ -923,14 +933,33 @@ impl Parse for LengthOrPercentageOrAuto {
     }
 }
 
-impl LengthOrPercentageOrAuto {
+/// A wrapper of LengthOrPercentageOrAuto, whose value must be >= 0.
-    /// Parses, with quirks.
+pub type NonNegativeLengthOrPercentageOrAuto = NonNegative<LengthOrPercentageOrAuto>;
+
+impl NonNegativeLengthOrPercentageOrAuto {
+    /// 0
     #[inline]
-    pub fn parse_quirky<'i, 't>(context: &ParserContext,
+    pub fn zero() -> Self {
-                                input: &mut Parser<'i, 't>,
+        NonNegative(LengthOrPercentageOrAuto::zero())
-                                allow_quirks: AllowQuirks)
+    }
-                                -> Result<Self, ParseError<'i>> {
+
-        Self::parse_internal(context, input, AllowedNumericType::All, allow_quirks)
+    /// 0%
+    #[inline]
+    pub fn zero_percent() -> Self {
+        NonNegative(LengthOrPercentageOrAuto::zero_percent())
+    }
+
+    /// `auto`
+    #[inline]
+    pub fn auto() -> Self {
+        NonNegative(LengthOrPercentageOrAuto::Auto)
+    }
+}
+
+impl Parse for NonNegativeLengthOrPercentageOrAuto {
+    #[inline]
+    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
+        Ok(NonNegative(LengthOrPercentageOrAuto::parse_non_negative(context, input)?))
     }
 }
 
@@ -1090,8 +1119,11 @@ impl LengthOrNumber {
 }
 
 /// A value suitable for a `min-width` or `min-height` property.
-/// Unlike `max-width` or `max-height` properties, a MozLength can be
+///
-/// `auto`, and cannot be `none`.
+/// Unlike `max-width` or `max-height` properties, a MozLength can be `auto`,
+/// and cannot be `none`.
+///
+/// Note that it only accepts non-negative values.
 #[allow(missing_docs)]
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss)]
 pub enum MozLength {

components/style/values/specified/mod.rs

@@ -55,7 +55,7 @@ pub use self::length::{FontRelativeLength, Length, LengthOrNumber};
 pub use self::length::{LengthOrPercentage, LengthOrPercentageOrAuto};
 pub use self::length::{LengthOrPercentageOrNone, MaxLength, MozLength};
 pub use self::length::{NoCalcLength, ViewportPercentageLength};
-pub use self::length::NonNegativeLengthOrPercentage;
+pub use self::length::{NonNegativeLengthOrPercentage, NonNegativeLengthOrPercentageOrAuto};
 pub use self::list::{ListStyleImage, Quotes};
 #[cfg(feature = "gecko")]
 pub use self::list::ListStyleType;

tests/unit/style/properties/serialization.rs

@@ -8,7 +8,6 @@ use style::properties::{PropertyDeclaration, Importance};
 use style::properties::declaration_block::PropertyDeclarationBlock;
 use style::properties::parse_property_declaration_list;
 use style::values::{CustomIdent, RGBA};
-use style::values::generics::flex::FlexBasis;
 use style::values::specified::{BorderStyle, BorderSideWidth, Color};
 use style::values::specified::{Length, LengthOrPercentage, LengthOrPercentageOrAuto};
 use style::values::specified::NoCalcLength;
@@ -559,42 +558,6 @@ mod shorthand_serialization {
         }
     }
 
-    #[test]
-    fn flex_should_serialize_all_available_properties() {
-        use style::values::specified::{NonNegativeNumber, Percentage};
-
-        let mut properties = Vec::new();
-
-        let grow = NonNegativeNumber::new(2f32);
-        let shrink = NonNegativeNumber::new(3f32);
-        let basis =
-            FlexBasis::Length(Percentage::new(0.5f32).into());
-
-        properties.push(PropertyDeclaration::FlexGrow(grow));
-        properties.push(PropertyDeclaration::FlexShrink(shrink));
-        properties.push(PropertyDeclaration::FlexBasis(basis));
-
-        let serialization = shorthand_properties_to_string(properties);
-        assert_eq!(serialization, "flex: 2 3 50%;");
-    }
-
-    #[test]
-    fn flex_flow_should_serialize_all_available_properties() {
-        use style::properties::longhands::flex_direction::SpecifiedValue as FlexDirection;
-        use style::properties::longhands::flex_wrap::SpecifiedValue as FlexWrap;
-
-        let mut properties = Vec::new();
-
-        let direction = FlexDirection::Row;
-        let wrap = FlexWrap::Wrap;
-
-        properties.push(PropertyDeclaration::FlexDirection(direction));
-        properties.push(PropertyDeclaration::FlexWrap(wrap));
-
-        let serialization = shorthand_properties_to_string(properties);
-        assert_eq!(serialization, "flex-flow: row wrap;");
-    }
-
     mod background {
         use super::*;