Simplify the rooting API and make it suitable for exact rooting

components/layout/wrapper.rs

@@ -518,6 +518,7 @@ pub struct LayoutElement<'le> {
 
 impl<'le> LayoutElement<'le> {
     pub fn style_attribute(&self) -> &'le Option<PropertyDeclarationBlock> {
+        use script::dom::element::ElementHelpers;
         let style: &Option<PropertyDeclarationBlock> = unsafe {
             &*self.element.style_attribute().borrow_for_layout()
         };

components/script/devtools.rs

@@ -5,14 +5,13 @@
 use devtools_traits::{EvaluateJSReply, NodeInfo, Modification, TimelineMarker, TimelineMarkerType};
 use dom::bindings::conversions::FromJSValConvertible;
 use dom::bindings::conversions::StringificationBehavior;
-use dom::bindings::js::{JSRef, OptionalRootable, Rootable, Temporary};
+use dom::bindings::js::Root;
 use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast};
 use dom::bindings::codegen::Bindings::DocumentBinding::DocumentMethods;
 use dom::bindings::codegen::Bindings::DOMRectBinding::{DOMRectMethods};
 use dom::bindings::codegen::Bindings::ElementBinding::{ElementMethods};
 use dom::node::{Node, NodeHelpers};
 use dom::window::{WindowHelpers, ScriptHelpers};
-use dom::element::Element;
 use dom::document::DocumentHelpers;
 use page::{IterablePage, Page};
 use msg::constellation_msg::PipelineId;
