servo: Merge #5767 - Refactored image cache task - details below (from glennw:image-cache); r=larsbergstrom,jdm

* Simpler image cache API for clients to use.
 * Significantly fewer threads.
   * One thread for image cache task (multiplexes commands, decoder threads and async resource requests).
   * 4 threads for decoder worker tasks.
 * Removed ReflowEvent hacks in script and layout tasks.
   * Image elements pass a Trusted<T> to image cache, which is used to dirty nodes via script task. Previous use of Untrusted addresses was unsafe.
   * Image requests such as background-image on layout / paint threads trigger repaint only rather than full reflow.
 * Add reflow batching for when multiple images load quickly.
   * Reduces the number of paints loading wikipedia from ~95 to ~35.
 * Reasonably simple to add proper prefetch support in a follow up PR.
 * Async loaded images always construct Image fragments now, instead of generic.
   * Image fragments support the image not being present.
 * Simpler implementation of synchronous image loading for reftests.
 * Removed image holder.
 * image.onload support.
 * image NaturalWidth and NaturalHeight support.
 * Updated WPT expectations.

Source-Repo: https://github.com/servo/servo
Source-Revision: ac0645c2363b5a6ea3930b0857b3a27f1b6d033f

servo/components/compositing/constellation.rs

@@ -21,7 +21,7 @@ use msg::constellation_msg::{Key, KeyState, KeyModifiers, LoadData};
 use msg::constellation_msg::{SubpageId, WindowSizeData};
 use msg::constellation_msg::{self, ConstellationChan, Failure};
 use net_traits::{self, ResourceTask};
-use net_traits::image_cache_task::{ImageCacheTask, ImageCacheTaskClient};
+use net_traits::image_cache_task::ImageCacheTask;
 use net_traits::storage_task::{StorageTask, StorageTaskMsg};
 use profile::mem;
 use profile::time;

servo/components/gfx/paint_task.rs

@@ -27,8 +27,8 @@ use msg::constellation_msg::{ConstellationChan, Failure, PipelineId};
 use msg::constellation_msg::PipelineExitType;
 use profile::time::{self, profile};
 use skia::SkiaGrGLNativeContextRef;
+use std::borrow::ToOwned;
 use std::mem;
-use std::thread::Builder;
 use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender, channel};
 use util::geometry::{Au, ZERO_POINT};
@@ -36,6 +36,7 @@ use util::opts;
 use util::smallvec::SmallVec;
 use util::task::spawn_named_with_send_on_failure;
 use util::task_state;
+use util::task::spawn_named;
 
 /// Information about a hardware graphics layer that layout sends to the painting task.
 #[derive(Clone)]
