style: Improve #[derive(Parse)].

I want to do this so that I can get rid of Either<>. The reasons for getting rid
of either are multiple:

 * It doesn't generate as nice C++ code using cbindgen.
 * It isn't that nice to use either from Rust.
 * cbindgen has bugs with zero-sized types.

I started using this for ColorOrAuto and a few others, for now.

Differential Revision: https://phabricator.services.mozilla.com/D19844

components/style/gecko_bindings/sugar/style_complex_color.rs

@@ -7,10 +7,8 @@
 use crate::gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor};
 use crate::gecko_bindings::structs::StyleComplexColor;
 use crate::gecko_bindings::structs::StyleComplexColor_Tag as Tag;
-use crate::values::computed::ui::ColorOrAuto;
+use crate::values::computed::{Color as ComputedColor, ColorOrAuto, RGBAColor as ComputedRGBA};
-use crate::values::computed::{Color as ComputedColor, RGBAColor as ComputedRGBA};
+use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto, ComplexColorRatios};
-use crate::values::generics::color::{Color as GenericColor, ComplexColorRatios};
-use crate::values::{Auto, Either};
 
 impl StyleComplexColor {
     /// Create a `StyleComplexColor` value that represents `currentColor`.
@@ -94,8 +92,8 @@ impl From<StyleComplexColor> for ComputedColor {
 impl From<ColorOrAuto> for StyleComplexColor {
     fn from(other: ColorOrAuto) -> Self {
         match other {
-            Either::First(color) => color.into(),
+            GenericColorOrAuto::Color(color) => color.into(),
-            Either::Second(_) => StyleComplexColor::auto(),
+            GenericColorOrAuto::Auto => StyleComplexColor::auto(),
         }
     }
 }
@@ -103,9 +101,9 @@ impl From<ColorOrAuto> for StyleComplexColor {
 impl From<StyleComplexColor> for ColorOrAuto {
     fn from(other: StyleComplexColor) -> Self {
         if other.mTag != Tag::eAuto {
-            Either::First(other.into())
+            GenericColorOrAuto::Color(other.into())
         } else {
-            Either::Second(Auto)
+            GenericColorOrAuto::Auto
         }
     }
 }

components/style/properties/longhands/inherited_ui.mako.rs

@@ -60,7 +60,7 @@ ${helpers.single_keyword(
 ${helpers.predefined_type(
     "caret-color",
     "ColorOrAuto",
-    "Either::Second(Auto)",
+    "generics::color::ColorOrAuto::Auto",
     spec="https://drafts.csswg.org/css-ui/#caret-color",
     animation_value_type="AnimatedCaretColor",
     ignored_when_colors_disabled=True,

components/style/values/computed/color.rs

@@ -6,7 +6,7 @@
 
 use crate::values::animated::color::RGBA as AnimatedRGBA;
 use crate::values::animated::ToAnimatedValue;
-use crate::values::generics::color::Color as GenericColor;
+use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto};
 use cssparser::{Color as CSSParserColor, RGBA};
 use std::fmt;
 use style_traits::{CssWriter, ToCss};
@@ -101,3 +101,6 @@ impl ToAnimatedValue for RGBA {
         RGBA::from_floats(animated.red, animated.green, animated.blue, animated.alpha)
     }
 }
+
+/// auto | <color>
+pub type ColorOrAuto = GenericColorOrAuto<Color>;

components/style/values/computed/mod.rs

@@ -45,7 +45,7 @@ pub use self::box_::{Appearance, BreakBetween, BreakWithin, Clear, Float};
 pub use self::box_::{Display, Overflow, OverflowAnchor, TransitionProperty};
 pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize};
 pub use self::box_::{ScrollSnapAlign, ScrollSnapType, TouchAction, VerticalAlign, WillChange};
-pub use self::color::{Color, ColorPropertyValue, RGBAColor};
+pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor};
 pub use self::column::ColumnCount;
 pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset};
 pub use self::easing::TimingFunction;
@@ -85,7 +85,7 @@ pub use self::transform::{Rotate, Scale, Transform, TransformOperation};
 pub use self::transform::{TransformOrigin, TransformStyle, Translate};
 #[cfg(feature = "gecko")]
 pub use self::ui::CursorImage;
-pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect};
+pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect};
 pub use super::specified::{BorderStyle, TextDecorationLine};
 pub use super::{Auto, Either, None_};
 pub use app_units::Au;

components/style/values/computed/ui.rs

@@ -8,14 +8,10 @@ use crate::values::computed::color::Color;
 use crate::values::computed::url::ComputedImageUrl;
 use crate::values::computed::Number;
 use crate::values::generics::ui as generics;
-use crate::values::{Auto, Either};
 
 pub use crate::values::specified::ui::CursorKind;
 pub use crate::values::specified::ui::{MozForceBrokenImageIcon, UserSelect};
 
