dom: Remove duplicate methods from the DOM node type

src/servo/content/content_task.rs

@@ -2,31 +2,31 @@
  * 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/. */
 
-/*!
+/// The content task (also called the script task) is the main task that owns the DOM in memory,
-The content task is the main task that runs JavaScript and spawns layout
+/// runs JavaScript, and spawns parsing and layout tasks.
-tasks.
-*/
 
 use dom::bindings::utils::GlobalStaticData;
 use dom::document::Document;
-use dom::node::define_bindings;
 use dom::event::{Event, ResizeEvent, ReflowEvent};
+use dom::node::define_bindings;
 use dom::window::Window;
-use layout::layout_task;
 use layout::layout_task::{AddStylesheet, BuildData, BuildMsg, Damage, LayoutTask};
 use layout::layout_task::{MatchSelectorsDamage, NoDamage, ReflowDamage};
+use layout::layout_task;
 
 use core::cell::Cell;
 use core::comm::{Port, SharedChan};
-use core::pipes::select2i;
 use core::either;
-use core::task::{SingleThreaded, task};
 use core::io::{println, read_whole_file};
+use core::pipes::select2i;
 use core::ptr::null;
+use core::task::{SingleThreaded, task};
 use core::util::replace;
+use dom;
 use geom::size::Size2D;
 use gfx::resource::image_cache_task::ImageCacheTask;
 use gfx::resource::resource_task::ResourceTask;
+use html;
 use js::JSVAL_NULL;
 use js::global::{global_class, debug_fns};
 use js::glue::bindgen::RUST_JSVAL_TO_OBJECT;
@@ -34,10 +34,9 @@ use js::jsapi::JSContext;
 use js::jsapi::bindgen::{JS_CallFunctionValue, JS_GetContextPrivate};
 use js::rust::{Compartment, Cx};
 use jsrt = js::rust::rt;
+use servo_util::tree::TreeNodeRef;
 use std::net::url::Url;
 use url_to_str = std::net::url::to_str;