@@ -432,14 +433,14 @@ impl WorkerThreadProxy {
             let native_graphics_metadata = native_graphics_metadata.clone();
             let font_cache_task = font_cache_task.clone();
             let time_profiler_chan = time_profiler_chan.clone();
-            Builder::new().spawn(move || {
+            spawn_named("PaintWorker".to_owned(), move || {
                 let mut worker_thread = WorkerThread::new(from_worker_sender,
                                                           to_worker_receiver,
                                                           native_graphics_metadata,
                                                           font_cache_task,
                                                           time_profiler_chan);
                 worker_thread.main();
-            }).unwrap();
+            });
             WorkerThreadProxy {
                 receiver: from_worker_receiver,
                 sender: to_worker_sender,

servo/components/layout/construct.rs

@@ -250,24 +250,6 @@ impl<'a> FlowConstructor<'a> {
         node.set_flow_construction_result(result);
     }
 
-    /// Builds the `ImageFragmentInfo` for the given image. This is out of line to guide inlining.
-    fn build_fragment_info_for_image(&mut self, node: &ThreadSafeLayoutNode, url: Option<Url>)
-                                -> SpecificFragmentInfo {
-        match url {
-            None => SpecificFragmentInfo::Generic,
-            Some(url) => {
-                // FIXME(pcwalton): The fact that image fragments store the cache within them makes
-                // little sense to me.
-                SpecificFragmentInfo::Image(box ImageFragmentInfo::new(node,
-                                                         url,
-                                                         self.layout_context
-                                                             .shared
-                                                             .image_cache
-                                                             .clone()))
-            }
-        }
-    }
-
     /// Builds the fragment for the given block or subclass thereof.
     fn build_fragment_for_block(&mut self, node: &ThreadSafeLayoutNode) -> Fragment {
         let specific_fragment_info = match node.type_id() {
@@ -277,12 +259,17 @@ impl<'a> FlowConstructor<'a> {
             }
             Some(NodeTypeId::Element(ElementTypeId::HTMLElement(
                         HTMLElementTypeId::HTMLImageElement))) => {
-                self.build_fragment_info_for_image(node, node.image_url())
+                let image_info = box ImageFragmentInfo::new(node,
+                                                            node.image_url(),
+                                                            &self.layout_context);
+                SpecificFragmentInfo::Image(image_info)
             }
             Some(NodeTypeId::Element(ElementTypeId::HTMLElement(
                         HTMLElementTypeId::HTMLObjectElement))) => {
-                let data = node.get_object_data();
-                self.build_fragment_info_for_image(node, data)
+                let image_info = box ImageFragmentInfo::new(node,
+                                                            node.get_object_data(),
+                                                            &self.layout_context);
+                SpecificFragmentInfo::Image(image_info)
             }
             Some(NodeTypeId::Element(ElementTypeId::HTMLElement(
                         HTMLElementTypeId::HTMLTableElement))) => {
@@ -1031,8 +1018,10 @@ impl<'a> FlowConstructor<'a> {
         };
         let marker_fragment = match node.style().get_list().list_style_image {
             Some(ref url) => {
-                Some(Fragment::new(node,
-                                   self.build_fragment_info_for_image(node, Some((*url).clone()))))
+                let image_info = box ImageFragmentInfo::new(node,
+                                                            Some((*url).clone()),
+                                                            &self.layout_context);
+                Some(Fragment::new(node, SpecificFragmentInfo::Image(image_info)))
             }
             None => {
                 match ListStyleTypeContent::from_list_style_type(node.style()

servo/components/layout/context.rs

@@ -13,17 +13,18 @@ use gfx::display_list::OpaqueNode;
 use gfx::font_cache_task::FontCacheTask;
 use gfx::font_context::FontContext;
 use msg::constellation_msg::ConstellationChan;
+use net_traits::image::base::Image;
+use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageState};
 use script::layout_interface::{Animation, LayoutChan};
-use script_traits::UntrustedNodeAddress;
-use net_traits::local_image_cache::LocalImageCache;
 use std::boxed;
 use std::cell::Cell;
 use std::ptr;
-use std::sync::mpsc::Sender;
-use std::sync::{Arc, Mutex};
+use std::sync::Arc;
+use std::sync::mpsc::{channel, Sender};
 use style::selector_matching::Stylist;
 use url::Url;
 use util::geometry::Au;
+use util::opts;
 
 struct LocalLayoutContext {
     font_context: FontContext,
@@ -55,8 +56,11 @@ fn create_or_get_local_context(shared_layout_context: &SharedLayoutContext)
 
 /// Layout information shared among all workers. This must be thread-safe.
 pub struct SharedLayoutContext {
-    /// The local image cache.
-    pub image_cache: Arc<Mutex<LocalImageCache<UntrustedNodeAddress>>>,
+    /// The shared image cache task.
+    pub image_cache_task: ImageCacheTask,
+
+    /// A channel for the image cache to send responses to.
+    pub image_cache_sender: ImageCacheChan,
 
     /// The current screen size.
     pub screen_size: Size2D<Au>,
@@ -139,4 +143,42 @@ impl<'a> LayoutContext<'a> {
             &mut cached_context.style_sharing_candidate_cache
         }
     }
+
+    pub fn get_or_request_image(&self, url: Url) -> Option<Arc<Image>> {
+        // See if the image is already available
+        let result = self.shared.image_cache_task.get_image_if_available(url.clone());
+
+        match result {
+            Ok(image) => Some(image),
+            Err(state) => {
+                // If we are emitting an output file, then we need to block on
+                // image load or we risk emitting an output file missing the image.
+                let is_sync = opts::get().output_file.is_some();
+
+                match (state, is_sync) {
+                    // Image failed to load, so just return nothing
+                    (ImageState::LoadError, _) => None,
+                    // Not loaded, test mode - load the image synchronously
+                    (_, true) => {
+                        let (sync_tx, sync_rx) = channel();
+                        self.shared.image_cache_task.request_image(url,
+                                                                   ImageCacheChan(sync_tx),
+                                                                   None);
+                        sync_rx.recv().unwrap().image
+                    }
+                    // Not yet requested, async mode - request image from the cache
+                    (ImageState::NotRequested, false) => {
+                        self.shared.image_cache_task.request_image(url,
+                                                                   self.shared.image_cache_sender.clone(),
+                                                                   None);
+                        None
+                    }
+                    // Image has been requested, is still pending. Return no image
+                    // for this paint loop. When the image loads it will trigger
+                    // a reflow and/or repaint.
+                    (ImageState::Pending, false) => None,
+                }
+            }
+        }
+    }
 }

servo/components/layout/display_list_builder.rs

@@ -20,7 +20,6 @@ use fragment::{ScannedTextFragmentInfo, SpecificFragmentInfo};
 use inline::InlineFlow;
 use list_item::ListItemFlow;
 use model::{self, MaybeAuto, ToGfxMatrix};
-use opaque_node::OpaqueNodeMethods;
 
 use geom::{Matrix2D, Point2D, Rect, Size2D, SideOffsets2D};
 use gfx::color;
@@ -36,7 +35,6 @@ use png::{self, PixelsByColorType};
 use msg::compositor_msg::ScrollPolicy;
 use msg::constellation_msg::Msg as ConstellationMsg;
 use msg::constellation_msg::ConstellationChan;
-use net_traits::image::holder::ImageHolder;
 use util::cursor::Cursor;
 use util::geometry::{self, Au, ZERO_POINT, to_px, to_frac_px};
 use util::logical_geometry::{LogicalPoint, LogicalRect, LogicalSize, WritingMode};
@@ -398,18 +396,8 @@ impl FragmentDisplayListBuilding for Fragment {
                                                clip: &ClippingRegion,
                                                image_url: &Url) {
         let background = style.get_background();
-        let mut holder = ImageHolder::new(image_url.clone(),
-                                          layout_context.shared.image_cache.clone());
-        let image = match holder.get_image(self.node.to_untrusted_node_address()) {
-            None => {
-                // No image data at all? Do nothing.
-                //
-                // TODO: Add some kind of placeholder background image.
-                debug!("(building display list) no background image :(");
-                return
-            }
-            Some(image) => image,
-        };
+        let image = layout_context.get_or_request_image(image_url.clone());
+        if let Some(image) = image {
             debug!("(building display list) building background image");
 
             // Use `background-size` to get the size.
@@ -488,6 +476,7 @@ impl FragmentDisplayListBuilding for Fragment {
                 image_rendering: style.get_effects().image_rendering.clone(),
             }), level);
         }
+    }
 
     fn build_display_list_for_background_linear_gradient(&self,
                                                          display_list: &mut DisplayList,
@@ -973,11 +962,8 @@ impl FragmentDisplayListBuilding for Fragment {
                 }
             }
             SpecificFragmentInfo::Image(ref mut image_fragment) => {
-                let image_ref = &mut image_fragment.image;
-                if let Some(image) = image_ref.get_image(self.node.to_untrusted_node_address()) {
-                    debug!("(building display list) building image fragment");
-
                 // Place the image into the display list.
+                if let Some(ref image) = image_fragment.image {
                     display_list.content.push_back(DisplayItem::ImageClass(box ImageDisplayItem {
                         base: BaseDisplayItem::new(stacking_relative_content_box,
                                                    DisplayItemMetadata::new(self.node,
@@ -988,11 +974,6 @@ impl FragmentDisplayListBuilding for Fragment {
                         stretch_size: stacking_relative_content_box.size,
                         image_rendering: self.style.get_effects().image_rendering.clone(),
                     }));
-                } else {
-                    // No image data at all? Do nothing.
-                    //
-                    // TODO: Add some kind of placeholder image.
-                    debug!("(building display list) no image :(");
                 }
             }
             SpecificFragmentInfo::Canvas(ref canvas_fragment_info) => {

servo/components/layout/fragment.rs

@@ -28,8 +28,7 @@ use gfx::display_list::{BLUR_INFLATION_FACTOR, OpaqueNode};
 use gfx::text::glyph::CharIndex;
 use gfx::text::text_run::{TextRun, TextRunSlice};
 use msg::constellation_msg::{ConstellationChan, Msg, PipelineId, SubpageId};
-use net_traits::image::holder::ImageHolder;
-use net_traits::local_image_cache::LocalImageCache;
+use net_traits::image::base::Image;
 use rustc_serialize::{Encodable, Encoder};
 use script_traits::UntrustedNodeAddress;
 use std::borrow::ToOwned;
@@ -297,7 +296,7 @@ impl CanvasFragmentInfo {
 pub struct ImageFragmentInfo {
     /// The image held within this fragment.
     pub replaced_image_fragment_info: ReplacedImageFragmentInfo,
-    pub image: ImageHolder<UntrustedNodeAddress>,
+    pub image: Option<Arc<Image>>,
 }
 
 impl ImageFragmentInfo {
@@ -306,8 +305,8 @@ impl ImageFragmentInfo {
     /// FIXME(pcwalton): The fact that image fragments store the cache in the fragment makes little
     /// sense to me.
     pub fn new(node: &ThreadSafeLayoutNode,
-               image_url: Url,
-               local_image_cache: Arc<Mutex<LocalImageCache<UntrustedNodeAddress>>>)
+               url: Option<Url>,
+               layout_context: &LayoutContext)
                -> ImageFragmentInfo {
         fn convert_length(node: &ThreadSafeLayoutNode, name: &Atom) -> Option<Au> {
             let element = node.as_element();
@@ -316,33 +315,43 @@ impl ImageFragmentInfo {
                    .map(|pixels| Au::from_px(pixels))
         }
 
+        let image = url.and_then(|url| layout_context.get_or_request_image(url));
+
         ImageFragmentInfo {
             replaced_image_fragment_info: ReplacedImageFragmentInfo::new(node,
                 convert_length(node, &atom!("width")),
                 convert_length(node, &atom!("height"))),
-            image: ImageHolder::new(image_url, local_image_cache)
+            image: image,
         }
     }
 
     /// Returns the original inline-size of the image.
     pub fn image_inline_size(&mut self) -> Au {
-        let size = self.image.get_size(self.replaced_image_fragment_info.for_node).unwrap_or(Size2D::zero());
+        match self.image {
+            Some(ref image) => {
                 Au::from_px(if self.replaced_image_fragment_info.writing_mode_is_vertical {
-            size.height
+                    image.height
                 } else {
-            size.width
+                    image.width
                 } as isize)
             }
+            None => Au(0)
+        }
+    }
 
     /// Returns the original block-size of the image.
     pub fn image_block_size(&mut self) -> Au {
-        let size = self.image.get_size(self.replaced_image_fragment_info.for_node).unwrap_or(Size2D::zero());
+        match self.image {
+            Some(ref image) => {
                 Au::from_px(if self.replaced_image_fragment_info.writing_mode_is_vertical {
-            size.width
+                    image.width
                 } else {
-            size.height
+                    image.height
                 } as isize)
             }
+            None => Au(0)
+        }
+    }
 
     /// Tile an image
     pub fn tile_image(position: &mut Au, size: &mut Au,

servo/components/layout/layout_task.rs

@@ -46,16 +46,15 @@ use profile::mem::{self, Report, ReportsChan};
 use profile::time::{self, ProfilerMetadata, profile};
 use profile::time::{TimerMetadataFrameType, TimerMetadataReflowType};
 use net_traits::{load_bytes_iter, ResourceTask};
-use net_traits::image_cache_task::{ImageCacheTask, ImageResponseMsg};
-use net_traits::local_image_cache::{ImageResponder, LocalImageCache};
+use net_traits::image_cache_task::{ImageCacheTask, ImageCacheResult, ImageCacheChan};
 use script::dom::bindings::js::LayoutJS;
 use script::dom::node::{LayoutData, Node};
 use script::layout_interface::{Animation, ContentBoxResponse, ContentBoxesResponse};
 use script::layout_interface::{HitTestResponse, LayoutChan, LayoutRPC};
 use script::layout_interface::{MouseOverResponse, Msg, Reflow, ReflowGoal, ReflowQueryType};
 use script::layout_interface::{ScriptLayoutChan, ScriptReflow, TrustedNodeAddress};
-use script_traits::{ConstellationControlMsg, CompositorEvent, OpaqueScriptLayoutChannel};
-use script_traits::{ScriptControlChan, UntrustedNodeAddress};
+use script_traits::{ConstellationControlMsg, OpaqueScriptLayoutChannel};
+use script_traits::ScriptControlChan;
 use std::borrow::ToOwned;
 use std::cell::Cell;
 use std::mem::transmute;
@@ -74,7 +73,7 @@ use util::geometry::{Au, MAX_RECT};
 use util::logical_geometry::LogicalPoint;
 use util::mem::HeapSizeOf;
 use util::opts;
-use util::smallvec::{SmallVec, SmallVec1, VecLike};
+use util::smallvec::SmallVec;
 use util::task::spawn_named_with_send_on_failure;
 use util::task_state;
 use util::workqueue::WorkQueue;
@@ -86,8 +85,8 @@ pub struct LayoutTaskData {
     /// The root of the flow tree.
     pub root_flow: Option<FlowRef>,
 
-    /// The local image cache.
-    pub local_image_cache: Arc<Mutex<LocalImageCache<UntrustedNodeAddress>>>,
+    /// The image cache.
+    pub image_cache_task: ImageCacheTask,
 
     /// The channel on which messages can be sent to the constellation.
     pub constellation_chan: ConstellationChan,
@@ -145,6 +144,12 @@ pub struct LayoutTask {
     /// The port on which we receive messages from the constellation
     pub pipeline_port: Receiver<LayoutControlMsg>,
 
+    /// The port on which we receive messages from the image cache
+    image_cache_receiver: Receiver<ImageCacheResult>,
+
+    /// The channel on which the image cache can send messages to ourself.
+    image_cache_sender: ImageCacheChan,
+
     /// The channel on which we or others can send messages to ourselves.
     pub chan: LayoutChan,
 
@@ -169,9 +174,6 @@ pub struct LayoutTask {
     /// The channel on which messages can be sent to the resource task.
     pub resource_task: ResourceTask,
 
-    /// The channel on which messages can be sent to the image cache.
-    pub image_cache_task: ImageCacheTask,
-
     /// Public interface to the font cache task.
     pub font_cache_task: FontCacheTask,
 
@@ -185,26 +187,6 @@ pub struct LayoutTask {
     pub rw_data: Arc<Mutex<LayoutTaskData>>,
 }
 
-struct LayoutImageResponder {
-    id: PipelineId,
-    script_chan: ScriptControlChan,
-}
-
-impl ImageResponder<UntrustedNodeAddress> for LayoutImageResponder {
-    fn respond(&self) -> Box<Fn(ImageResponseMsg, UntrustedNodeAddress)+Send> {
-        let id = self.id.clone();
-        let script_chan = self.script_chan.clone();
-        box move |_, node_address| {
-            let ScriptControlChan(ref chan) = script_chan;
-            debug!("Dirtying {:p}", node_address.0);
-            let mut nodes = SmallVec1::new();
-            nodes.vec_push(node_address);
-            drop(chan.send(ConstellationControlMsg::SendEvent(
-                id, CompositorEvent::ReflowEvent(nodes))))
-        }
-    }
-}
-
 impl LayoutTaskFactory for LayoutTask {
     /// Spawns a new layout task.
     fn create(_phantom: Option<&mut LayoutTask>,
@@ -218,7 +200,7 @@ impl LayoutTaskFactory for LayoutTask {
               script_chan: ScriptControlChan,
               paint_chan: PaintChan,
               resource_task: ResourceTask,
-              img_cache_task: ImageCacheTask,
+              image_cache_task: ImageCacheTask,
               font_cache_task: FontCacheTask,
               time_profiler_chan: time::ProfilerChan,
               memory_profiler_chan: mem::ProfilerChan,
@@ -237,7 +219,7 @@ impl LayoutTaskFactory for LayoutTask {
                                              script_chan,
                                              paint_chan,
                                              resource_task,
-                                             img_cache_task,
+                                             image_cache_task,
                                              font_cache_task,
                                              time_profiler_chan,
                                              memory_profiler_chan);
@@ -295,8 +277,6 @@ impl LayoutTask {
            time_profiler_chan: time::ProfilerChan,
            mem_profiler_chan: mem::ProfilerChan)
            -> LayoutTask {
-        let local_image_cache =
-            Arc::new(Mutex::new(LocalImageCache::new(image_cache_task.clone())));
         let screen_size = Size2D(Au(0), Au(0));
         let device = Device::new(
             MediaType::Screen,
@@ -317,6 +297,7 @@ impl LayoutTask {
 
         // Create the channel on which new animations can be sent.
         let (new_animations_sender, new_animations_receiver) = channel();
+        let (image_cache_sender, image_cache_receiver) = channel();
 
         LayoutTask {
             id: id,
@@ -332,13 +313,14 @@ impl LayoutTask {
             mem_profiler_chan: mem_profiler_chan,
             reporter_name: reporter_name,
             resource_task: resource_task,
-            image_cache_task: image_cache_task.clone(),
             font_cache_task: font_cache_task,
             first_reflow: Cell::new(true),
+            image_cache_receiver: image_cache_receiver,
+            image_cache_sender: ImageCacheChan(image_cache_sender),
             rw_data: Arc::new(Mutex::new(
                 LayoutTaskData {
                     root_flow: None,
-                    local_image_cache: local_image_cache,
+                    image_cache_task: image_cache_task,
                     constellation_chan: constellation_chan,
                     screen_size: screen_size,
                     stacking_context: None,
@@ -371,7 +353,8 @@ impl LayoutTask {
                                    url: &Url)
                                    -> SharedLayoutContext {
         SharedLayoutContext {
-            image_cache: rw_data.local_image_cache.clone(),
+            image_cache_task: rw_data.image_cache_task.clone(),
+            image_cache_sender: self.image_cache_sender.clone(),
             screen_size: rw_data.screen_size.clone(),
             screen_size_changed: screen_size_changed,
             constellation_chan: rw_data.constellation_chan.clone(),
@@ -393,21 +376,26 @@ impl LayoutTask {
         enum PortToRead {
             Pipeline,
             Script,
+            ImageCache,
         }
 
         let port_to_read = {
             let sel = Select::new();
             let mut port1 = sel.handle(&self.port);
             let mut port2 = sel.handle(&self.pipeline_port);
+            let mut port3 = sel.handle(&self.image_cache_receiver);
             unsafe {
                 port1.add();
                 port2.add();
+                port3.add();
             }
             let ret = sel.wait();
             if ret == port1.id() {
                 PortToRead::Script
             } else if ret == port2.id() {
                 PortToRead::Pipeline
+            } else if ret == port3.id() {
+                PortToRead::ImageCache
             } else {
                 panic!("invalid select result");
             }
@@ -424,11 +412,15 @@ impl LayoutTask {
                                                    possibly_locked_rw_data)
                     }
                 }
-            },
+            }
             PortToRead::Script => {
                 let msg = self.port.recv().unwrap();
                 self.handle_request_helper(msg, possibly_locked_rw_data)
             }
+            PortToRead::ImageCache => {
+                let _ = self.image_cache_receiver.recv().unwrap();
+                self.repaint(possibly_locked_rw_data)
+            }
         }
     }
 
@@ -458,6 +450,33 @@ impl LayoutTask {
         }
     }
 
+    /// Repaint the scene, without performing style matching. This is typically
+    /// used when an image arrives asynchronously and triggers a relayout and
+    /// repaint.
+    /// TODO: In the future we could detect if the image size hasn't changed
+    /// since last time and avoid performing a complete layout pass.
+    fn repaint<'a>(&'a self,
+                   possibly_locked_rw_data: &mut Option<MutexGuard<'a, LayoutTaskData>>) -> bool {
+        let mut rw_data = self.lock_rw_data(possibly_locked_rw_data);
+
+        let reflow_info = Reflow {
+            goal: ReflowGoal::ForDisplay,
+            page_clip_rect: MAX_RECT,
+        };
+
+        let mut layout_context = self.build_shared_layout_context(&*rw_data,
+                                                                  false,
+                                                                  None,
+                                                                  &self.url);
+
+        self.perform_post_style_recalc_layout_passes(&reflow_info,
+                                                     &mut *rw_data,
+                                                     &mut layout_context);
+
+
+        true
+    }
+
     /// Receives and dispatches messages from other tasks.
     fn handle_request_helper<'a>(&'a self,
                                  request: Msg,
@@ -823,12 +842,6 @@ impl LayoutTask {
 
         let mut rw_data = self.lock_rw_data(possibly_locked_rw_data);
 
-        {
-            // Reset the image cache.
-            let mut local_image_cache = rw_data.local_image_cache.lock().unwrap();
-            local_image_cache.next_round(self.make_on_image_available_cb());
-        }
-
         // TODO: Calculate the "actual viewport":
         // http://www.w3.org/TR/css-device-adapt/#actual-viewport
         let viewport_size = data.window_size.initial_viewport;
@@ -1039,24 +1052,6 @@ impl LayoutTask {
         }
     }
 
-    /// When images can't be loaded in time to display they trigger
-    /// this callback in some task somewhere. This will send a message
-    /// to the script task, and ultimately cause the image to be
-    /// re-requested. We probably don't need to go all the way back to
-    /// the script task for this.
-    ///
-    /// FIXME(pcwalton): Rewrite all of this.
-    fn make_on_image_available_cb(&self) -> Box<ImageResponder<UntrustedNodeAddress>+Send> {
-        // This has a crazy signature because the image cache needs to
-        // make multiple copies of the callback, and the dom event
-        // channel is not a copyable type, so this is actually a
-        // little factory to produce callbacks
-        box LayoutImageResponder {
-            id: self.id.clone(),
-            script_chan: self.script_chan.clone(),
-        } as Box<ImageResponder<UntrustedNodeAddress>+Send>
-    }
-
     /// Handles a message to destroy layout data. Layout data must be destroyed on *this* task
     /// because the struct type is transmuted to a different type on the script side.
     unsafe fn handle_reap_layout_data(&self, layout_data: LayoutData) {

servo/components/layout/wrapper.rs

@@ -116,7 +116,7 @@ pub trait TLayoutNode {
     fn image_url(&self) -> Option<Url> {
         unsafe {
             match HTMLImageElementCast::to_layout_js(self.get_jsmanaged()) {
-                Some(elem) => elem.image().as_ref().map(|url| (*url).clone()),
+                Some(elem) => elem.image_url().as_ref().map(|url| (*url).clone()),
                 None => panic!("not an image!")
             }
         }

servo/components/layout_traits/lib.rs

@@ -50,7 +50,7 @@ pub trait LayoutTaskFactory {
               script_chan: ScriptControlChan,
               paint_chan: PaintChan,
               resource_task: ResourceTask,
-              img_cache_task: ImageCacheTask,
+              image_cache_task: ImageCacheTask,
               font_cache_task: FontCacheTask,
               time_profiler_chan: time::ProfilerChan,
               mem_profiler_chan: mem::ProfilerChan,

servo/components/net/image_cache_task.rs

@@ -2,408 +2,361 @@
  * 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 net_traits::ResourceTask;
+use collections::borrow::ToOwned;
 use net_traits::image::base::{Image, load_from_memory};
-use net_traits::image_cache_task::{ImageResponseMsg, ImageCacheTask, Msg};
-use net_traits::image_cache_task::{load_image_data};
-use profile::time::{self, profile};
-use url::Url;
-
-use std::borrow::ToOwned;
+use net_traits::image_cache_task::{ImageState, ImageCacheTask, ImageCacheChan, ImageCacheCommand, ImageCacheResult};
+use net_traits::load_whole_resource;
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
-use std::mem::replace;
-use std::sync::{Arc, Mutex};
-use std::sync::mpsc::{channel, Receiver, Sender};
+use std::mem;
+use std::sync::Arc;
+use std::sync::mpsc::{channel, Sender, Receiver, Select};
 use util::resource_files::resources_dir_path;
 use util::task::spawn_named;
 use util::taskpool::TaskPool;
+use url::Url;
+use net_traits::{AsyncResponseTarget, ControlMsg, LoadData, ResponseAction, ResourceTask, LoadConsumer};
+use net_traits::image_cache_task::ImageResponder;
 
-pub trait ImageCacheTaskFactory {
-    fn new(resource_task: ResourceTask, task_pool: TaskPool,
-               time_profiler_chan: time::ProfilerChan,
-               load_placeholder: LoadPlaceholder) -> Self;
-    fn new_sync(resource_task: ResourceTask, task_pool: TaskPool,
-                    time_profiler_chan: time::ProfilerChan,
-                    load_placeholder: LoadPlaceholder) -> Self;
+///
+/// TODO(gw): Remaining work on image cache:
+///     * Make use of the prefetch support in various parts of the code.
+///     * Experiment with changing the image 'key' from being a url to a resource id
+///         This might be a decent performance win and/or memory saving in some cases (esp. data urls)
+///     * Profile time in GetImageIfAvailable - might be worth caching these results per paint / layout task.
+///
+
+/// Represents an image that is either being loaded
+/// by the resource task, or decoded by a worker thread.
+struct PendingLoad {
+    bytes: Vec<u8>,
+    result: Option<Result<(), String>>,
+    listeners: Vec<ImageListener>,
 }
 
-impl ImageCacheTaskFactory for ImageCacheTask {
-    fn new(resource_task: ResourceTask, task_pool: TaskPool,
-           time_profiler_chan: time::ProfilerChan,
-           load_placeholder: LoadPlaceholder) -> ImageCacheTask {
-        let (chan, port) = channel();
-        let chan_clone = chan.clone();
+impl PendingLoad {
+    fn new() -> PendingLoad {
+        PendingLoad {
+            bytes: vec!(),
+            result: None,
+            listeners: vec!(),
+        }
+    }
 
-        spawn_named("ImageCacheTask".to_owned(), move || {
-            let mut cache = ImageCache {
-                resource_task: resource_task,
-                port: port,
-                chan: chan_clone,
-                state_map: HashMap::new(),
-                wait_map: HashMap::new(),
-                need_exit: None,
-                task_pool: task_pool,
-                time_profiler_chan: time_profiler_chan,
-                placeholder_data: Arc::new(vec!()),
+    fn add_listener(&mut self, listener: ImageListener) {
+        self.listeners.push(listener);
+    }
+}
+
+/// Represents an image that has completed loading.
+/// Images that fail to load (due to network or decode
+/// failure) are still stored here, so that they aren't
+/// fetched again.
+struct CompletedLoad {
+    image: Option<Arc<Image>>,
+}
+
+impl CompletedLoad {
+    fn new(image: Option<Arc<Image>>) -> CompletedLoad {
+        CompletedLoad {
+            image: image,
+        }
+    }
+}
+
+/// Stores information to notify a client when the state
+/// of an image changes.
+struct ImageListener {
+    sender: ImageCacheChan,
+    responder: Option<Box<ImageResponder>>,
+}
+
+impl ImageListener {
+    fn new(sender: ImageCacheChan, responder: Option<Box<ImageResponder>>) -> ImageListener {
+        ImageListener {
+            sender: sender,
+            responder: responder,
+        }
+    }
+
+    fn notify(self, image: Option<Arc<Image>>) {
+        let ImageCacheChan(ref sender) = self.sender;
+        let msg = ImageCacheResult {
+            responder: self.responder,
+            image: image,
         };
-            cache.run(load_placeholder);
-        });
-
-        ImageCacheTask {
-            chan: chan,
+        sender.send(msg).ok();
     }
 }
 
-    fn new_sync(resource_task: ResourceTask, task_pool: TaskPool,
-                time_profiler_chan: time::ProfilerChan,
-                load_placeholder: LoadPlaceholder) -> ImageCacheTask {
-        let (chan, port) = channel();
+struct ResourceLoadInfo {
+    action: ResponseAction,
+    url: Url,
+}
 
-        spawn_named("ImageCacheTask (sync)".to_owned(), move || {
-            let inner_cache: ImageCacheTask = ImageCacheTaskFactory::new(resource_task, task_pool,
-                                                                         time_profiler_chan, load_placeholder);
+struct ResourceListener {
+    url: Url,
+    sender: Sender<ResourceLoadInfo>,
+}
 
-            loop {
-                let msg: Msg = port.recv().unwrap();
-
-                match msg {
-                    Msg::GetImage(url, response) => {
-                        inner_cache.send(Msg::WaitForImage(url, response));
-                    }
-                    Msg::Exit(response) => {
-                        inner_cache.send(Msg::Exit(response));
-                        break;
-                    }
-                    msg => inner_cache.send(msg)
-                }
-            }
-        });
-
-        ImageCacheTask {
-            chan: chan,
-        }
+impl AsyncResponseTarget for ResourceListener {
+    fn invoke_with_listener(&self, action: ResponseAction) {
+        self.sender.send(ResourceLoadInfo {
+            action: action,
+            url: self.url.clone(),
+        }).unwrap();
     }
 }
 
+/// Implementation of the image cache
 struct ImageCache {
-    /// A handle to the resource task for fetching the image binaries
-    resource_task: ResourceTask,
-    /// The port on which we'll receive client requests
-    port: Receiver<Msg>,
-    /// A copy of the shared chan to give to child tasks
-    chan: Sender<Msg>,
-    /// The state of processing an image for a URL
-    state_map: HashMap<Url, ImageState>,
-    /// List of clients waiting on a WaitForImage response
-    wait_map: HashMap<Url, Arc<Mutex<Vec<Sender<ImageResponseMsg>>>>>,
-    need_exit: Option<Sender<()>>,
+    // Receive commands from clients
+    cmd_receiver: Receiver<ImageCacheCommand>,
+
+    // Receive notifications from the resource task
+    progress_receiver: Receiver<ResourceLoadInfo>,
+    progress_sender: Sender<ResourceLoadInfo>,
+
+    // Receive notifications from the decoder thread pool
+    decoder_receiver: Receiver<DecoderMsg>,
+    decoder_sender: Sender<DecoderMsg>,
+
+    // Worker threads for decoding images.
     task_pool: TaskPool,
-    time_profiler_chan: time::ProfilerChan,
-    // Default image used when loading fails.
-    placeholder_data: Arc<Vec<u8>>,
+
+    // Resource task handle
+    resource_task: ResourceTask,
+
+    // Images that are loading over network, or decoding.
+    pending_loads: HashMap<Url, PendingLoad>,
+
+    // Images that have finished loading (successful or not)
+    completed_loads: HashMap<Url, CompletedLoad>,
+
+    // The placeholder image used when an image fails to load
+    placeholder_image: Option<Arc<Image>>,
 }
 
-#[derive(Clone)]
-enum ImageState {
-    Init,
-    Prefetching(AfterPrefetch),
-    Prefetched(Vec<u8>),
-    Decoding,
-    Decoded(Arc<Image>),
-    Failed
+/// Message that the decoder worker threads send to main image cache task.
+struct DecoderMsg {
+    url: Url,
+    image: Option<Image>,
 }
 
-#[derive(Clone)]
-enum AfterPrefetch {
-    DoDecode,
-    DoNotDecode
-}
-
-pub enum LoadPlaceholder {
-    Preload,
-    Ignore
+/// The types of messages that the main image cache task receives.
+enum SelectResult {
+    Command(ImageCacheCommand),
+    Progress(ResourceLoadInfo),
+    Decoder(DecoderMsg),
 }
 
 impl ImageCache {
-    // Used to preload the default placeholder.
-    fn init(&mut self) {
-        let mut placeholder_url = resources_dir_path();
-        placeholder_url.push("rippy.jpg");
-        let image = load_image_data(Url::from_file_path(&*placeholder_url).unwrap(), self.resource_task.clone(), &self.placeholder_data);
-
-        match image {
-            Err(..) => debug!("image_cache_task: failed loading the placeholder."),
-            Ok(image_data) => self.placeholder_data = Arc::new(image_data),
-        }
-    }
-
-    pub fn run(&mut self, load_placeholder: LoadPlaceholder) {
-        // We have to load the placeholder before running.
-        match load_placeholder {
-            LoadPlaceholder::Preload => self.init(),
-            LoadPlaceholder::Ignore => debug!("image_cache_task: use old behavior."),
-        }
-
-        let mut store_chan: Option<Sender<()>> = None;
-        let mut store_prefetched_chan: Option<Sender<()>> = None;
+    fn run(&mut self) {
+        let mut exit_sender: Option<Sender<()>> = None;
 
         loop {
-            let msg = self.port.recv().unwrap();
+            let result = {
+                let sel = Select::new();
 
-            match msg {
-                Msg::Prefetch(url) => self.prefetch(url),
-                Msg::StorePrefetchedImageData(url, data) => {
-                    store_prefetched_chan.map(|chan| {
-                        chan.send(()).unwrap();
-                    });
-                    store_prefetched_chan = None;
+                let mut cmd_handle = sel.handle(&self.cmd_receiver);
+                let mut progress_handle = sel.handle(&self.progress_receiver);
+                let mut decoder_handle = sel.handle(&self.decoder_receiver);
 
-                    self.store_prefetched_image_data(url, data);
-                }
-                Msg::Decode(url) => self.decode(url),
-                Msg::StoreImage(url, image) => {
-                    store_chan.map(|chan| {
-                        chan.send(()).unwrap();
-                    });
-                    store_chan = None;
-
-                    self.store_image(url, image)
-                }
-                Msg::GetImage(url, response) => self.get_image(url, response),
-                Msg::WaitForImage(url, response) => {
-                    self.wait_for_image(url, response)
-                }
-                Msg::WaitForStore(chan) => store_chan = Some(chan),
-                Msg::WaitForStorePrefetched(chan) => store_prefetched_chan = Some(chan),
-                Msg::Exit(response) => {
-                    assert!(self.need_exit.is_none());
-                    self.need_exit = Some(response);
-                }
+                unsafe {
+                    cmd_handle.add();
+                    progress_handle.add();
+                    decoder_handle.add();
                 }
 
-            let need_exit = replace(&mut self.need_exit, None);
+                let ret = sel.wait();
 
-            match need_exit {
-              Some(response) => {
-                // Wait until we have no outstanding requests and subtasks
-                // before exiting
-                let mut can_exit = true;
-                for (_, state) in self.state_map.iter() {
-                    match *state {
-                        ImageState::Prefetching(..) => can_exit = false,
-                        ImageState::Decoding => can_exit = false,
-
-                        ImageState::Init | ImageState::Prefetched(..) |
-                        ImageState::Decoded(..) | ImageState::Failed => ()
-                    }
-                }
-
-                if can_exit {
-                    response.send(()).unwrap();
-                    break;
+                if ret == cmd_handle.id() {
+                    SelectResult::Command(self.cmd_receiver.recv().unwrap())
+                } else if ret == decoder_handle.id() {
+                    SelectResult::Decoder(self.decoder_receiver.recv().unwrap())
                 } else {
-                    self.need_exit = Some(response);
+                    SelectResult::Progress(self.progress_receiver.recv().unwrap())
+                }
+            };
+
+            match result {
+                SelectResult::Command(cmd) => {
+                    exit_sender = self.handle_cmd(cmd);
+                }
+                SelectResult::Progress(msg) => {
+                    self.handle_progress(msg);
+                }
+                SelectResult::Decoder(msg) => {
+                    self.handle_decoder(msg);
                 }
             }
-              None => ()
+
+            // Can only exit when all pending loads are complete.
+            if let Some(ref exit_sender) = exit_sender {
+                if self.pending_loads.len() == 0 {
+                    exit_sender.send(()).unwrap();
+                    break;
+                }
             }
         }
     }
 
-    fn get_state(&self, url: &Url) -> ImageState {
-        match self.state_map.get(url) {
-            Some(state) => state.clone(),
-            None => ImageState::Init
+    // Handle a request from a client
+    fn handle_cmd(&mut self, cmd: ImageCacheCommand) -> Option<Sender<()>> {
+        match cmd {
+            ImageCacheCommand::Exit(sender) => {
+                return Some(sender);
             }
+            ImageCacheCommand::RequestImage(url, result_chan, responder) => {
+                self.request_image(url, result_chan, responder);
             }
-
-    fn set_state(&mut self, url: Url, state: ImageState) {
-        self.state_map.insert(url, state);
-    }
-
-    fn prefetch(&mut self, url: Url) {
-        match self.get_state(&url) {
-            ImageState::Init => {
-                let to_cache = self.chan.clone();
-                let resource_task = self.resource_task.clone();
-                let url_clone = url.clone();
-                let placeholder = self.placeholder_data.clone();
-                spawn_named("ImageCacheTask (prefetch)".to_owned(), move || {
-                    let url = url_clone;
-                    debug!("image_cache_task: started fetch for {}", url.serialize());
-
-                    let image = load_image_data(url.clone(), resource_task.clone(), &placeholder);
-                    to_cache.send(Msg::StorePrefetchedImageData(url.clone(), image)).unwrap();
-                    debug!("image_cache_task: ended fetch for {}", url.serialize());
-                });
-
-                self.set_state(url, ImageState::Prefetching(AfterPrefetch::DoNotDecode));
-            }
-
-            ImageState::Prefetching(..) | ImageState::Prefetched(..) |
-            ImageState::Decoding | ImageState::Decoded(..) | ImageState::Failed => {
-                // We've already begun working on this image
-            }
-        }
-    }
-
-    fn store_prefetched_image_data(&mut self, url: Url, data: Result<Vec<u8>, ()>) {
-        match self.get_state(&url) {
-          ImageState::Prefetching(next_step) => {
-            match data {
-              Ok(data) => {
-                self.set_state(url.clone(), ImageState::Prefetched(data));
-                match next_step {
-                  AfterPrefetch::DoDecode => self.decode(url),
-                  _ => ()
-                }
-              }
-              Err(..) => {
-                self.set_state(url.clone(), ImageState::Failed);
-                self.purge_waiters(url, || ImageResponseMsg::ImageFailed);
-              }
-            }
-          }
-
-          ImageState::Init
-          | ImageState::Prefetched(..)
-          | ImageState::Decoding
-          | ImageState::Decoded(..)
-          | ImageState::Failed => {
-            panic!("wrong state for storing prefetched image")
-          }
-        }
-    }
-
-    fn decode(&mut self, url: Url) {
-        match self.get_state(&url) {
-            ImageState::Init => panic!("decoding image before prefetch"),
-
-            ImageState::Prefetching(AfterPrefetch::DoNotDecode) => {
-                // We don't have the data yet, queue up the decode
-                self.set_state(url, ImageState::Prefetching(AfterPrefetch::DoDecode))
-            }
-
-            ImageState::Prefetching(AfterPrefetch::DoDecode) => {
-                // We don't have the data yet, but the decode request is queued up
-            }
-
-            ImageState::Prefetched(data) => {
-                let to_cache = self.chan.clone();
-                let url_clone = url.clone();
-                let time_profiler_chan = self.time_profiler_chan.clone();
-
-                self.task_pool.execute(move || {
-                    let url = url_clone;
-                    debug!("image_cache_task: started image decode for {}", url.serialize());
-                    let image = profile(time::ProfilerCategory::ImageDecoding,
-                                        None, time_profiler_chan, || {
-                        load_from_memory(&data)
-                    });
-
-                    let image = image.map(Arc::new);
-                    to_cache.send(Msg::StoreImage(url.clone(), image)).unwrap();
-                    debug!("image_cache_task: ended image decode for {}", url.serialize());
-                });
-
-                self.set_state(url, ImageState::Decoding);
-            }
-
-            ImageState::Decoding | ImageState::Decoded(..) | ImageState::Failed => {
-                // We've already begun decoding
-            }
-        }
-    }
-
-    fn store_image(&mut self, url: Url, image: Option<Arc<Image>>) {
-
-        match self.get_state(&url) {
-          ImageState::Decoding => {
-            match image {
-              Some(image) => {
-                self.set_state(url.clone(), ImageState::Decoded(image.clone()));
-                self.purge_waiters(url, || ImageResponseMsg::ImageReady(image.clone()) );
+            ImageCacheCommand::GetImageIfAvailable(url, consumer) => {
+                let result = match self.completed_loads.get(&url) {
+                    Some(completed_load) => {
+                        completed_load.image.clone().ok_or(ImageState::LoadError)
                     }
                     None => {
-                self.set_state(url.clone(), ImageState::Failed);
-                self.purge_waiters(url, || ImageResponseMsg::ImageFailed );
+                        let pending_load = self.pending_loads.get(&url);
+                        Err(pending_load.map_or(ImageState::NotRequested, |_| ImageState::Pending))
                     }
+                };
+                consumer.send(result).unwrap();
             }
+        };
+
+        None
     }
 
-          ImageState::Init
-          | ImageState::Prefetching(..)
-          | ImageState::Prefetched(..)
-          | ImageState::Decoded(..)
-          | ImageState::Failed => {
-            panic!("incorrect state in store_image")
-          }
-        }
+    // Handle progress messages from the resource task
+    fn handle_progress(&mut self, msg: ResourceLoadInfo) {
+        match msg.action {
+            ResponseAction::HeadersAvailable(_) => {}
+            ResponseAction::DataAvailable(data) => {
+                let pending_load = self.pending_loads.get_mut(&msg.url).unwrap();
+                pending_load.bytes.push_all(data.as_slice());
             }
+            ResponseAction::ResponseComplete(result) => {
+                match result {
+                    Ok(()) => {
+                        let pending_load = self.pending_loads.get_mut(&msg.url).unwrap();
+                        pending_load.result = Some(result);
 
-    fn purge_waiters<F>(&mut self, url: Url, f: F) where F: Fn() -> ImageResponseMsg {
-        match self.wait_map.remove(&url) {
-            Some(waiters) => {
-                let items = waiters.lock().unwrap();
-                for response in items.iter() {
-                    response.send(f()).unwrap();
-                }
-            }
-            None => ()
-        }
-    }
+                        let bytes = mem::replace(&mut pending_load.bytes, vec!());
+                        let url = msg.url.clone();
+                        let sender = self.decoder_sender.clone();
 
-    fn get_image(&self, url: Url, response: Sender<ImageResponseMsg>) {
-        match self.get_state(&url) {
-            ImageState::Init => panic!("request for image before prefetch"),
-            ImageState::Prefetching(AfterPrefetch::DoDecode) => response.send(ImageResponseMsg::ImageNotReady).unwrap(),
-            ImageState::Prefetching(AfterPrefetch::DoNotDecode) | ImageState::Prefetched(..) => panic!("request for image before decode"),
-            ImageState::Decoding => response.send(ImageResponseMsg::ImageNotReady).unwrap(),
-            ImageState::Decoded(image) => response.send(ImageResponseMsg::ImageReady(image)).unwrap(),
-            ImageState::Failed => response.send(ImageResponseMsg::ImageFailed).unwrap(),
-        }
-    }
-
-    fn wait_for_image(&mut self, url: Url, response: Sender<ImageResponseMsg>) {
-        match self.get_state(&url) {
-            ImageState::Init => panic!("request for image before prefetch"),
-
-            ImageState::Prefetching(AfterPrefetch::DoNotDecode) | ImageState::Prefetched(..) => panic!("request for image before decode"),
-
-            ImageState::Prefetching(AfterPrefetch::DoDecode) | ImageState::Decoding => {
-                // We don't have this image yet
-                match self.wait_map.entry(url) {
-                    Occupied(mut entry) => {
-                        entry.get_mut().lock().unwrap().push(response);
-                    }
-                    Vacant(entry) => {
-                        entry.insert(Arc::new(Mutex::new(vec!(response))));
-                    }
-                }
-            }
-
-            ImageState::Decoded(image) => {
-                response.send(ImageResponseMsg::ImageReady(image)).unwrap();
-            }
-
-            ImageState::Failed => {
-                response.send(ImageResponseMsg::ImageFailed).unwrap();
-            }
-        }
-    }
-}
-
-pub fn spawn_listener<F, A>(f: F) -> Sender<A>
-    where F: FnOnce(Receiver<A>) + Send + 'static,
-          A: Send + 'static
-{
-    let (setup_chan, setup_port) = channel();
-
-    spawn_named("ImageCacheTask (listener)".to_owned(), move || {
-        let (chan, port) = channel();
-        setup_chan.send(chan).unwrap();
-        f(port);
+                        self.task_pool.execute(move || {
+                            let image = load_from_memory(bytes.as_slice());
+                            let msg = DecoderMsg {
+                                url: url,
+                                image: image
+                            };
+                            sender.send(msg).unwrap();
                         });
-    setup_port.recv().unwrap()
+                    }
+                    Err(_) => {
+                        let placeholder_image = self.placeholder_image.clone();
+                        self.complete_load(msg.url, placeholder_image);
+                    }
+                }
+            }
+        }
+    }
+
+    // Handle a message from one of the decoder worker threads
+    fn handle_decoder(&mut self, msg: DecoderMsg) {
+        let image = msg.image.map(Arc::new);
+        self.complete_load(msg.url, image);
+    }
+
+    // Change state of a url from pending -> loaded.
+    fn complete_load(&mut self, url: Url, image: Option<Arc<Image>>) {
+        let pending_load = self.pending_loads.remove(&url).unwrap();
+
+        let completed_load = CompletedLoad::new(image.clone());
+        self.completed_loads.insert(url, completed_load);
+
+        for listener in pending_load.listeners.into_iter() {
+            listener.notify(image.clone());
+        }
+    }
+
+    // Request an image from the cache
+    fn request_image(&mut self, url: Url, result_chan: ImageCacheChan, responder: Option<Box<ImageResponder>>) {
+        let image_listener = ImageListener::new(result_chan, responder);
+
+        // Check if already completed
+        match self.completed_loads.get(&url) {
+            Some(completed_load) => {
+                // It's already completed, return a notify straight away
+                image_listener.notify(completed_load.image.clone());
+            }
+            None => {
+                // Check if the load is already pending
+                match self.pending_loads.entry(url.clone()) {
+                    Occupied(mut e) => {
+                        // It's pending, so add the listener for state changes
+                        let pending_load = e.get_mut();
+                        pending_load.add_listener(image_listener);
+                    }
+                    Vacant(e) => {
+                        // A new load request! Add the pending load and request
+                        // it from the resource task.
+                        let mut pending_load = PendingLoad::new();
+                        pending_load.add_listener(image_listener);
+                        e.insert(pending_load);
+
+                        let load_data = LoadData::new(url.clone());
+                        let listener = box ResourceListener {
+                            url: url,
+                            sender: self.progress_sender.clone(),
+                        };
+                        self.resource_task.send(ControlMsg::Load(load_data, LoadConsumer::Listener(listener))).unwrap();
+                    }
+                }
+            }
+        }
+    }
+}
+
+/// Create a new image cache.
+pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
+    let (cmd_sender, cmd_receiver) = channel();
+    let (progress_sender, progress_receiver) = channel();
+    let (decoder_sender, decoder_receiver) = channel();
+
+    spawn_named("ImageCacheThread".to_owned(), move || {
+
+        // Preload the placeholder image, used when images fail to load.
+        let mut placeholder_url = resources_dir_path();
+        // TODO (Savago): replace for a prettier one.
+        placeholder_url.push("rippy.jpg");
+        let url = Url::from_file_path(&*placeholder_url).unwrap();
+        let placeholder_image = match load_whole_resource(&resource_task, url) {
+            Err(..) => {
+                debug!("image_cache_task: failed loading the placeholder.");
+                None
+            }
+            Ok((_, image_data)) => {
+                Some(Arc::new(load_from_memory(&image_data).unwrap()))
+            }
+        };
+
+        let mut cache = ImageCache {
+            cmd_receiver: cmd_receiver,
+            progress_sender: progress_sender,
+            progress_receiver: progress_receiver,
+            decoder_sender: decoder_sender,
+            decoder_receiver: decoder_receiver,
+            task_pool: TaskPool::new(4),
+            pending_loads: HashMap::new(),
+            completed_loads: HashMap::new(),
+            resource_task: resource_task,
+            placeholder_image: placeholder_image,
+        };
+
+        cache.run();
+    });
+
+    ImageCacheTask::new(cmd_sender)
 }

servo/components/net_traits/image/holder.rs

@@ -1,100 +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 image::base::Image;
-use image_cache_task::ImageResponseMsg;
-use local_image_cache::LocalImageCache;
-
-use geom::size::Size2D;
-use std::sync::{Arc, Mutex};
-use url::Url;
-
-// FIXME: Nasty coupling here This will be a problem if we want to factor out image handling from
-// the network stack. This should probably be factored out into an interface and use dependency
-// injection.
-
-/// 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 this Arc are given out on demand.
-#[derive(Clone)]
-pub struct ImageHolder<NodeAddress> {
-    url: Url,
-    image: Option<Arc<Image>>,
-    cached_size: Size2D<u32>,
-    local_image_cache: Arc<Mutex<LocalImageCache<NodeAddress>>>,
-}
-
-impl<NodeAddress: Send + 'static> ImageHolder<NodeAddress> {
-    pub fn new(url: Url, local_image_cache: Arc<Mutex<LocalImageCache<NodeAddress>>>)
-               -> ImageHolder<NodeAddress> {
-        debug!("ImageHolder::new() {}", url.serialize());
-        let holder = ImageHolder {
-            url: url,
-            image: None,
-            cached_size: Size2D(0,0),
-            local_image_cache: local_image_cache.clone(),
-        };
-
-        // Tell the image cache we're going to be interested in this url
-        // FIXME: These two messages must be sent to prep an image for use
-        // 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.
-        {
-            let val = holder.local_image_cache.lock().unwrap();
-            let mut local_image_cache = val;
-            local_image_cache.prefetch(&holder.url);
-            local_image_cache.decode(&holder.url);
-        }
-
-        holder
-    }
-
-    /// This version doesn't perform any computation, but may be stale w.r.t. newly-available image
-    /// data that determines size.
-    ///
-    /// The intent is that the impure version is used during layout when dimensions are used for
-    /// computing layout.
-    pub fn size(&self) -> Size2D<u32> {
-        self.cached_size
-    }
-
-    /// Query and update the current image size.
-    pub fn get_size(&mut self, node_address: NodeAddress) -> Option<Size2D<u32>> {
-        debug!("get_size() {}", self.url.serialize());
-        self.get_image(node_address).map(|img| {
-            self.cached_size = Size2D(img.width, img.height);
-            self.cached_size.clone()
-        })
-    }
-
-    pub fn get_image_if_present(&self) -> Option<Arc<Image>> {
-        debug!("get_image_if_present() {}", self.url.serialize());
-        self.image.clone()
-    }
-
-    pub fn get_image(&mut self, node_address: NodeAddress) -> Option<Arc<Image>> {
-        debug!("get_image() {}", self.url.serialize());
-
-        // 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 port = {
-                let val = self.local_image_cache.lock().unwrap();
-                let mut local_image_cache = val;
-                local_image_cache.get_image(node_address, &self.url)
-            };
-            match port.recv().unwrap() {
-                ImageResponseMsg::ImageReady(image) => self.image = Some(image),
-                ImageResponseMsg::ImageNotReady => debug!("image not ready for {}", self.url.serialize()),
-                ImageResponseMsg::ImageFailed => debug!("image decoding failed for {}", self.url.serialize()),
-            }
-        }
-
-        return self.image.clone();
-    }
-
-    pub fn url(&self) -> &Url {
-        &self.url
-    }
-}

servo/components/net_traits/image_cache_task.rs

@@ -3,117 +3,92 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use image::base::Image;
-use LoadConsumer::Channel;
-use {ControlMsg, LoadData, ProgressMsg, ResourceTask};
 use url::Url;
-
 use std::sync::Arc;
 use std::sync::mpsc::{channel, Sender};
 
-pub enum Msg {
-    /// Tell the cache that we may need a particular image soon. Must be posted
-    /// before Decode
-    Prefetch(Url),
+/// This is optionally passed to the image cache when requesting
+/// and image, and returned to the specified event loop when the
+/// image load completes. It is typically used to trigger a reflow
+/// and/or repaint.
+pub trait ImageResponder : Send {
+    fn respond(&self, Option<Arc<Image>>);
+}
 
-    /// Tell the cache to decode an image. Must be posted before GetImage/WaitForImage
-    Decode(Url),
+/// The current state of an image in the cache.
+#[derive(PartialEq, Copy)]
+pub enum ImageState {
+    Pending,
+    LoadError,
+    NotRequested,
+}
 
-    /// Request an Image object for a URL. If the image is not is not immediately
-    /// available then ImageNotReady is returned.
-    GetImage(Url, Sender<ImageResponseMsg>),
+/// Channel for sending commands to the image cache.
+#[derive(Clone)]
+pub struct ImageCacheChan(pub Sender<ImageCacheResult>);
 
-    /// Wait for an image to become available (or fail to load).
-    WaitForImage(Url, Sender<ImageResponseMsg>),
+/// The result of an image cache command that is returned to the
+/// caller.
+pub struct ImageCacheResult {
+    pub responder: Option<Box<ImageResponder>>,
+    pub image: Option<Arc<Image>>,
+}
+
+/// Commands that the image cache understands.
+pub enum ImageCacheCommand {
+    /// Request an image asynchronously from the cache. Supply a channel
+    /// to receive the result, and optionally an image responder
+    /// that is passed to the result channel.
+    RequestImage(Url, ImageCacheChan, Option<Box<ImageResponder>>),
+
+    /// Synchronously check the state of an image in the cache.
+    /// TODO(gw): Profile this on some real world sites and see
+    /// if it's worth caching the results of this locally in each
+    /// layout / paint task.
+    GetImageIfAvailable(Url, Sender<Result<Arc<Image>, ImageState>>),
 
     /// Clients must wait for a response before shutting down the ResourceTask
     Exit(Sender<()>),
-
-    /// Used by the prefetch tasks to post back image binaries
-    StorePrefetchedImageData(Url, Result<Vec<u8>, ()>),
-
-    /// Used by the decoder tasks to post decoded images back to the cache
-    StoreImage(Url, Option<Arc<Image>>),
-
-    /// For testing
-    WaitForStore(Sender<()>),
-
-    /// For testing
-    WaitForStorePrefetched(Sender<()>),
-}
-
-#[derive(Clone)]
-pub enum ImageResponseMsg {
-    ImageReady(Arc<Image>),
-    ImageNotReady,
-    ImageFailed
-}
-
-impl PartialEq for ImageResponseMsg {
-    fn eq(&self, other: &ImageResponseMsg) -> bool {
-        match (self, other) {
-            (&ImageResponseMsg::ImageReady(..), &ImageResponseMsg::ImageReady(..)) => panic!("unimplemented comparison"),
-            (&ImageResponseMsg::ImageNotReady, &ImageResponseMsg::ImageNotReady) => true,
-            (&ImageResponseMsg::ImageFailed, &ImageResponseMsg::ImageFailed) => true,
-
-            (&ImageResponseMsg::ImageReady(..), _) | (&ImageResponseMsg::ImageNotReady, _) | (&ImageResponseMsg::ImageFailed, _) => false
-        }
-    }
 }
 
+/// The client side of the image cache task. This can be safely cloned
+/// and passed to different tasks.
 #[derive(Clone)]
 pub struct ImageCacheTask {
-    pub chan: Sender<Msg>,
+    chan: Sender<ImageCacheCommand>,
 }
 
+/// The public API for the image cache task.
 impl ImageCacheTask {
-    pub fn send(&self, msg: Msg) {
+
+    /// Construct a new image cache
+    pub fn new(chan: Sender<ImageCacheCommand>) -> ImageCacheTask {
+        ImageCacheTask {
+            chan: chan,
+        }
+    }
+
+    /// Asynchronously request and image. See ImageCacheCommand::RequestImage.
+    pub fn request_image(&self,
+                         url: Url,
+                         result_chan: ImageCacheChan,
+                         responder: Option<Box<ImageResponder>>) {
+        let msg = ImageCacheCommand::RequestImage(url, result_chan, responder);
         self.chan.send(msg).unwrap();
     }
+
+    /// Get the current state of an image. See ImageCacheCommand::GetImageIfAvailable.
+    pub fn get_image_if_available(&self, url: Url) -> Result<Arc<Image>, ImageState> {
+        let (sender, receiver) = channel();
+        let msg = ImageCacheCommand::GetImageIfAvailable(url, sender);
+        self.chan.send(msg).unwrap();
+        receiver.recv().unwrap()
     }
 
-pub trait ImageCacheTaskClient {
-    fn exit(&self);
-}
-
-impl ImageCacheTaskClient for ImageCacheTask {
-    fn exit(&self) {
+    /// Shutdown the image cache task.
+    pub fn exit(&self) {
         let (response_chan, response_port) = channel();
-        self.send(Msg::Exit(response_chan));
+        self.chan.send(ImageCacheCommand::Exit(response_chan)).unwrap();
         response_port.recv().unwrap();
     }
 }
-
-pub fn load_image_data(url: Url, resource_task: ResourceTask, placeholder: &[u8]) -> Result<Vec<u8>, ()> {
-    let (response_chan, response_port) = channel();
-    resource_task.send(ControlMsg::Load(LoadData::new(url.clone()), Channel(response_chan))).unwrap();
-
-    let mut image_data = vec!();
-
-    let progress_port = response_port.recv().unwrap().progress_port;
-    loop {
-        match progress_port.recv().unwrap() {
-            ProgressMsg::Payload(data) => {
-                image_data.push_all(&data);
-            }
-            ProgressMsg::Done(Ok(..)) => {
-                return Ok(image_data);
-            }
-            ProgressMsg::Done(Err(..)) => {
-                // Failure to load the requested image will return the
-                // placeholder instead. In case it failed to load at init(),
-                // we still recover and return Err() but nothing will be drawn.
-                if placeholder.len() != 0 {
-                    debug!("image_cache_task: failed to load {:?}, use placeholder instead.", url);
-                    // Clean in case there was an error after started loading the image.
-                    image_data.clear();
-                    image_data.push_all(&placeholder);
-                    return Ok(image_data);
-                } else {
-                    debug!("image_cache_task: invalid placeholder.");
-                    return Err(());
-                }
-            }
-        }
-    }
-}
-

servo/components/net_traits/lib.rs

@@ -6,7 +6,6 @@
 #![feature(collections)]
 #![feature(core)]
 #![feature(rustc_private)]
-#![feature(std_misc)]
 
 extern crate geom;
 extern crate hyper;
@@ -28,7 +27,6 @@ use std::borrow::IntoCow;
 use std::sync::mpsc::{channel, Receiver, Sender};
 
 pub mod image_cache_task;
-pub mod local_image_cache;
 pub mod storage_task;
 
 /// Image handling.
@@ -38,7 +36,6 @@ pub mod storage_task;
 /// caching is involved) and as a result it must live in here.
 pub mod image {
     pub mod base;
-    pub mod holder;
 }
 
 #[derive(Clone)]

servo/components/net_traits/local_image_cache.rs

@@ -1,170 +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/. */
-
-/*!
-An adapter for ImageCacheTask that does local caching to avoid
-extra message traffic, it also avoids waiting on the same image
-multiple times and thus triggering reflows multiple times.
-*/
-
-use image_cache_task::{ImageResponseMsg, ImageCacheTask, Msg};
-use url::Url;
-
-use std::borrow::ToOwned;
-use std::collections::HashMap;
-use std::collections::hash_map::Entry::{Occupied, Vacant};
-use std::sync::mpsc::{channel, Receiver};
-use util::task::spawn_named;
-
-pub trait ImageResponder<NodeAddress: Send> {
-    fn respond(&self) -> Box<Fn(ImageResponseMsg, NodeAddress)+Send>;
-}
-
-pub struct LocalImageCache<NodeAddress> {
-    image_cache_task: ImageCacheTask,
-    round_number: u32,
-    on_image_available: Option<Box<ImageResponder<NodeAddress>+Send>>,
-    state_map: HashMap<Url, ImageState>
-}
-
-impl<NodeAddress: Send> LocalImageCache<NodeAddress> {
-    pub fn new(image_cache_task: ImageCacheTask) -> LocalImageCache<NodeAddress> {
-        LocalImageCache {
-            image_cache_task: image_cache_task,
-            round_number: 1,
-            on_image_available: None,
-            state_map: HashMap::new()
-        }
-    }
-}
-
-#[derive(Clone)]
-struct ImageState {
-    prefetched: bool,
-    decoded: bool,
-    last_request_round: u32,
-    last_response: ImageResponseMsg
-}
-
-impl<NodeAddress: Send + 'static> LocalImageCache<NodeAddress> {
-    /// 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
-    pub fn next_round(&mut self, on_image_available: Box<ImageResponder<NodeAddress> + Send>) {
-        self.round_number += 1;
-        self.on_image_available = Some(on_image_available);
-    }
-
-    pub fn prefetch(&mut self, url: &Url) {
-        {
-            let state = self.get_state(url);
-            if state.prefetched {
-                return
-            }
-
-            state.prefetched = true;
-        }
-
-        self.image_cache_task.send(Msg::Prefetch((*url).clone()));
-    }
-
-    pub fn decode(&mut self, url: &Url) {
-        {
-            let state = self.get_state(url);
-            if state.decoded {
-                return
-            }
-            state.decoded = true;
-        }
-
-        self.image_cache_task.send(Msg::Decode((*url).clone()));
-    }
-
-    // FIXME: Should return a Future
-    pub fn get_image(&mut self, node_address: NodeAddress, url: &Url) -> Receiver<ImageResponseMsg> {
-        {
-            let round_number = self.round_number;
-            let state = self.get_state(url);
-
-            // Save the previous round number for comparison
-            let last_round = state.last_request_round;
-            // Set the current round number for this image
-            state.last_request_round = round_number;
-
-            match state.last_response {
-                ImageResponseMsg::ImageReady(ref image) => {
-                    let (chan, port) = channel();
-                    chan.send(ImageResponseMsg::ImageReady(image.clone())).unwrap();
-                    return port;
-                }
-                ImageResponseMsg::ImageNotReady => {
-                    if last_round == round_number {
-                        let (chan, port) = channel();
-                        chan.send(ImageResponseMsg::ImageNotReady).unwrap();
-                        return port;
-                    } else {
-                        // We haven't requested the image from the
-                        // remote cache this round
-                    }
-                }
-                ImageResponseMsg::ImageFailed => {
-                    let (chan, port) = channel();
-                    chan.send(ImageResponseMsg::ImageFailed).unwrap();
-                    return port;
-                }
-            }
-        }
-
-        let (response_chan, response_port) = channel();
-        self.image_cache_task.send(Msg::GetImage((*url).clone(), response_chan));
-
-        let response = response_port.recv().unwrap();
-        match response {
-            ImageResponseMsg::ImageNotReady => {
-                // Need to reflow when the image is available
-                // FIXME: Instead we should be just passing a Future
-                // to the caller, then to the display list. Finally,
-                // the compositor should be responsible for waiting
-                // on the image to load and triggering layout
-                let image_cache_task = self.image_cache_task.clone();
-                assert!(self.on_image_available.is_some());
-                let on_image_available =
-                    self.on_image_available.as_ref().unwrap().respond();
-                let url = (*url).clone();
-                spawn_named("LocalImageCache".to_owned(), move || {
-                    let (response_chan, response_port) = channel();
-                    image_cache_task.send(Msg::WaitForImage(url, response_chan));
-                    on_image_available(response_port.recv().unwrap(), node_address);
-                });
-            }
-            _ => ()
-        }
-
-        // Put a copy of the response in the cache
-        let response_copy = match response {
-            ImageResponseMsg::ImageReady(ref image) => ImageResponseMsg::ImageReady(image.clone()),
-            ImageResponseMsg::ImageNotReady => ImageResponseMsg::ImageNotReady,
-            ImageResponseMsg::ImageFailed => ImageResponseMsg::ImageFailed
-        };
-        self.get_state(url).last_response = response_copy;
-
-        let (chan, port) = channel();
-        chan.send(response).unwrap();
-        return port;
-    }
-
-    fn get_state<'a>(&'a mut self, url: &Url) -> &'a mut ImageState {
-        match self.state_map.entry((*url).clone()) {
-            Occupied(entry) => entry.into_mut(),
-            Vacant(entry) =>
-                entry.insert(ImageState {
-                    prefetched: false,
-                    decoded: false,
-                    last_request_round: 0,
-                    last_response: ImageResponseMsg::ImageNotReady,
-                })
-        }
-    }
-}
-
-

servo/components/script/dom/bindings/refcounted.rs

@@ -117,8 +117,11 @@ impl<T: Reflectable> Drop for Trusted<T> {
         assert!(*refcount > 0);
         *refcount -= 1;
         if *refcount == 0 {
-            self.script_chan.send(
-                ScriptMsg::RefcountCleanup(TrustedReference(self.ptr))).unwrap();
+            // It's possible this send will fail if the script task
+            // has already exited. There's not much we can do at this
+            // point though.
+            let msg = ScriptMsg::RefcountCleanup(TrustedReference(self.ptr));
+            let _ = self.script_chan.send(msg);
         }
     }
 }

servo/components/script/dom/bindings/trace.rs

@@ -49,12 +49,13 @@ use js::rust::{Cx, Runtime};
 use layout_interface::{LayoutRPC, LayoutChan};
 use libc;
 use msg::constellation_msg::{PipelineId, SubpageId, WindowSizeData, WorkerId};
-use net_traits::image_cache_task::ImageCacheTask;
+use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask};
 use net_traits::storage_task::StorageType;
 use script_traits::ScriptControlChan;
 use script_traits::UntrustedNodeAddress;
 use msg::compositor_msg::ScriptListener;
 use msg::constellation_msg::ConstellationChan;
+use net_traits::image::base::Image;
 use util::smallvec::{SmallVec1, SmallVec};
 use util::str::{LengthOrPercentageOrAuto};
 use std::cell::{Cell, RefCell};
@@ -66,6 +67,7 @@ use std::intrinsics::return_address;
 use std::old_io::timer::Timer;
 use std::ops::{Deref, DerefMut};
 use std::rc::Rc;
+use std::sync::Arc;
 use std::sync::mpsc::{Receiver, Sender};
 use string_cache::{Atom, Namespace};
 use style::properties::PropertyDeclarationBlock;
@@ -248,7 +250,8 @@ no_jsmanaged_fields!(isize, i8, i16, i32, i64);
 no_jsmanaged_fields!(Sender<T>);
 no_jsmanaged_fields!(Receiver<T>);
 no_jsmanaged_fields!(Rect<T>);
-no_jsmanaged_fields!(ImageCacheTask, ScriptControlChan);
+no_jsmanaged_fields!(Arc<T>);
+no_jsmanaged_fields!(Image, ImageCacheChan, ImageCacheTask, ScriptControlChan);
 no_jsmanaged_fields!(Atom, Namespace, Timer);
 no_jsmanaged_fields!(Trusted<T>);
 no_jsmanaged_fields!(PropertyDeclarationBlock);

servo/components/script/dom/canvasrenderingcontext2d.rs

@@ -33,7 +33,7 @@ use canvas::canvas_paint_task::{LinearGradientStyle, RadialGradientStyle};
 use canvas::canvas_paint_task::{LineCapStyle, LineJoinStyle, CompositionOrBlending};
 
 use net_traits::image::base::Image;
-use net_traits::image_cache_task::{ImageResponseMsg, Msg};
+use net_traits::image_cache_task::ImageCacheChan;
 use png::PixelsByColorType;
 
 use std::borrow::ToOwned;
@@ -275,16 +275,11 @@ impl CanvasRenderingContext2D {
         let canvas = self.canvas.root();
         let window = window_from_node(canvas.r()).root();
         let window = window.r();
-        let image_cache_task = window.image_cache_task().clone();
-        image_cache_task.send(Msg::Prefetch(url.clone()));
-        image_cache_task.send(Msg::Decode(url.clone()));
+        let image_cache = window.image_cache_task();
         let (response_chan, response_port) = channel();
-        image_cache_task.send(Msg::WaitForImage(url, response_chan));
-        match response_port.recv().unwrap() {
-           ImageResponseMsg::ImageReady(image) => Some(image),
-           ImageResponseMsg::ImageFailed => None,
-           _ => panic!("Image Cache: Unknown Result")
-         }
+        image_cache.request_image(url, ImageCacheChan(response_chan), None);
+        let result = response_port.recv().unwrap();
+        result.image
     }
 
     fn create_drawable_rect(&self, x: f64, y: f64, w: f64, h: f64) -> Option<Rect<f32>> {

servo/components/script/dom/htmlimageelement.rs

@@ -7,30 +7,36 @@ use dom::attr::{AttrHelpers, AttrValue};
 use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::HTMLImageElementBinding;
 use dom::bindings::codegen::Bindings::HTMLImageElementBinding::HTMLImageElementMethods;
-use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast, HTMLElementCast, HTMLImageElementDerived};
+use dom::bindings::codegen::InheritTypes::{NodeCast, ElementCast, EventTargetCast, HTMLElementCast, HTMLImageElementDerived};
+use dom::bindings::global::GlobalRef;
 use dom::bindings::js::{JSRef, LayoutJS, Temporary};
+use dom::bindings::refcounted::Trusted;
 use dom::document::{Document, DocumentHelpers};
 use dom::element::Element;
 use dom::element::AttributeHandlers;
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::element::ElementTypeId;
+use dom::event::{Event, EventBubbles, EventCancelable, EventHelpers};
 use dom::htmlelement::{HTMLElement, HTMLElementTypeId};
-use dom::node::{Node, NodeTypeId, NodeHelpers, NodeDamage, window_from_node};
+use dom::node::{document_from_node, Node, NodeTypeId, NodeHelpers, NodeDamage, window_from_node};
 use dom::virtualmethods::VirtualMethods;
 use dom::window::WindowHelpers;
-use net_traits::image_cache_task;
 use util::geometry::to_px;
 use util::str::DOMString;
 use string_cache::Atom;
 
+use net_traits::image::base::Image;
+use net_traits::image_cache_task::ImageResponder;
 use url::{Url, UrlParser};
 
 use std::borrow::ToOwned;
+use std::sync::Arc;
 
 #[dom_struct]
 pub struct HTMLImageElement {
     htmlelement: HTMLElement,
-    image: DOMRefCell<Option<Url>>,
+    url: DOMRefCell<Option<Url>>,
+    image: DOMRefCell<Option<Arc<Image>>>,
 }
 
 impl HTMLImageElementDerived for EventTarget {
@@ -45,7 +51,7 @@ pub trait HTMLImageElementHelpers {
 
 impl<'a> HTMLImageElementHelpers for JSRef<'a, HTMLImageElement> {
     fn get_url(&self) -> Option<Url>{
-        self.image.borrow().clone()
+        self.url.borrow().clone()
     }
 }
 
@@ -53,6 +59,48 @@ trait PrivateHTMLImageElementHelpers {
     fn update_image(self, value: Option<(DOMString, &Url)>);
 }
 
+/// This is passed to the image cache when the src attribute
+/// changes. It is returned via a message to the script task,
+/// which marks the element as dirty and triggers a reflow.
+struct Responder {
+    element: Trusted<HTMLImageElement>,
+}
+
+impl Responder {
+    fn new(element: Trusted<HTMLImageElement>) -> Responder {
+        Responder {
+            element: element
+        }
+    }
+}
+
+impl ImageResponder for Responder {
+    fn respond(&self, image: Option<Arc<Image>>) {
+        // Update the image field
+        let element = self.element.to_temporary().root();
+        let element_ref = element.r();
+        *element_ref.image.borrow_mut() = image;
+
+        // Mark the node dirty
+        let node = NodeCast::from_ref(element.r());
+        let document = document_from_node(node).root();
+        document.r().content_changed(node, NodeDamage::OtherNodeDamage);
+
+        // Fire image.onload
+        let window = window_from_node(document.r()).root();
+        let event = Event::new(GlobalRef::Window(window.r()),
+                               "load".to_owned(),
+                               EventBubbles::DoesNotBubble,
+                               EventCancelable::NotCancelable).root();
+        let event = event.r();
+        let target: JSRef<EventTarget> = EventTargetCast::from_ref(node);
+        event.fire(target);
+
+        // Trigger reflow
+        window.r().add_pending_reflow();
+    }
+}
+
 impl<'a> PrivateHTMLImageElementHelpers for JSRef<'a, HTMLImageElement> {
     /// Makes the local `image` member match the status of the `src` attribute and starts
     /// prefetching the image. This method must be called after `src` is changed.
@@ -64,17 +112,18 @@ impl<'a> PrivateHTMLImageElementHelpers for JSRef<'a, HTMLImageElement> {
         let image_cache = window.image_cache_task();
         match value {
             None => {
+                *self.url.borrow_mut() = None;
                 *self.image.borrow_mut() = None;
             }
             Some((src, base_url)) => {
                 let img_url = UrlParser::new().base_url(base_url).parse(src.as_slice());
                 // FIXME: handle URL parse errors more gracefully.
                 let img_url = img_url.unwrap();
-                *self.image.borrow_mut() = Some(img_url.clone());
+                *self.url.borrow_mut() = Some(img_url.clone());
 
-                // inform the image cache to load this, but don't store a
-                // handle.
-                image_cache.send(image_cache_task::Msg::Prefetch(img_url));
+                let trusted_node = Trusted::new(window.get_cx(), self, window.script_chan());
+                let responder = box Responder::new(trusted_node);
+                image_cache.request_image(img_url, window.image_cache_chan(), Some(responder));
             }
         }
     }
@@ -84,6 +133,7 @@ impl HTMLImageElement {
     fn new_inherited(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>) -> HTMLImageElement {
         HTMLImageElement {
             htmlelement: HTMLElement::new_inherited(HTMLElementTypeId::HTMLImageElement, localName, prefix, document),
+            url: DOMRefCell::new(None),
             image: DOMRefCell::new(None),
         }
     }
@@ -97,14 +147,22 @@ impl HTMLImageElement {
 
 pub trait LayoutHTMLImageElementHelpers {
     #[allow(unsafe_code)]
-    unsafe fn image(&self) -> Option<Url>;
+    unsafe fn image(&self) -> Option<Arc<Image>>;
+
+    #[allow(unsafe_code)]
+    unsafe fn image_url(&self) -> Option<Url>;
 }
 
 impl LayoutHTMLImageElementHelpers for LayoutJS<HTMLImageElement> {
     #[allow(unsafe_code)]
-    unsafe fn image(&self) -> Option<Url> {
+    unsafe fn image(&self) -> Option<Arc<Image>> {
         (*self.unsafe_get()).image.borrow_for_layout().clone()
     }
+
+    #[allow(unsafe_code)]
+    unsafe fn image_url(&self) -> Option<Url> {
+        (*self.unsafe_get()).url.borrow_for_layout().clone()
+    }
 }
 
 impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> {
@@ -163,6 +221,29 @@ impl<'a> HTMLImageElementMethods for JSRef<'a, HTMLImageElement> {
         elem.set_uint_attribute(&atom!("height"), height)
     }
 
+    fn NaturalWidth(self) -> u32 {
+        let image = self.image.borrow();
+
+        match *image {
+            Some(ref image) => image.width,
+            None => 0,
+        }
+    }
+
+    fn NaturalHeight(self) -> u32 {
+        let image = self.image.borrow();
+
+        match *image {
+            Some(ref image) => image.height,
+            None => 0,
+        }
+    }
+
+    fn Complete(self) -> bool {
+        let image = self.image.borrow();
+        image.is_some()
+    }
+
     make_getter!(Name);
 
     make_setter!(SetName, "name");

servo/components/script/dom/htmlobjectelement.rs

@@ -4,6 +4,7 @@
 
 use dom::attr::Attr;
 use dom::attr::AttrHelpers;
+use dom::bindings::cell::DOMRefCell;
 use dom::bindings::codegen::Bindings::AttrBinding::AttrMethods;
 use dom::bindings::codegen::Bindings::HTMLObjectElementBinding;
 use dom::bindings::codegen::Bindings::HTMLObjectElementBinding::HTMLObjectElementMethods;
@@ -20,15 +21,15 @@ use dom::node::{Node, NodeTypeId, NodeHelpers, window_from_node};
 use dom::validitystate::ValidityState;
 use dom::virtualmethods::VirtualMethods;
 
-use net_traits::image_cache_task::{self, ImageCacheTask};
+use net_traits::image::base::Image;
 use util::str::DOMString;
+use std::sync::Arc;
 use string_cache::Atom;
 
-use url::Url;
-
 #[dom_struct]
 pub struct HTMLObjectElement {
     htmlelement: HTMLElement,
+    image: DOMRefCell<Option<Arc<Image>>>,
 }
 
 impl HTMLObjectElementDerived for EventTarget {
@@ -41,6 +42,7 @@ impl HTMLObjectElement {
     fn new_inherited(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>) -> HTMLObjectElement {
         HTMLObjectElement {
             htmlelement: HTMLElement::new_inherited(HTMLElementTypeId::HTMLObjectElement, localName, prefix, document),
+            image: DOMRefCell::new(None),
         }
     }
 
@@ -52,24 +54,20 @@ impl HTMLObjectElement {
 }
 
 trait ProcessDataURL {
-    fn process_data_url(&self, image_cache: ImageCacheTask);
+    fn process_data_url(&self);
 }
 
 impl<'a> ProcessDataURL for JSRef<'a, HTMLObjectElement> {
     // Makes the local `data` member match the status of the `data` attribute and starts
     /// prefetching the image. This method must be called after `data` is changed.
-    fn process_data_url(&self, image_cache: ImageCacheTask) {
+    fn process_data_url(&self) {
         let elem: JSRef<Element> = ElementCast::from_ref(*self);
 
         // TODO: support other values
         match (elem.get_attribute(&ns!(""), &atom!("type")).map(|x| x.root().r().Value()),
                elem.get_attribute(&ns!(""), &atom!("data")).map(|x| x.root().r().Value())) {
-            (None, Some(uri)) => {
-                if is_image_data(uri.as_slice()) {
-                    let data_url = Url::parse(uri.as_slice()).unwrap();
-                    // Issue #84
-                    image_cache.send(image_cache_task::Msg::Prefetch(data_url));
-                }
+            (None, Some(_uri)) => {
+                // TODO(gw): Prefetch the image here.
             }
             _ => { }
         }
@@ -107,8 +105,7 @@ impl<'a> VirtualMethods for JSRef<'a, HTMLObjectElement> {
 
         match attr.local_name() {
             &atom!("data") => {
-                let window = window_from_node(*self).root();
-                self.process_data_url(window.r().image_cache_task().clone());
+                self.process_data_url();
             },
             _ => ()
         }

servo/components/script/dom/webidls/HTMLImageElement.webidl

@@ -14,9 +14,9 @@ interface HTMLImageElement : HTMLElement {
            attribute boolean isMap;
            attribute unsigned long width;
            attribute unsigned long height;
-  //readonly attribute unsigned long naturalWidth;
-  //readonly attribute unsigned long naturalHeight;
-  //readonly attribute boolean complete;
+  readonly attribute unsigned long naturalWidth;
+  readonly attribute unsigned long naturalHeight;
+  readonly attribute boolean complete;
 
   // also has obsolete members
 };

servo/components/script/dom/window.rs

@@ -38,7 +38,7 @@ use devtools_traits::{DevtoolsControlChan, TimelineMarker, TimelineMarkerType, T
 use msg::compositor_msg::ScriptListener;
 use msg::constellation_msg::{LoadData, PipelineId, SubpageId, ConstellationChan, WindowSizeData, WorkerId};
 use net_traits::ResourceTask;
-use net_traits::image_cache_task::ImageCacheTask;
+use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask};
 use net_traits::storage_task::{StorageTask, StorageType};
 use util::geometry::{self, Au, MAX_RECT};
 use util::opts;
@@ -67,18 +67,19 @@ use std::sync::mpsc::TryRecvError::{Empty, Disconnected};
 use time;
 
 /// Extra information concerning the reason for reflowing.
+#[derive(Debug)]
 pub enum ReflowReason {
     CachedPageNeededReflow,
     FirstLoad,
     KeyEvent,
     MouseEvent,
     Query,
-    ReceivedReflowEvent,
     Timer,
     Viewport,
     WindowResize,
     DOMContentLoaded,
     DocumentLoaded,
+    ImageLoaded,
 }
 
 #[dom_struct]
@@ -89,6 +90,7 @@ pub struct Window {
     console: MutNullableJS<Console>,
     navigator: MutNullableJS<Navigator>,
     image_cache_task: ImageCacheTask,
+    image_cache_chan: ImageCacheChan,
     compositor: DOMRefCell<Box<ScriptListener+'static>>,
     browser_context: DOMRefCell<Option<BrowserContext>>,
     page: Rc<Page>,
@@ -160,6 +162,9 @@ pub struct Window {
     /// An enlarged rectangle around the page contents visible in the viewport, used
     /// to prevent creating display list items for content that is far away from the viewport.
     page_clip_rect: Cell<Rect<Au>>,
+
+    /// A counter of the number of pending reflows for this window.
+    pending_reflow_count: Cell<u32>,
 }
 
 impl Window {
@@ -179,6 +184,10 @@ impl Window {
         self.script_chan.clone()
     }
 
+    pub fn image_cache_chan(&self) -> ImageCacheChan {
+        self.image_cache_chan.clone()
+    }
+
     pub fn get_next_worker_id(&self) -> WorkerId {
         let worker_id = self.next_worker_id.get();
         let WorkerId(id_num) = worker_id;
@@ -481,6 +490,8 @@ pub trait WindowHelpers {
     fn windowproxy_handler(self) -> WindowProxyHandler;
     fn get_next_subpage_id(self) -> SubpageId;
     fn layout_is_idle(self) -> bool;
+    fn get_pending_reflow_count(self) -> u32;
+    fn add_pending_reflow(self);
     fn set_resize_event(self, event: WindowSizeData);
     fn steal_resize_event(self) -> Option<WindowSizeData>;
     fn set_page_clip_rect_with_new_viewport(self, viewport: Rect<f32>) -> bool;
@@ -549,11 +560,9 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
             None => return,
         };
 
-        debug!("script: performing reflow for goal {:?}", goal);
-
         let root: JSRef<Node> = NodeCast::from_ref(root);
         if query_type == ReflowQueryType::NoQuery && !root.get_has_dirty_descendants() {
-            debug!("root has no dirty descendants; avoiding reflow");
+            debug!("root has no dirty descendants; avoiding reflow (reason {:?})", reason);
             return
         }
 
@@ -562,6 +571,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
             None => return,
         };
 
+        debug!("script: performing reflow for goal {:?} reason {:?}", goal, reason);
+
         if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) {
             let marker = TimelineMarker::new("Reflow".to_owned(), TracingMetadata::IntervalStart);
             self.emit_timeline_marker(marker);
@@ -604,6 +615,8 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
 
         self.join_layout();
 
+        self.pending_reflow_count.set(0);
+
         if self.need_emit_timeline_marker(TimelineMarkerType::Reflow) {
             let marker = TimelineMarker::new("Reflow".to_owned(), TracingMetadata::IntervalEnd);
             self.emit_timeline_marker(marker);
@@ -745,6 +758,14 @@ impl<'a> WindowHelpers for JSRef<'a, Window> {
         port.is_none()
     }
 
+    fn get_pending_reflow_count(self) -> u32 {
+        self.pending_reflow_count.get()
+    }
+
+    fn add_pending_reflow(self) {
+        self.pending_reflow_count.set(self.pending_reflow_count.get() + 1);
+    }
+
     fn set_resize_event(self, event: WindowSizeData) {
         self.resize_event.set(Some(event));
     }
@@ -828,6 +849,7 @@ impl Window {
     pub fn new(js_context: Rc<Cx>,
                page: Rc<Page>,
                script_chan: Box<ScriptChan+Send>,
+               image_cache_chan: ImageCacheChan,
                control_chan: ScriptControlChan,
                compositor: Box<ScriptListener+'static>,
                image_cache_task: ImageCacheTask,
@@ -850,6 +872,7 @@ impl Window {
         let win = box Window {
             eventtarget: EventTarget::new_inherited(EventTargetTypeId::Window),
             script_chan: script_chan,
+            image_cache_chan: image_cache_chan,
             control_chan: control_chan,
             console: Default::default(),
             compositor: DOMRefCell::new(compositor),
@@ -882,6 +905,7 @@ impl Window {
             layout_rpc: layout_rpc,
             layout_join_port: DOMRefCell::new(None),
             window_size: Cell::new(window_size),
+            pending_reflow_count: Cell::new(0),
 
             devtools_marker_sender: RefCell::new(None),
             devtools_markers: RefCell::new(HashSet::new()),
@@ -929,12 +953,12 @@ fn debug_reflow_events(goal: &ReflowGoal, query_type: &ReflowQueryType, reason:
         ReflowReason::KeyEvent => "\tKeyEvent",
         ReflowReason::MouseEvent => "\tMouseEvent",
         ReflowReason::Query => "\tQuery",
-        ReflowReason::ReceivedReflowEvent => "\tReceivedReflowEvent",
         ReflowReason::Timer => "\tTimer",
         ReflowReason::Viewport => "\tViewport",
         ReflowReason::WindowResize => "\tWindowResize",
         ReflowReason::DOMContentLoaded => "\tDOMContentLoaded",
         ReflowReason::DocumentLoaded => "\tDocumentLoaded",
+        ReflowReason::ImageLoaded => "\tImageLoaded",
     });
 
     println!("{}", debug_msg);

servo/components/script/script_task.rs

@@ -36,7 +36,7 @@ use dom::event::{Event, EventHelpers, EventBubbles, EventCancelable};
 use dom::htmliframeelement::{HTMLIFrameElement, HTMLIFrameElementHelpers};
 use dom::uievent::UIEvent;
 use dom::eventtarget::EventTarget;
-use dom::node::{self, Node, NodeHelpers, NodeDamage, window_from_node};
+use dom::node::{Node, NodeHelpers, NodeDamage, window_from_node};
 use dom::window::{Window, WindowHelpers, ScriptHelpers, ReflowReason};
 use dom::worker::TrustedWorkerAddress;
 use parse::html::{HTMLInput, parse_html};
@@ -50,7 +50,7 @@ use devtools_traits::{DevtoolsControlChan, DevtoolsControlPort, DevtoolsPageInfo
 use devtools_traits::{DevtoolsControlMsg, DevtoolScriptControlMsg};
 use devtools_traits::{TimelineMarker, TimelineMarkerType, TracingMetadata};
 use script_traits::CompositorEvent;
-use script_traits::CompositorEvent::{ResizeEvent, ReflowEvent, ClickEvent};
+use script_traits::CompositorEvent::{ResizeEvent, ClickEvent};
 use script_traits::CompositorEvent::{MouseDownEvent, MouseUpEvent};
 use script_traits::CompositorEvent::{MouseMoveEvent, KeyEvent};
 use script_traits::{NewLayoutInfo, OpaqueScriptLayoutChannel};
@@ -64,7 +64,7 @@ use msg::constellation_msg::{Failure, WindowSizeData, PipelineExitType};
 use msg::constellation_msg::Msg as ConstellationMsg;
 use net_traits::{ResourceTask, ControlMsg, LoadResponse, LoadConsumer};
 use net_traits::LoadData as NetLoadData;
-use net_traits::image_cache_task::ImageCacheTask;
+use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageCacheResult};
 use net_traits::storage_task::StorageTask;
 use string_cache::Atom;
 use util::geometry::to_frac_px;
@@ -296,6 +296,12 @@ pub struct ScriptTask {
     /// A handle to the compositor for communicating ready state messages.
     compositor: DOMRefCell<Box<ScriptListener+'static>>,
 
+    /// The port on which we receive messages from the image cache
+    image_cache_port: Receiver<ImageCacheResult>,
+
+    /// The channel on which the image cache can send messages to ourself.
+    image_cache_channel: ImageCacheChan,
+
     /// For providing instructions to an optional devtools server.
     devtools_chan: Option<DevtoolsControlChan>,
     /// For receiving commands from an optional devtools server. Will be ignored if
@@ -437,7 +443,7 @@ impl ScriptTask {
                constellation_chan: ConstellationChan,
                resource_task: ResourceTask,
                storage_task: StorageTask,
-               img_cache_task: ImageCacheTask,
+               image_cache_task: ImageCacheTask,
                devtools_chan: Option<DevtoolsControlChan>)
                -> ScriptTask {
         let runtime = ScriptTask::new_rt_and_cx();
@@ -463,11 +469,16 @@ impl ScriptTask {
         }
 
         let (devtools_sender, devtools_receiver) = channel();
+        let (image_cache_channel, image_cache_port) = channel();
+
         ScriptTask {
             page: DOMRefCell::new(None),
             incomplete_loads: DOMRefCell::new(vec!()),
 
-            image_cache_task: img_cache_task,
+            image_cache_task: image_cache_task,
+            image_cache_channel: ImageCacheChan(image_cache_channel),
+            image_cache_port: image_cache_port,
+
             resource_task: resource_task,
             storage_task: storage_task,
 
@@ -558,6 +569,7 @@ impl ScriptTask {
             FromConstellation(ConstellationControlMsg),
             FromScript(ScriptMsg),
             FromDevtools(DevtoolScriptControlMsg),
+            FromImageCache(ImageCacheResult),
         }
 
         // Store new resizes, and gather all other events.
@@ -569,12 +581,14 @@ impl ScriptTask {
             let mut port1 = sel.handle(&self.port);
             let mut port2 = sel.handle(&self.control_port);
             let mut port3 = sel.handle(&self.devtools_port);
+            let mut port4 = sel.handle(&self.image_cache_port);
             unsafe {
                 port1.add();
                 port2.add();
                 if self.devtools_chan.is_some() {
                     port3.add();
                 }
+                port4.add();
             }
             let ret = sel.wait();
             if ret == port1.id() {
@@ -583,6 +597,8 @@ impl ScriptTask {
                 MixedMessage::FromConstellation(self.control_port.recv().unwrap())
             } else if ret == port3.id() {
                 MixedMessage::FromDevtools(self.devtools_port.recv().unwrap())
+            } else if ret == port4.id() {
+                MixedMessage::FromImageCache(self.image_cache_port.recv().unwrap())
             } else {
                 panic!("unexpected select result")
             }
@@ -629,7 +645,10 @@ impl ScriptTask {
             match self.control_port.try_recv() {
                 Err(_) => match self.port.try_recv() {
                     Err(_) => match self.devtools_port.try_recv() {
+                        Err(_) => match self.image_cache_port.try_recv() {
                             Err(_) => break,
+                            Ok(ev) => event = MixedMessage::FromImageCache(ev),
+                        },
                         Ok(ev) => event = MixedMessage::FromDevtools(ev),
                     },
                     Ok(ev) => event = MixedMessage::FromScript(ev),
@@ -649,6 +668,23 @@ impl ScriptTask {
                 MixedMessage::FromConstellation(inner_msg) => self.handle_msg_from_constellation(inner_msg),
                 MixedMessage::FromScript(inner_msg) => self.handle_msg_from_script(inner_msg),
                 MixedMessage::FromDevtools(inner_msg) => self.handle_msg_from_devtools(inner_msg),
+                MixedMessage::FromImageCache(inner_msg) => self.handle_msg_from_image_cache(inner_msg),
+            }
+        }
+
+        // Issue batched reflows on any pages that require it (e.g. if images loaded)
+        // TODO(gw): In the future we could probably batch other types of reflows
+        // into this loop too, but for now it's only images.
+        let page = self.page.borrow();
+        if let Some(page) = page.as_ref() {
+            for page in page.iter() {
+                let window = page.window().root();
+                let pending_reflows = window.r().get_pending_reflow_count();
+                if pending_reflows > 0 {
+                    window.r().reflow(ReflowGoal::ForDisplay,
+                                      ReflowQueryType::NoQuery,
+                                      ReflowReason::ImageLoaded);
+                }
             }
         }
 
@@ -743,6 +779,10 @@ impl ScriptTask {
         }
     }
 
+    fn handle_msg_from_image_cache(&self, msg: ImageCacheResult) {
+        msg.responder.unwrap().respond(msg.image);
+    }
+
     fn handle_resize(&self, id: PipelineId, size: WindowSizeData) {
         let page = self.page.borrow();
         if let Some(ref page) = page.as_ref() {
@@ -1061,6 +1101,7 @@ impl ScriptTask {
         let window = Window::new(self.js_runtime.cx.clone(),
                                  page.clone(),
                                  self.chan.clone(),
+                                 self.image_cache_channel.clone(),
                                  self.control_chan.clone(),
                                  self.compositor.borrow_mut().dup(),
                                  self.image_cache_task.clone(),
@@ -1207,29 +1248,6 @@ impl ScriptTask {
                 self.handle_resize_event(pipeline_id, new_size);
             }
 
-            ReflowEvent(nodes) => {
-                // FIXME(pcwalton): This event seems to only be used by the image cache task, and
-                // the interaction between it and the image holder is really racy. I think that, in
-                // order to fix this race, we need to rewrite the image cache task to make the
-                // image holder responsible for the lifecycle of image loading instead of having
-                // the image holder and layout task both be observers. Then we can have the DOM
-                // image element observe the state of the image holder and have it send reflows
-                // via the normal dirtying mechanism, and ultimately remove this event.
-                //
-                // See the implementation of `Width()` and `Height()` in `HTMLImageElement` for
-                // fallout of this problem.
-                for node in nodes.iter() {
-                    let node_to_dirty = node::from_untrusted_node_address(self.js_runtime.rt(),
-                                                                          *node).root();
-                    let page = get_page(&self.root_page(), pipeline_id);
-                    let document = page.document().root();
-                    document.r().content_changed(node_to_dirty.r(),
-                                                 NodeDamage::OtherNodeDamage);
-                }
-
-                self.handle_reflow_event(pipeline_id);
-            }
-
             ClickEvent(button, point) => {
                 let _marker;
                 if self.need_emit_timeline_marker(TimelineMarkerType::DOMEvent) {
@@ -1332,16 +1350,6 @@ impl ScriptTask {
         event.fire(wintarget);
     }
 
-    fn handle_reflow_event(&self, pipeline_id: PipelineId) {
-        debug!("script got reflow event");
-        let page = get_page(&self.root_page(), pipeline_id);
-        let document = page.document().root();
-        let window = window_from_node(document.r()).root();
-        window.r().reflow(ReflowGoal::ForDisplay,
-                          ReflowQueryType::NoQuery,
-                          ReflowReason::ReceivedReflowEvent);
-    }
-
     /// Initiate a non-blocking fetch for a specified resource. Stores the InProgressLoad
     /// argument until a notification is received that the fetch is complete.
     fn start_page_load(&self, incomplete: InProgressLoad, mut load_data: LoadData) {

servo/components/script_traits/lib.rs

@@ -24,7 +24,6 @@ use msg::compositor_msg::ScriptListener;
 use net_traits::ResourceTask;
 use net_traits::image_cache_task::ImageCacheTask;
 use net_traits::storage_task::StorageTask;
-use util::smallvec::SmallVec1;
 use std::any::Any;
 use std::sync::mpsc::{Sender, Receiver};
 
@@ -89,7 +88,6 @@ pub enum MouseButton {
 /// Events from the compositor that the script task needs to know about
 pub enum CompositorEvent {
     ResizeEvent(WindowSizeData),
-    ReflowEvent(SmallVec1<UntrustedNodeAddress>),
     ClickEvent(MouseButton, Point2D<f32>),
     MouseDownEvent(MouseButton, Point2D<f32>),
     MouseUpEvent(MouseButton, Point2D<f32>),

servo/components/servo/lib.rs

@@ -46,17 +46,15 @@ use msg::constellation_msg::ConstellationChan;
 
 use script::dom::bindings::codegen::RegisterBindings;
 
-use net::image_cache_task::{ImageCacheTaskFactory, LoadPlaceholder};
+use net::image_cache_task::new_image_cache_task;
 use net::storage_task::StorageTaskFactory;
 use net::resource_task::new_resource_task;
-use net_traits::image_cache_task::ImageCacheTask;
 use net_traits::storage_task::StorageTask;
 
 use gfx::font_cache_task::FontCacheTask;
 use profile::mem;
 use profile::time;
 use util::opts;
-use util::taskpool::TaskPool;
 
 use std::rc::Rc;
 use std::sync::mpsc::Sender;
@@ -88,10 +86,6 @@ impl Browser  {
         // bindings to implement JS proxies.
         RegisterBindings::RegisterProxyHandlers();
 
-        // Use this thread pool to load-balance simple tasks, such as
-        // image decoding.
-        let shared_task_pool = TaskPool::new(8);
-
         // Get both endpoints of a special channel for communication between
         // the client window and the compositor. This channel is unique because
         // messages to client may need to pump a platform-specific event loop
@@ -111,8 +105,7 @@ impl Browser  {
                                                       compositor_proxy.clone_compositor_proxy(),
                                                       time_profiler_chan.clone(),
                                                       devtools_chan,
-                                                      mem_profiler_chan.clone(),
-                                                      shared_task_pool);
+                                                      mem_profiler_chan.clone());
 
         if let Some(port) = opts.webdriver_port {
             webdriver_server::start_server(port, constellation_chan.clone());
@@ -156,24 +149,13 @@ fn create_constellation(opts: opts::Opts,
                         compositor_proxy: Box<CompositorProxy+Send>,
                         time_profiler_chan: time::ProfilerChan,
                         devtools_chan: Option<Sender<devtools_traits::DevtoolsControlMsg>>,
-                        mem_profiler_chan: mem::ProfilerChan,
-                        shared_task_pool: TaskPool) -> ConstellationChan {
+                        mem_profiler_chan: mem::ProfilerChan) -> ConstellationChan {
     use std::env;
 
     // Create a Servo instance.
     let resource_task = new_resource_task(opts.user_agent.clone());
 
-    // If we are emitting an output file, then we need to block on
-    // image load or we risk emitting an output file missing the
-    // image.
-    let image_cache_task: ImageCacheTask = if opts.output_file.is_some() {
-        ImageCacheTaskFactory::new_sync(resource_task.clone(), shared_task_pool,
-                                        time_profiler_chan.clone(), LoadPlaceholder::Preload)
-    } else {
-        ImageCacheTaskFactory::new(resource_task.clone(), shared_task_pool,
-                                   time_profiler_chan.clone(), LoadPlaceholder::Preload)
-    };
-
+    let image_cache_task = new_image_cache_task(resource_task.clone());
     let font_cache_task = FontCacheTask::new(resource_task.clone());
     let storage_task: StorageTask = StorageTaskFactory::new();
 

servo/ports/gonk/src/lib.rs

@@ -39,12 +39,10 @@ use msg::constellation_msg::ConstellationChan;
 use script::dom::bindings::codegen::RegisterBindings;
 
 #[cfg(not(test))]
-use net::image_cache_task::{ImageCacheTaskFactory, LoadPlaceholder};
+use net::image_cache_task::new_image_cache_task;
 #[cfg(not(test))]
 use net::storage_task::StorageTaskFactory;
 #[cfg(not(test))]
-use net_traits::image_cache_task::ImageCacheTask;
-#[cfg(not(test))]
 use net::resource_task::new_resource_task;
 #[cfg(not(test))]
 use gfx::font_cache_task::FontCacheTask;
@@ -108,17 +106,7 @@ impl Browser {
         // Create a Servo instance.
         let resource_task = new_resource_task(opts.user_agent.clone());
 
-        // If we are emitting an output file, then we need to block on
-        // image load or we risk emitting an output file missing the
-        // image.
-        let image_cache_task: ImageCacheTask = if opts.output_file.is_some() {
-            ImageCacheTaskFactory::new_sync(resource_task.clone(), shared_task_pool,
-                                            time_profiler_chan.clone(), LoadPlaceholder::Preload)
-        } else {
-            ImageCacheTaskFactory::new(resource_task.clone(), shared_task_pool,
-                                       time_profiler_chan.clone(), LoadPlaceholder::Preload)
-         };
-
+        let image_cache_task = new_image_cache_task(resource_task.clone());
         let font_cache_task = FontCacheTask::new(resource_task.clone());
         let storage_task = StorageTaskFactory::new();
 

servo/tests/unit/net/image_cache_task.rs

@@ -1,574 +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 net::image_cache_task::*;
-use net_traits::image_cache_task::ImageResponseMsg::*;
-use net_traits::image_cache_task::Msg::*;
-
-use net::resource_task::{start_sending, ProgressSender};
-use net_traits::{ControlMsg, Metadata, ResourceTask};
-use net_traits::image_cache_task::{ImageCacheTask, ImageCacheTaskClient, ImageResponseMsg, Msg};
-use net_traits::ProgressMsg::{Payload, Done};
-use profile::time;
-use std::sync::mpsc::{Sender, channel, Receiver};
-use url::Url;
-use util::taskpool::TaskPool;
-
-static TEST_IMAGE: &'static [u8] = include_bytes!("test.jpeg");
-
-pub fn test_image_bin() -> Vec<u8> {
-    TEST_IMAGE.iter().map(|&x| x).collect()
-}
-
-trait ImageCacheTaskHelper {
-    fn wait_for_store(&self) -> Receiver<()>;
-    fn wait_for_store_prefetched(&self) -> Receiver<()>;
-}
-
-impl ImageCacheTaskHelper for ImageCacheTask {
-    fn wait_for_store(&self) -> Receiver<()> {
-        let (chan, port) = channel();
-        self.send(Msg::WaitForStore(chan));
-        port
-    }
-
-    fn wait_for_store_prefetched(&self) -> Receiver<()> {
-        let (chan, port) = channel();
-        self.send(Msg::WaitForStorePrefetched(chan));
-        port
-    }
-}
-
-trait Closure {
-    fn invoke(&self, _response: ProgressSender) { }
-}
-struct DoesNothing;
-impl Closure for DoesNothing { }
-
-struct JustSendOK {
-    url_requested_chan: Sender<()>,
-}
-impl Closure for JustSendOK {
-    fn invoke(&self, response: ProgressSender) {
-        self.url_requested_chan.send(()).unwrap();
-        response.send(Done(Ok(()))).unwrap();
-    }
-}
-
-struct SendTestImage;
-impl Closure for SendTestImage {
-    fn invoke(&self, response: ProgressSender) {
-        response.send(Payload(test_image_bin())).unwrap();
-        response.send(Done(Ok(()))).unwrap();
-    }
-}
-
-struct SendBogusImage;
-impl Closure for SendBogusImage {
-    fn invoke(&self, response: ProgressSender) {
-        response.send(Payload(vec!())).unwrap();
-        response.send(Done(Ok(()))).unwrap();
-    }
-}
-
-struct SendTestImageErr;
-impl Closure for SendTestImageErr {
-    fn invoke(&self, response: ProgressSender) {
-        response.send(Payload(test_image_bin())).unwrap();
-        response.send(Done(Err("".to_string()))).unwrap();
-    }
-}
-
-struct WaitSendTestImage {
-    wait_port: Receiver<()>,
-}
-impl Closure for WaitSendTestImage {
-    fn invoke(&self, response: ProgressSender) {
-        // Don't send the data until after the client requests
-        // the image
-        self.wait_port.recv().unwrap();
-        response.send(Payload(test_image_bin())).unwrap();
-        response.send(Done(Ok(()))).unwrap();
-    }
-}
-
-struct WaitSendTestImageErr {
-    wait_port: Receiver<()>,
-}
-impl Closure for WaitSendTestImageErr {
-    fn invoke(&self, response: ProgressSender) {
-        // Don't send the data until after the client requests
-        // the image
-        self.wait_port.recv().unwrap();
-        response.send(Payload(test_image_bin())).unwrap();
-        response.send(Done(Err("".to_string()))).unwrap();
-    }
-}
-
-fn mock_resource_task<T: Closure + Send + 'static>(on_load: Box<T>) -> ResourceTask {
-    spawn_listener(move |port: Receiver<ControlMsg>| {
-        loop {
-            match port.recv().unwrap() {
-                ControlMsg::Load(_, consumer) => {
-                    let chan = start_sending(consumer, Metadata::default(
-                        Url::parse("file:///fake").unwrap()));
-                    on_load.invoke(chan);
-                }
-                ControlMsg::Exit => break,
-                _ => {}
-            }
-        }
-    })
-}
-
-fn profiler() -> time::ProfilerChan {
-    time::Profiler::create(None)
-}
-
-#[test]
-fn should_exit_on_request() {
-    let mock_resource_task = mock_resource_task(Box::new(DoesNothing));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-#[should_panic]
-fn should_panic_if_unprefetched_image_is_requested() {
-    let mock_resource_task = mock_resource_task(Box::new(DoesNothing));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let (chan, port) = channel();
-    image_cache_task.send(Msg::GetImage(url, chan));
-    port.recv().unwrap();
-}
-
-#[test]
-fn should_request_url_from_resource_task_on_prefetch() {
-    let (url_requested_chan, url_requested) = channel();
-
-    let mock_resource_task = mock_resource_task(Box::new(JustSendOK { url_requested_chan: url_requested_chan}));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url));
-    url_requested.recv().unwrap();
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_not_request_url_from_resource_task_on_multiple_prefetches() {
-    let (url_requested_chan, url_requested) = channel();
-
-    let mock_resource_task = mock_resource_task(Box::new(JustSendOK { url_requested_chan: url_requested_chan}));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Prefetch(url));
-    url_requested.recv().unwrap();
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-    match url_requested.try_recv() {
-        Err(_) => (),
-        Ok(_) => panic!(),
-    };
-}
-
-#[test]
-fn should_return_image_not_ready_if_data_has_not_arrived() {
-    let (wait_chan, wait_port) = channel();
-
-    let mock_resource_task = mock_resource_task(Box::new(WaitSendTestImage{wait_port: wait_port}));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-    assert!(response_port.recv().unwrap() == ImageResponseMsg::ImageNotReady);
-    wait_chan.send(()).unwrap();
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_decoded_image_data_if_data_has_arrived() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImage));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-    match response_port.recv().unwrap() {
-      ImageResponseMsg::ImageReady(_) => (),
-      _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_decoded_image_data_for_multiple_requests() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImage));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    for _ in 0..2 {
-        let (response_chan, response_port) = channel();
-        image_cache_task.send(Msg::GetImage(url.clone(), response_chan));
-        match response_port.recv().unwrap() {
-          ImageResponseMsg::ImageReady(_) => (),
-          _ => panic!("bleh")
-        }
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_not_request_image_from_resource_task_if_image_is_already_available() {
-    let (image_bin_sent_chan, image_bin_sent) = channel();
-
-    let (resource_task_exited_chan, resource_task_exited) = channel();
-
-    let mock_resource_task = spawn_listener(move |port: Receiver<ControlMsg>| {
-        loop {
-            match port.recv().unwrap() {
-                ControlMsg::Load(_, consumer) => {
-                    let chan = start_sending(consumer, Metadata::default(
-                        Url::parse("file:///fake").unwrap()));
-                    chan.send(Payload(test_image_bin())).unwrap();
-                    chan.send(Done(Ok(()))).unwrap();
-                    image_bin_sent_chan.send(()).unwrap();
-                }
-                ControlMsg::Exit => {
-                    resource_task_exited_chan.send(()).unwrap();
-                    break
-                }
-                _ => {}
-            }
-        }
-    });
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    image_bin_sent.recv().unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-
-    resource_task_exited.recv().unwrap();
-
-    // Our resource task should not have received another request for the image
-    // because it's already cached
-    match image_bin_sent.try_recv() {
-        Err(_) => (),
-        Ok(_) => panic!(),
-    }
-}
-
-#[test]
-fn should_not_request_image_from_resource_task_if_image_fetch_already_failed() {
-    let (image_bin_sent_chan, image_bin_sent) = channel();
-
-    let (resource_task_exited_chan, resource_task_exited) = channel();
-    let mock_resource_task = spawn_listener(move |port: Receiver<ControlMsg>| {
-        loop {
-            match port.recv().unwrap() {
-                ControlMsg::Load(_, consumer) => {
-                    let chan = start_sending(consumer, Metadata::default(
-                        Url::parse("file:///fake").unwrap()));
-                    chan.send(Payload(test_image_bin())).unwrap();
-                    chan.send(Done(Err("".to_string()))).unwrap();
-                    image_bin_sent_chan.send(()).unwrap();
-                }
-                ControlMsg::Exit => {
-                    resource_task_exited_chan.send(()).unwrap();
-                    break
-                }
-                _ => {}
-            }
-        }
-    });
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    image_bin_sent.recv().unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-
-    resource_task_exited.recv().unwrap();
-
-    // Our resource task should not have received another request for the image
-    // because it's already cached
-    match image_bin_sent.try_recv() {
-        Err(_) => (),
-        Ok(_) => panic!(),
-    }
-}
-
-#[test]
-fn should_return_failed_if_image_bin_cannot_be_fetched() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImageErr));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store_prefetched();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageFailed => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_failed_for_multiple_get_image_requests_if_image_bin_cannot_be_fetched() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImageErr));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store_prefetched();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url.clone(), response_chan));
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageFailed => (),
-        _ => panic!("bleh")
-    }
-
-    // And ask again, we should get the same response
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageFailed => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_failed_if_image_decode_fails() {
-    let mock_resource_task = mock_resource_task(Box::new(SendBogusImage));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    // Make the request
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageFailed => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_image_on_wait_if_image_is_already_loaded() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImage));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    let join_port = image_cache_task.wait_for_store();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    // Wait until our mock resource task has sent the image to the image cache
-    join_port.recv().unwrap();
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::WaitForImage(url, response_chan));
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageReady(..) => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_image_on_wait_if_image_is_not_yet_loaded() {
-    let (wait_chan, wait_port) = channel();
-
-    let mock_resource_task = mock_resource_task(Box::new(WaitSendTestImage {wait_port: wait_port}));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::WaitForImage(url, response_chan));
-
-    wait_chan.send(()).unwrap();
-
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageReady(..) => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn should_return_image_failed_on_wait_if_image_fails_to_load() {
-    let (wait_chan, wait_port) = channel();
-
-    let mock_resource_task = mock_resource_task(Box::new(WaitSendTestImageErr{wait_port: wait_port}));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new(mock_resource_task.clone(),
-                                                                      TaskPool::new(4), profiler(),
-                                                                      LoadPlaceholder::Ignore);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::WaitForImage(url, response_chan));
-
-    wait_chan.send(()).unwrap();
-
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageFailed => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}
-
-#[test]
-fn sync_cache_should_wait_for_images() {
-    let mock_resource_task = mock_resource_task(Box::new(SendTestImage));
-
-    let image_cache_task: ImageCacheTask = ImageCacheTaskFactory::new_sync(mock_resource_task.clone(),
-                                                                           TaskPool::new(4), profiler(),
-                                                                           LoadPlaceholder::Preload);
-    let url = Url::parse("file:///").unwrap();
-
-    image_cache_task.send(Prefetch(url.clone()));
-    image_cache_task.send(Decode(url.clone()));
-
-    let (response_chan, response_port) = channel();
-    image_cache_task.send(Msg::GetImage(url, response_chan));
-    match response_port.recv().unwrap() {
-        ImageResponseMsg::ImageReady(_) => (),
-        _ => panic!("bleh")
-    }
-
-    image_cache_task.exit();
-    mock_resource_task.send(ControlMsg::Exit).unwrap();
-}

servo/tests/unit/net/lib.rs

@@ -12,6 +12,5 @@ extern crate util;
 
 #[cfg(test)] mod cookie;
 #[cfg(test)] mod data_loader;
-#[cfg(test)] mod image_cache_task;
 #[cfg(test)] mod mime_classifier;
 #[cfg(test)] mod resource_task;