dom: Document the node structure better and remove the node pointer stitching routines

4 files changed, 94 insertions, 119 deletions

diff --git a/src/servo/dom/node.rs b/src/servo/dom/node.rs
index 8f8ffe45ed2..5b97149aa1d 100644
--- a/src/servo/dom/node.rs
+++ b/src/servo/dom/node.rs
@@ -2,9 +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/. */
 
-//
-// The core DOM types. Defines the basic DOM hierarchy as well as all the HTML elements.
-//
+//! The core DOM types. Defines the basic DOM hierarchy as well as all the HTML elements.
 
 use content::content_task::global_content;
 use dom::bindings::codegen;
@@ -36,28 +34,47 @@ pub struct AbstractNode {
 }
 
 impl Eq for AbstractNode {
-    fn eq(&self, other: &AbstractNode) -> bool { self.obj == other.obj }
-    fn ne(&self, other: &AbstractNode) -> bool { self.obj != other.obj }
+    fn eq(&self, other: &AbstractNode) -> bool {
+        self.obj == other.obj
+    }
+    fn ne(&self, other: &AbstractNode) -> bool {
+        self.obj != other.obj
+    }
 }
 
+/// An HTML node.
 pub struct Node {
+    /// The JavaScript wrapper for this node.
     wrapper: WrapperCache,
+
+    /// The type of node that this is.
     type_id: NodeTypeId,
 
     abstract: Option<AbstractNode>,
 
+    /// The parent of this node.
     parent_node: Option<AbstractNode>,
+
+    /// The first child of this node.
     first_child: Option<AbstractNode>,
+
+    /// The last child of this node.
     last_child: Option<AbstractNode>,
+
+    /// The next sibling of this node.
     next_sibling: Option<AbstractNode>,
+
+    /// The previous sibling of this node.
     prev_sibling: Option<AbstractNode>,
 
+    /// The document that this node belongs to.
     owner_doc: Option<@mut Document>,
 
-    // You must not touch this if you are not layout.
+    /// Layout information. You must not touch this if you are not layout.
     priv layout_data: Option<@mut LayoutData>
 }
 
