Auto merge of #20238 - upsuper:url-value, r=emilio

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

This is the Servo side change of [bug 1443046](https://bugzilla.mozilla.org/show_bug.cgi?id=1443046).

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

components/script/stylesheet_loader.rs

@@ -34,7 +34,7 @@ use style::shared_lock::{Locked, SharedRwLock};
 use style::stylesheets::{CssRules, ImportRule, Namespaces, Stylesheet, StylesheetContents, Origin};
 use style::stylesheets::StylesheetLoader as StyleStylesheetLoader;
 use style::stylesheets::import_rule::ImportSheet;
-use style::values::specified::url::SpecifiedUrl;
+use style::values::CssUrl;
 
 pub trait StylesheetOwner {
     /// Returns whether this element was inserted by the parser (i.e., it should
@@ -276,7 +276,7 @@ impl<'a> StyleStylesheetLoader for StylesheetLoader<'a> {
     /// the constructed `@import` rule.
     fn request_stylesheet(
         &self,
-        url: SpecifiedUrl,
+        url: CssUrl,
         source_location: SourceLocation,
         context: &ParserContext,
         lock: &SharedRwLock,

components/style/gecko/conversions.rs

@@ -17,7 +17,7 @@ use gecko_bindings::structs::{nsStyleImage, nsresult, SheetType};
 use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut};
 use std::f32::consts::PI;
 use stylesheets::{Origin, RulesMutateError};
-use values::computed::{Angle, CalcLengthOrPercentage, ComputedUrl, Gradient, Image};
+use values::computed::{Angle, CalcLengthOrPercentage, ComputedImageUrl, Gradient, Image};
 use values::computed::{Integer, LengthOrPercentage, LengthOrPercentageOrAuto, Percentage, TextAlign};
 use values::generics::box_::VerticalAlign;
 use values::generics::grid::{TrackListValue, TrackSize};
@@ -155,12 +155,12 @@ impl nsStyleImage {
             },
             GenericImage::Url(ref url) => {
                 unsafe {
-                    Gecko_SetLayerImageImageValue(self, url.image_value.clone().unwrap().get());
+                    Gecko_SetLayerImageImageValue(self, url.image_value.get());
                 }
             },
             GenericImage::Rect(ref image_rect) => {
                 unsafe {
-                    Gecko_SetLayerImageImageValue(self, image_rect.url.image_value.clone().unwrap().get());
+                    Gecko_SetLayerImageImageValue(self, image_rect.url.image_value.get());
                     Gecko_InitializeImageCropRect(self);
 
                     // Set CropRect
@@ -419,13 +419,11 @@ impl nsStyleImage {
         }
     }
 
-    unsafe fn get_image_url(self: &nsStyleImage) -> ComputedUrl {
+    unsafe fn get_image_url(self: &nsStyleImage) -> ComputedImageUrl {
         use gecko_bindings::bindings::Gecko_GetURLValue;
         let url_value = Gecko_GetURLValue(self);
-        let mut url = ComputedUrl::from_url_value_data(url_value.as_ref().unwrap())
+        ComputedImageUrl::from_url_value_data(url_value.as_ref().unwrap())
-                                    .expect("Could not convert to ComputedUrl");
+            .expect("Could not convert to ComputedUrl")
-        url.build_image_value();
-        url
     }
 
     unsafe fn get_gradient(self: &nsStyleImage) -> Box<Gradient> {

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

@@ -4,21 +4,23 @@
 
 //! Common handling for the specified value CSS url() values.
 
+use cssparser::Parser;
+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::ParserContext;
+use parser::{Parse, ParserContext};
 use servo_arc::{Arc, RawOffsetArc};
 use std::mem;
 use style_traits::ParseError;
 
-/// A specified url() value for gecko. Gecko does not eagerly resolve SpecifiedUrls.
+/// A CSS url() value for gecko.
 #[css(function = "url")]
 #[derive(Clone, Debug, PartialEq, ToCss)]
-pub struct SpecifiedUrl {
+pub struct CssUrl {
     /// The URL in unresolved string form.
     ///
     /// Refcounted since cloning this should be cheap and data: uris can be
@@ -28,15 +30,9 @@ pub struct SpecifiedUrl {
     /// The URL extra data.
     #[css(skip)]
     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.
-    #[css(skip)]
-    pub image_value: Option<RefPtr<ImageValue>>,
 }
-trivial_to_computed_value!(SpecifiedUrl);
 
-impl SpecifiedUrl {
+impl CssUrl {
     /// Try to parse a URL from a string value that is a valid CSS token for a
     /// URL.
     ///
@@ -44,10 +40,9 @@ impl SpecifiedUrl {
     pub fn parse_from_string<'a>(url: String,
                                  context: &ParserContext)
                                  -> Result<Self, ParseError<'a>> {
-        Ok(SpecifiedUrl {
+        Ok(CssUrl {
             serialization: Arc::new(url),
             extra_data: context.url_data.clone(),
-            image_value: None,
         })
     }
 
@@ -59,9 +54,8 @@ impl SpecifiedUrl {
     }
 
     /// Convert from URLValueData to SpecifiedUrl.
-    pub unsafe fn from_url_value_data(url: &URLValueData)
+    unsafe fn from_url_value_data(url: &URLValueData) -> Result<Self, ()> {
-                                       -> Result<SpecifiedUrl, ()> {
+        Ok(CssUrl {
-        Ok(SpecifiedUrl {
             serialization: if url.mUsingRustString {
                 let arc_type = url.mStrings.mRustString.as_ref()
                     as *const _ as
@@ -71,23 +65,9 @@ impl SpecifiedUrl {
                 Arc::new(url.mStrings.mString.as_ref().to_string())
             },
             extra_data: url.mExtraData.to_safe(),
-            image_value: None,
         })
     }
 
-    /// Convert from nsStyleImageRequest to SpecifiedUrl.
-    pub unsafe fn from_image_request(image_request: &nsStyleImageRequest) -> Result<SpecifiedUrl, ()> {
-        if image_request.mImageValue.mRawPtr.is_null() {
-            return Err(());
-        }
-
-        let image_value = image_request.mImageValue.mRawPtr.as_ref().unwrap();
-        let ref url_value_data = image_value._base;
-        let mut result = Self::from_url_value_data(url_value_data)?;
-        result.build_image_value();
-        Ok(result)
-    }
-
     /// Returns true if this URL looks like a fragment.
     /// See https://drafts.csswg.org/css-values/#local-urls
     pub fn is_fragment(&self) -> bool {
@@ -118,41 +98,158 @@ impl SpecifiedUrl {
             mExtraData: self.extra_data.get(),
         }
     }
+}
 
-    /// Build and carry an image value on request.
+impl Parse for CssUrl {
-    pub fn build_image_value(&mut self) {
+    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        use gecko_bindings::bindings::Gecko_ImageValue_Create;
+        let url = input.expect_url()?;
-
+        Self::parse_from_string(url.as_ref().to_owned(), context)
-        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 MallocSizeOf for SpecifiedUrl {
+impl Eq for CssUrl {}
+
+impl MallocSizeOf for CssUrl {
     fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
-        use gecko_bindings::bindings::Gecko_ImageValue_SizeOfIncludingThis;
-
-        let mut n = 0;
-
         // XXX: measure `serialization` once bug 1397971 lands
 
         // We ignore `extra_data`, because RefPtr is tricky, and there aren't
         // many of them in practise (sharing is common).
 
-        if let Some(ref image_value) = self.image_value {
+        0
-            // Although this is a RefPtr, this is the primary reference because
+    }
-            // SpecifiedUrl is responsible for creating the image_value. So we
+}
-            // measure unconditionally here.
-            n += unsafe { Gecko_ImageValue_SizeOfIncludingThis(image_value.clone().get()) };
-        }
 
+/// A specified url() value for general usage.
+#[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 {
+        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)
+    }
+}
+
+impl PartialEq for SpecifiedUrl {
+    fn eq(&self, other: &Self) -> bool {
+        self.url.eq(&other.url)
+    }
+}
+
+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.
+#[derive(Clone, Debug, ToCss)]
+pub struct SpecifiedImageUrl {
+    /// The specified url value.
+    pub url: CssUrl,
+    /// Gecko's ImageValue so that we can reuse it while rematching a
+    /// property with this specified value.
+    #[css(skip)]
+    pub image_value: RefPtr<ImageValue>,
+}
+trivial_to_computed_value!(SpecifiedImageUrl);
+
+impl SpecifiedImageUrl {
+    fn from_css_url(url: CssUrl) -> Self {
+        let image_value = unsafe {
+            let ptr = bindings::Gecko_ImageValue_Create(url.for_ffi());
+            // We do not expect Gecko_ImageValue_Create returns null.
+            debug_assert!(!ptr.is_null());
+            RefPtr::from_addrefed(ptr)
+        };
+        SpecifiedImageUrl { url, image_value }
+    }
+
+    /// Parse a URL from a string value. See SpecifiedUrl::parse_from_string.
+    pub fn parse_from_string<'a>(
+        url: String,
+        context: &ParserContext
+    ) -> Result<Self, ParseError<'a>> {
+        CssUrl::parse_from_string(url, context).map(Self::from_css_url)
+    }
+
+    /// 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)
+    }
+
+    /// Convert from nsStyleImageRequest to SpecifiedUrl.
+    pub unsafe fn from_image_request(image_request: &nsStyleImageRequest) -> Result<Self, ()> {
+        if image_request.mImageValue.mRawPtr.is_null() {
+            return Err(());
+        }
+
+        let image_value = image_request.mImageValue.mRawPtr.as_ref().unwrap();
+        let url_value_data = &image_value._base;
+        Self::from_url_value_data(url_value_data)
+    }
+}
+
+impl Parse for SpecifiedImageUrl {
+    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 PartialEq for SpecifiedImageUrl {
+    fn eq(&self, other: &Self) -> bool {
+        self.url.eq(&other.url)
+    }
+}
+
+impl Eq for SpecifiedImageUrl {}
+
+impl MallocSizeOf for SpecifiedImageUrl {
+    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 image_value. So we
+        // measure unconditionally here.
+        n += unsafe { bindings::Gecko_ImageValue_SizeOfIncludingThis(self.image_value.get()) };
+        n
+    }
+}
+
+/// The computed value of a CSS `url()`.
+pub type ComputedUrl = SpecifiedUrl;
+/// The computed value of a CSS `url()` for image.
+pub type ComputedImageUrl = SpecifiedImageUrl;

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 {
@@ -4072,8 +4063,7 @@ fn static_assert() {
             }
             longhands::list_style_image::computed_value::T(Either::First(ref url)) => {
                 unsafe {
-                    Gecko_SetListStyleImageImageValue(&mut self.gecko,
+                    Gecko_SetListStyleImageImageValue(&mut self.gecko, url.image_value.get());
-                                                      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
@@ -4092,7 +4082,7 @@ fn static_assert() {
     }
 
     pub fn clone_list_style_image(&self) -> longhands::list_style_image::computed_value::T {
-        use values::specified::url::SpecifiedUrl;
+        use values::specified::url::SpecifiedImageUrl;
         use values::{Either, None_};
 
         longhands::list_style_image::computed_value::T(
@@ -4101,8 +4091,8 @@ fn static_assert() {
                 false => {
                     unsafe {
                         let ref gecko_image_request = *self.gecko.mListStyleImage.mRawPtr;
-                        Either::First(SpecifiedUrl::from_image_request(gecko_image_request)
+                        Either::First(SpecifiedImageUrl::from_image_request(gecko_image_request)
-                                      .expect("mListStyleImage could not convert to SpecifiedUrl"))
+                                      .expect("mListStyleImage could not convert to SpecifiedImageUrl"))
                     }
                 }
             }
@@ -4444,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());
                     }
                 },
             }
@@ -4970,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":
@@ -5290,8 +5280,10 @@ clip-path
         }
         for i in 0..v.images.len() {
             unsafe {
-                Gecko_SetCursorImageValue(&mut self.gecko.mCursorImages[i],
+                Gecko_SetCursorImageValue(
-                                          v.images[i].url.clone().image_value.unwrap().get());
+                    &mut self.gecko.mCursorImages[i],
+                    v.images[i].url.image_value.get(),
+                );
             }
 
             // We don't need to record this struct as uncacheable, like when setting
@@ -5326,7 +5318,7 @@ clip-path
     pub fn clone_cursor(&self) -> longhands::cursor::computed_value::T {
         use values::computed::pointing::CursorImage;
         use style_traits::cursor::CursorKind;
-        use values::specified::url::SpecifiedUrl;
+        use values::specified::url::SpecifiedImageUrl;
 
         let keyword = match self.gecko.mCursor as u32 {
             structs::NS_STYLE_CURSOR_AUTO => CursorKind::Auto,
@@ -5371,8 +5363,8 @@ clip-path
         let images = self.gecko.mCursorImages.iter().map(|gecko_cursor_image| {
             let url = unsafe {
                 let gecko_image_request = gecko_cursor_image.mImage.mRawPtr.as_ref().unwrap();
-                SpecifiedUrl::from_image_request(&gecko_image_request)
+                SpecifiedImageUrl::from_image_request(&gecko_image_request)
-                    .expect("mCursorImages.mImage could not convert to SpecifiedUrl")
+                    .expect("mCursorImages.mImage could not convert to SpecifiedImageUrl")
             };
 
             let hotspot =
@@ -5550,8 +5542,10 @@ clip-path
                         }
                         ContentItem::Url(ref url) => {
                             unsafe {
-                                bindings::Gecko_SetContentDataImageValue(&mut self.gecko.mContents[i],
+                                bindings::Gecko_SetContentDataImageValue(
-                                    url.image_value.clone().unwrap().get())
+                                    &mut self.gecko.mContents[i],
+                                    url.image_value.get(),
+                                )
                             }
                         }
                     }
@@ -5578,7 +5572,7 @@ clip-path
         use values::computed::counters::{Content, ContentItem};
         use values::{CustomIdent, Either};
         use values::generics::CounterStyleOrNone;
-        use values::specified::url::SpecifiedUrl;
+        use values::specified::url::SpecifiedImageUrl;
         use values::specified::Attr;
 
         if self.gecko.mContents.is_empty() {
@@ -5641,8 +5635,8 @@ clip-path
                             let gecko_image_request =
                                 &**gecko_content.mContent.mImage.as_ref();
                             ContentItem::Url(
-                                SpecifiedUrl::from_image_request(gecko_image_request)
+                                SpecifiedImageUrl::from_image_request(gecko_image_request)
-                                    .expect("mContent could not convert to SpecifiedUrl")
+                                    .expect("mContent could not convert to SpecifiedImageUrl")
                             )
                         }
                     },

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

@@ -611,7 +611,6 @@ ${helpers.single_keyword("-moz-appearance",
 
 ${helpers.predefined_type("-moz-binding", "UrlOrNone", "Either::Second(None_)",
                           products="gecko",
-                          boxed= product == "gecko",
                           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/inherited_svg.mako.rs

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

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

@@ -47,7 +47,6 @@ ${helpers.predefined_type("list-style-image",
                           initial_value="specified::ListStyleImage::none()",
                           initial_specified_value="specified::ListStyleImage::none()",
                           animation_value_type="discrete",
-                          boxed=product == "gecko",
                           spec="https://drafts.csswg.org/css-lists/#propdef-list-style-image",
                           servo_restyle_damage="rebuild_and_reflow")}
 

components/style/servo/url.rs

@@ -4,7 +4,8 @@
 
 //! Common handling for the specified value CSS url() values.
 
-use parser::ParserContext;
+use cssparser::Parser;
+use parser::{Parse, ParserContext};
 use servo_url::ServoUrl;
 use std::fmt::{self, Write};
 // Note: We use std::sync::Arc rather than servo_arc::Arc here because the
@@ -12,9 +13,9 @@ use std::fmt::{self, Write};
 // the threshold.
 use std::sync::Arc;
 use style_traits::{CssWriter, ParseError, ToCss};
-use values::computed::{Context, ToComputedValue, ComputedUrl};
+use values::computed::{Context, ToComputedValue};
 
-/// A specified url() value for servo.
+/// A CSS url() value for servo.
 ///
 /// Servo eagerly resolves SpecifiedUrls, which it can then take advantage of
 /// when computing values. In contrast, Gecko uses a different URL backend, so
@@ -23,7 +24,7 @@ use values::computed::{Context, ToComputedValue, ComputedUrl};
 /// However, this approach is still not necessarily optimal: See
 /// <https://bugzilla.mozilla.org/show_bug.cgi?id=1347435#c6>
 #[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize)]
-pub struct SpecifiedUrl {
+pub struct CssUrl {
     /// The original URI. This might be optional since we may insert computed
     /// values of images into the cascade directly, and we don't bother to
     /// convert their serialization.
@@ -37,7 +38,7 @@ pub struct SpecifiedUrl {
     resolved: Option<ServoUrl>,
 }
 
-impl SpecifiedUrl {
+impl CssUrl {
     /// Try to parse a URL from a string value that is a valid CSS token for a
     /// URL. Never fails - the API is only fallible to be compatible with the
     /// gecko version.
@@ -46,7 +47,7 @@ impl SpecifiedUrl {
                                  -> Result<Self, ParseError<'a>> {
         let serialization = Arc::new(url);
         let resolved = context.url_data.join(&serialization).ok();
-        Ok(SpecifiedUrl {
+        Ok(CssUrl {
             original: Some(serialization),
             resolved: resolved,
         })
@@ -87,7 +88,7 @@ impl SpecifiedUrl {
     /// Creates an already specified url value from an already resolved URL
     /// for insertion in the cascade.
     pub fn for_cascade(url: ServoUrl) -> Self {
-        SpecifiedUrl {
+        CssUrl {
             original: None,
             resolved: Some(url),
         }
@@ -95,14 +96,21 @@ impl SpecifiedUrl {
 
     /// Gets a new url from a string for unit tests.
     pub fn new_for_testing(url: &str) -> Self {
-        SpecifiedUrl {
+        CssUrl {
             original: Some(Arc::new(url.into())),
             resolved: ServoUrl::parse(url).ok(),
         }
     }
 }
 
-impl PartialEq for SpecifiedUrl {
+impl Parse for CssUrl {
+    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
+        let url = input.expect_url()?;
+        Self::parse_from_string(url.as_ref().to_owned(), context)
+    }
+}
+
+impl PartialEq for CssUrl {
     fn eq(&self, other: &Self) -> bool {
         // TODO(emilio): maybe we care about equality of the specified values if
         // present? Seems not.
@@ -110,7 +118,9 @@ impl PartialEq for SpecifiedUrl {
     }
 }
 
-impl ToCss for SpecifiedUrl {
+impl Eq for CssUrl {}
+
+impl ToCss for CssUrl {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
@@ -132,6 +142,9 @@ impl ToCss for SpecifiedUrl {
     }
 }
 
+/// A specified url() value for servo.
+pub type SpecifiedUrl = CssUrl;
+
 impl ToComputedValue for SpecifiedUrl {
     type ComputedValue = ComputedUrl;
 
@@ -163,3 +176,43 @@ impl ToComputedValue for SpecifiedUrl {
     }
 }
 
+/// A specified image url() value for servo.
+pub type SpecifiedImageUrl = CssUrl;
+
+/// The computed value of a CSS `url()`, resolved relative to the stylesheet URL.
+#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
+pub enum ComputedUrl {
+    /// The `url()` was invalid or it wasn't specified by the user.
+    Invalid(#[ignore_malloc_size_of = "Arc"] Arc<String>),
+    /// The resolved `url()` relative to the stylesheet URL.
+    Valid(ServoUrl),
+}
+
+impl ComputedUrl {
+    /// Returns the resolved url if it was valid.
+    pub fn url(&self) -> Option<&ServoUrl> {
+        match *self {
+            ComputedUrl::Valid(ref url) => Some(url),
+            _ => None,
+        }
+    }
+}
+
+impl ToCss for ComputedUrl {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: Write,
+    {
+        let string = match *self {
+            ComputedUrl::Valid(ref url) => url.as_str(),
+            ComputedUrl::Invalid(ref invalid_string) => invalid_string,
+        };
+
+        dest.write_str("url(")?;
+        string.to_css(dest)?;
+        dest.write_str(")")
+    }
+}
+
+/// The computed value of a CSS `url()` for image.
+pub type ComputedImageUrl = ComputedUrl;

components/style/stylesheets/document_rule.rs

@@ -17,7 +17,7 @@ use std::fmt::{self, Write};
 use str::CssStringWriter;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 use stylesheets::CssRules;
-use values::specified::url::SpecifiedUrl;
+use values::CssUrl;
 
 #[derive(Debug)]
 /// A @-moz-document rule
@@ -75,7 +75,7 @@ impl DeepCloneWithLock for DocumentRule {
 pub enum UrlMatchingFunction {
     /// Exact URL matching function. It evaluates to true whenever the
     /// URL of the document being styled is exactly the URL given.
-    Url(SpecifiedUrl),
+    Url(CssUrl),
     /// URL prefix matching function. It evaluates to true whenever the
     /// URL of the document being styled has the argument to the
     /// function as an initial substring (which is true when the two
@@ -130,7 +130,7 @@ impl UrlMatchingFunction {
             input.parse_nested_block(|input| {
                 Ok(UrlMatchingFunction::Regexp(input.expect_string()?.as_ref().to_owned()))
             })
-        } else if let Ok(url) = input.try(|input| SpecifiedUrl::parse(context, input)) {
+        } else if let Ok(url) = input.try(|input| CssUrl::parse(context, input)) {
             Ok(UrlMatchingFunction::Url(url))
         } else {
             Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))

components/style/stylesheets/import_rule.rs

@@ -13,7 +13,7 @@ use std::fmt::{self, Write};
 use str::CssStringWriter;
 use style_traits::{CssWriter, ToCss};
 use stylesheets::{StylesheetContents, StylesheetInDocument};
-use values::specified::url::SpecifiedUrl;
+use values::CssUrl;
 
 /// A sheet that is held from an import rule.
 #[cfg(feature = "gecko")]
@@ -80,7 +80,7 @@ impl DeepCloneWithLock for ImportSheet {
 #[derive(Debug)]
 pub struct ImportRule {
     /// The `<url>` this `@import` rule is loading.
-    pub url: SpecifiedUrl,
+    pub url: CssUrl,
 
     /// The stylesheet is always present.
     ///

components/style/stylesheets/loader.rs

@@ -11,7 +11,7 @@ use parser::ParserContext;
 use servo_arc::Arc;
 use shared_lock::{Locked, SharedRwLock};
 use stylesheets::import_rule::ImportRule;
-use values::specified::url::SpecifiedUrl;
+use values::CssUrl;
 
 /// The stylesheet loader is the abstraction used to trigger network requests
 /// for `@import` rules.
@@ -20,7 +20,7 @@ pub trait StylesheetLoader {
     /// the constructed `@import` rule.
     fn request_stylesheet(
         &self,
-        url: SpecifiedUrl,
+        url: CssUrl,
         location: SourceLocation,
         context: &ParserContext,
         lock: &SharedRwLock,

components/style/stylesheets/rule_parser.rs

@@ -28,10 +28,8 @@ use stylesheets::keyframes_rule::parse_keyframe_list;
 use stylesheets::stylesheet::Namespaces;
 use stylesheets::supports_rule::SupportsCondition;
 use stylesheets::viewport_rule;
-use values::CustomIdent;
+use values::{CssUrl, CustomIdent, KeyframesName};
-use values::KeyframesName;
 use values::computed::font::FamilyName;
-use values::specified::url::SpecifiedUrl;
 
 /// The parser for the top-level rules in a stylesheet.
 pub struct TopLevelRuleParser<'a, R: 'a> {
@@ -134,7 +132,7 @@ pub enum AtRuleBlockPrelude {
 /// A rule prelude for at-rule without block.
 pub enum AtRuleNonBlockPrelude {
     /// A @import rule prelude.
-    Import(SpecifiedUrl, Arc<Locked<MediaList>>, SourceLocation),
+    Import(CssUrl, Arc<Locked<MediaList>>, SourceLocation),
     /// A @namespace rule prelude.
     Namespace(Option<Prefix>, Namespace, SourceLocation),
 }
@@ -174,13 +172,13 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a,
                 }
 
                 let url_string = input.expect_url_or_string()?.as_ref().to_owned();
-                let specified_url = SpecifiedUrl::parse_from_string(url_string, &self.context)?;
+                let url = CssUrl::parse_from_string(url_string, &self.context)?;
 
                 let media = parse_media_query_list(&self.context, input,
                                                    self.error_context.error_reporter);
                 let media = Arc::new(self.shared_lock.wrap(media));
 
-                let prelude = AtRuleNonBlockPrelude::Import(specified_url, media, location);
+                let prelude = AtRuleNonBlockPrelude::Import(url, media, location);
                 return Ok(AtRuleType::WithoutBlock(prelude));
             },
             "namespace" => {
@@ -228,12 +226,12 @@ impl<'a, 'i, R: ParseErrorReporter> AtRuleParser<'i> for TopLevelRuleParser<'a,
     #[inline]
     fn rule_without_block(&mut self, prelude: AtRuleNonBlockPrelude) -> CssRule {
         match prelude {
-            AtRuleNonBlockPrelude::Import(specified_url, media, location) => {
+            AtRuleNonBlockPrelude::Import(url, media, location) => {
                 let loader =
                     self.loader.expect("Expected a stylesheet loader for @import");
 
                 let import_rule = loader.request_stylesheet(
-                    specified_url,
+                    url,
                     location,
                     &self.context,
                     &self.shared_lock,

components/style/values/computed/counters.rs

@@ -18,7 +18,7 @@ use values::generics::counters::CounterReset as GenericCounterReset;
 #[cfg(feature = "gecko")]
 use values::specified::Attr;
 #[cfg(feature = "gecko")]
-use values::specified::url::SpecifiedUrl;
+use values::specified::url::SpecifiedImageUrl;
 pub use values::specified::{Content, ContentItem};
 
 /// A computed value for the `counter-increment` property.
@@ -79,8 +79,7 @@ impl Parse for Content {
         let mut content = vec![];
         loop {
             #[cfg(feature = "gecko")] {
-                if let Ok(mut url) = input.try(|i| SpecifiedUrl::parse(_context, i)) {
+                if let Ok(url) = input.try(|i| SpecifiedImageUrl::parse(_context, i)) {
-                    url.build_image_value();
                     content.push(ContentItem::Url(url));
                     continue;
                 }

components/style/values/computed/image.rs

@@ -12,7 +12,8 @@ use std::f32::consts::PI;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ToCss};
 use values::{Either, None_};
-use values::computed::{Angle, ComputedUrl, Context, Length, LengthOrPercentage, NumberOrPercentage, ToComputedValue};
+use values::computed::{Angle, ComputedImageUrl, Context};
+use values::computed::{Length, LengthOrPercentage, NumberOrPercentage, ToComputedValue};
 #[cfg(feature = "gecko")]
 use values::computed::Percentage;
 use values::computed::position::Position;
@@ -28,7 +29,7 @@ pub type ImageLayer = Either<None_, Image>;
 
 /// Computed values for an image according to CSS-IMAGES.
 /// <https://drafts.csswg.org/css-images/#image-values>
-pub type Image = GenericImage<Gradient, MozImageRect, ComputedUrl>;
+pub type Image = GenericImage<Gradient, MozImageRect, ComputedImageUrl>;
 
 /// Computed values for a CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
@@ -76,7 +77,7 @@ pub type GradientItem = GenericGradientItem<RGBA, LengthOrPercentage>;
 pub type ColorStop = GenericColorStop<RGBA, LengthOrPercentage>;
 
 /// Computed values for `-moz-image-rect(...)`.
-pub type MozImageRect = GenericMozImageRect<NumberOrPercentage, ComputedUrl>;
+pub type MozImageRect = GenericMozImageRect<NumberOrPercentage, ComputedImageUrl>;
 
 impl GenericLineDirection for LineDirection {
     fn points_downwards(&self, compat_mode: CompatMode) -> bool {

components/style/values/computed/mod.rs

@@ -15,14 +15,10 @@ use media_queries::Device;
 use properties;
 use properties::{ComputedValues, LonghandId, StyleBuilder};
 use rule_cache::RuleCacheConditions;
-#[cfg(feature = "servo")]
-use servo_url::ServoUrl;
 use std::cell::RefCell;
 use std::cmp;
 use std::f32;
 use std::fmt::{self, Write};
-#[cfg(feature = "servo")]
-use std::sync::Arc;
 use style_traits::{CssWriter, ToCss};
 use style_traits::cursor::CursorKind;
 use super::{CSSFloat, CSSInteger};
@@ -84,6 +80,7 @@ pub use self::time::Time;
 pub use self::transform::{Rotate, Scale, TimingFunction, Transform, TransformOperation};
 pub use self::transform::{TransformOrigin, TransformStyle, Translate};
 pub use self::ui::MozForceBrokenImageIcon;
+pub use self::url::{ComputedUrl, ComputedImageUrl};
 
 #[cfg(feature = "gecko")]
 pub mod align;
@@ -117,6 +114,14 @@ pub mod time;
 pub mod transform;
 pub mod ui;
 
+/// Common handling for the computed value CSS url() values.
+pub mod url {
+#[cfg(feature = "servo")]
+pub use ::servo::url::{ComputedUrl, ComputedImageUrl};
+#[cfg(feature = "gecko")]
+pub use ::gecko::url::{ComputedUrl, ComputedImageUrl};
+}
+
 /// A `Context` is all the data a specified value could ever need to compute
 /// itself and be transformed to a computed value.
 pub struct Context<'a> {
@@ -636,47 +641,8 @@ impl ClipRectOrAuto {
     }
 }
 
-/// The computed value of a CSS `url()`, resolved relative to the stylesheet URL.
-#[cfg(feature = "servo")]
-#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
-pub enum ComputedUrl {
-    /// The `url()` was invalid or it wasn't specified by the user.
-    Invalid(#[ignore_malloc_size_of = "Arc"] Arc<String>),
-    /// The resolved `url()` relative to the stylesheet URL.
-    Valid(ServoUrl),
-}
-
-/// TODO: Properly build ComputedUrl for gecko
-#[cfg(feature = "gecko")]
-pub type ComputedUrl = specified::url::SpecifiedUrl;
-
-#[cfg(feature = "servo")]
-impl ComputedUrl {
-    /// Returns the resolved url if it was valid.
-    pub fn url(&self) -> Option<&ServoUrl> {
-        match *self {
-            ComputedUrl::Valid(ref url) => Some(url),
-            _ => None,
-        }
-    }
-}
-
-#[cfg(feature = "servo")]
-impl ToCss for ComputedUrl {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        let string = match *self {
-            ComputedUrl::Valid(ref url) => url.as_str(),
-            ComputedUrl::Invalid(ref invalid_string) => invalid_string,
-        };
-
-        dest.write_str("url(")?;
-        string.to_css(dest)?;
-        dest.write_str(")")
-    }
-}
-
 /// <url> | <none>
 pub type UrlOrNone = Either<ComputedUrl, None_>;
+
+/// <url> | <none> for image
+pub type ImageUrlOrNone = Either<ComputedImageUrl, None_>;

components/style/values/computed/pointing.rs

@@ -18,7 +18,7 @@ use style_traits::cursor::CursorKind;
 use values::computed::color::Color;
 use values::generics::pointing::CaretColor as GenericCaretColor;
 #[cfg(feature = "gecko")]
-use values::specified::url::SpecifiedUrl;
+use values::specified::url::SpecifiedImageUrl;
 
 /// The computed value for the `cursor` property.
 ///
@@ -65,10 +65,7 @@ impl Parse for Cursor {
         let mut images = vec![];
         loop {
             match input.try(|input| CursorImage::parse_image(context, input)) {
-                Ok(mut image) => {
+                Ok(image) => images.push(image),
-                    image.url.build_image_value();
-                    images.push(image)
-                }
                 Err(_) => break,
             }
             input.expect_comma()?;
@@ -114,7 +111,7 @@ impl CursorImage {
         input: &mut Parser<'i, 't>
     ) -> Result<Self, ParseError<'i>> {
         Ok(Self {
-            url: SpecifiedUrl::parse(context, input)?,
+            url: SpecifiedImageUrl::parse(context, input)?,
             // FIXME(emilio): Should use Number::parse to handle calc() correctly.
             hotspot: match input.try(|input| input.expect_number()) {
                 Ok(number) => Some((number, input.expect_number()?)),

components/style/values/mod.rs

@@ -17,6 +17,11 @@ use std::hash;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 use values::distance::{ComputeSquaredDistance, SquaredDistance};
 
+#[cfg(feature = "servo")]
+pub use servo::url::CssUrl;
+#[cfg(feature = "gecko")]
+pub use gecko::url::CssUrl;
+
 pub mod animated;
 pub mod computed;
 pub mod distance;

components/style/values/specified/counters.rs

@@ -18,7 +18,7 @@ use values::generics::counters::CounterReset as GenericCounterReset;
 use values::specified::Attr;
 use values::specified::Integer;
 #[cfg(feature = "gecko")]
-use values::specified::url::SpecifiedUrl;
+use values::specified::url::SpecifiedImageUrl;
 
 /// A specified value for the `counter-increment` property.
 pub type CounterIncrement = GenericCounterIncrement<Integer>;
@@ -119,5 +119,5 @@ pub enum ContentItem {
     Attr(Attr),
     /// `url(url)`
     #[cfg(feature = "gecko")]
-    Url(SpecifiedUrl),
+    Url(SpecifiedImageUrl),
 }

components/style/values/specified/image.rs

@@ -31,14 +31,14 @@ use values::generics::position::Position as GenericPosition;
 use values::specified::{Angle, Color, Length, LengthOrPercentage};
 use values::specified::{Number, NumberOrPercentage, Percentage, RGBAColor};
 use values::specified::position::{LegacyPosition, Position, PositionComponent, Side, X, Y};
-use values::specified::url::SpecifiedUrl;
+use values::specified::url::SpecifiedImageUrl;
 
 /// A specified image layer.
 pub type ImageLayer = Either<None_, Image>;
 
 /// Specified values for an image according to CSS-IMAGES.
 /// <https://drafts.csswg.org/css-images/#image-values>
-pub type Image = GenericImage<Gradient, MozImageRect, SpecifiedUrl>;
+pub type Image = GenericImage<Gradient, MozImageRect, SpecifiedImageUrl>;
 
 /// Specified values for a CSS gradient.
 /// <https://drafts.csswg.org/css-images/#gradients>
@@ -124,16 +124,11 @@ pub type ColorStop = GenericColorStop<RGBAColor, LengthOrPercentage>;
 
 /// Specified values for `moz-image-rect`
 /// -moz-image-rect(<uri>, top, right, bottom, left);
-pub type MozImageRect = GenericMozImageRect<NumberOrPercentage, SpecifiedUrl>;
+pub type MozImageRect = GenericMozImageRect<NumberOrPercentage, SpecifiedImageUrl>;
 
 impl Parse for Image {
-    #[cfg_attr(not(feature = "gecko"), allow(unused_mut))]
     fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Image, ParseError<'i>> {
-        if let Ok(mut url) = input.try(|input| SpecifiedUrl::parse(context, input)) {
+        if let Ok(url) = input.try(|input| SpecifiedImageUrl::parse(context, input)) {
-            #[cfg(feature = "gecko")]
-            {
-                url.build_image_value();
-            }
             return Ok(GenericImage::Url(url));
         }
         if let Ok(gradient) = input.try(|i| Gradient::parse(context, i)) {
@@ -145,11 +140,7 @@ impl Parse for Image {
                 return Ok(GenericImage::PaintWorklet(paint_worklet));
             }
         }
-        if let Ok(mut image_rect) = input.try(|input| MozImageRect::parse(context, input)) {
+        if let Ok(image_rect) = input.try(|input| MozImageRect::parse(context, input)) {
-            #[cfg(feature = "gecko")]
-            {
-                image_rect.url.build_image_value();
-            }
             return Ok(GenericImage::Rect(Box::new(image_rect)));
         }
         Ok(GenericImage::Element(Image::parse_element(input)?))
@@ -161,7 +152,8 @@ impl Image {
     /// for insertion in the cascade.
     #[cfg(feature = "servo")]
     pub fn for_cascade(url: ServoUrl) -> Self {
-        GenericImage::Url(SpecifiedUrl::for_cascade(url))
+        use values::CssUrl;
+        GenericImage::Url(CssUrl::for_cascade(url))
     }
 
     /// Parses a `-moz-element(# <element-id>)`.
@@ -944,7 +936,7 @@ impl Parse for MozImageRect {
         input.try(|i| i.expect_function_matching("-moz-image-rect"))?;
         input.parse_nested_block(|i| {
             let string = i.expect_url_or_string()?;
-            let url = SpecifiedUrl::parse_from_string(string.as_ref().to_owned(), context)?;
+            let url = SpecifiedImageUrl::parse_from_string(string.as_ref().to_owned(), context)?;
             i.expect_comma()?;
             let top = NumberOrPercentage::parse_non_negative(context, i)?;
             i.expect_comma()?;
@@ -953,14 +945,7 @@ impl Parse for MozImageRect {
             let bottom = NumberOrPercentage::parse_non_negative(context, i)?;
             i.expect_comma()?;
             let left = NumberOrPercentage::parse_non_negative(context, i)?;
-
+            Ok(MozImageRect { url, top, right, bottom, left })
-            Ok(MozImageRect {
-                url: url,
-                top: top,
-                right: right,
-                bottom: bottom,
-                left: left,
-            })
         })
     }
 }

components/style/values/specified/list.rs

@@ -13,7 +13,7 @@ use values::{Either, None_};
 use values::CustomIdent;
 #[cfg(feature = "gecko")]
 use values::generics::CounterStyleOrNone;
-use values::specified::UrlOrNone;
+use values::specified::ImageUrlOrNone;
 
 /// Specified and computed `list-style-type` property.
 #[cfg(feature = "gecko")]
@@ -75,7 +75,7 @@ impl Parse for ListStyleType {
 
 /// Specified and computed `list-style-image` property.
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToCss)]
-pub struct ListStyleImage(pub UrlOrNone);
+pub struct ListStyleImage(pub ImageUrlOrNone);
 
 // FIXME(nox): This is wrong, there are different types for specified
 // and computed URLs in Servo.
@@ -90,19 +90,11 @@ impl ListStyleImage {
 }
 
 impl Parse for ListStyleImage {
-    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>)
+    fn parse<'i, 't>(
-                         -> Result<ListStyleImage, ParseError<'i>> {
+        context: &ParserContext,
-        #[allow(unused_mut)]
+        input: &mut Parser<'i, 't>,
-        let mut value = input.try(|input| UrlOrNone::parse(context, input))?;
+    ) -> Result<ListStyleImage, ParseError<'i>> {
-
+        ImageUrlOrNone::parse(context, input).map(ListStyleImage)
-        #[cfg(feature = "gecko")]
-        {
-            if let Either::First(ref mut url) = value {
-                url.build_image_value();
-            }
-        }
-
-        return Ok(ListStyleImage(value));
     }
 }
 

components/style/values/specified/mod.rs

@@ -11,7 +11,7 @@ use context::QuirksMode;
 use cssparser::{Parser, Token, serialize_identifier};
 use num_traits::One;
 use parser::{ParserContext, Parse};
-use self::url::SpecifiedUrl;
+use self::url::{SpecifiedImageUrl, SpecifiedUrl};
 use std::f32;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
@@ -116,23 +116,10 @@ pub mod ui;
 
 /// Common handling for the specified value CSS url() values.
 pub mod url {
-use cssparser::Parser;
-use parser::{Parse, ParserContext};
-use style_traits::ParseError;
-
 #[cfg(feature = "servo")]
-pub use ::servo::url::*;
+pub use ::servo::url::{SpecifiedUrl, SpecifiedImageUrl};
 #[cfg(feature = "gecko")]
-pub use ::gecko::url::*;
+pub use ::gecko::url::{SpecifiedUrl, SpecifiedImageUrl};
-
-impl Parse for SpecifiedUrl {
-    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
-        let url = input.expect_url()?;
-        Self::parse_from_string(url.as_ref().to_owned(), context)
-    }
-}
-
-impl Eq for SpecifiedUrl {}
 }
 
 /// Parse a `<number>` value, with a given clamping mode.
@@ -534,6 +521,9 @@ impl Parse for PositiveInteger {
 #[allow(missing_docs)]
 pub type UrlOrNone = Either<SpecifiedUrl, None_>;
 
+/// The specified value of a `<url>` for image or `none`.
+pub type ImageUrlOrNone = Either<SpecifiedImageUrl, None_>;
+
 /// The specified value of a grid `<track-breadth>`
 pub type TrackBreadth = GenericTrackBreadth<LengthOrPercentage>;
 

components/style/values/specified/pointing.rs

@@ -13,7 +13,7 @@ use style_traits::cursor::CursorKind;
 use values::generics::pointing::CaretColor as GenericCaretColor;
 use values::specified::color::Color;
 #[cfg(feature = "gecko")]
-use values::specified::url::SpecifiedUrl;
+use values::specified::url::SpecifiedImageUrl;
 
 /// The specified value for the `cursor` property.
 ///
@@ -39,7 +39,7 @@ pub struct Cursor {
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue)]
 pub struct CursorImage {
     /// The url to parse images from.
-    pub url: SpecifiedUrl,
+    pub url: SpecifiedImageUrl,
     /// The <x> and <y> coordinates.
     pub hotspot: Option<(f32, f32)>,
 }

ports/geckolib/glue.rs

@@ -3544,7 +3544,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetBackgroundImage(
     use style::properties::longhands::background_image::SpecifiedValue as BackgroundImage;
     use style::values::Either;
     use style::values::generics::image::Image;
-    use style::values::specified::url::SpecifiedUrl;
+    use style::values::specified::url::SpecifiedImageUrl;
 
     let url_data = unsafe { RefPtr::from_ptr_ref(&raw_extra_data) };
     let string = unsafe { (*value).to_string() };
@@ -3555,8 +3555,7 @@ pub extern "C" fn Servo_DeclarationBlock_SetBackgroundImage(
         ParsingMode::DEFAULT,
         QuirksMode::NoQuirks,
     );
-    if let Ok(mut url) = SpecifiedUrl::parse_from_string(string.into(), &context) {
+    if let Ok(url) = SpecifiedImageUrl::parse_from_string(string.into(), &context) {
-        url.build_image_value();
         let decl = PropertyDeclaration::BackgroundImage(BackgroundImage(
             vec![Either::Second(Image::Url(url))]
         ));

ports/geckolib/stylesheet_loader.rs

@@ -13,7 +13,7 @@ use style::parser::ParserContext;
 use style::shared_lock::{Locked, SharedRwLock};
 use style::stylesheets::{ImportRule, StylesheetLoader as StyleStylesheetLoader};
 use style::stylesheets::import_rule::ImportSheet;
-use style::values::specified::url::SpecifiedUrl;
+use style::values::CssUrl;
 
 pub struct StylesheetLoader(*mut Loader, *mut ServoStyleSheet, *mut SheetLoadData, *mut LoaderReusableStyleSheets);
 
@@ -29,7 +29,7 @@ impl StylesheetLoader {
 impl StyleStylesheetLoader for StylesheetLoader {
     fn request_stylesheet(
         &self,
-        url: SpecifiedUrl,
+        url: CssUrl,
         source_location: SourceLocation,
         _context: &ParserContext,
         lock: &SharedRwLock,

ports/geckolib/tests/size_of.rs

@@ -42,10 +42,10 @@ size_of_test!(test_size_of_rule_node, RuleNode, 80);
 // we only pass `&mut SourcePropertyDeclaration` references around.
 size_of_test!(test_size_of_parsed_declaration, style::properties::SourcePropertyDeclaration, 608);
 
-size_of_test!(test_size_of_computed_image, computed::image::Image, 40);
+size_of_test!(test_size_of_computed_image, computed::image::Image, 32);
-size_of_test!(test_size_of_specified_image, specified::image::Image, 40);
+size_of_test!(test_size_of_specified_image, specified::image::Image, 32);
 
 // 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, 40);
+size_of_test!(test_size_of_computed_image_layer, computed::image::ImageLayer, 32);
-size_of_test!(test_size_of_specified_image_layer, specified::image::ImageLayer, 40);
+size_of_test!(test_size_of_specified_image_layer, specified::image::ImageLayer, 32);