Merge pull request #416 from pcwalton/refactor-flow

Refactor flow

src/servo-util/servo_util.rc

@@ -13,6 +13,7 @@ extern mod std;
 pub mod cache;
 pub mod range;
 pub mod time;
+pub mod tree;
 pub mod url;
 pub mod vec;
 

src/servo-util/tree.rs

@@ -0,0 +1,171 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+//! Helper functions for garbage collected doubly-linked trees.
+
+/// The basic trait. This function is meant to encapsulate a clonable reference to a tree node.
+pub trait TreeNodeRef<N> : Clone {
+    /// Borrows this node as immutable.
+    fn with_imm_node<R>(&self, callback: &fn(&N) -> R) -> R;
+
+    /// Borrows this node as mutable.
+    fn with_mut_node<R>(&self, callback: &fn(&mut N) -> R) -> R;
+}
+
+/// The contents of a tree node.
+pub trait TreeNode<NR> {
+    /// Returns the parent of this node.
+    fn parent_node(&self) -> Option<NR>;
+
+    /// Returns the first child of this node.
+    fn first_child(&self) -> Option<NR>;
+
+    /// Returns the last child of this node.
+    fn last_child(&self) -> Option<NR>;
+
+    /// Returns the previous sibling of this node.
+    fn prev_sibling(&self) -> Option<NR>;
+
+    /// Returns the next sibling of this node.
+    fn next_sibling(&self) -> Option<NR>;
+
+    /// Sets the parent of this node.
+    fn set_parent_node(&mut self, new_parent: Option<NR>);
+
+    /// Sets the first child of this node.
+    fn set_first_child(&mut self, new_first_child: Option<NR>);
+
+    /// Sets the last child of this node.
+    fn set_last_child(&mut self, new_last_child: Option<NR>);
+
+    /// Sets the previous sibling of this node.
+    fn set_prev_sibling(&mut self, new_prev_sibling: Option<NR>);
+
+    /// Sets the next sibling of this node.
+    fn set_next_sibling(&mut self, new_next_sibling: Option<NR>);
+}
+
+/// A set of helper functions useful for operating on trees.
+pub trait TreeUtils {
+    /// Returns true if this node is disconnected from the tree or has no children.
+    fn is_leaf(&self) -> bool;
+
+    /// Adds a new child to the end of this node's list of children.
+    ///
+    /// Fails unless `new_child` is disconnected from the tree.
+    fn add_child(&self, new_child: Self);
+
+    /// Removes the given child from this node's list of children.
+    ///
+    /// Fails unless `child` is a child of this node. (FIXME: This is not yet checked.)
+    fn remove_child(&self, child: Self);
+
+    /// Iterates over all children of this node.
+    fn each_child(&self, callback: &fn(Self) -> bool);
+
+    /// Iterates over this node and all its descendants, in preorder.
+    fn traverse_preorder(&self, callback: &fn(Self) -> bool) -> bool;
+
+    /// Iterates over this node and all its descendants, in postorder.
+    fn traverse_postorder(&self, callback: &fn(Self) -> bool) -> bool;
+}
+
+impl<NR:TreeNodeRef<N>,N:TreeNode<NR>> TreeUtils for NR {
+    fn is_leaf(&self) -> bool {
+        do self.with_imm_node |this_node| {
+            this_node.first_child().is_none()
+        }
+    }
+
+    fn add_child(&self, new_child: NR) {
+        do self.with_mut_node |this_node| {
+            do new_child.with_mut_node |new_child_node| {
+                assert!(new_child_node.parent_node().is_none());
+                assert!(new_child_node.prev_sibling().is_none());
+                assert!(new_child_node.next_sibling().is_none());
+
+                match this_node.last_child() {
+                    None => this_node.set_first_child(Some(new_child.clone())),
+                    Some(last_child) => {
+                        do last_child.with_mut_node |last_child_node| {
+                            assert!(last_child_node.next_sibling().is_none());
+                            last_child_node.set_next_sibling(Some(new_child.clone()));
+                            new_child_node.set_prev_sibling(Some(last_child.clone()));
+                        }
+                    }
+                }
+
+                this_node.set_last_child(Some(new_child.clone()));
+                new_child_node.set_parent_node(Some((*self).clone()));
+            }
+        }
+    }
+
+    fn remove_child(&self, child: NR) {
+        do self.with_mut_node |this_node| {
+            do child.with_mut_node |child_node| {
+                assert!(child_node.parent_node().is_some());
+
+                match child_node.prev_sibling() {
+                    None => this_node.set_first_child(child_node.next_sibling()),
+                    Some(prev_sibling) => {
+                        do prev_sibling.with_mut_node |prev_sibling_node| {
+                            prev_sibling_node.set_next_sibling(child_node.next_sibling());
+                        }
+                    }
+                }
+
+                match child_node.next_sibling() {
+                    None => this_node.set_last_child(child_node.prev_sibling()),
+                    Some(next_sibling) => {
+                        do next_sibling.with_mut_node |next_sibling_node| {
+                            next_sibling_node.set_prev_sibling(child_node.prev_sibling());
+                        }
+                    }
+                }
+
+                child_node.set_prev_sibling(None);
+                child_node.set_next_sibling(None);
+                child_node.set_parent_node(None);
+            }
+        }
+    }
+
+    fn each_child(&self, callback: &fn(NR) -> bool) {
+        let mut maybe_current = self.with_imm_node(|n| n.first_child());
+        while !maybe_current.is_none() {
+            let current = maybe_current.get_ref().clone();
+            if !callback(current.clone()) {
+                break;
+            }
+
+            maybe_current = current.with_imm_node(|n| n.next_sibling());
+        }
+    }
+
+    fn traverse_preorder(&self, callback: &fn(NR) -> bool) -> bool {
+        if !callback((*self).clone()) {
+            return false;
+        }
+
+        for self.each_child |kid| {
+            if !kid.traverse_preorder(callback) {
+                return false;
+            }
+        }
+
+        true
+    }
+
+    fn traverse_postorder(&self, callback: &fn(NR) -> bool) -> bool {
+        for self.each_child |kid| {
+            if !kid.traverse_postorder(callback) {
+                return false;
+            }
+        }
+
+        callback((*self).clone())
+    }
+}
+

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),
@@ -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/css/matching.rs

@@ -10,6 +10,8 @@ use dom::node::AbstractNode;
 use newcss::complete::CompleteSelectResults;
 use newcss::select::{SelectCtx, SelectResults};
 
+use servo_util::tree::TreeUtils;
+
 pub trait MatchMethods {
     fn restyle_subtree(&self, select_ctx: &SelectCtx);
 }

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);
@@ -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_mut_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_mut_node blocks.
-          Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
+            Some(ref mut n) => Some(unsafe { cast::transmute(n) }),
-          None => None
+            None => None
         }
     }
  }
@@ -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_mut_node |node| {
-            unsafe { cast::transmute(&n.wrapper) }
+            unsafe {
+                cast::transmute(&node.wrapper)
+            }
         }
     }
 

src/servo/dom/characterdata.rs

@@ -1,3 +1,9 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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/. */
+
+//! DOM bindings for `CharacterData`.
+
 use dom::bindings::utils::{DOMString, null_string, str};
 use dom::node::{Node, NodeTypeId};
 
@@ -8,50 +14,51 @@ pub struct CharacterData {
     data: DOMString
 }
 
