Auto merge of #19850 - emilio:tidy-align, r=nox

style: Minor nits on the alignment properties.

I'm going to touch this in a bit, let's do it a bit less painful.

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

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

@@ -69,8 +69,8 @@ ${helpers.single_keyword("flex-wrap", "nowrap wrap wrap-reverse",
                              animation_value_type="discrete")}
 % else:
     ${helpers.predefined_type(name="justify-content",
-                              type="AlignJustifyContent",
+                              type="ContentDistribution",
-                              initial_value="specified::AlignJustifyContent::normal()",
+                              initial_value="specified::ContentDistribution::normal()",
                               spec="https://drafts.csswg.org/css-align/#propdef-justify-content",
                               extra_prefixes="webkit",
                               animation_value_type="discrete")}
@@ -90,8 +90,8 @@ ${helpers.single_keyword("flex-wrap", "nowrap wrap wrap-reverse",
                              animation_value_type="discrete")}
 % else:
     ${helpers.predefined_type(name="align-content",
-                              type="AlignJustifyContent",
+                              type="ContentDistribution",
-                              initial_value="specified::AlignJustifyContent::normal()",
+                              initial_value="specified::ContentDistribution::normal()",
                               spec="https://drafts.csswg.org/css-align/#propdef-align-content",
                               extra_prefixes="webkit",
                               animation_value_type="discrete")}
@@ -138,20 +138,20 @@ ${helpers.predefined_type("flex-shrink", "NonNegativeNumber",
                              animation_value_type="discrete")}
 % else:
     ${helpers.predefined_type(name="align-self",
-                              type="AlignJustifySelf",
+                              type="SelfAlignment",
-                              initial_value="specified::AlignJustifySelf::auto()",
+                              initial_value="specified::SelfAlignment::auto()",
                               spec="https://drafts.csswg.org/css-align/#align-self-property",
                               extra_prefixes="webkit",
                               animation_value_type="discrete")}
 
     ${helpers.predefined_type(name="justify-self",
-                              type="AlignJustifySelf",
+                              type="SelfAlignment",
-                              initial_value="specified::AlignJustifySelf::auto()",
+                              initial_value="specified::SelfAlignment::auto()",
                               spec="https://drafts.csswg.org/css-align/#justify-self-property",
                               animation_value_type="discrete")}
 
     #[cfg(feature = "gecko")]
-    impl_align_conversions!(::values::specified::align::AlignJustifySelf);
+    impl_align_conversions!(::values::specified::align::SelfAlignment);
 % endif
 
 // https://drafts.csswg.org/css-flexbox/#propdef-order

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

@@ -615,18 +615,18 @@
 <%helpers:shorthand name="place-content" sub_properties="align-content justify-content"
                     spec="https://drafts.csswg.org/css-align/#propdef-place-content"
                     products="gecko">
