diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 356636fc6bb..e36ff141eae 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -123,14 +123,11 @@ impl nsStyleImage { match image { GenericImage::Gradient(boxed_gradient) => self.set_gradient(*boxed_gradient), GenericImage::Url(ref url) => unsafe { - bindings::Gecko_SetLayerImageImageValue(self, (url.0).0.url_value.get()); + bindings::Gecko_SetLayerImageImageValue(self, url.url_value_ptr()) }, GenericImage::Rect(ref image_rect) => { unsafe { - bindings::Gecko_SetLayerImageImageValue( - self, - (image_rect.url.0).0.url_value.get(), - ); + bindings::Gecko_SetLayerImageImageValue(self, image_rect.url.url_value_ptr()); bindings::Gecko_InitializeImageCropRect(self); // Set CropRect diff --git a/components/style/gecko/url.rs b/components/style/gecko/url.rs index df5f1137532..e5f1b32e04c 100644 --- a/components/style/gecko/url.rs +++ b/components/style/gecko/url.rs @@ -17,7 +17,9 @@ use cssparser::Parser; use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; use nsstring::nsCString; use servo_arc::Arc; +use std::collections::HashMap; use std::fmt::{self, Write}; +use std::sync::RwLock; use style_traits::{CssWriter, ParseError, ToCss}; /// A CSS url() value for gecko. @@ -81,6 +83,20 @@ impl CssUrlData { } } +#[cfg(debug_assertions)] +impl Drop for CssUrlData { + fn drop(&mut self) { + assert!( + !URL_VALUE_TABLE + .read() + .unwrap() + .contains_key(&CssUrlDataKey(self as *mut _ as *const _)), + "All CssUrlData objects used as keys in URL_VALUE_TABLE should be \ + from shared memory style sheets, and so should never be dropped", + ); + } +} + impl Parse for CssUrl { fn parse<'i, 't>( context: &ParserContext, @@ -104,6 +120,24 @@ impl MallocSizeOf for CssUrl { } } +/// A key type for URL_VALUE_TABLE. +#[derive(Hash, PartialEq, Eq)] +struct CssUrlDataKey(*const CssUrlData); + +unsafe impl Sync for CssUrlDataKey {} +unsafe impl Send for CssUrlDataKey {} + +/// The source of a Gecko URLValue object for a SpecifiedUrl. +#[derive(Clone, Debug)] +pub enum URLValueSource { + /// A strong reference to a Gecko URLValue object. + URLValue(RefPtr), + /// A CORSMode value used to lazily construct a Gecko URLValue object. + /// + /// The lazily created object will be stored in URL_VALUE_TABLE. + CORSMode(CORSMode), +} + /// A specified non-image `url()` value. #[derive(Clone, Debug, SpecifiedValueInfo, ToCss)] pub struct SpecifiedUrl { @@ -111,8 +145,20 @@ pub struct SpecifiedUrl { pub url: CssUrl, /// Gecko's URLValue so that we can reuse it while rematching a /// property with this specified value. + /// + /// Box this to avoid SpecifiedUrl getting bigger than two words, + /// and increasing the size of PropertyDeclaration. #[css(skip)] - pub url_value: RefPtr, + url_value: Box, +} + +fn make_url_value(url: &CssUrl, cors_mode: CORSMode) -> RefPtr { + unsafe { + let ptr = bindings::Gecko_URLValue_Create(url.0.clone().into_strong(), cors_mode); + // We do not expect Gecko_URLValue_Create returns null. + debug_assert!(!ptr.is_null()); + RefPtr::from_addrefed(ptr) + } } impl SpecifiedUrl { @@ -122,12 +168,7 @@ impl SpecifiedUrl { } fn from_css_url_with_cors(url: CssUrl, cors: CORSMode) -> Self { - let url_value = unsafe { - let ptr = bindings::Gecko_URLValue_Create(url.0.clone().into_strong(), cors); - // We do not expect Gecko_URLValue_Create returns null. - debug_assert!(!ptr.is_null()); - RefPtr::from_addrefed(ptr) - }; + let url_value = Box::new(URLValueSource::URLValue(make_url_value(&url, cors))); Self { url, url_value } } @@ -140,6 +181,45 @@ impl SpecifiedUrl { use crate::gecko_bindings::structs::root::mozilla::CORSMode_CORS_ANONYMOUS; Self::from_css_url_with_cors(url, CORSMode_CORS_ANONYMOUS) } + + fn with_url_value(&self, f: F) -> T + where + F: FnOnce(&RefPtr) -> T, + { + match *self.url_value { + URLValueSource::URLValue(ref r) => f(r), + URLValueSource::CORSMode(cors_mode) => { + { + let guard = URL_VALUE_TABLE.read().unwrap(); + if let Some(r) = guard.get(&(CssUrlDataKey(&*self.url.0 as *const _))) { + return f(r); + } + } + let mut guard = URL_VALUE_TABLE.write().unwrap(); + let r = guard + .entry(CssUrlDataKey(&*self.url.0 as *const _)) + .or_insert_with(|| make_url_value(&self.url, cors_mode)); + f(r) + }, + } + } + + /// Clone a new, strong reference to the Gecko URLValue. + pub fn clone_url_value(&self) -> RefPtr { + self.with_url_value(RefPtr::clone) + } + + /// Get a raw pointer to the URLValue held by this SpecifiedUrl, for FFI. + pub fn url_value_ptr(&self) -> *mut URLValue { + self.with_url_value(RefPtr::get) + } +} + +/// Clears URL_VALUE_TABLE. Entries in this table, which are for specified URL +/// values that come from shared memory style sheets, would otherwise persist +/// until the end of the process and be reported as leaks. +pub fn shutdown() { + URL_VALUE_TABLE.write().unwrap().clear(); } impl Parse for SpecifiedUrl { @@ -165,7 +245,7 @@ impl MallocSizeOf for SpecifiedUrl { // 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 += unsafe { bindings::Gecko_URLValue_SizeOfIncludingThis(self.url_value_ptr()) }; n } } @@ -258,7 +338,8 @@ impl ToCss for ComputedUrl { where W: Write, { - serialize_computed_url(&self.0.url_value, dest, bindings::Gecko_GetComputedURLSpec) + self.0 + .with_url_value(|r| serialize_computed_url(r, dest, bindings::Gecko_GetComputedURLSpec)) } } @@ -267,12 +348,20 @@ impl ComputedUrl { pub unsafe fn from_url_value(url_value: RefPtr) -> Self { let css_url = &*url_value.mCssUrl.mRawPtr; let url = CssUrl(CssUrlData::as_arc(&css_url).clone_arc()); - ComputedUrl(SpecifiedUrl { url, url_value }) + ComputedUrl(SpecifiedUrl { + url, + url_value: Box::new(URLValueSource::URLValue(url_value)), + }) + } + + /// Clone a new, strong reference to the Gecko URLValue. + pub fn clone_url_value(&self) -> RefPtr { + self.0.clone_url_value() } /// Get a raw pointer to the URLValue held by this ComputedUrl, for FFI. pub fn url_value_ptr(&self) -> *mut URLValue { - self.0.url_value.get() + self.0.url_value_ptr() } } @@ -285,11 +374,9 @@ impl ToCss for ComputedImageUrl { where W: Write, { - serialize_computed_url( - &(self.0).0.url_value, - dest, - bindings::Gecko_GetComputedImageURLSpec, - ) + (self.0).0.with_url_value(|r| { + serialize_computed_url(r, dest, bindings::Gecko_GetComputedImageURLSpec) + }) } } @@ -299,11 +386,27 @@ impl ComputedImageUrl { let url_value = image_request.mImageValue.to_safe(); let css_url = &*url_value.mCssUrl.mRawPtr; let url = CssUrl(CssUrlData::as_arc(&css_url).clone_arc()); - ComputedImageUrl(SpecifiedImageUrl(SpecifiedUrl { url, url_value })) + ComputedImageUrl(SpecifiedImageUrl(SpecifiedUrl { + url, + url_value: Box::new(URLValueSource::URLValue(url_value)), + })) + } + + /// Clone a new, strong reference to the Gecko URLValue. + pub fn clone_url_value(&self) -> RefPtr { + (self.0).0.clone_url_value() } /// Get a raw pointer to the URLValue held by this ComputedImageUrl, for FFI. pub fn url_value_ptr(&self) -> *mut URLValue { - (self.0).0.url_value.get() + (self.0).0.url_value_ptr() } } + +lazy_static! { + /// A table mapping CssUrlData objects to their lazily created Gecko + /// URLValue objects. + static ref URL_VALUE_TABLE: RwLock>> = { + Default::default() + }; +} diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 5af9f5be7c6..17f10cd4552 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -826,7 +826,7 @@ def set_gecko_property(ffi_name, expr): pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { match v { UrlOrNone::Url(ref url) => { - self.gecko.${gecko_ffi_name}.set_move(url.0.url_value.clone()) + self.gecko.${gecko_ffi_name}.set_move(url.clone_url_value()) } UrlOrNone::None => { unsafe { @@ -3783,7 +3783,7 @@ fn static_assert() { }, Url(ref url) => { unsafe { - bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, url.0.url_value.get()); + bindings::Gecko_nsStyleFilter_SetURLValue(gecko_filter, url.url_value_ptr()); } }, } @@ -4164,7 +4164,7 @@ fn set_style_svg_path( % if ident == "clip_path": ShapeSource::ImageOrUrl(ref url) => { unsafe { - bindings::Gecko_StyleShapeSource_SetURLValue(${ident}, url.0.url_value.get()) + bindings::Gecko_StyleShapeSource_SetURLValue(${ident}, url.url_value_ptr()) } } % elif ident == "shape_outside":