-/// auto | <color>
-pub type ColorOrAuto = Either<Color, Auto>;
-
 /// A computed value for the `cursor` property.
 pub type Cursor = generics::Cursor<CursorImage>;
 

components/style/values/generics/color.rs

@@ -74,3 +74,26 @@ impl<RGBA> From<RGBA> for Color<RGBA> {
         Self::rgba(color)
     }
 }
+
+/// Either `<color>` or `auto`.
+#[derive(
+    Animate,
+    Clone,
+    ComputeSquaredDistance,
+    Copy,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    Parse,
+    SpecifiedValueInfo,
+    ToAnimatedValue,
+    ToAnimatedZero,
+    ToComputedValue,
+    ToCss,
+)]
+pub enum ColorOrAuto<C> {
+    /// A `<color>
+    Color(C),
+    /// `auto`
+    Auto,
+}

components/style/values/generics/column.rs

@@ -12,6 +12,7 @@
     Copy,
     Debug,
     MallocSizeOf,
+    Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToAnimatedValue,

components/style/values/generics/flex.rs

@@ -12,6 +12,7 @@
     ComputeSquaredDistance,
     Copy,
     Debug,
+    Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToAnimatedValue,

components/style/values/generics/length.rs

@@ -165,6 +165,7 @@ impl<LengthPercentage> MaxSize<LengthPercentage> {
     Copy,
     Debug,
     MallocSizeOf,
+    Parse,
     PartialEq,
     SpecifiedValueInfo,
     ToAnimatedValue,
@@ -174,29 +175,17 @@ impl<LengthPercentage> MaxSize<LengthPercentage> {
 )]
 #[repr(C, u8)]
 pub enum GenericLengthOrNumber<L, N> {
+    /// A number.
+    ///
+    /// NOTE: Numbers need to be before lengths, in order to parse them
+    /// first, since `0` should be a number, not the `0px` length.
+    Number(N),
     /// A length.
     Length(L),
-    /// A number.
-    Number(N),
 }
 
 pub use self::GenericLengthOrNumber as LengthOrNumber;
 
-impl<L: Parse, N: Parse> Parse for LengthOrNumber<L, N> {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(number) = input.try(|i| N::parse(context, i)) {
-            // Numbers need to be parsed first because `0` must be recognised
-            // as the number `0` and not the length `0px`.
-            return Ok(LengthOrNumber::Number(number));
-        }
-
-        Ok(LengthOrNumber::Length(L::parse(context, input)?))
-    }
-}
-
 impl<L, N> LengthOrNumber<L, N> {
     /// Returns `0`.
     pub fn zero() -> Self

components/style/values/generics/mod.rs

@@ -119,27 +119,25 @@ impl Parse for CounterStyleOrNone {
         if input.try(|i| i.expect_ident_matching("none")).is_ok() {
             return Ok(CounterStyleOrNone::None);
         }
-        if input.try(|i| i.expect_function_matching("symbols")).is_ok() {
+        input.expect_function_matching("symbols")?;
-            return input.parse_nested_block(|input| {
+        input.parse_nested_block(|input| {
-                let symbols_type = input
+            let symbols_type = input
-                    .try(|i| SymbolsType::parse(i))
+                .try(SymbolsType::parse)
-                    .unwrap_or(SymbolsType::Symbolic);
+                .unwrap_or(SymbolsType::Symbolic);
-                let symbols = Symbols::parse(context, input)?;
+            let symbols = Symbols::parse(context, input)?;
-                // There must be at least two symbols for alphabetic or
+            // There must be at least two symbols for alphabetic or
-                // numeric system.
+            // numeric system.
-                if (symbols_type == SymbolsType::Alphabetic ||
+            if (symbols_type == SymbolsType::Alphabetic ||
-                    symbols_type == SymbolsType::Numeric) && symbols.0.len() < 2
+                symbols_type == SymbolsType::Numeric) && symbols.0.len() < 2
-                {
+            {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                }
+            }
-                // Identifier is not allowed in symbols() function.
+            // Identifier is not allowed in symbols() function.
-                if symbols.0.iter().any(|sym| !sym.is_allowed_in_symbols()) {
+            if symbols.0.iter().any(|sym| !sym.is_allowed_in_symbols()) {
-                    return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
+                return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-                }
+            }
-                Ok(CounterStyleOrNone::Symbols(symbols_type, symbols))
+            Ok(CounterStyleOrNone::Symbols(symbols_type, symbols))
-            });
+        })
-        }
-        Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
 }
 

components/style/values/generics/svg.rs

@@ -136,6 +136,7 @@ impl<ColorType: Parse, UrlPaintServer: Parse> Parse for SVGPaint<ColorType, UrlP
     Debug,
     MallocSizeOf,
     PartialEq,
+    Parse,
     SpecifiedValueInfo,
     ToAnimatedValue,
     ToAnimatedZero,
@@ -143,28 +144,26 @@ impl<ColorType: Parse, UrlPaintServer: Parse> Parse for SVGPaint<ColorType, UrlP
     ToCss,
 )]
 pub enum SvgLengthPercentageOrNumber<LengthPercentage, Number> {
+    /// <number>
+    ///
+    /// Note that this needs to be before, so it gets parsed before the length,
+    /// to handle `0` correctly as a number instead of a `<length>`.
+    Number(Number),
     /// <length> | <percentage>
     LengthPercentage(LengthPercentage),
-    /// <number>
-    Number(Number),
 }
 
