servo: Merge #12458 - Cleanups in autoarray helper (from Manishearth:style-gecko-only); r=emilio
authorManish Goregaokar <manishsmail@gmail.com>
Fri, 15 Jul 2016 22:31:37 -0700
changeset 339306 06a3525560321ad6f69dc7ede4035c73f8ef2cde
parent 339305 2f5db0f9f6e0e800de7b5a997428004b91233bdc
child 339307 147f08f69f65df94bdfbff7f76d0834c200ca2dc
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
servo: Merge #12458 - Cleanups in autoarray helper (from Manishearth:style-gecko-only); r=emilio Addresses @emilio's comments from #11851 - Replace gecko_autoarray_longhand with vector_longhand, make it configurable - Allow for empty vectors, use empty vector longhand in box-shadow Source-Repo: https://github.com/servo/servo Source-Revision: a8a6d83d07fa18c870a20113eca45ec16270e142
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/longhand/background.mako.rs
servo/components/style/properties/longhand/effects.mako.rs
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -32,22 +32,28 @@
         }
     </%call>
 </%def>
 
 // FIXME (Manishearth): Add computed_value_as_specified argument
 // and handle the empty case correctly
 <%doc>
     To be used in cases where we have a grammar like
-    "<thing> [ , <thing> ]*", but only support a single value
-    in servo
+    "<thing> [ , <thing> ]*". `gecko_only` should be set
+    to True for cases where Servo takes a single value
+    and Stylo supports vector values.
+
+    Setting allow_empty to False allows for cases where the vector
+    is empty. The grammar for these is usually "none | <thing> [ , <thing> ]*".
+    We assume that the default/initial value is an empty vector for these.
+    `initial_value` need not be defined for these.
 </%doc>
-<%def name="gecko_autoarray_longhand(name, **kwargs)">
+<%def name="vector_longhand(name, gecko_only=False, allow_empty=False, **kwargs)">
     <%call expr="longhand(name, **kwargs)">