-use dom;
-use html;
 
 pub enum ControlMsg {
     ParseMsg(Url),
@@ -232,7 +231,7 @@ pub impl Content {
                                   ptr::to_mut_unsafe_ptr(&mut *self)); //FIXME store this safely
             let document = Document(root, Some(window));
 
-            do root.with_mut_node |node| {
+            do root.with_mutable_node |node| {
                 node.add_to_doc(document);
             }
 
@@ -312,24 +311,20 @@ pub impl Content {
         }
     }
 
-    /**
+    /// This method will wait until the layout task has completed its current action, join the
-       This method will wait until the layout task has completed its current action,
+    /// layout task, and then request a new layout run. It won't wait for the new layout
-       join the layout task, and then request a new layout run. It won't wait for the
+    /// computation to finish.
-       new layout computation to finish.
-    */
     fn relayout(&mut self, document: &Document, doc_url: &Url) {
         debug!("content: performing relayout");
 
-        // Now, join the layout so that they will see the latest
+        // Now, join the layout so that they will see the latest changes we have made.
-        // changes we have made.
         self.join_layout();
 
-        // Layout will let us know when it's done
+        // Layout will let us know when it's done.
         let (join_port, join_chan) = comm::stream();
         self.layout_join_port = Some(join_port);
 
-        // Send new document and relevant styles to layout
+        // Send new document and relevant styles to layout.
-
         let data = ~BuildData {
             node: document.root,
             url: copy *doc_url,
@@ -344,7 +339,8 @@ pub impl Content {
         debug!("content: layout forked");
     }
 
-     fn query_layout(&mut self, query: layout_task::LayoutQuery) -> layout_task::LayoutQueryResponse {
+     fn query_layout(&mut self, query: layout_task::LayoutQuery)
+                     -> layout_task::LayoutQueryResponse {
          //self.relayout(self.document.get(), &(copy self.doc_url).get());
          self.join_layout();
 
@@ -378,7 +374,7 @@ pub impl Content {
           ReflowEvent => {
             debug!("content got reflow event");
             self.damage.add(MatchSelectorsDamage);
-            match copy self.document {
+            match /*bad*/ copy self.document {
                 None => {
                     // Nothing to do.
                 }

src/servo/dom/bindings/node.rs

@@ -18,6 +18,7 @@ use js::jsval::{INT_TO_JSVAL};
 use js::rust::{Compartment, jsobj};
 use js::{JSPROP_ENUMERATE, JSPROP_SHARED, JSVAL_NULL};
 use js::{JS_THIS_OBJECT, JSPROP_NATIVE_ACCESSORS};
+use servo_util::tree::TreeNodeRef;
 
 pub fn init(compartment: @mut Compartment) {
     let obj = utils::define_empty_prototype(~"Node", None, compartment);
@@ -80,7 +81,7 @@ extern fn getFirstChild(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool
         }
 
         let node = unwrap(obj);
-        let rval = do node.with_mut_node |node| {
+        let rval = do node.with_mutable_node |node| {
             node.getFirstChild()
         };
         match rval {
@@ -102,7 +103,7 @@ extern fn getNextSibling(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBoo
         }
 
         let node = unwrap(obj);
-        let rval = do node.with_mut_node |node| {
+        let rval = do node.with_mutable_node |node| {
             node.getNextSibling()
         };
         match rval {
@@ -127,19 +128,19 @@ impl Node {
 
     fn getNextSibling(&mut self) -> Option<&mut AbstractNode> {
         match self.next_sibling {
-          // transmute because the compiler can't deduce that the reference
+            // transmute because the compiler can't deduce that the reference
-          // is safe outside of with_mut_node blocks.
+            // is safe outside of with_mutable_node blocks.
-          Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
+            Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
-          None => None
+            None => None
         }
     }
 
     fn getFirstChild(&mut self) -> Option<&mut AbstractNode> {
         match self.first_child {
-          // transmute because the compiler can't deduce that the reference
+            // transmute because the compiler can't deduce that the reference
-          // is safe outside of with_mut_node blocks.
+            // is safe outside of with_mutable_node blocks.
-          Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
+            Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
-          None => None
+            None => None
         }
     }
  }
@@ -152,7 +153,7 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool {
         }
 
         let node = unwrap(obj);
-        let rval = do node.with_imm_node |node| {
+        let rval = do node.with_immutable_node |node| {
             node.getNodeType()
         };
         *vp = INT_TO_JSVAL(rval);
@@ -162,8 +163,10 @@ extern fn getNodeType(cx: *JSContext, _argc: c_uint, vp: *mut JSVal) -> JSBool {
 
 impl CacheableWrapper for AbstractNode {
     fn get_wrappercache(&mut self) -> &mut WrapperCache {
-        do self.with_mut_node |n| {
+        do self.with_mutable_node |node| {
-            unsafe { cast::transmute(&n.wrapper) }
+            unsafe {
+                cast::transmute(&node.wrapper)
+            }
         }
     }
 

src/servo/dom/document.rs

@@ -11,7 +11,7 @@ use dom::node::AbstractNode;
 use dom::window::Window;
 
 use js::jsapi::bindgen::{JS_AddObjectRoot, JS_RemoveObjectRoot};
-use servo_util::tree::TreeUtils;
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct Document {
     root: AbstractNode,
@@ -27,7 +27,7 @@ pub fn Document(root: AbstractNode,
         window: window
     };
     let compartment = global_content().compartment.get();
-    do root.with_imm_node |node| {
+    do root.with_immutable_node |node| {
         assert!(node.wrapper.get_wrapper().is_not_null());
         let rootable = node.wrapper.get_rootable();
         JS_AddObjectRoot(compartment.cx.ptr, rootable);
@@ -40,7 +40,7 @@ pub fn Document(root: AbstractNode,
 impl Drop for Document {
     fn finalize(&self) {
         let compartment = global_content().compartment.get();
-        do self.root.with_imm_node |node| {
+        do self.root.with_immutable_node |node| {
             assert!(node.wrapper.get_wrapper().is_not_null());
             let rootable = node.wrapper.get_rootable();
             JS_RemoveObjectRoot(compartment.cx.ptr, rootable);

src/servo/dom/node.rs

@@ -205,13 +205,13 @@ impl TreeNode<AbstractNode> for Node {
 }
 
 impl TreeNodeRef<Node> for AbstractNode {
-    // FIXME: The duplication between `with_imm_node` and `with_immutable_node` is ugly.
+    // FIXME: The duplication between `with_immutable_node` and `with_immutable_node` is ugly.
     fn with_immutable_node<R>(&self, callback: &fn(&Node) -> R) -> R {
-        self.with_imm_node(callback)
+        self.transmute(callback)
     }
 
     fn with_mutable_node<R>(&self, callback: &fn(&mut Node) -> R) -> R {
-        self.with_mut_node(callback)
+        self.transmute_mut(callback)
     }
 }
 
@@ -220,44 +220,44 @@ impl AbstractNode {
 
     /// Returns the type ID of this node. Fails if this node is borrowed mutably.
     pub fn type_id(self) -> NodeTypeId {
-        self.with_imm_node(|n| n.type_id)
+        self.with_immutable_node(|n| n.type_id)
     }
 
     /// Returns the parent node of this node. Fails if this node is borrowed mutably.
     pub fn parent_node(self) -> Option<AbstractNode> {
-        self.with_imm_node(|n| n.parent_node)
+        self.with_immutable_node(|n| n.parent_node)
     }
 
     /// Returns the first child of this node. Fails if this node is borrowed mutably.
     pub fn first_child(self) -> Option<AbstractNode> {
-        self.with_imm_node(|n| n.first_child)
+        self.with_immutable_node(|n| n.first_child)
     }
 
     /// Returns the last child of this node. Fails if this node is borrowed mutably.
     pub fn last_child(self) -> Option<AbstractNode> {
-        self.with_imm_node(|n| n.last_child)
+        self.with_immutable_node(|n| n.last_child)
     }
 
     /// Returns the previous sibling of this node. Fails if this node is borrowed mutably.
     pub fn prev_sibling(self) -> Option<AbstractNode> {
-        self.with_imm_node(|n| n.prev_sibling)
+        self.with_immutable_node(|n| n.prev_sibling)
     }
 
     /// Returns the next sibling of this node. Fails if this node is borrowed mutably.
     pub fn next_sibling(self) -> Option<AbstractNode> {
-        self.with_imm_node(|n| n.next_sibling)
+        self.with_immutable_node(|n| n.next_sibling)
     }
 
     // NB: You must not call these if you are not layout. We should do something with scoping to
     // ensure this.
     pub fn layout_data(self) -> @mut LayoutData {
-        self.with_imm_node(|n| n.layout_data.get())
+        self.with_immutable_node(|n| n.layout_data.get())
     }
     pub fn has_layout_data(self) -> bool {
-        self.with_imm_node(|n| n.layout_data.is_some())
+        self.with_immutable_node(|n| n.layout_data.is_some())
     }
     pub fn set_layout_data(self, data: @mut LayoutData) {
-        self.with_mut_node(|n| n.layout_data = Some(data))
+        self.with_mutable_node(|n| n.layout_data = Some(data))
     }
 
     //
@@ -290,14 +290,6 @@ impl AbstractNode {
         }
     }
 
-    pub fn with_imm_node<R>(self, f: &fn(&Node) -> R) -> R {
-        self.transmute(f)
-    }
-
-    pub fn with_mut_node<R>(self, f: &fn(&mut Node) -> R) -> R {
-        self.transmute_mut(f)
-    }
-
     pub fn is_text(self) -> bool {
         self.type_id() == TextNodeTypeId
     }
@@ -408,12 +400,11 @@ impl Node {
         self.owner_doc = Some(doc);
         let mut node = self.first_child;
         while node.is_some() {
-            node.get().traverse_preorder(|n| {
+            for node.get().traverse_preorder |node| {
-                do n.with_mut_node |n| {
+                do node.with_mutable_node |node_data| {
-                    n.owner_doc = Some(doc);
+                    node_data.owner_doc = Some(doc);
                 }
-                true
+            };
-            });
             node = node.get().next_sibling();
         }
     }