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.

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

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

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,

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();
+                        match self.placeholder_image.clone() {
-                        self.complete_load(msg.url, placeholder_image);
+                            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)
 }
+

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

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, ImageResponse};
-use net_traits::image_cache_task::ImageCacheChan;
 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,
+            ImageResponse::Loaded(img) => img,
-            None => return None,
+            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>> {

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

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

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

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

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