Revamp how images are handled. HTMLImageElement sends a prefetch message to the image cache during parsing. Other miscellaneous cleanup.

src/servo/content/content_task.rs

@@ -20,6 +20,7 @@ use gfx::compositor::Compositor;
 use html::lexer::spawn_html_lexer_task;
 use layout::layout_task;
 use layout_task::{LayoutTask, BuildMsg};
+use resource::image_cache_task::ImageCacheTask;
 
 use css::styles::Stylesheet;
 
@@ -59,9 +60,12 @@ enum PingMsg {
 
 type ContentTask = Chan<ControlMsg>;
 
-fn ContentTask<S: Compositor Send Copy>(layout_task: LayoutTask, +compositor: S, resource_task: ResourceTask) -> ContentTask {
+fn ContentTask<S: Compositor Send Copy>(layout_task: LayoutTask,
+                                        +compositor: S,
+                                        resource_task: ResourceTask,
+                                        img_cache_task: ImageCacheTask) -> ContentTask {
     do task().sched_mode(SingleThreaded).spawn_listener::<ControlMsg> |from_master| {
-        Content(layout_task, compositor, from_master, resource_task).start();
+        Content(layout_task, compositor, from_master, resource_task, img_cache_task).start();
     }
 }
 
@@ -81,6 +85,7 @@ fn join_layout(scope: NodeScope, layout_task: LayoutTask) {
 struct Content<C:Compositor> {
     compositor: C,
     layout_task: LayoutTask,
+    image_cache_task: ImageCacheTask,
     from_master: comm::Port<ControlMsg>,
     event_port: comm::Port<Event>,
 
@@ -100,7 +105,8 @@ struct Content<C:Compositor> {
 fn Content<C:Compositor>(layout_task: LayoutTask, 
                          compositor: C, 
                          from_master: Port<ControlMsg>,
-                         resource_task: ResourceTask) -> Content<C> {
+                         resource_task: ResourceTask,
+                         img_cache_task: ImageCacheTask) -> Content<C> {
 
     let jsrt = jsrt();
     let cx = jsrt.cx();
@@ -117,6 +123,7 @@ fn Content<C:Compositor>(layout_task: LayoutTask,
 
     Content {
         layout_task : layout_task,
+        image_cache_task : img_cache_task,
         compositor : compositor,
         from_master : from_master,
         event_port : event_port,
@@ -159,7 +166,8 @@ impl<C:Compositor> Content<C> {
 
             let result = html::hubbub_html_parser::parse_html(self.scope,
                                                               url,
-                                                              self.resource_task);
+                                                              self.resource_task,
+                                                              self.image_cache_task);
 
             let root = result.root;
             let css_rules = result.style_port.recv();

src/servo/css/resolve/apply.rs

@@ -1,6 +1,9 @@
-#[doc="Applies the appropriate CSS style to boxes."]
+/**
+ * Applies the appropriate CSS style to nodes.
+*/
 
 use au = gfx::geometry;
+use dom::node::{Node, NodeTree};
 use dom::element::*;
 use layout::base::{RenderBox, SpecifiedStyle, RenderBoxTree};
 use layout::context::LayoutContext;
@@ -16,7 +19,7 @@ trait ResolveMethods<T> {
 }
 
 impl CSSValue<CSSBackgroundColor> : ResolveMethods<CSSBackgroundColor> {
-    pure fn initial() -> CSSBackgroundColor { return BgTransparent; }
+    pure fn initial() -> CSSBackgroundColor { return BgColorTransparent; }
 }
 
 impl CSSValue<CSSDisplay> : ResolveMethods<CSSDisplay> {
@@ -33,14 +36,14 @@ impl CSSValue<CSSFontSize> : ResolveMethods<CSSFontSize> {
 
 
 struct StyleApplicator {
-    box: @RenderBox,
+    node: Node,
     reflow: fn~(),
 }
 
 // TODO: normalize this into a normal preorder tree traversal function
-fn apply_style(layout_ctx: &LayoutContext, box: @RenderBox, reflow: fn~()) {
+fn apply_style(layout_ctx: &LayoutContext, node: Node, reflow: fn~()) {
     let applicator = StyleApplicator {
-        box: box,
+        node: node,
         reflow: reflow
     };
 
@@ -49,14 +52,15 @@ fn apply_style(layout_ctx: &LayoutContext, box: @RenderBox, reflow: fn~()) {
 
 // TODO: this is misleadingly-named. It is actually trying to resolve CSS 'inherit' values.
 
-#[doc="A wrapper around a set of functions that can be applied as a top-down traversal of layout
+/** A wrapper around a set of functions that can be applied as a
-       boxes."]
+ * top-down traversal of layout boxes.
-fn inheritance_wrapper(layout_ctx: &LayoutContext, box : @RenderBox, reflow: fn~()) {
+ */
+fn inheritance_wrapper(layout_ctx: &LayoutContext, node : Node, reflow: fn~()) {
     let applicator = StyleApplicator {
-        box: box,
+        node: node,
         reflow: reflow
     };
-    applicator.apply_style(layout_ctx);
+    applicator.resolve_style(layout_ctx);
 }
 
 /*
@@ -103,41 +107,19 @@ impl StyleApplicator {
     fn apply_css_style(layout_ctx: &LayoutContext) {
         let reflow = copy self.reflow;
 
-        do RenderBoxTree.each_child(self.box) |child| {
+        do NodeTree.each_child(self.node) |child| {
             inheritance_wrapper(layout_ctx, child, reflow); true
         }
     }
 
-    #[doc="Applies CSS style to a layout box.
+    /** 
-
+     * Convert the cascaded, specified style for this node into a resolved style:
-      Get the specified style and apply the existing traits to a
+     * one which additionally resolves the values of Initial, Inherit based on 
-      layout box.  If a trait does not exist, calculate the default
+     * defaults and node parent style. It also converts Node attributes into 
-      value for the given type of element and use that instead.
+     * equivalent inline style declarations (TODO: where is this defined??)
-
+     */
-     "]
+    fn resolve_style(_layout_ctx: &LayoutContext) {
-    fn apply_style(layout_ctx: &LayoutContext) {
+        // TODO: implement
-
-        // Right now, we only handle images.
-        do self.box.node.read |node| {
-            match node.kind {
-              ~dom::node::Element(element) => {
-                match element.kind {
-                  ~HTMLImageElement(*) => {
-                    let url = element.get_attr(~"src");
-                    
-                    if url.is_some() {
-                        // FIXME: Some sort of BASE HREF support!
-                        // FIXME: Parse URLs!
-                        let new_url = make_url(option::unwrap(url), Some(copy layout_ctx.doc_url));
-                        self.box.data.background_image = Some(ImageHolder(new_url, layout_ctx.image_cache, self.reflow))
-                    };
-                  }
-                  _ => { /* Ignore. */ }
-                }
-              }
-              _ => { /* Ignore. */ }
-            }
-        }
     }
 }
 

src/servo/css/styles.rs

@@ -14,6 +14,7 @@ use util::color::css_colors::{white, black};
 use layout::context::LayoutContext;
 
 type SpecifiedStyle = {mut background_color : CSSValue<CSSBackgroundColor>,
+                       mut background_image : CSSValue<CSSBackgroundImage>,
                         mut display_type : CSSValue<CSSDisplay>,
                         mut font_size : CSSValue<CSSFontSize>,
                         mut height : CSSValue<BoxSizing>,
@@ -75,6 +76,7 @@ fn empty_style_for_node_kind(kind: NodeKind) -> SpecifiedStyle {
     let display_type = kind.default_display_type();
 
     {mut background_color : Initial,
+     mut background_image: Initial,
      mut display_type : Specified(display_type),
      mut font_size : Initial,
      mut height : Initial,

src/servo/css/values.rs

@@ -1,5 +1,6 @@
 use SharedColor = util::color::Color;
 use cmp::Eq;
+use std::net::url::Url;
 
 #[doc = "
   Defines how css rules, both selectors and style specifications, are
@@ -101,7 +102,7 @@ enum CSSBackgroundAttachment {
 
 enum CSSBackgroundColor {
     BgColor(SharedColor),
-    BgTransparent
+    BgColorTransparent
 }
 
 enum CSSBackgroundRepeat {
@@ -111,6 +112,11 @@ enum CSSBackgroundRepeat {
     BgNoRepeat
 }
 
+enum CSSBackgroundImage {
+    BgImage(Url),
+    BgImageNone,
+}
+
 enum CSSColor {
     TextColor(SharedColor)
 }
@@ -232,7 +238,7 @@ impl CSSBackgroundColor: cmp::Eq {
     pure fn eq(other: &CSSBackgroundColor) -> bool {
         match (self, *other) {
             (BgColor(a), BgColor(b)) => a == b,
-            (BgTransparent, BgTransparent) => true,
+            (BgColorTransparent, BgColorTransparent) => true,
             (_, _) => false
         }
     }

src/servo/dom/element.rs

@@ -2,6 +2,7 @@ use au = gfx::geometry;
 use au::au;
 use dvec::DVec;
 use geom::size::Size2D;
+use std::net::url::Url;
 
 struct ElementData {
     tag_name: ~str,
@@ -47,6 +48,16 @@ fn Attr(name: ~str, value: ~str) -> Attr {
     }
 }
 
+fn HTMLImageData() -> HTMLImageData {
+    HTMLImageData {
+        image: None
+    }
+}
+
+struct HTMLImageData {
+    mut image: Option<Url>
+}
+
 enum HeadingLevel {
     Heading1,
     Heading2,
@@ -69,9 +80,7 @@ enum ElementKind {
     HTMLHeadElement,
     HTMLHeadingElement(HeadingLevel),
     HTMLHtmlElement,
-    // TODO: should not take this as argument--it is fetched from
+    HTMLImageElement(HTMLImageData),
-    // layout task as requested.
-    HTMLImageElement({mut size: Size2D<au>}),
     HTMLInputElement,
     HTMLItalicElement,
     HTMLLinkElement,

src/servo/dom/node.rs

@@ -71,7 +71,7 @@ impl Node : DebugMethods {
     }
 
     fn debug_str() -> ~str {
-        fmt!("%?", self.read(|n| copy n.kind ))
+        do self.read |n| { fmt!("%?", n.kind) }
     }
 }
 

src/servo/engine.rs

@@ -37,7 +37,7 @@ fn EngineTask_<C: Compositor Send Copy>(
 
         let render_task = RenderTask(compositor);
         let layout_task = LayoutTask(render_task, image_cache_task);
-        let content_task = ContentTask(layout_task, compositor, resource_task);
+        let content_task = ContentTask(layout_task, compositor, resource_task, image_cache_task);
 
         Engine {
             compositor: compositor,

src/servo/html/hubbub_html_parser.rs

@@ -1,10 +1,12 @@
 use au = gfx::geometry;
-use dom::element::UnknownElement;
+use content::content_task::ContentTask;
+use css::values::Stylesheet;
+use dom::element::*;
+use dom::event::{Event, ReflowEvent};
 use dom::node::{Comment, Doctype, DoctypeData, Text,
                 Element, Node, NodeScope};
-use dom::element::*;
+use resource::image_cache_task::ImageCacheTask;
-use css::values::Stylesheet;
+use resource::image_cache_task;
-use geom::size::Size2D;
 use resource::resource_task::{Done, Load, Payload, ResourceTask};
 
 use comm::{Chan, Port};
@@ -138,7 +140,7 @@ fn build_element_kind(tag: &str) -> ~ElementKind {
     else if tag == ~"h5" { ~HTMLHeadingElement(Heading5) }
     else if tag == ~"h6" { ~HTMLHeadingElement(Heading6) }
     else if tag == ~"html" { ~HTMLHtmlElement }
-    else if tag == ~"img" { ~HTMLImageElement({ mut size: au::zero_size() }) }
+    else if tag == ~"img" { ~HTMLImageElement(HTMLImageData()) }
     else if tag == ~"input" { ~HTMLInputElement }
     else if tag == ~"i" { ~HTMLItalicElement }
     else if tag == ~"link" { ~HTMLLinkElement }
@@ -162,7 +164,10 @@ fn build_element_kind(tag: &str) -> ~ElementKind {
     else { ~UnknownElement }
 }
 
-fn parse_html(scope: NodeScope, url: Url, resource_task: ResourceTask) -> HtmlParserResult unsafe {
+fn parse_html(scope: NodeScope,
+              url: Url,
+              resource_task: ResourceTask,
+              image_cache_task: ImageCacheTask) -> HtmlParserResult unsafe {
     // Spawn a CSS parser to receive links to CSS style sheets.
     let (css_port, css_chan): (comm::Port<Stylesheet>, comm::Chan<CSSMessage>) =
             do task::spawn_conversation |css_port: comm::Port<CSSMessage>,
@@ -217,10 +222,10 @@ fn parse_html(scope: NodeScope, url: Url, resource_task: ResourceTask) -> HtmlPa
                                       from_slice(attribute.value)));
             }
 
-            // Spawn additional parsing, network loads, etc. from opening tag
+            // Spawn additional parsing, network loads, etc. from tag and attrs
-            match elem.tag_name {
+            match elem.kind {
                 //Handle CSS style sheets from <link> elements
-                ~"link" => {
+                ~HTMLLinkElement => {
                     match (elem.get_attr(~"rel"), elem.get_attr(~"href")) {
                         (Some(rel), Some(href)) if rel == ~"stylesheet" => {
                             debug!("found CSS stylesheet: %s", href);
@@ -228,6 +233,14 @@ fn parse_html(scope: NodeScope, url: Url, resource_task: ResourceTask) -> HtmlPa
                         }
                         _ => {}
                     }
+                },
+                ~HTMLImageElement(d) => {
+                    do elem.get_attr(~"src").iter |img_url_str| {
+                        let img_url = make_url(copy img_url_str, Some(copy *url));
+                        d.image = Some(copy img_url);
+                        // inform the image cache to load this, but don't store a handle.
+                        image_cache_task.send(image_cache_task::Prefetch(move img_url));
+                    }
                 }
                 //TODO: handle inline styles ('style' attr)
                 _ => {}

src/servo/image/holder.rs

@@ -1,7 +1,8 @@
 use std::net::url::Url;
-use std::arc::{ARC, clone};
+use std::arc::{ARC, clone, get};
 use resource::image_cache_task::ImageCacheTask;
 use resource::image_cache_task;
+use geom::size::Size2D;
 
 /** A struct to store image data. The image will be loaded once, the
     first time it is requested, and an arc will be stored.  Clones of
@@ -17,9 +18,10 @@ pub struct ImageHolder {
 
 }
 
-fn ImageHolder(-url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageHolder {
+fn ImageHolder(url : &Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageHolder {
+    debug!("ImageHolder() %?", url.to_str());
     let holder = ImageHolder {
-        url : Some(copy url),
+        url : Some(copy *url),
         image : None,
         image_cache_task : image_cache_task,
         reflow_cb : copy cb,
@@ -30,23 +32,34 @@ fn ImageHolder(-url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> Image
     // but they are intended to be spread out in time. Ideally prefetch
     // should be done as early as possible and decode only once we
     // are sure that the image will be used.
-    image_cache_task.send(image_cache_task::Prefetch(copy url));
+    image_cache_task.send(image_cache_task::Prefetch(copy *url));
-    image_cache_task.send(image_cache_task::Decode(move url));
+    image_cache_task.send(image_cache_task::Decode(copy *url));
 
     holder
 }
 
 impl ImageHolder {
+    fn get_size() -> Option<Size2D<int>> {
+        debug!("get_size() %?", self.url);
+        match self.get_image() {
+            Some(img) => { 
+                let img_ref = get(&img);
+                Some(Size2D(img_ref.width as int,
+                            img_ref.height as int))
+            },
+            None => None
+        }
+    }
+
     // This function should not be called by two tasks at the same time
     fn get_image() -> Option<ARC<~Image>> {
+        debug!("get_image() %?", self.url);
+
         // If this is the first time we've called this function, load
         // the image and store it for the future
         if self.image.is_none() {
             assert self.url.is_some();
-
+            let url = copy self.url.get();
-            let mut temp = None;
-            temp <-> self.url;
-            let url = option::unwrap(temp);
 
             let response_port = Port();
             self.image_cache_task.send(image_cache_task::GetImage(copy url, response_port.chan()));
@@ -68,7 +81,7 @@ impl ImageHolder {
                 None
               }
               image_cache_task::ImageFailed => {
-                #info("image was not ready for %s", url.to_str());
+                debug!("image was not ready for %s", url.to_str());
                 // FIXME: Need to schedule another layout when the image is ready
                 None
               }

src/servo/layout/base.rs

@@ -8,7 +8,7 @@ use core::dvec::DVec;
 use core::to_str::ToStr;
 use core::rand;
 use css::styles::SpecifiedStyle;
-use css::values::{BoxSizing, Length, Px, CSSDisplay, Specified, BgColor, BgTransparent};
+use css::values::{BoxSizing, Length, Px, CSSDisplay, Specified, BgColor, BgColorTransparent};
 use dl = gfx::display_list;
 use dom::element::{ElementKind, HTMLDivElement, HTMLImageElement};
 use dom::node::{Element, Node, NodeData, NodeKind, NodeTree};
@@ -121,6 +121,7 @@ enum FlowContextData {
 struct FlowContext {
     kind: FlowContextData,
     data: FlowLayoutData,
+    mut node: Option<Node>,
     /* reference to parent, children flow contexts */
     tree: tree::Tree<@FlowContext>,
     /* TODO: debug only */
@@ -131,6 +132,7 @@ fn FlowContext(id: int, kind: FlowContextData, tree: tree::Tree<@FlowContext>) -
     FlowContext {
         kind: kind,
         data: FlowLayoutData(),
+        node: None,
         tree: tree,
         id: id
     }
@@ -230,7 +232,7 @@ fn BoxLayoutData() -> BoxLayoutData {
 
 enum BoxData {
     GenericBox,
-    ImageBox(Size2D<au>),
+    ImageBox(ImageHolder),
     TextBox(TextBoxData)
 }
 
@@ -270,7 +272,11 @@ impl @RenderBox {
         }
     }
 
-    pure fn get_min_width() -> au {
+    /** In general, these functions are transitively impure because they
+     * may cause glyphs to be allocated. For now, it's impure because of 
+     * holder.get_image()
+    */
+    fn get_min_width() -> au {
         match self.kind {
             // TODO: this should account for min/pref widths of the
             // box element in isolation. That includes
@@ -279,18 +285,15 @@ impl @RenderBox {
             // that of its children to arrive at the context width.
             GenericBox => au(0),
             // TODO: consult CSS 'width', margin, border.
-            // TODO: If image isn't available, consult Node
+            // TODO: If image isn't available, consult 'width'.
-            // attrs, etc. to determine intrinsic dimensions. These
+            ImageBox(i) => au::from_px(i.get_size().get_default(Size2D(0,0)).width),
-            // dimensions are not defined by CSS 2.1, but are defined
-            // by the HTML5 spec in Section 4.8.1
-            ImageBox(size) => size.width,
             TextBox(d) => d.runs.foldl(au(0), |sum, run| {
                 au::max(sum, run.min_break_width())
             })
         }
     }
 
-    pure fn get_pref_width() -> au {
+    fn get_pref_width() -> au {
         match self.kind {
             // TODO: this should account for min/pref widths of the
             // box element in isolation. That includes
@@ -298,11 +301,7 @@ impl @RenderBox {
             // FlowContext will combine the width of this element and
             // that of its children to arrive at the context width.
             GenericBox => au(0),
-            // TODO: If image isn't available, consult Node
+            ImageBox(i) => au::from_px(i.get_size().get_default(Size2D(0,0)).width),
-            // attrs, etc. to determine intrinsic dimensions. These
-            // dimensions are not defined by CSS 2.1, but are defined
-            // by the HTML5 spec in Section 4.8.1
-            ImageBox(size) => size.width,
             // TODO: account for line breaks, etc. The run should know
             // how to compute its own min and pref widths, and should
             // probably cache them.
@@ -330,6 +329,7 @@ impl @RenderBox {
         (au(0), au(0))
     }
 
+
     // This will be very unhappy if it is getting run in parallel with
     // anything trying to read the background image
     fn get_image() -> Option<ARC<~Image>> {
@@ -408,7 +408,7 @@ impl @RenderBox {
                         let boxed_color = self.node.style().background_color;
                         let color = match boxed_color {
                             Specified(BgColor(c)) => c,
-                            Specified(BgTransparent) | _ => util::color::rgba(0,0,0,0.0)
+                            Specified(BgColorTransparent) | _ => util::color::rgba(0,0,0,0.0)
                         };
                         list.push(~dl::SolidColor(bounds, color.red, color.green, color.blue));
                     }

src/servo/layout/box_builder.rs

@@ -6,6 +6,7 @@ use css::values::{CSSDisplay, DisplayBlock, DisplayInline, DisplayInlineBlock, D
 use css::values::{Inherit, Initial, Specified};
 use dom::element::*;
 use dom::node::{Comment, Doctype, Element, Text, Node, NodeTree, LayoutData};
+use image::holder::ImageHolder;
 use layout::base::{RenderBox, BoxData, GenericBox, ImageBox, TextBox, RenderBoxTree};
 use layout::base::{FlowContext, FlowContextData, BlockFlow, InlineFlow, InlineBlockFlow, RootFlow, FlowTree};
 use layout::block::BlockFlowData;
@@ -21,7 +22,6 @@ use servo_text::font_cache::FontCache;
 export LayoutTreeBuilder;
 
 struct LayoutTreeBuilder {
-    mut root_box: Option<@RenderBox>,
     mut root_ctx: Option<@FlowContext>,
     mut next_bid: int,
     mut next_cid: int
@@ -29,7 +29,6 @@ struct LayoutTreeBuilder {
 
 fn LayoutTreeBuilder() -> LayoutTreeBuilder {
     LayoutTreeBuilder {
-        root_box: None,
         root_ctx: None,
         next_bid: -1,
         next_cid: -1
@@ -43,7 +42,9 @@ impl LayoutTreeBuilder {
 
     /** Creates necessary box(es) and flow context(s) for the current DOM node,
     and recurses on its children. */
-    fn construct_recursively(layout_ctx: &LayoutContext, cur_node: Node, parent_ctx: @FlowContext, parent_box: @RenderBox) {
+    fn construct_recursively(layout_ctx: &LayoutContext, cur_node: Node, 
+                             parent_ctx: @FlowContext, parent_box: Option<@RenderBox>) {
+
         let style = cur_node.style();
         
         // DEBUG
@@ -58,10 +59,7 @@ impl LayoutTreeBuilder {
         };
 
         // first, create the proper box kind, based on node characteristics
-        let box_data = match self.create_box_data(layout_ctx, cur_node, simulated_display) {
+        let box_data = self.create_box_data(layout_ctx, cur_node, simulated_display);
-            None => return,
-            Some(data) => data
-        };
 
         // then, figure out its proper context, possibly reorganizing.
         let next_ctx: @FlowContext = match box_data {
@@ -94,27 +92,40 @@ impl LayoutTreeBuilder {
             }
         };
 
+        // store reference to the flow context which contains any boxes
+        // that correspond to cur_node
+        assert cur_node.has_aux();
+        do cur_node.aux |data| { data.flow = Some(next_ctx) }
+
         // make box, add box to any context-specific list.
         let mut new_box = self.make_box(cur_node, parent_ctx, box_data);
         debug!("Assign ^box to flow: %?", next_ctx.debug_str());
 
         match next_ctx.kind {
-            InlineFlow(d) => { d.boxes.push(new_box) }
+            InlineFlow(d) => {
+                d.boxes.push(new_box);
+
+                if (parent_box.is_some()) {
+                    let parent = parent_box.get();
+
+                    // connect the box to its parent box
+                    debug!("In inline flow f%?, set child b%? of parent b%?", next_ctx.id, parent.id, new_box.id);
+                    RenderBoxTree.add_child(parent, new_box);
+                }
+            }
             BlockFlow(d) => { d.box = Some(new_box) }
             _ => {} // TODO: float lists, etc.
         };
 
-        // connect the box to its parent box
-        debug!("Adding child box b%? of b%?", parent_box.id, new_box.id);
-        RenderBoxTree.add_child(parent_box, new_box);
     
         if (!next_ctx.eq(&parent_ctx)) {
             debug!("Adding child flow f%? of f%?", parent_ctx.id, next_ctx.id);
             FlowTree.add_child(parent_ctx, next_ctx);
         }
         // recurse
+        // TODO: don't set parent box unless this is an inline flow?
         do NodeTree.each_child(cur_node) |child_node| {
-            self.construct_recursively(layout_ctx, child_node, next_ctx, new_box); true
+            self.construct_recursively(layout_ctx, child_node, next_ctx, Some(new_box)); true
         }
 
         // Fixup any irregularities, such as split inlines (CSS 2.1 Section 9.2.1.1)
@@ -164,12 +175,11 @@ impl LayoutTreeBuilder {
 
     /** entry point for box creation. Should only be 
     called on root DOM element. */
-    fn construct_trees(layout_ctx: &LayoutContext, root: Node) -> Result<@RenderBox, ()> {
+    fn construct_trees(layout_ctx: &LayoutContext, root: Node) -> Result<@FlowContext, ()> {
         self.root_ctx = Some(self.make_ctx(RootFlow(RootFlowData()), tree::empty()));
-        self.root_box = Some(self.make_box(root, self.root_ctx.get(), GenericBox));
 
-        self.construct_recursively(layout_ctx, root, self.root_ctx.get(), self.root_box.get());
+        self.construct_recursively(layout_ctx, root, self.root_ctx.get(), None);
-        return Ok(self.root_box.get())
+        return Ok(self.root_ctx.get())
     }
 
     fn make_ctx(kind : FlowContextData, tree: tree::Tree<@FlowContext>) -> @FlowContext {
@@ -185,22 +195,27 @@ impl LayoutTreeBuilder {
     }
 
     /* Based on the DOM node type, create a specific type of box */
-    fn create_box_data(layout_ctx: &LayoutContext, node: Node, display: CSSDisplay) -> Option<BoxData> {
+    fn create_box_data(layout_ctx: &LayoutContext, node: Node, display: CSSDisplay) -> BoxData {
         // TODO: handle more types of nodes.
         do node.read |n| {
             match n.kind {
-                ~Doctype(*) | ~Comment(*) => None,
+                ~Doctype(*) | ~Comment(*) => fail ~"Hey, doctypes and comments shouldn't get here! They are display:none!",
                 ~Text(string) => {
                     // TODO: clean this up. Fonts should not be created here.
                     let font = layout_ctx.font_cache.get_test_font();
                     let run = TextRun(font, string);
-                    Some(TextBox(TextBoxData(copy string, ~[move run])))
+                    TextBox(TextBoxData(copy string, ~[move run]))
                 }
                 ~Element(element) => {
                     match (element.kind, display) {
-                        (~HTMLImageElement({size}), _) => Some(ImageBox(size)),
+                        (~HTMLImageElement(d), _) if d.image.is_some() => {
-//                      (_, Specified(_)) => Some(GenericBox),
+                            let holder = ImageHolder(&d.image.get(), 
-                        (_, _) => Some(GenericBox) // TODO: replace this with the commented lines
+                                                     layout_ctx.image_cache,
+                                                     copy layout_ctx.reflow_cb);
+                            ImageBox(holder)
+                        },
+//                      (_, Specified(_)) => GenericBox,
+                        (_, _) => GenericBox // TODO: replace this with the commented lines
 //                      (_, _) => fail ~"Can't create box for Node with non-specified 'display' type"
                     }
                 }

src/servo/layout/context.rs

@@ -10,5 +10,6 @@ struct LayoutContext {
     font_cache: @FontCache,
     image_cache: ImageCacheTask,
     doc_url: Url,
+    reflow_cb: fn~(),
     screen_size: Rect<au>
 }

src/servo/layout/display_list_builder.rs

@@ -2,7 +2,7 @@ export DisplayListBuilder;
 
 use au = gfx::geometry;
 use base::{RenderBox, RenderBoxTree};
-use css::values::{BgColor, BgTransparent, Specified};
+use css::values::{BgColor, BgColorTransparent, Specified};
 use dl = gfx::display_list;
 use dom::node::{Text, NodeScope};
 use dom::rcu::Scope;

src/servo/layout/inline.rs

@@ -116,14 +116,14 @@ impl @FlowContext : InlineLayout {
                 }
             
                 box.data.position.size.width = match box.kind {
-                    ImageBox(sz) => sz.width,
+                    ImageBox(img) => au::from_px(img.get_size().get_default(Size2D(0,0)).width),
                     TextBox(d) => d.runs[0].size().width,
                     // TODO: this should be set to the extents of its children
                     GenericBox(*) => au(0)
                 };
 
                 box.data.position.size.height = match box.kind {
-                    ImageBox(sz) => sz.height,
+                    ImageBox(img) => au::from_px(img.get_size().get_default(Size2D(0,0)).height),
                     TextBox(d) => d.runs[0].size().height,
                     // TODO: this should be set to the extents of its children
                     GenericBox(*) => au(0)

src/servo/layout/layout_task.rs

@@ -60,37 +60,31 @@ fn LayoutTask(render_task: RenderTask, image_cache_task: ImageCacheTask) -> Layo
                         image_cache: image_cache_task,
                         font_cache: font_cache,
                         doc_url: doc_url,
+                        reflow_cb: || event_chan.send(ReflowEvent),
                         // TODO: obtain screen size from a real data source
                         screen_size: Rect(Point2D(au(0), au(0)), Size2D(au::from_px(800), au::from_px(600)))
                     };
 
                     do util::time::time(~"layout") {
+                        // TODO: this is dumb. we don't need 3 separate traversals.
                         node.initialize_style_for_subtree(&layout_ctx, &layout_data_refs);
                         node.recompute_style_for_subtree(&layout_ctx, styles);
-
+                        /* resolve styles (convert relative values) down the node tree */
-                        // TODO: this should care about root flow, not root box.
+                        apply_style(&layout_ctx, node, layout_ctx.reflow_cb);
-                        let root_box: @RenderBox;
+                        
                         let builder = LayoutTreeBuilder();
-                        match builder.construct_trees(&layout_ctx, node) {
+                        let layout_root: @FlowContext = match builder.construct_trees(&layout_ctx, node) {
-                            Ok(root) => root_box = root,
+                            Ok(root) => root,
-                            Err(*) => fail ~"Root node should always exist"
+                            Err(*) => fail ~"Root flow should always exist"
-                        }
+                        };
-
-                        debug!("layout: constructed RenderBox tree");
-                        root_box.dump();
 
                         debug!("layout: constructed Flow tree");
-                        root_box.ctx.dump();
+                        layout_root.dump();
-
-                        /* resolve styles (convert relative values) down the box tree */
-                        let reflow_cb: fn~() = || event_chan.send(ReflowEvent);
-                        apply_style(&layout_ctx, root_box, reflow_cb);
 
                         /* perform layout passes over the flow tree */
-                        let root_flow = root_box.ctx;
+                        do layout_root.traverse_postorder |f| { f.bubble_widths(&layout_ctx) }
-                        do root_flow.traverse_postorder |f| { f.bubble_widths(&layout_ctx) }
+                        do layout_root.traverse_preorder  |f| { f.assign_widths(&layout_ctx) }
-                        do root_flow.traverse_preorder |f| { f.assign_widths(&layout_ctx) }
+                        do layout_root.traverse_postorder |f| { f.assign_height(&layout_ctx) }
-                        do root_flow.traverse_postorder |f| { f.assign_height(&layout_ctx) }
 
                         let dlist = DVec();
                         let builder = dl::DisplayListBuilder {
@@ -98,7 +92,7 @@ fn LayoutTask(render_task: RenderTask, image_cache_task: ImageCacheTask) -> Layo
                         };
                         // TODO: set options on the builder before building
                         // TODO: be smarter about what needs painting
-                        root_flow.build_display_list(&builder, &copy root_flow.data.position, &dlist);
+                        layout_root.build_display_list(&builder, &copy layout_root.data.position, &dlist);
                         render_task.send(render_task::RenderMsg(dlist));
                     }
                 }