servo: Merge #18641 - Improve DomRoot<T> (from servo:ROOT-ALL-THE-THINGS); r=SimonSapin
authorAnthony Ramine <n.oxyde@gmail.com>
Tue, 26 Sep 2017 14:25:20 -0500
changeset 433861 f31e9db442adf1a5bc208beabbcba435bc052ecf
parent 433860 50d414550fa4fe84656254dbdcac5e811a7fe6d9
child 433862 ea69ee15ed90aa56ea1811b6ec473b0b0c31bad8
push id8114
push userjlorenzo@mozilla.com
push dateThu, 02 Nov 2017 16:33:21 +0000
treeherdermozilla-beta@73e0d89a540f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersSimonSapin
milestone58.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #18641 - Improve DomRoot<T> (from servo:ROOT-ALL-THE-THINGS); r=SimonSapin Source-Repo: https://github.com/servo/servo Source-Revision: 0160aaeeea848f685ba090b4541646b0429a37e1
servo/components/script/dom/bindings/refcounted.rs
servo/components/script/dom/bindings/root.rs
servo/components/script/dom/bindings/weakref.rs
--- a/servo/components/script/dom/bindings/refcounted.rs
+++ b/servo/components/script/dom/bindings/refcounted.rs
@@ -17,17 +17,16 @@
 //! The `Trusted<T>` object contains an atomic reference counted pointer to the Rust DOM object.
 //! A hashtable resides in the script thread, keyed on the pointer.
 //! The values in this hashtable are weak reference counts. When a `Trusted<T>` object is
 //! created or cloned, the reference count is increased. When a `Trusted<T>` is dropped, the count
 //! decreases. If the count hits zero, the weak reference is emptied, and is removed from
 //! its hash table during the next GC. During GC, the entries of the hash table are counted
 //! as JS roots.
 
-use core::nonzero::NonZero;
 use dom::bindings::conversions::ToJSValConvertible;
 use dom::bindings::error::Error;
 use dom::bindings::reflector::{DomObject, Reflector};
 use dom::bindings::root::DomRoot;
 use dom::bindings::trace::trace_reflector;
 use dom::promise::Promise;
 use js::jsapi::JSTracer;
 use libc;
@@ -180,17 +179,17 @@ impl<T: DomObject> Trusted<T> {
     /// obtained.
     pub fn root(&self) -> DomRoot<T> {
         assert!(LIVE_REFERENCES.with(|ref r| {
             let r = r.borrow();
             let live_references = r.as_ref().unwrap();
             self.owner_thread == (&*live_references) as *const _ as *const libc::c_void
         }));
         unsafe {
-            DomRoot::new(NonZero::new_unchecked(self.refcount.0 as *const T))
+            DomRoot::from_ref(&*(self.refcount.0 as *const T))
         }
     }
 }
 
 impl<T: DomObject> Clone for Trusted<T> {
     fn clone(&self) -> Trusted<T> {
         Trusted {
             refcount: self.refcount.clone(),
--- a/servo/components/script/dom/bindings/root.rs
+++ b/servo/components/script/dom/bindings/root.rs
@@ -553,20 +553,21 @@ pub unsafe fn trace_roots(tracer: *mut J
 }
 
 /// A rooted reference to a DOM object.
 ///
 /// The JS value is pinned for the duration of this object's lifetime; roots
 /// are additive, so this object's destruction will not invalidate other roots
 /// for the same JS value. `Root`s cannot outlive the associated
 /// `RootCollection` object.
+#[allow(unrooted_must_root)]
 #[allow_unrooted_interior]
 pub struct DomRoot<T: DomObject> {
     /// Reference to rooted value that must not outlive this container
-    ptr: NonZero<*const T>,
+    ptr: Dom<T>,
     /// List that ensures correct dynamic root ordering
     root_list: *const RootCollection,
 }
 
 impl<T: Castable> DomRoot<T> {
     /// Cast a DOM object root upwards to one of the interfaces it derives from.
     pub fn upcast<U>(root: DomRoot<T>) -> DomRoot<U>
         where U: Castable,
@@ -586,46 +587,47 @@ impl<T: Castable> DomRoot<T> {
         }
     }
 }
 
 impl<T: DomObject> DomRoot<T> {
     /// Create a new stack-bounded root for the provided JS-owned value.
     /// It cannot outlive its associated `RootCollection`, and it gives
     /// out references which cannot outlive this new `Root`.
-    pub fn new(unrooted: NonZero<*const T>) -> DomRoot<T> {
+    #[allow(unrooted_must_root)]
+    unsafe fn new(unrooted: Dom<T>) -> DomRoot<T> {
         debug_assert!(thread_state::get().is_script());
         STACK_ROOTS.with(|ref collection| {
             let RootCollectionPtr(collection) = collection.get().unwrap();
-            unsafe { (*collection).root(&*(*unrooted.get()).reflector()) }
+            (*collection).root(unrooted.reflector());
             DomRoot {
                 ptr: unrooted,
                 root_list: collection,
             }
         })
     }
 
     /// Generate a new root from a reference
     pub fn from_ref(unrooted: &T) -> DomRoot<T> {
-        DomRoot::new(unsafe { NonZero::new_unchecked(unrooted) })
+        unsafe { DomRoot::new(Dom::from_ref(unrooted)) }
     }
 }
 
 impl<'root, T: DomObject + 'root> RootedReference<'root> for DomRoot<T> {
     type Ref = &'root T;
     fn r(&'root self) -> &'root T {
         self
     }
 }
 
 impl<T: DomObject> Deref for DomRoot<T> {
     type Target = T;
     fn deref(&self) -> &T {
         debug_assert!(thread_state::get().is_script());
-        unsafe { &*self.ptr.get() }
+        &self.ptr
     }
 }
 
 impl<T: DomObject + HeapSizeOf> HeapSizeOf for DomRoot<T> {
     fn heap_size_of_children(&self) -> usize {
         (**self).heap_size_of_children()
     }
 }
--- a/servo/components/script/dom/bindings/weakref.rs
+++ b/servo/components/script/dom/bindings/weakref.rs
@@ -81,17 +81,19 @@ impl<T: WeakReferenceable> WeakRef<T> {
     /// This is just a convenience wrapper around `<T as WeakReferenceable>::downgrade`
     /// to not have to import `WeakReferenceable`.
     pub fn new(value: &T) -> Self {
         value.downgrade()
     }
 
     /// DomRoot a weak reference. Returns `None` if the object was already collected.
     pub fn root(&self) -> Option<DomRoot<T>> {
-        unsafe { &*self.ptr.get() }.value.get().map(DomRoot::new)
+        unsafe { &*self.ptr.get() }.value.get().map(|ptr| unsafe {
+            DomRoot::from_ref(&*ptr.get())
+        })
     }
 
     /// Return whether the weakly-referenced object is still alive.
     pub fn is_alive(&self) -> bool {
         unsafe { &*self.ptr.get() }.value.get().is_some()
     }
 }