Bug 1549593 - Use OwnedSlice in the specified and computed values of most vector properties. r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 16 May 2019 23:21:37 +0000
changeset 474252 7321594dab268ea3f11722178c98d6412f1db078
parent 474251 c110061df4c16715a491ead8cab815a259ae7846
child 474253 7c2ff60588882f354678a676c2028eebd71f7557
push id113144
push usershindli@mozilla.com
push dateFri, 17 May 2019 16:44:55 +0000
treeherdermozilla-inbound@f4c4b796f845 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
bugs1549593
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1549593 - Use OwnedSlice in the specified and computed values of most vector properties. r=heycam This is just a refactor in the right direction. Eventual goal is: * All inherited properties use ArcSlice<>. * All reset properties use OwnedSlice<> (or ThinVec<>). No conversion happens at all, so we can remove all that glue, and also compute_iter and co. Of course there's work to do, but this is a step towards that. Differential Revision: https://phabricator.services.mozilla.com/D30127
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/helpers/animated_properties.mako.rs
servo/components/style/properties/shorthands/background.mako.rs
servo/components/style/properties/shorthands/box.mako.rs
servo/components/style/properties/shorthands/svg.mako.rs
servo/components/style/values/computed/mod.rs
servo/components/style_traits/owned_slice.rs
servo/components/style_traits/specified_value_info.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -3709,52 +3709,51 @@ fn static_assert() {
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_INVERT;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_OPACITY;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_SATURATE;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_SEPIA;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_HUE_ROTATE;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_DROP_SHADOW;
         use crate::gecko_bindings::structs::NS_STYLE_FILTER_URL;
 
-        let mut filters = Vec::new();
-        for filter in self.gecko.mFilters.iter(){
+        longhands::filter::computed_value::List(self.gecko.mFilters.iter().map(|filter| {
             match filter.mType {
                 % for func in FILTER_FUNCTIONS:
                 NS_STYLE_FILTER_${func.upper()} => {
-                    filters.push(Filter::${func}(
+                    Filter::${func}(
                         GeckoStyleCoordConvertible::from_gecko_style_coord(
-                            &filter.mFilterParameter).unwrap()));
+                            &filter.mFilterParameter
+                        ).unwrap()
+                    )
                 },
                 % endfor
                 NS_STYLE_FILTER_BLUR => {
-                    filters.push(Filter::Blur(NonNegativeLength::from_gecko_style_coord(
-                        &filter.mFilterParameter).unwrap()));
+                    Filter::Blur(NonNegativeLength::from_gecko_style_coord(
+                        &filter.mFilterParameter
+                    ).unwrap())
                 },
                 NS_STYLE_FILTER_HUE_ROTATE => {
-                    filters.push(Filter::HueRotate(
-                        GeckoStyleCoordConvertible::from_gecko_style_coord(
-                            &filter.mFilterParameter).unwrap()));
+                    Filter::HueRotate(GeckoStyleCoordConvertible::from_gecko_style_coord(
+                        &filter.mFilterParameter,
+                    ).unwrap())
                 },
                 NS_STYLE_FILTER_DROP_SHADOW => {
-                    filters.push(unsafe {
-                        Filter::DropShadow(
-                            (**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow(),
-                        )
-                    });
+                    Filter::DropShadow(unsafe {
+                        (**filter.__bindgen_anon_1.mDropShadow.as_ref()).mArray[0].to_simple_shadow()
+                    })
                 },
                 NS_STYLE_FILTER_URL => {
-                    filters.push(Filter::Url(unsafe {
+                    Filter::Url(unsafe {
                         let url = RefPtr::new(*filter.__bindgen_anon_1.mURL.as_ref());
                         ComputedUrl::from_url_value(url)
-                    }));
+                    })
                 }
-                _ => {},
+                _ => unreachable!("Unknown filter function?"),
             }
-        }
-        longhands::filter::computed_value::List(filters)
+        }).collect())
     }
 
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="InheritedBox">
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="InheritedTable"
@@ -3788,18 +3787,19 @@ fn static_assert() {
                                   -webkit-text-stroke-width text-emphasis-position">
 
     <% text_align_keyword = Keyword("text-align",
                                     "start end left right center justify -moz-center -moz-left -moz-right char",
                                     gecko_strip_moz_prefix=False) %>
     ${impl_keyword('text_align', 'mTextAlign', text_align_keyword)}
 
     pub fn set_text_shadow<I>(&mut self, v: I)
-        where I: IntoIterator<Item = SimpleShadow>,
-              I::IntoIter: ExactSizeIterator
+    where
+        I: IntoIterator<Item = SimpleShadow>,
+        I::IntoIter: ExactSizeIterator
     {
         let v = v.into_iter();
         self.gecko.mTextShadow.replace_with_new(v.len() as u32);
         for (servo, gecko_shadow) in v.zip(self.gecko.mTextShadow.iter_mut()) {
             gecko_shadow.set_from_simple_shadow(servo);
         }
     }
 
@@ -3807,18 +3807,18 @@ fn static_assert() {
         self.gecko.mTextShadow.copy_from(&other.gecko.mTextShadow);
     }
 
     pub fn reset_text_shadow(&mut self, other: &Self) {
         self.copy_text_shadow_from(other)
     }
 
     pub fn clone_text_shadow(&self) -> longhands::text_shadow::computed_value::T {
-        let buf = self.gecko.mTextShadow.iter().map(|v| v.to_simple_shadow()).collect();
-        longhands::text_shadow::computed_value::List(buf)
+        let buf = self.gecko.mTextShadow.iter().map(|v| v.to_simple_shadow()).collect::<Vec<_>>();
+        longhands::text_shadow::computed_value::List(buf.into())
     }
 
     fn clear_text_emphasis_style_if_string(&mut self) {
         if self.gecko.mTextEmphasisStyle == structs::NS_STYLE_TEXT_EMPHASIS_STYLE_STRING as u8 {
             self.gecko.mTextEmphasisStyleString.truncate();
             self.gecko.mTextEmphasisStyle = structs::NS_STYLE_TEXT_EMPHASIS_STYLE_NONE as u8;
         }
     }
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -108,20 +108,18 @@
             use crate::values::{Auto, Either, None_};
             ${caller.body()}
         }
 
         /// The definition of the computed value for ${name}.
         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};
