Bug 1563315 - Simplify parsing and storage of SVG paint server fallback. r=boris

Depends on D36805 Differential Revision: https://phabricator.services.mozilla.com/D36806 --HG-- extra : moz-landing-system : lando
2019-07-06 08:24:39 +00:00 · 2019-07-06 08:24:39 +00:00 · 42ce0fc8e5
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@ -525,8 +525,8 @@ def set_gecko_property(ffi_name, expr):

 <%def name="impl_svg_paint(ident, gecko_ffi_name)">
    #[allow(non_snake_case)]
-    pub fn set_${ident}(&mut self, mut v: longhands::${ident}::computed_value::T) {
-        use crate::values::generics::svg::SVGPaintKind;
+    pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
+        use crate::values::generics::svg::{SVGPaintKind, SVGPaintFallback};
        use self::structs::nsStyleSVGPaintType;
        use self::structs::nsStyleSVGFallbackType;

@ -534,7 +534,6 @@ def set_gecko_property(ffi_name, expr):
        unsafe {
            bindings::Gecko_nsStyleSVGPaint_Reset(paint);
        }
-        let fallback = v.fallback.take();
        match v.kind {
            SVGPaintKind::None => return,
            SVGPaintKind::ContextFill => {
@ -559,15 +558,17 @@ def set_gecko_property(ffi_name, expr):
            }
        }

-        paint.mFallbackType = match fallback {
-            Some(Either::First(color)) => {
-                paint.mFallbackColor = color.into();
+        paint.mFallbackType = match v.fallback {
+            SVGPaintFallback::Color(c) => {
+                paint.mFallbackColor = c.into();
                nsStyleSVGFallbackType::eStyleSVGFallbackType_Color
            },
-            Some(Either::Second(_)) => {
+            SVGPaintFallback::None => {
                nsStyleSVGFallbackType::eStyleSVGFallbackType_None
            },
-            None => nsStyleSVGFallbackType::eStyleSVGFallbackType_NotSet
+            SVGPaintFallback::Unset => {
+                nsStyleSVGFallbackType::eStyleSVGFallbackType_NotSet
+            }
        };
    }

@ -588,19 +589,21 @@ def set_gecko_property(ffi_name, expr):

    #[allow(non_snake_case)]
    pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
-        use crate::values::generics::svg::{SVGPaint, SVGPaintKind};
+        use crate::values::generics::svg::{SVGPaint, SVGPaintKind, SVGPaintFallback};
        use self::structs::nsStyleSVGPaintType;
        use self::structs::nsStyleSVGFallbackType;
        let ref paint = ${get_gecko_property(gecko_ffi_name)};

        let fallback = match paint.mFallbackType {
            nsStyleSVGFallbackType::eStyleSVGFallbackType_Color => {
-                Some(Either::First(paint.mFallbackColor.into()))
+                SVGPaintFallback::Color(paint.mFallbackColor.into())
            },
            nsStyleSVGFallbackType::eStyleSVGFallbackType_None => {
-                Some(Either::Second(None_))
+                SVGPaintFallback::None
            },
-            nsStyleSVGFallbackType::eStyleSVGFallbackType_NotSet => None,
+            nsStyleSVGFallbackType::eStyleSVGFallbackType_NotSet => {
+                SVGPaintFallback::Unset
+            }
        };

        let kind = match paint.mType {
--- a/servo/components/style/values/animated/svg.rs
+++ b/servo/components/style/values/animated/svg.rs
@ -4,25 +4,10 @@

 //! Animation implementations for various SVG-related types.

-use super::{Animate, Procedure, ToAnimatedZero};
+use super::{Animate, Procedure};
 use crate::properties::animated_properties::ListAnimation;
-use crate::values::animated::color::Color as AnimatedColor;
-use crate::values::computed::url::ComputedUrl;
 use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
-use crate::values::generics::svg::{SVGPaint, SVGStrokeDashArray};
-
-/// Animated SVGPaint.
-pub type IntermediateSVGPaint = SVGPaint<AnimatedColor, ComputedUrl>;
-
-impl ToAnimatedZero for IntermediateSVGPaint {
-    #[inline]
-    fn to_animated_zero(&self) -> Result<Self, ()> {
-        Ok(IntermediateSVGPaint {
-            kind: self.kind.to_animated_zero()?,
-            fallback: self.fallback.and_then(|v| v.to_animated_zero().ok()),
-        })
-    }
-}
+use crate::values::generics::svg::SVGStrokeDashArray;

 /// <https://www.w3.org/TR/SVG11/painting.html#StrokeDasharrayProperty>
 impl<L> Animate for SVGStrokeDashArray<L>
--- a/servo/components/style/values/computed/svg.rs
+++ b/servo/components/style/values/computed/svg.rs
@ -20,22 +20,13 @@ pub type SVGPaint = generic::SVGPaint<Color, ComputedUrl>;
 /// Computed SVG Paint Kind value
 pub type SVGPaintKind = generic::SVGPaintKind<Color, ComputedUrl>;

-impl Default for SVGPaint {
-    fn default() -> Self {
-        SVGPaint {
-            kind: generic::SVGPaintKind::None,
-            fallback: None,
-        }
-    }
-}
-
 impl SVGPaint {
    /// Opaque black color
    pub fn black() -> Self {
        let rgba = RGBA::from_floats(0., 0., 0., 1.).into();
        SVGPaint {
            kind: generic::SVGPaintKind::Color(rgba),
-            fallback: None,
+            fallback: generic::SVGPaintFallback::Unset,
        }
    }
 }
--- a/servo/components/style/values/generics/svg.rs
+++ b/servo/components/style/values/generics/svg.rs
@ -5,9 +5,36 @@
 //! Generic types for CSS values in SVG

 use crate::parser::{Parse, ParserContext};
-use crate::values::{Either, None_};
 use cssparser::Parser;
-use style_traits::{ParseError, StyleParseErrorKind};
+use style_traits::ParseError;
+
+/// The fallback of an SVG paint server value.
+#[derive(
+    Animate,
+    Clone,
+    ComputeSquaredDistance,
+    Debug,
+    MallocSizeOf,
+    PartialEq,
+    Parse,
+    SpecifiedValueInfo,
+    ToAnimatedValue,
+    ToAnimatedZero,
+    ToComputedValue,
+    ToCss,
+    ToResolvedValue,
+    ToShmem,
+)]
+pub enum SVGPaintFallback<C> {
+    /// The `none` keyword.
+    None,
+    /// A magic value that represents no fallback specified and serializes to
+    /// the empty string.
+    #[css(skip)]
+    Unset,
+    /// A color.
+    Color(C),
+}

 /// An SVG paint value
 ///
@ -22,16 +49,26 @@ use style_traits::{ParseError, StyleParseErrorKind};
    PartialEq,
    SpecifiedValueInfo,
    ToAnimatedValue,
+    ToAnimatedZero,
    ToComputedValue,
    ToCss,
    ToResolvedValue,
    ToShmem,
 )]
 pub struct SVGPaint<ColorType, UrlPaintServer> {
-    /// The paint source
+    /// The paint source.
    pub kind: SVGPaintKind<ColorType, UrlPaintServer>,
-    /// The fallback color. It would be empty, the `none` keyword or <color>.
-    pub fallback: Option<Either<ColorType, None_>>,
+    /// The fallback color.
+    pub fallback: SVGPaintFallback<ColorType>,
+}
+
+impl<C, U> Default for SVGPaint<C, U> {
+    fn default() -> Self {
+        Self {
+            kind: SVGPaintKind::None,
+            fallback: SVGPaintFallback::Unset,
+        }
+    }
 }

 /// An SVG paint value without the fallback
