Auto merge of #17530 - servo:derive-all-the-things, r=emilio

Improve sequence values in style

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

components/gfx/display_list/mod.rs

@@ -28,7 +28,8 @@ use std::cmp::{self, Ordering};
 use std::collections::HashMap;
 use std::fmt;
 use std::sync::Arc;
-use style::computed_values::{border_style, filter, image_rendering};
+use style::computed_values::{border_style, image_rendering};
+use style::values::computed::Filter;
 use style_traits::cursor::Cursor;
 use text::TextRun;
 use text::glyph::ByteIndex;
@@ -419,7 +420,7 @@ pub struct StackingContext {
     pub z_index: i32,
 
     /// CSS filters to be applied to this stacking context (including opacity).
-    pub filters: filter::T,
+    pub filters: Vec<Filter>,
 
     /// The blend mode with which this stacking context blends with its backdrop.
     pub mix_blend_mode: MixBlendMode,
@@ -448,7 +449,7 @@ impl StackingContext {
                bounds: &Rect<Au>,
                overflow: &Rect<Au>,
                z_index: i32,
-               filters: filter::T,
+               filters: Vec<Filter>,
                mix_blend_mode: MixBlendMode,
                transform: Option<Transform3D<f32>>,
                transform_style: TransformStyle,
@@ -479,7 +480,7 @@ impl StackingContext {
                              &Rect::zero(),
                              &Rect::zero(),
                              0,
-                             filter::T::none(),
+                             vec![],
                              MixBlendMode::Normal,
                              None,
                              TransformStyle::Flat,

components/layout/display_list_builder.rs

@@ -2010,7 +2010,7 @@ impl FragmentDisplayListBuilding for Fragment {
 
         // Create the filter pipeline.
         let effects = self.style().get_effects();
-        let mut filters = effects.filter.clone().0.into_vec();
+        let mut filters = effects.filter.0.clone();
         if effects.opacity != 1.0 {
             filters.push(Filter::Opacity(effects.opacity))
         }

components/layout/webrender_helpers.rs

@@ -13,9 +13,8 @@ use gfx::display_list::{BorderDetails, BorderRadii, BoxShadowClipMode, ClippingR
 use gfx::display_list::{DisplayItem, DisplayList, DisplayListTraversal, StackingContextType};
 use msg::constellation_msg::PipelineId;
 use style::computed_values::{image_rendering, mix_blend_mode, transform_style};
-use style::computed_values::filter;
+use style::values::computed::{BorderStyle, Filter};
-use style::values::computed::BorderStyle;
+use style::values::generics::effects::Filter as GenericFilter;
-use style::values::generics::effects::Filter;
 use webrender_traits::{self, DisplayListBuilder, ExtendMode};
 use webrender_traits::{LayoutTransform, ClipId, ClipRegionToken};
 
@@ -202,21 +201,21 @@ trait ToFilterOps {
     fn to_filter_ops(&self) -> Vec<webrender_traits::FilterOp>;
 }
 
-impl ToFilterOps for filter::T {
+impl ToFilterOps for Vec<Filter> {
     fn to_filter_ops(&self) -> Vec<webrender_traits::FilterOp> {
-        let mut result = Vec::with_capacity(self.0.len());
+        let mut result = Vec::with_capacity(self.len());
-        for filter in self.0.iter() {
+        for filter in self.iter() {
             match *filter {
-                Filter::Blur(radius) => result.push(webrender_traits::FilterOp::Blur(radius)),
+                GenericFilter::Blur(radius) => result.push(webrender_traits::FilterOp::Blur(radius)),
-                Filter::Brightness(amount) => result.push(webrender_traits::FilterOp::Brightness(amount)),
+                GenericFilter::Brightness(amount) => result.push(webrender_traits::FilterOp::Brightness(amount)),
-                Filter::Contrast(amount) => result.push(webrender_traits::FilterOp::Contrast(amount)),
+                GenericFilter::Contrast(amount) => result.push(webrender_traits::FilterOp::Contrast(amount)),
-                Filter::Grayscale(amount) => result.push(webrender_traits::FilterOp::Grayscale(amount)),
+                GenericFilter::Grayscale(amount) => result.push(webrender_traits::FilterOp::Grayscale(amount)),
-                Filter::HueRotate(angle) => result.push(webrender_traits::FilterOp::HueRotate(angle.radians())),
+                GenericFilter::HueRotate(angle) => result.push(webrender_traits::FilterOp::HueRotate(angle.radians())),
-                Filter::Invert(amount) => result.push(webrender_traits::FilterOp::Invert(amount)),
+                GenericFilter::Invert(amount) => result.push(webrender_traits::FilterOp::Invert(amount)),
-                Filter::Opacity(amount) => result.push(webrender_traits::FilterOp::Opacity(amount.into())),
+                GenericFilter::Opacity(amount) => result.push(webrender_traits::FilterOp::Opacity(amount.into())),
-                Filter::Saturate(amount) => result.push(webrender_traits::FilterOp::Saturate(amount)),
+                GenericFilter::Saturate(amount) => result.push(webrender_traits::FilterOp::Saturate(amount)),
-                Filter::Sepia(amount) => result.push(webrender_traits::FilterOp::Sepia(amount)),
+                GenericFilter::Sepia(amount) => result.push(webrender_traits::FilterOp::Sepia(amount)),
-                Filter::DropShadow(ref shadow) => match *shadow {},
+                GenericFilter::DropShadow(ref shadow) => match *shadow {},
             }
         }
         result

components/style/counter_style/mod.rs

@@ -19,7 +19,7 @@ use std::ascii::AsciiExt;
 use std::borrow::Cow;
 use std::fmt;
 use std::ops::Range;
-use style_traits::{ToCss, OneOrMoreSeparated, CommaSeparator, ParseError, StyleParseError};
+use style_traits::{Comma, OneOrMoreSeparated, ParseError, StyleParseError, ToCss};
 use values::CustomIdent;
 
 /// Parse the prelude of an @counter-style rule
@@ -553,7 +553,7 @@ pub struct AdditiveTuple {
 }
 
 impl OneOrMoreSeparated for AdditiveTuple {
-    type S = CommaSeparator;
+    type S = Comma;
 }
 
 impl Parse for AdditiveTuple {

components/style/font_face.rs

@@ -22,7 +22,7 @@ use properties::longhands::font_language_override;
 use selectors::parser::SelectorParseError;
 use shared_lock::{SharedRwLockReadGuard, ToCssWithGuard};
 use std::fmt;
-use style_traits::{ToCss, OneOrMoreSeparated, CommaSeparator, ParseError, StyleParseError};
+use style_traits::{Comma, OneOrMoreSeparated, ParseError, StyleParseError, ToCss};
 use values::specified::url::SpecifiedUrl;
 
 /// A source for a font-face rule.
@@ -37,7 +37,7 @@ pub enum Source {
 }
 
 impl OneOrMoreSeparated for Source {
-    type S = CommaSeparator;
+    type S = Comma;
 }
 
 /// A `UrlSource` represents a font-face source that has been specified with a

components/style/parser.rs

@@ -7,7 +7,7 @@
 use context::QuirksMode;
 use cssparser::{Parser, SourcePosition, UnicodeRange};
 use error_reporting::{ParseErrorReporter, ContextualParseError};
-use style_traits::{OneOrMoreSeparated, IsCommaSeparator, ParseError, ParsingMode};
+use style_traits::{OneOrMoreSeparated, ParseError, ParsingMode, Separator};
 #[cfg(feature = "gecko")]
 use style_traits::{PARSING_MODE_DEFAULT, PARSING_MODE_ALLOW_UNITLESS_LENGTH, PARSING_MODE_ALLOW_ALL_NUMERIC_VALUES};
 use stylesheets::{CssRuleType, Origin, UrlExtraData, Namespaces};
@@ -161,31 +161,17 @@ pub trait Parse : Sized {
                      -> Result<Self, ParseError<'i>>;
 }
 
-impl<T> Parse for Vec<T> where T: Parse + OneOrMoreSeparated,
+impl<T> Parse for Vec<T>
-                               <T as OneOrMoreSeparated>::S: IsCommaSeparator
+where
+    T: Parse + OneOrMoreSeparated,
+    <T as OneOrMoreSeparated>::S: Separator,
 {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                      -> Result<Self, ParseError<'i>> {
-        input.parse_comma_separated(|input| T::parse(context, input))
+        <T as OneOrMoreSeparated>::S::parse(input, |i| T::parse(context, i))
     }
 }
 
-/// Parse a non-empty space-separated or comma-separated list of objects parsed by parse_one
-pub fn parse_space_or_comma_separated<'i, 't, F, T>(input: &mut Parser<'i, 't>, mut parse_one: F)
-        -> Result<Vec<T>, ParseError<'i>>
-        where F: for<'ii, 'tt> FnMut(&mut Parser<'ii, 'tt>) -> Result<T, ParseError<'ii>> {
-    let first = parse_one(input)?;
-    let mut vec = vec![first];
-    loop {
-        let _ = input.try(|i| i.expect_comma());
-        if let Ok(val) = input.try(|i| parse_one(i)) {
-            vec.push(val)
-        } else {
-            break
-        }
-    }
-    Ok(vec)
-}
 impl Parse for UnicodeRange {
     fn parse<'i, 't>(_context: &ParserContext, input: &mut Parser<'i, 't>)
                      -> Result<Self, ParseError<'i>> {

components/style/properties/gecko.mako.rs

@@ -60,7 +60,7 @@ use std::ptr;
 use stylearc::Arc;
 use std::cmp;
 use values::{Auto, CustomIdent, Either, KeyframesName};
-use values::computed::Shadow;
+use values::computed::{Filter, Shadow};
 use values::specified::length::Percentage;
 use computed_values::border_style;
 
@@ -3300,7 +3300,11 @@ fn static_assert() {
                          'Opacity', 'Saturate', 'Sepia' ]
      %>
 
-    pub fn set_filter(&mut self, v: longhands::filter::computed_value::T) {
+    pub fn set_filter<I>(&mut self, v: I)
+    where
+        I: IntoIterator<Item = Filter>,
+        I::IntoIter: ExactSizeIterator,
+    {
         use values::generics::effects::Filter::*;
         use gecko_bindings::structs::nsCSSShadowArray;
         use gecko_bindings::structs::nsStyleFilter;
@@ -3320,12 +3324,13 @@ fn static_assert() {
             gecko_filter.mFilterParameter.set_value(value);
         }
 
+        let v = v.into_iter();
         unsafe {
-            Gecko_ResetFilters(&mut self.gecko, v.0.len());
+            Gecko_ResetFilters(&mut self.gecko, v.len());
         }
-        debug_assert!(v.0.len() == self.gecko.mFilters.len());
+        debug_assert_eq!(v.len(), self.gecko.mFilters.len());
 
-        for (servo, gecko_filter) in v.0.into_vec().into_iter().zip(self.gecko.mFilters.iter_mut()) {
+        for (servo, gecko_filter) in v.zip(self.gecko.mFilters.iter_mut()) {
             match servo {
                 % for func in FILTER_FUNCTIONS:
                 ${func}(factor) => fill_filter(NS_STYLE_FILTER_${func.upper()},
@@ -3372,7 +3377,7 @@ fn static_assert() {
     }
 
     pub fn clone_filter(&self) -> longhands::filter::computed_value::T {
-        use values::generics::effects::{Filter, FilterList};
+        use values::generics::effects::Filter;
         use values::specified::url::SpecifiedUrl;
         use gecko_bindings::structs::NS_STYLE_FILTER_BLUR;
         use gecko_bindings::structs::NS_STYLE_FILTER_BRIGHTNESS;
@@ -3422,7 +3427,7 @@ fn static_assert() {
                 _ => {},
             }
         }
-        FilterList(filters.into_boxed_slice())
+        longhands::filter::computed_value::T(filters)
     }
 
 </%self:impl_trait>
@@ -3942,10 +3947,9 @@ clip-path
     }
 
     pub fn clone_stroke_dasharray(&self) -> longhands::stroke_dasharray::computed_value::T {
-        use smallvec::SmallVec;
         use values::computed::LengthOrPercentage;
 
-        let mut vec = SmallVec::new();
+        let mut vec = vec![];
         for gecko in self.gecko.mStrokeDasharray.iter() {
             match gecko.as_value() {
                 CoordDataValue::Factor(number) => vec.push(Either::First(number)),

components/style/properties/helpers.mako.rs

@@ -80,14 +80,15 @@
     `initial_value` need not be defined for these.
 </%doc>
 <%def name="vector_longhand(name, gecko_only=False, allow_empty=False,
-            delegate_animate=False, space_separated_allowed=False, **kwargs)">
+            delegate_animate=False, separator='Comma', **kwargs)">
     <%call expr="longhand(name, vector=True, **kwargs)">
         % if not gecko_only:
+            #[allow(unused_imports)]
             use smallvec::SmallVec;
             use std::fmt;
             #[allow(unused_imports)]
             use style_traits::HasViewportPercentage;
-            use style_traits::ToCss;
+            use style_traits::{Separator, ToCss};
 
             pub mod single_value {
                 #[allow(unused_imports)]
@@ -113,13 +114,23 @@
             pub mod computed_value {
                 pub use super::single_value::computed_value as single_value;
                 pub use self::single_value::T as SingleComputedValue;
+                % if allow_empty and allow_empty != "NotInitial":
+                use std::vec::IntoIter;
+                % else:
                 use smallvec::{IntoIter, SmallVec};
+                % endif
                 use values::computed::ComputedVecIter;
 
                 /// The computed value, effectively a list of single values.
                 #[derive(Debug, Clone, PartialEq)]
                 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-                pub struct T(pub SmallVec<[single_value::T; 1]>);
+                pub struct T(
+                    % if allow_empty and allow_empty != "NotInitial":
+                    pub Vec<single_value::T>,
+                    % else:
+                    pub SmallVec<[single_value::T; 1]>,
+                    % endif
+                );
 
                 % if delegate_animate:
                     use properties::animated_properties::Animatable;
@@ -149,7 +160,11 @@
 
                 impl IntoIterator for T {
                     type Item = single_value::T;
+                    % if allow_empty and allow_empty != "NotInitial":
+                    type IntoIter = IntoIter<single_value::T>;
+                    % else:
                     type IntoIter = IntoIter<[single_value::T; 1]>;
+                    % endif
                     fn into_iter(self) -> Self::IntoIter {
                         self.0.into_iter()
                     }
@@ -171,7 +186,7 @@
                         % endif
                     }
                     for i in iter {
-                        dest.write_str(", ")?;
+                        dest.write_str(::style_traits::${separator}::separator())?;
                         i.to_css(dest)?;
                     }
                     Ok(())
@@ -198,7 +213,7 @@
                         % endif
                     }
                     for i in iter {
-                        dest.write_str(", ")?;
+                        dest.write_str(::style_traits::${separator}::separator())?;
                         i.to_css(dest)?;
                     }
                     Ok(())
@@ -207,7 +222,7 @@
 
             pub fn get_initial_value() -> computed_value::T {
                 % if allow_empty and allow_empty != "NotInitial":
-                    computed_value::T(SmallVec::new())
+                    computed_value::T(vec![])
                 % else:
                     let mut v = SmallVec::new();
                     v.push(single_value::get_initial_value());
@@ -217,14 +232,7 @@
 
             pub fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                  -> Result<SpecifiedValue, ParseError<'i>> {
-                #[allow(unused_imports)]
+                use style_traits::Separator;
-                use parser::parse_space_or_comma_separated;
-
-                <%
-                    parse_func = "Parser::parse_comma_separated"
-                    if space_separated_allowed:
-                        parse_func = "parse_space_or_comma_separated"
-                %>
 
                 % if allow_empty:
                     if input.try(|input| input.expect_ident_matching("none")).is_ok() {
@@ -232,7 +240,7 @@
                     }
                 % endif
 
-                ${parse_func}(input, |parser| {
+                ::style_traits::${separator}::parse(input, |parser| {
                     single_value::parse(context, parser)
                 }).map(SpecifiedValue)
             }

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

@@ -808,7 +808,9 @@ pub trait RepeatableListAnimatable: Animatable {}
 impl RepeatableListAnimatable for LengthOrPercentage {}
 impl RepeatableListAnimatable for Either<f32, LengthOrPercentage> {}
 
-impl<T: RepeatableListAnimatable> Animatable for SmallVec<[T; 1]> {
+macro_rules! repeated_vec_impl {
+    ($($ty:ty),*) => {
+        $(impl<T: RepeatableListAnimatable> Animatable for $ty {
             fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64)
                 -> Result<Self, ()> {
                 use num_integer::lcm;
@@ -829,10 +831,14 @@ impl<T: RepeatableListAnimatable> Animatable for SmallVec<[T; 1]> {
                 let len = lcm(self.len(), other.len());
                 self.iter().cycle().zip(other.iter().cycle()).take(len).map(|(me, you)| {
                     me.compute_squared_distance(you)
-        }).collect::<Result<Vec<_>, _>>().map(|d| d.iter().sum())
+                }).sum()
             }
+        })*
+    };
 }
 
+repeated_vec_impl!(SmallVec<[T; 1]>, Vec<T>);
+
 /// https://drafts.csswg.org/css-transitions/#animtype-number
 impl Animatable for Au {
     #[inline]
@@ -3053,9 +3059,9 @@ pub struct IntermediateShadow {
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[allow(missing_docs)]
 /// Intermediate type for box-shadow list and text-shadow list.
-pub struct IntermediateShadowList(pub SmallVec<[IntermediateShadow; 1]>);
+pub struct IntermediateShadowList(pub Vec<IntermediateShadow>);
 
-type ShadowList = SmallVec<[Shadow; 1]>;
+type ShadowList = Vec<Shadow>;
 
 impl From<IntermediateShadowList> for ShadowList {
     fn from(shadow_list: IntermediateShadowList) -> Self {
@@ -3172,11 +3178,7 @@ impl Animatable for IntermediateShadowList {
 
         let max_len = cmp::max(self.0.len(), other.0.len());
 
-        let mut result = if max_len > 1 {
+        let mut result = Vec::with_capacity(max_len);
-            SmallVec::from_vec(Vec::with_capacity(max_len))
-        } else {
-            SmallVec::new()
-        };
 
         for i in 0..max_len {
             let shadow = match (self.0.get(i), other.0.get(i)) {
@@ -3202,11 +3204,7 @@ impl Animatable for IntermediateShadowList {
     fn add(&self, other: &Self) -> Result<Self, ()> {
         let len = self.0.len() + other.0.len();
 
-        let mut result = if len > 1 {
+        let mut result = Vec::with_capacity(len);
-            SmallVec::from_vec(Vec::with_capacity(len))
-        } else {
-            SmallVec::new()
-        };
 
         result.extend(self.0.iter().cloned());
         result.extend(other.0.iter().cloned());
@@ -3342,11 +3340,11 @@ impl Animatable for AnimatedFilterList {
             }
         }
 
-        Ok(filters.into())
+        Ok(AnimatedFilterList(filters))
     }
 
     fn add(&self, other: &Self) -> Result<Self, ()> {
-        Ok(self.0.iter().chain(other.0.iter()).cloned().collect::<Vec<_>>().into())
+        Ok(AnimatedFilterList(self.0.iter().chain(other.0.iter()).cloned().collect()))
     }
 
     #[inline]

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

@@ -43,8 +43,10 @@ ${helpers.predefined_type("clip",
 
 ${helpers.predefined_type(
     "filter",
-    "FilterList",
+    "Filter",
-    "computed::FilterList::none()",
+    None,
+    vector=True,
+    separator="Space",
     animation_value_type="AnimatedFilterList",
     extra_prefixes="webkit",
     flags="CREATES_STACKING_CONTEXT FIXPOS_CB",

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

@@ -97,7 +97,7 @@ ${helpers.predefined_type(
     delegate_animate=True,
     products="gecko",
     animation_value_type="ComputedValue",
-    space_separated_allowed="True",
+    separator="CommaWithSpace",
     spec="https://www.w3.org/TR/SVG2/painting.html#StrokeDashing",
 )}
 

components/style/values/animated/effects.rs

@@ -5,21 +5,21 @@
 //! Animated types for CSS values related to effects.
 
 use properties::animated_properties::{Animatable, IntermediateColor};
+use properties::longhands::filter::computed_value::T as ComputedFilterList;
 #[cfg(not(feature = "gecko"))]
 use values::Impossible;
 use values::computed::{Angle, Number};
 #[cfg(feature = "gecko")]
 use values::computed::effects::Filter as ComputedFilter;
-#[cfg(feature = "gecko")]
-use values::computed::effects::FilterList as ComputedFilterList;
 use values::computed::effects::SimpleShadow as ComputedSimpleShadow;
 use values::computed::length::Length;
 use values::generics::effects::Filter as GenericFilter;
-use values::generics::effects::FilterList as GenericFilterList;
 use values::generics::effects::SimpleShadow as GenericSimpleShadow;
 
 /// An animated value for the `filter` property.
-pub type FilterList = GenericFilterList<Filter>;
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Debug, PartialEq)]
+pub struct FilterList(pub Vec<Filter>);
 
 /// An animated value for a single `filter`.
 #[cfg(feature = "gecko")]
@@ -32,19 +32,31 @@ pub type Filter = GenericFilter<Angle, Number, Length, Impossible>;
 /// An animated value for the `drop-shadow()` filter.
 pub type SimpleShadow = GenericSimpleShadow<IntermediateColor, Length, Length>;
 
-#[cfg(feature = "gecko")]
 impl From<ComputedFilterList> for FilterList {
+    #[cfg(not(feature = "gecko"))]
     #[inline]
     fn from(filters: ComputedFilterList) -> Self {
-        filters.0.into_vec().into_iter().map(|f| f.into()).collect::<Vec<_>>().into()
+        FilterList(filters.0)
+    }
+
+    #[cfg(feature = "gecko")]
+    #[inline]
+    fn from(filters: ComputedFilterList) -> Self {
+        FilterList(filters.0.into_iter().map(|f| f.into()).collect())
     }
 }
 
-#[cfg(feature = "gecko")]
 impl From<FilterList> for ComputedFilterList {
+    #[cfg(not(feature = "gecko"))]
     #[inline]
     fn from(filters: FilterList) -> Self {
-        filters.0.into_vec().into_iter().map(|f| f.into()).collect::<Vec<_>>().into()
+        ComputedFilterList(filters.0)
+    }
+
+    #[cfg(feature = "gecko")]
+    #[inline]
+    fn from(filters: FilterList) -> Self {
+        ComputedFilterList(filters.0.into_iter().map(|f| f.into()).collect())
     }
 }
 

components/style/values/computed/effects.rs

@@ -10,12 +10,8 @@ use values::computed::{Angle, Number};
 use values::computed::color::Color;
 use values::computed::length::Length;
 use values::generics::effects::Filter as GenericFilter;
-use values::generics::effects::FilterList as GenericFilterList;
 use values::generics::effects::SimpleShadow as GenericSimpleShadow;
 
-/// A computed value for the `filter` property.
-pub type FilterList = GenericFilterList<Filter>;
-
 /// A computed value for a single `filter`.
 #[cfg(feature = "gecko")]
 pub type Filter = GenericFilter<Angle, Number, Length, SimpleShadow>;
@@ -26,16 +22,3 @@ pub type Filter = GenericFilter<Angle, Number, Length, Impossible>;
 
 /// A computed value for the `drop-shadow()` filter.
 pub type SimpleShadow = GenericSimpleShadow<Color, Length, Length>;
-
-impl FilterList {
-    /// Returns the resulting opacity of this filter pipeline.
-    pub fn opacity(&self) -> Number {
-        let mut opacity = 0.;
-        for filter in &*self.0 {
-            if let GenericFilter::Opacity(factor) = *filter {
-                opacity *= factor
-            }
-        }
-        opacity
-    }
-}

components/style/values/computed/mod.rs

@@ -28,7 +28,7 @@ pub use self::background::BackgroundSize;
 pub use self::border::{BorderImageSlice, BorderImageWidth, BorderImageSideWidth};
 pub use self::border::{BorderRadius, BorderCornerRadius};
 pub use self::color::{Color, RGBAColor};
-pub use self::effects::FilterList;
+pub use self::effects::Filter;
 pub use self::flex::FlexBasis;
 pub use self::image::{Gradient, GradientItem, ImageLayer, LineDirection, Image, ImageRect};
 #[cfg(feature = "gecko")]

components/style/values/generics/effects.rs

@@ -4,18 +4,9 @@
 
 //! Generic types for CSS values related to effects.
 
-use std::fmt;
-use style_traits::ToCss;
 #[cfg(feature = "gecko")]
 use values::specified::url::SpecifiedUrl;
 
-/// A generic value for the `filter` property.
-///
-/// Keyword `none` is represented by an empty slice.
-#[cfg_attr(feature = "servo", derive(Deserialize, HeapSizeOf, Serialize))]
-#[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToComputedValue)]
-pub struct FilterList<Filter>(pub Box<[Filter]>);
-
 /// A generic value for a single `filter`.
 #[cfg_attr(feature = "servo", derive(Deserialize, HeapSizeOf, Serialize))]
 #[derive(Clone, Debug, HasViewportPercentage, PartialEq, ToComputedValue, ToCss)]
@@ -71,39 +62,3 @@ pub struct SimpleShadow<Color, SizeLength, ShapeLength> {
     /// Blur radius.
     pub blur: ShapeLength,
 }
-
-impl<F> FilterList<F> {
-    /// Returns `none`.
-    #[inline]
-    pub fn none() -> Self {
-        FilterList(vec![].into_boxed_slice())
-    }
-}
-
-impl<F> From<Vec<F>> for FilterList<F> {
-    #[inline]
-    fn from(vec: Vec<F>) -> Self {
-        FilterList(vec.into_boxed_slice())
-    }
-}
-
-impl<F> ToCss for FilterList<F>
-where
-    F: ToCss,
-{
-    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
-    where
-        W: fmt::Write
-    {
-        if let Some((first, rest)) = self.0.split_first() {
-            first.to_css(dest)?;
-            for filter in rest {
-                dest.write_str(" ")?;
-                filter.to_css(dest)?;
-            }
-            Ok(())
-        } else {
-            dest.write_str("none")
-        }
-    }
-}

components/style/values/generics/mod.rs

@@ -9,7 +9,7 @@ use counter_style::{Symbols, parse_counter_style_name};
 use cssparser::Parser;
 use parser::{Parse, ParserContext};
 use std::fmt;
-use style_traits::{OneOrMoreSeparated, CommaSeparator, ToCss, ParseError, StyleParseError};
+use style_traits::{Comma, OneOrMoreSeparated, ParseError, StyleParseError, ToCss};
 use super::CustomIdent;
 use values::specified::url::SpecifiedUrl;
 
@@ -125,7 +125,7 @@ pub struct FontSettingTag<T> {
 }
 
 impl<T> OneOrMoreSeparated for FontSettingTag<T> {
-    type S = CommaSeparator;
+    type S = Comma;
 }
 
 impl<T: ToCss> ToCss for FontSettingTag<T> {

components/style/values/mod.rs

@@ -52,7 +52,7 @@ impl Parse for Impossible {
 
 /// A struct representing one of two kinds of values.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-#[derive(Clone, Copy, HasViewportPercentage, PartialEq, ToCss)]
+#[derive(Clone, Copy, HasViewportPercentage, PartialEq, ToComputedValue, ToCss)]
 pub enum Either<A, B> {
     /// The first value.
     First(A),
@@ -80,27 +80,6 @@ impl<A: Parse, B: Parse> Parse for Either<A, B> {
     }
 }
 
-use self::computed::{Context, ToComputedValue};
-
-impl<A: ToComputedValue, B: ToComputedValue> ToComputedValue for Either<A, B> {
-    type ComputedValue = Either<A::ComputedValue, B::ComputedValue>;
-
-    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        match *self {
-            Either::First(ref a) => Either::First(a.to_computed_value(context)),
-            Either::Second(ref a) => Either::Second(a.to_computed_value(context)),
-        }
-    }
-
-    #[inline]
-    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        match *computed {
-            Either::First(ref a) => Either::First(ToComputedValue::from_computed_value(a)),
-            Either::Second(ref a) => Either::Second(ToComputedValue::from_computed_value(a)),
-        }
-    }
-}
-
 /// https://drafts.csswg.org/css-values-4/#custom-idents
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]

components/style/values/specified/effects.rs

@@ -12,7 +12,6 @@ use values::Impossible;
 use values::computed::{Context, Number as ComputedNumber, ToComputedValue};
 use values::computed::effects::SimpleShadow as ComputedSimpleShadow;
 use values::generics::effects::Filter as GenericFilter;
-use values::generics::effects::FilterList as GenericFilterList;
 use values::generics::effects::SimpleShadow as GenericSimpleShadow;
 use values::specified::{Angle, Percentage};
 use values::specified::color::Color;
@@ -20,9 +19,6 @@ use values::specified::length::Length;
 #[cfg(feature = "gecko")]
 use values::specified::url::SpecifiedUrl;
 
-/// A specified value for the `filter` property.
-pub type FilterList = GenericFilterList<Filter>;
-
 /// A specified value for a single `filter`.
 #[cfg(feature = "gecko")]
 pub type Filter = GenericFilter<Angle, Factor, Length, SimpleShadow>;
@@ -46,23 +42,6 @@ pub enum Factor {
 /// A specified value for the `drop-shadow()` filter.
 pub type SimpleShadow = GenericSimpleShadow<Option<Color>, Length, Option<Length>>;
 
-impl Parse for FilterList {
-    #[inline]
-    fn parse<'i, 't>(
-        context: &ParserContext,
-        input: &mut Parser<'i, 't>
-    ) -> Result<Self, ParseError<'i>> {
-        let mut filters = vec![];
-        while let Ok(filter) = input.try(|i| Filter::parse(context, i)) {
-            filters.push(filter);
-        }
-        if filters.is_empty() {
-            input.expect_ident_matching("none")?;
-        }
-        Ok(GenericFilterList(filters.into_boxed_slice()))
-    }
-}
-
 impl Parse for Filter {
     #[inline]
     fn parse<'i, 't>(
@@ -113,7 +92,7 @@ impl Parse for Factor {
 
 impl ToComputedValue for Factor {
     /// This should actually be `ComputedNumberOrPercentage`, but layout uses
-    /// `computed::effects::FilterList` directly in `StackingContext`.
+    /// `computed::effects::Filter` directly in `StackingContext`.
     type ComputedValue = ComputedNumber;
 
     #[inline]

components/style/values/specified/mod.rs

@@ -33,7 +33,7 @@ pub use self::background::BackgroundSize;
 pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth};
 pub use self::border::{BorderImageSideWidth, BorderRadius, BorderSideWidth};
 pub use self::color::{Color, RGBAColor};
-pub use self::effects::FilterList;
+pub use self::effects::Filter;
 pub use self::flex::FlexBasis;
 #[cfg(feature = "gecko")]
 pub use self::gecko::ScrollSnapPoint;

components/style_traits/lib.rs

@@ -71,7 +71,7 @@ pub mod values;
 #[macro_use]
 pub mod viewport;
 
-pub use values::{ToCss, OneOrMoreSeparated, CommaSeparator, SpaceSeparator, IsCommaSeparator};
+pub use values::{Comma, CommaWithSpace, OneOrMoreSeparated, Separator, Space, ToCss};
 pub use viewport::HasViewportPercentage;
 
 /// The error type for all CSS parsing routines.

components/style_traits/values.rs

@@ -5,7 +5,7 @@
 //! Helper types and traits for the handling of CSS values.
 
 use app_units::Au;
-use cssparser::{UnicodeRange, serialize_string};
+use cssparser::{BasicParseError, ParseError, Parser, Token, UnicodeRange, serialize_string};
 use std::fmt::{self, Write};
 
 /// Serialises a value according to its CSS representation.
@@ -184,38 +184,102 @@ where
 /// Type used as the associated type in the `OneOrMoreSeparated` trait on a
 /// type to indicate that a serialized list of elements of this type is
 /// separated by commas.
-pub struct CommaSeparator;
+pub struct Comma;
 
 /// Type used as the associated type in the `OneOrMoreSeparated` trait on a
 /// type to indicate that a serialized list of elements of this type is
 /// separated by spaces.
-pub struct SpaceSeparator;
+pub struct Space;
+
+/// Type used as the associated type in the `OneOrMoreSeparated` trait on a
+/// type to indicate that a serialized list of elements of this type is
+/// separated by commas, but spaces without commas are also allowed when
+/// parsing.
+pub struct CommaWithSpace;
 
 /// A trait satisfied by the types corresponding to separators.
 pub trait Separator {
     /// The separator string that the satisfying separator type corresponds to.
     fn separator() -> &'static str;
+
+    /// Parses a sequence of values separated by this separator.
+    ///
+    /// The given closure is called repeatedly for each item in the sequence.
+    ///
+    /// Successful results are accumulated in a vector.
+    ///
+    /// This method returns `Err(_)` the first time a closure does or if
+    /// the separators aren't correct.
+    fn parse<'i, 't, F, T, E>(
+        parser: &mut Parser<'i, 't>,
+        parse_one: F,
+    ) -> Result<Vec<T>, ParseError<'i, E>>
+    where
+        F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>;
 }
 
-impl Separator for CommaSeparator {
+impl Separator for Comma {
     fn separator() -> &'static str {
         ", "
     }
-}
 
-impl Separator for SpaceSeparator {
+    fn parse<'i, 't, F, T, E>(
-    fn separator() -> &'static str {
+        input: &mut Parser<'i, 't>,
-        " "
+        parse_one: F,
+    ) -> Result<Vec<T>, ParseError<'i, E>>
+    where
+        F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>
+    {
+        input.parse_comma_separated(parse_one)
     }
 }
 
-/// Trait that indicates that satisfying separator types are comma separators.
+impl Separator for Space {
-/// This seems kind of redundant, but we aren't able to express type equality
+    fn separator() -> &'static str {
-/// constraints yet.
+        " "
-/// https://github.com/rust-lang/rust/issues/20041
+    }
-pub trait IsCommaSeparator {}
 
-impl IsCommaSeparator for CommaSeparator {}
+    fn parse<'i, 't, F, T, E>(
+        input: &mut Parser<'i, 't>,
+        mut parse_one: F,
+    ) -> Result<Vec<T>, ParseError<'i, E>>
+    where
+        F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>
+    {
+        let mut results = vec![parse_one(input)?];
+        while let Ok(item) = input.try(&mut parse_one) {
+            results.push(item);
+        }
+        Ok(results)
+    }
+}
+
+impl Separator for CommaWithSpace {
+    fn separator() -> &'static str {
+        ", "
+    }
+
+    fn parse<'i, 't, F, T, E>(
+        input: &mut Parser<'i, 't>,
+        mut parse_one: F,
+    ) -> Result<Vec<T>, ParseError<'i, E>>
+    where
+        F: for<'tt> FnMut(&mut Parser<'i, 'tt>) -> Result<T, ParseError<'i, E>>
+    {
+        let mut results = vec![parse_one(input)?];
+        loop {
+            let comma = input.try(|i| i.expect_comma()).is_ok();
+            if let Ok(item) = input.try(&mut parse_one) {
+                results.push(item);
+            } else if comma {
+                return Err(BasicParseError::UnexpectedToken(Token::Comma).into());
+            } else {
+                break;
+            }
+        }
+        Ok(results)
+    }
+}
 
 /// Marker trait on T to automatically implement ToCss for Vec<T> when T's are
 /// separated by some delimiter `delim`.
@@ -225,7 +289,7 @@ pub trait OneOrMoreSeparated {
 }
 
 impl OneOrMoreSeparated for UnicodeRange {
-    type S = CommaSeparator;
+    type S = Comma;
 }
 
 impl<T> ToCss for Vec<T> where T: ToCss + OneOrMoreSeparated {