Auto merge of #18430 - bzbarsky:shrink-background-storage, r=xidorn

Shrink the storage needed by specified background images

<!-- Please describe your changes on the following line: -->
This makes a specified background image take 48 bytes instead of 1024 bytes.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix https://bugzilla.mozilla.org/show_bug.cgi?id=1397614

<!-- Either: -->
- [X] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

components/style/gecko/conversions.rs

@@ -151,8 +151,8 @@ impl nsStyleImage {
     /// Set a given Servo `Image` value into this `nsStyleImage`.
     pub fn set(&mut self, image: Image) {
         match image {
-            GenericImage::Gradient(gradient) => {
+            GenericImage::Gradient(boxed_gradient) => {
-                self.set_gradient(gradient)
+                self.set_gradient(*boxed_gradient)
             },
             GenericImage::Url(ref url) => {
                 unsafe {
@@ -399,7 +399,7 @@ impl nsStyleImage {
                            NumberOrPercentage::from_gecko_style_coord(&rect.data_at(2)),
                            NumberOrPercentage::from_gecko_style_coord(&rect.data_at(3))) {
                         (Some(top), Some(right), Some(bottom), Some(left)) =>
-                            Some(GenericImage::Rect(MozImageRect { url, top, right, bottom, left } )),
+                            Some(GenericImage::Rect(Box::new(MozImageRect { url, top, right, bottom, left } ))),
                         _ => {
                             debug_assert!(false, "mCropRect could not convert to NumberOrPercentage");
                             None
@@ -428,7 +428,7 @@ impl nsStyleImage {
         url
     }
 
-    unsafe fn get_gradient(self: &nsStyleImage) -> Gradient {
+    unsafe fn get_gradient(self: &nsStyleImage) -> Box<Gradient> {
         use gecko::values::convert_nscolor_to_rgba;
         use gecko_bindings::bindings::Gecko_GetGradientImageValue;
         use gecko_bindings::structs::{NS_STYLE_GRADIENT_SHAPE_CIRCULAR, NS_STYLE_GRADIENT_SHAPE_ELLIPTICAL};
@@ -581,7 +581,7 @@ impl nsStyleImage {
                 CompatMode::Modern
             };
 
-        Gradient { items, repeating: gecko_gradient.mRepeating, kind, compat_mode }
+        Box::new(Gradient { items, repeating: gecko_gradient.mRepeating, kind, compat_mode })
     }
 }
 

components/style/properties/shorthand/background.mako.rs

@@ -37,7 +37,11 @@
         let mut background_color = None;
 
         % for name in "image position_x position_y repeat size attachment origin clip".split():
-            let mut background_${name} = background_${name}::SpecifiedValue(Vec::new());
+            // 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));
         % endfor
         input.parse_comma_separated(|input| {
             // background-color can only be in the last element, so if it
@@ -197,8 +201,12 @@
 
     pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                -> Result<Longhands, ParseError<'i>> {
-        let mut position_x = background_position_x::SpecifiedValue(Vec::new());
+        // Vec grows from 0 to 4 by default on first push().  So allocate with
-        let mut position_y = background_position_y::SpecifiedValue(Vec::new());
+        // 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));
         let mut any = false;
 
         input.parse_comma_separated(|input| {

components/style/properties/shorthand/mask.mako.rs

@@ -38,7 +38,11 @@
     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():
-            let mut mask_${name} = mask_${name}::SpecifiedValue(Vec::new());
+            // 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));
         % endfor
 
         input.parse_comma_separated(|input| {
@@ -183,8 +187,12 @@
 
     pub fn parse_value<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
                                -> Result<Longhands, ParseError<'i>> {
-        let mut position_x = mask_position_x::SpecifiedValue(Vec::new());
+        // Vec grows from 0 to 4 by default on first push().  So allocate with
-        let mut position_y = mask_position_y::SpecifiedValue(Vec::new());
+        // 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));
         let mut any = false;
 
         input.parse_comma_separated(|input| {

components/style/values/generics/image.rs

@@ -11,20 +11,21 @@ use cssparser::serialize_identifier;
 use custom_properties::SpecifiedValue;
 use std::fmt;
 use style_traits::ToCss;
-use values::computed::ComputedValueAsSpecified;
+use values::computed::{ComputedValueAsSpecified, Context, ToComputedValue};
 
 /// An [image].
 ///
 /// [image]: https://drafts.csswg.org/css-images/#image-values
-#[derive(Clone, PartialEq, ToComputedValue)]
+#[derive(Clone, PartialEq)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum Image<Gradient, MozImageRect, ImageUrl> {
     /// A `<url()>` image.
     Url(ImageUrl),
-    /// A `<gradient>` image.
+    /// A `<gradient>` image.  Gradients are rather large, and not nearly as
-    Gradient(Gradient),
+    /// common as urls, so we box them here to keep the size of this enum sane.
-    /// A `-moz-image-rect` image
+    Gradient(Box<Gradient>),
-    Rect(MozImageRect),
+    /// A `-moz-image-rect` image.  Also fairly large and rare.
+    Rect(Box<MozImageRect>),
     /// A `-moz-element(# <element-id>)`
     Element(Atom),
     /// A paint worklet image.
@@ -33,6 +34,47 @@ pub enum Image<Gradient, MozImageRect, ImageUrl> {
     PaintWorklet(PaintWorklet),
 }
 
+// Can't just use derive(ToComputedValue) on Image, because when trying to do
+// "impl<T> ToComputedValue for Box<T>" the Rust compiler complains that
+// "impl<T> ToComputedValue for T where T: ComputedValueAsSpecified + Clone"
+// aleady implements ToComputedValue for std::boxed::Box<_> and hence we have
+// conflicting implementations.
+impl<Gradient: ToComputedValue,
+     MozImageRect: ToComputedValue,
+     ImageUrl: ToComputedValue> ToComputedValue for Image<Gradient, MozImageRect, ImageUrl> {
+    type ComputedValue = Image<<Gradient as ToComputedValue>::ComputedValue,
+                               <MozImageRect as ToComputedValue>::ComputedValue,
+                               <ImageUrl as ToComputedValue>::ComputedValue>;
+
+    #[inline]
+    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
+        match *self {
+            Image::Url(ref url) => Image::Url(url.to_computed_value(context)),
+            Image::Gradient(ref gradient) =>
+                Image::Gradient(Box::new(gradient.to_computed_value(context))),
+            Image::Rect(ref rect) => Image::Rect(Box::new(rect.to_computed_value(context))),
+            Image::Element(ref atom) => Image::Element(atom.to_computed_value(context)),
+            #[cfg(feature = "servo")]
+            Image::PaintWorklet(ref worklet) => Image::PaintWorklet(worklet.to_computed_value(context)),
+        }
+    }
+
+    #[inline]
+    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
+        match *computed {
+            Image::Url(ref url) => Image::Url(ImageUrl::from_computed_value(url)),
+            Image::Gradient(ref boxed_gradient) =>
+                Image::Gradient(Box::new(Gradient::from_computed_value(&*boxed_gradient))),
+            Image::Rect(ref boxed_rect) =>
+                Image::Rect(Box::new(MozImageRect::from_computed_value(&*boxed_rect))),
+            Image::Element(ref atom) => Image::Element(Atom::from_computed_value(atom)),
+            #[cfg(feature = "servo")]
+            Image::PaintWorklet(ref worklet) =>
+                Image::PaintWorklet(PaintWorklet::from_computed_value(worklet)),
+        }
+    }
+}
+
 /// A CSS gradient.
 /// https://drafts.csswg.org/css-images/#gradients
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]

components/style/values/specified/image.rs

@@ -138,7 +138,7 @@ impl Parse for Image {
             return Ok(GenericImage::Url(url));
         }
         if let Ok(gradient) = input.try(|i| Gradient::parse(context, i)) {
-            return Ok(GenericImage::Gradient(gradient));
+            return Ok(GenericImage::Gradient(Box::new(gradient)));
         }
         #[cfg(feature = "servo")]
         {
@@ -151,7 +151,7 @@ impl Parse for Image {
             {
                 image_rect.url.build_image_value();
             }
-            return Ok(GenericImage::Rect(image_rect));
+            return Ok(GenericImage::Rect(Box::new(image_rect)));
         }
         Ok(GenericImage::Element(Image::parse_element(input)?))
     }

tests/unit/stylo/size_of.rs

@@ -11,6 +11,8 @@ use style::data::{ElementData, ElementStyles, RestyleData};
 use style::gecko::selector_parser as real;
 use style::properties::ComputedValues;
 use style::rule_tree::{RuleNode, StrongRuleNode};
+use style::values::computed;
+use style::values::specified;
 
 #[test]
 fn size_of_selectors_dummy_types() {
@@ -47,3 +49,11 @@ size_of_test!(test_size_of_rule_node, RuleNode, 80);
 // This is huge, but we allocate it on the stack and then never move it,
 // we only pass `&mut SourcePropertyDeclaration` references around.
 size_of_test!(test_size_of_parsed_declaration, style::properties::SourcePropertyDeclaration, 704);
+
+size_of_test!(test_size_of_computed_image, computed::image::Image, 40);
+size_of_test!(test_size_of_specified_image, specified::image::Image, 40);
+
+// FIXME(bz): These can shrink if we move the None_ value inside the
+// enum instead of paying an extra word for the Either discriminant.
+size_of_test!(test_size_of_computed_image_layer, computed::image::ImageLayer, 48);
+size_of_test!(test_size_of_specified_image_layer, specified::image::ImageLayer, 48);