@ -47,6 +84,7 @@ pub struct SVGPaint<ColorType, UrlPaintServer> {
    Debug,
    MallocSizeOf,
    PartialEq,
+    Parse,
    SpecifiedValueInfo,
    ToAnimatedValue,
    ToAnimatedZero,
@ -70,65 +108,22 @@ pub enum SVGPaintKind<ColorType, UrlPaintServer> {
    ContextStroke,
 }

-impl<ColorType, UrlPaintServer> SVGPaintKind<ColorType, UrlPaintServer> {
-    /// Parse a keyword value only
-    fn parse_ident<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        try_match_ident_ignore_ascii_case! { input,
-            "none" => Ok(SVGPaintKind::None),
-            "context-fill" => Ok(SVGPaintKind::ContextFill),
-            "context-stroke" => Ok(SVGPaintKind::ContextStroke),
-        }
-    }
-}
-
-/// Parse SVGPaint's fallback.
-/// fallback is keyword(none), Color or empty.
-/// <https://svgwg.org/svg2-draft/painting.html#SpecifyingPaint>
-fn parse_fallback<'i, 't, ColorType: Parse>(
-    context: &ParserContext,
-    input: &mut Parser<'i, 't>,
-) -> Option<Either<ColorType, None_>> {
-    if input.try(|i| i.expect_ident_matching("none")).is_ok() {
-        Some(Either::Second(None_))
-    } else {
-        if let Ok(color) = input.try(|i| ColorType::parse(context, i)) {
-            Some(Either::First(color))
-        } else {
-            None
-        }
-    }
-}
-
 impl<ColorType: Parse, UrlPaintServer: Parse> Parse for SVGPaint<ColorType, UrlPaintServer> {
    fn parse<'i, 't>(
        context: &ParserContext,
        input: &mut Parser<'i, 't>,
    ) -> Result<Self, ParseError<'i>> {
-        if let Ok(url) = input.try(|i| UrlPaintServer::parse(context, i)) {
-            Ok(SVGPaint {
-                kind: SVGPaintKind::PaintServer(url),
-                fallback: parse_fallback(context, input),
-            })
-        } else if let Ok(kind) = input.try(SVGPaintKind::parse_ident) {
-            if let SVGPaintKind::None = kind {
-                Ok(SVGPaint {
-                    kind: kind,
-                    fallback: None,
-                })
-            } else {
-                Ok(SVGPaint {
-                    kind: kind,
-                    fallback: parse_fallback(context, input),
-                })
-            }
-        } else if let Ok(color) = input.try(|i| ColorType::parse(context, i)) {
-            Ok(SVGPaint {
-                kind: SVGPaintKind::Color(color),
-                fallback: None,
-            })
-        } else {
-            Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+        let kind = SVGPaintKind::parse(context, input)?;
+        if matches!(kind, SVGPaintKind::None | SVGPaintKind::Color(..)) {
+            return Ok(SVGPaint {
+                kind,
+                fallback: SVGPaintFallback::Unset
+            });
        }
+        let fallback = input
+            .try(|i| SVGPaintFallback::parse(context, i))
+            .unwrap_or(SVGPaintFallback::Unset);
+        Ok(SVGPaint { kind, fallback })
    }
 }

--- a/servo/components/style/values/specified/mod.rs
+++ b/servo/components/style/values/specified/mod.rs
@ -342,8 +342,6 @@ impl Parse for GreaterThanOrEqualToOneNumber {
 /// <number> | <percentage>
 ///
 /// Accepts only non-negative numbers.
-///
-/// FIXME(emilio): Should probably use Either.
 #[allow(missing_docs)]
 #[derive(Clone, Copy, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
 pub enum NumberOrPercentage {