From 77d7490d59948865c1fce492892794e3a0ca746e Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sun, 12 Mar 2017 21:08:29 -0700 Subject: [PATCH] stylo: Use ServoBundledURI everywhere else, fix from_ffi to handle the error case MozReview-Commit-ID: DHNKLm3y5Gv --- components/style/gecko/conversions.rs | 27 ++++------- components/style/gecko_bindings/bindings.rs | 10 +--- components/style/properties/gecko.mako.rs | 52 ++++++++------------- components/style/values/specified/url.rs | 14 ++++-- 4 files changed, 41 insertions(+), 62 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index a88d7c66d95..6b42c1e04bf 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -106,26 +106,17 @@ impl nsStyleImage { self.set_gradient(gradient) }, Image::Url(ref url) if with_url => { - let (ptr, len) = match url.as_slice_components() { - Ok(value) | Err(value) => value - }; - let extra_data = url.extra_data(); unsafe { - Gecko_SetUrlImageValue(self, - ptr, - len as u32, - extra_data.base.get(), - extra_data.referrer.get(), - extra_data.principal.get()); + Gecko_SetUrlImageValue(self, url.for_ffi()); + // 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 + // structs don't like to be tracked by more than one document. + // + // FIXME(emilio): With the scoped TLS thing this is no longer + // true, remove this line in a follow-up! + *cacheable = false; } - // 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 - // structs don't like to be tracked by more than one document. - // - // FIXME(emilio): With the scoped TLS thing this is no longer - // true, remove this line in a follow-up! - *cacheable = false; }, _ => (), } diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs index 0722a861ae6..cced8859505 100644 --- a/components/style/gecko_bindings/bindings.rs +++ b/components/style/gecko_bindings/bindings.rs @@ -631,10 +631,7 @@ extern "C" { } extern "C" { pub fn Gecko_SetUrlImageValue(image: *mut nsStyleImage, - url_bytes: *const u8, url_length: u32, - base_uri: *mut ThreadSafeURIHolder, - referrer: *mut ThreadSafeURIHolder, - principal: *mut ThreadSafePrincipalHolder); + uri: ServoBundledURI); } extern "C" { pub fn Gecko_CopyImageValueFrom(image: *mut nsStyleImage, @@ -662,10 +659,7 @@ extern "C" { } extern "C" { pub fn Gecko_SetCursorImage(cursor: *mut nsCursorImage, - string_bytes: *const u8, string_length: u32, - base_uri: *mut ThreadSafeURIHolder, - referrer: *mut ThreadSafeURIHolder, - principal: *mut ThreadSafePrincipalHolder); + uri: ServoBundledURI); } extern "C" { pub fn Gecko_CopyCursorArrayFrom(dest: *mut nsStyleUserInterface, diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 691f5cb5641..ddda61a9dc1 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -394,11 +394,7 @@ fn color_to_nscolor_zero_currentcolor(color: Color) -> structs::nscolor { } SVGPaintKind::PaintServer(url) => { unsafe { - if let Some(ffi) = url.for_ffi() { - bindings::Gecko_nsStyleSVGPaint_SetURLValue(paint, ffi); - } else { - return; - } + bindings::Gecko_nsStyleSVGPaint_SetURLValue(paint, url.for_ffi()); } } SVGPaintKind::Color(color) => { @@ -511,20 +507,26 @@ fn color_to_nscolor_zero_currentcolor(color: Color) -> structs::nscolor { % endif -<%def name="impl_css_url(ident, gecko_ffi_name, need_clone=False)"> +<%def name="impl_css_url(ident, gecko_ffi_name, need_clone=False, only_resolved=False)"> #[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 { - if let Some(ffi) = url.for_ffi() { - let ptr = bindings::Gecko_NewURLValue(ffi); - RefPtr::from_addrefed(ptr) - } else { + % if only_resolved: + // -moz-binding can't handle relative URIs + if !url.has_resolved() { + self.gecko.${gecko_ffi_name}.clear(); + return; + } + % endif + 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) } @@ -1423,7 +1425,7 @@ fn static_assert() { page-break-before page-break-after scroll-snap-points-x scroll-snap-points-y transform scroll-snap-type-y scroll-snap-coordinate - perspective-origin transform-origin""" %> + perspective-origin transform-origin -moz-binding""" %> <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> // We manually-implement the |display| property until we get general @@ -1611,6 +1613,8 @@ fn static_assert() { longhands::scroll_snap_coordinate::computed_value::T(vec) } + ${impl_css_url('_moz_binding', 'mBinding', only_resolved=True)} + <%def name="transform_function_arm(name, keyword, items)"> <% pattern = None @@ -2281,12 +2285,8 @@ fn static_assert() { } Either::First(ref url) => { unsafe { - if let Some(ffi) = url.for_ffi() { - Gecko_SetListStyleImage(&mut self.gecko, - ffi); - } else { - Gecko_SetListStyleImageNone(&mut self.gecko); - } + Gecko_SetListStyleImage(&mut self.gecko, + url.for_ffi()); } // 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 @@ -2575,9 +2575,7 @@ fn static_assert() { } Url(ref url) => { unsafe { - if let Some(ffi) = url.for_ffi() { - bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, ffi); - } + bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, url.for_ffi()); } } } @@ -2940,9 +2938,7 @@ clip-path match v { ShapeSource::Url(ref url) => { unsafe { - if let Some(ffi) = url.for_ffi() { - bindings::Gecko_StyleClipPath_SetURLValue(clip_path, ffi); - } + bindings::Gecko_StyleClipPath_SetURLValue(clip_path, url.for_ffi()); } } ShapeSource::None => {} // don't change the type @@ -3150,16 +3146,8 @@ clip-path } for i in 0..v.images.len() { let image = &v.images[i]; - let extra_data = image.url.extra_data(); - let (ptr, len) = match image.url.as_slice_components() { - Ok(value) | Err(value) => value, - }; unsafe { - Gecko_SetCursorImage(&mut self.gecko.mCursorImages[i], - ptr, len as u32, - extra_data.base.get(), - extra_data.referrer.get(), - extra_data.principal.get()); + Gecko_SetCursorImage(&mut self.gecko.mCursorImages[i], image.url.for_ffi()); } // 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 diff --git a/components/style/values/specified/url.rs b/components/style/values/specified/url.rs index f6b8dd2de29..450330dc631 100644 --- a/components/style/values/specified/url.rs +++ b/components/style/values/specified/url.rs @@ -150,6 +150,11 @@ impl SpecifiedUrl { } } + /// Check if it has a resolved URI + pub fn has_resolved(&self) -> bool { + self.resolved.is_some() + } + /// Creates an already specified url value from an already resolved URL /// for insertion in the cascade. pub fn for_cascade(url: ServoUrl, extra_data: UrlExtraData) -> Self { @@ -173,19 +178,20 @@ impl SpecifiedUrl { /// Create a bundled URI suitable for sending to Gecko /// to be constructed into a css::URLValue #[cfg(feature = "gecko")] - pub fn for_ffi(&self) -> Option { + pub fn for_ffi(&self) -> ServoBundledURI { let extra_data = self.extra_data(); let (ptr, len) = match self.as_slice_components() { Ok(value) => value, - Err(_) => return None, + // we're okay with passing down the unresolved relative URI + Err(value) => value, }; - Some(ServoBundledURI { + ServoBundledURI { mURLString: ptr, mURLStringLength: len as u32, mBaseURI: extra_data.base.get(), mReferrer: extra_data.referrer.get(), mPrincipal: extra_data.principal.get(), - }) + } } }