+            % if not allow_empty or allow_empty == "NotInitial":
+            use smallvec::SmallVec;
             % endif
             use crate::values::computed::ComputedVecIter;
 
             /// The generic type defining the value for this property.
             ///
             /// Making this type generic allows the compiler to figure out the
             /// animated value for us, instead of having to implement it
             /// manually for every type we care about.
@@ -139,24 +137,26 @@
             )]
             pub struct List<T>(
                 % if not allow_empty:
                 #[css(iterable)]
                 % else:
                 #[css(if_empty = "none", iterable)]
                 % endif
                 % if allow_empty and allow_empty != "NotInitial":
-                pub Vec<T>,
+                pub crate::OwnedSlice<T>,
                 % else:
+                // FIXME(emilio): Add an OwnedNonEmptySlice type, and figure out
+                // something for transition-name, which is the only remaining
+                // user of NotInitial.
                 pub SmallVec<[T; 1]>,
                 % endif
             );
 
 
-            /// The computed value, effectively a list of single values.
             % if vector_animation_type:
             % if not animation_value_type:
                 Sorry, this is stupid but needed for now.
             % endif
 
             use crate::properties::animated_properties::ListAnimation;
             use crate::values::animated::{Animate, ToAnimatedValue, ToAnimatedZero, Procedure};
             use crate::values::distance::{SquaredDistance, ComputeSquaredDistance};
@@ -186,98 +186,87 @@
                     &self,
                     other: &Self,
                 ) -> Result<SquaredDistance, ()> {
                     self.0.squared_distance_${vector_animation_type}(&other.0)
                 }
             }
             % endif
 
+            /// The computed value, effectively a list of single values.
             pub type T = List<single_value::T>;
 
             pub type Iter<'a, 'cx, 'cx_a> = ComputedVecIter<'a, 'cx, 'cx_a, super::single_value::SpecifiedValue>;
-
-            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()
-                }
-            }
         }
 
         /// The specified value of ${name}.
         % if separator == "Comma":
         #[css(comma)]
         % endif
         #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
         pub struct SpecifiedValue(
             % if not allow_empty:
             #[css(iterable)]
             % else:
             #[css(if_empty = "none", iterable)]
             % endif
-            pub Vec<single_value::SpecifiedValue>,
+            pub crate::OwnedSlice<single_value::SpecifiedValue>,
         );
 
         pub fn get_initial_value() -> computed_value::T {
             % if allow_empty and allow_empty != "NotInitial":
-                computed_value::List(vec![])
+                computed_value::List(Default::default())
             % else:
                 let mut v = SmallVec::new();
                 v.push(single_value::get_initial_value());
                 computed_value::List(v)
             % endif
         }
 
         pub fn parse<'i, 't>(
             context: &ParserContext,
             input: &mut Parser<'i, 't>,
         ) -> Result<SpecifiedValue, ParseError<'i>> {
             use style_traits::Separator;
 
             % if allow_empty:
-                if input.try(|input| input.expect_ident_matching("none")).is_ok() {
-                    return Ok(SpecifiedValue(Vec::new()))
-                }
+            if input.try(|input| input.expect_ident_matching("none")).is_ok() {
+                return Ok(SpecifiedValue(Default::default()))
+            }
             % endif
 
-            style_traits::${separator}::parse(input, |parser| {
+            let v = style_traits::${separator}::parse(input, |parser| {
                 single_value::parse(context, parser)
-            }).map(SpecifiedValue)
+            })?;
+            Ok(SpecifiedValue(v.into()))
         }
 
         pub use self::single_value::SpecifiedValue as SingleSpecifiedValue;
 
         impl SpecifiedValue {
-            pub fn compute_iter<'a, 'cx, 'cx_a>(
+            fn compute_iter<'a, 'cx, 'cx_a>(
                 &'a self,
                 context: &'cx Context<'cx_a>,
             ) -> computed_value::Iter<'a, 'cx, 'cx_a> {
                 computed_value::Iter::new(context, &self.0)
             }
         }
 
         impl ToComputedValue for SpecifiedValue {
             type ComputedValue = computed_value::T;
 
             #[inline]
             fn to_computed_value(&self, context: &Context) -> computed_value::T {
-                computed_value::List(self.compute_iter(context).collect())
+                computed_value::List(self.0.iter().map(|i| i.to_computed_value(context)).collect())
             }
 
             #[inline]
             fn from_computed_value(computed: &computed_value::T) -> Self {
-                SpecifiedValue(computed.0.iter()
-                                    .map(ToComputedValue::from_computed_value)
-                                    .collect())
+                let iter = computed.0.iter().map(ToComputedValue::from_computed_value);
+                SpecifiedValue(iter.collect())
             }
         }
     </%call>
 </%def>
 <%def name="longhand(*args, **kwargs)">
     <%
         property = data.declare_longhand(*args, **kwargs)
         if property is None:
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -768,16 +768,17 @@ macro_rules! animated_list_impl {
                         },
                     }
                 }).sum()
             }
         }
     }
 }
 
