Implement safe rooting strategy via Unrooted, Root, JSRef, and JS.

src/components/script/dom/bindings/codegen/CodegenRust.py

@@ -1008,6 +1008,13 @@ def typeNeedsCx(type, retVal=False):
         return True
     return type.isCallback() or type.isAny() or type.isObject()
 
+def typeRetValNeedsRooting(type):
+    if type is None:
+        return False
+    if type.nullable():
+        type = type.inner
+    return type.isGeckoInterface() and not type.isCallback()
+
 def memberIsCreator(member):
     return member.getExtendedAttribute("Creator") is not None
 
@@ -1039,7 +1046,7 @@ def getRetvalDeclarationForType(returnType, descriptorProvider):
     if returnType.isGeckoInterface():
         descriptor = descriptorProvider.getDescriptor(
             returnType.unroll().inner.identifier.name)
-        result = CGGeneric(descriptor.nativeType)
+        result = CGGeneric(descriptor.returnType)
         if returnType.nullable():
             result = CGWrapper(result, pre="Option<", post=">")
         return result
@@ -2235,6 +2242,14 @@ class CGCallGenerator(CGThing):
             self.cgRoot.append(CGGeneric("}"))
             self.cgRoot.append(CGGeneric("result = result_fallible.unwrap();"))
 
+        if typeRetValNeedsRooting(returnType):
+            rooted_value = CGGeneric("result.root(&roots).root_ref().unrooted()")
+            if returnType.nullable():
+                rooted_value = CGWrapper(rooted_value, pre="result.map(|result| ", post=")")
+            rooted_value = CGWrapper(rooted_value, pre="let result = ", post=";")
+
+            self.cgRoot.append(rooted_value)
+
     def define(self):
         return self.cgRoot.define()
 
@@ -4306,7 +4321,7 @@ class CGBindingRoot(CGThing):
             'js::glue::{RUST_JS_NumberValue, RUST_JSID_IS_STRING}',
             'dom::types::*',
             'dom::bindings',
-            'dom::bindings::js::{JS, JSRef, RootCollection, RootedReference}',
+            'dom::bindings::js::{JS, JSRef, RootCollection, RootedReference, Unrooted}',
             'dom::bindings::utils::{CreateDOMGlobal, CreateInterfaceObjects2}',
             'dom::bindings::utils::{ConstantSpec, cx_for_dom_object, Default}',
             'dom::bindings::utils::{dom_object_slot, DOM_OBJECT_SLOT, DOMClass}',
@@ -5277,8 +5292,9 @@ class GlobalGenRoots():
         descriptors = config.getDescriptors(register=True, hasInterfaceObject=True)
         allprotos = [CGGeneric("#![allow(unused_imports)]\n"),
                      CGGeneric("use dom::types::*;\n"),
-                     CGGeneric("use dom::bindings::js::{JS, JSRef};\n"),
+                     CGGeneric("use dom::bindings::js::{JS, JSRef, Unrooted};\n"),
                      CGGeneric("use dom::bindings::trace::JSTraceable;\n"),
+                     CGGeneric("use dom::bindings::utils::Reflectable;\n"),
                      CGGeneric("use serialize::{Encodable, Encoder};\n"),
                      CGGeneric("use js::jsapi::JSTracer;\n\n")]
         for descriptor in descriptors:
@@ -5310,7 +5326,7 @@ class GlobalGenRoots():
   }
 
   #[inline(always)]
