Construct URLValue eagerly and share it between specified value and style structs.

components/style/gecko/generated/bindings.rs

@@ -1378,7 +1378,7 @@ extern "C" {
     pub fn Gecko_NewShapeImage(shape: *mut StyleShapeSource);
 }
 extern "C" {
-    pub fn Gecko_StyleShapeSource_SetURLValue(shape: *mut StyleShapeSource, uri: ServoBundledURI);
+    pub fn Gecko_StyleShapeSource_SetURLValue(shape: *mut StyleShapeSource, uri: *mut URLValue);
 }
 extern "C" {
     pub fn Gecko_ResetFilters(effects: *mut nsStyleEffects, new_len: usize);
@@ -1387,13 +1387,13 @@ extern "C" {
     pub fn Gecko_CopyFiltersFrom(aSrc: *mut nsStyleEffects, aDest: *mut nsStyleEffects);
 }
 extern "C" {
-    pub fn Gecko_nsStyleFilter_SetURLValue(effects: *mut nsStyleFilter, uri: ServoBundledURI);
+    pub fn Gecko_nsStyleFilter_SetURLValue(effects: *mut nsStyleFilter, uri: *mut URLValue);
 }
 extern "C" {
     pub fn Gecko_nsStyleSVGPaint_CopyFrom(dest: *mut nsStyleSVGPaint, src: *const nsStyleSVGPaint);
 }
 extern "C" {
-    pub fn Gecko_nsStyleSVGPaint_SetURLValue(paint: *mut nsStyleSVGPaint, uri: ServoBundledURI);
+    pub fn Gecko_nsStyleSVGPaint_SetURLValue(paint: *mut nsStyleSVGPaint, uri: *mut URLValue);
 }
 extern "C" {
     pub fn Gecko_nsStyleSVGPaint_Reset(paint: *mut nsStyleSVGPaint);
@@ -1413,6 +1413,9 @@ extern "C" {
 extern "C" {
     pub fn Gecko_NewURLValue(uri: ServoBundledURI) -> *mut URLValue;
 }
+extern "C" {
+    pub fn Gecko_URLValue_SizeOfIncludingThis(url: *mut URLValue) -> usize;
+}
 extern "C" {
     pub fn Gecko_AddRefCSSURLValueArbitraryThread(aPtr: *mut URLValue);
 }
@@ -1522,7 +1525,7 @@ extern "C" {
     pub fn Gecko_CSSValue_SetArray(css_value: nsCSSValueBorrowedMut, len: i32);
 }
 extern "C" {
-    pub fn Gecko_CSSValue_SetURL(css_value: nsCSSValueBorrowedMut, uri: ServoBundledURI);
+    pub fn Gecko_CSSValue_SetURL(css_value: nsCSSValueBorrowedMut, uri: *mut URLValue);
 }
 extern "C" {
     pub fn Gecko_CSSValue_SetInt(css_value: nsCSSValueBorrowedMut, integer: i32, unit: nsCSSUnit);

components/style/gecko/url.rs

@@ -9,7 +9,7 @@ use gecko_bindings::bindings;
 use gecko_bindings::structs::{ServoBundledURI, URLExtraData};
 use gecko_bindings::structs::mozilla::css::URLValueData;
 use gecko_bindings::structs::root::{nsStyleImageRequest, RustString};
-use gecko_bindings::structs::root::mozilla::css::ImageValue;
+use gecko_bindings::structs::root::mozilla::css::{ImageValue, URLValue};
 use gecko_bindings::sugar::refptr::RefPtr;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use parser::{Parse, ParserContext};
@@ -121,39 +121,59 @@ impl MallocSizeOf for CssUrl {
 }
 
 /// A specified url() value for general usage.
-#[derive(Clone, Debug, PartialEq, Eq, MallocSizeOf, ToCss)]
+#[derive(Clone, Debug, ToCss)]
 pub struct SpecifiedUrl {
     /// The specified url value.
     pub url: CssUrl,
+    /// Gecko's URLValue so that we can reuse it while rematching a
+    /// property with this specified value.
+    #[css(skip)]
+    pub url_value: RefPtr<URLValue>,
 }
 trivial_to_computed_value!(SpecifiedUrl);
 
 impl SpecifiedUrl {
     fn from_css_url(url: CssUrl) -> Self {
-        SpecifiedUrl { url }
+        let url_value = unsafe {
+            let ptr = bindings::Gecko_NewURLValue(url.for_ffi());
+            // We do not expect Gecko_NewURLValue returns null.
+            debug_assert!(!ptr.is_null());
+            RefPtr::from_addrefed(ptr)
+        };
+        SpecifiedUrl { url, url_value }
     }
 
     /// Convert from URLValueData to SpecifiedUrl.
     pub unsafe fn from_url_value_data(url: &URLValueData) -> Result<Self, ()> {
         CssUrl::from_url_value_data(url).map(Self::from_css_url)
     }
+}
 
-    /// Create a bundled URI suitable for sending to Gecko
+impl PartialEq for SpecifiedUrl {
-    /// to be constructed into a css::URLValue.
+    fn eq(&self, other: &Self) -> bool {
-    ///
+        self.url.eq(&other.url)
-    /// XXX This is added temporially. It would be removed once we store
-    /// URLValue in SpecifiedUrl directly.
-    pub fn for_ffi(&self) -> ServoBundledURI {
-        self.url.for_ffi()
     }
 }
 
+impl Eq for SpecifiedUrl {}
+
 impl Parse for SpecifiedUrl {
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         CssUrl::parse(context, input).map(Self::from_css_url)
     }
 }
 
+impl MallocSizeOf for SpecifiedUrl {
+    fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
+        let mut n = self.url.size_of(ops);
+        // Although this is a RefPtr, this is the primary reference because
+        // SpecifiedUrl is responsible for creating the url_value. So we
+        // measure unconditionally here.
+        n += unsafe { bindings::Gecko_URLValue_SizeOfIncludingThis(self.url_value.get()) };
+        n
+    }
+}
+
 /// A specified url() value for image.
 ///
 /// This exists so that we can construct `ImageValue` and reuse it.

components/style/gecko_bindings/sugar/ns_css_value.rs

@@ -212,7 +212,7 @@ impl nsCSSValue {
 
     /// Set to a url value
     pub fn set_url(&mut self, url: &SpecifiedUrl) {
-        unsafe { bindings::Gecko_CSSValue_SetURL(self, url.for_ffi()) }
+        unsafe { bindings::Gecko_CSSValue_SetURL(self, url.url_value.get()) }
     }
 
     /// Set to an array of given length

components/style/properties/gecko.mako.rs

@@ -696,7 +696,7 @@ def set_gecko_property(ffi_name, expr):
             }
             SVGPaintKind::PaintServer(url) => {
                 unsafe {
-                    bindings::Gecko_nsStyleSVGPaint_SetURLValue(paint, url.for_ffi());
+                    bindings::Gecko_nsStyleSVGPaint_SetURLValue(paint, url.url_value.get());
                 }
             }
             SVGPaintKind::Color(color) => {
@@ -936,18 +936,9 @@ def set_gecko_property(ffi_name, expr):
 <%def name="impl_css_url(ident, gecko_ffi_name)">
     #[allow(non_snake_case)]
     pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
-        use gecko_bindings::sugar::refptr::RefPtr;
         match v {
             Either::First(url) => {
-                let refptr = unsafe {
+                self.gecko.${gecko_ffi_name}.set_move(url.url_value.clone())
-                    let ptr = bindings::Gecko_NewURLValue(url.for_ffi());
-                    if ptr.is_null() {
-                        self.gecko.${gecko_ffi_name}.clear();
-                        return;
-                    }
-                    RefPtr::from_addrefed(ptr)
-                };
-                self.gecko.${gecko_ffi_name}.set_move(refptr)
             }
             Either::Second(_none) => {
                 unsafe {
@@ -4443,7 +4434,7 @@ fn static_assert() {
                 },
                 Url(ref url) => {
                     unsafe {
-                        bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, url.for_ffi());
+                        bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, url.url_value.get());
                     }
                 },
             }
@@ -4969,7 +4960,7 @@ fn static_assert() {
             % if ident == "clip_path":
             ShapeSource::ImageOrUrl(ref url) => {
                 unsafe {
-                    bindings::Gecko_StyleShapeSource_SetURLValue(${ident}, url.for_ffi())
+                    bindings::Gecko_StyleShapeSource_SetURLValue(${ident}, url.url_value.get())
                 }
             }
             % elif ident == "shape_outside":