-/// Parsing the SvgLengthPercentageOrNumber. At first, we need to parse number
+/// Whether the `context-value` value is enabled.
-/// since prevent converting to the length.
+#[cfg(feature = "gecko")]
-impl<LengthPercentageType: Parse, NumberType: Parse> Parse
+pub fn is_context_value_enabled(_: &ParserContext) -> bool {
-    for SvgLengthPercentageOrNumber<LengthPercentageType, NumberType>
+    use crate::gecko_bindings::structs::mozilla;
-{
+    unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled }
-    fn parse<'i, 't>(
+}
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(num) = input.try(|i| NumberType::parse(context, i)) {
-            return Ok(SvgLengthPercentageOrNumber::Number(num));
-        }
 
-        let lp = LengthPercentageType::parse(context, input)?;
+/// Whether the `context-value` value is enabled.
-        Ok(SvgLengthPercentageOrNumber::LengthPercentage(lp))
+#[cfg(not(feature = "gecko"))]
-    }
+pub fn is_context_value_enabled(_: &ParserContext) -> bool {
+    false
 }
 
 /// An SVG length value supports `context-value` in addition to length.
@@ -175,6 +174,7 @@ impl<LengthPercentageType: Parse, NumberType: Parse> Parse
     Debug,
     MallocSizeOf,
     PartialEq,
+    Parse,
     SpecifiedValueInfo,
     ToAnimatedValue,
     ToAnimatedZero,
@@ -185,6 +185,7 @@ pub enum SVGLength<LengthType> {
     /// `<length> | <percentage> | <number>`
     Length(LengthType),
     /// `context-value`
+    #[parse(condition = "is_context_value_enabled")]
     ContextValue,
 }
 
@@ -216,6 +217,7 @@ pub enum SVGStrokeDashArray<LengthType> {
     Debug,
     MallocSizeOf,
     PartialEq,
+    Parse,
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,

components/style/values/generics/url.rs

@@ -4,10 +4,6 @@
 
 //! Generic types for url properties.
 
-use crate::parser::{Parse, ParserContext};
-use cssparser::Parser;
-use style_traits::ParseError;
-
 /// An image url or none, used for example in list-style-image
 #[derive(
     Animate,
@@ -16,6 +12,7 @@ use style_traits::ParseError;
     Debug,
     MallocSizeOf,
     PartialEq,
+    Parse,
     SpecifiedValueInfo,
     ToAnimatedValue,
     ToAnimatedZero,
@@ -35,19 +32,3 @@ impl<Url> UrlOrNone<Url> {
         UrlOrNone::None
     }
 }
-
-impl<Url> Parse for UrlOrNone<Url>
-where
-    Url: Parse,
-{
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<UrlOrNone<Url>, ParseError<'i>> {
-        if let Ok(url) = input.try(|input| Url::parse(context, input)) {
-            return Ok(UrlOrNone::Url(url));
-        }
-        input.expect_ident_matching("none")?;
-        Ok(UrlOrNone::None)
-    }
-}

components/style/values/mod.rs

@@ -125,6 +125,7 @@ impl Parse for Impossible {
     Copy,
     MallocSizeOf,
     PartialEq,
+    Parse,
     SpecifiedValueInfo,
     ToAnimatedValue,
     ToAnimatedZero,
@@ -147,19 +148,6 @@ impl<A: Debug, B: Debug> Debug for Either<A, B> {
     }
 }
 
-impl<A: Parse, B: Parse> Parse for Either<A, B> {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Either<A, B>, ParseError<'i>> {
-        if let Ok(v) = input.try(|i| A::parse(context, i)) {
-            Ok(Either::First(v))
-        } else {
-            B::parse(context, input).map(Either::Second)
-        }
-    }
-}
-
 /// <https://drafts.csswg.org/css-values-4/#custom-idents>
 #[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToComputedValue)]
 pub struct CustomIdent(pub Atom);

components/style/values/specified/color.rs

@@ -11,7 +11,7 @@ use crate::parser::{Parse, ParserContext};
 #[cfg(feature = "gecko")]
 use crate::properties::longhands::system_colors::SystemColor;
 use crate::values::computed::{Color as ComputedColor, Context, ToComputedValue};
