Make Arc helpers into methods

components/style/gecko_conversions.rs

@@ -11,17 +11,19 @@
 use app_units::Au;
 use gecko_bindings::bindings::{RawServoStyleSheet, ServoComputedValues};
 use gecko_bindings::structs::nsStyleCoord_CalcValue;
-use gecko_bindings::sugar::refptr::HasArcFFI;
+use gecko_bindings::sugar::refptr::{HasArcFFI, HasFFI};
 use properties::ComputedValues;
 use stylesheets::Stylesheet;
 use values::computed::{CalcLengthOrPercentage, LengthOrPercentage};
 
-unsafe impl HasArcFFI for Stylesheet {
+unsafe impl HasFFI for Stylesheet {
     type FFIType = RawServoStyleSheet;
 }
-unsafe impl HasArcFFI for ComputedValues {
+unsafe impl HasArcFFI for Stylesheet {}
+unsafe impl HasFFI for ComputedValues {
     type FFIType = ServoComputedValues;
 }
+unsafe impl HasArcFFI for ComputedValues {}
 
 impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {

components/style/properties/gecko.mako.rs

@@ -27,7 +27,6 @@ use gecko_bindings::bindings::{Gecko_FontFamilyList_Clear, Gecko_InitializeImage
 use gecko_bindings::bindings::ServoComputedValuesBorrowed;
 use gecko_bindings::structs;
 use gecko_bindings::sugar::ns_style_coord::{CoordDataValue, CoordData, CoordDataMut};
-use gecko_bindings::sugar::refptr::HasArcFFI;
 use gecko_values::{StyleCoordHelpers, GeckoStyleCoordConvertible, convert_nscolor_to_rgba};
 use gecko_values::convert_rgba_to_nscolor;
 use gecko_values::round_border_to_device_pixels;
@@ -1715,8 +1714,8 @@ fn static_assert() {
 #[allow(non_snake_case, unused_variables)]
 pub extern "C" fn Servo_GetStyle${style_struct.gecko_name}(computed_values: ServoComputedValuesBorrowed)
   -> *const ${style_struct.gecko_ffi_name} {
-    ComputedValues::with(computed_values, |values| values.get_${style_struct.name_lower}().get_gecko()
+    computed_values.as_arc::<ComputedValues>().get_${style_struct.name_lower}().get_gecko()
-                                                as *const ${style_struct.gecko_ffi_name})
+        as *const ${style_struct.gecko_ffi_name}
 }
 </%def>
 

ports/geckolib/gecko_bindings/sugar/refptr.rs

@@ -7,55 +7,24 @@ use std::mem::{forget, transmute};
 use std::ptr;
 use std::sync::Arc;
 
+/// Indicates that a given Servo type has a corresponding
+/// Gecko FFI type
+/// The correspondence is not defined at this stage,
+/// use HasArcFFI or similar traits to define it
+pub unsafe trait HasFFI : Sized {
+    type FFIType: Sized;
+}
+
 /// Helper trait for conversions between FFI Strong/Borrowed types and Arcs
 ///
 /// Should be implemented by types which are passed over FFI as Arcs
 /// via Strong and Borrowed
-pub unsafe trait HasArcFFI where Self: Sized {
+pub unsafe trait HasArcFFI : HasFFI {
-    /// Gecko's name for the type
+    // these methods can't be on Borrowed because it leads to an unspecified
-    /// This is equivalent to ArcInner<Self>
+    // impl parameter
-    type FFIType: Sized;
-
-    /// Given a non-null borrowed FFI reference, this produces a temporary
-    /// Arc which is borrowed by the given closure and used.
-    /// Panics on null.
-    fn with<F, Output>(raw: Borrowed<Self::FFIType>, cb: F) -> Output
-               where F: FnOnce(&Arc<Self>) -> Output {
-        Self::maybe_with(raw, |opt| cb(opt.unwrap()))
-    }
-
-    /// Given a maybe-null borrowed FFI reference, this produces a temporary
-    /// Option<Arc> (None if null) which is borrowed by the given closure and used
-    fn maybe_with<F, Output>(maybe_raw: Borrowed<Self::FFIType>, cb: F) -> Output
-                         where F: FnOnce(Option<&Arc<Self>>) -> Output {
-        cb(Self::borrowed_as(&maybe_raw))
-    }
-
-    /// Given a non-null strong FFI reference, converts it into an Arc.
-    /// Panics on null.
-    fn into(ptr: Strong<Self::FFIType>) -> Arc<Self> {
-        assert!(!ptr.is_null());
-        unsafe { transmute(ptr) }
-    }
-
-    fn borrowed_as<'a>(ptr: &'a Borrowed<'a, Self::FFIType>) -> Option<&'a Arc<Self>> {
-        unsafe {
-            if ptr.is_null() {
-                None
-            } else {
-                Some(transmute::<&Borrowed<_>, &Arc<_>>(ptr))
-            }
-        }
-    }
-
-    /// Converts an Arc into a strong FFI reference.
-    fn from_arc(owned: Arc<Self>) -> Strong<Self::FFIType> {
-        unsafe { transmute(owned) }
-    }
-
     /// Artificially increments the refcount of a borrowed Arc over FFI.
     unsafe fn addref(ptr: Borrowed<Self::FFIType>) {
-        Self::with(ptr, |arc| forget(arc.clone()));
+        forget(ptr.as_arc::<Self>().clone())
     }
 
     /// Given a (possibly null) borrowed FFI reference, decrements the refcount.
@@ -63,22 +32,10 @@ pub unsafe trait HasArcFFI where Self: Sized {
     /// know that a strong reference to the backing Arc is disappearing
     /// (usually on the C++ side) without running the Arc destructor.
     unsafe fn release(ptr: Borrowed<Self::FFIType>) {
-        if let Some(arc) = Self::borrowed_as(&ptr) {
+        if let Some(arc) = ptr.as_arc_opt::<Self>() {
             let _: Arc<_> = ptr::read(arc as *const Arc<_>);
         }
     }
-
-    /// Produces a borrowed FFI reference by borrowing an Arc.
-    fn to_borrowed<'a>(arc: &'a Arc<Self>)
-        -> Borrowed<'a, Self::FFIType> {
-        let borrowedptr = arc as *const Arc<Self> as *const Borrowed<'a, Self::FFIType>;
-        unsafe { ptr::read(borrowedptr) }
-    }
-
-    /// Produces a null strong FFI reference
-    fn null_strong() -> Strong<Self::FFIType> {
-        unsafe { transmute(ptr::null::<Self::FFIType>()) }
-    }
 }
 
 #[repr(C)]
@@ -100,6 +57,22 @@ impl<'a, T> Borrowed<'a, T> {
     pub fn is_null(&self) -> bool {
         self.ptr == ptr::null()
     }
+
+    pub fn as_arc_opt<U>(&self) -> Option<&Arc<U>> where U: HasArcFFI<FFIType = T> {
+        unsafe {
+            if self.is_null() {
+                None
+            } else {
+                Some(transmute::<&Borrowed<_>, &Arc<_>>(self))
+            }
+        }
+    }
+
+    /// Converts a borrowed FFI reference to a borrowed Arc.
+    /// Panics on null
+    pub fn as_arc<U>(&self) -> &Arc<U> where U: HasArcFFI<FFIType = T> {
+        self.as_arc_opt().unwrap()
+    }
 }
 
 #[repr(C)]
@@ -114,4 +87,35 @@ impl<T> Strong<T> {
     pub fn is_null(&self) -> bool {
         self.ptr == ptr::null()
     }
+
+    /// Given a non-null strong FFI reference, converts it into an Arc.
+    /// Panics on null.
+    pub fn into_arc<U>(self) -> Arc<U> where U: HasArcFFI<FFIType = T> {
+        assert!(!self.is_null());
+        unsafe { transmute(self) }
+    }
+
+    /// Produces a null strong FFI reference
+    pub fn null_strong() -> Self {
+        unsafe { transmute(ptr::null::<T>()) }
+    }
+}
+
+pub unsafe trait FFIArcHelpers {
+    type Inner: HasArcFFI;
+    /// Converts an Arc into a strong FFI reference.
+    fn into_strong(self) -> Strong<<Self::Inner as HasFFI>::FFIType>;
+    /// Produces a borrowed FFI reference by borrowing an Arc.
+    fn as_borrowed(&self) -> Borrowed<<Self::Inner as HasFFI>::FFIType>;
+}
+
+unsafe impl<T: HasArcFFI> FFIArcHelpers for Arc<T> {
+    type Inner = T;
+    fn into_strong(self) -> Strong<T::FFIType> {
+        unsafe {transmute(self)}
+    }
+    fn as_borrowed(&self) -> Borrowed<T::FFIType> {
+        let borrowedptr = self as *const Arc<T> as *const Borrowed<T::FFIType>;
+        unsafe { ptr::read(borrowedptr) }
+    }
 }

ports/geckolib/glue.rs

@@ -19,7 +19,7 @@ use gecko_bindings::ptr::{GeckoArcPrincipal, GeckoArcURI};
 use gecko_bindings::structs::ServoElementSnapshot;
 use gecko_bindings::structs::nsRestyleHint;
 use gecko_bindings::structs::{SheetParsingMode, nsIAtom};
-use gecko_bindings::sugar::refptr::HasArcFFI;
+use gecko_bindings::sugar::refptr::{FFIArcHelpers, HasArcFFI, HasFFI, Strong};
 use gecko_string_cache::Atom;
 use snapshot::GeckoElementSnapshot;
 use std::mem::transmute;
@@ -176,22 +176,20 @@ pub extern "C" fn Servo_StyleSheet_FromUTF8Bytes(bytes: *const u8,
 pub extern "C" fn Servo_StyleSet_AppendStyleSheet(raw_data: *mut RawServoStyleSet,
                                                   raw_sheet: RawServoStyleSheetBorrowed) {
     let data = PerDocumentStyleData::borrow_mut_from_raw(raw_data);
-    Stylesheet::with(raw_sheet, |sheet| {
+    let sheet = raw_sheet.as_arc();
-        data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
+    data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
-        data.stylesheets.push(sheet.clone());
+    data.stylesheets.push(sheet.clone());
-        data.stylesheets_changed = true;
+    data.stylesheets_changed = true;
-    });
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_PrependStyleSheet(raw_data: *mut RawServoStyleSet,
                                                    raw_sheet: RawServoStyleSheetBorrowed) {
     let data = PerDocumentStyleData::borrow_mut_from_raw(raw_data);
-    Stylesheet::with(raw_sheet, |sheet| {
+    let sheet = raw_sheet.as_arc();
-        data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
+    data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
-        data.stylesheets.insert(0, sheet.clone());
+    data.stylesheets.insert(0, sheet.clone());
-        data.stylesheets_changed = true;
+    data.stylesheets_changed = true;
-    })
 }
 
 #[no_mangle]
@@ -199,29 +197,26 @@ pub extern "C" fn Servo_StyleSet_InsertStyleSheetBefore(raw_data: *mut RawServoS
                                                         raw_sheet: RawServoStyleSheetBorrowed,
                                                         raw_reference: RawServoStyleSheetBorrowed) {
     let data = PerDocumentStyleData::borrow_mut_from_raw(raw_data);
-    Stylesheet::with(raw_sheet, |sheet| {
+    let sheet = raw_sheet.as_arc();
-        Stylesheet::with(raw_reference, |reference| {
+    let reference = raw_reference.as_arc();
-            data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
+    data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
-            let index = data.stylesheets.iter().position(|x| arc_ptr_eq(x, reference)).unwrap();
+    let index = data.stylesheets.iter().position(|x| arc_ptr_eq(x, reference)).unwrap();
-            data.stylesheets.insert(index, sheet.clone());
+    data.stylesheets.insert(index, sheet.clone());
-            data.stylesheets_changed = true;
+    data.stylesheets_changed = true;
-        })
-    })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSet_RemoveStyleSheet(raw_data: *mut RawServoStyleSet,
                                                   raw_sheet: RawServoStyleSheetBorrowed) {
     let data = PerDocumentStyleData::borrow_mut_from_raw(raw_data);
-    Stylesheet::with(raw_sheet, |sheet| {
+    let sheet = raw_sheet.as_arc();
-        data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
+    data.stylesheets.retain(|x| !arc_ptr_eq(x, sheet));
-        data.stylesheets_changed = true;
+    data.stylesheets_changed = true;
-    });
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_StyleSheet_HasRules(raw_sheet: RawServoStyleSheetBorrowed) -> bool {
-    Stylesheet::with(raw_sheet, |sheet| !sheet.rules.is_empty())
+    !raw_sheet.as_arc::<Stylesheet>().rules.is_empty()
 }
 
 #[no_mangle]
@@ -249,7 +244,7 @@ pub extern "C" fn Servo_ComputedValues_Get(node: *mut RawGeckoNode)
             Arc::new(ComputedValues::initial_values().clone())
         },
     };
-    ComputedValues::from_arc(arc_cv)
+    arc_cv.into_strong()
 }
 
 #[no_mangle]
@@ -265,10 +260,9 @@ pub extern "C" fn Servo_ComputedValues_GetForAnonymousBox(parent_style_or_null:
     let pseudo = PseudoElement::from_atom_unchecked(atom, /* anon_box = */ true);
 
 
-    ComputedValues::maybe_with(parent_style_or_null, |maybe_parent| {
+    let maybe_parent = parent_style_or_null.as_arc_opt();
-        let new_computed = data.stylist.precomputed_values_for_pseudo(&pseudo, maybe_parent);
+    let new_computed = data.stylist.precomputed_values_for_pseudo(&pseudo, maybe_parent);
-        new_computed.map_or(ComputedValues::null_strong(), |c| ComputedValues::from_arc(c))
+    new_computed.map_or(Strong::null_strong(), |c| c.into_strong())
-    })
 }
 
 #[no_mangle]
@@ -282,9 +276,9 @@ pub extern "C" fn Servo_ComputedValues_GetForPseudoElement(parent_style: ServoCo
 
     let parent_or_null = || {
         if is_probe {
-            ComputedValues::null_strong()
+            Strong::null_strong()
         } else {
-            ComputedValues::from_arc(ComputedValues::with(parent_style, |parent| parent.clone()))
+            parent_style.as_arc::<ComputedValues>().clone().into_strong()
         }
     };
 
@@ -305,14 +299,13 @@ pub extern "C" fn Servo_ComputedValues_GetForPseudoElement(parent_style: ServoCo
                                      .and_then(|data| {
                                          data.per_pseudo.get(&pseudo).map(|c| c.clone())
                                      });
-            maybe_computed.map_or_else(parent_or_null, ComputedValues::from_arc)
+            maybe_computed.map_or_else(parent_or_null, FFIArcHelpers::into_strong)
         }
         PseudoElementCascadeType::Lazy => {
-            ComputedValues::with(parent_style, |parent| {
+            let parent = parent_style.as_arc::<ComputedValues>();
-                data.stylist
+            data.stylist
-                    .lazily_compute_pseudo_element_style(&element, &pseudo, parent)
+                .lazily_compute_pseudo_element_style(&element, &pseudo, parent)
-                    .map_or_else(parent_or_null, ComputedValues::from_arc)
+                .map_or_else(parent_or_null, FFIArcHelpers::into_strong)
-            })
         }
         PseudoElementCascadeType::Precomputed => {
             unreachable!("Anonymous pseudo found in \
@@ -327,9 +320,9 @@ pub extern "C" fn Servo_ComputedValues_Inherit(parent_style: ServoComputedValues
     let style = if parent_style.is_null() {
         Arc::new(ComputedValues::initial_values().clone())
     } else {
-        ComputedValues::with(parent_style, ComputedValues::inherit_from)
+        ComputedValues::inherit_from(parent_style.as_arc())
     };
-    ComputedValues::from_arc(style)
+    style.into_strong()
 }
 
 #[no_mangle]
@@ -367,20 +360,21 @@ pub struct GeckoDeclarationBlock {
     pub immutable: AtomicBool,
 }
 
-unsafe impl HasArcFFI for GeckoDeclarationBlock {
+unsafe impl HasFFI for GeckoDeclarationBlock {
     type FFIType = ServoDeclarationBlock;
 }
+unsafe impl HasArcFFI for GeckoDeclarationBlock {}
 
 #[no_mangle]
 pub extern "C" fn Servo_ParseStyleAttribute(bytes: *const u8, length: u32,
                                             cache: *mut nsHTMLCSSStyleSheet)
                                             -> ServoDeclarationBlockStrong {
     let value = unsafe { from_utf8_unchecked(slice::from_raw_parts(bytes, length as usize)) };
-    GeckoDeclarationBlock::from_arc(Arc::new(GeckoDeclarationBlock {
+    Arc::new(GeckoDeclarationBlock {
         declarations: GeckoElement::parse_style_attribute(value).map(Arc::new),
         cache: AtomicPtr::new(cache),
         immutable: AtomicBool::new(false),
-    }))
+    }).into_strong()
 }
 
 #[no_mangle]
@@ -396,23 +390,17 @@ pub extern "C" fn Servo_DeclarationBlock_Release(declarations: ServoDeclarationB
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_GetCache(declarations: ServoDeclarationBlockBorrowed)
                                                  -> *mut nsHTMLCSSStyleSheet {
-    GeckoDeclarationBlock::with(declarations, |declarations| {
+    declarations.as_arc::<GeckoDeclarationBlock>().cache.load(Ordering::Relaxed)
-        declarations.cache.load(Ordering::Relaxed)
-    })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SetImmutable(declarations: ServoDeclarationBlockBorrowed) {
-    GeckoDeclarationBlock::with(declarations, |declarations| {
+    declarations.as_arc::<GeckoDeclarationBlock>().immutable.store(true, Ordering::Relaxed)
-        declarations.immutable.store(true, Ordering::Relaxed)
-    })
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_ClearCachePointer(declarations: ServoDeclarationBlockBorrowed) {
-    GeckoDeclarationBlock::with(declarations, |declarations| {
+    declarations.as_arc::<GeckoDeclarationBlock>().cache.store(ptr::null_mut(), Ordering::Relaxed)
-        declarations.cache.store(ptr::null_mut(), Ordering::Relaxed)
-    });
 }
 
 #[no_mangle]

ports/geckolib/wrapper.rs

@@ -25,7 +25,7 @@ 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::refptr::HasArcFFI;
+use gecko_bindings::sugar::refptr::FFIArcHelpers;
 use gecko_string_cache::{Atom, Namespace, WeakAtom, WeakNamespace};
 use glue::GeckoDeclarationBlock;
 use libc::uintptr_t;
@@ -109,7 +109,7 @@ impl TRestyleDamage for GeckoRestyleDamage {
     fn compute(source: &nsStyleContext,
                new_style: &Arc<ComputedValues>) -> Self {
         let context = source as *const nsStyleContext as *mut nsStyleContext;
-        let hint = unsafe { Gecko_CalcStyleDifference(context, ComputedValues::to_borrowed(new_style)) };
+        let hint = unsafe { Gecko_CalcStyleDifference(context, new_style.as_borrowed()) };
         GeckoRestyleDamage(hint)
     }
 
@@ -474,11 +474,8 @@ impl<'le> TElement for GeckoElement<'le> {
         if declarations.is_null() {
             None
         } else {
-            let opt_ptr = GeckoDeclarationBlock::with(declarations, |declarations| {
+            let declarations = declarations.as_arc::<GeckoDeclarationBlock>();
-                // Use a raw pointer to extend the lifetime
+            declarations.declarations.as_ref().map(|r| r as *const Arc<_>).map(|ptr| unsafe { &*ptr })
-                declarations.declarations.as_ref().map(|r| r as *const Arc<_>)
-            });
-            opt_ptr.map(|ptr| unsafe { &*ptr })
         }
     }