-        % if product == "gecko":
+        % if product == "gecko" or not gecko_only:
             use cssparser::ToCss;
             use std::fmt;
 
             pub mod single_value {
                 use cssparser::Parser;
                 use parser::{ParserContext, ParserContextExtraData};
                 use properties::{CSSWideKeyword, DeclaredValue, Shorthand};
                 use values::computed::{TContext, ToComputedValue};
@@ -58,50 +64,78 @@
                 use super::single_value;
                 #[derive(Debug, Clone, PartialEq)]
                 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
                 pub struct T(pub Vec<single_value::computed_value::T>);
             }
 
             impl ToCss for computed_value::T {
                 fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                    if !self.0.is_empty() {
-                        try!(self.0[0].to_css(dest));
+                    let mut iter = self.0.iter();
+                    if let Some(val) = iter.next() {
+                        try!(val.to_css(dest));
+                    } else {
+                        % if allow_empty:
+                            try!(dest.write_str("none"));
+                        % else:
+                            error!("Found empty value for property ${name}");
+                        % endif
                     }
-                    for i in self.0.iter().skip(1) {
+                    for i in iter {
                         try!(dest.write_str(", "));
                         try!(i.to_css(dest));
                     }
                     Ok(())
                 }
             }
 
             #[derive(Debug, Clone, PartialEq)]
             #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
             pub struct SpecifiedValue(pub Vec<single_value::SpecifiedValue>);
 
             impl ToCss for SpecifiedValue {
                 fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                    if !self.0.is_empty() {
-                        try!(self.0[0].to_css(dest))
+                    let mut iter = self.0.iter();
+                    if let Some(val) = iter.next() {
+                        try!(val.to_css(dest));
+                    } else {
+                        % if allow_empty:
+                            try!(dest.write_str("none"));
+                        % else:
+                            error!("Found empty value for property ${name}");
+                        % endif
                     }
-                    for i in self.0.iter().skip(1) {
+                    for i in iter {
                         try!(dest.write_str(", "));
                         try!(i.to_css(dest));
                     }
                     Ok(())
                 }
             }
             pub fn get_initial_value() -> computed_value::T {
-                computed_value::T(vec![single_value::get_initial_value()])
+                % if allow_empty:
+                    computed_value::T(vec![])
+                % else:
+                    computed_value::T(vec![single_value::get_initial_value()])
+                % endif
             }
             pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-                input.parse_comma_separated(|parser| {
-                    single_value::parse(context, parser)
-                }).map(|ok| SpecifiedValue(ok))
+                % if allow_empty:
+                    if input.try(|input| input.expect_ident_matching("none")).is_ok() {
+                        Ok(SpecifiedValue(Vec::new()))
+                    } else {
+                        input.parse_comma_separated(|parser| {
+                            single_value::parse(context, parser)
+                        }).map(SpecifiedValue)
+                    }
+                % else:
+                    input.parse_comma_separated(|parser| {
+                        single_value::parse(context, parser)
+                    }).map(SpecifiedValue)
+                % endif
             }
             impl ToComputedValue for SpecifiedValue {
                 type ComputedValue = computed_value::T;
 
                 #[inline]
                 fn to_computed_value<Cx: TContext>(&self, context: &Cx) -> computed_value::T {
                     computed_value::T(self.0.iter().map(|x| x.to_computed_value(context)).collect())
                 }
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -11,17 +11,17 @@ use properties::longhands::background_po
 use properties::longhands::background_size::computed_value::T as BackgroundSize;
 use properties::longhands::border_spacing::computed_value::T as BorderSpacing;
 use properties::longhands::clip::computed_value::ClipRect;
 use properties::longhands::font_weight::computed_value::T as FontWeight;
 use properties::longhands::line_height::computed_value::T as LineHeight;
 use properties::longhands::text_shadow::computed_value::T as TextShadowList;
 use properties::longhands::text_shadow::computed_value::TextShadow;
 use properties::longhands::box_shadow::computed_value::T as BoxShadowList;
-use properties::longhands::box_shadow::computed_value::BoxShadow;
+use properties::longhands::box_shadow::single_value::computed_value::T as BoxShadow;
 use properties::longhands::transform::computed_value::ComputedMatrix;
 use properties::longhands::transform::computed_value::ComputedOperation as TransformOperation;
 use properties::longhands::transform::computed_value::T as TransformList;
 use properties::longhands::transform_origin::computed_value::T as TransformOrigin;
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
 use properties::longhands::z_index::computed_value::T as ZIndex;
 use properties::style_struct_traits::*;
--- a/servo/components/style/properties/longhand/background.mako.rs
+++ b/servo/components/style/properties/longhand/background.mako.rs
@@ -5,17 +5,17 @@
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
 <% data.new_style_struct("Background", inherited=False) %>
 
 ${helpers.predefined_type("background-color", "CSSColor",
     "::cssparser::Color::RGBA(::cssparser::RGBA { red: 0., green: 0., blue: 0., alpha: 0. }) /* transparent */",
     animatable=True)}
 
-<%helpers:gecko_autoarray_longhand name="background-image" animatable="False">
+<%helpers:vector_longhand gecko_only="True" name="background-image" animatable="False">
     use cssparser::ToCss;
     use std::fmt;
     use values::specified::Image;
     use values::LocalToCss;
 
     pub mod computed_value {
         use values::computed;
         #[derive(Debug, Clone, PartialEq)]
@@ -65,17 +65,17 @@
         fn to_computed_value<Cx: TContext>(&self, context: &Cx) -> computed_value::T {
             match *self {
                 SpecifiedValue(None) => computed_value::T(None),
                 SpecifiedValue(Some(ref image)) =>
                     computed_value::T(Some(image.to_computed_value(context))),
             }
         }
     }
-</%helpers:gecko_autoarray_longhand>
+</%helpers:vector_longhand>
 
 <%helpers:longhand name="background-position" animatable="True">
         use cssparser::ToCss;
         use std::fmt;
         use values::LocalToCss;
 
         pub mod computed_value {
             use values::computed::LengthOrPercentage;
--- a/servo/components/style/properties/longhand/effects.mako.rs
+++ b/servo/components/style/properties/longhand/effects.mako.rs
@@ -7,55 +7,34 @@
 // Box-shadow, etc.
 <% data.new_style_struct("Effects", inherited=False) %>
 
 ${helpers.predefined_type("opacity",
                           "Opacity",
                           "1.0",
                           animatable=True)}
 
-<%helpers:longhand name="box-shadow" animatable="True">
+<%helpers:vector_longhand name="box-shadow" allow_empty="True" animatable="True">
     use cssparser::{self, ToCss};
     use std::fmt;
     use values::LocalToCss;
 
     #[derive(Debug, Clone, PartialEq)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub struct SpecifiedValue(Vec<SpecifiedBoxShadow>);
-
-    #[derive(Debug, Clone, PartialEq)]
-    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub struct SpecifiedBoxShadow {
+    pub struct SpecifiedValue {
         pub offset_x: specified::Length,
         pub offset_y: specified::Length,
         pub blur_radius: specified::Length,
         pub spread_radius: specified::Length,
         pub color: Option<specified::CSSColor>,
         pub inset: bool,
     }
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            let mut iter = self.0.iter();
-            if let Some(shadow) = iter.next() {
-                try!(shadow.to_css(dest));
-            } else {
-                try!(dest.write_str("none"));
-                return Ok(())
-            }
-            for shadow in iter {
-                try!(dest.write_str(", "));
-                try!(shadow.to_css(dest));
-            }
-            Ok(())
-        }
-    }
-
-    impl ToCss for SpecifiedBoxShadow {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             if self.inset {
                 try!(dest.write_str("inset "));
             }
             try!(self.blur_radius.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.spread_radius.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.offset_x.to_css(dest));
@@ -70,105 +49,66 @@
         }
     }
 
     pub mod computed_value {
         use app_units::Au;
         use std::fmt;
         use values::computed;
 
-        #[derive(Clone, PartialEq, Debug)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub struct T(pub Vec<BoxShadow>);
-
         #[derive(Clone, PartialEq, Copy, Debug)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub struct BoxShadow {
+        pub struct T {
             pub offset_x: Au,
             pub offset_y: Au,
             pub blur_radius: Au,
             pub spread_radius: Au,
             pub color: computed::CSSColor,
             pub inset: bool,
         }
     }
 
     impl ToCss for computed_value::T {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            let mut iter = self.0.iter();
-            if let Some(shadow) = iter.next() {
-                try!(shadow.to_css(dest));
-            } else {
-                try!(dest.write_str("none"));
-                return Ok(())
-            }
-            for shadow in iter {
-                try!(dest.write_str(", "));
-                try!(shadow.to_css(dest));
-            }
-            Ok(())
-        }
-    }
-
-    impl ToCss for computed_value::BoxShadow {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             if self.inset {
                 try!(dest.write_str("inset "));
             }
             try!(self.blur_radius.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.spread_radius.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.offset_x.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.offset_y.to_css(dest));
             try!(dest.write_str(" "));
             try!(self.color.to_css(dest));
             Ok(())
         }
     }
 
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T(Vec::new())
-    }
-
-    pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        if input.try(|input| input.expect_ident_matching("none")).is_ok() {
-            Ok(SpecifiedValue(Vec::new()))
-        } else {
-            input.parse_comma_separated(parse_one_box_shadow).map(SpecifiedValue)
-        }
-    }
-
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         #[inline]
         fn to_computed_value<Cx: TContext>(&self, context: &Cx) -> computed_value::T {
-            computed_value::T(self.0.iter().map(|value| compute_one_box_shadow(value, context)).collect())
+            computed_value::T {
+                offset_x: self.offset_x.to_computed_value(context),
+                offset_y: self.offset_y.to_computed_value(context),
+                blur_radius: self.blur_radius.to_computed_value(context),
+                spread_radius: self.spread_radius.to_computed_value(context),
+                color: self.color
+                            .as_ref()
+                            .map(|color| color.parsed)
+                            .unwrap_or(cssparser::Color::CurrentColor),
+                inset: self.inset,
+            }
         }
     }
 
