Auto merge of #16862 - CJKu:bug-1310885-part-6, r=heycam

stylo: Pass Cached ImageValue from stylo back to gecko

<!-- Please describe your changes on the following line: -->
This is  part 6 ~ part 9 patch in bug 1310885

gecko bug link:
https://bugzilla.mozilla.org/show_bug.cgi?id=1310885
---
<!-- 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
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] 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. -->

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

components/layout/construct.rs

@@ -53,6 +53,7 @@ use style::computed_values::position;
 use style::context::SharedStyleContext;
 use style::logical_geometry::Direction;
 use style::properties::ServoComputedValues;
+use style::properties::longhands::list_style_image;
 use style::selector_parser::{PseudoElement, RestyleDamage};
 use style::servo::restyle_damage::{BUBBLE_ISIZES, RECONSTRUCT_FLOW};
 use style::values::Either;
@@ -1206,13 +1207,13 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode>
                                 -> ConstructionResult {
         let flotation = FloatKind::from_property(flotation);
         let marker_fragments = match node.style(self.style_context()).get_list().list_style_image {
-            Either::First(ref url_value) => {
+            list_style_image::computed_value::T(Either::First(ref url_value)) => {
                 let image_info = box ImageFragmentInfo::new(url_value.url().map(|u| u.clone()),
                                                             node,
                                                             &self.layout_context);
                 vec![Fragment::new(node, SpecificFragmentInfo::Image(image_info), self.layout_context)]
             }
-            Either::Second(_none) => {
+            list_style_image::computed_value::T(Either::Second(_none)) => {
                 match ListStyleTypeContent::from_list_style_type(node.style(self.style_context())
                                                                      .get_list()
                                                                      .list_style_type) {

components/style/gecko/conversions.rs

@@ -10,7 +10,7 @@
 
 use app_units::Au;
 use gecko::values::{convert_rgba_to_nscolor, GeckoStyleCoordConvertible};
-use gecko_bindings::bindings::{Gecko_CreateGradient, Gecko_SetGradientImageValue, Gecko_SetUrlImageValue};
+use gecko_bindings::bindings::{Gecko_CreateGradient, Gecko_SetGradientImageValue, Gecko_SetLayerImageImageValue};
 use gecko_bindings::bindings::{Gecko_InitializeImageCropRect, Gecko_SetImageElement};
 use gecko_bindings::structs::{nsCSSUnit, nsStyleCoord_CalcValue, nsStyleImage};
 use gecko_bindings::structs::{nsresult, SheetType};
@@ -144,7 +144,7 @@ impl nsStyleImage {
             },
             GenericImage::Url(ref url) => {
                 unsafe {
-                    Gecko_SetUrlImageValue(self, url.for_ffi());
+                    Gecko_SetLayerImageImageValue(self, url.image_value.clone().unwrap().get());
                     // We unfortunately must make any url() value uncacheable, since
                     // the applicable declarations cache is not per document, but
                     // global, and the imgRequestProxy objects we store in the style
@@ -157,7 +157,7 @@ impl nsStyleImage {
             },
             GenericImage::Rect(ref image_rect) => {
                 unsafe {
-                    Gecko_SetUrlImageValue(self, image_rect.url.for_ffi());
+                    Gecko_SetLayerImageImageValue(self, image_rect.url.image_value.clone().unwrap().get());
                     Gecko_InitializeImageCropRect(self);
 
                     // We unfortunately must make any url() value uncacheable, since

components/style/gecko/generated/bindings.rs

@@ -832,8 +832,11 @@ extern "C" {
     pub fn Gecko_ReleaseImageValueArbitraryThread(aPtr: *mut ImageValue);
 }
 extern "C" {
-    pub fn Gecko_SetUrlImageValue(image: *mut nsStyleImage,
-                                  uri: ServoBundledURI);
+    pub fn Gecko_ImageValue_Create(uri: ServoBundledURI) -> *mut ImageValue;
+}
+extern "C" {
+    pub fn Gecko_SetLayerImageImageValue(image: *mut nsStyleImage,
+                                         imageValue: *mut ImageValue);
 }
 extern "C" {
     pub fn Gecko_SetImageElement(image: *mut nsStyleImage,
@@ -855,8 +858,8 @@ extern "C" {
     pub fn Gecko_SetListStyleImageNone(style_struct: *mut nsStyleList);
 }
 extern "C" {
-    pub fn Gecko_SetListStyleImage(style_struct: *mut nsStyleList,
-                                   uri: ServoBundledURI);
+    pub fn Gecko_SetListStyleImageImageValue(style_struct: *mut nsStyleList,
+    	                                     imageValue: *mut ImageValue);
 }
 extern "C" {
     pub fn Gecko_CopyListStyleImageFrom(dest: *mut nsStyleList,
@@ -867,16 +870,16 @@ extern "C" {
                                       len: usize);
 }
 extern "C" {
-    pub fn Gecko_SetCursorImage(cursor: *mut nsCursorImage,
-                                uri: ServoBundledURI);
+    pub fn Gecko_SetCursorImageValue(cursor: *mut nsCursorImage,
+                                     imageValue: *mut ImageValue);
 }
 extern "C" {
     pub fn Gecko_CopyCursorArrayFrom(dest: *mut nsStyleUserInterface,
                                      src: *const nsStyleUserInterface);
 }
 extern "C" {
-    pub fn Gecko_SetContentDataImage(content_data: *mut nsStyleContentData,
-                                     uri: ServoBundledURI);
+    pub fn Gecko_SetContentDataImageValue(content_data: *mut nsStyleContentData,
+                                    imageValue: *mut ImageValue);
 }
 extern "C" {
     pub fn Gecko_SetContentDataArray(content_data: *mut nsStyleContentData,

components/style/gecko/url.rs

@@ -6,6 +6,7 @@
 
 use cssparser::CssStringWriter;
 use gecko_bindings::structs::{ServoBundledURI, URLExtraData};
+use gecko_bindings::structs::root::mozilla::css::ImageValue;
 use gecko_bindings::sugar::refptr::RefPtr;
 use parser::ParserContext;
 use std::borrow::Cow;
@@ -24,6 +25,10 @@ pub struct SpecifiedUrl {
 
     /// The URL extra data.
     pub extra_data: RefPtr<URLExtraData>,
+
+    /// Cache ImageValue, if any, so that we can reuse it while rematching a
+    /// a property with this specified url value.
+    pub image_value: Option<RefPtr<ImageValue>>,
 }
 
 impl SpecifiedUrl {
@@ -37,6 +42,7 @@ impl SpecifiedUrl {
         Ok(SpecifiedUrl {
             serialization: Arc::new(url.into_owned()),
             extra_data: context.url_data.clone(),
+            image_value: None,
         })
     }
 
@@ -76,6 +82,21 @@ impl SpecifiedUrl {
             mExtraData: self.extra_data.get(),
         }
     }
+
+    /// Build and carry an image value on request.
+    pub fn build_image_value(&mut self) {
+        use gecko_bindings::bindings::Gecko_ImageValue_Create;
+
+        debug_assert_eq!(self.image_value, None);
+        self.image_value = {
+            unsafe {
+                let ptr = Gecko_ImageValue_Create(self.for_ffi());
+                // We do not expect Gecko_ImageValue_Create returns null.
+                debug_assert!(!ptr.is_null());
+                Some(RefPtr::from_addrefed(ptr))
+            }
+        }
+    }
 }
 
 impl ToCss for SpecifiedUrl {

components/style/properties/gecko.mako.rs

@@ -32,13 +32,13 @@ use gecko_bindings::bindings::Gecko_FontFamilyList_AppendGeneric;
 use gecko_bindings::bindings::Gecko_FontFamilyList_AppendNamed;
 use gecko_bindings::bindings::Gecko_FontFamilyList_Clear;
 use gecko_bindings::bindings::Gecko_SetCursorArrayLength;
-use gecko_bindings::bindings::Gecko_SetCursorImage;
+use gecko_bindings::bindings::Gecko_SetCursorImageValue;
 use gecko_bindings::bindings::Gecko_StyleTransition_SetUnsupportedProperty;
 use gecko_bindings::bindings::Gecko_NewCSSShadowArray;
 use gecko_bindings::bindings::Gecko_nsStyleFont_SetLang;
 use gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom;
-use gecko_bindings::bindings::Gecko_SetListStyleImage;
 use gecko_bindings::bindings::Gecko_SetListStyleImageNone;
+use gecko_bindings::bindings::Gecko_SetListStyleImageImageValue;
 use gecko_bindings::bindings::Gecko_SetListStyleType;
 use gecko_bindings::bindings::Gecko_SetNullImageValue;
 use gecko_bindings::bindings::ServoComputedValuesBorrowedOrNull;
@@ -2964,15 +2964,15 @@ fn static_assert() {
     pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) {
         use values::Either;
         match image {
-            Either::Second(_none) => {
+            longhands::list_style_image::computed_value::T(Either::Second(_none)) => {
                 unsafe {
                     Gecko_SetListStyleImageNone(&mut self.gecko);
                 }
             }
-            Either::First(ref url) => {
+            longhands::list_style_image::computed_value::T(Either::First(ref url)) => {
                 unsafe {
-                    Gecko_SetListStyleImage(&mut self.gecko,
-                                            url.for_ffi());
+                    Gecko_SetListStyleImageImageValue(&mut self.gecko,
+                                                      url.image_value.clone().unwrap().get());
                 }
                 // We don't need to record this struct as uncacheable, like when setting
                 // background-image to a url() value, since only properties in reset structs
@@ -3995,10 +3995,11 @@ clip-path
             Gecko_SetCursorArrayLength(&mut self.gecko, v.images.len());
         }
         for i in 0..v.images.len() {
-            let image = &v.images[i];
             unsafe {
-                Gecko_SetCursorImage(&mut self.gecko.mCursorImages[i], image.url.for_ffi());
+                Gecko_SetCursorImageValue(&mut self.gecko.mCursorImages[i],
+                                          v.images[i].url.clone().image_value.unwrap().get());
             }
+
             // We don't need to record this struct as uncacheable, like when setting
             // background-image to a url() value, since only properties in reset structs
             // are re-used from the applicable declaration cache, and the Pointing struct
@@ -4160,8 +4161,11 @@ clip-path
                             // When we support <custom-ident> values for list-style-type this will need to be updated
                             array[2].set_atom_ident(style.to_css_string().into());
                         }
-                        ContentItem::Url(url) => {
-                            unsafe { bindings::Gecko_SetContentDataImage(&mut self.gecko.mContents[i], url.for_ffi()) }
+                        ContentItem::Url(ref url) => {
+                            unsafe {
+                                bindings::Gecko_SetContentDataImageValue(&mut self.gecko.mContents[i],
+                                    url.image_value.clone().unwrap().get())
+                            }
                         }
                     }
                 }

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

@@ -2441,6 +2441,7 @@ ${helpers.single_keyword("-moz-appearance",
 
 ${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed="True" if product == "gecko" else "False",
                           animation_value_type="none",
                           gecko_ffi_name="mBinding",
                           spec="Nonstandard (https://developer.mozilla.org/en-US/docs/Web/CSS/-moz-binding)",

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

@@ -148,7 +148,8 @@
         let mut content = vec![];
         loop {
             % if product == "gecko":
-                if let Ok(url) = input.try(|i| SpecifiedUrl::parse(context, i)) {
+                if let Ok(mut url) = input.try(|i| SpecifiedUrl::parse(context, i)) {
+                    url.build_image_value();
                     content.push(ContentItem::Url(url));
                     continue;
                 }

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

@@ -120,16 +120,19 @@ ${helpers.single_keyword("clip-rule", "nonzero evenodd",
 
 ${helpers.predefined_type("marker-start", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed="True" if product == "gecko" else "False",
                           animation_value_type="none",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-mid", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed="True" if product == "gecko" else "False",
                           animation_value_type="none",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 
 ${helpers.predefined_type("marker-end", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
+                          boxed="True" if product == "gecko" else "False",
                           animation_value_type="none",
                           spec="https://www.w3.org/TR/SVG2/painting.html#VertexMarkerProperties")}
 

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

@@ -37,9 +37,55 @@ ${helpers.single_keyword("list-style-type", """
     animation_value_type="none",
     spec="https://drafts.csswg.org/css-lists/#propdef-list-style-type")}
 
-${helpers.predefined_type("list-style-image", "UrlOrNone", "Either::Second(None_)",
-                          initial_specified_value="Either::Second(None_)", animation_value_type="none",
-                          spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image")}
+<%helpers:longhand name="list-style-image" animation_value_type="none"
+                   boxed="${product == 'gecko'}"
+                   spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image">
+    use std::fmt;
+    use values::HasViewportPercentage;
+    use values::computed::ComputedValueAsSpecified;
+    use values::specified::UrlOrNone;
+    pub use self::computed_value::T as SpecifiedValue;
+    use style_traits::ToCss;
+
+    pub mod computed_value {
+        use values::specified::UrlOrNone;
+
+        #[derive(Debug, Clone, PartialEq)]
+        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+        pub struct T(pub UrlOrNone);
+    }
+
+
+    impl ComputedValueAsSpecified for SpecifiedValue {}
+    no_viewport_percentage!(SpecifiedValue);
+
+    impl ToCss for SpecifiedValue {
+        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+            self.0.to_css(dest)
+        }
+    }
+
+    #[inline]
+    pub fn get_initial_value() -> computed_value::T {
+        computed_value::T(Either::Second(None_))
+    }
+    #[inline]
+    pub fn get_initial_specified_value() -> SpecifiedValue {
+        SpecifiedValue(Either::Second(None_))
+    }
+    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
+        % if product == "gecko":
+        let mut value = input.try(|input| UrlOrNone::parse(context, input))?;
+        if let Either::First(ref mut url) = value {
+            url.build_image_value();
+        }
+        % else :
+        let value = input.try(|input| UrlOrNone::parse(context, input))?;
+        % endif
+
+        return Ok(SpecifiedValue(value));
+    }
+</%helpers:longhand>
 
 <%helpers:longhand name="quotes" animation_value_type="none"
                    spec="https://drafts.csswg.org/css-content/#propdef-quotes">

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

@@ -131,7 +131,10 @@
         let mut images = vec![];
         loop {
             match input.try(|input| parse_image(context, input)) {
-                Ok(image) => images.push(image),
+                Ok(mut image) => {
+                    image.url.build_image_value();
+                    images.push(image)
+                }
                 Err(()) => break,
             }
             try!(input.expect_comma());

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

@@ -60,7 +60,7 @@
             (true, 2, None, None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Either::Second(None_),
+                    list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)),
                     list_style_type: list_style_type::SpecifiedValue::none,
                 })
             }
@@ -74,14 +74,14 @@
             (true, 1, Some(list_style_type), None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Either::Second(None_),
+                    list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)),
                     list_style_type: list_style_type,
                 })
             }
             (true, 1, None, None) => {
                 Ok(Longhands {
                     list_style_position: position,
-                    list_style_image: Either::Second(None_),
+                    list_style_image: list_style_image::SpecifiedValue(Either::Second(None_)),
                     list_style_type: list_style_type::SpecifiedValue::none,
                 })
             }

components/style/values/specified/image.rs

@@ -82,14 +82,34 @@ pub type ImageRect = GenericImageRect<NumberOrPercentage>;
 
 impl Parse for Image {
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Image, ()> {
-        if let Ok(url) = input.try(|input| SpecifiedUrl::parse(context, input)) {
-            return Ok(GenericImage::Url(url));
+        #[cfg(feature = "gecko")]
+        {
+          if let Ok(mut url) = input.try(|input| SpecifiedUrl::parse(context, input)) {
+              url.build_image_value();
+              return Ok(GenericImage::Url(url));
+          }
+        }
+        #[cfg(feature = "servo")]
+        {
+          if let Ok(url) = input.try(|input| SpecifiedUrl::parse(context, input)) {
+              return Ok(GenericImage::Url(url));
+          }
         }
         if let Ok(gradient) = input.try(|i| Gradient::parse(context, i)) {
             return Ok(GenericImage::Gradient(gradient));
         }
-        if let Ok(image_rect) = input.try(|input| ImageRect::parse(context, input)) {
-            return Ok(GenericImage::Rect(image_rect));
+        #[cfg(feature = "gecko")]
+        {
+            if let Ok(mut image_rect) = input.try(|input| ImageRect::parse(context, input)) {
+                image_rect.url.build_image_value();
+                return Ok(GenericImage::Rect(image_rect));
+            }
+        }
+        #[cfg(feature = "servo")]
+        {
+            if let Ok(image_rect) = input.try(|input| ImageRect::parse(context, input)) {
+                return Ok(GenericImage::Rect(image_rect));
+            }
         }
 
         Ok(GenericImage::Element(Image::parse_element(input)?))

ports/geckolib/glue.rs

@@ -1845,7 +1845,8 @@ pub extern "C" fn Servo_DeclarationBlock_SetBackgroundImage(declarations:
     let context = ParserContext::new(Origin::Author, url_data, &error_reporter,
                                      Some(CssRuleType::Style), PARSING_MODE_DEFAULT,
                                      QuirksMode::NoQuirks);
-    if let Ok(url) = SpecifiedUrl::parse_from_string(string.into(), &context) {
+    if let Ok(mut url) = SpecifiedUrl::parse_from_string(string.into(), &context) {
+        url.build_image_value();
         let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(
             vec![Either::Second(Image::Url(url))]
         ));

tests/unit/style/properties/serialization.rs

@@ -493,6 +493,7 @@ mod shorthand_serialization {
     }
 
     mod list_style {
+        use style::properties::longhands::list_style_image::SpecifiedValue as ListStyleImage;
         use style::properties::longhands::list_style_position::SpecifiedValue as ListStylePosition;
         use style::properties::longhands::list_style_type::SpecifiedValue as ListStyleType;
         use style::values::Either;
@@ -503,12 +504,17 @@ mod shorthand_serialization {
             let mut properties = Vec::new();
 
             let position = ListStylePosition::inside;
-            let image = Either::First(
-                SpecifiedUrl::new_for_testing("http://servo/test.png"));
+            let image =
+                ListStyleImage(Either::First(SpecifiedUrl::new_for_testing("http://servo/test.png")));
             let style_type = ListStyleType::disc;
 
             properties.push(PropertyDeclaration::ListStylePosition(position));
+
+            #[cfg(feature = "gecko")]
+            properties.push(PropertyDeclaration::ListStyleImage(Box::new(image)));
+            #[cfg(not(feature = "gecko"))]
             properties.push(PropertyDeclaration::ListStyleImage(image));
+
             properties.push(PropertyDeclaration::ListStyleType(style_type));
 
             let serialization = shorthand_properties_to_string(properties);