Add MaybeOwned, and use for ServoNodeData. More docs.

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:
+    if "servo_maybe_arc_types" in current_target:
-        for ty in current_target["servo_arc_types"]:
+        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("::")

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<RawServoStyleSet>;
+pub type RawGeckoNodeBorrowed<'a> = &'a RawGeckoNode;
+pub type RawGeckoNodeBorrowedMut<'a> = &'a mut RawGeckoNode;
+pub type RawGeckoNodeOwned = ::sugar::ownership::Owned<RawGeckoNode>;
+pub type ServoNodeDataBorrowed<'a> = ::sugar::ownership::Borrowed<'a, ServoNodeData>;
+pub type ServoNodeDataBorrowedMut<'a> = ::sugar::ownership::BorrowedMut<'a, ServoNodeData>;
+pub type ServoNodeDataOwned = ::sugar::ownership::MaybeOwned<ServoNodeData>;
 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_GetNodeData(node: RawGeckoNodeBorrowed)
-    pub fn Gecko_SetNodeData(node: *mut RawGeckoNode,
+     -> ServoNodeDataBorrowed;
-                             data: *mut ServoNodeData);
+    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,

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<Self>) -> Owned<Self::FFIType> {
         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<GeckoType> -> Option<&Arc<ServoType>>
     pub fn as_arc_opt<U>(&self) -> Option<&Arc<U>> where U: HasArcFFI<FFIType = T> {
         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<GeckoType> -> &Arc<ServoType>
     pub fn as_arc<U>(&self) -> &Arc<U> where U: HasArcFFI<FFIType = T> {
         self.as_arc_opt().unwrap()
     }
+
+    #[inline]
+    /// Borrowed<ServoType> -> Borrowed<GeckoType>
+    pub fn as_ffi(&self) -> Borrowed<<Self as HasFFI>::FFIType> where Self: HasSimpleFFI {
+        unsafe { transmute(self) }
+    }
+
+    #[inline]
+    /// Borrowed<GeckoType> -> Borrowed<ServoType>
+    pub fn from_ffi<U>(self) -> Borrowed<'a, U> where U: HasSimpleFFI<FFIType = T> {
+        unsafe { transmute(self) }
+    }
+
+    #[inline]
+    /// Borrowed<GeckoType> -> &ServoType
+    pub fn as_servo_ref<U>(self) -> Option<&'a U> where U: HasSimpleFFI<FFIType = T> {
+        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<GeckoType> -> &mut ServoType
+    pub fn as_servo_mut_ref<U>(self) -> Option<&'a mut U> where U: HasSimpleFFI<FFIType = T> {
+        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<T> {
 }
 
 impl<T> Strong<T> {
+    #[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<GeckoType> -> Arc<ServoType>
     pub fn into_arc<U>(self) -> Arc<U> where U: HasArcFFI<FFIType = T> {
         assert!(!self.is_null());
         unsafe { transmute(self) }
     }
 
+    #[inline]
     /// Produces a null strong FFI reference
     pub fn null_strong() -> Self {
         unsafe { transmute(ptr::null::<T>()) }
@@ -132,16 +241,22 @@ impl<T> Strong<T> {
 pub unsafe trait FFIArcHelpers {
     type Inner: HasArcFFI;
     /// Converts an Arc into a strong FFI reference.
+    ///
+    /// Arc<ServoType> -> Strong<GeckoType>
     fn into_strong(self) -> Strong<<Self::Inner as HasFFI>::FFIType>;
     /// Produces a borrowed FFI reference by borrowing an Arc.
+    ///
+    /// &Arc<ServoType> -> Borrowed<GeckoType>
     fn as_borrowed(&self) -> Borrowed<<Self::Inner as HasFFI>::FFIType>;
 }
 
 unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
     type Inner = T;
+    #[inline]
     fn into_strong(self) -> Strong<T::FFIType> {
         unsafe { transmute(self) }
     }
+    #[inline]
     fn as_borrowed(&self) -> Borrowed<T::FFIType> {
         let borrowedptr = self as *const Arc<T> as *const Borrowed<T::FFIType>;
         unsafe { ptr::read(borrowedptr) }
@@ -157,9 +272,13 @@ pub struct Owned<T> {
 }
 
 impl<T> Owned<T> {
+    /// Owned<GeckoType> -> Box<ServoType>
     pub fn into_box<U>(self) -> Box<T> where U: HasBoxFFI<FFIType = T> {
         unsafe { transmute(self) }
     }
+    pub fn maybe(self) -> MaybeOwned<T> {
+        unsafe { transmute(self) }
+    }
 }
 
 impl<T> Deref for Owned<T> {
@@ -174,3 +293,33 @@ impl<T> DerefMut for Owned<T> {
         unsafe { &mut *self.ptr }
     }
 }
+
+#[repr(C)]
+/// Gecko-FFI-safe owned pointer
+/// Can be null
+pub struct MaybeOwned<T> {
+    ptr: *mut T,
+    _marker: PhantomData<T>,
+}
+
+impl<T> MaybeOwned<T> {
+    pub fn is_null(&self) -> bool {
+        self.ptr == ptr::null_mut()
+    }
+    /// MaybeOwned<GeckoType> -> Option<Box<ServoType>>
+    pub fn into_box_opt<U>(self) -> Option<Box<T>> where U: HasBoxFFI<FFIType = T> {
+        if self.is_null() {
+            None
+        } else {
+            Some(unsafe { transmute(self) })
+        }
+    }
+
+    pub fn borrow(&self) -> Borrowed<T> {
+        unsafe { transmute(self) }
+    }
+
+    pub fn borrow_mut(&self) -> BorrowedMut<T> {
+        unsafe { transmute(self) }
+    }
+}

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) -> () {
+pub extern "C" fn Servo_NodeData_Drop(data: ServoNodeDataOwned) -> () {
-    unsafe {
+    let _ = data.into_box_opt::<NonOpaqueStyleData>();
-        let _ = Box::<NonOpaqueStyleData>::from_raw(data as *mut NonOpaqueStyleData);
-    }
 }
 
 #[no_mangle]

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<PrivateStyleData>;
+pub struct NonOpaqueStyleData(RefCell<PrivateStyleData>);
-pub type NonOpaqueStyleDataPtr = *mut NonOpaqueStyleData;
+
+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<NonOpaqueStyleData> {
         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())));
+                let ptr = Box::new(NonOpaqueStyleData::new());
-                Gecko_SetNodeData(self.node, ptr as *mut ServoNodeData);
+                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<Ref<PrivateStyleData>> {
-        unsafe {
+        self.get_node_data().borrow_opt().map(|d| d.0.borrow())
-            self.get_node_data().as_ref().map(|d| d.borrow())
-        }
     }
 
     #[inline(always)]
     fn mutate_data(&self) -> Option<RefMut<PrivateStyleData>> {
-        unsafe {
+        self.get_node_data().borrow_opt().map(|d| d.0.borrow_mut())
-            self.get_node_data().as_ref().map(|d| d.borrow_mut())
-        }
     }
 
     fn restyle_damage(self) -> Self::ConcreteRestyleDamage {