-    pub fn compute_one_box_shadow<Cx: TContext>(value: &SpecifiedBoxShadow, context: &Cx)
-                                  -> computed_value::BoxShadow {
-        computed_value::BoxShadow {
-            offset_x: value.offset_x.to_computed_value(context),
-            offset_y: value.offset_y.to_computed_value(context),
-            blur_radius: value.blur_radius.to_computed_value(context),
-            spread_radius: value.spread_radius.to_computed_value(context),
-            color: value.color
-                        .as_ref()
-                        .map(|color| color.parsed)
-                        .unwrap_or(cssparser::Color::CurrentColor),
-            inset: value.inset,
-        }
-    }
-
-    pub fn parse_one_box_shadow(input: &mut Parser) -> Result<SpecifiedBoxShadow, ()> {
+    pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
         use app_units::Au;
         let mut lengths = [specified::Length::Absolute(Au(0)); 4];
         let mut lengths_parsed = false;
         let mut color = None;
         let mut inset = false;
 
         loop {
             if !inset {
@@ -208,26 +148,26 @@
             break
         }
 
         // Lengths must be specified.
         if !lengths_parsed {
             return Err(())
         }
 
-        Ok(SpecifiedBoxShadow {
+        Ok(SpecifiedValue {
             offset_x: lengths[0],
             offset_y: lengths[1],
             blur_radius: lengths[2],
             spread_radius: lengths[3],
             color: color,
             inset: inset,
         })
     }
-</%helpers:longhand>
+</%helpers:vector_longhand>
 
 // FIXME: This prop should be animatable
 <%helpers:longhand name="clip" animatable="False">
     use cssparser::ToCss;
     use std::fmt;
     use values::LocalToCss;
 
     // NB: `top` and `left` are 0 if `auto` per CSS 2.1 11.1.2.