+animated_list_impl!(<T> for crate::OwnedSlice<T>);
 animated_list_impl!(<T> for SmallVec<[T; 1]>);
 animated_list_impl!(<T> for Vec<T>);
 
 /// <https://drafts.csswg.org/css-transitions/#animtype-visibility>
 impl Animate for Visibility {
     #[inline]
     fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
         let (this_weight, other_weight) = procedure.weights();
--- a/servo/components/style/properties/shorthands/background.mako.rs
+++ b/servo/components/style/properties/shorthands/background.mako.rs
@@ -35,21 +35,21 @@
 
     pub fn parse_value<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
         let mut background_color = None;
 
         % for name in "image position_x position_y repeat size attachment origin clip".split():
-            // Vec grows from 0 to 4 by default on first push().  So allocate
-            // with capacity 1, so in the common case of only one item we don't
-            // way overallocate.  Note that we always push at least one item if
-            // parsing succeeds.
-            let mut background_${name} = background_${name}::SpecifiedValue(Vec::with_capacity(1));
+        // Vec grows from 0 to 4 by default on first push().  So allocate with
+        // capacity 1, so in the common case of only one item we don't way
+        // overallocate, then shrink.  Note that we always push at least one
+        // item if parsing succeeds.
+        let mut background_${name} = Vec::with_capacity(1);
         % endfor
         input.parse_comma_separated(|input| {
             // background-color can only be in the last element, so if it
             // is parsed anywhere before, the value is invalid.
             if background_color.is_some() {
                 return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
             }
 
@@ -94,46 +94,41 @@
             }
             let mut any = false;
             % for name in "image position repeat size attachment origin clip".split():
                 any = any || ${name}.is_some();
             % endfor
             any = any || background_color.is_some();
             if any {
                 if let Some(position) = position {
-                    background_position_x.0.push(position.horizontal);
-                    background_position_y.0.push(position.vertical);
+                    background_position_x.push(position.horizontal);
+                    background_position_y.push(position.vertical);
                 } else {
-                    background_position_x.0.push(PositionComponent::zero());
-                    background_position_y.0.push(PositionComponent::zero());
+                    background_position_x.push(PositionComponent::zero());
+                    background_position_y.push(PositionComponent::zero());
                 }
                 % for name in "image repeat size attachment origin clip".split():
                     if let Some(bg_${name}) = ${name} {
-                        background_${name}.0.push(bg_${name});
+                        background_${name}.push(bg_${name});
                     } else {
-                        background_${name}.0.push(background_${name}::single_value
+                        background_${name}.push(background_${name}::single_value
                                                                     ::get_initial_specified_value());
                     }
                 % endfor
                 Ok(())
             } else {
                 Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             }
         })?;
 
         Ok(expanded! {
              background_color: background_color.unwrap_or(Color::transparent()),
-             background_image: background_image,
-             background_position_x: background_position_x,
-             background_position_y: background_position_y,
-             background_repeat: background_repeat,
-             background_attachment: background_attachment,
-             background_size: background_size,
-             background_origin: background_origin,
-             background_clip: background_clip,
+             % for name in "image position_x position_y repeat size attachment origin clip".split():
+             background_${name}: background_${name}::SpecifiedValue(background_${name}.into()),
+             % endfor
          })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             let len = self.background_image.0.len();
             // There should be at least one declared value
             if len == 0 {
@@ -204,36 +199,36 @@
     use crate::values::specified::position::Position;
 
     pub fn parse_value<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
         // Vec grows from 0 to 4 by default on first push().  So allocate with
         // capacity 1, so in the common case of only one item we don't way
-        // overallocate.  Note that we always push at least one item if parsing
-        // succeeds.
-        let mut position_x = background_position_x::SpecifiedValue(Vec::with_capacity(1));
-        let mut position_y = background_position_y::SpecifiedValue(Vec::with_capacity(1));
+        // overallocate, then shrink.  Note that we always push at least one
+        // item if parsing succeeds.
+        let mut position_x = Vec::with_capacity(1);
+        let mut position_y = Vec::with_capacity(1);
         let mut any = false;
 
         input.parse_comma_separated(|input| {
             let value = Position::parse_quirky(context, input, AllowQuirks::Yes)?;
-            position_x.0.push(value.horizontal);
-            position_y.0.push(value.vertical);
+            position_x.push(value.horizontal);
+            position_y.push(value.vertical);
             any = true;
             Ok(())
         })?;
         if !any {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
         Ok(expanded! {
-            background_position_x: position_x,
-            background_position_y: position_y,
+            background_position_x: background_position_x::SpecifiedValue(position_x.into()),
+            background_position_y: background_position_y::SpecifiedValue(position_y.into()),
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             let len = self.background_position_x.0.len();
             if len == 0 || len != self.background_position_y.0.len() {
                 return Ok(());
--- a/servo/components/style/properties/shorthands/box.mako.rs
+++ b/servo/components/style/properties/shorthands/box.mako.rs
@@ -132,17 +132,17 @@ macro_rules! try_parse_one {
 
             % for prop in "duration timing_function delay".split():
             ${prop}s.push(result.transition_${prop});
             % endfor
         }
 
         Ok(expanded! {
             % for prop in "property duration timing_function delay".split():
-            transition_${prop}: transition_${prop}::SpecifiedValue(${prop}s),
+            transition_${prop}: transition_${prop}::SpecifiedValue(${prop}s.into()),
             % endfor
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             let property_len = self.transition_property.0.len();
 
@@ -261,17 +261,17 @@ macro_rules! try_parse_one {
         for result in results.into_iter() {
             % for prop in props:
             ${prop}s.push(result.animation_${prop});
             % endfor
         }
 
         Ok(expanded! {
             % for prop in props:
-            animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s),
+            animation_${prop}: animation_${prop}::SpecifiedValue(${prop}s.into()),
             % endfor
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             let len = self.animation_name.0.len();
             // There should be at least one declared value
--- a/servo/components/style/properties/shorthands/svg.mako.rs
+++ b/servo/components/style/properties/shorthands/svg.mako.rs
@@ -37,21 +37,21 @@
         }
     }
 
     pub fn parse_value<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
         % for name in "image mode position_x position_y size repeat origin clip composite".split():
-            // Vec grows from 0 to 4 by default on first push().  So allocate
-            // with capacity 1, so in the common case of only one item we don't
-            // way overallocate.  Note that we always push at least one item if
-            // parsing succeeds.
-            let mut mask_${name} = mask_${name}::SpecifiedValue(Vec::with_capacity(1));
+        // Vec grows from 0 to 4 by default on first push().  So allocate with
+        // capacity 1, so in the common case of only one item we don't way
+        // overallocate, then shrink.  Note that we always push at least one
+        // item if parsing succeeds.
+        let mut mask_${name} = Vec::with_capacity(1);
         % endfor
 
         input.parse_comma_separated(|input| {
             % for name in "image mode position size repeat origin clip composite".split():
                 let mut ${name} = None;
             % endfor
             loop {
                 if image.is_none() {
@@ -91,39 +91,39 @@
                 }
             }
             let mut any = false;
             % for name in "image mode position size repeat origin clip composite".split():
                 any = any || ${name}.is_some();
             % endfor
             if any {
                 if let Some(position) = position {
-                    mask_position_x.0.push(position.horizontal);
-                    mask_position_y.0.push(position.vertical);
+                    mask_position_x.push(position.horizontal);
+                    mask_position_y.push(position.vertical);
                 } else {
-                    mask_position_x.0.push(PositionComponent::zero());
-                    mask_position_y.0.push(PositionComponent::zero());
+                    mask_position_x.push(PositionComponent::zero());
+                    mask_position_y.push(PositionComponent::zero());
                 }
                 % for name in "image mode size repeat origin clip composite".split():
                     if let Some(m_${name}) = ${name} {
-                        mask_${name}.0.push(m_${name});
+                        mask_${name}.push(m_${name});
                     } else {
-                        mask_${name}.0.push(mask_${name}::single_value
+                        mask_${name}.push(mask_${name}::single_value
                                                         ::get_initial_specified_value());
                     }
                 % endfor
                 Ok(())
             } else {
                 Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
             }
         })?;
 
         Ok(expanded! {
             % for name in "image mode position_x position_y size repeat origin clip composite".split():
-                mask_${name}: mask_${name},
+                mask_${name}: mask_${name}::SpecifiedValue(mask_${name}.into()),
             % endfor
          })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             use crate::properties::longhands::mask_origin::single_value::computed_value::T as Origin;
             use crate::properties::longhands::mask_clip::single_value::computed_value::T as Clip;
@@ -204,37 +204,38 @@
     use crate::parser::Parse;
 
     pub fn parse_value<'i, 't>(
         context: &ParserContext,
         input: &mut Parser<'i, 't>,
     ) -> Result<Longhands, ParseError<'i>> {
         // Vec grows from 0 to 4 by default on first push().  So allocate with
         // capacity 1, so in the common case of only one item we don't way
-        // overallocate.  Note that we always push at least one item if parsing
-        // succeeds.
-        let mut position_x = mask_position_x::SpecifiedValue(Vec::with_capacity(1));
-        let mut position_y = mask_position_y::SpecifiedValue(Vec::with_capacity(1));
+        // overallocate, then shrink.  Note that we always push at least one
+        // item if parsing succeeds.
+        let mut position_x = Vec::with_capacity(1);
+        let mut position_y = Vec::with_capacity(1);
         let mut any = false;
 
         input.parse_comma_separated(|input| {
             let value = Position::parse(context, input)?;
-            position_x.0.push(value.horizontal);
-            position_y.0.push(value.vertical);
+            position_x.push(value.horizontal);
+            position_y.push(value.vertical);
             any = true;
             Ok(())
         })?;
 
         if !any {
             return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError));
         }
 
+
         Ok(expanded! {
-            mask_position_x: position_x,
-            mask_position_y: position_y,
+            mask_position_x: mask_position_x::SpecifiedValue(position_x.into()),
+            mask_position_y: mask_position_y::SpecifiedValue(position_y.into()),
         })
     }
 
     impl<'a> ToCss for LonghandsToSerialize<'a>  {
         fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
             let len = self.mask_position_x.0.len();
             if len == 0 || self.mask_position_y.0.len() != len {
                 return Ok(());
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -449,27 +449,25 @@ where
     T: ToComputedValue,
 {
     type ComputedValue = crate::OwnedSlice<<T as ToComputedValue>::ComputedValue>;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
         self.iter()
             .map(|item| item.to_computed_value(context))
-            .collect::<Vec<_>>()
-            .into()
+            .collect()
     }
 
     #[inline]
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
         computed
             .iter()
             .map(T::from_computed_value)
-            .collect::<Vec<_>>()
-            .into()
+            .collect()
     }
 }
 
 trivial_to_computed_value!(());
 trivial_to_computed_value!(bool);
 trivial_to_computed_value!(f32);
 trivial_to_computed_value!(i32);
 trivial_to_computed_value!(u8);
--- a/servo/components/style_traits/owned_slice.rs
+++ b/servo/components/style_traits/owned_slice.rs
@@ -5,17 +5,17 @@
 #![allow(unsafe_code)]
 
 //! A replacement for `Box<[T]>` that cbindgen can understand.
 
 use malloc_size_of::{MallocShallowSizeOf, MallocSizeOf, MallocSizeOfOps};
 use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
 use std::ptr::NonNull;
-use std::{fmt, mem, slice};
+use std::{fmt, iter, mem, slice};
 use to_shmem::{SharedMemoryBuilder, ToShmem};
 
 /// A struct that basically replaces a `Box<[T]>`, but which cbindgen can
 /// understand.
 ///
 /// We could rely on the struct layout of `Box<[T]>` per:
 ///
 ///   https://github.com/rust-lang/unsafe-code-guidelines/blob/master/reference/src/layout/pointers.md
@@ -159,8 +159,15 @@ impl<T: MallocSizeOf + Sized> MallocSize
 impl<T: ToShmem + Sized> ToShmem for OwnedSlice<T> {
     fn to_shmem(&self, builder: &mut SharedMemoryBuilder) -> mem::ManuallyDrop<Self> {
         unsafe {
             let dest = to_shmem::to_shmem_slice(self.iter(), builder);
             mem::ManuallyDrop::new(Self::from(Box::from_raw(dest)))
         }
     }
 }
+
+impl<T> iter::FromIterator<T> for OwnedSlice<T> {
+    #[inline]
+    fn from_iter<I: IntoIterator<Item = T>>(iter: I) -> Self {
+        Vec::from_iter(iter).into()
+    }
+}
--- a/servo/components/style_traits/specified_value_info.rs
+++ b/servo/components/style_traits/specified_value_info.rs
@@ -1,15 +1,16 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 //! Value information for devtools.
 
 use crate::arc_slice::ArcSlice;
+use crate::owned_slice::OwnedSlice;
 use servo_arc::Arc;
 use std::ops::Range;
 use std::sync::Arc as StdArc;
 
 /// Type of value that a property supports. This is used by Gecko's
 /// devtools to make sense about value it parses, and types listed
 /// here should match InspectorPropertyType in InspectorUtils.webidl.
 ///
@@ -109,16 +110,17 @@ macro_rules! impl_generic_specified_valu
             const SUPPORTED_TYPES: u8 = $param::SUPPORTED_TYPES;
             fn collect_completion_keywords(f: KeywordsCollectFn) {
                 $param::collect_completion_keywords(f);
             }
         }
     };
 }
 impl_generic_specified_value_info!(Option<T>);
+impl_generic_specified_value_info!(OwnedSlice<T>);
 impl_generic_specified_value_info!(Vec<T>);
 impl_generic_specified_value_info!(Arc<T>);
 impl_generic_specified_value_info!(StdArc<T>);
 impl_generic_specified_value_info!(ArcSlice<T>);
 impl_generic_specified_value_info!(Range<Idx>);
 
 impl<T1, T2> SpecifiedValueInfo for (T1, T2)
 where
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -4720,17 +4720,17 @@ pub extern "C" fn Servo_DeclarationBlock
         ParsingMode::DEFAULT,
         QuirksMode::NoQuirks,
         None,
         None,
     );
     let url = SpecifiedImageUrl::parse_from_string(string.into(), &context);
     let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(vec![Either::Second(
         Image::Url(url),
-    )]));
+    )].into()));
     write_locked_arc(declarations, |decls: &mut PropertyDeclarationBlock| {
         decls.push(decl, Importance::Normal);
     });
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SetTextDecorationColorOverride(
     declarations: &RawServoDeclarationBlock,