-use crate::values::generics::color::Color as GenericColor;
+use crate::values::generics::color::{Color as GenericColor, ColorOrAuto as GenericColorOrAuto};
 use crate::values::specified::calc::CalcNode;
 use cssparser::{AngleOrNumber, Color as CSSParserColor, Parser, Token, RGBA};
 use cssparser::{BasicParseErrorKind, NumberOrPercentage, ParseErrorKind};
@@ -469,3 +469,6 @@ impl Parse for ColorPropertyValue {
         Color::parse_quirky(context, input, AllowQuirks::Yes).map(ColorPropertyValue)
     }
 }
+
+/// auto | <color>
+pub type ColorOrAuto = GenericColorOrAuto<Color>;

components/style/values/specified/column.rs

@@ -4,25 +4,8 @@
 
 //! Specified types for the column properties.
 
-use crate::parser::{Parse, ParserContext};
 use crate::values::generics::column::ColumnCount as GenericColumnCount;
 use crate::values::specified::PositiveInteger;
-use cssparser::Parser;
-use style_traits::ParseError;
 
 /// A specified type for `column-count` values.
 pub type ColumnCount = GenericColumnCount<PositiveInteger>;
-
-impl Parse for ColumnCount {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if input.try(|i| i.expect_ident_matching("auto")).is_ok() {
-            return Ok(GenericColumnCount::Auto);
-        }
-        Ok(GenericColumnCount::Integer(PositiveInteger::parse(
-            context, input,
-        )?))
-    }
-}

components/style/values/specified/flex.rs

@@ -4,29 +4,12 @@
 
 //! Specified types for CSS values related to flexbox.
 
-use crate::parser::{Parse, ParserContext};
 use crate::values::generics::flex::FlexBasis as GenericFlexBasis;
 use crate::values::specified::Size;
-use cssparser::Parser;
-use style_traits::ParseError;
 
 /// A specified value for the `flex-basis` property.
 pub type FlexBasis = GenericFlexBasis<Size>;
 
-impl Parse for FlexBasis {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(size) = input.try(|i| Size::parse(context, i)) {
-            return Ok(GenericFlexBasis::Size(size));
-        }
-        try_match_ident_ignore_ascii_case! { input,
-            "content" => Ok(GenericFlexBasis::Content),
-        }
-    }
-}
-
 impl FlexBasis {
     /// `auto`
     #[inline]

components/style/values/specified/font.rs

@@ -17,7 +17,7 @@ use crate::values::generics::font::{KeywordSize, VariationValue};
 use crate::values::generics::NonNegative;
 use crate::values::specified::length::{FontBaseSize, AU_PER_PT, AU_PER_PX};
 use crate::values::specified::{AllowQuirks, Angle, Integer, LengthPercentage};
-use crate::values::specified::{NoCalcLength, Number, Percentage};
+use crate::values::specified::{NoCalcLength, Number, NonNegativeNumber, Percentage};
 use crate::values::CustomIdent;
 use crate::Atom;
 use app_units::Au;
@@ -81,7 +81,7 @@ pub const MAX_FONT_WEIGHT: f32 = 1000.;
 /// A specified font-weight value.
 ///
 /// https://drafts.csswg.org/css-fonts-4/#propdef-font-weight
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)]
 pub enum FontWeight {
     /// `<font-weight-absolute>`
     Absolute(AbsoluteFontWeight),
@@ -111,22 +111,6 @@ impl FontWeight {
     }
 }
 
-impl Parse for FontWeight {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<FontWeight, ParseError<'i>> {
-        if let Ok(absolute) = input.try(|input| AbsoluteFontWeight::parse(context, input)) {
-            return Ok(FontWeight::Absolute(absolute));
-        }
-
-        Ok(try_match_ident_ignore_ascii_case! { input,
-            "bolder" => FontWeight::Bolder,
-            "lighter" => FontWeight::Lighter,
-        })
-    }
-}
-
 impl ToComputedValue for FontWeight {
     type ComputedValue = computed::FontWeight;
 
@@ -335,7 +319,7 @@ impl SpecifiedFontStyle {
 }
 
 /// The specified value of the `font-style` property.
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)]
 #[allow(missing_docs)]
 pub enum FontStyle {
     Specified(SpecifiedFontStyle),
@@ -368,20 +352,11 @@ impl ToComputedValue for FontStyle {
     }
 }
 
-impl Parse for FontStyle {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        Ok(FontStyle::Specified(SpecifiedFontStyle::parse(
-            context, input,
-        )?))
-    }
-}
-
 /// A value for the `font-stretch` property.
 ///
 /// https://drafts.csswg.org/css-fonts-4/#font-stretch-prop
+///
+/// TODO(emilio): We could derive Parse if we had NonNegativePercentage.
 #[allow(missing_docs)]
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
 #[repr(u8)]
@@ -628,13 +603,13 @@ impl Parse for FamilyName {
     }
 }
 
