net: Don't load the placeholder image for background images, only for

image fragments.

This also changes the way the placeholder is handled in the image cache
task to decode it up front instead of each time an image fails to load,
both because it was more convenient to implement that way and because
it saves CPU cycles to do so.

This matches the behavior of Gecko and WebKit. It improves the look of
our cached copy of Wikipedia.

11 files changed, 135 insertions, 38 deletions

diff --git a/components/layout/context.rs b/components/layout/context.rs
index e83f4453b04..08114bec3e8 100644
--- a/components/layout/context.rs
+++ b/components/layout/context.rs
@@ -15,7 +15,8 @@ use gfx::font_context::FontContext;
 use msg::compositor_msg::LayerId;
 use msg::constellation_msg::ConstellationChan;
 use net_traits::image::base::Image;
-use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageState};
+use net_traits::image_cache_task::{ImageCacheChan, ImageCacheTask, ImageResponse, ImageState};
+use net_traits::image_cache_task::{UsePlaceholder};
 use script::layout_interface::{Animation, LayoutChan, ReflowGoal};
 use std::boxed;
 use std::cell::Cell;
@@ -154,9 +155,11 @@ impl<'a> LayoutContext<'a> {
         }
     }
 
-    pub fn get_or_request_image(&self, url: Url) -> Option<Arc<Image>> {
+    pub fn get_or_request_image(&self, url: Url, use_placeholder: UsePlaceholder)
+                                -> Option<Arc<Image>> {
         // See if the image is already available
-        let result = self.shared.image_cache_task.get_image_if_available(url.clone());
+        let result = self.shared.image_cache_task.get_image_if_available(url.clone(),
+                                                                         use_placeholder);
 
         match result {
             Ok(image) => Some(image),
@@ -174,7 +177,11 @@ impl<'a> LayoutContext<'a> {
                         self.shared.image_cache_task.request_image(url,
                                                                    ImageCacheChan(sync_tx),
                                                                    None);
-                        sync_rx.recv().unwrap().image
+                        match sync_rx.recv().unwrap().image_response {
+                            ImageResponse::Loaded(image) |
+                            ImageResponse::PlaceholderLoaded(image) => Some(image),
+                            ImageResponse::None => None,
+                        }
                     }
                     // Not yet requested, async mode - request image from the cache
                     (ImageState::NotRequested, false) => {
diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs
index aac8a7c2317..dc1ba50699c 100644
--- a/components/layout/display_list_builder.rs
+++ b/components/layout/display_list_builder.rs
@@ -35,6 +35,7 @@ use gfx::paint_task::{PaintLayer, THREAD_TINT_COLORS};
 use msg::compositor_msg::ScrollPolicy;
 use msg::constellation_msg::ConstellationChan;
 use msg::constellation_msg::Msg as ConstellationMsg;
+use net_traits::image_cache_task::UsePlaceholder;
 use png::{self, PixelsByColorType};
 use std::cmp;
 use std::default::Default;
@@ -432,7 +433,7 @@ impl FragmentDisplayListBuilding for Fragment {
                                                clip: &ClippingRegion,
                                                image_url: &Url) {
         let background = style.get_background();
-        let image = layout_context.get_or_request_image(image_url.clone());
+        let image = layout_context.get_or_request_image(image_url.clone(), UsePlaceholder::No);
         if let Some(image) = image {
             debug!("(building display list) building background image");
 
diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs
index c8d496c04bf..b7b80c2da53 100644
--- a/components/layout/fragment.rs
+++ b/components/layout/fragment.rs
@@ -27,6 +27,7 @@ use gfx::text::glyph::CharIndex;
 use gfx::text::text_run::{TextRun, TextRunSlice};
 use msg::constellation_msg::{ConstellationChan, Msg, PipelineId, SubpageId};
 use net_traits::image::base::Image;
+use net_traits::image_cache_task::UsePlaceholder;
 use rustc_serialize::{Encodable, Encoder};
 use script_traits::UntrustedNodeAddress;
 use std::borrow::ToOwned;
@@ -337,7 +338,9 @@ impl ImageFragmentInfo {
                    .map(Au::from_px)
         }
 
-        let image = url.and_then(|url| layout_context.get_or_request_image(url));
+        let image = url.and_then(|url| {
+            layout_context.get_or_request_image(url, UsePlaceholder::Yes)
+        });
 
         ImageFragmentInfo {
             replaced_image_fragment_info: ReplacedImageFragmentInfo::new(node,
diff --git a/components/net/image_cache_task.rs b/components/net/image_cache_task.rs
index 66636487542..7b322f85b40 100644
--- a/components/net/image_cache_task.rs
+++ b/components/net/image_cache_task.rs
@@ -4,7 +4,8 @@
 
 use collections::borrow::ToOwned;
 use net_traits::image::base::{Image, load_from_memory};
-use net_traits::image_cache_task::{ImageState, ImageCacheTask, ImageCacheChan, ImageCacheCommand, ImageCacheResult};
+use net_traits::image_cache_task::{ImageState, ImageCacheTask, ImageCacheChan, ImageCacheCommand};
+use net_traits::image_cache_task::{ImageCacheResult, ImageResponse, UsePlaceholder};
 use net_traits::load_whole_resource;
 use std::collections::HashMap;
 use std::collections::hash_map::Entry::{Occupied, Vacant};
@@ -53,13 +54,13 @@ impl PendingLoad {
 /// failure) are still stored here, so that they aren't
 /// fetched again.
 struct CompletedLoad {
-    image: Option<Arc<Image>>,
+    image_response: ImageResponse,
 }
 
 impl CompletedLoad {
-    fn new(image: Option<Arc<Image>>) -> CompletedLoad {
+    fn new(image_response: ImageResponse) -> CompletedLoad {
         CompletedLoad {
-            image: image,
+            image_response: image_response,
         }
     }
 }
@@ -79,11 +80,11 @@ impl ImageListener {
         }
     }
 
-    fn notify(self, image: Option<Arc<Image>>) {
+    fn notify(self, image_response: ImageResponse) {
         let ImageCacheChan(ref sender) = self.sender;
         let msg = ImageCacheResult {
             responder: self.responder,
-            image: image,
+            image_response: image_response,
         };
         sender.send(msg).ok();
     }
@@ -210,10 +211,19 @@ impl ImageCache {
             ImageCacheCommand::RequestImage(url, result_chan, responder) => {
                 self.request_image(url, result_chan, responder);
             }
-            ImageCacheCommand::GetImageIfAvailable(url, consumer) => {
+            ImageCacheCommand::GetImageIfAvailable(url, use_placeholder, consumer) => {
                 let result = match self.completed_loads.get(&url) {
                     Some(completed_load) => {
-                        completed_load.image.clone().ok_or(ImageState::LoadError)
+                        match (completed_load.image_response.clone(), use_placeholder) {
+                            (ImageResponse::Loaded(image), _) |
+                            (ImageResponse::PlaceholderLoaded(image), UsePlaceholder::Yes) => {
+                                Ok(image)
+                            }
+                            (ImageResponse::PlaceholderLoaded(_), UsePlaceholder::No) |
+                            (ImageResponse::None, _) => {
+                                Err(ImageState::LoadError)
+                            }
+                        }
                     }
                     None => {
                         let pending_load = self.pending_loads.get(&url);
@@ -255,8 +265,13 @@ impl ImageCache {
                         });
                     }
                     Err(_) => {
-                        let placeholder_image = self.placeholder_image.clone();
-                        self.complete_load(msg.url, placeholder_image);
+                        match self.placeholder_image.clone() {
+                            Some(placeholder_image) => {
+                                self.complete_load(msg.url, ImageResponse::PlaceholderLoaded(
+                                        placeholder_image))
+                            }
+                            None => self.complete_load(msg.url, ImageResponse::None),
+                        }
                     }
                 }
             }
@@ -265,31 +280,37 @@ impl ImageCache {
 
     // Handle a message from one of the decoder worker threads
     fn handle_decoder(&mut self, msg: DecoderMsg) {
-        let image = msg.image.map(Arc::new);
+        let image = match msg.image {
+            None => ImageResponse::None,
+            Some(image) => ImageResponse::Loaded(Arc::new(image)),
+        };
         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>>) {
+    fn complete_load(&mut self, url: Url, image_response: ImageResponse) {
         let pending_load = self.pending_loads.remove(&url).unwrap();
 
-        let completed_load = CompletedLoad::new(image.clone());
+        let completed_load = CompletedLoad::new(image_response.clone());
         self.completed_loads.insert(url, completed_load);
 
         for listener in pending_load.listeners.into_iter() {
-            listener.notify(image.clone());
+            listener.notify(image_response.clone());
         }
     }
 
     // Request an image from the cache
-    fn request_image(&mut self, url: Url, result_chan: ImageCacheChan, responder: Option<Box<ImageResponder>>) {
+    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());
+                image_listener.notify(completed_load.image_response.clone());
             }
             None => {
                 // Check if the load is already pending
@@ -366,3 +387,4 @@ pub fn new_image_cache_task(resource_task: ResourceTask) -> ImageCacheTask {
 
     ImageCacheTask::new(cmd_sender)
 }
+
diff --git a/components/net_traits/image_cache_task.rs b/components/net_traits/image_cache_task.rs
index dd6bb3c98ff..3614cf4d58a 100644
--- a/components/net_traits/image_cache_task.rs
+++ b/components/net_traits/image_cache_task.rs
@@ -12,7 +12,7 @@ use std::sync::mpsc::{channel, Sender};
 /// image load completes. It is typically used to trigger a reflow
 /// and/or repaint.
 pub trait ImageResponder : Send {
-    fn respond(&self, Option<Arc<Image>>);
+    fn respond(&self, ImageResponse);
 }
 
 /// The current state of an image in the cache.
@@ -23,6 +23,17 @@ pub enum ImageState {
     NotRequested,
 }
 
+/// The returned image.
+#[derive(Clone)]
+pub enum ImageResponse {
+    /// The requested image was loaded.
+    Loaded(Arc<Image>),
+    /// The requested image failed to load, so a placeholder was loaded instead.
+    PlaceholderLoaded(Arc<Image>),
+    /// Neither the requested image nor the placeholder could be loaded.
+    None
+}
+
 /// Channel for sending commands to the image cache.
 #[derive(Clone)]
 pub struct ImageCacheChan(pub Sender<ImageCacheResult>);
@@ -31,7 +42,7 @@ pub struct ImageCacheChan(pub Sender<ImageCacheResult>);
 /// caller.
 pub struct ImageCacheResult {
     pub responder: Option<Box<ImageResponder>>,
-    pub image: Option<Arc<Image>>,
+    pub image_response: ImageResponse,
 }
 
 /// Commands that the image cache understands.
@@ -45,12 +56,18 @@ pub enum ImageCacheCommand {
     /// 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>>),
+    GetImageIfAvailable(Url, UsePlaceholder, Sender<Result<Arc<Image>, ImageState>>),
 
     /// Clients must wait for a response before shutting down the ResourceTask
     Exit(Sender<()>),
 }
 
+#[derive(Copy, Clone, PartialEq)]
+pub enum UsePlaceholder {
+    No,
+    Yes,
+}
+
 /// The client side of the image cache task. This can be safely cloned
 /// and passed to different tasks.
 #[derive(Clone)]
@@ -78,9 +95,10 @@ impl ImageCacheTask {
     }
 
     /// Get the current state of an image. See ImageCacheCommand::GetImageIfAvailable.
-    pub fn get_image_if_available(&self, url: Url) -> Result<Arc<Image>, ImageState> {
+    pub fn get_image_if_available(&self, url: Url, use_placeholder: UsePlaceholder)
+                                  -> Result<Arc<Image>, ImageState> {
         let (sender, receiver) = channel();
-        let msg = ImageCacheCommand::GetImageIfAvailable(url, sender);
+        let msg = ImageCacheCommand::GetImageIfAvailable(url, use_placeholder, sender);
         self.chan.send(msg).unwrap();
         receiver.recv().unwrap()
     }
@@ -92,3 +110,4 @@ impl ImageCacheTask {
         response_port.recv().unwrap();
     }
 }
+
diff --git a/components/script/dom/canvasrenderingcontext2d.rs b/components/script/dom/canvasrenderingcontext2d.rs
index 6353479b116..ad49955c64f 100644
--- a/components/script/dom/canvasrenderingcontext2d.rs
+++ b/components/script/dom/canvasrenderingcontext2d.rs
@@ -33,14 +33,12 @@ use canvas::canvas_paint_task::{CanvasPaintTask, FillOrStrokeStyle};
 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::ImageCacheChan;
+use net_traits::image_cache_task::{ImageCacheChan, ImageResponse};
 use png::PixelsByColorType;
 
 use num::{Float, ToPrimitive};
 use std::borrow::ToOwned;
 use std::cell::RefCell;
-use std::sync::{Arc};
 use std::sync::mpsc::{channel, Sender};
 
 use util::str::DOMString;
@@ -260,8 +258,8 @@ impl CanvasRenderingContext2D {
         };
 
         let img = match self.request_image_from_cache(url) {
-            Some(img) => img,
-            None => return None,
+            ImageResponse::Loaded(img) => img,
+            ImageResponse::PlaceholderLoaded(_) | ImageResponse::None => return None,
         };
 
         let image_size = Size2D(img.width as f64, img.height as f64);
@@ -277,7 +275,7 @@ impl CanvasRenderingContext2D {
         return Some((image_data, image_size));
     }
 
-    fn request_image_from_cache(&self, url: Url) -> Option<Arc<Image>> {
+    fn request_image_from_cache(&self, url: Url) -> ImageResponse {
         let canvas = self.canvas.root();
         let window = window_from_node(canvas.r()).root();
         let window = window.r();
@@ -285,7 +283,7 @@ impl CanvasRenderingContext2D {
         let (response_chan, response_port) = channel();
         image_cache.request_image(url, ImageCacheChan(response_chan), None);
         let result = response_port.recv().unwrap();
-        result.image
+        result.image_response
     }
 
     fn create_drawable_rect(&self, x: f64, y: f64, w: f64, h: f64) -> Option<Rect<f32>> {
diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs
index a88f4b5ea7b..09b6c950da4 100644
--- a/components/script/dom/htmlimageelement.rs
+++ b/components/script/dom/htmlimageelement.rs
@@ -25,7 +25,7 @@ use util::str::DOMString;
 use string_cache::Atom;
 
 use net_traits::image::base::Image;
-use net_traits::image_cache_task::ImageResponder;
+use net_traits::image_cache_task::{ImageResponder, ImageResponse};
 use url::{Url, UrlParser};
 
 use std::borrow::ToOwned;
@@ -74,11 +74,16 @@ impl Responder {
 }
 
 impl ImageResponder for Responder {
-    fn respond(&self, image: Option<Arc<Image>>) {
+    fn respond(&self, image: ImageResponse) {
         // Update the image field
         let element = self.element.to_temporary().root();
         let element_ref = element.r();
-        *element_ref.image.borrow_mut() = image;
+        *element_ref.image.borrow_mut() = match image {
+            ImageResponse::Loaded(image) | ImageResponse::PlaceholderLoaded(image) => {
+                Some(image)
+            }
+            ImageResponse::None => None,
+        };
 
         // Mark the node dirty
         let node = NodeCast::from_ref(element.r());
diff --git a/components/script/script_task.rs b/components/script/script_task.rs
index 014e32f2f97..df049115e20 100644
--- a/components/script/script_task.rs
+++ b/components/script/script_task.rs
@@ -799,7 +799,7 @@ impl ScriptTask {
     }
 
     fn handle_msg_from_image_cache(&self, msg: ImageCacheResult) {
-        msg.responder.unwrap().respond(msg.image);
+        msg.responder.unwrap().respond(msg.image_response);
     }
 
     fn handle_webdriver_msg(&self, pipeline_id: PipelineId, msg: WebDriverScriptCommand) {
diff --git a/tests/ref/basic.list b/tests/ref/basic.list
index 27d5b8aa0ce..139d3adedc1 100644
--- a/tests/ref/basic.list
+++ b/tests/ref/basic.list
@@ -205,6 +205,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html
 == negative_margin_uncle_a.html negative_margin_uncle_b.html
 == negative_margins_a.html negative_margins_b.html
 == no-image.html no-image-ref.html
+== no_image_background_a.html no_image_background_ref.html
 == noscript.html noscript_ref.html
 != noteq_attr_exists_selector.html attr_exists_selector_ref.html
 == nth_child_pseudo_a.html nth_child_pseudo_b.html
diff --git a/tests/ref/no_image_background_a.html b/tests/ref/no_image_background_a.html
new file mode 100644
index 00000000000..188112e3dc1
--- /dev/null
+++ b/tests/ref/no_image_background_a.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    background: peachpuff;
+}
+section {
+    display: block;
+    width: 256px;
+    height: 256px;
+    border: solid black 1px;
+    background: url(bogusybogusbogus.jpg);
+}
+</style>
+</head>
+<body>
+<section></section>
+</body>
+</html>
+
diff --git a/tests/ref/no_image_background_ref.html b/tests/ref/no_image_background_ref.html
new file mode 100644
index 00000000000..d1793b43b86
--- /dev/null
+++ b/tests/ref/no_image_background_ref.html
@@ -0,0 +1,20 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+body {
+    background: peachpuff;
+}
+section {
+    display: block;
+    width: 256px;
+    height: 256px;
+    border: solid black 1px;
+}
+</style>
+</head>
+<body>
+<section></section>
+</body>
+</html>
+