Refactor ImageCache::find_image_or_metadata API.

components/layout/context.rs

@@ -11,7 +11,7 @@ use gfx::font_cache_thread::FontCacheThread;
 use gfx::font_context::FontContext;
 use malloc_size_of::{MallocSizeOf, MallocSizeOfOps};
 use msg::constellation_msg::PipelineId;
-use net_traits::image_cache::{CanRequestImages, ImageCache, ImageState};
+use net_traits::image_cache::{CanRequestImages, ImageCache, ImageCacheResult};
 use net_traits::image_cache::{ImageOrMetadataAvailable, UsePlaceholder};
 use parking_lot::RwLock;
 use script_layout_interface::{PendingImage, PendingImageState};
@@ -122,51 +122,52 @@ impl<'a> LayoutContext<'a> {
             CanRequestImages::No
         };
 
-        // See if the image is already available
-        let result = self.image_cache.find_image_or_metadata(
+        // Check for available image or start tracking.
+        let cache_result = self.image_cache.get_cached_image_status(
             url.clone(),
             self.origin.clone(),
             None,
             use_placeholder,
             can_request,
         );
-        match result {
-            Ok(image_or_metadata) => Some(image_or_metadata),
-            // Image failed to load, so just return nothing
-            Err(ImageState::LoadError) => None,
-            // Not yet requested - request image or metadata from the cache
-            Err(ImageState::NotRequested(id)) => {
-                let image = PendingImage {
-                    state: PendingImageState::Unrequested(url),
-                    node: node.to_untrusted_node_address(),
-                    id: id,
-                    origin: self.origin.clone(),
-                };
-                self.pending_images
-                    .as_ref()
-                    .unwrap()
-                    .lock()
-                    .unwrap()
-                    .push(image);
-                None
-            },
+
+        match cache_result {
+            ImageCacheResult::Available(img_or_meta) => Some(img_or_meta),
             // 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.
-            Err(ImageState::Pending(id)) => {
+            ImageCacheResult::Pending(id) => {
                 //XXXjdm if self.pending_images is not available, we should make sure that
                 //       this node gets marked dirty again so it gets a script-initiated
                 //       reflow that deals with this properly.
                 if let Some(ref pending_images) = self.pending_images {
                     let image = PendingImage {
                         state: PendingImageState::PendingResponse,
                         node: node.to_untrusted_node_address(),
-                        id: id,
+                        id,
                         origin: self.origin.clone(),
                     };
                     pending_images.lock().unwrap().push(image);
                 }
                 None
             },
+            // Not yet requested - request image or metadata from the cache
+            ImageCacheResult::ReadyForRequest(id) => {
+                let image = PendingImage {
+                    state: PendingImageState::Unrequested(url),
+                    node: node.to_untrusted_node_address(),
+                    id,
+                    origin: self.origin.clone(),
+                };
+                self.pending_images
+                    .as_ref()
+                    .unwrap()
+                    .lock()
+                    .unwrap()
+                    .push(image);
+                None
+            },
+            // Image failed to load, so just return nothing
+            ImageCacheResult::LoadError => None,
         }
     }
 

components/layout_2020/context.rs

@@ -3,11 +3,12 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 use crate::display_list::WebRenderImageInfo;
+use crate::opaque_node::OpaqueNodeMethods;
 use fnv::FnvHashMap;
 use gfx::font_cache_thread::FontCacheThread;
 use gfx::font_context::FontContext;
 use msg::constellation_msg::PipelineId;
-use net_traits::image_cache::{CanRequestImages, ImageCache, ImageState};
+use net_traits::image_cache::{CanRequestImages, ImageCache, ImageCacheResult};
 use net_traits::image_cache::{ImageOrMetadataAvailable, UsePlaceholder};
 use parking_lot::RwLock;
 use script_layout_interface::{PendingImage, PendingImageState};
@@ -70,51 +71,52 @@ impl<'a> LayoutContext<'a> {
             CanRequestImages::No
         };
 
-        // See if the image is already available
-        let result = self.image_cache.find_image_or_metadata(
+        // Check for available image or start tracking.
+        let cache_result = self.image_cache.get_cached_image_status(
             url.clone(),
             self.origin.clone(),
             None,
             use_placeholder,
             can_request,
         );
-        match result {
-            Ok(image_or_metadata) => Some(image_or_metadata),
-            // Image failed to load, so just return nothing
-            Err(ImageState::LoadError) => None,
-            // Not yet requested - request image or metadata from the cache
-            Err(ImageState::NotRequested(id)) => {
-                let image = PendingImage {
-                    state: PendingImageState::Unrequested(url),
-                    node: node.into(),
-                    id: id,
-                    origin: self.origin.clone(),
-                };
-                self.pending_images
-                    .as_ref()
-                    .unwrap()
-                    .lock()
-                    .unwrap()
-                    .push(image);
-                None
-            },
+
+        match cache_result {
+            ImageCacheResult::Available(img_or_meta) => Some(img_or_meta),
             // 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.
-            Err(ImageState::Pending(id)) => {
+            ImageCacheResult::Pending(id) => {
                 //XXXjdm if self.pending_images is not available, we should make sure that
                 //       this node gets marked dirty again so it gets a script-initiated
                 //       reflow that deals with this properly.
                 if let Some(ref pending_images) = self.pending_images {
                     let image = PendingImage {
                         state: PendingImageState::PendingResponse,
-                        node: node.into(),
-                        id: id,
+                        node: node.to_untrusted_node_address(),
+                        id,
                         origin: self.origin.clone(),
                     };
                     pending_images.lock().unwrap().push(image);
                 }
                 None
             },
+            // Not yet requested - request image or metadata from the cache
+            ImageCacheResult::ReadyForRequest(id) => {
+                let image = PendingImage {
+                    state: PendingImageState::Unrequested(url),
+                    node: node.to_untrusted_node_address(),
+                    id,
+                    origin: self.origin.clone(),
+                };
+                self.pending_images
+                    .as_ref()
+                    .unwrap()
+                    .lock()
+                    .unwrap()
+                    .push(image);
+                None
+            },
+            // Image failed to load, so just return nothing
+            ImageCacheResult::LoadError => None,
         }
     }
 

components/net/image_cache.rs

@@ -4,8 +4,12 @@
 
 use embedder_traits::resources::{self, Resource};
 use immeta::load_from_buf;
+use ipc_channel::ipc::IpcSender;
 use net_traits::image::base::{load_from_memory, Image, ImageMetadata};
-use net_traits::image_cache::{CanRequestImages, CorsStatus, ImageCache, ImageResponder};
+use net_traits::image_cache::{
+    CanRequestImages, CorsStatus, ImageCache, ImageCacheResult, ImageResponder,
+    PendingImageResponse,
+};
 use net_traits::image_cache::{ImageOrMetadataAvailable, ImageResponse, ImageState};
 use net_traits::image_cache::{PendingImageId, UsePlaceholder};
 use net_traits::request::CorsSettings;
@@ -16,7 +20,6 @@ use pixels::PixelFormat;
 use servo_url::{ImmutableOrigin, ServoUrl};
 use std::collections::hash_map::Entry::{Occupied, Vacant};
 use std::collections::HashMap;
-use std::io;
 use std::mem;
 use std::sync::{Arc, Mutex};
 use std::thread;
@@ -45,13 +48,10 @@ fn decode_bytes_sync(key: LoadKey, bytes: &[u8], cors: CorsStatus) -> DecoderMsg
     }
 }
 
-fn get_placeholder_image(
-    webrender_api: &WebrenderIpcSender,
-    data: &[u8],
-) -> io::Result<Arc<Image>> {
+fn get_placeholder_image(webrender_api: &WebrenderIpcSender, data: &[u8]) -> Arc<Image> {
     let mut image = load_from_memory(&data, CorsStatus::Unsafe).unwrap();
     set_webrender_image_key(webrender_api, &mut image);
-    Ok(Arc::new(image))
+    Arc::new(image)
 }
 
 fn set_webrender_image_key(webrender_api: &WebrenderIpcSender, image: &mut Image) {
@@ -335,7 +335,7 @@ struct ImageCacheStore {
     completed_loads: HashMap<ImageKey, CompletedLoad>,
 
     // The placeholder image used when an image fails to load
-    placeholder_image: Option<Arc<Image>>,
+    placeholder_image: Arc<Image>,
 
     // The URL used for the placeholder image
     placeholder_url: ServoUrl,
@@ -436,34 +436,45 @@ impl ImageCache for ImageCacheImpl {
             store: Arc::new(Mutex::new(ImageCacheStore {
                 pending_loads: AllPendingLoads::new(),
                 completed_loads: HashMap::new(),
-                placeholder_image: get_placeholder_image(&webrender_api, &rippy_data).ok(),
+                placeholder_image: get_placeholder_image(&webrender_api, &rippy_data),
                 placeholder_url: ServoUrl::parse("chrome://resources/rippy.png").unwrap(),
                 webrender_api: webrender_api,
             })),
         }
     }
 
-    /// Return any available metadata or image for the given URL,
-    /// or an indication that the image is not yet available if it is in progress,
-    /// or else reserve a slot in the cache for the URL if the consumer can request images.
-    fn find_image_or_metadata(
+    fn get_image(
+        &self,
+        url: ServoUrl,
+        origin: ImmutableOrigin,
+        cors_setting: Option<CorsSettings>,
+    ) -> Option<Arc<Image>> {
+        let store = self.store.lock().unwrap();
+        let result =
+            store.get_completed_image_if_available(url, origin, cors_setting, UsePlaceholder::No);
+        match result {
+            Some(Ok(ImageOrMetadataAvailable::ImageAvailable(img, _))) => Some(img),
+            _ => None,
+        }
+    }
+
+    fn get_cached_image_status(
         &self,
         url: ServoUrl,
         origin: ImmutableOrigin,
         cors_setting: Option<CorsSettings>,
         use_placeholder: UsePlaceholder,
         can_request: CanRequestImages,
-    ) -> Result<ImageOrMetadataAvailable, ImageState> {
-        debug!("Find image or metadata for {} ({:?})", url, origin);
+    ) -> ImageCacheResult {
         let mut store = self.store.lock().unwrap();
-        if let Some(result) = store.get_completed_image_if_available(
+        if let Some(Ok(result)) = store.get_completed_image_if_available(
             url.clone(),
             origin.clone(),
             cors_setting,
             use_placeholder,
         ) {
             debug!("{} is available", url);
-            return result;
+            return ImageCacheResult::Available(result);
         }
 
         let decoded = {
@@ -481,20 +492,22 @@ impl ImageCache for ImageCacheImpl {
                     },
                     (&None, &Some(ref meta)) => {
                         debug!("Metadata available for {} ({:?})", url, key);
-                        return Ok(ImageOrMetadataAvailable::MetadataAvailable(meta.clone()));
+                        return ImageCacheResult::Available(
+                            ImageOrMetadataAvailable::MetadataAvailable(meta.clone()),
+                        );
                     },
                     (&Some(Err(_)), _) | (&None, &None) => {
                         debug!("{} ({:?}) is still pending", url, key);
-                        return Err(ImageState::Pending(key));
+                        return ImageCacheResult::Pending(key);
                     },
                 },
                 CacheResult::Miss(Some((key, _pl))) => {
                     debug!("Should be requesting {} ({:?})", url, key);
-                    return Err(ImageState::NotRequested(key));
+                    return ImageCacheResult::ReadyForRequest(key);
                 },
                 CacheResult::Miss(None) => {
                     debug!("Couldn't find an entry for {}", url);
-                    return Err(ImageState::LoadError);
+                    return ImageCacheResult::LoadError;
                 },
             }
         };
@@ -505,11 +518,48 @@ impl ImageCache for ImageCacheImpl {
         // TODO: make this behaviour configurable according to the caller's needs.
         store.handle_decoder(decoded);
         match store.get_completed_image_if_available(url, origin, cors_setting, use_placeholder) {
-            Some(result) => result,
-            None => Err(ImageState::LoadError),
+            Some(Ok(result)) => ImageCacheResult::Available(result),
+            _ => ImageCacheResult::LoadError,
         }
     }
 
+    fn track_image(
+        &self,
+        url: ServoUrl,
+        origin: ImmutableOrigin,
+        cors_setting: Option<CorsSettings>,
+        sender: IpcSender<PendingImageResponse>,
+        use_placeholder: UsePlaceholder,
+        can_request: CanRequestImages,
+    ) -> ImageCacheResult {
+        debug!("Track image for {} ({:?})", url, origin);
+        let cache_result = self.get_cached_image_status(
+            url.clone(),
+            origin.clone(),
+            cors_setting,
+            use_placeholder,
+            can_request,
+        );
+
+        match cache_result {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(_)) => {
+                let store = self.store.lock().unwrap();
+                let id = store
+                    .pending_loads
+                    .url_to_load_key
+                    .get(&(url, origin, cors_setting))
+                    .unwrap();
+                self.add_listener(*id, ImageResponder::new(sender, *id));
+            },
+            ImageCacheResult::Pending(id) | ImageCacheResult::ReadyForRequest(id) => {
+                self.add_listener(id, ImageResponder::new(sender, id));
+            },
+            _ => {},
+        }
+
+        cache_result
+    }
+
     /// Add a new listener for the given pending image id. If the image is already present,
     /// the responder will still receive the expected response.
     fn add_listener(&self, id: PendingImageId, listener: ImageResponder) {
@@ -600,13 +650,8 @@ impl ImageCache for ImageCacheImpl {
                     Err(_) => {
                         debug!("Processing error for {:?}", key);
                         let mut store = self.store.lock().unwrap();
-                        match store.placeholder_image.clone() {
-                            Some(placeholder_image) => store.complete_load(
-                                id,
-                                LoadResult::PlaceholderLoaded(placeholder_image),
-                            ),
-                            None => store.complete_load(id, LoadResult::None),
-                        }
+                        let placeholder_image = store.placeholder_image.clone();
+                        store.complete_load(id, LoadResult::PlaceholderLoaded(placeholder_image))
                     },
                 }
             },