-#[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
+#[derive(Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToCss)]
 /// Preserve the readability of text when font fallback occurs
 pub enum FontSizeAdjust {
     /// None variant
     None,
     /// Number variant
-    Number(Number),
+    Number(NonNegativeNumber),
     /// system font
     #[css(skip)]
     System(SystemFont),
@@ -657,7 +632,9 @@ impl ToComputedValue for FontSizeAdjust {
         match *self {
             FontSizeAdjust::None => computed::FontSizeAdjust::None,
             FontSizeAdjust::Number(ref n) => {
-                computed::FontSizeAdjust::Number(n.to_computed_value(context))
+                // The computed version handles clamping of animated values
+                // itself.
+                computed::FontSizeAdjust::Number(n.to_computed_value(context).0)
             },
             FontSizeAdjust::System(_) => self.compute_system(context),
         }
@@ -666,32 +643,13 @@ impl ToComputedValue for FontSizeAdjust {
     fn from_computed_value(computed: &computed::FontSizeAdjust) -> Self {
         match *computed {
             computed::FontSizeAdjust::None => FontSizeAdjust::None,
-            computed::FontSizeAdjust::Number(ref v) => {
+            computed::FontSizeAdjust::Number(v) => {
-                FontSizeAdjust::Number(Number::from_computed_value(v))
+                FontSizeAdjust::Number(NonNegativeNumber::from_computed_value(&v.into()))
             },
         }
     }
 }
 
-impl Parse for FontSizeAdjust {
-    /// none | <number>
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<FontSizeAdjust, ParseError<'i>> {
-        if input
-            .try(|input| input.expect_ident_matching("none"))
-            .is_ok()
-        {
-            return Ok(FontSizeAdjust::None);
-        }
-
-        Ok(FontSizeAdjust::Number(Number::parse_non_negative(
-            context, input,
-        )?))
-    }
-}
-
 /// Additional information for specified keyword-derived font sizes.
 pub type KeywordInfo = generics::KeywordInfo<NonNegativeLength>;
 

components/style/values/specified/image.rs

@@ -305,7 +305,7 @@ impl Gradient {
     ) -> Result<Self, ParseError<'i>> {
         type Point = GenericPosition<Component<X>, Component<Y>>;
 
-        #[derive(Clone, Copy)]
+        #[derive(Clone, Copy, Parse)]
         enum Component<S> {
             Center,
             Number(NumberOrPercentage),
@@ -408,22 +408,6 @@ impl Gradient {
             }
         }
 
-        impl<S: Parse> Parse for Component<S> {
-            fn parse<'i, 't>(
-                context: &ParserContext,
-                input: &mut Parser<'i, 't>,
-            ) -> Result<Self, ParseError<'i>> {
-                if let Ok(side) = input.try(|i| S::parse(context, i)) {
-                    return Ok(Component::Side(side));
-                }
-                if let Ok(number) = input.try(|i| NumberOrPercentage::parse(context, i)) {
-                    return Ok(Component::Number(number));
-                }
-                input.try(|i| i.expect_ident_matching("center"))?;
-                Ok(Component::Center)
-            }
-        }
-
         let ident = input.expect_ident_cloned()?;
         input.expect_comma()?;
 

components/style/values/specified/mod.rs

@@ -42,7 +42,7 @@ pub use self::box_::{Clear, Float, Overflow, OverflowAnchor};
 pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize};
 pub use self::box_::{ScrollSnapAlign, ScrollSnapType};
 pub use self::box_::{TouchAction, TransitionProperty, VerticalAlign, WillChange};
-pub use self::color::{Color, ColorPropertyValue, RGBAColor};
+pub use self::color::{Color, ColorOrAuto, ColorPropertyValue, RGBAColor};
 pub use self::column::ColumnCount;
 pub use self::counters::{Content, ContentItem, CounterIncrement, CounterReset};
 pub use self::easing::TimingFunction;
@@ -87,7 +87,7 @@ pub use self::transform::{Rotate, Scale, Transform};
 pub use self::transform::{TransformOrigin, TransformStyle, Translate};
 #[cfg(feature = "gecko")]
 pub use self::ui::CursorImage;
-pub use self::ui::{ColorOrAuto, Cursor, MozForceBrokenImageIcon, UserSelect};
+pub use self::ui::{Cursor, MozForceBrokenImageIcon, UserSelect};
 pub use super::generics::grid::GridTemplateComponent as GenericGridTemplateComponent;
 
 #[cfg(feature = "gecko")]

components/style/values/specified/svg.rs

@@ -23,31 +23,6 @@ pub type SVGPaint = generic::SVGPaint<Color, SpecifiedUrl>;
 /// Specified SVG Paint Kind value
 pub type SVGPaintKind = generic::SVGPaintKind<Color, SpecifiedUrl>;
 
