Refactored image cache task - details below.

* 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.

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);
         }
     }
 }

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);

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>> {

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");

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();
             },
             _ => ()
         }

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
 };

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);