From 9b92d0bc3a590697b2f17ef16b2dfeee057a57f2 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 2 Aug 2016 17:58:10 -0700 Subject: [PATCH 1/4] Make Servo_InheritComputedValues return the default computed values if the passed parent is null. We could make two separate FFI functions for this, but in practice this is what the callers are likely to want. --- ports/geckolib/glue.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 87e69aabef0..f0c7f2446d4 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -355,10 +355,12 @@ pub extern "C" fn Servo_GetComputedValuesForPseudoElement(parent_style: *mut Ser pub extern "C" fn Servo_InheritComputedValues(parent_style: *mut ServoComputedValues) -> *mut ServoComputedValues { type Helpers = ArcHelpers; - Helpers::with(parent_style, |parent| { - let style = ComputedValues::inherit_from(parent); - Helpers::from(style) - }) + let style = if parent_style.is_null() { + Arc::new(ComputedValues::initial_values().clone()) + } else { + Helpers::with(parent_style, ComputedValues::inherit_from) + }; + Helpers::from(style) } #[no_mangle] From f0f39904a2361e1d249a4fe359660d9c6516afe8 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Tue, 2 Aug 2016 22:03:15 -0700 Subject: [PATCH 2/4] Explicitly allocate and deallocate our initial computed values. This allows us to trigger style struct destructors before Gecko's leak checking runs. --- components/style/properties/gecko.mako.rs | 37 +++++++++++++++-------- ports/geckolib/gecko_bindings/bindings.rs | 1 + ports/geckolib/glue.rs | 9 ++++++ 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 24893da9f7b..bb8fd5220fa 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -100,7 +100,30 @@ impl ComputedValues { ComputedValues::inherit_from(parent) } - pub fn initial_values() -> &'static Self { &*INITIAL_GECKO_VALUES } + pub fn initial_values() -> &'static Self { + unsafe { + debug_assert!(!INITIAL_GECKO_VALUES.is_null()); + &*INITIAL_GECKO_VALUES + } + } + + pub unsafe fn initialize() { + debug_assert!(INITIAL_GECKO_VALUES.is_null()); + INITIAL_GECKO_VALUES = Box::into_raw(Box::new(ComputedValues { + % for style_struct in data.style_structs: + ${style_struct.ident}: style_structs::${style_struct.name}::initial(), + % endfor + custom_properties: None, + shareable: true, + writing_mode: WritingMode::empty(), + root_font_size: longhands::font_size::get_initial_value(), + })); + } + + pub unsafe fn shutdown() { + debug_assert!(!INITIAL_GECKO_VALUES.is_null()); + let _ = Box::from_raw(INITIAL_GECKO_VALUES); + } #[inline] pub fn do_cascade_property(f: F) { @@ -1338,17 +1361,7 @@ ${impl_style_struct(style_struct)} ${define_ffi_struct_accessor(style_struct)} % endfor -lazy_static! { - pub static ref INITIAL_GECKO_VALUES: ComputedValues = ComputedValues { - % for style_struct in data.style_structs: - ${style_struct.ident}: style_structs::${style_struct.name}::initial(), - % endfor - custom_properties: None, - shareable: true, - writing_mode: WritingMode::empty(), - root_font_size: longhands::font_size::get_initial_value(), - }; -} +static mut INITIAL_GECKO_VALUES: *mut ComputedValues = 0 as *mut ComputedValues; static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [ % for property in data.longhands: diff --git a/ports/geckolib/gecko_bindings/bindings.rs b/ports/geckolib/gecko_bindings/bindings.rs index 10d2bbb7c43..309805e6ae8 100644 --- a/ports/geckolib/gecko_bindings/bindings.rs +++ b/ports/geckolib/gecko_bindings/bindings.rs @@ -368,6 +368,7 @@ extern "C" { pub fn Servo_AddRefComputedValues(arg1: *mut ServoComputedValues); pub fn Servo_ReleaseComputedValues(arg1: *mut ServoComputedValues); pub fn Servo_Initialize(); + pub fn Servo_Shutdown(); pub fn Servo_RestyleDocument(doc: *mut RawGeckoDocument, set: *mut RawServoStyleSet); pub fn Servo_RestyleSubtree(node: *mut RawGeckoNode, diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index f0c7f2446d4..98932fb8efc 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -76,6 +76,15 @@ pub extern "C" fn Servo_Initialize() -> () { // // See https://doc.rust-lang.org/log/env_logger/index.html for instructions. env_logger::init().unwrap(); + + // Allocate our default computed values. + unsafe { ComputedValues::initialize(); } +} + +#[no_mangle] +pub extern "C" fn Servo_Shutdown() -> () { + // Destroy our default computed values. + unsafe { ComputedValues::shutdown(); } } fn restyle_subtree(node: GeckoNode, raw_data: *mut RawServoStyleSet) { From 1e7307601d24697bd748f6a5f9f398715f06d8c9 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 3 Aug 2016 12:52:14 -0700 Subject: [PATCH 3/4] Properly deallocate PrivateStyleData. In Servo_DropNodeData, we do Box::::from_raw, which is off by one level of indirection with the current types. --- ports/geckolib/wrapper.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index d71666975a0..6fc4a319c2a 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -49,7 +49,8 @@ use style::selector_impl::ElementExt; use style::sink::Push; use url::Url; -pub type NonOpaqueStyleData = *mut RefCell; +pub type NonOpaqueStyleData = RefCell; +pub type NonOpaqueStyleDataPtr = *mut NonOpaqueStyleData; // Important: We don't currently refcount the DOM, because the wrapper lifetime // magic guarantees that our LayoutFoo references won't outlive the root, and @@ -74,16 +75,16 @@ impl<'ln> GeckoNode<'ln> { GeckoNode::from_raw(n as *const RawGeckoNode as *mut RawGeckoNode) } - fn get_node_data(&self) -> NonOpaqueStyleData { + fn get_node_data(&self) -> NonOpaqueStyleDataPtr { unsafe { - Gecko_GetNodeData(self.node) as NonOpaqueStyleData + Gecko_GetNodeData(self.node) as NonOpaqueStyleDataPtr } } pub fn initialize_data(self) { unsafe { if self.get_node_data().is_null() { - let ptr: NonOpaqueStyleData = Box::into_raw(Box::new(RefCell::new(PrivateStyleData::new()))); + let ptr: NonOpaqueStyleDataPtr = Box::into_raw(Box::new(RefCell::new(PrivateStyleData::new()))); Gecko_SetNodeData(self.node, ptr as *mut ServoNodeData); } } From b916ad0a85b88eb40d919986cdc0c6b7420a4fab Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 3 Aug 2016 17:55:02 -0700 Subject: [PATCH 4/4] Convert currentColor crash into a warning. --- components/style/properties/gecko.mako.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index bb8fd5220fa..8fd16e33c07 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -259,7 +259,7 @@ def set_gecko_property(ffi_name, expr): // In the longer term, Gecko should store currentColor as a computed // value, so that we don't need to do this: // https://bugzilla.mozilla.org/show_bug.cgi?id=760345 - unimplemented!(); + warn!("stylo: mishandling currentColor"); % endif