-#[cfg(feature = "gecko")]
-fn is_context_value_enabled() -> bool {
-    // The prefs can only be mutated on the main thread, so it is safe
-    // to read whenever we are on the main thread or the main thread is
-    // blocked.
-    use crate::gecko_bindings::structs::mozilla;
-    unsafe { mozilla::StaticPrefs_sVarCache_gfx_font_rendering_opentype_svg_enabled }
-}
-#[cfg(not(feature = "gecko"))]
-fn is_context_value_enabled() -> bool {
-    false
-}
-
-fn parse_context_value<'i, 't, T>(
-    input: &mut Parser<'i, 't>,
-    value: T,
-) -> Result<T, ParseError<'i>> {
-    if !is_context_value_enabled() {
-        return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-    }
-
-    input.expect_ident_matching("context-value")?;
-    Ok(value)
-}
-
 /// A value of <length> | <percentage> | <number> for stroke-dashoffset.
 /// <https://www.w3.org/TR/SVG11/painting.html#StrokeProperties>
 pub type SvgLengthPercentageOrNumber =
@@ -56,18 +31,6 @@ pub type SvgLengthPercentageOrNumber =
 /// <length> | <percentage> | <number> | context-value
 pub type SVGLength = generic::SVGLength<SvgLengthPercentageOrNumber>;
 
-impl Parse for SVGLength {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        input
-            .try(|i| SvgLengthPercentageOrNumber::parse(context, i))
-            .map(Into::into)
-            .or_else(|_| parse_context_value(input, generic::SVGLength::ContextValue))
-    }
-}
-
 impl From<SvgLengthPercentageOrNumber> for SVGLength {
     fn from(length: SvgLengthPercentageOrNumber) -> Self {
         generic::SVGLength::Length(length)
@@ -82,18 +45,6 @@ pub type NonNegativeSvgLengthPercentageOrNumber =
 /// A non-negative version of SVGLength.
 pub type SVGWidth = generic::SVGLength<NonNegativeSvgLengthPercentageOrNumber>;
 
-impl Parse for SVGWidth {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        input
-            .try(|i| NonNegativeSvgLengthPercentageOrNumber::parse(context, i))
-            .map(Into::into)
-            .or_else(|_| parse_context_value(input, generic::SVGLength::ContextValue))
-    }
-}
-
 impl From<NonNegativeSvgLengthPercentageOrNumber> for SVGWidth {
     fn from(length: NonNegativeSvgLengthPercentageOrNumber) -> Self {
         generic::SVGLength::Length(length)
@@ -113,11 +64,14 @@ impl Parse for SVGStrokeDashArray {
                 NonNegativeSvgLengthPercentageOrNumber::parse(context, i)
             })
         }) {
-            Ok(generic::SVGStrokeDashArray::Values(values))
+            return Ok(generic::SVGStrokeDashArray::Values(values))
-        } else if let Ok(_) = input.try(|i| i.expect_ident_matching("none")) {
+        }
-            Ok(generic::SVGStrokeDashArray::Values(vec![]))
+
-        } else {
+        try_match_ident_ignore_ascii_case! { input,
-            parse_context_value(input, generic::SVGStrokeDashArray::ContextValue)
+            "context-value" if generic::is_context_value_enabled(context) => {
+                Ok(generic::SVGStrokeDashArray::ContextValue)
+            },
+            "none" => Ok(generic::SVGStrokeDashArray::Values(vec![])),
         }
     }
 }
@@ -125,22 +79,6 @@ impl Parse for SVGStrokeDashArray {
 /// <opacity-value> | context-fill-opacity | context-stroke-opacity
 pub type SVGOpacity = generic::SVGOpacity<Opacity>;
 
-impl Parse for SVGOpacity {
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(opacity) = input.try(|i| Opacity::parse(context, i)) {
-            return Ok(generic::SVGOpacity::Opacity(opacity));
-        }
-
-        try_match_ident_ignore_ascii_case! { input,
-            "context-fill-opacity" => Ok(generic::SVGOpacity::ContextFillOpacity),
-            "context-stroke-opacity" => Ok(generic::SVGOpacity::ContextStrokeOpacity),
-        }
-    }
-}
-
 /// The specified value for a single CSS paint-order property.
 #[repr(u8)]
 #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd, ToCss)]

components/style/values/specified/text.rs

