layout: Remove duplicate methods from flows

src/servo/layout/block.rs

@@ -16,7 +16,7 @@ use geom::point::Point2D;
 use geom::rect::Rect;
 use gfx::display_list::DisplayList;
 use gfx::geometry::Au;
-use servo_util::tree::TreeUtils;
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct BlockFlowData {
     /// Data common to all flows.
@@ -99,9 +99,9 @@ impl BlockLayout for FlowContext {
         for self.each_child |child_ctx| {
             assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow());
 
-            do child_ctx.with_common_info |child_info| {
-                min_width = au::max(min_width, child_info.min_width);
-                pref_width = au::max(pref_width, child_info.pref_width);
+            do child_ctx.with_immutable_node |child_node| {
+                min_width = au::max(min_width, child_node.min_width);
+                pref_width = au::max(pref_width, child_node.pref_width);
             }
         }
 
@@ -112,39 +112,37 @@ impl BlockLayout for FlowContext {
             pref_width = pref_width.add(&box.get_pref_width(ctx));
         }
 
-        do self.with_common_info |info| {
-            info.min_width = min_width;
-            info.pref_width = pref_width;
+        do self.with_mutable_node |this_node| {
+            this_node.min_width = min_width;
+            this_node.pref_width = pref_width;
         }
     }
 
-    /* Recursively (top-down) determines the actual width of child
-    contexts and boxes. When called on this context, the context has
-    had its width set by the parent context.
-
-    Dual boxes consume some width first, and the remainder is assigned to
-    all child (block) contexts. */
-
-    fn assign_widths_block(&self, _ctx: &LayoutContext) { 
+    /// Recursively (top-down) determines the actual width of child contexts and boxes. When called
+    /// on this context, the context has had its width set by the parent context.
+    ///
+    /// Dual boxes consume some width first, and the remainder is assigned to all child (block)
+    /// contexts.
+    fn assign_widths_block(&self, _: &LayoutContext) { 
         assert!(self.starts_block_flow());
 
-        let mut remaining_width = self.with_common_info(|info| info.position.size.width);
+        let mut remaining_width = self.with_immutable_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
-           for its children. */
+        // Let the box consume some width. It will return the amount remaining 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| {
-            assert!(child_ctx.starts_block_flow() || child_ctx.starts_inline_flow());
-            do child_ctx.with_common_info |child_info| {
-                child_info.position.origin.x = left_used;
-                child_info.position.size.width = remaining_width;
+        for self.each_child |kid| {
+            assert!(kid.starts_block_flow() || kid.starts_inline_flow());
+
+            do kid.with_mutable_node |child_node| {
+                child_node.position.origin.x = left_used;
+                child_node.position.size.width = remaining_width;
             }
         }
     }
@@ -154,15 +152,15 @@ impl BlockLayout for FlowContext {
 
         let mut cur_y = Au(0);
 
-        for self.each_child |child_ctx| {
-            do child_ctx.with_common_info |child_info| {
-                child_info.position.origin.y = cur_y;
-                cur_y += child_info.position.size.height;
+        for self.each_child |kid| {
+            do kid.with_mutable_node |child_node| {
+                child_node.position.origin.y = cur_y;
+                cur_y += child_node.position.size.height;
             }
         }
 
-        do self.with_common_info |info| {
-            info.position.size.height = cur_y;
+        do self.with_mutable_node |this_node| {
+            this_node.position.size.height = cur_y;
         }
 
         let _used_top = Au(0);

src/servo/layout/box_builder.rs

@@ -19,7 +19,7 @@ use gfx::image::holder::ImageHolder;
 use newcss::values::{CSSDisplay, CSSDisplayBlock, CSSDisplayInline, CSSDisplayInlineBlock};
 use newcss::values::{CSSDisplayNone};
 use servo_util::range::Range;
-use servo_util::tree::TreeUtils;
+use servo_util::tree::{TreeNodeRef, TreeUtils};
 
 pub struct LayoutTreeBuilder {
     root_flow: Option<FlowContext>,
@@ -122,7 +122,6 @@ impl BoxGenerator {
         // depending on flow, make a box for this node.
         match self.flow {
             InlineFlow(inline) => {
-
                 let mut inline = &mut *inline;
                 let node_range_start = inline.boxes.len();
                 self.range_stack.push(node_range_start);
@@ -140,42 +139,31 @@ impl BoxGenerator {
                 }
                 // TODO: cases for inline-block, etc.
             },
-            BlockFlow(*) => {
-                do self.flow.with_common_info |flow_info| {
-                    debug!("BoxGenerator[f%d]: point b", flow_info.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)",
-                           flow_info.id,
-                           new_box.d().id,
-                           node.debug_str());
-
-                    assert!(self.flow.block().box.is_none());
-                    //XXXjdm We segfault when returning without this temporary.
-                    let block = self.flow.block();
-                    block.box = Some(new_box);
-                }
+            BlockFlow(block) => {
+                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)",
+                       block.common.id,
+                       new_box.d().id,
+                       node.debug_str());
+
+                assert!(block.box.is_none());
+                block.box = Some(new_box);
             },
-            RootFlow(*) => {
-                do self.flow.with_common_info |info| {
-                    debug!("BoxGenerator[f%d]: point c", info.id);
-                    let new_box = builder.make_box(ctx, box_type, node, self.flow);
-                    debug!("BoxGenerator[f%d]: (node is: %s)", info.id, node.debug_str());
-                    debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)",
-                           info.id,
-                           new_box.d().id,
-                           node.debug_str());
-
-                    assert!(self.flow.root().box.is_none());
-                    //XXXjdm We segfault when returning without this temporary.
-                    let root = self.flow.root();
-                    root.box = Some(new_box);
-                }
+            RootFlow(root) => {
+                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)", root.common.id, node.debug_str());
+                debug!("BoxGenerator[f%d]: attaching box[b%d] to root flow (node: %s)",
+                       root.common.id,
+                       new_box.d().id,
+                       node.debug_str());
+
+                assert!(root.box.is_none());
+                root.box = Some(new_box);
             },
-            _ => {
-                do self.flow.with_common_info |flow_info| {
-                    warn!("push_node() not implemented for flow f%d", flow_info.id)
-                }
-            }
+            _ => warn!("push_node() not implemented for flow f%d", self.flow.id()),
         }
     }
 
@@ -236,14 +224,10 @@ impl BuilderContext {
 
     priv fn attach_child_flow(&self, child: FlowContext) {
         let default_collector = &mut *self.default_collector;
-        do default_collector.flow.with_common_info |flow_info| {
-            do child.with_common_info |child_flow_info| {
-                debug!("BuilderContext: Adding child flow f%? of f%?",
-                       flow_info.id,
-                       child_flow_info.id);
-                default_collector.flow.add_child(child);
-            }
-        }
+        debug!("BuilderContext: Adding child flow f%? of f%?",
+               default_collector.flow.id(),
+               child.id());
+        default_collector.flow.add_child(child);
     }
 
     priv fn create_child_flow_of_type(&self,
@@ -352,23 +336,22 @@ pub impl LayoutTreeBuilder {
         let flow = &mut this_ctx.default_collector.flow;
         let flow: &FlowContext = flow;
         for flow.each_child |child_flow| {
-            do child_flow.with_common_info |child_flow_info| {
-                let node = child_flow_info.node;
-                assert!(node.has_layout_data());
-                node.layout_data().flow = Some(child_flow);
+            do child_flow.with_immutable_node |child_node| {
+                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:
-    //
-    // * split inlines (CSS 2.1 Section 9.2.1.1)
-    // * elide non-preformatted whitespace-only text boxes and their
-    //   flows (CSS 2.1 Section 9.2.2.1).
-    //
-    // The latter can only be done immediately adjacent to, or at the
-    // beginning or end of a block flow. Otherwise, the whitespace
-    // might affect whitespace collapsing with adjacent text.
+    /// Fix up any irregularities such as:
+    ///
+    /// * split inlines (CSS 2.1 Section 9.2.1.1)
+    /// * elide non-preformatted whitespace-only text boxes and their flows (CSS 2.1 Section
+    ///   9.2.2.1).
+    ///
+    /// The latter can only be done immediately adjacent to, or at the beginning or end of a block
+    /// flow. Otherwise, the whitespace might affect whitespace collapsing with adjacent text.
     fn simplify_children_of_flow(&self, _: &LayoutContext, parent_ctx: &BuilderContext) {
         match parent_ctx.default_collector.flow {
             InlineFlow(*) => {
@@ -394,10 +377,9 @@ pub impl LayoutTreeBuilder {
                 // of its RenderBox or FlowContext children, and possibly keep alive other junk
                 let parent_flow = parent_ctx.default_collector.flow;
 
-                let (first_child, last_child) =
-                    do parent_flow.with_common_info |parent_flow_info| {
-                        (parent_flow_info.first_child, parent_flow_info.last_child)
-                    };
+                let (first_child, last_child) = do parent_flow.with_immutable_node |parent_node| {
+                    (parent_node.first_child, parent_node.last_child)
+                };
 
                 // check first/last child for whitespace-ness
                 for first_child.each |first_flow| {

src/servo/layout/display_list_builder.rs

@@ -16,14 +16,14 @@ 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
- hold information about the initial request and desired result---for
- example, whether the DisplayList to be used for painting or hit
- 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 support DL-based hit testing &c.  */
+/// A builder object that manages display list builder should mainly hold information about the
+/// initial request and desired result--for example, whether the `DisplayList` is to be used for
+/// painting or hit 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
+/// support display-list-based hit testing and so forth.
 pub struct DisplayListBuilder<'self> {
     ctx:  &'self LayoutContext,
 }
@@ -53,13 +53,13 @@ impl FlowDisplayListBuilderMethods for FlowContext {
                                     dirty: &Rect<Au>,
                                     offset: &Point2D<Au>,
                                     list: &Cell<DisplayList>) {
-        // adjust the dirty rect to child flow context coordinates
-        do child_flow.with_common_info |child_flow_info| {
-            let abs_flow_bounds = child_flow_info.position.translate(offset);
-            let adj_offset = offset.add(&child_flow_info.position.origin);
+        // Adjust the dirty rect to child flow context coordinates.
+        do child_flow.with_immutable_node |child_node| {
+            let abs_flow_bounds = child_node.position.translate(offset);
+            let adj_offset = offset.add(&child_node.position.origin);
 
             debug!("build_display_list_for_child: rel=%?, abs=%?",
-                   child_flow_info.position,
+                   child_node.position,
                    abs_flow_bounds);
             debug!("build_display_list_for_child: dirty=%?, offset=%?", dirty, offset);