+/// The different types of nodes.
 #[deriving(Eq)]
 pub enum NodeTypeId {
     DoctypeNodeTypeId,
@@ -70,12 +87,17 @@ pub enum NodeTypeId {
 // Auxiliary layout data
 //
 
+/// Data that layout associates with a node.
 pub struct LayoutData {
+    /// The results of CSS styling for this node.
     style: Option<CompleteSelectResults>,
+
+    /// The CSS flow that this node is associated with.
     flow: Option<FlowContext>,
 }
 
 impl LayoutData {
+    /// Creates new layout data.
     pub fn new() -> LayoutData {
         LayoutData {
             style: None,
@@ -88,6 +110,7 @@ impl LayoutData {
 // Basic node types
 //
 
+/// The `DOCTYPE` tag.
 pub struct Doctype {
     parent: Node,
     name: ~str,
@@ -97,6 +120,7 @@ pub struct Doctype {
 }
 
 impl Doctype {
+    /// Creates a new `DOCTYPE` tag.
     pub fn new(name: ~str,
                public_id: Option<~str>,
                system_id: Option<~str>,
@@ -112,11 +136,13 @@ impl Doctype {
     }
 }
 
+/// An HTML comment.
 pub struct Comment {
     parent: CharacterData,
 }
 
 impl Comment {
+    /// Creates a new HTML comment.
     pub fn new(text: ~str) -> Comment {
         Comment {
             parent: CharacterData::new(CommentNodeTypeId, text)
@@ -124,11 +150,13 @@ impl Comment {
     }
 }
 
+/// An HTML text node.
 pub struct Text {
     parent: CharacterData,
 }
 
 impl Text {
+    /// Creates a new HTML text node.
     pub fn new(text: ~str) -> Text {
         Text {
             parent: CharacterData::new(TextNodeTypeId, text)
@@ -187,118 +215,56 @@ impl TreeNodeRef<Node> for AbstractNode {
     }
 }
 
-pub impl AbstractNode {
+impl AbstractNode {
     // Convenience accessors
 
-    /// Returns the type ID of this node.
-    fn type_id(self)         -> NodeTypeId           { self.with_imm_node(|n| n.type_id)      }
+    /// 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)
+    }
 
-    /// Returns the parent node of this node.
-    fn parent_node(self)     -> Option<AbstractNode> { self.with_imm_node(|n| n.parent_node)  }
+    /// 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)
+    }
 
-    /// Returns the first child of this node.
-    fn first_child(self)     -> Option<AbstractNode> { self.with_imm_node(|n| n.first_child)  }
+    /// 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)
+    }
 
-    /// Returns the last child of this node.
-    fn last_child(self)      -> Option<AbstractNode> { self.with_imm_node(|n| n.last_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)
+    }
 
-    /// Returns the previous sibling of this node.
-    fn prev_sibling(self)    -> Option<AbstractNode> { self.with_imm_node(|n| n.prev_sibling) }
+    /// 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)
+    }
 
-    /// Returns the next sibling of this node.
-    fn next_sibling(self)    -> Option<AbstractNode> { self.with_imm_node(|n| n.next_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)
+    }
 
     // NB: You must not call these if you are not layout. We should do something with scoping to
     // ensure this.
-    fn layout_data(self)     -> @mut LayoutData {
+    pub fn layout_data(self) -> @mut LayoutData {
         self.with_imm_node(|n| n.layout_data.get())
     }
-    fn has_layout_data(self) -> bool {
+    pub fn has_layout_data(self) -> bool {
         self.with_imm_node(|n| n.layout_data.is_some())
     }
-    fn set_layout_data(self, data: @mut LayoutData) {
+    pub fn set_layout_data(self, data: @mut LayoutData) {
         self.with_mut_node(|n| n.layout_data = Some(data))
     }
 
     //
-    // Tree operations
-    //
-    // FIXME: Fold this into util::tree.
-    //
-
-    fn is_leaf(self) -> bool { self.first_child().is_none() }
-
-    // Invariant: `child` is disconnected from the document.
-    fn append_child(self, child: AbstractNode) {
-        assert!(self != child);
-
-        do self.with_mut_node |parent_n| {
-            do child.with_mut_node |child_n| {
-                assert!(child_n.parent_node.is_none());
-                assert!(child_n.prev_sibling.is_none());
-                assert!(child_n.next_sibling.is_none());
-
-                child_n.parent_node = Some(self);
-
-                match parent_n.last_child {
-                    None => parent_n.first_child = Some(child),
-                    Some(last_child) => {
-                        do last_child.with_mut_node |last_child_n| {
-                            assert!(last_child_n.next_sibling.is_none());
-                            last_child_n.next_sibling = Some(child);
-                        }
-                    }
-                }
-
-                child_n.prev_sibling = parent_n.last_child;
-                parent_n.last_child = Some(child);
-            }
-        }
-    }
-
-    //
-    // Tree traversal
-    //
-    // FIXME: Fold this into util::tree.
-    //
-
-    fn each_child(self, f: &fn(AbstractNode) -> bool) {
-        let mut current_opt = self.first_child();
-        while !current_opt.is_none() {
-            let current = current_opt.get();
-            if !f(current) {
-                break;
-            }
-            current_opt = current.next_sibling();
-        }
-    }
-
-    fn traverse_preorder(self, f: &fn(AbstractNode) -> bool) -> bool {
-        if !f(self) {
-            return false;
-        }
-        for self.each_child |kid| {
-            if !kid.traverse_preorder(f) {
-                return false;
-            }
-        }
-        true
-    }
-
-    fn traverse_postorder(self, f: &fn(AbstractNode) -> bool) -> bool {
-        for self.each_child |kid| {
-            if !kid.traverse_postorder(f) {
-                return false;
-            }
-        }
-        f(self)
-    }
-
-    //
     // Downcasting borrows
     //
 
-    fn transmute<T, R>(self, f: &fn(&T) -> R) -> R {
+    pub fn transmute<T, R>(self, f: &fn(&T) -> R) -> R {
         unsafe {
             let node_box: *mut bindings::utils::rust_box<Node> = transmute(self.obj);
             let node = &mut (*node_box).payload;
@@ -311,7 +277,7 @@ pub impl AbstractNode {
         }
     }
 
-    fn transmute_mut<T, R>(self, f: &fn(&mut T) -> R) -> R {
+    pub fn transmute_mut<T, R>(self, f: &fn(&mut T) -> R) -> R {
         unsafe {
             let node_box: *mut bindings::utils::rust_box<Node> = transmute(self.obj);
             let node = &mut (*node_box).payload;
@@ -324,25 +290,27 @@ pub impl AbstractNode {
         }
     }
 
-    fn with_imm_node<R>(self, f: &fn(&Node) -> R) -> R {
+    pub fn with_imm_node<R>(self, f: &fn(&Node) -> R) -> R {
         self.transmute(f)
     }
 
-    fn with_mut_node<R>(self, f: &fn(&mut Node) -> R) -> R {
+    pub fn with_mut_node<R>(self, f: &fn(&mut Node) -> R) -> R {
         self.transmute_mut(f)
     }
 
-    fn is_text(self) -> bool { self.type_id() == TextNodeTypeId }
+    pub fn is_text(self) -> bool {
+        self.type_id() == TextNodeTypeId
+    }
 
     // FIXME: This should be doing dynamic borrow checking for safety.
-    fn with_imm_text<R>(self, f: &fn(&Text) -> R) -> R {
+    pub fn with_imm_text<R>(self, f: &fn(&Text) -> R) -> R {
         if !self.is_text() {
             fail!(~"node is not text");
         }
         self.transmute(f)
     }
 
-    fn is_element(self) -> bool {
+    pub fn is_element(self) -> bool {
         match self.type_id() {
             ElementNodeTypeId(*) => true,
             _ => false
@@ -350,7 +318,7 @@ pub impl AbstractNode {
     }
 
     // FIXME: This should be doing dynamic borrow checking for safety.
-    fn with_imm_element<R>(self, f: &fn(&Element) -> R) -> R {
+    pub fn with_imm_element<R>(self, f: &fn(&Element) -> R) -> R {
         if !self.is_element() {
             fail!(~"node is not an element");
         }
@@ -358,40 +326,40 @@ pub impl AbstractNode {
     }
 
     // FIXME: This should be doing dynamic borrow checking for safety.
-    fn as_mut_element<R>(self, f: &fn(&mut Element) -> R) -> R {
+    pub fn as_mut_element<R>(self, f: &fn(&mut Element) -> R) -> R {
         if !self.is_element() {
             fail!(~"node is not an element");
         }
         self.transmute_mut(f)
     }
 
-    fn is_image_element(self) -> bool {
+    pub fn is_image_element(self) -> bool {
         self.type_id() == ElementNodeTypeId(HTMLImageElementTypeId)
     }
 
-    fn with_imm_image_element<R>(self, f: &fn(&HTMLImageElement) -> R) -> R {
+    pub fn with_imm_image_element<R>(self, f: &fn(&HTMLImageElement) -> R) -> R {
         if !self.is_image_element() {
             fail!(~"node is not an image element");
         }
         self.transmute(f)
     }
 
-    fn with_mut_image_element<R>(self, f: &fn(&mut HTMLImageElement) -> R) -> R {
+    pub fn with_mut_image_element<R>(self, f: &fn(&mut HTMLImageElement) -> R) -> R {
         if !self.is_image_element() {
             fail!(~"node is not an image element");
         }
         self.transmute_mut(f)
     }
 
-    fn is_style_element(self) -> bool {
+    pub fn is_style_element(self) -> bool {
         self.type_id() == ElementNodeTypeId(HTMLStyleElementTypeId)
     }
 
-    unsafe fn raw_object(self) -> *mut Node {
+    pub unsafe fn raw_object(self) -> *mut Node {
         self.obj
     }
 
-    fn from_raw(raw: *mut Node) -> AbstractNode {
+    pub fn from_raw(raw: *mut Node) -> AbstractNode {
         AbstractNode {
             obj: raw
         }
diff --git a/src/servo/html/hubbub_html_parser.rs b/src/servo/html/hubbub_html_parser.rs
index 2a7f37a2113..fef7c6584a6 100644
--- a/src/servo/html/hubbub_html_parser.rs
+++ b/src/servo/html/hubbub_html_parser.rs
@@ -14,8 +14,9 @@ use util::task::spawn_conversation;
 use core::cell::Cell;
 use core::comm::{Chan, Port, SharedChan};
 use core::str::eq_slice;
-use servo_util::url::make_url;
 use hubbub::hubbub;
+use servo_util::tree::TreeUtils;
+use servo_util::url::make_url;
 use std::net::url::Url;
 use std::net::url;
 
@@ -336,7 +337,7 @@ pub fn parse_html(url: Url,
                 debug!("append child %x %x", cast::transmute(parent), cast::transmute(child));
                 let parent: AbstractNode = NodeWrapping::from_hubbub_node(parent);
                 let child: AbstractNode = NodeWrapping::from_hubbub_node(child);
-                parent.append_child(child);
+                parent.add_child(child);
                 append_hook(parent, child);
             }
             child
diff --git a/src/servo/layout/aux.rs b/src/servo/layout/aux.rs
index b803a036bac..f56f204e0d1 100644
--- a/src/servo/layout/aux.rs
+++ b/src/servo/layout/aux.rs
@@ -2,12 +2,12 @@
  * 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/. */
 
-/**
-Code for managing the DOM aux pointer
-*/
+//! Code for managing the layout data in the DOM.
 
 use dom::node::{AbstractNode, LayoutData};
 
+use servo_util::tree::TreeUtils;
+
 pub trait LayoutAuxMethods {
     fn initialize_layout_data(self) -> Option<@mut LayoutData>;
     fn initialize_style_for_subtree(self, refs: &mut ~[@mut LayoutData]);
@@ -36,5 +36,5 @@ impl LayoutAuxMethods for AbstractNode {
             }
         };
     }
-
 }
+
diff --git a/src/servo/layout/box_builder.rs b/src/servo/layout/box_builder.rs
index d0fb0deb55d..8bc838b18d1 100644
--- a/src/servo/layout/box_builder.rs
+++ b/src/servo/layout/box_builder.rs
@@ -121,6 +121,8 @@ impl BoxGenerator {
         // depending on flow, make a box for this node.
         match self.flow {
             InlineFlow(inline) => {
+                use servo_util::tree::TreeUtils;    // For `is_leaf()`.
+
                 let mut inline = &mut *inline;
                 let node_range_start = inline.boxes.len();
                 self.range_stack.push(node_range_start);
@@ -336,8 +338,12 @@ pub impl LayoutTreeBuilder {
         debug!("point b: %s", cur_node.debug_str());
 
         // recurse on child nodes.
-        for cur_node.each_child |child_node| {
-            self.construct_recursively(layout_ctx, child_node, &mut this_ctx);
+        {
+            use servo_util::tree::TreeUtils;    // For `each_child()`.
+
+            for cur_node.each_child |child_node| {
+                self.construct_recursively(layout_ctx, child_node, &mut this_ctx);
+            }
         }
 
         this_ctx.default_collector.pop_node(layout_ctx, self, cur_node);