-    use values::specified::align::{AlignJustifyContent, FallbackAllowed};
+    use values::specified::align::{ContentDistribution, FallbackAllowed};
 
     pub fn parse_value<'i, 't>(
         _: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
-        let align = AlignJustifyContent::parse_with_fallback(input, FallbackAllowed::No)?;
+        let align = ContentDistribution::parse_with_fallback(input, FallbackAllowed::No)?;
         if align.has_extra_flags() {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
         let justify =
-            input.try(|input| AlignJustifyContent::parse_with_fallback(input, FallbackAllowed::No))
+            input.try(|input| ContentDistribution::parse_with_fallback(input, FallbackAllowed::No))
                 .unwrap_or(align);
         if justify.has_extra_flags() {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
@@ -653,16 +653,16 @@
 <%helpers:shorthand name="place-self" sub_properties="align-self justify-self"
                     spec="https://drafts.csswg.org/css-align/#place-self-property"
                     products="gecko">
-    use values::specified::align::AlignJustifySelf;
+    use values::specified::align::SelfAlignment;
     use parser::Parse;
 
     pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                -> Result<Longhands, ParseError<'i>> {
-        let align = AlignJustifySelf::parse(context, input)?;
+        let align = SelfAlignment::parse(context, input)?;
         if align.has_extra_flags() {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
-        let justify = input.try(|input| AlignJustifySelf::parse(context, input)).unwrap_or(align.clone());
+        let justify = input.try(|input| SelfAlignment::parse(context, input)).unwrap_or(align.clone());
         if justify.has_extra_flags() {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }

components/style/values/computed/align.rs

@@ -11,7 +11,7 @@ use style_traits::{CssWriter, ToCss};
 use values::computed::{Context, ToComputedValue};
 use values::specified;
 
-pub use super::specified::{AlignItems, AlignJustifyContent, AlignJustifySelf};
+pub use super::specified::{AlignItems, ContentDistribution, SelfAlignment};
 
 /// The computed value for the `justify-items` property.
 ///

components/style/values/computed/mod.rs

@@ -32,7 +32,7 @@ use super::specified;
 pub use app_units::Au;
 pub use properties::animated_properties::TransitionProperty;
 #[cfg(feature = "gecko")]
-pub use self::align::{AlignItems, AlignJustifyContent, AlignJustifySelf, JustifyItems};
+pub use self::align::{AlignItems, ContentDistribution, SelfAlignment, JustifyItems};
 pub use self::angle::Angle;
 pub use self::background::{BackgroundSize, BackgroundRepeat};
 pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageSideWidth};

components/style/values/specified/align.rs

@@ -115,12 +115,12 @@ const ALIGN_ALL_SHIFT: u32 = structs::NS_STYLE_ALIGN_ALL_SHIFT;
 /// Value of the `align-content` or `justify-content` property.
 ///
 /// <https://drafts.csswg.org/css-align/#content-distribution>
-///
-/// The 16-bit field stores the primary value in its lower 8 bits, and the optional fallback value
-/// in its upper 8 bits.  This matches the representation of these properties in Gecko.
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
-pub struct AlignJustifyContent(u16);
+pub struct ContentDistribution {
+    primary: AlignFlags,
+    fallback: AlignFlags,
+}
 
 /// Whether fallback is allowed in align-content / justify-content parsing.
 ///
@@ -136,8 +136,7 @@ pub enum FallbackAllowed {
     No,
 }
 
-
+impl ContentDistribution {
-impl AlignJustifyContent {
     /// The initial value 'normal'
     #[inline]
     pub fn normal() -> Self {
@@ -147,35 +146,34 @@ impl AlignJustifyContent {
     /// Construct a value with no fallback.
     #[inline]
     pub fn new(flags: AlignFlags) -> Self {
-        AlignJustifyContent(flags.bits() as u16)
+        Self::with_fallback(flags, AlignFlags::empty())
     }
 
     /// Construct a value including a fallback alignment.
     ///
     /// <https://drafts.csswg.org/css-align/#fallback-alignment>
     #[inline]
-    pub fn with_fallback(flags: AlignFlags, fallback: AlignFlags) -> Self {
+    pub fn with_fallback(primary: AlignFlags, fallback: AlignFlags) -> Self {
-        AlignJustifyContent(flags.bits() as u16 | ((fallback.bits() as u16) << ALIGN_ALL_SHIFT))
+        Self { primary, fallback }
     }
 
     /// The primary alignment
     #[inline]
     pub fn primary(self) -> AlignFlags {
-        AlignFlags::from_bits((self.0 & ALIGN_ALL_BITS) as u8)
+        self.primary
-            .expect("AlignJustifyContent must contain valid flags")
     }
 
     /// The fallback alignment
     #[inline]
     pub fn fallback(self) -> AlignFlags {
-        AlignFlags::from_bits((self.0 >> ALIGN_ALL_SHIFT) as u8)
+        self.fallback
-            .expect("AlignJustifyContent must contain valid flags")
     }
 
     /// Whether this value has extra flags.
     #[inline]
     pub fn has_extra_flags(self) -> bool {
-        self.primary().intersects(AlignFlags::FLAG_BITS) || self.fallback().intersects(AlignFlags::FLAG_BITS)
+        self.primary().intersects(AlignFlags::FLAG_BITS) ||
+        self.fallback().intersects(AlignFlags::FLAG_BITS)
     }
 
     /// Parse a value for align-content / justify-content, optionally allowing
@@ -186,32 +184,32 @@ impl AlignJustifyContent {
     ) -> Result<Self, ParseError<'i>> {
         // normal | <baseline-position>
         if let Ok(value) = input.try(|input| parse_normal_or_baseline(input)) {
-            return Ok(AlignJustifyContent::new(value))
+            return Ok(ContentDistribution::new(value))
         }
 
         // <content-distribution> followed by optional <*-position>
         if let Ok(value) = input.try(|input| parse_content_distribution(input)) {
             if fallback_allowed == FallbackAllowed::Yes {
                 if let Ok(fallback) = input.try(|input| parse_overflow_content_position(input)) {
-                    return Ok(AlignJustifyContent::with_fallback(value, fallback))
+                    return Ok(ContentDistribution::with_fallback(value, fallback))
                 }
             }
-            return Ok(AlignJustifyContent::new(value))
+            return Ok(ContentDistribution::new(value))
         }
 
         // <*-position> followed by optional <content-distribution>
         let fallback = parse_overflow_content_position(input)?;
         if fallback_allowed == FallbackAllowed::Yes {
             if let Ok(value) = input.try(|input| parse_content_distribution(input)) {
-                return Ok(AlignJustifyContent::with_fallback(value, fallback))
+                return Ok(ContentDistribution::with_fallback(value, fallback))
             }
         }
 
-        Ok(AlignJustifyContent::new(fallback))
+        Ok(ContentDistribution::new(fallback))
     }
 }
 
-impl ToCss for AlignJustifyContent {
+impl ToCss for ContentDistribution {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
@@ -229,7 +227,7 @@ impl ToCss for AlignJustifyContent {
 }
 
 
-impl Parse for AlignJustifyContent {
+impl Parse for ContentDistribution {
     // normal | <baseline-position> |
     // [ <content-distribution> || [ <overflow-position>? && <content-position> ] ]
     fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
@@ -242,13 +240,13 @@ impl Parse for AlignJustifyContent {
 /// <https://drafts.csswg.org/css-align/#self-alignment>
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
 #[derive(Clone, Copy, Debug, Eq, PartialEq, ToComputedValue, ToCss)]
-pub struct AlignJustifySelf(pub AlignFlags);
+pub struct SelfAlignment(pub AlignFlags);
 
-impl AlignJustifySelf {
+impl SelfAlignment {
     /// The initial value 'auto'
     #[inline]
     pub fn auto() -> Self {
-        AlignJustifySelf(AlignFlags::AUTO)
+        SelfAlignment(AlignFlags::AUTO)
     }
 
     /// Whether this value has extra flags.
@@ -259,19 +257,16 @@ impl AlignJustifySelf {
 }
 
 
-impl Parse for AlignJustifySelf {
+impl Parse for SelfAlignment {
     // auto | normal | stretch | <baseline-position> |
     // [ <overflow-position>? && <self-position> ]
     fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         // auto | normal | stretch | <baseline-position>
         if let Ok(value) = input.try(parse_auto_normal_stretch_baseline) {
-            return Ok(AlignJustifySelf(value))
+            return Ok(SelfAlignment(value))
         }
         // [ <overflow-position>? && <self-position> ]
-        if let Ok(value) = input.try(parse_overflow_self_position) {
+        Ok(SelfAlignment(parse_overflow_self_position(input)?))
-            return Ok(AlignJustifySelf(value))
-        }
-        Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 }
 
@@ -306,10 +301,7 @@ impl Parse for AlignItems {
             return Ok(AlignItems(value))
         }
         // [ <overflow-position>? && <self-position> ]
-        if let Ok(value) = input.try(parse_overflow_self_position) {
+        Ok(AlignItems(parse_overflow_self_position(input)?))
-            return Ok(AlignItems(value))
-        }
-        Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 }
 
@@ -355,30 +347,33 @@ impl Parse for JustifyItems {
             return Ok(JustifyItems(value))
         }
         // [ <overflow-position>? && <self-position> ]
-        if let Ok(value) = parse_overflow_self_position(input) {
+        Ok(JustifyItems(parse_overflow_self_position(input)?))
-            return Ok(JustifyItems(value))
-        }
-        Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 }
 
 #[cfg(feature = "gecko")]
-impl From<u16> for AlignJustifyContent {
+impl From<u16> for ContentDistribution {
-    fn from(bits: u16) -> AlignJustifyContent {
+    fn from(bits: u16) -> ContentDistribution {
-        AlignJustifyContent(bits)
+        let primary =
+            AlignFlags::from_bits_truncate((bits & ALIGN_ALL_BITS) as u8);
+        let fallback =
+            AlignFlags::from_bits_truncate((bits >> ALIGN_ALL_SHIFT) as u8);
+        ContentDistribution::with_fallback(primary, fallback)
     }
 }
 
 #[cfg(feature = "gecko")]
-impl From<AlignJustifyContent> for u16 {
+impl From<ContentDistribution> for u16 {
-    fn from(v: AlignJustifyContent) -> u16 {
+    fn from(v: ContentDistribution) -> u16 {
-        v.0
+        v.primary().bits() as u16 |
+        ((v.fallback().bits() as u16) << ALIGN_ALL_SHIFT)
     }
 }
 
 // auto | normal | stretch | <baseline-position>
-fn parse_auto_normal_stretch_baseline<'i, 't>(input: &mut Parser<'i, 't>)
+fn parse_auto_normal_stretch_baseline<'i, 't>(
-                                              -> Result<AlignFlags, ParseError<'i>> {
+    input: &mut Parser<'i, 't>,
+) -> Result<AlignFlags, ParseError<'i>> {
     if let Ok(baseline) = input.try(parse_baseline) {
         return Ok(baseline);
     }

components/style/values/specified/mod.rs

@@ -26,7 +26,7 @@ use values::specified::calc::CalcNode;
 pub use properties::animated_properties::TransitionProperty;
 pub use self::angle::Angle;
 #[cfg(feature = "gecko")]
-pub use self::align::{AlignItems, AlignJustifyContent, AlignJustifySelf, JustifyItems};
+pub use self::align::{AlignItems, ContentDistribution, SelfAlignment, JustifyItems};
 pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth};
 pub use self::border::{BorderImageSideWidth, BorderRadius, BorderSideWidth, BorderSpacing};