-pub impl CharacterData {
+impl CharacterData {
-    fn new(id: NodeTypeId, data: ~str) -> CharacterData {
+    pub fn new(id: NodeTypeId, data: ~str) -> CharacterData {
         CharacterData {
             parent: Node::new(id),
             data: str(data)
         }
     }
     
-    fn GetData(&self) -> DOMString {
+    pub fn GetData(&self) -> DOMString {
         copy self.data
     }
 
-    fn SetData(&mut self, arg: DOMString) {
+    pub fn SetData(&mut self, arg: DOMString) {
         self.data = arg;
     }
 
-    fn Length(&self) -> u32 {
+    pub fn Length(&self) -> u32 {
         match self.data {
-          str(ref s) => s.len() as u32,
+            str(ref s) => s.len() as u32,
-          null_string => 0
+            null_string => 0
         }
     }
 
-    fn SubstringData(&self, offset: u32, count: u32) -> DOMString {
+    pub fn SubstringData(&self, offset: u32, count: u32) -> DOMString {
         match self.data {
-          str(ref s) => str(s.slice(offset as uint, count as uint).to_str()),
+            str(ref s) => str(s.slice(offset as uint, count as uint).to_str()),
-          null_string => null_string
+            null_string => null_string
         }
     }
 
-    fn AppendData(&mut self, arg: DOMString) {
+    pub fn AppendData(&mut self, arg: DOMString) {
         let s = self.data.to_str();
         self.data = str(str::append(s, arg.to_str()));
     }
 
-    fn InsertData(&mut self, _offset: u32, _arg: DOMString) {
+    pub fn InsertData(&mut self, _offset: u32, _arg: DOMString) {
-        fail!(~"nyi")
+        fail!("CharacterData::InsertData() is unimplemented")
     }
 
-    fn DeleteData(&mut self, _offset: u32, _count: u32) {
+    pub fn DeleteData(&mut self, _offset: u32, _count: u32) {
-        fail!(~"nyi")
+        fail!("CharacterData::DeleteData() is unimplemented")
     }
 
-    fn ReplaceData(&mut self, _offset: u32, _count: u32, _arg: DOMString) {
+    pub fn ReplaceData(&mut self, _offset: u32, _count: u32, _arg: DOMString) {
-        fail!(~"nyi")
+        fail!("CharacterData::ReplaceData() is unimplemented")
     }
 }
+

src/servo/dom/document.rs

@@ -11,6 +11,7 @@ use dom::node::AbstractNode;
 use dom::window::Window;
 
 use js::jsapi::bindgen::{JS_AddObjectRoot, JS_RemoveObjectRoot};
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct Document {
     root: AbstractNode,
@@ -69,4 +70,4 @@ pub impl Document {
             chan.send(ReflowEvent)
         };
     }
-}
+}

src/servo/dom/domparser.rs

@@ -1,6 +1,10 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * 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 content::content_task::global_content;
-use dom::bindings::utils::{DOMString, ErrorResult, WrapperCache, CacheableWrapper};
 use dom::bindings::codegen::DOMParserBinding;
+use dom::bindings::utils::{DOMString, ErrorResult, WrapperCache, CacheableWrapper};
 use dom::document::Document;
 use dom::element::{Element, HTMLHtmlElement, HTMLHtmlElementTypeId};
 use dom::node::Node;
@@ -11,12 +15,13 @@ pub struct DOMParser {
     wrapper: WrapperCache
 }
 
-pub impl DOMParser {
+impl DOMParser {
-    fn new(owner: @mut Window) -> @mut DOMParser {
+    pub fn new(owner: @mut Window) -> @mut DOMParser {
         let parser = @mut DOMParser {
             owner: owner,
             wrapper: WrapperCache::new()
         };
+
         let cx = global_content().compartment.get().cx.ptr;
         let cache = owner.get_wrappercache();
         let scope = cache.get_wrapper();
@@ -24,13 +29,23 @@ pub impl DOMParser {
         parser
     }
 
-    fn Constructor(owner: @mut Window, _rv: &mut ErrorResult) -> @mut DOMParser {
+    pub fn Constructor(owner: @mut Window, _rv: &mut ErrorResult) -> @mut DOMParser {
         DOMParser::new(owner)
     }
 
-    fn ParseFromString(&self, _s: DOMString, _type_: DOMParserBinding::SupportedType, _rv: &mut ErrorResult) -> @mut Document {
+    pub fn ParseFromString(&self,
-        let root = ~HTMLHtmlElement { parent: Element::new(HTMLHtmlElementTypeId, ~"html") };
+                           _s: DOMString,
-        let root = unsafe { Node::as_abstract_node(root) };
+                           _type: DOMParserBinding::SupportedType,
-        Document(root, None)
+                           _rv: &mut ErrorResult)
+                           -> @mut Document {
+        unsafe {
+            let root = ~HTMLHtmlElement {
+                parent: Element::new(HTMLHtmlElementTypeId, ~"html")
+            };
+
+            let root = Node::as_abstract_node(root);
+            Document(root, None)
+        }
     }
-}
+}
+

src/servo/dom/node.rs

@@ -2,25 +2,24 @@
  * 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;
 use dom::bindings::codegen;
 use dom::bindings::node;
 use dom::bindings::utils::WrapperCache;
+use dom::bindings;
 use dom::characterdata::CharacterData;
 use dom::document::Document;
 use dom::element::{Element, ElementTypeId, HTMLImageElement, HTMLImageElementTypeId};
 use dom::element::{HTMLStyleElementTypeId};
+use js::rust::Compartment;
 use layout::debug::DebugMethods;
 use layout::flow::FlowContext;
 use newcss::complete::CompleteSelectResults;
-use js::rust::Compartment;
 
 use core::cast::transmute;
+use servo_util::tree::{TreeNode, TreeNodeRef, TreeUtils};
 
 //
 // The basic Node structure
@@ -35,28 +34,47 @@ pub struct AbstractNode {
 }
 
 impl Eq for AbstractNode {
-    fn eq(&self, other: &AbstractNode) -> bool { self.obj == other.obj }
+    fn eq(&self, other: &AbstractNode) -> bool {
-    fn ne(&self, other: &AbstractNode) -> bool { self.obj != other.obj }
+        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,
@@ -69,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>,
-    flow: Option<@mut FlowContext>,
+
+    /// 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,
@@ -87,6 +110,7 @@ impl LayoutData {
 // Basic node types
 //
 
+/// The `DOCTYPE` tag.
 pub struct Doctype {
     parent: Node,
     name: ~str,
@@ -96,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>,
@@ -111,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)
@@ -123,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)
@@ -135,110 +164,107 @@ impl Text {
     }
 }
 
-pub impl AbstractNode {
+impl Clone for AbstractNode {
-    //
+    fn clone(&self) -> AbstractNode {
-    // Convenience accessors
+        *self
-    //
+    }
-    // FIXME: Fold these into util::tree.
+}
 
-    fn type_id(self)         -> NodeTypeId           { self.with_imm_node(|n| n.type_id)      }
+impl TreeNode<AbstractNode> for Node {
-    fn parent_node(self)     -> Option<AbstractNode> { self.with_imm_node(|n| n.parent_node)  }
+    fn parent_node(&self) -> Option<AbstractNode> {
-    fn first_child(self)     -> Option<AbstractNode> { self.with_imm_node(|n| n.first_child)  }
+        self.parent_node
-    fn last_child(self)      -> Option<AbstractNode> { self.with_imm_node(|n| n.last_child)   }
+    }
-    fn prev_sibling(self)    -> Option<AbstractNode> { self.with_imm_node(|n| n.prev_sibling) }
+    fn first_child(&self) -> Option<AbstractNode> {
-    fn next_sibling(self)    -> Option<AbstractNode> { self.with_imm_node(|n| n.next_sibling) }
+        self.first_child
+    }
+    fn last_child(&self) -> Option<AbstractNode> {
+        self.last_child
+    }
+    fn prev_sibling(&self) -> Option<AbstractNode> {
+        self.prev_sibling
+    }
+    fn next_sibling(&self) -> Option<AbstractNode> {
+        self.next_sibling
+    }
+
+    fn set_parent_node(&mut self, new_parent_node: Option<AbstractNode>) {
+        self.parent_node = new_parent_node
+    }
+    fn set_first_child(&mut self, new_first_child: Option<AbstractNode>) {
+        self.first_child = new_first_child
+    }
+    fn set_last_child(&mut self, new_last_child: Option<AbstractNode>) {
+        self.last_child = new_last_child
+    }
+    fn set_prev_sibling(&mut self, new_prev_sibling: Option<AbstractNode>) {
+        self.prev_sibling = new_prev_sibling
+    }
+    fn set_next_sibling(&mut self, new_next_sibling: Option<AbstractNode>) {
+        self.next_sibling = new_next_sibling
+    }
+}
+
+impl TreeNodeRef<Node> for AbstractNode {
+    // FIXME: The duplication between `with_imm_node` and `with_imm_node` is ugly.
+    fn with_imm_node<R>(&self, callback: &fn(&Node) -> R) -> R {
+        self.transmute(callback)
+    }
+
+    fn with_mut_node<R>(&self, callback: &fn(&mut Node) -> R) -> R {
+        self.transmute_mut(callback)
+    }
+}
+
+impl AbstractNode {
+    // Convenience accessors
+
+    /// 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. 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. 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. 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. 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. 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;
@@ -251,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;
@@ -264,25 +290,19 @@ pub impl AbstractNode {
         }
     }
 
-    fn with_imm_node<R>(self, f: &fn(&Node) -> R) -> R {
+    pub fn is_text(self) -> bool {
-        self.transmute(f)
+        self.type_id() == TextNodeTypeId
     }
 
-    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 }
-
     // 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
@@ -290,7 +310,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");
         }
@@ -298,40 +318,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
         }
@@ -380,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_mut_node |node_data| {
-                    n.owner_doc = Some(doc);
+                    node_data.owner_doc = Some(doc);
                 }
-                true
+            };
-            });
             node = node.get().next_sibling();
         }
     }

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

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 layout data in the DOM.
-Code for managing the DOM aux pointer
-*/
 
 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 {
             }
         };
     }
-
 }
+

src/servo/layout/block.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/. */
 
-// Block layout.
+//! CSS block layout.
 
 use layout::box::{RenderBox};
 use layout::context::LayoutContext;
 use layout::display_list_builder::{DisplayListBuilder, FlowDisplayListBuilderMethods};
-use layout::flow::{BlockFlow, FlowContext, InlineBlockFlow, RootFlow};
+use layout::flow::{BlockFlow, FlowContext, FlowData, InlineBlockFlow, RootFlow};
 use layout::inline::InlineLayout;
 
 use au = gfx::geometry;
@@ -16,25 +16,36 @@ use geom::point::Point2D;
 use geom::rect::Rect;
 use gfx::display_list::DisplayList;
 use gfx::geometry::Au;
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct BlockFlowData {
+    /// Data common to all flows.
+    common: FlowData,
+
+    /// The associated render box.
     box: Option<@mut RenderBox>
 }
 
-pub fn BlockFlowData() -> BlockFlowData {
+impl BlockFlowData {
-    BlockFlowData {
+    pub fn new(common: FlowData) -> BlockFlowData {
-        box: None
+        BlockFlowData {
+            common: common,
+            box: None,
+        }
     }
 }
 
+/// NB: These are part of FlowContext, not part of BlockFlowData, because the root flow calls these
+/// as well. It is not clear to me whether this needs to be the case, or whether `RootFlow` can be
+/// merged into this.
 pub trait BlockLayout {
     fn starts_block_flow(&self) -> bool;
-    fn with_block_box(@mut self, &fn(box: &@mut RenderBox) -> ()) -> ();
+    fn with_block_box(&self, &fn(box: &@mut RenderBox) -> ()) -> ();
 
-    fn bubble_widths_block(@mut self, ctx: &LayoutContext);
+    fn bubble_widths_block(&self, ctx: &LayoutContext);
-    fn assign_widths_block(@mut self, ctx: &LayoutContext);
+    fn assign_widths_block(&self, ctx: &LayoutContext);
-    fn assign_height_block(@mut self, ctx: &LayoutContext);
+    fn assign_height_block(&self, ctx: &LayoutContext);
-    fn build_display_list_block(@mut self,
+    fn build_display_list_block(&self,
                                 a: &DisplayListBuilder,
                                 b: &Rect<Au>,
                                 c: &Point2D<Au>,
@@ -49,17 +60,21 @@ impl BlockLayout for FlowContext {
         }
     }
 
-    /* Get the current flow's corresponding block box, if it exists, and do something with it. 
+    /// Get the current flow's corresponding block box, if it exists, and do something with it. 
-       This works on both BlockFlow and RootFlow, since they are mostly the same. */
+    /// This works on both BlockFlow and RootFlow, since they are mostly the same.
-    fn with_block_box(@mut self, cb: &fn(box: &@mut RenderBox) -> ()) -> () {
+    fn with_block_box(&self, callback: &fn(box: &@mut RenderBox) -> ()) -> () {
         match *self {
             BlockFlow(*) => {
                 let box = self.block().box;
-                for box.each |b| { cb(b); }
+                for box.each |b| {
+                    callback(b);
+                }
             },                
             RootFlow(*) => {
                 let mut box = self.root().box;
-                for box.each |b| { cb(b); }
+                for box.each |b| {
+                    callback(b);
+                }
             },
             _  => fail!(fmt!("Tried to do something with_block_box(), but this is a %?", self))
         }
@@ -74,7 +89,7 @@ impl BlockLayout for FlowContext {
     /* TODO: floats */
     /* TODO: absolute contexts */
     /* TODO: inline-blocks */
-    fn bubble_widths_block(@mut self, ctx: &LayoutContext) {
+    fn bubble_widths_block(&self, ctx: &LayoutContext) {
         assert!(self.starts_block_flow());
 
         let mut min_width = Au(0);
@@ -84,8 +99,10 @@ impl BlockLayout for FlowContext {
         for self.each_child |child_ctx| {
             assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow());
 
-            min_width  = au::max(min_width, child_ctx.d().min_width);
+            do child_ctx.with_imm_node |child_node| {
-            pref_width = au::max(pref_width, child_ctx.d().pref_width);
+                min_width = au::max(min_width, child_node.min_width);
+                pref_width = au::max(pref_width, child_node.pref_width);
+            }
         }
 
         /* if not an anonymous block context, add in block box's widths.
@@ -95,50 +112,56 @@ impl BlockLayout for FlowContext {
             pref_width = pref_width.add(&box.get_pref_width(ctx));
         }
 
-        self.d().min_width = min_width;
+        do self.with_mut_node |this_node| {
-        self.d().pref_width = pref_width;
+            this_node.min_width = min_width;
+            this_node.pref_width = pref_width;
+        }
     }
  
-    /* Recursively (top-down) determines the actual width of child
+    /// Recursively (top-down) determines the actual width of child contexts and boxes. When called
-    contexts and boxes. When called on this context, the context has
+    /// on this context, the context has had its width set by the parent context.
-    had its width set by the parent context.
+    ///
-
+    /// Dual boxes consume some width first, and the remainder is assigned to all child (block)
-    Dual boxes consume some width first, and the remainder is assigned to
+    /// contexts.
-    all child (block) contexts. */
+    fn assign_widths_block(&self, _: &LayoutContext) { 
-
-    fn assign_widths_block(@mut self, _ctx: &LayoutContext) { 
         assert!(self.starts_block_flow());
 
-        let mut remaining_width = self.d().position.size.width;
+        let mut remaining_width = self.with_imm_node(|this| this.position.size.width);
         let mut _right_used = Au(0);
         let mut left_used = Au(0);
 
-        /* Let the box consume some width. It will return the amount remaining
+        // Let the box consume some width. It will return the amount remaining for its children.
-           for its children. */
         do self.with_block_box |box| {
             box.d().position.size.width = remaining_width;
             let (left_used, right_used) = box.get_used_width();
             remaining_width -= left_used.add(&right_used);
         }
 
-        for self.each_child |child_ctx| {
+        for self.each_child |kid| {
-            assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow());
+            assert!(kid.starts_block_flow() || kid.starts_inline_flow());
-            child_ctx.d().position.origin.x = left_used;
+
-            child_ctx.d().position.size.width = remaining_width;
+            do kid.with_mut_node |child_node| {
+                child_node.position.origin.x = left_used;
+                child_node.position.size.width = remaining_width;
+            }
         }
     }
 
-    fn assign_height_block(@mut self, _ctx: &LayoutContext) {
+    fn assign_height_block(&self, _ctx: &LayoutContext) {
         assert!(self.starts_block_flow());
 
         let mut cur_y = Au(0);
 
-        for self.each_child |child_ctx| {
+        for self.each_child |kid| {
-            child_ctx.d().position.origin.y = cur_y;
+            do kid.with_mut_node |child_node| {
-            cur_y += child_ctx.d().position.size.height;
+                child_node.position.origin.y = cur_y;
+                cur_y += child_node.position.size.height;
+            }
         }
 
-        self.d().position.size.height = cur_y;
+        do self.with_mut_node |this_node| {
+            this_node.position.size.height = cur_y;
+        }
 
         let _used_top = Au(0);
         let _used_bot = Au(0);
@@ -150,9 +173,11 @@ impl BlockLayout for FlowContext {
         }
     }
 
-    fn build_display_list_block(@mut self, builder: &DisplayListBuilder, dirty: &Rect<Au>, 
+    fn build_display_list_block(&self,
-                                offset: &Point2D<Au>, list: &Cell<DisplayList>) {
+                                builder: &DisplayListBuilder,
-
+                                dirty: &Rect<Au>, 
+                                offset: &Point2D<Au>,
+                                list: &Cell<DisplayList>) {
         assert!(self.starts_block_flow());
         
         // add box that starts block context

src/servo/layout/box.rs

@@ -71,13 +71,13 @@ padding, backgrounds. It is analogous to a CSS nonreplaced content box.
 */
 pub struct RenderBoxData {
     /* originating DOM node */
-    node : AbstractNode,
+    node: AbstractNode,
     /* reference to containing flow context, which this box
        participates in */
-    ctx  : @mut FlowContext,
+    ctx: FlowContext,
     /* position of this box relative to owning flow */
-    position : Rect<Au>,
+    position: Rect<Au>,
-    font_size : Length,
+    font_size: Length,
     /* TODO (Issue #87): debug only */
     id: int
 }
@@ -103,7 +103,7 @@ pub enum SplitBoxResult {
     SplitDidNotFit(Option<@mut RenderBox>, Option<@mut RenderBox>)
 }
 
-pub fn RenderBoxData(node: AbstractNode, ctx: @mut FlowContext, id: int) -> RenderBoxData {
+pub fn RenderBoxData(node: AbstractNode, ctx: FlowContext, id: int) -> RenderBoxData {
     RenderBoxData {
         node : node,
         ctx  : ctx,

src/servo/layout/box_builder.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/. */
 
-/** Creates CSS boxes from a DOM. */
+//! Creates CSS boxes from a DOM tree.
 
 use dom::element::*;
 use dom::node::{AbstractNode, CommentNodeTypeId, DoctypeNodeTypeId};
@@ -16,12 +16,13 @@ use layout::inline::{InlineFlowData, InlineLayout};
 use layout::root::RootFlowData;
 
 use gfx::image::holder::ImageHolder;
-use servo_util::range::Range;
 use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock};
 use newcss::values::{CSSDisplayNone};
+use servo_util::range::Range;
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct LayoutTreeBuilder {
-    root_flow: Option<@mut FlowContext>,
+    root_flow: Option<FlowContext>,
     next_bid: int,
     next_cid: int
 }
@@ -39,7 +40,7 @@ pub impl LayoutTreeBuilder {
 // helper object for building the initial box list and making the
 // mapping between DOM nodes and boxes.
 struct BoxGenerator {
-    flow: @mut FlowContext,
+    flow: FlowContext,
     range_stack: ~[uint],
 }
 
@@ -58,7 +59,9 @@ priv fn simulate_UA_display_rules(node: AbstractNode) -> CSSDisplay {
     };*/
 
     let resolved = CSSDisplayInline;
-    if (resolved == CSSDisplayNone) { return resolved; }
+    if resolved == CSSDisplayNone {
+        return resolved;
+    }
 
     match node.type_id() {
         DoctypeNodeTypeId | CommentNodeTypeId => CSSDisplayNone,
@@ -81,7 +84,7 @@ priv fn simulate_UA_display_rules(node: AbstractNode) -> CSSDisplay {
 }
 
 impl BoxGenerator {
-    fn new(flow: @mut FlowContext) -> BoxGenerator {
+    fn new(flow: FlowContext) -> BoxGenerator {
         debug!("Creating box generator for flow: %s", flow.debug_str());
         BoxGenerator {
             flow: flow,
@@ -99,103 +102,99 @@ impl BoxGenerator {
                                         _: &LayoutContext,
                                         _: AbstractNode,
                                         _: InlineSpacerSide)
-                                     -> Option<@mut RenderBox> {
+                                        -> Option<@mut RenderBox> {
         None
     }
 
-    pub fn push_node(@mut self, ctx: &LayoutContext, builder: &mut LayoutTreeBuilder, node: AbstractNode) {
+    pub fn push_node(&mut self,
-        debug!("BoxGenerator[f%d]: pushing node: %s", self.flow.d().id, node.debug_str());
+                     ctx: &LayoutContext,
+                     builder: &mut LayoutTreeBuilder,
+                     node: AbstractNode) {
+        debug!("BoxGenerator[f%d]: pushing node: %s", self.flow.id(), node.debug_str());
 
         // first, determine the box type, based on node characteristics
         let simulated_display = simulate_UA_display_rules(node);
         // TODO: remove this once UA styles work
         let box_type = builder.decide_box_type(node, simulated_display);
 
-        debug!("BoxGenerator[f%d]: point a", self.flow.d().id);
+        debug!("BoxGenerator[f%d]: point a", self.flow.id());
 
         // depending on flow, make a box for this node.
         match self.flow {
-            @InlineFlow(*) => {
+            InlineFlow(inline) => {
-                let node_range_start = match self.flow {
+                let mut inline = &mut *inline;
-                    @InlineFlow(*) => {
+                let node_range_start = inline.boxes.len();
-                        let inline_flow = self.flow.inline();
-                        inline_flow.boxes.len()
-                    }
-                    _ => 0
-                };
                 self.range_stack.push(node_range_start);
 
                 // if a leaf, make a box.
                 if node.is_leaf() {
                     let new_box = builder.make_box(ctx, box_type, node, self.flow);
-                    let boxes = &mut self.flow.inline().boxes;
+                    inline.boxes.push(new_box);
-                    boxes.push(new_box);
                 } else if self.inline_spacers_needed_for_node(node) {
                     // else, maybe make a spacer for "left" margin, border, padding
                     for self.make_inline_spacer_for_node_side(ctx, node, LogicalBefore).each
                             |spacer: &@mut RenderBox| {
-                        let boxes = &mut self.flow.inline().boxes;
+                        inline.boxes.push(*spacer);
-                        boxes.push(*spacer);
                     }
                 }
                 // TODO: cases for inline-block, etc.
             },
-            @BlockFlow(*) => {
+            BlockFlow(block) => {
-                debug!("BoxGenerator[f%d]: point b", self.flow.d().id);
+                debug!("BoxGenerator[f%d]: point b", block.common.id);
                 let new_box = builder.make_box(ctx, box_type, node, self.flow);
-                debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)",
-                       self.flow.d().id, new_box.d().id, node.debug_str());
 
-                assert!(self.flow.block().box.is_none());
+                debug!("BoxGenerator[f%d]: attaching box[b%d] to block flow (node: %s)",
-                //XXXjdm We segfault when returning without this temporary.
+                       block.common.id,
-                let block = self.flow.block();
+                       new_box.d().id,
+                       node.debug_str());
+
+                assert!(block.box.is_none());
                 block.box = Some(new_box);
             },
-            @RootFlow(*) => {
+            RootFlow(root) => {
-                debug!("BoxGenerator[f%d]: point c", self.flow.d().id);
+                debug!("BoxGenerator[f%d]: point c", root.common.id);
                 let new_box = builder.make_box(ctx, box_type, node, self.flow);
-                debug!("BoxGenerator[f%d]: (node is: %s)", self.flow.d().id, node.debug_str());
+                debug!("BoxGenerator[f%d]: (node is: %s)", root.common.id, node.debug_str());
                 debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)",
-                       self.flow.d().id, new_box.d().id, node.debug_str());
+                       root.common.id,
+                       new_box.d().id,
+                       node.debug_str());
 
-                assert!(self.flow.root().box.is_none());
+                assert!(root.box.is_none());
-                //XXXjdm We segfault when returning without this temporary.
-                let root = self.flow.root();
                 root.box = Some(new_box);
             },
-            _ => { warn!("push_node() not implemented for flow f%d", self.flow.d().id) }
+            _ => warn!("push_node() not implemented for flow f%d", self.flow.id()),
         }
     }
 
-    pub fn pop_node(&mut self, ctx: &LayoutContext, _builder: &LayoutTreeBuilder, node: AbstractNode) {
+    pub fn pop_node(&mut self,
-        debug!("BoxGenerator[f%d]: popping node: %s", self.flow.d().id, node.debug_str());
+                    ctx: &LayoutContext,
+                    _builder: &LayoutTreeBuilder,
+                    node: AbstractNode) {
+        debug!("BoxGenerator[f%d]: popping node: %s", self.flow.id(), node.debug_str());
 
         match self.flow {
-            @InlineFlow(*) => {
+            InlineFlow(inline) => {
+                let inline = &mut *inline;
+
                 if self.inline_spacers_needed_for_node(node) {
-                    // if this non-leaf box generates extra horizontal
+                    // If this non-leaf box generates extra horizontal spacing, add a SpacerBox for
-                    // spacing, add a SpacerBox for it.
+                    // it.
-                    for self.make_inline_spacer_for_node_side(ctx, node, LogicalAfter).each |spacer: &@mut RenderBox| {
+                    let result = self.make_inline_spacer_for_node_side(ctx, node, LogicalAfter);
+                    for result.each |spacer| {
                         let boxes = &mut self.flow.inline().boxes;
                         boxes.push(*spacer);
                     }
                 }
                 let mut node_range: Range = Range::new(self.range_stack.pop(), 0);
-                let inline_flow = self.flow.inline(); // FIXME: borrow checker workaround
+                node_range.extend_to(inline.boxes.len());
-                node_range.extend_to(inline_flow.boxes.len());
                 assert!(node_range.length() > 0);
 
                 debug!("BoxGenerator: adding element range=%?", node_range);
-                let elems = &mut inline_flow.elems;
+                inline.elems.add_mapping(node, &node_range);
-                elems.add_mapping(node, &node_range);
             },
-            @BlockFlow(*) | @RootFlow(*) => {
+            BlockFlow(*) | RootFlow(*) => assert!(self.range_stack.len() == 0),
-                assert!(self.range_stack.len() == 0);
+            _ => warn!("pop_node() not implemented for flow %?", self.flow.id()),
-            },
-            _ => { 
-                let d = self.flow.d(); // FIXME: borrow checker workaround
-                warn!("pop_node() not implemented for flow %?", d.id)
-            }
         }
     }
 }
@@ -207,7 +206,11 @@ struct BuilderContext {
 
 impl BuilderContext {
     fn new(collector: @mut BoxGenerator) -> BuilderContext {
-        debug!("Creating new BuilderContext for flow: %s", collector.flow.debug_str());
+        {
+            let collector = &mut *collector;
+            debug!("Creating new BuilderContext for flow: %s", collector.flow.debug_str());
+        }
+
         BuilderContext {
             default_collector: collector,
             inline_collector: None,
@@ -219,11 +222,12 @@ impl BuilderContext {
         copy self
     }
     
-    priv fn attach_child_flow(&self, child: @mut FlowContext) {
+    priv fn attach_child_flow(&self, child: FlowContext) {
-        let d = self.default_collector.flow.d(); // FIXME: borrow checker workaround
+        let default_collector = &mut *self.default_collector;
-        let cd = child.d(); // FIXME: borrow checker workaround
+        debug!("BuilderContext: Adding child flow f%? of f%?",
-        debug!("BuilderContext: Adding child flow f%? of f%?", d.id, cd.id);
+               default_collector.flow.id(),
-        self.default_collector.flow.add_child(child);
+               child.id());
+        default_collector.flow.add_child(child);
     }
     
     priv fn create_child_flow_of_type(&self,
@@ -236,7 +240,8 @@ impl BuilderContext {
         BuilderContext::new(@mut BoxGenerator::new(new_flow))
     }
         
-    priv fn make_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) -> BuilderContext {
+    priv fn make_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode)
+                                  -> BuilderContext {
         debug!("BuilderContext: making new inline collector flow");
         let new_flow = builder.make_flow(Flow_Inline, node);
         let new_generator = @mut BoxGenerator::new(new_flow);
@@ -247,7 +252,8 @@ impl BuilderContext {
         BuilderContext::new(new_generator)
     }
 
-    priv fn get_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode) -> BuilderContext {
+    priv fn get_inline_collector(&mut self, builder: &mut LayoutTreeBuilder, node: AbstractNode)
+                                 -> BuilderContext {
         match copy self.inline_collector {
             Some(collector) => BuilderContext::new(collector),
             None => self.make_inline_collector(builder, node)
@@ -261,10 +267,8 @@ impl BuilderContext {
     // returns a context for the current node, or None if the document subtree rooted
     // by the node should not generate a layout tree. For example, nodes with style 'display:none'
     // should just not generate any flows or boxes.
-    fn containing_context_for_node(&mut self,
+    fn containing_context_for_node(&mut self, node: AbstractNode, builder: &mut LayoutTreeBuilder)
-                                   node: AbstractNode,
+                                   -> Option<BuilderContext> {
-                                   builder: &mut LayoutTreeBuilder)
-                                -> Option<BuilderContext> {
         // TODO: remove this once UA styles work
         // TODO: handle interactions with 'float', 'position' (CSS 2.1, Section 9.7)
         let simulated_display = match simulate_UA_display_rules(node) {
@@ -273,7 +277,7 @@ impl BuilderContext {
         };
 
         let containing_context = match (simulated_display, self.default_collector.flow) { 
-            (CSSDisplayBlock, @RootFlow(*)) => {
+            (CSSDisplayBlock, RootFlow(*)) => {
                 // If this is the root node, then use the root flow's
                 // context. Otherwise, make a child block context.
                 match node.parent_node() {
@@ -281,14 +285,14 @@ impl BuilderContext {
                     None => { self.clone() },
                 }
             },
-            (CSSDisplayBlock, @BlockFlow(*)) => {
+            (CSSDisplayBlock, BlockFlow(*)) => {
                 self.clear_inline_collector();
                 self.create_child_flow_of_type(Flow_Block, builder, node)
             },
-            (CSSDisplayInline, @InlineFlow(*)) => self.clone(),
+            (CSSDisplayInline, InlineFlow(*)) => self.clone(),
-            (CSSDisplayInlineBlock, @InlineFlow(*)) => self.clone(),
+            (CSSDisplayInlineBlock, InlineFlow(*)) => self.clone(),
-            (CSSDisplayInline, @BlockFlow(*)) => self.get_inline_collector(builder, node),
+            (CSSDisplayInline, BlockFlow(*)) => self.get_inline_collector(builder, node),
-            (CSSDisplayInlineBlock, @BlockFlow(*)) => self.get_inline_collector(builder, node),
+            (CSSDisplayInlineBlock, BlockFlow(*)) => self.get_inline_collector(builder, node),
             _ => self.clone()
         };
 
@@ -330,31 +334,34 @@ pub impl LayoutTreeBuilder {
         // eventually be elided or split, but the mapping between
         // nodes and FlowContexts should not change during layout.
         let flow = &mut this_ctx.default_collector.flow;
-        for flow.each_child |child_flow: @mut FlowContext| {
+        let flow: &FlowContext = flow;
-            let node = child_flow.d().node;
+        for flow.each_child |child_flow| {
-            assert!(node.has_layout_data());
+            do child_flow.with_imm_node |child_node| {
-            node.layout_data().flow = Some(child_flow);
+                let dom_node = child_node.node;
+                assert!(dom_node.has_layout_data());
+                dom_node.layout_data().flow = Some(child_flow);
+            }
         }
     }
 
-    // Fixup any irregularities such as:
+    /// Fix up any irregularities such as:
-    //
+    ///
-    // * split inlines (CSS 2.1 Section 9.2.1.1)
+    /// * split inlines (CSS 2.1 Section 9.2.1.1)
-    // * elide non-preformatted whitespace-only text boxes and their
+    /// * elide non-preformatted whitespace-only text boxes and their flows (CSS 2.1 Section
-    //   flows (CSS 2.1 Section 9.2.2.1).
+    ///   9.2.2.1).
-    //
+    ///
-    // The latter can only be done immediately adjacent to, or at the
+    /// The latter can only be done immediately adjacent to, or at the beginning or end of a block
-    // beginning or end of a block flow. Otherwise, the whitespace
+    /// flow. Otherwise, the whitespace might affect whitespace collapsing with adjacent text.
-    // might affect whitespace collapsing with adjacent text.
     fn simplify_children_of_flow(&self, _: &LayoutContext, parent_ctx: &BuilderContext) {
-        match *parent_ctx.default_collector.flow {
+        match parent_ctx.default_collector.flow {
             InlineFlow(*) => {
                 let mut found_child_inline = false;
                 let mut found_child_block = false;
 
                 let flow = &mut parent_ctx.default_collector.flow;
-                for flow.each_child |child_ctx: @mut FlowContext| {
+                let flow: &FlowContext = flow;
-                    match *child_ctx {
+                for flow.each_child |child_ctx| {
+                    match child_ctx {
                         InlineFlow(*) | InlineBlockFlow(*) => found_child_inline = true,
                         BlockFlow(*) => found_child_block = true,
                         _ => {}
@@ -370,34 +377,31 @@ pub impl LayoutTreeBuilder {
                 // of its RenderBox or FlowContext children, and possibly keep alive other junk
                 let parent_flow = parent_ctx.default_collector.flow;
 
-                // FIXME: Workaround for the borrow check.
+                let (first_child, last_child) = do parent_flow.with_imm_node |parent_node| {
-                let (first_child, last_child) = {
+                    (parent_node.first_child, parent_node.last_child)
-                    let parent_flow: &mut FlowContext = parent_flow;
-                    let parent_flow_data = parent_flow.d();
-                    (parent_flow_data.first_child, parent_flow_data.last_child)
                 };
 
                 // check first/last child for whitespace-ness
-                for first_child.each |first_flow: &@mut FlowContext| {
+                for first_child.each |first_flow| {
                     if first_flow.starts_inline_flow() {
                         let boxes = &mut first_flow.inline().boxes;
                         if boxes.len() == 1 && boxes[0].is_whitespace_only() {
                             debug!("LayoutTreeBuilder: pruning whitespace-only first child flow \
                                     f%d from parent f%d", 
-                                   first_flow.d().id,
+                                   first_flow.id(),
-                                   parent_flow.d().id);
+                                   parent_flow.id());
                             parent_flow.remove_child(*first_flow);
                         }
                     }
                 }
-                for last_child.each |last_flow: &@mut FlowContext| {
+                for last_child.each |last_flow| {
                     if last_flow.starts_inline_flow() {
                         let boxes = &mut last_flow.inline().boxes;
                         if boxes.len() == 1 && boxes.last().is_whitespace_only() {
                             debug!("LayoutTreeBuilder: pruning whitespace-only last child flow \
                                     f%d from parent f%d", 
-                                   last_flow.d().id,
+                                   last_flow.id(),
-                                   parent_flow.d().id);
+                                   parent_flow.id());
                             parent_flow.remove_child(*last_flow);
                         }
                     }
@@ -407,15 +411,14 @@ pub impl LayoutTreeBuilder {
         }
     }
 
-    fn fixup_split_inline(&self, _: @mut FlowContext) {
+    fn fixup_split_inline(&self, _: FlowContext) {
         // TODO: finish me. 
         fail!(~"TODO: handle case where an inline is split by a block")
     }
 
-    /** entry point for box creation. Should only be 
+    /// Entry point for box creation. Should only be called on the root DOM element.
-    called on root DOM element. */
     fn construct_trees(&mut self, layout_ctx: &LayoutContext, root: AbstractNode)
-                    -> Result<@mut FlowContext, ()> {
+                       -> Result<FlowContext, ()> {
         let new_flow = self.make_flow(Flow_Root, root);
         let new_generator = @mut BoxGenerator::new(new_flow);
         let mut root_ctx = BuilderContext::new(new_generator);
@@ -425,53 +428,46 @@ pub impl LayoutTreeBuilder {
         return Ok(new_flow)
     }
 
-    fn make_flow(&mut self, ty: FlowContextType, node: AbstractNode) -> @mut FlowContext {
+    /// Creates a flow of the given type for the supplied node.
-        let data = FlowData::new(self.next_flow_id(), node);
+    fn make_flow(&mut self, ty: FlowContextType, node: AbstractNode) -> FlowContext {
-        let ret = match ty {
+        let info = FlowData::new(self.next_flow_id(), node);
-            Flow_Absolute    => @mut AbsoluteFlow(data),
+        let result = match ty {
-            Flow_Block       => @mut BlockFlow(data, BlockFlowData()),
+            Flow_Absolute    => AbsoluteFlow(@mut info),
-            Flow_Float       => @mut FloatFlow(data),
+            Flow_Block       => BlockFlow(@mut BlockFlowData::new(info)),
-            Flow_InlineBlock => @mut InlineBlockFlow(data),
+            Flow_Float       => FloatFlow(@mut info),
-            Flow_Inline      => @mut InlineFlow(data, InlineFlowData()),
+            Flow_InlineBlock => InlineBlockFlow(@mut info),
-            Flow_Root        => @mut RootFlow(data, RootFlowData()),
+            Flow_Inline      => InlineFlow(@mut InlineFlowData::new(info)),
-            Flow_Table       => @mut TableFlow(data)
+            Flow_Root        => RootFlow(@mut RootFlowData::new(info)),
+            Flow_Table       => TableFlow(@mut info),
         };
-        debug!("LayoutTreeBuilder: created flow: %s", ret.debug_str());
+        debug!("LayoutTreeBuilder: created flow: %s", result.debug_str());
-        ret
+        result
     }
 
-    /**
+    /// Disambiguate between different methods here instead of inlining, since each case has very
-       disambiguate between different methods here instead of inlining, since each
+    /// different complexity.
-       case has very different complexity 
-    */
     fn make_box(&mut self,
                 layout_ctx: &LayoutContext,
                 ty: RenderBoxType,
                 node: AbstractNode,
-                ctx: @mut FlowContext)
+                ctx: FlowContext)
-             -> @mut RenderBox {
+                -> @mut RenderBox {
-        let ret = match ty {
+        let result = match ty {
             RenderBox_Generic => self.make_generic_box(layout_ctx, node, ctx),
             RenderBox_Text    => self.make_text_box(layout_ctx, node, ctx),
             RenderBox_Image   => self.make_image_box(layout_ctx, node, ctx),
         };
-        debug!("LayoutTreeBuilder: created box: %s", ret.debug_str());
+        debug!("LayoutTreeBuilder: created box: %s", result.debug_str());
-        ret
+        result
     }
 
-    fn make_generic_box(&mut self,
+    fn make_generic_box(&mut self, _: &LayoutContext, node: AbstractNode, ctx: FlowContext)
-                        _: &LayoutContext,
+                        -> @mut RenderBox {
-                        node: AbstractNode,
-                        ctx: @mut FlowContext)
-                     -> @mut RenderBox {
         @mut GenericBox(RenderBoxData(copy node, ctx, self.next_box_id()))
     }
 
-    fn make_image_box(&mut self,
+    fn make_image_box(&mut self, layout_ctx: &LayoutContext, node: AbstractNode, ctx: FlowContext)
-                      layout_ctx: &LayoutContext,
+                      -> @mut RenderBox {
-                      node: AbstractNode,
-                      ctx: @mut FlowContext)
-                   -> @mut RenderBox {
         if !node.is_image_element() {
             fail!(~"WAT error: why couldn't we make an image box?");
         }
@@ -482,17 +478,15 @@ pub impl LayoutTreeBuilder {
                                               layout_ctx.image_cache);
                 @mut ImageBox(RenderBoxData(node, ctx, self.next_box_id()), holder)
             } else {
-                info!("Tried to make image box, but couldn't find image. Made generic box instead.");
+                info!("Tried to make image box, but couldn't find image. Made generic box \
+                       instead.");
                 self.make_generic_box(layout_ctx, node, ctx)
             }
         }
     }
 
-    fn make_text_box(&mut self,
+    fn make_text_box(&mut self, _: &LayoutContext, node: AbstractNode, ctx: FlowContext)
-                     _: &LayoutContext,
+                     -> @mut RenderBox {
-                     node: AbstractNode,
-                     ctx: @mut FlowContext)
-                  -> @mut RenderBox {
         if !node.is_text() {
             fail!(~"WAT error: why couldn't we make a text box?");
         }

src/servo/layout/display_list_builder.rs

@@ -16,30 +16,30 @@ use geom::rect::Rect;
 use gfx::display_list::DisplayList;
 use gfx::geometry::Au;
 use gfx;
+use servo_util::tree::TreeNodeRef;
 
-/** A builder object that manages display list builder should mainly
+/// A builder object that manages display list builder should mainly hold information about the
- hold information about the initial request and desired result---for
+/// initial request and desired result--for example, whether the `DisplayList` is to be used for
- example, whether the DisplayList to be used for painting or hit
+/// painting or hit testing. This can affect which boxes are created.
- testing. This can affect which boxes are created.
+///
-
+/// Right now, the builder isn't used for much, but it establishes the pattern we'll need once we
- Right now, the builder isn't used for much, but it  establishes the
+/// support display-list-based hit testing and so forth.
- pattern we'll need once we support DL-based hit testing &c.  */
 pub struct DisplayListBuilder<'self> {
     ctx:  &'self LayoutContext,
 }
 
 pub trait FlowDisplayListBuilderMethods {
-    fn build_display_list(@mut self, a: &DisplayListBuilder, b: &Rect<Au>, c: &Cell<DisplayList>);
+    fn build_display_list(&self, a: &DisplayListBuilder, b: &Rect<Au>, c: &Cell<DisplayList>);
-    fn build_display_list_for_child(@mut self,
+    fn build_display_list_for_child(&self,
                                     a: &DisplayListBuilder,
-                                    b: @mut FlowContext,
+                                    b: FlowContext,
                                     c: &Rect<Au>,
                                     d: &Point2D<Au>,
                                     e: &Cell<DisplayList>);
 }
 
 impl FlowDisplayListBuilderMethods for FlowContext {
-    fn build_display_list(@mut self,
+    fn build_display_list(&self,
                           builder: &DisplayListBuilder,
                           dirty: &Rect<Au>,
                           list: &Cell<DisplayList>) {
@@ -47,27 +47,28 @@ impl FlowDisplayListBuilderMethods for FlowContext {
         self.build_display_list_recurse(builder, dirty, &zero, list);
     }
 
-    fn build_display_list_for_child(@mut self,
+    fn build_display_list_for_child(&self,
                                     builder: &DisplayListBuilder,
-                                    child_flow: @mut FlowContext,
+                                    child_flow: FlowContext,
-                                    dirty: &Rect<Au>, offset: &Point2D<Au>,
+                                    dirty: &Rect<Au>,
+                                    offset: &Point2D<Au>,
                                     list: &Cell<DisplayList>) {
+        // Adjust the dirty rect to child flow context coordinates.
+        do child_flow.with_imm_node |child_node| {
+            let abs_flow_bounds = child_node.position.translate(offset);
+            let adj_offset = offset.add(&child_node.position.origin);
 
-        // adjust the dirty rect to child flow context coordinates
+            debug!("build_display_list_for_child: rel=%?, abs=%?",
-        let d = child_flow.d(); // FIXME: borrow checker workaround
+                   child_node.position,
-        let abs_flow_bounds = d.position.translate(offset);
+                   abs_flow_bounds);
-        let adj_offset = offset.add(&d.position.origin);
+            debug!("build_display_list_for_child: dirty=%?, offset=%?", dirty, offset);
 
-        debug!("build_display_list_for_child: rel=%?, abs=%?",
+            if dirty.intersects(&abs_flow_bounds) {
-               d.position, abs_flow_bounds);
+                debug!("build_display_list_for_child: intersected. recursing into child flow...");
-        debug!("build_display_list_for_child: dirty=%?, offset=%?",
+                child_flow.build_display_list_recurse(builder, dirty, &adj_offset, list);
-               dirty, offset);
+            } else {
-
+                debug!("build_display_list_for_child: Did not intersect...");
-        if dirty.intersects(&abs_flow_bounds) {
+            }
-            debug!("build_display_list_for_child: intersected. recursing into child flow...");
-            child_flow.build_display_list_recurse(builder, dirty, &adj_offset, list);
-        } else {
-            debug!("build_display_list_for_child: Did not intersect...");
         }
     }
 }

src/servo/layout/flow.rs

@@ -14,42 +14,45 @@
 ///
 /// Currently, the important types of flows are:
 /// 
-/// * `BlockFlow`: a flow that establishes a block context. It has several child flows, each of
+/// * `BlockFlow`: A flow that establishes a block context. It has several child flows, each of
 ///   which are positioned according to block formatting context rules (CSS block boxes). Block
 ///   flows also contain a single `GenericBox` to represent their rendered borders, padding, etc.
 ///   (In the future, this render box may be folded into `BlockFlow` to save space.)
 ///
-/// * `InlineFlow`: a flow that establishes an inline context. It has a flat list of child
+/// * `InlineFlow`: A flow that establishes an inline context. It has a flat list of child
 ///   boxes/flows that are subject to inline layout and line breaking and structs to represent
 ///   line breaks and mapping to CSS boxes, for the purpose of handling `getClientRects()` and
 ///   similar methods.
+///
+/// * `RootFlow`: The flow at the root of the tree. This flow behaves like a `BlockFlow`, except
+///   that stretches to the boundaries of the viewport.
 
 use dom::node::AbstractNode;
 use layout::block::{BlockFlowData, BlockLayout};
 use layout::box::RenderBox;
 use layout::context::LayoutContext;
-use layout::debug::BoxedMutDebugMethods;
+use layout::debug::DebugMethods;
 use layout::display_list_builder::DisplayListBuilder;
-use layout::inline::{InlineFlowData, InlineLayout};
+use layout::inline::{InlineFlowData};
-use layout::root::{RootFlowData, RootLayout};
+use layout::root::{RootFlowData};
 
 use core::cell::Cell;
-use core::ptr;
 use geom::point::Point2D;
 use geom::rect::Rect;
 use gfx::display_list::DisplayList;
 use gfx::geometry::Au;
+use servo_util::tree::{TreeNode, TreeNodeRef, TreeUtils};
 
 /// The type of the formatting context and data specific to each context, such as line box
 /// structures or float lists.
 pub enum FlowContext {
-    AbsoluteFlow(FlowData), 
+    AbsoluteFlow(@mut FlowData), 
-    BlockFlow(FlowData, BlockFlowData),
+    BlockFlow(@mut BlockFlowData),
-    FloatFlow(FlowData),
+    FloatFlow(@mut FlowData),
-    InlineBlockFlow(FlowData),
+    InlineBlockFlow(@mut FlowData),
-    InlineFlow(FlowData, InlineFlowData),
+    InlineFlow(@mut InlineFlowData),
-    RootFlow(FlowData, RootFlowData),
+    RootFlow(@mut RootFlowData),
-    TableFlow(FlowData)
+    TableFlow(@mut FlowData),
 }
 
 pub enum FlowContextType {
@@ -62,16 +65,67 @@ pub enum FlowContextType {
     Flow_Table
 }
 
-/* A particular kind of layout context. It manages the positioning of
+impl Clone for FlowContext {
-   render boxes within the context.  */
+    fn clone(&self) -> FlowContext {
+        *self
+    }
+}
+
+impl TreeNodeRef<FlowData> for FlowContext {
+    fn with_imm_node<R>(&self, callback: &fn(&FlowData) -> R) -> R {
+        match *self {
+            AbsoluteFlow(info) => callback(info),
+            BlockFlow(info) => {
+                let info = &*info;  // FIXME: Borrow check workaround.
+                callback(&info.common)
+            }
+            FloatFlow(info) => callback(info),
+            InlineBlockFlow(info) => callback(info),
+            InlineFlow(info) => {
+                let info = &*info;  // FIXME: Borrow check workaround.
+                callback(&info.common)
+            }
+            RootFlow(info) => {
+                let info = &*info;  // FIXME: Borrow check workaround.
+                callback(&info.common)
+            }
+            TableFlow(info) => callback(info),
+        }
+    }
+    fn with_mut_node<R>(&self, callback: &fn(&mut FlowData) -> R) -> R {
+        match *self {
+            AbsoluteFlow(info) => callback(info),
+            BlockFlow(info) => {
+                let info = &mut *info;  // FIXME: Borrow check workaround.
+                callback(&mut info.common)
+            }
+            FloatFlow(info) => callback(info),
+            InlineBlockFlow(info) => callback(info),
+            InlineFlow(info) => {
+                let info = &mut *info;  // FIXME: Borrow check workaround.
+                callback(&mut info.common)
+            }
+            RootFlow(info) => {
+                let info = &mut *info;  // FIXME: Borrow check workaround.
+                callback(&mut info.common)
+            }
+            TableFlow(info) => callback(info),
+        }
+    }
+}
+
+/// Data common to all flows.
+///
+/// FIXME: We need a naming convention for pseudo-inheritance like this. How about
+/// `CommonFlowInfo`?
 pub struct FlowData {
     node: AbstractNode,
 
-    parent: Option<@mut FlowContext>,
+    parent: Option<FlowContext>,
-    first_child: Option<@mut FlowContext>,
+    first_child: Option<FlowContext>,
-    last_child: Option<@mut FlowContext>,
+    last_child: Option<FlowContext>,
-    prev_sibling: Option<@mut FlowContext>,
+    prev_sibling: Option<FlowContext>,
-    next_sibling: Option<@mut FlowContext>,
+    next_sibling: Option<FlowContext>,
 
     /* TODO (Issue #87): debug only */
     id: int,
@@ -84,6 +138,48 @@ pub struct FlowData {
     position: Rect<Au>,
 }
 
+impl TreeNode<FlowContext> for FlowData {
+    fn parent_node(&self) -> Option<FlowContext> {
+        self.parent
+    }
+
+    fn first_child(&self) -> Option<FlowContext> {
+        self.first_child
+    }
+
+    fn last_child(&self) -> Option<FlowContext> {
+        self.last_child
+    }
+
+    fn prev_sibling(&self) -> Option<FlowContext> {
+        self.prev_sibling
+    }
+
+    fn next_sibling(&self) -> Option<FlowContext> {
+        self.next_sibling
+    }
+
+    fn set_parent_node(&mut self, new_parent_node: Option<FlowContext>) {
+        self.parent = new_parent_node
+    }
+
+    fn set_first_child(&mut self, new_first_child: Option<FlowContext>) {
+        self.first_child = new_first_child
+    }
+
+    fn set_last_child(&mut self, new_last_child: Option<FlowContext>) {
+        self.last_child = new_last_child
+    }
+
+    fn set_prev_sibling(&mut self, new_prev_sibling: Option<FlowContext>) {
+        self.prev_sibling = new_prev_sibling
+    }
+
+    fn set_next_sibling(&mut self, new_next_sibling: Option<FlowContext>) {
+        self.next_sibling = new_next_sibling
+    }
+}
+
 impl FlowData {
     pub fn new(id: int, node: AbstractNode) -> FlowData {
         FlowData {
@@ -105,205 +201,142 @@ impl FlowData {
 }
 
 impl<'self> FlowContext {
-    pub fn d(&'self mut self) -> &'self mut FlowData {
+    /// A convenience method to return the position of this flow. Fails if the flow is currently
-        unsafe {
+    /// being borrowed mutably.
-            match *self {
+    #[inline(always)]
-                AbsoluteFlow(ref d)    => cast::transmute(d),
+    pub fn position(&self) -> Rect<Au> {
-                BlockFlow(ref d, _)    => cast::transmute(d),
+        do self.with_imm_node |common_info| {
-                FloatFlow(ref d)       => cast::transmute(d),
+            common_info.position
-                InlineBlockFlow(ref d) => cast::transmute(d),
-                InlineFlow(ref d, _)   => cast::transmute(d),
-                RootFlow(ref d, _)     => cast::transmute(d),
-                TableFlow(ref d)       => cast::transmute(d)
-            }
         }
     }
 
-    /// Iterates over the immediate children of this flow.
+    /// A convenience method to return the ID of this flow. Fails if the flow is currently being
-    ///
+    /// borrowed mutably.
-    /// TODO: Fold me into `util::tree`.
+    #[inline(always)]
-    pub fn each_child(@mut self, f: &fn(@mut FlowContext) -> bool) {
+    pub fn id(&self) -> int {
-        let mut current_opt = self.d().first_child;
+        do self.with_imm_node |info| {
-        while !current_opt.is_none() {
+            info.id
-            let current = current_opt.get();
-            if !f(current) {
-                break;
-            }
-            current_opt = current.d().next_sibling;
         }
     }
 
-    /// Adds the given flow to the end of the list of this flow's children. The new child must be
+    pub fn inline(&self) -> @mut InlineFlowData {
-    /// detached from the tree before calling this method.
+        match *self {
-    ///
+            InlineFlow(info) => info,
-    /// TODO: Fold me into `util::tree`.
+            _ => fail!(fmt!("Tried to access inline data of non-inline: f%d", self.id()))
-    pub fn add_child(@mut self, child: @mut FlowContext) {
-        let self_data = self.d(), child_data = child.d();
-
-        assert!(child_data.parent.is_none());
-        assert!(child_data.prev_sibling.is_none());
-        assert!(child_data.next_sibling.is_none());
-
-        match self_data.last_child {
-            None => {
-                self_data.first_child = Some(child);
-            }
-            Some(last_child) => {
-                assert!(last_child.d().next_sibling.is_none());
-                last_child.d().next_sibling = Some(child);
-                child_data.prev_sibling = Some(last_child);
-            }
-        }
-
-        self_data.last_child = Some(child);
-        child_data.parent = Some(self);
-    }
-
-    /// Removes the given flow from the tree.
-    ///
-    /// TODO: Fold me into `util::tree`.
-    pub fn remove_child(@mut self, child: @mut FlowContext) {
-        let self_data = self.d(), child_data = child.d();
-
-        assert!(child_data.parent.is_some());
-        assert!(ptr::ref_eq(&*child_data.parent.get(), self));
-
-        match child_data.prev_sibling {
-            None => self_data.first_child = child_data.next_sibling,
-            Some(prev_sibling) => {
-                prev_sibling.d().next_sibling = child_data.next_sibling;
-                child_data.prev_sibling = None;
-            }
-        }
-
-        match child_data.next_sibling {
-            None => self_data.last_child = child.d().prev_sibling,
-            Some(next_sibling) => {
-                next_sibling.d().prev_sibling = Some(next_sibling);
-                child_data.next_sibling = None;
-            }
-        }
-
-        child_data.parent = None;
-    }
-
-    pub fn inline(&'self mut self) -> &'self mut InlineFlowData {
-        match self {
-            &InlineFlow(_, ref i) => unsafe { cast::transmute(i) },
-            _ => fail!(fmt!("Tried to access inline data of non-inline: f%d", self.d().id))
         }
     }
 
-    pub fn block(&'self mut self) -> &'self mut BlockFlowData {
+    pub fn block(&self) -> @mut BlockFlowData {
-        match self {
+        match *self {
-            &BlockFlow(_, ref mut b) => unsafe { cast::transmute(b) },
+            BlockFlow(info) => info,
-            _ => fail!(fmt!("Tried to access block data of non-block: f%d", self.d().id))
+            _ => fail!(fmt!("Tried to access block data of non-block: f%d", self.id()))
         }
     }
 
-    pub fn root(&'self mut self) -> &'self mut RootFlowData {
+    pub fn root(&self) -> @mut RootFlowData {
-        match self {
+        match *self {
-            &RootFlow(_, ref r) => unsafe { cast::transmute(r) },
+            RootFlow(info) => info,
-            _ => fail!(fmt!("Tried to access root data of non-root: f%d", self.d().id))
+            _ => fail!(fmt!("Tried to access root data of non-root: f%d", self.id()))
         }
     }
 
-    pub fn bubble_widths(@mut self, ctx: &mut LayoutContext) {
+    pub fn bubble_widths(&self, ctx: &mut LayoutContext) {
-        match self {
+        match *self {
-            @BlockFlow(*)  => self.bubble_widths_block(ctx),
+            BlockFlow(*)     => self.bubble_widths_block(ctx),
-            @InlineFlow(*) => self.bubble_widths_inline(ctx),
+            InlineFlow(info) => info.bubble_widths_inline(ctx),
-            @RootFlow(*)   => self.bubble_widths_root(ctx),
+            RootFlow(info)   => info.bubble_widths_root(ctx),
-            _ => fail!(fmt!("Tried to bubble_widths of flow: f%d", self.d().id))
+            _ => fail!(fmt!("Tried to bubble_widths of flow: f%d", self.id()))
         }
     }
 
-    pub fn assign_widths(@mut self, ctx: &mut LayoutContext) {
+    pub fn assign_widths(&self, ctx: &mut LayoutContext) {
-        match self {
+        match *self {
-            @BlockFlow(*)  => self.assign_widths_block(ctx),
+            BlockFlow(*)     => self.assign_widths_block(ctx),
-            @InlineFlow(*) => self.assign_widths_inline(ctx),
+            InlineFlow(info) => info.assign_widths_inline(ctx),
-            @RootFlow(*)   => self.assign_widths_root(ctx),
+            RootFlow(info)   => info.assign_widths_root(ctx),
-            _ => fail!(fmt!("Tried to assign_widths of flow: f%d", self.d().id))
+            _ => fail!(fmt!("Tried to assign_widths of flow: f%d", self.id()))
         }
     }
 
-    pub fn assign_height(@mut self, ctx: &mut LayoutContext) {
+    pub fn assign_height(&self, ctx: &mut LayoutContext) {
-        match self {
+        match *self {
-            @BlockFlow(*)  => self.assign_height_block(ctx),
+            BlockFlow(*)     => self.assign_height_block(ctx),
-            @InlineFlow(*) => self.assign_height_inline(ctx),
+            InlineFlow(info) => info.assign_height_inline(ctx),
-            @RootFlow(*)   => self.assign_height_root(ctx),
+            RootFlow(info)   => info.assign_height_root(ctx),
-            _ => fail!(fmt!("Tried to assign_height of flow: f%d", self.d().id))
+            _ => fail!(fmt!("Tried to assign_height of flow: f%d", self.id()))
         }
     }
 
-    pub fn build_display_list_recurse(@mut self,
+    pub fn build_display_list_recurse(&self,
                                       builder: &DisplayListBuilder,
                                       dirty: &Rect<Au>,
                                       offset: &Point2D<Au>,
                                       list: &Cell<DisplayList>) {
-        let d = self.d(); // FIXME: borrow checker workaround
+        do self.with_imm_node |info| {
-        debug!("FlowContext::build_display_list at %?: %s", d.position, self.debug_str());
+            debug!("FlowContext::build_display_list at %?: %s", info.position, self.debug_str());
+        }
 
-        match self {
+        match *self {
-            @RootFlow(*) => self.build_display_list_root(builder, dirty, offset, list),
+            RootFlow(info) => info.build_display_list_root(builder, dirty, offset, list),
-            @BlockFlow(*) => self.build_display_list_block(builder, dirty, offset, list),
+            BlockFlow(*) => self.build_display_list_block(builder, dirty, offset, list),
-            @InlineFlow(*) => self.build_display_list_inline(builder, dirty, offset, list),
+            InlineFlow(info) => info.build_display_list_inline(builder, dirty, offset, list),
             _ => fail!(fmt!("Tried to build_display_list_recurse of flow: %?", self))
         }
     }
 
     // Actual methods that do not require much flow-specific logic
-    pub fn foldl_all_boxes<B:Copy>(&mut self,
+    pub fn foldl_all_boxes<B:Copy>(&self, seed: B, cb: &fn(a: B, b: @mut RenderBox) -> B) -> B {
-                                   seed: B,
+        match *self {
-                                   cb: &fn(a: B, b: @mut RenderBox) -> B)
+            RootFlow(root) => {
-                                   -> B {
+                let root = &mut *root;
-        match self {
-            &RootFlow(*)   => {
-                let root = self.root(); // FIXME: borrow checker workaround
                 root.box.map_default(seed, |box| { cb(seed, *box) })
             }
-            &BlockFlow(*)  => {
+            BlockFlow(block) => {
-                let block = self.block(); // FIXME: borrow checker workaround
+                let block = &mut *block;
                 block.box.map_default(seed, |box| { cb(seed, *box) })
             }
-            &InlineFlow(*) => {
+            InlineFlow(inline) => {
-                let inline = self.inline(); // FIXME: borrow checker workaround
+                let inline = &mut *inline;
                 inline.boxes.foldl(seed, |acc, box| { cb(*acc, *box) })
             }
             _ => fail!(fmt!("Don't know how to iterate node's RenderBoxes for %?", self))
         }
     }
 
-    pub fn foldl_boxes_for_node<B:Copy>(&mut self,
+    pub fn foldl_boxes_for_node<B:Copy>(&self,
                                         node: AbstractNode,
                                         seed: B,
-                                        cb: &fn(a: B, @mut RenderBox) -> B)
+                                        callback: &fn(a: B, @mut RenderBox) -> B)
                                         -> B {
         do self.foldl_all_boxes(seed) |acc, box| {
-            if box.d().node == node { cb(acc, box) }
+            if box.d().node == node {
-            else { acc }
+                callback(acc, box)
+            } else {
+                acc
+            }
         }
     }
 
-    pub fn iter_all_boxes(&mut self, cb: &fn(@mut RenderBox) -> bool) {
+    pub fn iter_all_boxes(&self, cb: &fn(@mut RenderBox) -> bool) {
-        match self {
+        match *self {
-            &RootFlow(*)   => {
+            RootFlow(root) => {
-                let root = self.root(); // FIXME: borrow checker workaround
+                let root = &mut *root;
                 for root.box.each |box| {
                     if !cb(*box) {
                         break;
                     }
                 }
             }
-            &BlockFlow(*)  => {
+            BlockFlow(block) => {
-                let block = self.block(); // FIXME: borrow checker workaround
+                let block = &mut *block;
                 for block.box.each |box| {
                     if !cb(*box) {
                         break;
                     }
                 }
             }
-            &InlineFlow(*) => {
+            InlineFlow(inline) => {
-                let inline = self.inline(); // FIXME: borrow checker workaround
+                let inline = &mut *inline;
                 for inline.boxes.each |box| {
                     if !cb(*box) {
                         break;
@@ -314,10 +347,10 @@ impl<'self> FlowContext {
         }
     }
 
-    pub fn iter_boxes_for_node(&mut self, node: AbstractNode, cb: &fn(@mut RenderBox) -> bool) {
+    pub fn iter_boxes_for_node(&self, node: AbstractNode, callback: &fn(@mut RenderBox) -> bool) {
         for self.iter_all_boxes |box| {
             if box.d().node == node {
-                if !cb(box) {
+                if !callback(box) {
                     break;
                 }
             }
@@ -325,13 +358,13 @@ impl<'self> FlowContext {
     }
 }
 
-impl BoxedMutDebugMethods for FlowContext {
+impl DebugMethods for FlowContext {
-    fn dump(@mut self) {
+    fn dump(&self) {
         self.dump_indent(0);
     }
 
     /// Dumps the flow tree, for debugging, with indentation.
-    fn dump_indent(@mut self, indent: uint) {
+    fn dump_indent(&self, indent: uint) {
         let mut s = ~"|";
         for uint::range(0, indent) |_i| {
             s += ~"---- ";
@@ -346,24 +379,26 @@ impl BoxedMutDebugMethods for FlowContext {
         }
     }
     
-    fn debug_str(@mut self) -> ~str {
+    fn debug_str(&self) -> ~str {
         let repr = match *self {
-            InlineFlow(*) => {
+            InlineFlow(inline) => {
-                let inline = self.inline(); // FIXME: borrow checker workaround
+                let inline = &mut *inline;
                 let mut s = inline.boxes.foldl(~"InlineFlow(children=", |s, box| {
                     fmt!("%s b%d", *s, box.d().id)
                 });
                 s += ~")";
                 s
             },
-            BlockFlow(*) => {
+            BlockFlow(block) => {
-                match self.block().box {
+                let block = &mut *block;
+                match block.box {
                     Some(box) => fmt!("BlockFlow(box=b%d)", box.d().id),
                     None => ~"BlockFlow",
                 }
             },
-            RootFlow(*) => {
+            RootFlow(root) => {
-                match self.root().box {
+                let root = &mut *root;
+                match root.box {
                     Some(box) => fmt!("RootFlo(box=b%d)", box.d().id),
                     None => ~"RootFlow",
                 }
@@ -371,8 +406,9 @@ impl BoxedMutDebugMethods for FlowContext {
             _ => ~"(Unknown flow)"
         };
 
-        let d = self.d(); // FIXME: borrow checker workaround
+        do self.with_imm_node |this_node| {
-        fmt!("f%? %?", d.id, repr)
+            fmt!("f%? %?", this_node.id, repr)
+        }
     }
 }
 

src/servo/layout/inline.rs

@@ -2,26 +2,27 @@
  * 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 core;
 use core::cell::Cell;
+use core;
 use dom::node::AbstractNode;
 use layout::box::*;
 use layout::context::LayoutContext;
 use layout::debug::{BoxedDebugMethods, BoxedMutDebugMethods, DebugMethods};
 use layout::display_list_builder::DisplayListBuilder;
-use layout::flow::{FlowContext, InlineFlow};
+use layout::flow::{FlowContext, FlowData, InlineFlow};
 use layout::text::{UnscannedMethods, adapt_textbox_with_range};
 
+use core::util;
 use geom::{Point2D, Rect, Size2D};
 use gfx::display_list::DisplayList;
 use gfx::geometry::Au;
 use gfx::image::holder;
 use gfx::text::text_run::TextRun;
 use gfx::text::util::*;
+use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft};
+use newcss::values::{CSSTextAlignRight};
 use servo_util::range::Range;
-use newcss::values::{CSSTextAlignCenter, CSSTextAlignJustify, CSSTextAlignLeft, CSSTextAlignRight};
 use std::deque::Deque;
-use core::util;
 
 /*
 Lineboxes are represented as offsets into the child list, rather than
@@ -60,28 +61,30 @@ struct ElementMapping {
     priv entries: ~[NodeRange],
 }
 
-pub impl ElementMapping {
+impl ElementMapping {
-    fn new() -> ElementMapping {
+    pub fn new() -> ElementMapping {
         ElementMapping { entries: ~[] }
     }
 
-    fn add_mapping(&mut self, node: AbstractNode, range: &Range) {
+    pub fn add_mapping(&mut self, node: AbstractNode, range: &Range) {
         self.entries.push(NodeRange::new(node, range))
     }
 
-    fn each(&self, cb: &fn(nr: &NodeRange) -> bool) {
+    pub fn each(&self, cb: &fn(nr: &NodeRange) -> bool) {
         do self.entries.each |nr| { cb(nr) }
     }
 
-    fn eachi(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) {
+    pub fn eachi(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) {
         do self.entries.eachi |i, nr| { cb(i, nr) }
     }
 
-    fn eachi_mut(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) {
+    pub fn eachi_mut(&self, cb: &fn(i: uint, nr: &NodeRange) -> bool) {
         do self.entries.eachi |i, nr| { cb(i, nr) }
     }
 
-    fn repair_for_box_changes(&mut self, old_boxes: &[@mut RenderBox], new_boxes: &[@mut RenderBox]) {
+    pub fn repair_for_box_changes(&mut self,
+                                  old_boxes: &[@mut RenderBox],
+                                  new_boxes: &[@mut RenderBox]) {
         let entries = &mut self.entries;
 
         debug!("--- Old boxes: ---");
@@ -151,13 +154,12 @@ pub impl ElementMapping {
     }
 }
 
-// stack-allocated object for scanning an inline flow into
+/// A stack-allocated object for scanning an inline flow into `TextRun`-containing `TextBox`es.
-// TextRun-containing TextBoxes.
+struct TextRunScanner {
-priv struct TextRunScanner {
     clump: Range,
 }
 
-priv impl TextRunScanner {
+impl TextRunScanner {
     fn new() -> TextRunScanner {
         TextRunScanner {
             clump: Range::empty(),
@@ -165,35 +167,31 @@ priv impl TextRunScanner {
     }
 }
 
-priv impl TextRunScanner {
+impl TextRunScanner {
-    fn scan_for_runs(&mut self, ctx: &mut LayoutContext, flow: @mut FlowContext) {
+    fn scan_for_runs(&mut self, ctx: &mut LayoutContext, flow: FlowContext) {
-        let inline = flow.inline();
+        let inline = &mut *flow.inline();
         assert!(inline.boxes.len() > 0);
+        debug!("TextRunScanner: scanning %u boxes for text runs...", inline.boxes.len());
 
-        let in_boxes = &mut flow.inline().boxes;
+        let mut out_boxes = ~[];
-        //do boxes.swap |in_boxes| {
+        for uint::range(0, inline.boxes.len()) |box_i| {
-            debug!("TextRunScanner: scanning %u boxes for text runs...", in_boxes.len());
+            debug!("TextRunScanner: considering box: %?", inline.boxes[box_i].debug_str());
-            let mut out_boxes = ~[];
+            if box_i > 0 && !can_coalesce_text_nodes(inline.boxes, box_i-1, box_i) {
-
+                self.flush_clump_to_list(ctx, flow, inline.boxes, &mut out_boxes);
-            for uint::range(0, in_boxes.len()) |box_i| {
-                debug!("TextRunScanner: considering box: %?", in_boxes[box_i].debug_str());
-                if box_i > 0 && !can_coalesce_text_nodes(*in_boxes, box_i-1, box_i) {
-                    self.flush_clump_to_list(ctx, flow, *in_boxes, &mut out_boxes);
-                }
-                self.clump.extend_by(1);
-            }
-            // handle remaining clumps
-            if self.clump.length() > 0 {
-                self.flush_clump_to_list(ctx, flow, *in_boxes, &mut out_boxes);
             }
+            self.clump.extend_by(1);
+        }
+        // handle remaining clumps
+        if self.clump.length() > 0 {
+            self.flush_clump_to_list(ctx, flow, inline.boxes, &mut out_boxes);
+        }
 
-            debug!("TextRunScanner: swapping out boxes.");
+        debug!("TextRunScanner: swapping out boxes.");
-            // swap out old and new box list of flow, by supplying
-            // temp boxes as return value to boxes.swap |...|
-        util::swap(in_boxes, &mut out_boxes);
-        //}
 
-        // helper functions
+        // Swap out the old and new box list of the flow.
+        inline.boxes = out_boxes;
+
+        // A helper function.
         fn can_coalesce_text_nodes(boxes: &[@mut RenderBox], left_i: uint, right_i: uint) -> bool {
             assert!(left_i < boxes.len());
             assert!(right_i > 0 && right_i < boxes.len());
@@ -207,29 +205,27 @@ priv impl TextRunScanner {
         }
     }
 
-    // a 'clump' is a range of inline flow leaves that can be merged
+    /// A "clump" is a range of inline flow leaves that can be merged together into a single
-    // together into a single RenderBox. Adjacent text with the same
+    /// `RenderBox`. Adjacent text with the same style can be merged, and nothing else can. 
-    // style can be merged, and nothing else can. 
+    ///
-    //
+    /// The flow keeps track of the `RenderBox`es contained by all non-leaf DOM nodes. This is
-    // the flow keeps track of the RenderBoxes contained by all
+    /// necessary for correct painting order. Since we compress several leaf `RenderBox`es here,
-    // non-leaf DOM nodes. This is necessary for correct painting
+    /// the mapping must be adjusted.
-    // order. Since we compress several leaf RenderBoxes here, the
+    ///
-    // mapping must be adjusted.
+    /// N.B. `in_boxes` is passed by reference, since the old code used a `DVec`. The caller is
-    // 
+    /// responsible for swapping out the list. It is not clear to me (pcwalton) that this is still
-    // N.B. in_boxes is passed by reference, since we cannot
+    /// necessary.
-    // recursively borrow or swap the flow's dvec of boxes. When all
-    // boxes are appended, the caller swaps the flow's box list.
     fn flush_clump_to_list(&mut self,
                            ctx: &mut LayoutContext, 
-                           flow: @mut FlowContext,
+                           flow: FlowContext,
                            in_boxes: &[@mut RenderBox],
                            out_boxes: &mut ~[@mut RenderBox]) {
         assert!(self.clump.length() > 0);
 
         debug!("TextRunScanner: flushing boxes in range=%?", self.clump);
         let is_singleton = self.clump.length() == 1;
-        let is_text_clump = match in_boxes[self.clump.begin()] {
+        let is_text_clump = match *in_boxes[self.clump.begin()] {
-            @UnscannedTextBox(*) => true,
+            UnscannedTextBox(*) => true,
             _ => false
         };
 
@@ -245,14 +241,18 @@ priv impl TextRunScanner {
                 let old_box = in_boxes[self.clump.begin()];
                 let text = old_box.raw_text();
                 let font_style = old_box.font_style();
-                // TODO(Issue #115): use actual CSS 'white-space' property of relevant style.
+
+                // TODO(#115): Use the actual CSS `white-space` property of the relevant style.
                 let compression = CompressWhitespaceNewline;
+
                 let transformed_text = transform_text(text, compression);
-                // TODO(Issue #177): text run creation must account for text-renderability by fontgroup fonts.
+
-                // this is probably achieved by creating fontgroup above, and then letting FontGroup decide
+                // TODO(#177): Text run creation must account for the renderability of text by
-                // which Font to stick into the TextRun.
+                // font group fonts. This is probably achieved by creating the font group above
+                // and then letting `FontGroup` decide which `Font` to stick into the text run.
                 let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style);
                 let run = @fontgroup.create_textrun(transformed_text);
+
                 debug!("TextRunScanner: pushing single text box in range: %?", self.clump);
                 let new_box = adapt_textbox_with_range(old_box.d(),
                                                        run,
@@ -260,23 +260,23 @@ priv impl TextRunScanner {
                 out_boxes.push(new_box);
             },
             (false, true) => {
-                // TODO(Issue #115): use actual CSS 'white-space' property of relevant style.
+                // TODO(#115): Use the actual CSS `white-space` property of the relevant style.
                 let compression = CompressWhitespaceNewline;
 
-                // first, transform/compress text of all the nodes
+                // First, transform/compress text of all the nodes.
-                let transformed_strs : ~[~str] = vec::from_fn(self.clump.length(), |i| {
+                let transformed_strs: ~[~str] = do vec::from_fn(self.clump.length()) |i| {
-                    // TODO(Issue #113): we shoud be passing compression context
+                    // TODO(#113): We should be passing the compression context between calls to
-                    // between calls to transform_text, so that boxes
+                    // `transform_text`, so that boxes starting and/or ending with whitespace can
-                    // starting/ending with whitespace &c can be
+                    // be compressed correctly with respect to the text run.
-                    // compressed correctly w.r.t. the TextRun.
                     let idx = i + self.clump.begin();
                     transform_text(in_boxes[idx].raw_text(), compression)
-                });
+                };
 
-                // next, concatenate all of the transformed strings together, saving the new char indices
+                // Next, concatenate all of the transformed strings together, saving the new
-                let mut run_str : ~str = ~"";
+                // character indices.
-                let mut new_ranges : ~[Range] = ~[];
+                let mut run_str: ~str = ~"";
-                let mut char_total = 0u;
+                let mut new_ranges: ~[Range] = ~[];
+                let mut char_total = 0;
                 for uint::range(0, transformed_strs.len()) |i| {
                     let added_chars = str::char_len(transformed_strs[i]);
                     new_ranges.push(Range::new(char_total, added_chars));
@@ -284,28 +284,32 @@ priv impl TextRunScanner {
                     char_total += added_chars;
                 }
 
-                // create the run, then make new boxes with the run and adjusted text indices
+                // Now create the run.
-
+                //
-                // TODO(Issue #177): text run creation must account for text-renderability by fontgroup fonts.
+                // TODO(#177): Text run creation must account for the renderability of text by
-                // this is probably achieved by creating fontgroup above, and then letting FontGroup decide
+                // font group fonts. This is probably achieved by creating the font group above
-                // which Font to stick into the TextRun.
+                // and then letting `FontGroup` decide which `Font` to stick into the text run.
                 let font_style = in_boxes[self.clump.begin()].font_style();
                 let fontgroup = ctx.font_ctx.get_resolved_font_for_style(&font_style);
                 let run = @TextRun::new(fontgroup.fonts[0], run_str);
+
+                // Make new boxes with the run and adjusted text indices.
                 debug!("TextRunScanner: pushing box(es) in range: %?", self.clump);
                 let clump = self.clump;
                 for clump.eachi |i| {
                     let range = &new_ranges[i - self.clump.begin()];
                     if range.length() == 0 { 
-                        error!("Elided an UnscannedTextbox because it was zero-length after compression; %s",
+                        error!("Elided an `UnscannedTextbox` because it was zero-length after \
-                              in_boxes[i].debug_str());
+                                compression; %s",
+                               in_boxes[i].debug_str());
                         loop
                     }
+
                     let new_box = adapt_textbox_with_range(in_boxes[i].d(), run, range);
                     out_boxes.push(new_box);
                 }
             }
-        } /* /match */
+        } // End of match.
     
         debug!("--- In boxes: ---");
         for in_boxes.eachi |i, box| {
@@ -328,7 +332,7 @@ priv impl TextRunScanner {
 
         let end = self.clump.end(); // FIXME: borrow checker workaround
         self.clump.reset(end, 0);
-    } /* /fn flush_clump_to_list */
+    } // End of `flush_clump_to_list`.
 }
 
 struct PendingLine {
@@ -337,28 +341,28 @@ struct PendingLine {
 }
 
 struct LineboxScanner {
-    flow: @mut FlowContext,
+    flow: FlowContext,
     new_boxes: ~[@mut RenderBox],
     work_list: @mut Deque<@mut RenderBox>,
     pending_line: PendingLine,
     line_spans: ~[Range],
 }
 
-fn LineboxScanner(inline: @mut FlowContext) -> LineboxScanner {
-    assert!(inline.starts_inline_flow());
-
-    LineboxScanner {
-        flow: inline,
-        new_boxes: ~[],
-        work_list: @mut Deque::new(),
-        pending_line: PendingLine {mut range: Range::empty(), mut width: Au(0)},
-        line_spans: ~[]
-    }
-}
-
 impl LineboxScanner {
+    fn new(inline: FlowContext) -> LineboxScanner {
+        assert!(inline.starts_inline_flow());
+
+        LineboxScanner {
+            flow: inline,
+            new_boxes: ~[],
+            work_list: @mut Deque::new(),
+            pending_line: PendingLine {mut range: Range::empty(), mut width: Au(0)},
+            line_spans: ~[]
+        }
+    }
+
     priv fn reset_scanner(&mut self) {
-        debug!("Resetting line box scanner's state for flow f%d.", self.flow.d().id);
+        debug!("Resetting line box scanner's state for flow f%d.", self.flow.id());
         self.line_spans = ~[];
         self.new_boxes = ~[];
         self.reset_linebox();
@@ -412,20 +416,13 @@ impl LineboxScanner {
 
     priv fn swap_out_results(&mut self) {
         debug!("LineboxScanner: Propagating scanned lines[n=%u] to inline flow f%d", 
-               self.line_spans.len(), self.flow.d().id);
+               self.line_spans.len(),
+               self.flow.id());
 
-        //do self.new_boxes.swap |boxes| {
+        let inline_boxes = &mut self.flow.inline().boxes;
-            let inline_boxes = &mut self.flow.inline().boxes;
         util::swap(inline_boxes, &mut self.new_boxes);
-            //inline_boxes = boxes;
+        let lines = &mut self.flow.inline().lines;
-        //    ~[]
-        //};
-        //do self.line_spans.swap |boxes| {
-            let lines = &mut self.flow.inline().lines;
         util::swap(lines, &mut self.line_spans);
-         //   lines = boxes;
-        //    ~[]
-        //};
     }
 
     priv fn flush_current_line(&mut self) {
@@ -450,7 +447,7 @@ impl LineboxScanner {
             linebox_align = CSSTextAlignLeft;
         }
 
-        let slack_width = self.flow.d().position.size.width - self.pending_line.width;
+        let slack_width = self.flow.position().size.width - self.pending_line.width;
         match linebox_align {
             // So sorry, but justified text is more complicated than shuffling linebox coordinates.
             // TODO(Issue #213): implement `text-align: justify`
@@ -478,6 +475,7 @@ impl LineboxScanner {
                 }
             },
         }
+
         // clear line and add line mapping
         debug!("LineboxScanner: Saving information for flushed line %u.", self.line_spans.len());
         self.line_spans.push(line_range);
@@ -486,7 +484,7 @@ impl LineboxScanner {
 
     // return value: whether any box was appended.
     priv fn try_append_to_line(&mut self, ctx: &LayoutContext, in_box: @mut RenderBox) -> bool {
-        let remaining_width = self.flow.d().position.size.width - self.pending_line.width;
+        let remaining_width = self.flow.position().size.width - self.pending_line.width;
         let in_box_width = in_box.d().position.size.width;
         let line_is_empty: bool = self.pending_line.range.length() == 0;
 
@@ -579,6 +577,9 @@ impl LineboxScanner {
 }
 
 pub struct InlineFlowData {
+    /// Data common to all flows.
+    common: FlowData,
+
     // A vec of all inline render boxes. Several boxes may
     // correspond to one Node/Element.
     boxes: ~[@mut RenderBox],
@@ -591,77 +592,84 @@ pub struct InlineFlowData {
     elems: ElementMapping
 }
 
-pub fn InlineFlowData() -> InlineFlowData {
+impl InlineFlowData {
-    InlineFlowData {
+    pub fn new(common: FlowData) -> InlineFlowData {
-        boxes: ~[],
+        InlineFlowData {
-        lines: ~[],
+            common: common,
-        elems: ElementMapping::new(),
+            boxes: ~[],
+            lines: ~[],
+            elems: ElementMapping::new(),
+        }
     }
 }
 
 pub trait InlineLayout {
     fn starts_inline_flow(&self) -> bool;
-
-    fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext);
-    fn assign_widths_inline(@mut self, ctx: &mut LayoutContext);
-    fn assign_height_inline(@mut self, ctx: &mut LayoutContext);
-    fn build_display_list_inline(@mut self, a: &DisplayListBuilder, b: &Rect<Au>, c: &Point2D<Au>,
-                                 d: &Cell<DisplayList>);
 }
 
 impl InlineLayout for FlowContext {
-    fn starts_inline_flow(&self) -> bool { match *self { InlineFlow(*) => true, _ => false } }
+    fn starts_inline_flow(&self) -> bool {
-
+        match *self {
-    fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext) {
+            InlineFlow(*) => true,
-        assert!(self.starts_inline_flow());
+            _ => false
-
-        let mut scanner = TextRunScanner::new();
-        scanner.scan_for_runs(ctx, self);
-
-        let mut min_width = Au(0);
-        let mut pref_width = Au(0);
-
-        let boxes = &mut self.inline().boxes;
-        for boxes.each |box| {
-            debug!("FlowContext[%d]: measuring %s", self.d().id, box.debug_str());
-            min_width = Au::max(min_width, box.get_min_width(ctx));
-            pref_width = Au::max(pref_width, box.get_pref_width(ctx));
         }
+    }
+}
 
-        self.d().min_width = min_width;
+impl InlineFlowData {
-        self.d().pref_width = pref_width;
+    pub fn bubble_widths_inline(@mut self, ctx: &mut LayoutContext) {
+        let mut scanner = TextRunScanner::new();
+        scanner.scan_for_runs(ctx, InlineFlow(self));
+
+        {
+            let this = &mut *self;
+
+            let mut min_width = Au(0);
+            let mut pref_width = Au(0);
+
+            for this.boxes.each |box| {
+                debug!("FlowContext[%d]: measuring %s", self.common.id, box.debug_str());
+                min_width = Au::max(min_width, box.get_min_width(ctx));
+                pref_width = Au::max(pref_width, box.get_pref_width(ctx));
+            }
+
+            this.common.min_width = min_width;
+            this.common.pref_width = pref_width;
+        }
     }
 
-    /* Recursively (top-down) determines the actual width of child
+    /// Recursively (top-down) determines the actual width of child contexts and boxes. When called
-    contexts and boxes. When called on this context, the context has
+    /// on this context, the context has had its width set by the parent context.
-    had its width set by the parent context. */
+    pub fn assign_widths_inline(@mut self, ctx: &mut LayoutContext) {
-    fn assign_widths_inline(@mut self, ctx: &mut LayoutContext) {
+        // Initialize content box widths if they haven't been initialized already.
-        assert!(self.starts_inline_flow());
+        //
+        // TODO: Combine this with `LineboxScanner`'s walk in the box list, or put this into
+        // `RenderBox`.
+        {
+            let this = &mut *self;
+            for this.boxes.each |&box| {
+                let box2 = &mut *box;
+                box.d().position.size.width = match *box2 {
+                    ImageBox(_, ref img) => {
+                        let img2: &mut holder::ImageHolder = unsafe { cast::transmute(img) };
+                        Au::from_px(img2.get_size().get_or_default(Size2D(0,0)).width)
+                    }
+                    TextBox(*) => {
+                        // Text boxes are initialized with dimensions.
+                        box.d().position.size.width
+                    },
+                    // TODO(#225): There will be different cases here for `inline-block` and other
+                    // replaced content.
+                    GenericBox(*) => Au::from_px(45), 
+                    _ => fail!(fmt!("Tried to assign width to unknown Box variant: %?", box))
+                };
+            } // End of for loop.
+        }
 
-        // initialize (content) box widths, if they haven't been
+        let mut scanner = LineboxScanner::new(InlineFlow(self));
-        // already. This could be combined with LineboxScanner's walk
-        // over the box list, and/or put into RenderBox.
-        let boxes = &mut self.inline().boxes;
-        for boxes.each |&box| {
-            let box2 = &mut *box;
-            box.d().position.size.width = match *box2 {
-                ImageBox(_, ref img) => {
-                    let img2: &mut holder::ImageHolder = unsafe { cast::transmute(img) };
-                    Au::from_px(img2.get_size().get_or_default(Size2D(0,0)).width)
-                }
-                TextBox(*) => { /* text boxes are initialized with dimensions */
-                                   box.d().position.size.width
-                },
-                // TODO(Issue #225): different cases for 'inline-block', other replaced content
-                GenericBox(*) => Au::from_px(45), 
-                _ => fail!(fmt!("Tried to assign width to unknown Box variant: %?", box))
-            };
-        } // for boxes.each |box|
-
-        let mut scanner = LineboxScanner(self);
         scanner.scan_for_lines(ctx);
    
-        /* There are no child contexts, so stop here. */
+        // There are no child contexts, so stop here.
 
         // TODO(Issue #225): once there are 'inline-block' elements, this won't be
         // true.  In that case, set the InlineBlockBox's width to the
@@ -670,31 +678,34 @@ impl InlineLayout for FlowContext {
         // 'inline-block' box that created this flow before recursing.
     }
 
-    fn assign_height_inline(@mut self, _ctx: &mut LayoutContext) {
+    pub fn assign_height_inline(&mut self, _: &mut LayoutContext) {
-        // TODO(Issue #226): get CSS 'line-height' property from
+        // TODO(#226): Get the CSS `line-height` property from the containing block's style to
-        // containing block's style to determine minimum linebox height.
+        // determine minimum linebox height.
-        // TODO(Issue #226): get CSS 'line-height' property from each non-replaced
+        //
-        // inline element to determine its height for computing linebox height.
+        // TODO(#226): Get the CSS `line-height` property from each non-replaced inline element to
+        // determine its height for computing linebox height.
+
         let line_height = Au::from_px(20);
         let mut cur_y = Au(0);
 
-        let lines = &mut self.inline().lines;
+        for self.lines.eachi |i, line_span| {
-        for lines.eachi |i, line_span| {
             debug!("assign_height_inline: processing line %u with box span: %?", i, line_span);
-            // coords relative to left baseline
-            let mut linebox_bounding_box = Au::zero_rect();
-            let boxes = &mut self.inline().boxes;
-            for line_span.eachi |box_i| {
-                let cur_box : &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround
 
-                // compute box height.
+            // These coordinates are relative to the left baseline.
+            let mut linebox_bounding_box = Au::zero_rect();
+            let boxes = &mut self.boxes;
+            for line_span.eachi |box_i| {
+                let cur_box = boxes[box_i]; // FIXME: borrow checker workaround
+
+                // Compute the height of each box.
                 let d = cur_box.d(); // FIXME: borrow checker workaround
-                let cur_box : &mut RenderBox = boxes[box_i]; // FIXME: borrow checker workaround
+                let cur_box = &mut *cur_box;    // FIXME: borrow checker workaround
                 d.position.size.height = match *cur_box {
                     ImageBox(_, ref img) => {
                         Au::from_px(img.size().height)
                     }
-                    TextBox(*) => { /* text boxes are initialized with dimensions */
+                    TextBox(*) => {
+                        // Text boxes are initialized with dimensions.
                         d.position.size.height
                     },
                     // TODO(Issue #225): different cases for 'inline-block', other replaced content
@@ -706,72 +717,91 @@ impl InlineLayout for FlowContext {
                     }
                 };
 
-                // compute bounding rect, with left baseline as origin.
+                // Compute the bounding rect with the left baseline as origin. Determining line box
-                // so, linebox height is a matter of lining up ideal baselines,
+                // height is a matter of lining up ideal baselines and then taking the union of all
-                // and then using the union of all these rects.
+                // these rects.
                 let bounding_box = match *cur_box {
-                    // adjust to baseline coords
+                    // Adjust to baseline coordinates.
-                    // TODO(Issue #227): use left/right margins, border, padding for nonreplaced content,
+                    //
-                    // and also use top/bottom  margins, border, padding for replaced or inline-block content.
+                    // TODO(#227): Use left/right margins, border, padding for nonreplaced content,
-                    // TODO(Issue #225): use height, width for 'inline-block', other replaced content
+                    // and also use top/bottom margins, border, padding for replaced or
+                    // inline-block content.
+                    //
+                    // TODO(#225): Use height, width for 'inline-block' and other replaced content.
                     ImageBox(*) | GenericBox(*) => {
                         let box_bounds = d.position;
                         box_bounds.translate(&Point2D(Au(0), -d.position.size.height))
                     },
-                    // adjust bounding box metric to box's horizontal offset
+
-                    // TODO: we can use font metrics directly instead of re-measuring for the bounding box.
+                    // Adjust the bounding box metric to the box's horizontal offset.
+                    //
+                    // TODO: We can use font metrics directly instead of re-measuring for the
+                    // bounding box.
                     TextBox(_, data) => {
                         let text_bounds = data.run.metrics_for_range(&data.range).bounding_box;
                         text_bounds.translate(&Point2D(d.position.origin.x, Au(0)))
                     },
+
                     _ => {
                         let cur_box = boxes[box_i]; // FIXME: borrow checker workaround
                         fail!(fmt!("Tried to compute bounding box of unknown Box variant: %s",
                                    cur_box.debug_str()))
                     }
                 };
-                debug!("assign_height_inline: bounding box for box b%d = %?", cur_box.d().id, bounding_box);
+
+                debug!("assign_height_inline: bounding box for box b%d = %?",
+                       cur_box.d().id,
+                       bounding_box);
+
                 linebox_bounding_box = linebox_bounding_box.union(&bounding_box);
+
                 debug!("assign_height_inline: linebox bounding box = %?", linebox_bounding_box);
             }
+
             let linebox_height = linebox_bounding_box.size.height;
             let baseline_offset = -linebox_bounding_box.origin.y;
-            // now go back and adjust y coordinates to match determined baseline
+
+            // Now go back and adjust the Y coordinates to match the baseline we determined.
             for line_span.eachi |box_i| {
                 let cur_box = boxes[box_i];
-                // TODO(Issue #226): this is completely wrong. Need to use element's 
+
-                // 'line-height' when calculating linebox height. Then, go back over 
+                // TODO(#226): This is completely wrong. We need to use the element's `line-height`
-                // and set y offsets according to 'vertical-align' property of containing block.
+                // when calculating line box height. Then we should go back over and set Y offsets
-                let halfleading = match cur_box {
+                // according to the `vertical-align` property of the containing block.
-                    @TextBox(_, data) => { (data.run.font.metrics.em_size - line_height).scale_by(0.5f) },
+                let halfleading = match *cur_box {
-                    _ => { Au(0) }
+                    TextBox(_, data) => {
+                        (data.run.font.metrics.em_size - line_height).scale_by(0.5f)
+                    },
+                    _ => Au(0),
                 };
-                cur_box.d().position.origin.y = cur_y + halfleading + (baseline_offset - cur_box.d().position.size.height);
+
+                cur_box.d().position.origin.y =
+                    cur_y + halfleading + (baseline_offset - cur_box.d().position.size.height);
             }
             
             cur_y += Au::max(line_height, linebox_height);
-        } // /lines.each |line_span|
+        } // End of `lines.each` loop.
 
-        self.d().position.size.height = cur_y;
+        self.common.position.size.height = cur_y;
     }
 
-    fn build_display_list_inline(@mut self, builder: &DisplayListBuilder, dirty: &Rect<Au>, 
+    pub fn build_display_list_inline(&mut self,
-                                 offset: &Point2D<Au>, list: &Cell<DisplayList>) {
+                                     builder: &DisplayListBuilder,
-
+                                     dirty: &Rect<Au>, 
-        assert!(self.starts_inline_flow());
+                                     offset: &Point2D<Au>,
-
+                                     list: &Cell<DisplayList>) {
-        // TODO(Issue #228): once we form line boxes and have their cached bounds, we can be 
+        // TODO(#228): Once we form line boxes and have their cached bounds, we can be smarter and
-        // smarter and not recurse on a line if nothing in it can intersect dirty
+        // not recurse on a line if nothing in it can intersect the dirty region.
-        let inline = self.inline(); // FIXME: borrow checker workaround
         debug!("FlowContext[%d]: building display list for %u inline boxes",
-               self.d().id, inline.boxes.len());
+               self.common.id,
-        let boxes = &mut self.inline().boxes;
+               self.boxes.len());
-        for boxes.each |box| {
+
+        for self.boxes.each |box| {
             box.build_display_list(builder, dirty, offset, list)
         }
 
-        // TODO(Issue #225): should inline-block elements have flows as children
+        // TODO(#225): Should `inline-block` elements have flows as children of the inline flow or
-        // of the inline flow, or should the flow be nested inside the box somehow?
+        // should the flow be nested inside the box somehow?
     }
+}
 
-} // @FlowContext : InlineLayout

src/servo/layout/layout_task.rs

@@ -15,7 +15,6 @@ use layout::context::LayoutContext;
 use layout::debug::{BoxedMutDebugMethods, DebugMethods};
 use layout::display_list_builder::{DisplayListBuilder, FlowDisplayListBuilderMethods};
 use layout::flow::FlowContext;
-use layout::traverse::*;
 use resource::image_cache_task::{ImageCacheTask, ImageResponseMsg};
 use resource::local_image_cache::LocalImageCache;
 use util::task::spawn_listener;
@@ -35,6 +34,7 @@ use gfx::render_task::{RenderMsg, RenderTask};
 use newcss::select::SelectCtx;
 use newcss::stylesheet::Stylesheet;
 use newcss::types::OriginAuthor;
+use servo_util::tree::TreeUtils;
 use std::net::url::Url;
 
 pub type LayoutTask = SharedChan<Msg>;
@@ -165,11 +165,12 @@ impl Layout {
         self.css_select_ctx.append_sheet(sheet.take(), OriginAuthor);
     }
 
+    /// The high-level routine that performs layout tasks.
     fn handle_build(&mut self, data: &BuildData) {
         let node = &data.node;
-        // FIXME: Bad copy
+        // FIXME: Bad copy!
         let doc_url = copy data.url;
-        // FIXME: Bad clone
+        // FIXME: Bad clone!
         let dom_event_chan = data.dom_event_chan.clone();
 
         debug!("layout: received layout request for: %s", doc_url.to_str());
@@ -177,12 +178,13 @@ impl Layout {
         debug!("layout: parsed Node tree");
         debug!("%?", node.dump());
 
-        // Reset the image cache
+        // Reset the image cache.
         self.local_image_cache.next_round(self.make_on_image_available_cb(dom_event_chan));
 
         let screen_size = Size2D(Au::from_px(data.window_size.width as int),
                                  Au::from_px(data.window_size.height as int));
 
+        // Create a layout context for use throughout the following passes.
         let mut layout_ctx = LayoutContext {
             image_cache: self.local_image_cache,
             font_ctx: self.font_ctx,
@@ -190,8 +192,10 @@ impl Layout {
             screen_size: Rect(Point2D(Au(0), Au(0)), screen_size)
         };
 
+        // Initialize layout data for each node.
+        //
+        // FIXME: This is inefficient. We don't need an entire traversal to do this!
         do time("layout: aux initialization") {
-            // TODO: this is dumb. we don't need an entire traversal to do this
             node.initialize_style_for_subtree(&mut self.layout_refs);
         }
 
@@ -205,10 +209,10 @@ impl Layout {
             }
         }
 
-        let layout_root: @mut FlowContext = do time("layout: tree construction") {
+        // Construct the flow tree.
+        let layout_root: FlowContext = do time("layout: tree construction") {
             let mut builder = LayoutTreeBuilder::new();
-            let layout_root: @mut FlowContext = match builder.construct_trees(&layout_ctx,
+            let layout_root: FlowContext = match builder.construct_trees(&layout_ctx, *node) {
-                                                                              *node) {
                 Ok(root) => root,
                 Err(*) => fail!(~"Root flow should always exist")
             };
@@ -219,13 +223,21 @@ impl Layout {
             layout_root
         };
 
+        // Perform the primary layout passes over the flow tree to compute the locations of all
+        // the boxes.
         do time("layout: main layout") {
-            /* perform layout passes over the flow tree */
+            for layout_root.traverse_postorder |flow| {
-            do layout_root.traverse_postorder |f| { f.bubble_widths(&mut layout_ctx) }
+                flow.bubble_widths(&mut layout_ctx);
-            do layout_root.traverse_preorder  |f| { f.assign_widths(&mut layout_ctx) }
+            };
-            do layout_root.traverse_postorder |f| { f.assign_height(&mut layout_ctx) }
+            for layout_root.traverse_preorder |flow| {
+                flow.assign_widths(&mut layout_ctx);
+            };
+            for layout_root.traverse_postorder |flow| {
+                flow.assign_height(&mut layout_ctx);
+            };
         }
 
+        // Build the display list, and send it to the renderer.
         do time("layout: display list building") {
             let builder = DisplayListBuilder {
                 ctx: &layout_ctx,
@@ -233,28 +245,25 @@ impl Layout {
 
             let display_list = @Cell(DisplayList::new());
             
-            // TODO: set options on the builder before building
+            // TODO: Set options on the builder before building.
-            // TODO: be smarter about what needs painting
+            // TODO: Be smarter about what needs painting.
-            layout_root.build_display_list(&builder,
+            layout_root.build_display_list(&builder, &layout_root.position(), display_list);
-                                           &copy layout_root.d().position,
-                                           display_list);
 
             let render_layer = RenderLayer {
                 display_list: display_list.take(),
-                size: Size2D(screen_size.width.to_px() as uint,
+                size: Size2D(screen_size.width.to_px() as uint, screen_size.height.to_px() as uint)
-                             screen_size.height.to_px() as uint)
             };
 
             self.render_task.send(RenderMsg(render_layer));
         } // time(layout: display list building)
 
-        // Tell content we're done
+        // Tell content that we're done.
         data.content_join_chan.send(());
     }
 
-
+    /// Handles a query from the script task. This is the main routine that DOM functions like
-    fn handle_query(&self, query: LayoutQuery, 
+    /// `getClientRects()` or `getBoundingClientRect()` ultimately invoke.
-                    reply_chan: Chan<LayoutQueryResponse>) {
+    fn handle_query(&self, query: LayoutQuery, reply_chan: Chan<LayoutQueryResponse>) {
         match query {
             ContentBox(node) => {
                 let response = match node.layout_data().flow {

src/servo/layout/root.rs

@@ -10,27 +10,30 @@ use gfx::geometry::Au;
 use layout::block::BlockLayout;
 use layout::box::RenderBox;
 use layout::context::LayoutContext;
-use layout::flow::{FlowContext, RootFlow};
+use layout::flow::{FlowContext, FlowData, RootFlow};
 use layout::display_list_builder::DisplayListBuilder;
 
+use servo_util::tree::{TreeNodeRef, TreeUtils};
+
 pub struct RootFlowData {
+    /// Data common to all flows.
+    common: FlowData,
+
+    /// The render box at the root of the tree.
     box: Option<@mut RenderBox>
 }
 
-pub fn RootFlowData() -> RootFlowData {
+impl RootFlowData {
-    RootFlowData {
+    pub fn new(common: FlowData) -> RootFlowData {
-        box: None
+        RootFlowData {
+            common: common,
+            box: None,
+        }
     }
 }
 
 pub trait RootLayout {
     fn starts_root_flow(&self) -> bool;
-
-    fn bubble_widths_root(@mut self, ctx: &LayoutContext);
-    fn assign_widths_root(@mut self, ctx: &LayoutContext);
-    fn assign_height_root(@mut self, ctx: &LayoutContext);
-    fn build_display_list_root(@mut self, a: &DisplayListBuilder, b: &Rect<Au>,
-                               c: &Point2D<Au>, d: &Cell<DisplayList>);
 }
 
 impl RootLayout for FlowContext {
@@ -40,47 +43,47 @@ impl RootLayout for FlowContext {
             _ => false 
         }
     }
+}
 
-    /* defer to the block algorithm */
+impl RootFlowData {
-    fn bubble_widths_root(@mut self, ctx: &LayoutContext) {
+    /// Defer to the block algorithm.
-        assert!(self.starts_root_flow());
+    pub fn bubble_widths_root(@mut self, ctx: &LayoutContext) {
-        self.bubble_widths_block(ctx)
+        RootFlow(self).bubble_widths_block(ctx)
     }
  
-    fn assign_widths_root(@mut self, ctx: &LayoutContext) { 
+    pub fn assign_widths_root(@mut self, ctx: &LayoutContext) { 
-        assert!(self.starts_root_flow());
+        self.common.position.origin = Au::zero_point();
+        self.common.position.size.width = ctx.screen_size.size.width;
 
-        self.d().position.origin = Au::zero_point();
+        RootFlow(self).assign_widths_block(ctx)
-        self.d().position.size.width = ctx.screen_size.size.width;
-
-        self.assign_widths_block(ctx)
     }
 
-    fn assign_height_root(@mut self, ctx: &LayoutContext) {
+    pub fn assign_height_root(@mut self, ctx: &LayoutContext) {
-        assert!(self.starts_root_flow());
-
         // this is essentially the same as assign_height_block(), except
         // the root adjusts self height to at least cover the viewport.
         let mut cur_y = Au(0);
 
-        for self.each_child |child_ctx| {
+        for RootFlow(self).each_child |child_flow| {
-            child_ctx.d().position.origin.y = cur_y;
+            do child_flow.with_mut_node |child_node| {
-            cur_y += child_ctx.d().position.size.height;
+                child_node.position.origin.y = cur_y;
+                cur_y += child_node.position.size.height;
+            }
         }
 
-        self.d().position.size.height = Au::max(ctx.screen_size.size.height, cur_y);
+        self.common.position.size.height = Au::max(ctx.screen_size.size.height, cur_y);
 
-        do self.with_block_box |box| {
+        do RootFlow(self).with_block_box |box| {
             box.d().position.origin.y = Au(0);
             box.d().position.size.height = Au::max(ctx.screen_size.size.height, cur_y);
             let (_used_top, _used_bot) = box.get_used_height();
         }
     }
 
-    fn build_display_list_root(@mut self, builder: &DisplayListBuilder, dirty: &Rect<Au>, 
+    pub fn build_display_list_root(@mut self,
-                               offset: &Point2D<Au>, list: &Cell<DisplayList>) {
+                                   builder: &DisplayListBuilder,
-        assert!(self.starts_root_flow());
+                                   dirty: &Rect<Au>, 
-
+                                   offset: &Point2D<Au>,
-        self.build_display_list_block(builder, dirty, offset, list);
+                                   list: &Cell<DisplayList>) {
+        RootFlow(self).build_display_list_block(builder, dirty, offset, list);
     }
 }

src/servo/layout/traverse.rs

@@ -1,27 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * 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 layout::flow::FlowContext;
-
-/// A trait for running tree-based traversals over flows contexts.
-pub trait FlowContextTraversals {
-    fn traverse_preorder(@mut self, preorder_cb: &fn(@mut FlowContext));
-    fn traverse_postorder(@mut self, postorder_cb: &fn(@mut FlowContext));
-}
-
-impl FlowContextTraversals for FlowContext {
-    fn traverse_preorder(@mut self, preorder_cb: &fn(@mut FlowContext)) {
-        preorder_cb(self);
-        for self.each_child |child| {
-            child.traverse_preorder(preorder_cb);
-        }
-    }
-
-    fn traverse_postorder(@mut self, postorder_cb: &fn(@mut FlowContext)) {
-        for self.each_child |child| {
-            child.traverse_postorder(postorder_cb);
-        }
-        postorder_cb(self);
-    }
-}

src/servo/net.rs

@@ -1,33 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * 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/. */
-
-export uri, input_stream, channel, io_service, file_channel;
-
-import uri::uri;
-
-iface input_stream {
-    fn close();
-    fn read() -> ~[u8];
-}
-
-iface channel {
-    fn uri() -> uri;
-    fn open() -> input_stream;
-}
-
-iface io_service {
-    fn new_uri(spec: ~str) -> uri;
-    fn new_channel(uri: uri) -> channel;
-}
-
-class file_channel: channel {
-    let bogus : int;
-
-    new() {
-        self.bogus = 0;
-    }
-
-    fn uri() -> uri { fail }
-    fn open() -> input_stream { fail }
-}

src/servo/servo.rc

@@ -104,7 +104,6 @@ pub mod layout {
     pub mod inline;
     pub mod root;
     pub mod text;
-    pub mod traverse;
     mod aux;
 }