From 97382a2c415c6526491a1b13cb0ad4c69d3fa0c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 7 Feb 2020 20:36:34 +0000 Subject: [PATCH] style: Remove nsStyleImageRequest. This removes nsStyleImageRequest by moving the load state to LoadData instead (where other lazy state like the resolved URL and load id lives). That way we can use cbindgen for more stuff (there's no blocker for using it for all images now), and we can undo the image tracking shenanigans that I had to do in bug 1605803 in nsImageFrame. This removes the mDocGroup member because well, there's no real upside of that now that quantum DOM is not a thing. It also removes the static clones of the image requests, and the need for each computed value instance to have its own request. These were needed because we needed the image loader for the particular document to observe the image changes. But we were also tracking the request -> loader for other purposes. Instead, Now all the images get loaded with GlobalImageObserver as a listener, which looks in the image map and forwards the notification to all the interested loaders instead dynamically. The style value is only responsible to load the image, and no longer tracks / locks it. Instead, the loader does so, via the image tracker. Differential Revision: https://phabricator.services.mozilla.com/D58519 --- components/style/gecko/conversions.rs | 10 +--- components/style/gecko/url.rs | 54 +++++++++++++--------- components/style/properties/gecko.mako.rs | 56 ++--------------------- 3 files changed, 38 insertions(+), 82 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 9bc9573bf91..3deaeacadc8 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -15,7 +15,6 @@ use crate::gecko_bindings::structs::{self, Matrix4x4Components}; use crate::gecko_bindings::structs::{nsStyleImage, nsresult}; use crate::stylesheets::RulesMutateError; use crate::values::computed::transform::Matrix3D; -use crate::values::computed::url::ComputedImageUrl; use crate::values::computed::{Gradient, Image, TextAlign}; use crate::values::generics::image::GenericImage; use crate::values::generics::rect::Rect; @@ -63,7 +62,7 @@ impl nsStyleImage { match self.mType { nsStyleImageType::eStyleImageType_Null => None, nsStyleImageType::eStyleImageType_Image => { - let url = self.get_image_url(); + let url = self.__bindgen_anon_1.mImage.as_ref().clone(); if self.mCropRect.mPtr.is_null() { Some(GenericImage::Url(url)) } else { @@ -88,13 +87,6 @@ impl nsStyleImage { }, } } - - unsafe fn get_image_url(&self) -> ComputedImageUrl { - let image_request = bindings::Gecko_GetImageRequest(self) - .as_ref() - .expect("Null image request?"); - ComputedImageUrl::from_image_request(image_request) - } } pub mod basic_shape { diff --git a/components/style/gecko/url.rs b/components/style/gecko/url.rs index 3a53f639afe..27f3c60884c 100644 --- a/components/style/gecko/url.rs +++ b/components/style/gecko/url.rs @@ -6,8 +6,6 @@ use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs; -use crate::gecko_bindings::structs::nsStyleImageRequest; -use crate::gecko_bindings::sugar::refptr::RefPtr; use crate::parser::{Parse, ParserContext}; use crate::stylesheets::{CorsMode, UrlExtraData}; use crate::values::computed::{Context, ToComputedValue}; @@ -150,31 +148,52 @@ struct LoadDataKey(*const LoadDataSource); unsafe impl Sync for LoadDataKey {} unsafe impl Send for LoadDataKey {} -/// The load data for a given URL. This is mutable from C++, for now at least. +bitflags! { + /// Various bits of mutable state that are kept for image loads. + #[repr(C)] + pub struct LoadDataFlags: u8 { + /// Whether we tried to resolve the uri at least once. + const TRIED_TO_RESOLVE_URI = 1 << 0; + /// Whether we tried to resolve the image at least once. + const TRIED_TO_RESOLVE_IMAGE = 1 << 1; + } +} + +/// This is usable and movable from multiple threads just fine, as long as it's +/// not cloned (it is not clonable), and the methods that mutate it run only on +/// the main thread (when all the other threads we care about are paused). +unsafe impl Sync for LoadData {} +unsafe impl Send for LoadData {} + +/// The load data for a given URL. This is mutable from C++, and shouldn't be +/// accessed from rust for anything. #[repr(C)] #[derive(Debug)] pub struct LoadData { - resolved: RefPtr, - load_id: u64, - tried_to_resolve: bool, + /// A strong reference to the imgRequestProxy, if any, that should be + /// released on drop. + /// + /// These are raw pointers because they are not safe to reference-count off + /// the main thread. + resolved_image: *mut structs::imgRequestProxy, + /// A strong reference to the resolved URI of this image. + resolved_uri: *mut structs::nsIURI, + /// A few flags that are set when resolving the image or such. + flags: LoadDataFlags, } impl Drop for LoadData { fn drop(&mut self) { - if self.load_id != 0 { - unsafe { - bindings::Gecko_LoadData_DeregisterLoad(self); - } - } + unsafe { bindings::Gecko_LoadData_Drop(self) } } } impl Default for LoadData { fn default() -> Self { Self { - resolved: RefPtr::null(), - load_id: 0, - tried_to_resolve: false, + resolved_image: std::ptr::null_mut(), + resolved_uri: std::ptr::null_mut(), + flags: LoadDataFlags::empty(), } } } @@ -342,13 +361,6 @@ impl ToCss for ComputedUrl { #[repr(transparent)] pub struct ComputedImageUrl(pub ComputedUrl); -impl ComputedImageUrl { - /// Convert from nsStyleImageRequest to ComputedImageUrl. - pub unsafe fn from_image_request(image_request: &nsStyleImageRequest) -> Self { - image_request.mImageURL.clone() - } -} - impl ToCss for ComputedImageUrl { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index ba9c4975851..6cfb4c859b4 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -25,14 +25,9 @@ use crate::gecko_bindings::bindings::Gecko_CopyCounterStyle; use crate::gecko_bindings::bindings::Gecko_CopyCursorArrayFrom; use crate::gecko_bindings::bindings::Gecko_CopyFontFamilyFrom; use crate::gecko_bindings::bindings::Gecko_CopyImageValueFrom; -use crate::gecko_bindings::bindings::Gecko_CopyListStyleImageFrom; use crate::gecko_bindings::bindings::Gecko_EnsureImageLayersLength; -use crate::gecko_bindings::bindings::Gecko_SetCursorArrayLength; -use crate::gecko_bindings::bindings::Gecko_SetCursorImageValue; use crate::gecko_bindings::bindings::Gecko_nsStyleFont_SetLang; use crate::gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom; -use crate::gecko_bindings::bindings::Gecko_SetListStyleImageNone; -use crate::gecko_bindings::bindings::Gecko_SetListStyleImageImageValue; use crate::gecko_bindings::bindings::Gecko_SetNullImageValue; use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsCSSPropertyID; @@ -53,7 +48,6 @@ use crate::values::computed::BorderStyle; use crate::values::computed::font::FontSize; use crate::values::generics::column::ColumnCount; use crate::values::generics::image::ImageLayer; -use crate::values::generics::url::UrlOrNone; pub mod style_structs { @@ -2108,43 +2102,7 @@ fn static_assert() { <% impl_simple_image_array_property("blend_mode", "background", "mImage", "mBlendMode", "Background") %> -<%self:impl_trait style_struct_name="List" - skip_longhands="list-style-image list-style-type"> - - pub fn set_list_style_image(&mut self, image: longhands::list_style_image::computed_value::T) { - match image { - UrlOrNone::None => { - unsafe { - Gecko_SetListStyleImageNone(&mut *self.gecko); - } - } - UrlOrNone::Url(ref url) => { - unsafe { - Gecko_SetListStyleImageImageValue(&mut *self.gecko, url); - } - } - } - } - - pub fn copy_list_style_image_from(&mut self, other: &Self) { - unsafe { Gecko_CopyListStyleImageFrom(&mut *self.gecko, &*other.gecko); } - } - - pub fn reset_list_style_image(&mut self, other: &Self) { - self.copy_list_style_image_from(other) - } - - pub fn clone_list_style_image(&self) -> longhands::list_style_image::computed_value::T { - if self.gecko.mListStyleImage.mRawPtr.is_null() { - return UrlOrNone::None; - } - - unsafe { - let ref gecko_image_request = *self.gecko.mListStyleImage.mRawPtr; - UrlOrNone::Url(ComputedImageUrl::from_image_request(gecko_image_request)) - } - } - +<%self:impl_trait style_struct_name="List" skip_longhands="list-style-type"> pub fn set_list_style_type(&mut self, v: longhands::list_style_type::computed_value::T) { use nsstring::{nsACString, nsCStr}; use self::longhands::list_style_type::computed_value::T; @@ -2433,14 +2391,11 @@ clip-path pub fn set_cursor(&mut self, v: longhands::cursor::computed_value::T) { self.gecko.mCursor = v.keyword; unsafe { - Gecko_SetCursorArrayLength(&mut *self.gecko, v.images.len()); + bindings::Gecko_SetCursorArrayCapacity(&mut *self.gecko, v.images.len()); } for i in 0..v.images.len() { unsafe { - Gecko_SetCursorImageValue( - &mut self.gecko.mCursorImages[i], - &v.images[i].url - ); + bindings::Gecko_AppendCursorImage(&mut *self.gecko, &v.images[i].url); } match v.images[i].hotspot { @@ -2473,10 +2428,7 @@ clip-path let keyword = self.gecko.mCursor; 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(); - ComputedImageUrl::from_image_request(&gecko_image_request) - }; + let url = gecko_cursor_image.mImage.clone(); let hotspot = if gecko_cursor_image.mHaveHotspot {