Use LocalImageCache in LayoutTask instead of ImageCacheTask

src/servo/css/resolve/apply.rs

@@ -36,14 +36,12 @@ impl CSSValue<CSSFontSize> : ResolveMethods<CSSFontSize> {
 
 struct StyleApplicator {
     node: Node,
-    reflow: fn~(),
 }
 
 // TODO: normalize this into a normal preorder tree traversal function
-fn apply_style(layout_ctx: &LayoutContext, node: Node, reflow: fn~()) {
+fn apply_style(layout_ctx: &LayoutContext, node: Node) {
     let applicator = StyleApplicator {
         node: node,
-        reflow: reflow
     };
 
     applicator.apply_css_style(layout_ctx);
@@ -54,10 +52,9 @@ fn apply_style(layout_ctx: &LayoutContext, node: Node, reflow: fn~()) {
 /** A wrapper around a set of functions that can be applied as a
  * top-down traversal of layout boxes.
  */
-fn inheritance_wrapper(layout_ctx: &LayoutContext, node : Node, reflow: fn~()) {
+fn inheritance_wrapper(layout_ctx: &LayoutContext, node : Node) {
     let applicator = StyleApplicator {
         node: node,
-        reflow: reflow
     };
     applicator.resolve_style(layout_ctx);
 }
@@ -104,10 +101,9 @@ fn resolve_width(box : @RenderBox) {
 
 impl StyleApplicator {
     fn apply_css_style(layout_ctx: &LayoutContext) {
-        let reflow = copy self.reflow;
 
         for NodeTree.each_child(&self.node) |child| {
-            inheritance_wrapper(layout_ctx, *child, copy reflow)
+            inheritance_wrapper(layout_ctx, *child)
         }
     }
 

src/servo/image/holder.rs

@@ -1,7 +1,9 @@
+use core::util::replace;
 use std::net::url::Url;
 use std::arc::{ARC, clone, get};
-use resource::image_cache_task::ImageCacheTask;
+use resource::image_cache_task::{ImageCacheTask, ImageReady, ImageNotReady, ImageFailed};
 use mod resource::image_cache_task;
+use resource::local_image_cache::LocalImageCache;
 use geom::size::Size2D;
 
 /** A struct to store image data. The image will be loaded once, the
@@ -9,24 +11,19 @@ use geom::size::Size2D;
     this arc are given out on demand.
  */
 pub struct ImageHolder {
-    // Invariant: at least one of url and image is not none, except
+    url : Url,
-    // occasionally while get_image is being called
-    mut url : Option<Url>,
     mut image : Option<ARC<~Image>>,
     mut cached_size: Size2D<int>,
-    image_cache_task: ImageCacheTask,
+    local_image_cache: @LocalImageCache,
-    reflow_cb: fn~(),
-
 }
 
-fn ImageHolder(url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageHolder {
+fn ImageHolder(url : Url, local_image_cache: @LocalImageCache) -> ImageHolder {
     debug!("ImageHolder() %?", url.to_str());
     let holder = ImageHolder {
-        url : Some(copy url),
+        url : url,
         image : None,
         cached_size : Size2D(0,0),
-        image_cache_task : image_cache_task,
+        local_image_cache: local_image_cache,
-        reflow_cb : copy cb,
     };
 
     // Tell the image cache we're going to be interested in this url
@@ -34,8 +31,8 @@ fn ImageHolder(url : Url, image_cache_task: ImageCacheTask, cb: fn~()) -> ImageH
     // 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));
+    local_image_cache.prefetch(&holder.url);
-    image_cache_task.send(image_cache_task::Decode(move url));
+    local_image_cache.decode(&holder.url);
 
     holder
 }
@@ -66,57 +63,35 @@ impl ImageHolder {
         }
     }
 
-    // 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() {
-            let url = match copy self.url {
+            match self.local_image_cache.get_image(&self.url).recv() {
-                Some(move url) => url,
+                ImageReady(move image) => {
-                None => fail ~"expected to have a url"
+                    self.image = Some(image);
+                }
+                ImageNotReady => {
+                    debug!("image not ready for %s", self.url.to_str());
+                }
+                ImageFailed => {
+                    debug!("image decoding failed for %s", self.url.to_str());
+                }
+            }
+        }
+
+        // Clone isn't pure so we have to swap out the mutable image option
+        let image = replace(&mut self.image, None);
+
+        let result = match image {
+            Some(ref image) => Some(clone(image)),
+            None => None
         };
 
-            let (response_chan, response_port) = pipes::stream();
+        replace(&mut self.image, move image);
-            self.image_cache_task.send(image_cache_task::GetImage(copy url, response_chan));
-            self.image = match response_port.recv() {
-              image_cache_task::ImageReady(image) => Some(clone(&image)),
-              image_cache_task::ImageNotReady => {
-                // Need to reflow when the image is available
-                let image_cache_task = self.image_cache_task.clone();
-                let reflow = copy self.reflow_cb;
-                do task::spawn |copy url, move reflow| {
-                    let (response_chan, response_port) = pipes::stream();
-                    image_cache_task.send(image_cache_task::WaitForImage(copy url, response_chan));
-                    match response_port.recv() {
-                      image_cache_task::ImageReady(*) => reflow(),
-                      image_cache_task::ImageNotReady => fail /*not possible*/,
-                      image_cache_task::ImageFailed => ()
-                    }
-                }
-                None
-              }
-              image_cache_task::ImageFailed => {
-                debug!("image was not ready for %s", url.to_str());
-                // FIXME: Need to schedule another layout when the image is ready
-                None
-              }
-            };
-        }
 
-        if self.image.is_some() {
+        return result;
-            // Temporarily swap out the arc of the image so we can clone
-            // it without breaking purity, then put it back and return the
-            // clone.  This is not threadsafe.
-            let mut temp = None;
-            temp <-> self.image;
-            let im_arc = option::unwrap(temp);
-            self.image = Some(clone(&im_arc));
-
-            return Some(im_arc);
-        } else {
-            return None;
-        }
     }
 }

src/servo/layout/box_builder.rs

@@ -224,8 +224,7 @@ impl LayoutTreeBuilder {
                         // an ICE (mozilla/rust issue #3601)
                         if d.image.is_some() {
                             let holder = ImageHolder({copy *d.image.get_ref()},
-                                                     layout_ctx.image_cache.clone(),
+                                                     layout_ctx.image_cache);
-                                                     copy layout_ctx.reflow_cb);
 
                             @ImageBox(RenderBoxData(node, ctx, self.next_box_id()), holder)
                         } else {

src/servo/layout/context.rs

@@ -1,4 +1,4 @@
-use resource::image_cache_task::ImageCacheTask;
+use resource::local_image_cache::LocalImageCache;
 use servo_text::font_cache::FontCache;
 use std::net::url::Url;
 use geom::rect::Rect;
@@ -8,8 +8,7 @@ use au = gfx::geometry::au;
 
 struct LayoutContext {
     font_cache: @FontCache,
-    image_cache: ImageCacheTask,
+    image_cache: @LocalImageCache,
     doc_url: Url,
-    reflow_cb: fn~(),
     screen_size: Rect<au>
 }

src/servo/layout/layout_task.rs

@@ -22,7 +22,8 @@ use layout::box_builder::LayoutTreeBuilder;
 use layout::context::LayoutContext;
 use opt = core::option;
 use render_task::RenderTask;
-use resource::image_cache_task::ImageCacheTask;
+use resource::image_cache_task::{ImageCacheTask, ImageResponseMsg};
+use resource::local_image_cache::LocalImageCache;
 use servo_text::font_cache::FontCache;
 use std::arc::ARC;
 use std::net::url::Url;
@@ -60,6 +61,7 @@ fn LayoutTask(render_task: RenderTask,
 struct Layout {
     render_task: RenderTask,
     image_cache_task: ImageCacheTask,
+    local_image_cache: @LocalImageCache,
     from_content: comm::Port<Msg>,
 
     font_cache: @FontCache,
@@ -73,7 +75,8 @@ fn Layout(render_task: RenderTask,
 
     Layout {
         render_task: render_task,
-        image_cache_task: image_cache_task,
+        image_cache_task: image_cache_task.clone(),
+        local_image_cache: @LocalImageCache(move image_cache_task),
         from_content: from_content,
         font_cache: FontCache(),
         layout_refs: DVec()
@@ -133,14 +136,16 @@ impl Layout {
                 debug!("layout: parsed Node tree");
                 debug!("%?", node.dump());
 
+                // Reset the image cache
+                self.local_image_cache.next_round(self.make_on_image_available_cb(to_content));
+
                 let screen_size = Size2D(au::from_px(window_size.width as int),
                                          au::from_px(window_size.height as int));
 
                 let layout_ctx = LayoutContext {
-                    image_cache: self.image_cache_task.clone(),
+                    image_cache: self.local_image_cache,
                     font_cache: self.font_cache,
                     doc_url: doc_url,
-                    reflow_cb: || to_content.send(ReflowEvent),
                     screen_size: Rect(Point2D(au(0), au(0)), screen_size)
                 };
 
@@ -149,7 +154,7 @@ impl Layout {
                     node.initialize_style_for_subtree(&layout_ctx, &self.layout_refs);
                     node.recompute_style_for_subtree(&layout_ctx, &styles);
                     /* resolve styles (convert relative values) down the node tree */
-                    apply_style(&layout_ctx, node, copy layout_ctx.reflow_cb);
+                    apply_style(&layout_ctx, node);
                     
                     let builder = LayoutTreeBuilder();
                     let layout_root: @FlowContext = match builder.construct_trees(&layout_ctx,
@@ -193,5 +198,14 @@ impl Layout {
 
         true
     }
+
+    // When images can't be loaded in time to display they trigger
+    // this callback in some task somewhere. This w
+    fn make_on_image_available_cb(to_content: comm::Chan<Event>) -> ~fn(ImageResponseMsg) {
+        let f: ~fn(ImageResponseMsg) = |_msg| {
+            to_content.send(ReflowEvent)
+        };
+        return f;
+    }
 }
 

src/servo/resource/local_image_cache.rs

@@ -9,27 +9,27 @@ use pipes::{Port, Chan, stream};
 use image_cache_task::{ImageCacheTask, ImageResponseMsg, Prefetch, Decode, GetImage, WaitForImage};
 
 pub fn LocalImageCache(
-    image_cache_task: ImageCacheTask,
+    image_cache_task: ImageCacheTask
-    on_image_available: ~fn(ImageResponseMsg)
 ) -> LocalImageCache {
     LocalImageCache {
+        image_cache_task: image_cache_task,
         round_number: 0,
-        image_cache_task: move image_cache_task,
+        mut on_image_available: None
-        on_image_available: on_image_available
     }
 }
 
 pub struct LocalImageCache {
-    priv mut round_number: uint,
     priv image_cache_task: ImageCacheTask,
-    priv on_image_available: ~fn(ImageResponseMsg)
+    priv mut round_number: uint,
+    priv mut on_image_available: Option<~fn(ImageResponseMsg)>
 }
 
 pub impl LocalImageCache {
     /// The local cache will only do a single remote request for a given
     /// URL in each 'round'. Layout should call this each time it begins
-    fn next_round() {
+    fn next_round(on_image_available: ~fn(ImageResponseMsg)) {
         self.round_number += 1;
+        self.on_image_available = Some(move on_image_available);
     }
 
     fn prefetch(url: &Url) {
@@ -54,7 +54,8 @@ pub impl LocalImageCache {
                 // the compositor should be resonsible for waiting
                 // on the image to load and triggering layout
                 let image_cache_task = self.image_cache_task.clone();
-                let on_image_available = copy self.on_image_available;
+                assert self.on_image_available.is_some();
+                let on_image_available = {copy *self.on_image_available.get_ref()};
                 let url = copy *url;
                 do task::spawn |move url, move on_image_available| {
                     let (response_chan, response_port) = pipes::stream();