@@ -26,7 +25,7 @@ use std::rc::Rc;
 
 pub fn handle_evaluate_js(page: &Rc<Page>, pipeline: PipelineId, eval: String, reply: Sender<EvaluateJSReply>){
     let page = get_page(&*page, pipeline);
-    let window = page.window().root();
+    let window = page.window();
     let cx = window.r().get_cx();
     let mut rval = RootedValue::new(cx, UndefinedValue());
     window.r().evaluate_js_on_global_with_result(&eval, rval.handle_mut());
@@ -51,28 +50,28 @@ pub fn handle_evaluate_js(page: &Rc<Page>, pipeline: PipelineId, eval: String, r
 
 pub fn handle_get_root_node(page: &Rc<Page>, pipeline: PipelineId, reply: Sender<NodeInfo>) {
     let page = get_page(&*page, pipeline);
-    let document = page.document().root();
+    let document = page.document();
 
-    let node: JSRef<Node> = NodeCast::from_ref(document.r());
+    let node = NodeCast::from_ref(document.r());
     reply.send(node.summarize()).unwrap();
 }
 
 pub fn handle_get_document_element(page: &Rc<Page>, pipeline: PipelineId, reply: Sender<NodeInfo>) {
     let page = get_page(&*page, pipeline);
-    let document = page.document().root();
-    let document_element = document.r().GetDocumentElement().root().unwrap();
+    let document = page.document();
+    let document_element = document.r().GetDocumentElement().unwrap();
 
-    let node: JSRef<Node> = NodeCast::from_ref(document_element.r());
+    let node = NodeCast::from_ref(document_element.r());
     reply.send(node.summarize()).unwrap();
 }
 
-fn find_node_by_unique_id(page: &Rc<Page>, pipeline: PipelineId, node_id: String) -> Temporary<Node> {
+fn find_node_by_unique_id(page: &Rc<Page>, pipeline: PipelineId, node_id: String) -> Root<Node> {
     let page = get_page(&*page, pipeline);
-    let document = page.document().root();
-    let node: JSRef<Node> = NodeCast::from_ref(document.r());
+    let document = page.document();
+    let node = NodeCast::from_ref(document.r());
 
     for candidate in node.traverse_preorder() {
-        if candidate.root().r().get_unique_id() == node_id {
+        if candidate.r().get_unique_id() == node_id {
             return candidate;
         }
     }
@@ -81,26 +80,25 @@ fn find_node_by_unique_id(page: &Rc<Page>, pipeline: PipelineId, node_id: String
 }
 
 pub fn handle_get_children(page: &Rc<Page>, pipeline: PipelineId, node_id: String, reply: Sender<Vec<NodeInfo>>) {
-    let parent = find_node_by_unique_id(&*page, pipeline, node_id).root();
+    let parent = find_node_by_unique_id(&*page, pipeline, node_id);
     let children = parent.r().children().map(|child| {
-        let child = child.root();
         child.r().summarize()
     }).collect();
     reply.send(children).unwrap();
 }
 
 pub fn handle_get_layout(page: &Rc<Page>, pipeline: PipelineId, node_id: String, reply: Sender<(f32, f32)>) {
-    let node = find_node_by_unique_id(&*page, pipeline, node_id).root();
-    let elem: JSRef<Element> = ElementCast::to_ref(node.r()).expect("should be getting layout of element");
-    let rect = elem.GetBoundingClientRect().root();
+    let node = find_node_by_unique_id(&*page, pipeline, node_id);
+    let elem = ElementCast::to_ref(node.r()).expect("should be getting layout of element");
+    let rect = elem.GetBoundingClientRect();
     let width = *rect.r().Width();
     let height = *rect.r().Height();
     reply.send((width, height)).unwrap();
 }
 
 pub fn handle_modify_attribute(page: &Rc<Page>, pipeline: PipelineId, node_id: String, modifications: Vec<Modification>) {
-    let node = find_node_by_unique_id(&*page, pipeline, node_id).root();
-    let elem: JSRef<Element> = ElementCast::to_ref(node.r()).expect("should be getting layout of element");
+    let node = find_node_by_unique_id(&*page, pipeline, node_id);
+    let elem = ElementCast::to_ref(node.r()).expect("should be getting layout of element");
 
     for modification in modifications.iter(){
         match modification.newValue {
@@ -114,7 +112,7 @@ pub fn handle_modify_attribute(page: &Rc<Page>, pipeline: PipelineId, node_id: S
 
 pub fn handle_wants_live_notifications(page: &Rc<Page>, pipeline_id: PipelineId, send_notifications: bool) {
     let page = get_page(&*page, pipeline_id);
-    let window = page.window().root();
+    let window = page.window();
     window.r().set_devtools_wants_updates(send_notifications);
 }
 
@@ -125,7 +123,7 @@ pub fn handle_set_timeline_markers(page: &Rc<Page>,
     for marker_type in &marker_types {
         match *marker_type {
             TimelineMarkerType::Reflow => {
-                let window = page.window().root();
+                let window = page.window();
                 window.r().set_devtools_timeline_marker(TimelineMarkerType::Reflow, reply.clone());
             }
             TimelineMarkerType::DOMEvent => {
@@ -138,7 +136,7 @@ pub fn handle_set_timeline_markers(page: &Rc<Page>,
 pub fn handle_drop_timeline_markers(page: &Rc<Page>,
                                     script_task: &ScriptTask,
                                     marker_types: Vec<TimelineMarkerType>) {
-    let window = page.window().root();
+    let window = page.window();
     for marker_type in &marker_types {
         match *marker_type {
             TimelineMarkerType::Reflow => {
@@ -153,6 +151,6 @@ pub fn handle_drop_timeline_markers(page: &Rc<Page>,
 
 pub fn handle_request_animation_frame(page: &Rc<Page>, id: PipelineId, callback: Box<Fn(f64, )>) {
     let page = page.find(id).expect("There is no such page");
-    let doc = page.document().root();
+    let doc = page.document();
     doc.r().request_animation_frame(callback);
 }

components/script/dom/activation.rs

@@ -4,7 +4,6 @@
 
 use dom::bindings::codegen::Bindings::EventBinding::EventMethods;
 use dom::bindings::codegen::InheritTypes::{EventCast, EventTargetCast};
-use dom::bindings::js::{JSRef, Temporary, OptionalRootable, Rootable};
 use dom::element::{Element, ActivationElementHelpers};
 use dom::event::{Event, EventHelpers, EventBubbles, EventCancelable};
 use dom::eventtarget::EventTarget;
@@ -15,7 +14,7 @@ use std::borrow::ToOwned;
 
 /// Trait for elements with defined activation behavior
 pub trait Activatable {
-    fn as_element(&self) -> Temporary<Element>;
+    fn as_element<'a>(&'a self) -> &'a Element;
 
     // Is this particular instance of the element activatable?
     fn is_instance_activatable(&self) -> bool;
@@ -27,32 +26,32 @@ pub trait Activatable {
     fn canceled_activation(&self);
 
     // https://html.spec.whatwg.org/multipage/#run-post-click-activation-steps
-    fn activation_behavior(&self, event: JSRef<Event>, target: JSRef<EventTarget>);
+    fn activation_behavior(&self, event: &Event, target: &EventTarget);
 
     // https://html.spec.whatwg.org/multipage/#implicit-submission
     fn implicit_submission(&self, ctrlKey: bool, shiftKey: bool, altKey: bool, metaKey: bool);
 
     // https://html.spec.whatwg.org/multipage/#run-synthetic-click-activation-steps
     fn synthetic_click_activation(&self, ctrlKey: bool, shiftKey: bool, altKey: bool, metaKey: bool) {
-        let element = self.as_element().root();
+        let element = self.as_element();
         // Step 1
-        if element.r().click_in_progress() {
+        if element.click_in_progress() {
             return;
         }
         // Step 2
-        element.r().set_click_in_progress(true);
+        element.set_click_in_progress(true);
         // Step 3
         self.pre_click_activation();
 
         // Step 4
         // https://html.spec.whatwg.org/multipage/#fire-a-synthetic-mouse-event
-        let win = window_from_node(element.r()).root();
-        let target: JSRef<EventTarget> = EventTargetCast::from_ref(element.r());
+        let win = window_from_node(element);
+        let target = EventTargetCast::from_ref(element);
         let mouse = MouseEvent::new(win.r(), "click".to_owned(),
                                     EventBubbles::DoesNotBubble, EventCancelable::NotCancelable, Some(win.r()), 1,
                                     0, 0, 0, 0, ctrlKey, shiftKey, altKey, metaKey,
-                                    0, None).root();
-        let event: JSRef<Event> = EventCast::from_ref(mouse.r());
+                                    0, None);
+        let event = EventCast::from_ref(mouse.r());
         event.fire(target);
 
         // Step 5
@@ -64,6 +63,6 @@ pub trait Activatable {
         }
 
         // Step 6
-        element.r().set_click_in_progress(false);
+        element.set_click_in_progress(false);
     }
 }

components/script/dom/attr.rs

@@ -6,11 +6,10 @@ use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::AttrBinding::{self, AttrMethods};
 use dom::bindings::codegen::InheritTypes::NodeCast;
 use dom::bindings::global::GlobalRef;
-use dom::bindings::js::{JS, JSRef, MutNullableHeap, Temporary};
-use dom::bindings::js::{OptionalRootable, Rootable, RootedReference};
+use dom::bindings::js::{JS, MutNullableHeap};
+use dom::bindings::js::{Root, RootedReference};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
 use dom::element::{Element, AttributeHandlers};
-use dom::node::Node;
 use dom::window::Window;
 use dom::virtualmethods::vtable_for;
 
@@ -125,21 +124,21 @@ pub struct Attr {
 
 impl Attr {
     fn new_inherited(local_name: Atom, value: AttrValue, name: Atom, namespace: Namespace,
-                     prefix: Option<Atom>, owner: Option<JSRef<Element>>) -> Attr {
+                     prefix: Option<Atom>, owner: Option<&Element>) -> Attr {
         Attr {
             reflector_: Reflector::new(),
             local_name: local_name,
             value: DOMRefCell::new(value),
             name: name,
             namespace: namespace,
             prefix: prefix,
-            owner: MutNullableHeap::new(owner.map(JS::from_rooted)),
+            owner: MutNullableHeap::new(owner.map(JS::from_ref)),
         }
     }
 
-    pub fn new(window: JSRef<Window>, local_name: Atom, value: AttrValue,
+    pub fn new(window: &Window, local_name: Atom, value: AttrValue,
                name: Atom, namespace: Namespace,
-               prefix: Option<Atom>, owner: Option<JSRef<Element>>) -> Temporary<Attr> {
+               prefix: Option<Atom>, owner: Option<&Element>) -> Root<Attr> {
         reflect_dom_object(
             box Attr::new_inherited(local_name, value, name, namespace, prefix, owner),
             GlobalRef::Window(window),
@@ -162,7 +161,7 @@ impl Attr {
     }
 }
 
-impl<'a> AttrMethods for JSRef<'a, Attr> {
+impl<'a> AttrMethods for &'a Attr {
     // https://dom.spec.whatwg.org/#dom-attr-localname
     fn LocalName(self) -> DOMString {
         (**self.local_name()).to_owned()
@@ -177,8 +176,7 @@ impl<'a> AttrMethods for JSRef<'a, Attr> {
     fn SetValue(self, value: DOMString) {
         match self.owner() {
             None => *self.value.borrow_mut() = AttrValue::String(value),
-            Some(o) => {
-                let owner = o.root();
+            Some(owner) => {
                 let value = owner.r().parse_attribute(&self.namespace, self.local_name(), value);
                 self.set_value(AttrSettingType::ReplacedAttr, value, owner.r());
             }
@@ -225,7 +223,7 @@ impl<'a> AttrMethods for JSRef<'a, Attr> {
     }
 
     // https://dom.spec.whatwg.org/#dom-attr-ownerelement
-    fn GetOwnerElement(self) -> Option<Temporary<Element>> {
+    fn GetOwnerElement(self) -> Option<Root<Element>> {
         self.owner()
     }
 
@@ -236,19 +234,19 @@ impl<'a> AttrMethods for JSRef<'a, Attr> {
 }
 
 pub trait AttrHelpers<'a> {
-    fn set_value(self, set_type: AttrSettingType, value: AttrValue, owner: JSRef<Element>);
+    fn set_value(self, set_type: AttrSettingType, value: AttrValue, owner: &Element);
     fn value(self) -> Ref<'a, AttrValue>;
     fn local_name(self) -> &'a Atom;
-    fn set_owner(self, owner: Option<JSRef<Element>>);
-    fn owner(self) -> Option<Temporary<Element>>;
+    fn set_owner(self, owner: Option<&Element>);
+    fn owner(self) -> Option<Root<Element>>;
     fn summarize(self) -> AttrInfo;
 }
 
-impl<'a> AttrHelpers<'a> for JSRef<'a, Attr> {
-    fn set_value(self, set_type: AttrSettingType, value: AttrValue, owner: JSRef<Element>) {
-        assert!(Some(owner) == self.owner().root().r());
+impl<'a> AttrHelpers<'a> for &'a Attr {
+    fn set_value(self, set_type: AttrSettingType, value: AttrValue, owner: &Element) {
+        assert!(Some(owner) == self.owner().r());
 
-        let node: JSRef<Node> = NodeCast::from_ref(owner);
+        let node = NodeCast::from_ref(owner);
         let namespace_is_null = self.namespace == ns!("");
 
         match set_type {
@@ -265,33 +263,33 @@ impl<'a> AttrHelpers<'a> for JSRef<'a, Attr> {
     }
 
     fn value(self) -> Ref<'a, AttrValue> {
-        self.extended_deref().value.borrow()
+        self.value.borrow()
     }
 
     fn local_name(self) -> &'a Atom {
-        &self.extended_deref().local_name
+        &self.local_name
     }
 
     /// Sets the owner element. Should be called after the attribute is added
     /// or removed from its older parent.
-    fn set_owner(self, owner: Option<JSRef<Element>>) {
+    fn set_owner(self, owner: Option<&Element>) {
         let ref ns = self.namespace;
-        match (self.owner().root().r(), owner) {
+        match (self.owner().r(), owner) {
             (None, Some(new)) => {
                 // Already in the list of attributes of new owner.
-                assert!(new.get_attribute(&ns, &self.local_name).root().r() == Some(self))
+                assert!(new.get_attribute(&ns, &self.local_name) == Some(Root::from_ref(self)))
             }
             (Some(old), None) => {
                 // Already gone from the list of attributes of old owner.
                 assert!(old.get_attribute(&ns, &self.local_name).is_none())
             }
             (old, new) => assert!(old == new)
         }
-        self.owner.set(owner.map(JS::from_rooted))
+        self.owner.set(owner.map(JS::from_ref))
     }
 
-    fn owner(self) -> Option<Temporary<Element>> {
-        self.owner.get().map(Temporary::from_rooted)
+    fn owner(self) -> Option<Root<Element>> {
+        self.owner.get().map(Root::from_rooted)
     }
 
     fn summarize(self) -> AttrInfo {
@@ -311,7 +309,7 @@ pub trait AttrHelpersForLayout {
     unsafe fn value_atom_forever(&self) -> Option<Atom>;
     unsafe fn value_tokens_forever(&self) -> Option<&'static [Atom]>;
     unsafe fn local_name_atom_forever(&self) -> Atom;
-    unsafe fn value(&self) -> &AttrValue;
+    unsafe fn value_for_layout(&self) -> &AttrValue;
 }
 
 #[allow(unsafe_code)]
@@ -354,7 +352,7 @@ impl AttrHelpersForLayout for Attr {
     }
 
     #[inline]
-    unsafe fn value(&self) -> &AttrValue {
+    unsafe fn value_for_layout(&self) -> &AttrValue {
         self.value.borrow_for_layout()
     }
 }

components/script/dom/bindings/callback.rs

@@ -5,7 +5,6 @@
 //! Base classes to work with IDL callbacks.
 
 use dom::bindings::global::global_object_for_js_object;
-use dom::bindings::js::JSRef;
 use dom::bindings::utils::Reflectable;
 use js::jsapi::{JSContext, JSObject, JS_WrapObject, IsCallable};
 use js::jsapi::{JS_GetProperty, JS_IsExceptionPending, JS_ReportPendingException};
@@ -124,7 +123,7 @@ impl CallbackInterface {
 
 /// Wraps the reflector for `p` into the compartment of `cx`.
 pub fn wrap_call_this_object<T: Reflectable>(cx: *mut JSContext,
-                                             p: JSRef<T>) -> *mut JSObject {
+                                             p: &T) -> *mut JSObject {
     let mut obj = RootedObject::new(cx, p.reflector().get_jsobject());
     assert!(!obj.ptr.is_null());
 
@@ -155,7 +154,6 @@ impl CallSetup {
     #[allow(unrooted_must_root)]
     pub fn new<T: CallbackContainer>(callback: T, handling: ExceptionHandling) -> CallSetup {
         let global = global_object_for_js_object(callback.callback());
-        let global = global.root();
         let cx = global.r().get_cx();
         unsafe { JS_BeginRequest(cx); }