Remove JS::get/get_mut to enforce sound rooting practices.

src/components/main/layout/construct.rs

@@ -45,7 +45,6 @@ use layout::wrapper::{Before, BeforeBlock, After, AfterBlock, Normal};
 
 use gfx::display_list::OpaqueNode;
 use gfx::font_context::FontContext;
-use script::dom::bindings::codegen::InheritTypes::TextCast;
 use script::dom::bindings::js::JS;
 use script::dom::element::{HTMLIFrameElementTypeId, HTMLImageElementTypeId};
 use script::dom::element::{HTMLObjectElementTypeId};
@@ -1064,8 +1063,8 @@ impl<'ln> NodeUtils for ThreadSafeLayoutNode<'ln> {
         match self.type_id() {
             Some(TextNodeTypeId) => {
                 unsafe {
-                    let text: JS<Text> = TextCast::to(self.get_jsmanaged()).unwrap();
-                    if !is_whitespace(text.get().characterdata.data) {
+                    let text: JS<Text> = self.get_jsmanaged().transmute_copy();
+                    if !is_whitespace((*text.unsafe_get()).characterdata.data) {
                         return false
                     }
 

src/components/main/layout/wrapper.rs

@@ -242,7 +242,7 @@ impl<'ln> TNode<LayoutElement<'ln>> for LayoutNode<'ln> {
     fn as_element(&self) -> LayoutElement<'ln> {
         unsafe {
             let elem: JS<Element> = self.node.transmute_copy();
-            let element = elem.get();
+            let element = &*elem.unsafe_get();
             LayoutElement {
                 element: cast::transmute_region(element),
             }
@@ -509,10 +509,10 @@ impl<'ln> ThreadSafeLayoutNode<'ln> {
     /// Returns the next sibling of this node. Unsafe and private because this can lead to races.
     unsafe fn next_sibling(&self) -> Option<ThreadSafeLayoutNode<'ln>> {
         if self.pseudo == Before || self.pseudo == BeforeBlock {
-            return self.get().first_child_ref().map(|node| self.new_with_this_lifetime(node))
+            return (*self.get_jsmanaged().unsafe_get()).first_child_ref().map(|node| self.new_with_this_lifetime(node))
         }
 
-        self.node.get().next_sibling_ref().map(|node| self.new_with_this_lifetime(node))
+        (*self.get_jsmanaged().unsafe_get()).next_sibling_ref().map(|node| self.new_with_this_lifetime(node))
     }
 
     /// Returns an iterator over this node's children.

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, Unrooted};
+use dom::bindings::js::{JS, JSRef, Unrooted, RootCollection};
 use dom::bindings::utils::{Reflectable, Reflector, reflect_dom_object};
 use dom::element::Element;
 use dom::window::Window;
@@ -31,11 +31,13 @@ impl AttrList {
     }
 
     pub fn Length(&self) -> u32 {
-        self.owner.get().attrs.len() as u32
+        let roots = RootCollection::new();
+        self.owner.root(&roots).attrs.len() as u32
     }
 
     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()))
+        let roots = RootCollection::new();
+        self.owner.root(&roots).attrs.as_slice().get(index as uint).map(|x| Unrooted::new(x.clone()))
     }
 
     pub fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Unrooted<Attr>> {

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

@@ -2243,12 +2243,7 @@ def __init__(self, errorReport, arguments, argsPre, returnType,
             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)
+            self.cgRoot.append(CGGeneric("let result = result.root(&roots);"))
 
     def define(self):
         return self.cgRoot.define()
@@ -4321,7 +4316,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, Unrooted}',
+            'dom::bindings::js::{JS, JSRef, RootCollection, RootedReference, Unrooted, OptionalRootable}',
             'dom::bindings::utils::{CreateDOMGlobal, CreateInterfaceObjects2}',
             'dom::bindings::utils::{ConstantSpec, cx_for_dom_object, Default}',
             'dom::bindings::utils::{dom_object_slot, DOM_OBJECT_SLOT, DOMClass}',
@@ -5320,19 +5315,6 @@ def InheritTypes(config):
             derived += [CGGeneric('\n')]
 
             cast = [CGGeneric(string.Template('''pub trait ${castTraitName} {
-  #[inline(always)]
-  fn from<T: ${fromBound}>(derived: &JS<T>) -> JS<Self> {
-    unsafe { derived.clone().transmute() }
-  }
-
-  #[inline(always)]
-  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)]
   fn to_ref<'a, 'b, T: ${toBound}+Reflectable>(base: &'a JSRef<'b, T>) -> Option<&'a JSRef<'b, Self>> {
     match base.get().${checkFn}() {
@@ -5350,19 +5332,16 @@ def InheritTypes(config):
   }
 
   #[inline(always)]
-  unsafe fn to_unchecked<T: ${toBound}+Reflectable>(base: &JS<T>) -> JS<Self> {
-    assert!(base.get().${checkFn}());
-    base.clone().transmute()
-  }
-
   fn from_ref<'a, 'b, T: ${fromBound}>(derived: &'a JSRef<'b, T>) -> &'a JSRef<'b, Self> {
     unsafe { derived.transmute() }
   }
 
+  #[inline(always)]
   fn from_mut_ref<'a, 'b, T: ${fromBound}>(derived: &'a mut JSRef<'b, T>) -> &'a mut JSRef<'b, Self> {
     unsafe { derived.transmute_mut() }
   }
 
+  #[inline(always)]
   fn from_unrooted<T: ${fromBound}+Reflectable>(derived: Unrooted<T>) -> Unrooted<Self> {
     unsafe { derived.transmute() }
   }

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

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use dom::bindings::js::{JS, JSRef};
+use dom::bindings::js::{JS, JSRef, Root};
 use dom::bindings::str::ByteString;
 use dom::bindings::utils::{Reflectable, Reflector};
 use dom::bindings::utils::jsstring_to_str;
@@ -316,7 +316,7 @@ impl<T: Reflectable+IDLInterface> FromJSValConvertible<()> for JS<T> {
     }
 }
 
-impl<T: Reflectable> ToJSValConvertible for JS<T> {
+impl<'a, 'b, T: Reflectable> ToJSValConvertible for Root<'a, 'b, T> {
     fn to_jsval(&self, cx: *JSContext) -> JSVal {
         self.reflector().to_jsval(cx)
     }

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

@@ -106,31 +106,23 @@ impl<T: Reflectable> JS<T> {
     }
 }
 
+//XXXjdm This is disappointing. This only gets called from trace hooks, in theory,
+//       so it's safe to assume that self is rooted and thereby safe to access.
 impl<T: Reflectable> Reflectable for JS<T> {
     fn reflector<'a>(&'a self) -> &'a Reflector {
-        self.get().reflector()
-    }
-
-    fn mut_reflector<'a>(&'a mut self) -> &'a mut Reflector {
-        self.get_mut().mut_reflector()
-    }
-}
-
-impl<T: Reflectable> JS<T> {
-    pub fn get<'a>(&'a self) -> &'a T {
-        let borrowed = self.ptr.borrow();
         unsafe {
-            &**borrowed
+            (*self.unsafe_get()).reflector()
         }
     }
 
-    pub fn get_mut<'a>(&'a mut self) -> &'a mut T {
-        let mut borrowed = self.ptr.borrow_mut();
+    fn mut_reflector<'a>(&'a mut self) -> &'a mut Reflector {
         unsafe {
-            &mut **borrowed
+            (*self.unsafe_get()).mut_reflector()
         }
     }
+}
 
+impl<T: Reflectable> JS<T> {
     /// Returns an unsafe pointer to the interior of this JS object without touching the borrow
     /// flags. This is the only method that be safely accessed from layout. (The fact that this
     /// is unsafe is what necessitates the layout wrappers.)
@@ -213,6 +205,16 @@ impl<T: Reflectable> OptionalRootable<T> for Option<Unrooted<T>> {
     }
 }
 
+pub trait OptionalRootedRootable<T> {
+    fn root<'a, 'b>(&self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>>;
+}
+
+impl<T: Reflectable> OptionalRootedRootable<T> for Option<JS<T>> {
+    fn root<'a, 'b>(&self, roots: &'a RootCollection) -> Option<Root<'a, 'b, T>> {
+        self.as_ref().map(|inner| inner.root(roots))
+    }
+}
+
 pub trait ResultRootable<T,U> {
     fn root<'a, 'b>(self, roots: &'a RootCollection) -> Result<Root<'a, 'b, T>, U>;
 }

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, RootCollection, Unrooted};
+use dom::bindings::js::{JS, JSRef, RootCollection, Unrooted, Root};
 use dom::bindings::trace::Untraceable;
 use dom::browsercontext;
 use dom::window;
@@ -602,13 +602,16 @@ pub extern fn wrap_for_same_compartment(cx: *JSContext, obj: *JSObject) -> *JSOb
 
 pub extern fn outerize_global(_cx: *JSContext, obj: JSHandleObject) -> *JSObject {
     unsafe {
+        let roots = RootCollection::new();
         debug!("outerizing");
         let obj = *obj.unnamed;
-        let win: JS<window::Window> =
+        let win: Root<window::Window> =
             unwrap_jsmanaged(obj,
                              IDLInterface::get_prototype_id(None::<window::Window>),
-                             IDLInterface::get_prototype_depth(None::<window::Window>)).unwrap();
-        win.get().browser_context.get_ref().window_proxy()
+                             IDLInterface::get_prototype_depth(None::<window::Window>))
+            .unwrap()
+            .root(&roots);
+        win.deref().browser_context.get_ref().window_proxy()
     }
 }
 

src/components/script/dom/browsercontext.rs

@@ -2,7 +2,7 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-use dom::bindings::js::{JS, JSRef};
+use dom::bindings::js::{JS, JSRef, Unrooted, RootCollection};
 use dom::bindings::trace::Traceable;
 use dom::bindings::utils::Reflectable;
 use dom::document::Document;
@@ -32,13 +32,14 @@ impl BrowserContext {
         context
     }
 
-    pub fn active_document(&self) -> JS<Document> {
-        self.history.get(self.active_index).document.clone()
+    pub fn active_document(&self) -> Unrooted<Document> {
+        Unrooted::new(self.history.get(self.active_index).document.clone())
     }
 
-    pub fn active_window(&self) -> JS<Window> {
-        let doc = self.active_document();
-        doc.get().window.clone()
+    pub fn active_window(&self) -> Unrooted<Window> {
+        let roots = RootCollection::new();
+        let doc = self.active_document().root(&roots);
+        Unrooted::new(doc.deref().window.clone())
     }
 
     pub fn window_proxy(&self) -> *JSObject {
@@ -47,7 +48,8 @@ impl BrowserContext {
     }
 
     pub fn create_window_proxy(&self) -> *JSObject {
-        let win = self.active_window();
+        let roots = RootCollection::new();
+        let win = self.active_window().root(&roots);
         let page = win.get().page();
         let js_info = page.js_info();
 

src/components/script/dom/document.rs

@@ -660,8 +660,9 @@ impl Document {
 
     pub fn Location(&mut self, _abstract_self: &JSRef<Document>) -> Unrooted<Location> {
         let roots = RootCollection::new();
-        let window = self.window.root(&roots);
-        self.window.get_mut().Location(&*window)
+        let mut window = self.window.root(&roots);
+        let window_alias = self.window.root(&roots);
+        window.get_mut().Location(&*window_alias)
     }
 
     pub fn Children(&self, abstract_self: &JSRef<Document>) -> Unrooted<HTMLCollection> {
@@ -695,11 +696,13 @@ impl Document {
     }
 
     pub fn damage_and_reflow(&self, damage: DocumentDamageLevel) {
-        self.window.get().damage_and_reflow(damage);
+        let roots = RootCollection::new();
+        self.window.root(&roots).damage_and_reflow(damage);
     }
 
     pub fn wait_until_safe_to_modify_dom(&self) {
-        self.window.get().wait_until_safe_to_modify_dom();
+        let roots = RootCollection::new();
+        self.window.root(&roots).wait_until_safe_to_modify_dom();
     }
 
 

src/components/script/dom/domimplementation.rs

@@ -61,9 +61,9 @@ impl DOMImplementation {
             Name => Err(NamespaceError),
             // Step 3.
             QName => {
-                let document = self.owner.get().Document();
-                let document = document.root(&roots);
-                Ok(DocumentType::new(qname, Some(pubid), Some(sysid), &document.root_ref()))
+                let owner = self.owner.root(&roots);
+                let document = owner.deref().Document().root(&roots);
+                Ok(DocumentType::new(qname, Some(pubid), Some(sysid), &*document))
             }
         }
     }