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

src/components/main/layout/wrapper.rs

@@ -40,7 +40,7 @@ use script::dom::element::{Element, HTMLAreaElementTypeId, HTMLAnchorElementType
 use script::dom::element::{HTMLLinkElementTypeId};
 use script::dom::htmliframeelement::HTMLIFrameElement;
 use script::dom::htmlimageelement::HTMLImageElement;
-use script::dom::node::{DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId, NodeHelpers};
+use script::dom::node::{DocumentNodeTypeId, ElementNodeTypeId, Node, NodeTypeId, LayoutNodeHelpers};
 use script::dom::text::Text;
 use servo_msg::constellation_msg::{PipelineId, SubpageId};
 use servo_util::namespace;
@@ -163,7 +163,7 @@ impl<'ln> TLayoutNode for LayoutNode<'ln> {
     }
 
     fn type_id(&self) -> Option<NodeTypeId> {
-        Some(self.node.type_id())
+        Some(self.node.type_id_for_layout())
     }
 
     unsafe fn get_jsmanaged<'a>(&'a self) -> &'a JS<Node> {

src/components/script/dom/attr.rs

@@ -4,7 +4,7 @@
 
 use dom::bindings::codegen::BindingDeclarations::AttrBinding;
 use dom::bindings::codegen::InheritTypes::NodeCast;
-use dom::bindings::js::{JS, JSRef};
+use dom::bindings::js::{JS, JSRef, Unrooted, RootCollection};
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::element::Element;
 use dom::node::Node;
@@ -45,33 +45,35 @@ impl Reflectable for Attr {
 impl Attr {
     fn new_inherited(local_name: DOMString, value: DOMString,
                      name: DOMString, namespace: Namespace,
-                     prefix: Option<DOMString>, owner: JS<Element>) -> Attr {
+                     prefix: Option<DOMString>, owner: &JSRef<Element>) -> Attr {
         Attr {
             reflector_: Reflector::new(),
             local_name: local_name,
             value: value,
             name: name, //TODO: Intern attribute names
             namespace: namespace,
             prefix: prefix,
-            owner: owner,
+            owner: owner.unrooted(),
         }
     }
 
     pub fn new(window: &JSRef<Window>, local_name: DOMString, value: DOMString,
                name: DOMString, namespace: Namespace,
-               prefix: Option<DOMString>, owner: JS<Element>) -> JS<Attr> {
+               prefix: Option<DOMString>, owner: &JSRef<Element>) -> Unrooted<Attr> {
         let attr = Attr::new_inherited(local_name, value, name, namespace, prefix, owner);
         reflect_dom_object(~attr, window, AttrBinding::Wrap)
     }
 
     pub fn set_value(&mut self, set_type: AttrSettingType, value: DOMString) {
-        let node: JS<Node> = NodeCast::from(&self.owner);
+        let roots = RootCollection::new();
+        let owner = self.owner.root(&roots);
+        let node: &JSRef<Node> = NodeCast::from_ref(&*owner);
         let namespace_is_null = self.namespace == namespace::Null;
 
         match set_type {
             ReplacedAttr => {
                 if namespace_is_null {
-                    vtable_for(&node).before_remove_attr(self.local_name.clone(), self.value.clone());
+                    vtable_for(node).before_remove_attr(self.local_name.clone(), self.value.clone());
                 }
             }
             FirstSetAttr => {}
@@ -80,7 +82,7 @@ impl Attr {
         self.value = value;
 
         if namespace_is_null {
-            vtable_for(&node).after_set_attr(self.local_name.clone(), self.value.clone());
+            vtable_for(node).after_set_attr(self.local_name.clone(), self.value.clone());
         }
     }
 

src/components/script/dom/attrlist.rs

@@ -4,7 +4,7 @@
 
 use dom::attr::Attr;
 use dom::bindings::codegen::BindingDeclarations::AttrListBinding;
-use dom::bindings::js::{JS, JSRef};
+use dom::bindings::js::{JS, JSRef, Unrooted};
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::element::Element;
 use dom::window::Window;
@@ -25,7 +25,7 @@ impl AttrList {
         }
     }
 
-    pub fn new(window: &JSRef<Window>, elem: &JSRef<Element>) -> JS<AttrList> {
+    pub fn new(window: &JSRef<Window>, elem: &JSRef<Element>) -> Unrooted<AttrList> {
         reflect_dom_object(~AttrList::new_inherited(window.unrooted(), elem.unrooted()),
                            window, AttrListBinding::Wrap)
     }
@@ -34,11 +34,11 @@ impl AttrList {
         self.owner.get().attrs.len() as u32
     }
 
-    pub fn Item(&self, index: u32) -> Option<JS<Attr>> {
-        self.owner.get().attrs.as_slice().get(index as uint).map(|x| x.clone())
+    pub fn Item(&self, index: u32) -> Option<Unrooted<Attr>> {
+        self.owner.get().attrs.as_slice().get(index as uint).map(|x| Unrooted::new(x.clone()))
     }
 
-    pub fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<JS<Attr>> {
+    pub fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Unrooted<Attr>> {
         let item = self.Item(index);
         *found = item.is_some();
         item

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 @@ def __init__(self, errorReport, arguments, argsPre, returnType,
             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 @@ def __init__(self, config, prefix, webIDLFile):
             '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 @@ def InheritTypes(config):
         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,15 +5326,31 @@ def InheritTypes(config):
   }
 
   #[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
     }
   }
 
   #[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 @@ def InheritTypes(config):
   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 @@ def __init__(self, config, interface, desc):
         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', [])