@@ -419,7 +419,7 @@ pub enum TextAlignKeyword {
 
 /// Specified value of text-align property.
 #[cfg_attr(feature = "gecko", derive(MallocSizeOf))]
-#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, SpecifiedValueInfo, ToCss)]
+#[derive(Clone, Copy, Debug, Eq, Hash, Parse, PartialEq, SpecifiedValueInfo, ToCss)]
 pub enum TextAlign {
     /// Keyword value of text-align property.
     Keyword(TextAlignKeyword),
@@ -435,27 +435,6 @@ pub enum TextAlign {
     MozCenterOrInherit,
 }
 
-impl Parse for TextAlign {
-    fn parse<'i, 't>(
-        _context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        // MozCenterOrInherit cannot be parsed, only set directly on the elements
-        if let Ok(key) = input.try(TextAlignKeyword::parse) {
-            return Ok(TextAlign::Keyword(key));
-        }
-        #[cfg(feature = "gecko")]
-        {
-            input.expect_ident_matching("match-parent")?;
-            return Ok(TextAlign::MatchParent);
-        }
-        #[cfg(feature = "servo")]
-        {
-            return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
-        }
-    }
-}
-
 impl TextAlign {
     /// Convert an enumerated value coming from Gecko to a `TextAlign`.
     #[cfg(feature = "gecko")]

components/style/values/specified/transform.rs

@@ -231,7 +231,7 @@ impl Parse for Transform {
 }
 
 /// The specified value of a component of a CSS `<transform-origin>`.
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, Parse, SpecifiedValueInfo, ToCss)]
 pub enum OriginComponent<S> {
     /// `center`
     Center,
@@ -295,25 +295,6 @@ impl Parse for TransformOrigin {
     }
 }
 
-impl<S> Parse for OriginComponent<S>
-where
-    S: Parse,
-{
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>,
-    ) -> Result<Self, ParseError<'i>> {
-        if input.try(|i| i.expect_ident_matching("center")).is_ok() {
-            return Ok(OriginComponent::Center);
-        }
-        if let Ok(lp) = input.try(|i| LengthPercentage::parse(context, i)) {
-            return Ok(OriginComponent::Length(lp));
-        }
-        let keyword = S::parse(context, input)?;
-        Ok(OriginComponent::Side(keyword))
-    }
-}
-
 impl<S> ToComputedValue for OriginComponent<S>
 where
     S: Side,

components/style/values/specified/ui.rs

@@ -9,14 +9,10 @@ use crate::values::generics::ui as generics;
 use crate::values::specified::color::Color;
 use crate::values::specified::url::SpecifiedImageUrl;
 use crate::values::specified::Number;
-use crate::values::{Auto, Either};
 use cssparser::Parser;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 
-/// auto | <color>
-pub type ColorOrAuto = Either<Color, Auto>;
-
 /// A specified value for the `cursor` property.
 pub type Cursor = generics::Cursor<CursorImage>;
 

components/style_derive/parse.rs

@@ -5,8 +5,8 @@
 use crate::cg;
 use crate::to_css::CssVariantAttrs;
 use proc_macro2::TokenStream;
-use syn::{DeriveInput, Path};
+use syn::{self, DeriveInput, Path};
-use synstructure;
+use synstructure::{Structure, VariantInfo};
 
 #[darling(attributes(parse), default)]
 #[derive(Default, FromVariant)]
@@ -15,22 +15,88 @@ pub struct ParseVariantAttrs {
     pub condition: Option<Path>,
 }
 
