From de90f5fce801d9d2171900b361a422a83c35a3a2 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Wed, 24 Aug 2016 17:52:15 +0530 Subject: [PATCH] Add MaybeOwned, and use for ServoNodeData. More docs. --- ports/geckolib/binding_tools/regen.py | 26 ++- ports/geckolib/gecko_bindings/bindings.rs | 15 +- .../gecko_bindings/sugar/ownership.rs | 155 +++++++++++++++++- ports/geckolib/glue.rs | 10 +- ports/geckolib/wrapper.rs | 42 +++-- 5 files changed, 216 insertions(+), 32 deletions(-) diff --git a/ports/geckolib/binding_tools/regen.py b/ports/geckolib/binding_tools/regen.py index 556e2d184c7..d293d5047ec 100755 --- a/ports/geckolib/binding_tools/regen.py +++ b/ports/geckolib/binding_tools/regen.py @@ -150,12 +150,16 @@ COMPILATION_TARGETS = { "void_types": [ "nsINode", "nsIDocument", "nsIPrincipal", "nsIURI", ], - "servo_arc_types": [ + "servo_maybe_arc_types": [ "ServoComputedValues", "RawServoStyleSheet", "ServoDeclarationBlock" ], "servo_owned_types": [ "RawServoStyleSet", + "RawGeckoNode", + ], + "servo_maybe_owned_types": [ + "ServoNodeData", ], }, @@ -318,8 +322,8 @@ def build(objdir, target_name, debug, debugger, kind_name=None, for ty in current_target["void_types"]: flags.append("--raw-line") flags.append("pub enum {} {{}}".format(ty)) - if "servo_arc_types" in current_target: - for ty in current_target["servo_arc_types"]: + if "servo_maybe_arc_types" in current_target: + for ty in current_target["servo_maybe_arc_types"]: flags.append("--blacklist-type") flags.append("{}Strong".format(ty)) flags.append("--raw-line") @@ -342,6 +346,22 @@ def build(objdir, target_name, debug, debugger, kind_name=None, flags.append("{}Owned".format(ty)) flags.append("--raw-line") flags.append("pub type {0}Owned = ::sugar::ownership::Owned<{0}>;".format(ty)) + if "servo_maybe_owned_types" in current_target: + for ty in current_target["servo_maybe_owned_types"]: + flags.append("--blacklist-type") + flags.append("{}Borrowed".format(ty)) + flags.append("--raw-line") + flags.append("pub type {0}Borrowed<'a> = ::sugar::ownership::Borrowed<'a, {0}>;" + .format(ty)) + flags.append("--blacklist-type") + flags.append("{}BorrowedMut".format(ty)) + flags.append("--raw-line") + flags.append("pub type {0}BorrowedMut<'a> = ::sugar::ownership::BorrowedMut<'a, {0}>;" + .format(ty)) + flags.append("--blacklist-type") + flags.append("{}Owned".format(ty)) + flags.append("--raw-line") + flags.append("pub type {0}Owned = ::sugar::ownership::MaybeOwned<{0}>;".format(ty)) if "structs_types" in current_target: for ty in current_target["structs_types"]: ty_fragments = ty.split("::") diff --git a/ports/geckolib/gecko_bindings/bindings.rs b/ports/geckolib/gecko_bindings/bindings.rs index 546a27b14b1..52c11df627d 100644 --- a/ports/geckolib/gecko_bindings/bindings.rs +++ b/ports/geckolib/gecko_bindings/bindings.rs @@ -14,6 +14,12 @@ pub type ServoDeclarationBlockBorrowed<'a> = ::sugar::ownership::Borrowed<'a, Se pub type RawServoStyleSetBorrowed<'a> = &'a RawServoStyleSet; pub type RawServoStyleSetBorrowedMut<'a> = &'a mut RawServoStyleSet; pub type RawServoStyleSetOwned = ::sugar::ownership::Owned; +pub type RawGeckoNodeBorrowed<'a> = &'a RawGeckoNode; +pub type RawGeckoNodeBorrowedMut<'a> = &'a mut RawGeckoNode; +pub type RawGeckoNodeOwned = ::sugar::ownership::Owned; +pub type ServoNodeDataBorrowed<'a> = ::sugar::ownership::Borrowed<'a, ServoNodeData>; +pub type ServoNodeDataBorrowedMut<'a> = ::sugar::ownership::BorrowedMut<'a, ServoNodeData>; +pub type ServoNodeDataOwned = ::sugar::ownership::MaybeOwned; use structs::nsStyleFont; unsafe impl Send for nsStyleFont {} unsafe impl Sync for nsStyleFont {} @@ -267,9 +273,10 @@ extern "C" { -> u32; pub fn Gecko_GetServoDeclarationBlock(element: *mut RawGeckoElement) -> ServoDeclarationBlockBorrowed; - pub fn Gecko_GetNodeData(node: *mut RawGeckoNode) -> *mut ServoNodeData; - pub fn Gecko_SetNodeData(node: *mut RawGeckoNode, - data: *mut ServoNodeData); + pub fn Gecko_GetNodeData(node: RawGeckoNodeBorrowed) + -> ServoNodeDataBorrowed; + pub fn Gecko_SetNodeData(node: RawGeckoNodeBorrowedMut, + data: ServoNodeDataOwned); pub fn Gecko_Atomize(aString: *const ::std::os::raw::c_char, aLength: u32) -> *mut nsIAtom; pub fn Gecko_AddRefAtom(aAtom: *mut nsIAtom); @@ -459,7 +466,7 @@ extern "C" { pub fn Gecko_CopyConstruct_nsStyleEffects(ptr: *mut nsStyleEffects, other: *const nsStyleEffects); pub fn Gecko_Destroy_nsStyleEffects(ptr: *mut nsStyleEffects); - pub fn Servo_NodeData_Drop(data: *mut ServoNodeData); + pub fn Servo_NodeData_Drop(data: ServoNodeDataOwned); pub fn Servo_StyleSheet_FromUTF8Bytes(bytes: *const u8, length: u32, parsing_mode: SheetParsingMode, base_bytes: *const u8, diff --git a/ports/geckolib/gecko_bindings/sugar/ownership.rs b/ports/geckolib/gecko_bindings/sugar/ownership.rs index 2030ccbe807..46a7500819c 100644 --- a/ports/geckolib/gecko_bindings/sugar/ownership.rs +++ b/ports/geckolib/gecko_bindings/sugar/ownership.rs @@ -19,15 +19,35 @@ pub unsafe trait HasFFI : Sized { /// Indicates that a given Servo type has the same layout /// as the corresponding HasFFI::FFIType type pub unsafe trait HasSimpleFFI : HasFFI { + #[inline] + /// Given a Servo-side reference, converts it to an + /// FFI-safe reference which can be passed to Gecko + /// + /// &ServoType -> &GeckoType fn as_ffi(&self) -> &Self::FFIType { unsafe { transmute(self) } } + #[inline] + /// Given a Servo-side mutable reference, converts it to an + /// FFI-safe mutable reference which can be passed to Gecko + /// + /// &mut ServoType -> &mut GeckoType fn as_ffi_mut(&mut self) -> &mut Self::FFIType { unsafe { transmute(self) } } + #[inline] + /// Given an FFI-safe reference obtained from Gecko + /// converts it to a Servo-side reference + /// + /// &GeckoType -> &ServoType fn from_ffi(ffi: &Self::FFIType) -> &Self { unsafe { transmute(ffi) } } + #[inline] + /// Given an FFI-safe mutable reference obtained from Gecko + /// converts it to a Servo-side mutable reference + /// + /// &mut GeckoType -> &mut ServoType fn from_ffi_mut(ffi: &mut Self::FFIType) -> &mut Self { unsafe { transmute(ffi) } } @@ -36,6 +56,7 @@ pub unsafe trait HasSimpleFFI : HasFFI { /// Indicates that the given Servo type is passed over FFI /// as a Box pub unsafe trait HasBoxFFI : HasSimpleFFI { + #[inline] fn into_ffi(self: Box) -> Owned { unsafe { transmute(self) } } @@ -67,25 +88,47 @@ pub unsafe trait HasArcFFI : HasFFI { } #[repr(C)] -/// Gecko-FFI-safe borrowed Arc (&T where T is an ArcInner). +/// Gecko-FFI-safe borrowed type /// This can be null. pub struct Borrowed<'a, T: 'a> { ptr: *const T, _marker: PhantomData<&'a T>, } +#[repr(C)] +/// Gecko-FFI-safe mutably borrowed type +/// This can be null. +pub struct BorrowedMut<'a, T: 'a> { + ptr: *mut T, + _marker: PhantomData<&'a mut T>, +} + // manual impls because derive doesn't realize that `T: Clone` isn't necessary impl<'a, T> Copy for Borrowed<'a, T> {} impl<'a, T> Clone for Borrowed<'a, T> { + #[inline] fn clone(&self) -> Self { *self } } impl<'a, T> Borrowed<'a, T> { + #[inline] pub fn is_null(&self) -> bool { self.ptr == ptr::null() } + #[inline] + /// Like Deref, but gives an Option + pub fn borrow_opt(self) -> Option<&'a T> { + if self.is_null() { + None + } else { + Some(unsafe { &*self.ptr }) + } + } + + #[inline] + /// Borrowed -> Option<&Arc> pub fn as_arc_opt(&self) -> Option<&Arc> where U: HasArcFFI { unsafe { if self.is_null() { @@ -96,11 +139,71 @@ impl<'a, T> Borrowed<'a, T> { } } + #[inline] /// Converts a borrowed FFI reference to a borrowed Arc. - /// Panics on null + /// Panics on null. + /// + /// &Borrowed -> &Arc pub fn as_arc(&self) -> &Arc where U: HasArcFFI { self.as_arc_opt().unwrap() } + + #[inline] + /// Borrowed -> Borrowed + pub fn as_ffi(&self) -> Borrowed<::FFIType> where Self: HasSimpleFFI { + unsafe { transmute(self) } + } + + #[inline] + /// Borrowed -> Borrowed + pub fn from_ffi(self) -> Borrowed<'a, U> where U: HasSimpleFFI { + unsafe { transmute(self) } + } + + #[inline] + /// Borrowed -> &ServoType + pub fn as_servo_ref(self) -> Option<&'a U> where U: HasSimpleFFI { + self.borrow_opt().map(HasSimpleFFI::from_ffi) + } +} + +impl<'a, T> BorrowedMut<'a, T> { + #[inline] + /// Like DerefMut, but gives an Option + pub fn borrow_mut_opt(self) -> Option<&'a mut T> { + // We have two choices for the signature here, it can either be + // Self -> Option<&'a mut T> or + // &'b mut Self -> Option<'b mut T> + // The former consumes the BorrowedMut (which isn't Copy), + // which can be annoying. The latter only temporarily + // borrows it, so the return value can't exit the scope + // even if Self has a longer lifetime ('a) + // + // This is basically the implicit "reborrow" pattern used with &mut + // not cleanly translating to our custom types. + + // I've chosen the former solution -- you can manually convert back + // if you need to reuse the BorrowedMut. + if self.is_null() { + None + } else { + Some(unsafe { &mut *self.ptr }) + } + } + + #[inline] + /// BorrowedMut -> &mut ServoType + pub fn as_servo_mut_ref(self) -> Option<&'a mut U> where U: HasSimpleFFI { + self.borrow_mut_opt().map(HasSimpleFFI::from_ffi_mut) + } +} + +// technically not +impl<'a, T> Deref for BorrowedMut<'a, T> { + type Target = Borrowed<'a, T>; + fn deref(&self) -> &Self::Target { + unsafe { transmute(self) } + } } #[repr(C)] @@ -112,17 +215,23 @@ pub struct Strong { } impl Strong { + #[inline] pub fn is_null(&self) -> bool { self.ptr == ptr::null() } - /// Given a non-null strong FFI reference, converts it into an Arc. + #[inline] + /// Given a non-null strong FFI reference, + /// converts it into a servo-side Arc /// Panics on null. + /// + /// Strong -> Arc pub fn into_arc(self) -> Arc where U: HasArcFFI { assert!(!self.is_null()); unsafe { transmute(self) } } + #[inline] /// Produces a null strong FFI reference pub fn null_strong() -> Self { unsafe { transmute(ptr::null::()) } @@ -132,16 +241,22 @@ impl Strong { pub unsafe trait FFIArcHelpers { type Inner: HasArcFFI; /// Converts an Arc into a strong FFI reference. + /// + /// Arc -> Strong fn into_strong(self) -> Strong<::FFIType>; /// Produces a borrowed FFI reference by borrowing an Arc. + /// + /// &Arc -> Borrowed fn as_borrowed(&self) -> Borrowed<::FFIType>; } unsafe impl FFIArcHelpers for Arc { type Inner = T; + #[inline] fn into_strong(self) -> Strong { unsafe { transmute(self) } } + #[inline] fn as_borrowed(&self) -> Borrowed { let borrowedptr = self as *const Arc as *const Borrowed; unsafe { ptr::read(borrowedptr) } @@ -157,9 +272,13 @@ pub struct Owned { } impl Owned { + /// Owned -> Box pub fn into_box(self) -> Box where U: HasBoxFFI { unsafe { transmute(self) } } + pub fn maybe(self) -> MaybeOwned { + unsafe { transmute(self) } + } } impl Deref for Owned { @@ -174,3 +293,33 @@ impl DerefMut for Owned { unsafe { &mut *self.ptr } } } + +#[repr(C)] +/// Gecko-FFI-safe owned pointer +/// Can be null +pub struct MaybeOwned { + ptr: *mut T, + _marker: PhantomData, +} + +impl MaybeOwned { + pub fn is_null(&self) -> bool { + self.ptr == ptr::null_mut() + } + /// MaybeOwned -> Option> + pub fn into_box_opt(self) -> Option> where U: HasBoxFFI { + if self.is_null() { + None + } else { + Some(unsafe { transmute(self) }) + } + } + + pub fn borrow(&self) -> Borrowed { + unsafe { transmute(self) } + } + + pub fn borrow_mut(&self) -> BorrowedMut { + unsafe { transmute(self) } + } +} diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index ce339a4fbf1..95696089349 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -10,10 +10,10 @@ use env_logger; use euclid::Size2D; use gecko_bindings::bindings::{RawGeckoDocument, RawGeckoElement, RawGeckoNode}; use gecko_bindings::bindings::{RawServoStyleSet, RawServoStyleSetBorrowedMut}; -use gecko_bindings::bindings::RawServoStyleSetOwned; +use gecko_bindings::bindings::{RawServoStyleSetOwned, ServoNodeDataOwned}; use gecko_bindings::bindings::{RawServoStyleSheetBorrowed, ServoComputedValuesBorrowed}; use gecko_bindings::bindings::{RawServoStyleSheetStrong, ServoComputedValuesStrong}; -use gecko_bindings::bindings::{ServoDeclarationBlock, ServoNodeData, ThreadSafePrincipalHolder}; +use gecko_bindings::bindings::{ServoDeclarationBlock, ThreadSafePrincipalHolder}; use gecko_bindings::bindings::{ServoDeclarationBlockBorrowed, ServoDeclarationBlockStrong}; use gecko_bindings::bindings::{ThreadSafeURIHolder, nsHTMLCSSStyleSheet}; use gecko_bindings::ptr::{GeckoArcPrincipal, GeckoArcURI}; @@ -136,10 +136,8 @@ pub extern "C" fn Servo_StyleWorkerThreadCount() -> u32 { } #[no_mangle] -pub extern "C" fn Servo_NodeData_Drop(data: *mut ServoNodeData) -> () { - unsafe { - let _ = Box::::from_raw(data as *mut NonOpaqueStyleData); - } +pub extern "C" fn Servo_NodeData_Drop(data: ServoNodeDataOwned) -> () { + let _ = data.into_box_opt::(); } #[no_mangle] diff --git a/ports/geckolib/wrapper.rs b/ports/geckolib/wrapper.rs index b4c055e34e5..74e2e47fed7 100644 --- a/ports/geckolib/wrapper.rs +++ b/ports/geckolib/wrapper.rs @@ -25,7 +25,8 @@ use gecko_bindings::bindings::{Gecko_LocalName, Gecko_Namespace, Gecko_NodeIsEle use gecko_bindings::bindings::{RawGeckoDocument, RawGeckoElement, RawGeckoNode}; use gecko_bindings::structs::{NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO, NODE_IS_DIRTY_FOR_SERVO}; use gecko_bindings::structs::{nsIAtom, nsChangeHint, nsStyleContext}; -use gecko_bindings::sugar::ownership::FFIArcHelpers; +use gecko_bindings::sugar::ownership::Borrowed; +use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI, FFIArcHelpers}; use gecko_string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use glue::GeckoDeclarationBlock; use libc::uintptr_t; @@ -53,8 +54,19 @@ use style::selector_matching::DeclarationBlock; use style::sink::Push; use url::Url; -pub type NonOpaqueStyleData = RefCell; -pub type NonOpaqueStyleDataPtr = *mut NonOpaqueStyleData; +pub struct NonOpaqueStyleData(RefCell); + +unsafe impl HasFFI for NonOpaqueStyleData { + type FFIType = ServoNodeData; +} +unsafe impl HasSimpleFFI for NonOpaqueStyleData {} +unsafe impl HasBoxFFI for NonOpaqueStyleData {} + +impl NonOpaqueStyleData { + pub fn new() -> Self { + NonOpaqueStyleData(RefCell::new(PrivateStyleData::new())) + } +} // Important: We don't currently refcount the DOM, because the wrapper lifetime // magic guarantees that our LayoutFoo references won't outlive the root, and @@ -79,17 +91,18 @@ impl<'ln> GeckoNode<'ln> { GeckoNode::from_raw(n as *const RawGeckoNode as *mut RawGeckoNode) } - fn get_node_data(&self) -> NonOpaqueStyleDataPtr { + fn get_node_data(&self) -> Borrowed { unsafe { - Gecko_GetNodeData(self.node) as NonOpaqueStyleDataPtr + // XXXManishearth should GeckoNode just contain an &'ln RawGeckoNode? + Borrowed::from_ffi(Gecko_GetNodeData(&*self.node)) } } pub fn initialize_data(self) { unsafe { if self.get_node_data().is_null() { - let ptr: NonOpaqueStyleDataPtr = Box::into_raw(Box::new(RefCell::new(PrivateStyleData::new()))); - Gecko_SetNodeData(self.node, ptr as *mut ServoNodeData); + let ptr = Box::new(NonOpaqueStyleData::new()); + Gecko_SetNodeData(&mut *self.node, ptr.into_ffi().maybe()); } } } @@ -212,7 +225,7 @@ impl<'ln> TNode for GeckoNode<'ln> { fn is_dirty(&self) -> bool { // Return true unconditionally if we're not yet styled. This is a hack // and should go away soon. - if unsafe { Gecko_GetNodeData(self.node) }.is_null() { + if self.get_node_data().is_null() { return true; } @@ -231,7 +244,7 @@ impl<'ln> TNode for GeckoNode<'ln> { fn has_dirty_descendants(&self) -> bool { // Return true unconditionally if we're not yet styled. This is a hack // and should go away soon. - if unsafe { Gecko_GetNodeData(self.node) }.is_null() { + if self.get_node_data().is_null() { return true; } let flags = unsafe { Gecko_GetNodeFlags(self.node) }; @@ -259,21 +272,18 @@ impl<'ln> TNode for GeckoNode<'ln> { #[inline(always)] unsafe fn borrow_data_unchecked(&self) -> Option<*const PrivateStyleData> { - self.get_node_data().as_ref().map(|d| d.as_unsafe_cell().get() as *const PrivateStyleData) + self.get_node_data().borrow_opt().map(|d| d.0.as_unsafe_cell().get() + as *const PrivateStyleData) } #[inline(always)] fn borrow_data(&self) -> Option> { - unsafe { - self.get_node_data().as_ref().map(|d| d.borrow()) - } + self.get_node_data().borrow_opt().map(|d| d.0.borrow()) } #[inline(always)] fn mutate_data(&self) -> Option> { - unsafe { - self.get_node_data().as_ref().map(|d| d.borrow_mut()) - } + self.get_node_data().borrow_opt().map(|d| d.0.borrow_mut()) } fn restyle_damage(self) -> Self::ConcreteRestyleDamage {