-  fn to<T: ${toBound}>(base: &JS<T>) -> Option<JS<Self>> {
+  fn to<T: ${toBound}+Reflectable>(base: &JS<T>) -> Option<JS<Self>> {
     match base.get().${checkFn}() {
         true => unsafe { Some(base.clone().transmute()) },
         false => None
@@ -5318,7 +5334,23 @@ class GlobalGenRoots():
   }
 
   #[inline(always)]
-  unsafe fn to_unchecked<T: ${toBound}>(base: &JS<T>) -> JS<Self> {
+  fn to_ref<'a, 'b, T: ${toBound}+Reflectable>(base: &'a JSRef<'b, T>) -> Option<&'a JSRef<'b, Self>> {
+    match base.get().${checkFn}() {
+        true => unsafe { Some(base.transmute()) },
+        false => None
+    }
+  }
+
+  #[inline(always)]
+  fn to_mut_ref<'a, 'b, T: ${toBound}+Reflectable>(base: &'a mut JSRef<'b, T>) -> Option<&'a mut JSRef<'b, Self>> {
+    match base.get().${checkFn}() {
+        true => unsafe { Some(base.transmute_mut()) },
+        false => None
+    }
+  }
+
+  #[inline(always)]
+  unsafe fn to_unchecked<T: ${toBound}+Reflectable>(base: &JS<T>) -> JS<Self> {
     assert!(base.get().${checkFn}());
     base.clone().transmute()
   }
@@ -5330,6 +5362,10 @@ class GlobalGenRoots():
   fn from_mut_ref<'a, 'b, T: ${fromBound}>(derived: &'a mut JSRef<'b, T>) -> &'a mut JSRef<'b, Self> {
     unsafe { derived.transmute_mut() }
   }
+
+  fn from_unrooted<T: ${fromBound}+Reflectable>(derived: Unrooted<T>) -> Unrooted<Self> {
+    unsafe { derived.transmute() }
+  }
 }
 ''').substitute({'checkFn': 'is_' + name.lower(),
                  'castTraitName': name + 'Cast',

src/components/script/dom/bindings/codegen/Configuration.py

@@ -133,6 +133,7 @@ class Descriptor(DescriptorProvider):
         else:
             nativeTypeDefault = 'JS<%s>' % ifaceName
 
+        self.returnType = "Unrooted<%s>" % ifaceName
         self.nativeType = desc.get('nativeType', nativeTypeDefault)
         self.concreteType = desc.get('concreteType', ifaceName)
         self.needsAbstract = desc.get('needsAbstract', [])

src/components/script/dom/bindings/js.rs

@@ -10,8 +10,52 @@ use layout_interface::TrustedNodeAddress;
 use std::cast;
 use std::cell::{Cell, RefCell};
 use std::ptr;
-//use std::ops::{Deref, DerefMut};
 
+/// A type that represents a JS-owned value that may or may not be rooted.
+/// Importantly, it requires rooting in order to interact with the value in any way.
+/// Can be assigned into JS-owned member fields (ie. JS<T> types) safely via the
+/// `JS<T>::assign` method or `OptionalAssignable::assign` (for Option<JS<T>> fields).
+pub struct Unrooted<T> {
+    inner: JS<T>
+}
+
+impl<T> Eq for Unrooted<T> {
+    fn eq(&self, other: &Unrooted<T>) -> bool {
+        self.inner == other.inner
+    }
+}
+
+impl<T: Reflectable> Unrooted<T> {
+    /// Create a new Unrooted value from a JS-owned value.
+    pub fn new(inner: JS<T>) -> Unrooted<T> {
+        Unrooted {
+            inner: inner
+        }
+    }
+
+    /// Create a new Unrooted value from a rooted value.
+    pub fn new_rooted<'a>(root: &JSRef<'a, T>) -> Unrooted<T> {
+        Unrooted {
+            inner: root.unrooted()
+        }
+    }
+
+    /// Root this unrooted value.
+    pub fn root<'a, 'b>(self, collection: &'a RootCollection) -> Root<'a, 'b, T> {
+        collection.new_root(&self.inner)
+    }
+
+    unsafe fn inner(&self) -> JS<T> {
+        self.inner.clone()
+    }
+
+    //XXXjdm It would be lovely if this could be private.
+    pub unsafe fn transmute<To>(self) -> Unrooted<To> {
+        cast::transmute(self)
+    }
+}
+
+/// A rooted, JS-owned value. Must only be used as a field in other JS-owned types.
 pub struct JS<T> {
     ptr: RefCell<*mut T>
 }
@@ -32,12 +76,15 @@ impl <T> Clone for JS<T> {
 }
 
 impl<T: Reflectable> JS<T> {
+    /// Create a new JS-reflected DOM object; returns an Unrooted type because the new value
+    /// is not safe to use until it is rooted.
     pub fn new(obj: ~T,
                window:  &JSRef<Window>,
-               wrap_fn: extern "Rust" fn(*JSContext, &JSRef<Window>, ~T) -> JS<T>) -> JS<T> {
-        wrap_fn(window.get().get_cx(), window, obj)
+               wrap_fn: extern "Rust" fn(*JSContext, &JSRef<Window>, ~T) -> JS<T>) -> Unrooted<T> {
+        Unrooted::new(wrap_fn(window.get().get_cx(), window, obj))
     }
 
+    /// Create a new JS-owned value wrapped from a raw Rust pointer.
     pub unsafe fn from_raw(raw: *mut T) -> JS<T> {
         JS {
             ptr: RefCell::new(raw)
@@ -45,6 +92,7 @@ impl<T: Reflectable> JS<T> {
     }
 
 
+    /// Create a new JS-owned value wrapped from an address known to be a Node pointer.
     pub unsafe fn from_trusted_node_address(inner: TrustedNodeAddress) -> JS<T> {
         let TrustedNodeAddress(addr) = inner;
         JS {
@@ -52,7 +100,8 @@ impl<T: Reflectable> JS<T> {
         }
     }
 
-    pub fn root<'a>(&self, collection: &'a RootCollection) -> Root<'a, T> {
+    /// Root this JS-owned value to prevent its collection as garbage.
+    pub fn root<'a, 'b>(&self, collection: &'a RootCollection) -> Root<'a, 'b, T> {
         collection.new_root(self)
     }
 }
@@ -67,7 +116,7 @@ impl<T: Reflectable> Reflectable for JS<T> {
     }
 }
 
-impl<T> JS<T> {
+impl<T: Reflectable> JS<T> {
     pub fn get<'a>(&'a self) -> &'a T {
         let borrowed = self.ptr.borrow();
         unsafe {
@@ -88,6 +137,13 @@ impl<T> JS<T> {
     pub unsafe fn unsafe_get(&self) -> *mut T {
         cast::transmute_copy(&self.ptr)
     }
+
+    /// Store an unrooted value in this field. This is safe under the assumption that JS<T>
+    /// values are only used as fields in DOM types that are reachable in the GC graph,
+    /// so this unrooted value becomes transitively rooted for the lifetime of its new owner.
+    pub fn assign(&mut self, val: Unrooted<T>) {
+        *self = unsafe { val.inner() };
+    }
 }
 
 impl<From, To> JS<From> {
@@ -105,17 +161,86 @@ pub trait RootedReference<T> {
     fn root_ref<'a>(&'a self) -> Option<JSRef<'a, T>>;
 }
 
-impl<'a, T: Reflectable> RootedReference<T> for Option<Root<'a, T>> {
+impl<'a, 'b, T: Reflectable> RootedReference<T> for Option<Root<'a, 'b, T>> {
     fn root_ref<'a>(&'a self) -> Option<JSRef<'a, T>> {
         self.as_ref().map(|root| root.root_ref())
     }
 }
 
+// This trait should never be public; it allows access to unrooted values, and is thus
+// easy to misuse.
+/*definitely not public*/ trait Assignable<T> {
+    fn get_js(&self) -> JS<T>;
+}
+
+impl<T> Assignable<T> for JS<T> {
+    fn get_js(&self) -> JS<T> {
+        self.clone()
+    }
+}
+
+impl<'a, T> Assignable<T> for JSRef<'a, T> {
+    fn get_js(&self) -> JS<T> {
+        self.unrooted()
+    }
+}
+
+// Assignable should not be exposed publically, since it's used to
+// extract unrooted values in a safe way WHEN USED CORRECTLY.
+impl<T: Reflectable> Assignable<T> for Unrooted<T> {
+    fn get_js(&self) -> JS<T> {
+        unsafe { self.inner() }
+    }
+}
+
+pub trait OptionalAssignable<T> {
+    fn assign(&mut self, val: Option<T>);
+}
+
+impl<T: Assignable<U>, U: Reflectable> OptionalAssignable<T> for Option<JS<U>> {
+    fn assign(&mut self, val: Option<T>) {
+        *self = val.map(|val| val.get_js());
+    }
+}
+
+pub trait OptionalRootable<T> {
+    fn root<'a, 'b>(self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>>;
+}
+
+impl<T: Reflectable> OptionalRootable<T> for Option<Unrooted<T>> {
+    fn root<'a, 'b>(self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>> {
+        self.map(|inner| inner.root(roots))
+    }
+}
+
+pub trait ResultRootable<T,U> {
+    fn root<'a, 'b>(self, roots: &'a RootCollection) -> Result<Root<'a, 'b, T>, U>;
+}
+
+impl<T: Reflectable, U> ResultRootable<T, U> for Result<Unrooted<T>, U> {
+    fn root<'a, 'b>(self, roots: &'a RootCollection) -> Result<Root<'a, 'b, T>, U> {
+        self.map(|inner| inner.root(roots))
+    }
+}
+
+/// Provides a facility to push unrooted values onto lists of rooted values. This is safe
+/// under the assumption that said lists are reachable via the GC graph, and therefore the
+/// new values are transitively rooted for the lifetime of their new owner.
+pub trait UnrootedPushable<T> {
+    fn push_unrooted(&mut self, val: Unrooted<T>);
+}
+
+impl<T: Reflectable> UnrootedPushable<T> for Vec<JS<T>> {
+    fn push_unrooted(&mut self, val: Unrooted<T>) {
+        unsafe { self.push(val.inner()) };
+    }
+}
+
 #[deriving(Eq, Clone)]
 struct RootReference(*JSObject);
 
 impl RootReference {
-    fn new<'a, T: Reflectable>(unrooted: &Root<'a, T>) -> RootReference {
+    fn new<'a, 'b, T: Reflectable>(unrooted: &Root<'a, 'b, T>) -> RootReference {
         RootReference(unrooted.rooted())
     }
 
@@ -126,6 +251,7 @@ impl RootReference {
 
 static MAX_STACK_ROOTS: uint = 10;
 
+/// An opaque, LIFO rooting mechanism.
 pub struct RootCollection {
     roots: [Cell<RootReference>, ..MAX_STACK_ROOTS],
     current: Cell<uint>,
@@ -139,7 +265,7 @@ impl RootCollection {
         }
     }
 
-    fn new_root<'a, T: Reflectable>(&'a self, unrooted: &JS<T>) -> Root<'a, T> {
+    fn new_root<'a, 'b, T: Reflectable>(&'a self, unrooted: &JS<T>) -> Root<'a, 'b, T> {
         Root::new(self, unrooted)
     }
 
@@ -147,13 +273,15 @@ impl RootCollection {
         let current = self.current.get();
         assert!(current < MAX_STACK_ROOTS);
         self.roots[current].set(unrooted);
+        debug!("  rooting {:?}", unrooted);
         self.current.set(current + 1);
     }
 
-    fn root<'a, T: Reflectable>(&self, unrooted: &Root<'a, T>) {
+    fn root<'a, 'b, T: Reflectable>(&self, unrooted: &Root<'a, 'b, T>) {
         self.root_impl(RootReference::new(unrooted));
     }
 
+    /// Root a raw JS pointer.
     pub fn root_raw(&self, unrooted: *JSObject) {
         self.root_impl(RootReference(unrooted));
     }
@@ -162,29 +290,41 @@ impl RootCollection {
         let mut current = self.current.get();
         assert!(current != 0);
         current -= 1;
+        debug!("unrooting {:?} (expecting {:?}", self.roots[current].get(), rooted);
         assert!(self.roots[current].get() == rooted);
         self.roots[current].set(RootReference::null());
         self.current.set(current);
     }
 
-    fn unroot<'a, T: Reflectable>(&self, rooted: &Root<'a, T>) {
+    fn unroot<'a, 'b, T: Reflectable>(&self, rooted: &Root<'a, 'b, T>) {
         self.unroot_impl(RootReference::new(rooted));
     }
 
+    /// Unroot a raw JS pointer. Must occur in reverse order to its rooting.
     pub fn unroot_raw(&self, rooted: *JSObject) {
         self.unroot_impl(RootReference(rooted));
     }
 }
 
-pub struct Root<'a, T> {
+/// A rooted JS value. 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. Roots cannot outlive the associated RootCollection object.
+/// Attempts to transfer ownership of a Root via moving will trigger dynamic unrooting
+/// failures due to incorrect ordering.
+pub struct Root<'a, 'b, T> {
     root_list: &'a RootCollection,
+    jsref: JSRef<'b, T>,
     ptr: RefCell<*mut T>,
 }
 
-impl<'a, T: Reflectable> Root<'a, T> {
-    fn new(roots: &'a RootCollection, unrooted: &JS<T>) -> Root<'a, T> {
+impl<'a, 'b, T: Reflectable> Root<'a, 'b, T> {
+    fn new(roots: &'a RootCollection, unrooted: &JS<T>) -> Root<'a, 'b, T> {
         let root = Root {
             root_list: roots,
+            jsref: JSRef {
+                ptr: unrooted.ptr.clone(),
+                chain: unsafe { ::std::cast::transmute_region(&()) },
+            },
             ptr: unrooted.ptr.clone()
         };
         roots.root(&root);
@@ -209,24 +349,27 @@ impl<'a, T: Reflectable> Root<'a, T> {
         self.reflector().get_jsobject()
     }
 
+    fn internal_root_ref<'a>(&'a self) -> &'a JSRef<'b, T> {
+        &'a self.jsref
+    }
+
+    fn mut_internal_root_ref<'a>(&'a mut self) -> &'a mut JSRef<'b, T> {
+        &'a mut self.jsref
+    }
+
     pub fn root_ref<'b>(&'b self) -> JSRef<'b,T> {
-        unsafe {
-            JSRef {
-                ptr: self.ptr.clone(),
-                chain: ::std::cast::transmute_region(&()),
-            }
-        }
+        self.internal_root_ref().clone()
     }
 }
 
 #[unsafe_destructor]
-impl<'a, T: Reflectable> Drop for Root<'a, T> {
+impl<'a, 'b, T: Reflectable> Drop for Root<'a, 'b, T> {
     fn drop(&mut self) {
         self.root_list.unroot(self);
     }
 }
 
-impl<'a, T: Reflectable> Reflectable for Root<'a, T> {
+impl<'a, 'b, T: Reflectable> Reflectable for Root<'a, 'b, T> {
     fn reflector<'a>(&'a self) -> &'a Reflector {
         self.get().reflector()
     }
@@ -236,17 +379,29 @@ impl<'a, T: Reflectable> Reflectable for Root<'a, T> {
     }
 }
 
-/*impl<'a, T> Deref for Root<'a, T> {
-    fn deref<'a>(&'a self) -> &'a T {
+impl<'a, 'b, T: Reflectable> Deref<JSRef<'b, T>> for Root<'a, 'b, T> {
+    fn deref<'c>(&'c self) -> &'c JSRef<'b, T> {
+        self.internal_root_ref()
+    }
+}
+
+impl<'a, 'b, T: Reflectable> DerefMut<JSRef<'b, T>> for Root<'a, 'b, T> {
+    fn deref_mut<'c>(&'c mut self) -> &'c mut JSRef<'b, T> {
+        self.mut_internal_root_ref()
+    }
+}
+
+impl<'a, T: Reflectable> Deref<T> for JSRef<'a, T> {
+    fn deref<'b>(&'b self) -> &'b T {
         self.get()
     }
 }
 
-impl<'a, T> DerefMut for Root<'a, T> {
-    fn deref_mut<'a>(&'a mut self) -> &'a mut T {
+impl<'a, T: Reflectable> DerefMut<T> for JSRef<'a, T> {
+    fn deref_mut<'b>(&'b mut self) -> &'b mut T {
         self.get_mut()
     }
-}*/
+}
 
 /// Encapsulates a reference to something that is guaranteed to be alive. This is freely copyable.
 pub struct JSRef<'a, T> {

src/components/script/dom/bindings/utils.rs

@@ -5,7 +5,7 @@
 use dom::bindings::codegen::PrototypeList;
 use dom::bindings::codegen::PrototypeList::MAX_PROTO_CHAIN_LENGTH;
 use dom::bindings::conversions::{FromJSValConvertible, IDLInterface};
-use dom::bindings::js::{JS, JSRef};
+use dom::bindings::js::{JS, JSRef, RootCollection, Unrooted};
 use dom::bindings::trace::Untraceable;
 use dom::browsercontext;
 use dom::window;
@@ -391,7 +391,7 @@ pub fn reflect_dom_object<T: Reflectable>
         (obj:     ~T,
          window:  &JSRef<window::Window>,
          wrap_fn: extern "Rust" fn(*JSContext, &JSRef<window::Window>, ~T) -> JS<T>)
-         ->       JS<T> {
+         -> Unrooted<T> {
     JS::new(obj, window, wrap_fn)
 }
 
@@ -613,18 +613,20 @@ pub extern fn outerize_global(_cx: *JSContext, obj: JSHandleObject) -> *JSObject
 }
 
 /// Returns the global object of the realm that the given JS object was created in.
-pub fn global_object_for_js_object(obj: *JSObject) -> JS<window::Window> {
+pub fn global_object_for_js_object(obj: *JSObject) -> Unrooted<window::Window> {
     unsafe {
         let global = GetGlobalForObjectCrossCompartment(obj);
         let clasp = JS_GetClass(global);
         assert!(((*clasp).flags & (JSCLASS_IS_DOMJSCLASS | JSCLASS_IS_GLOBAL)) != 0);
-        FromJSValConvertible::from_jsval(ptr::null(), ObjectOrNullValue(global), ())
-            .ok().expect("found DOM global that doesn't unwrap to Window")
+        Unrooted::new(
+            FromJSValConvertible::from_jsval(ptr::null(), ObjectOrNullValue(global), ())
+                .ok().expect("found DOM global that doesn't unwrap to Window"))
     }
 }
 
 fn cx_for_dom_reflector(obj: *JSObject) -> *JSContext {
-    let win = global_object_for_js_object(obj);
+    let roots = RootCollection::new();
+    let win = global_object_for_js_object(obj).root(&roots);
     let js_info = win.get().page().js_info();
     match *js_info {
         Some(ref info) => info.js_context.deref().deref().ptr,