-pub fn derive(input: DeriveInput) -> TokenStream {
+fn parse_non_keyword_variant(
+    name: &syn::Ident,
+    variant: &VariantInfo,
+    variant_attrs: &CssVariantAttrs,
+    parse_attrs: &ParseVariantAttrs,
+    skip_try: bool,
+) -> TokenStream {
+    let bindings = variant.bindings();
+    assert!(parse_attrs.aliases.is_none());
+    assert!(variant_attrs.function.is_none());
+    assert!(variant_attrs.keyword.is_none());
+    assert_eq!(
+        bindings.len(),
+        1,
+        "We only support deriving parse for simple variants"
+    );
+    let variant_name = &variant.ast().ident;
+    let ty = &bindings[0].ast().ty;
+    let mut parse = if skip_try {
+        quote! {
+            let v = <#ty as crate::parser::Parse>::parse(context, input)?;
+            return Ok(#name::#variant_name(v));
+        }
+    } else {
+        quote! {
+            if let Ok(v) = input.try(|i| <#ty as crate::parser::Parse>::parse(context, i)) {
+                return Ok(#name::#variant_name(v));
+            }
+        }
+    };
+
+    if let Some(ref condition) = parse_attrs.condition {
+        parse = quote! {
+            if #condition(context) {
+                #parse
+            }
+        };
+
+        if skip_try {
+            // We're the last variant and we can fail to parse due to the
+            // condition clause. If that happens, we need to return an error.
+            parse = quote! {
+                #parse
+                Err(input.new_custom_error(style_traits::StyleParseErrorKind::UnspecifiedError))
+            };
+        }
+    }
+
+    parse
+}
+
+pub fn derive(mut input: DeriveInput) -> TokenStream {
+    {
+        let mut where_clause = input.generics.where_clause.take();
+        for param in input.generics.type_params() {
+            cg::add_predicate(&mut where_clause, parse_quote!(#param: crate::parser::Parse));
+        }
+        input.generics.where_clause = where_clause;
+    }
+
+
     let name = &input.ident;
-    let s = synstructure::Structure::new(&input);
+    let s = Structure::new(&input);
 
     let mut saw_condition = false;
-    let match_body = s.variants().iter().fold(quote!(), |match_body, variant| {
+    let mut match_keywords = quote! {};
-        let bindings = variant.bindings();
+    let mut non_keywords = vec![];
-        assert!(
-            bindings.is_empty(),
-            "Parse is only supported for single-variant enums for now"
-        );
 
+    let mut effective_variants = 0;
+    for variant in s.variants().iter() {
         let css_variant_attrs = cg::parse_variant_attrs_from_ast::<CssVariantAttrs>(&variant.ast());
         let parse_attrs = cg::parse_variant_attrs_from_ast::<ParseVariantAttrs>(&variant.ast());
         if css_variant_attrs.skip {
-            return match_body;
+            continue;
+        }
+        effective_variants += 1;
+
+        saw_condition |= parse_attrs.condition.is_some();
+
+        if !variant.bindings().is_empty() {
+            non_keywords.push((variant, css_variant_attrs, parse_attrs));
+            continue;
         }
 
         let identifier = cg::to_css_identifier(
@@ -40,55 +106,78 @@ pub fn derive(input: DeriveInput) -> TokenStream {
         );
         let ident = &variant.ast().ident;
 
-        saw_condition |= parse_attrs.condition.is_some();
         let condition = match parse_attrs.condition {
             Some(ref p) => quote! { if #p(context) },
             None => quote! {},
         };
 
-        let mut body = quote! {
+        match_keywords.extend(quote! {
-            #match_body
             #identifier #condition => Ok(#name::#ident),
-        };
+        });
 
         let aliases = match parse_attrs.aliases {
             Some(aliases) => aliases,
-            None => return body,
+            None => continue,
         };
 
         for alias in aliases.split(',') {
-            body = quote! {
+            match_keywords.extend(quote! {
-                #body
                 #alias #condition => Ok(#name::#ident),
-            };
+            });
         }
+    }
 
-        body
+    let needs_context = saw_condition || !non_keywords.is_empty();
-    });
 
-    let context_ident = if saw_condition {
+    let context_ident = if needs_context {
         quote! { context }
     } else {
         quote! { _ }
     };
 
-    let parse_body = if saw_condition {
+    let has_keywords = non_keywords.len() != effective_variants;
-        quote! {
+
-            let location = input.current_source_location();
+    let mut parse_non_keywords = quote! {};
-            let ident = input.expect_ident()?;
+    for (i, (variant, css_attrs, parse_attrs)) in non_keywords.iter().enumerate() {
-            match_ignore_ascii_case! { &ident,
+        let skip_try = !has_keywords && i == non_keywords.len() - 1;
-                #match_body
+        let parse_variant = parse_non_keyword_variant(
-                _ => Err(location.new_unexpected_token_error(
+            name,
-                    cssparser::Token::Ident(ident.clone())
+            variant,
-                ))
+            css_attrs,
+            parse_attrs,
+            skip_try,
+        );
+        parse_non_keywords.extend(parse_variant);
+    }
+
+    let parse_body = if needs_context {
+        let parse_keywords = if has_keywords {
+            quote! {
+                let location = input.current_source_location();
+                let ident = input.expect_ident()?;
+                match_ignore_ascii_case! { &ident,
+                    #match_keywords
+                    _ => Err(location.new_unexpected_token_error(
+                        cssparser::Token::Ident(ident.clone())
+                    ))
+                }
             }
+        } else {
+            quote! {}
+        };
+
+        quote! {
+            #parse_non_keywords
+            #parse_keywords
         }
     } else {
         quote! { Self::parse(input) }
     };
 
+    let (impl_generics, ty_generics, where_clause) = input.generics.split_for_impl();
+
     let parse_trait_impl = quote! {
-        impl crate::parser::Parse for #name {
+        impl #impl_generics crate::parser::Parse for #name #ty_generics #where_clause {
             #[inline]
             fn parse<'i, 't>(
                 #context_ident: &crate::parser::ParserContext,
@@ -99,10 +188,12 @@ pub fn derive(input: DeriveInput) -> TokenStream {
         }
     };
 
-    if saw_condition {
+    if needs_context {
         return parse_trait_impl;
     }
 
+    assert!(non_keywords.is_empty());
+
     // TODO(emilio): It'd be nice to get rid of these, but that makes the
     // conversion harder...
     let methods_impl = quote! {
@@ -125,7 +216,7 @@ pub fn derive(input: DeriveInput) -> TokenStream {
             #[inline]
             pub fn from_ident(ident: &str) -> Result<Self, ()> {
                 match_ignore_ascii_case! { ident,
-                    #match_body
+                    #match_keywords
                     _ => Err(()),
                 }
             }