Load a placeholder when a url to an image is broken.

I decided to use the old Netscape broken image link icon (later we may
replace the image asset for something more trendier). The ref test will
expect that a failed load should display the rippy image.

ImageCacheTask users can define if a placeholder image should be loaded
at start up or not. This enables both the new behavior (e.g. always
return an image even for broken urls) as also the previous one.

components/net/image_cache_task.rs

@@ -15,6 +15,7 @@ use std::mem::replace;
 use std::sync::{Arc, Mutex};
 use std::sync::mpsc::{channel, Receiver, Sender};
 use url::Url;
+use util::resource_files::resources_dir_path;
 use util::task::spawn_named;
 use util::taskpool::TaskPool;
 
@@ -75,7 +76,8 @@ pub struct ImageCacheTask {
 
 impl ImageCacheTask {
     pub fn new(resource_task: ResourceTask, task_pool: TaskPool,
-               time_profiler_chan: time::ProfilerChan) -> ImageCacheTask {
+               time_profiler_chan: time::ProfilerChan,
+               load_placeholder: LoadPlaceholder) -> ImageCacheTask {
         let (chan, port) = channel();
         let chan_clone = chan.clone();
 
@@ -89,8 +91,9 @@ impl ImageCacheTask {
                 need_exit: None,
                 task_pool: task_pool,
                 time_profiler_chan: time_profiler_chan,
+                placeholder_data: Arc::new(vec!()),
             };
-            cache.run();
+            cache.run(load_placeholder);
         });
 
         ImageCacheTask {
@@ -99,12 +102,13 @@ impl ImageCacheTask {
     }
 
     pub fn new_sync(resource_task: ResourceTask, task_pool: TaskPool,
-                    time_profiler_chan: time::ProfilerChan) -> ImageCacheTask {
+                    time_profiler_chan: time::ProfilerChan,
+                    load_placeholder: LoadPlaceholder) -> ImageCacheTask {
         let (chan, port) = channel();
 
         spawn_named("ImageCacheTask (sync)".to_owned(), move || {
             let inner_cache = ImageCacheTask::new(resource_task, task_pool,
-                                                  time_profiler_chan);
+                                                  time_profiler_chan, load_placeholder);
 
             loop {
                 let msg: Msg = port.recv().unwrap();
@@ -142,6 +146,8 @@ struct ImageCache {
     need_exit: Option<Sender<()>>,
     task_pool: TaskPool,
     time_profiler_chan: time::ProfilerChan,
+    // Default image used when loading fails.
+    placeholder_data: Arc<Vec<u8>>,
 }
 
 #[derive(Clone)]
@@ -160,8 +166,32 @@ enum AfterPrefetch {
     DoNotDecode
 }
 
+pub enum LoadPlaceholder {
+    Preload,
+    Ignore
+}
+
 impl ImageCache {
-    pub fn run(&mut self) {
+    // Used to preload the default placeholder.
+    fn init(&mut self) {
+        let mut placeholder_url = resources_dir_path();
+        // TODO (Savago): replace for a prettier one.
+        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;
 
@@ -245,12 +275,12 @@ impl ImageCache {
                 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());
+                    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());
                 });
@@ -446,9 +476,9 @@ impl ImageCacheTask {
     }
 }
 
-fn load_image_data(url: Url, resource_task: ResourceTask) -> Result<Vec<u8>, ()> {
+fn load_image_data(url: Url, resource_task: ResourceTask, placeholder: &[u8]) -> Result<Vec<u8>, ()> {
     let (response_chan, response_port) = channel();
-    resource_task.send(resource_task::ControlMsg::Load(LoadData::new(url, response_chan))).unwrap();
+    resource_task.send(resource_task::ControlMsg::Load(LoadData::new(url.clone(), response_chan))).unwrap();
 
     let mut image_data = vec!();
 
@@ -462,7 +492,19 @@ fn load_image_data(url: Url, resource_task: ResourceTask) -> Result<Vec<u8>, ()>
                 return Ok(image_data);
             }
             Done(Err(..)) => {
-                return 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(());
+                }
             }
         }
     }
@@ -595,7 +637,7 @@ mod tests {
     fn should_exit_on_request() {
         let mock_resource_task = mock_resource_task(box DoesNothing);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
 
         image_cache_task.exit();
         mock_resource_task.send(resource_task::ControlMsg::Exit);
@@ -606,7 +648,7 @@ mod tests {
     fn should_panic_if_unprefetched_image_is_requested() {
         let mock_resource_task = mock_resource_task(box DoesNothing);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Preload);
         let url = Url::parse("file:///").unwrap();
 
         let (chan, port) = channel();
@@ -620,7 +662,7 @@ mod tests {
 
         let mock_resource_task = mock_resource_task(box JustSendOK { url_requested_chan: url_requested_chan});
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Preload);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url));
@@ -635,7 +677,7 @@ mod tests {
 
         let mock_resource_task = mock_resource_task(box JustSendOK { url_requested_chan: url_requested_chan});
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -655,7 +697,7 @@ mod tests {
 
         let mock_resource_task = mock_resource_task(box WaitSendTestImage{wait_port: wait_port});
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -672,7 +714,7 @@ mod tests {
     fn should_return_decoded_image_data_if_data_has_arrived() {
         let mock_resource_task = mock_resource_task(box SendTestImage);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -698,7 +740,7 @@ mod tests {
     fn should_return_decoded_image_data_for_multiple_requests() {
         let mock_resource_task = mock_resource_task(box SendTestImage);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -752,7 +794,7 @@ mod tests {
             }
         });
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -804,7 +846,7 @@ mod tests {
             }
         });
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -833,7 +875,7 @@ mod tests {
     fn should_return_failed_if_image_bin_cannot_be_fetched() {
         let mock_resource_task = mock_resource_task(box SendTestImageErr);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -859,7 +901,7 @@ mod tests {
     fn should_return_failed_for_multiple_get_image_requests_if_image_bin_cannot_be_fetched() {
         let mock_resource_task = mock_resource_task(box SendTestImageErr);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -893,7 +935,7 @@ mod tests {
     fn should_return_failed_if_image_decode_fails() {
         let mock_resource_task = mock_resource_task(box SendBogusImage);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -921,7 +963,7 @@ mod tests {
     fn should_return_image_on_wait_if_image_is_already_loaded() {
         let mock_resource_task = mock_resource_task(box SendTestImage);
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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();
@@ -949,7 +991,7 @@ mod tests {
 
         let mock_resource_task = mock_resource_task(box WaitSendTestImage {wait_port: wait_port});
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -975,7 +1017,7 @@ mod tests {
 
         let mock_resource_task = mock_resource_task(box WaitSendTestImageErr{wait_port: wait_port});
 
-        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::new(mock_resource_task.clone(), TaskPool::new(4), profiler(), LoadPlaceholder::Ignore);
         let url = Url::parse("file:///").unwrap();
 
         image_cache_task.send(Prefetch(url.clone()));
@@ -999,7 +1041,7 @@ mod tests {
     fn sync_cache_should_wait_for_images() {
         let mock_resource_task = mock_resource_task(box SendTestImage);
 
-        let image_cache_task = ImageCacheTask::new_sync(mock_resource_task.clone(), TaskPool::new(4), profiler());
+        let image_cache_task = ImageCacheTask::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()));

components/servo/lib.rs

@@ -33,7 +33,7 @@ use msg::constellation_msg::ConstellationChan;
 use script::dom::bindings::codegen::RegisterBindings;
 
 #[cfg(not(test))]
-use net::image_cache_task::ImageCacheTask;
+use net::image_cache_task::{ImageCacheTask, LoadPlaceholder};
 #[cfg(not(test))]
 use net::resource_task::new_resource_task;
 #[cfg(not(test))]
@@ -84,10 +84,10 @@ impl Browser  {
         // image.
         let image_cache_task = if opts.output_file.is_some() {
             ImageCacheTask::new_sync(resource_task.clone(), shared_task_pool,
-                                     time_profiler_chan.clone())
+                                     time_profiler_chan.clone(), LoadPlaceholder::Preload)
         } else {
             ImageCacheTask::new(resource_task.clone(), shared_task_pool,
-                                time_profiler_chan.clone())
+                                time_profiler_chan.clone(), LoadPlaceholder::Preload)
         };
 
         let font_cache_task = FontCacheTask::new(resource_task.clone());

ports/gonk/src/lib.rs

@@ -38,7 +38,7 @@ use msg::constellation_msg::ConstellationChan;
 use script::dom::bindings::codegen::RegisterBindings;
 
 #[cfg(not(test))]
-use net::image_cache_task::ImageCacheTask;
+use net::image_cache_task::{ImageCacheTask, LoadPlaceholder};
 #[cfg(not(test))]
 use net::storage_task::StorageTaskFactory;
 #[cfg(not(test))]
@@ -89,10 +89,10 @@ impl Browser {
         // image.
         let image_cache_task = if opts.output_file.is_some() {
             ImageCacheTask::new_sync(resource_task.clone(), shared_task_pool,
-                                     time_profiler_chan.clone())
+                                     time_profiler_chan.clone(), LoadPlaceholder::Preload)
         } else {
             ImageCacheTask::new(resource_task.clone(), shared_task_pool,
-                                time_profiler_chan.clone())
+                                time_profiler_chan.clone(), LoadPlaceholder::Preload)
         };
         let font_cache_task = FontCacheTask::new(resource_task.clone());
         let storage_task = StorageTaskFactory::new();

tests/ref/basic.list

@@ -193,6 +193,7 @@ flaky_cpu == linebreak_simple_a.html linebreak_simple_b.html
 == multiple_css_class_a.html multiple_css_class_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
 == 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-ref.html

@@ -0,0 +1 @@
+<img width="100px" height="100px" src="rippy.jpg">

tests/ref/no-image.html

@@ -0,0 +1 @@
+<img width="100px" height="100px" src="i-feel-a-disturbance-in-the-force.wtf">