Auto merge of #23661 - julientregoat:i-21289, r=jdm

Refactor ImageCache::find_image_or_metadata -> ImageCache::{get_image, track_image}

<!-- Please describe your changes on the following line: -->
Updated the `ImageCache` trait to replace `find_image_or_metadata` with two new functions `track_image` and `get_image`, as well as a new enum (`ImageCacheResult`).

As a result, I was able to refactor the functions that previously called `find_image_or_metadata` pretty cleanly. For a list of these functions, please see the commit information.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #21289  (GitHub issue number if applicable)

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because tests already exist for these components. I ran `cargo test` in `net`, `net_traits`, `layout`, and `script` successfully.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23661)
<!-- Reviewable:end -->

9 files changed, 280 insertions, 240 deletions

diff --git a/components/layout/context.rs b/components/layout/context.rs
index 1374ba175f9..f98f8bc8395 100644
--- a/components/layout/context.rs
+++ b/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,37 +122,20 @@ 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.
@@ -160,13 +143,31 @@ impl<'a> LayoutContext<'a> {
                     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,
         }
     }
 
diff --git a/components/layout_2020/context.rs b/components/layout_2020/context.rs
index dc894e65766..117a02e0940 100644
--- a/components/layout_2020/context.rs
+++ b/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,
         }
     }
 
diff --git a/components/net/image_cache.rs b/components/net/image_cache.rs
index b5f0c98d49d..fea75ee7b95 100644
--- a/components/net/image_cache.rs
+++ b/components/net/image_cache.rs
@@ -4,9 +4,13 @@
 
 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::{ImageOrMetadataAvailable, ImageResponse, ImageState};
+use net_traits::image_cache::{
+    CanRequestImages, CorsStatus, ImageCache, ImageCacheResult, ImageResponder,
+    PendingImageResponse,
+};
+use net_traits::image_cache::{ImageOrMetadataAvailable, ImageResponse};
 use net_traits::image_cache::{PendingImageId, UsePlaceholder};
 use net_traits::request::CorsSettings;
 use net_traits::{
@@ -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,
@@ -391,7 +391,7 @@ impl ImageCacheStore {
         origin: ImmutableOrigin,
         cors_setting: Option<CorsSettings>,
         placeholder: UsePlaceholder,
-    ) -> Option<Result<ImageOrMetadataAvailable, ImageState>> {
+    ) -> Option<Result<(Arc<Image>, ServoUrl), ()>> {
         self.completed_loads
             .get(&(url, origin, cors_setting))
             .map(
@@ -400,13 +400,10 @@ impl ImageCacheStore {
                     (
                         &ImageResponse::PlaceholderLoaded(ref image, ref url),
                         UsePlaceholder::Yes,
-                    ) => Ok(ImageOrMetadataAvailable::ImageAvailable(
-                        image.clone(),
-                        url.clone(),
-                    )),
+                    ) => Ok((image.clone(), url.clone())),
                     (&ImageResponse::PlaceholderLoaded(_, _), UsePlaceholder::No) |
                     (&ImageResponse::None, _) |
-                    (&ImageResponse::MetadataLoaded(_), _) => Err(ImageState::LoadError),
+                    (&ImageResponse::MetadataLoaded(_), _) => Err(()),
                 },
             )
     }
@@ -436,25 +433,36 @@ 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((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(
             url.clone(),
@@ -462,8 +470,18 @@ impl ImageCache for ImageCacheImpl {
             cors_setting,
             use_placeholder,
         ) {
-            debug!("{} is available", url);
-            return result;
+            match result {
+                Ok((image, image_url)) => {
+                    debug!("{} is available", url);
+                    return ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(
+                        image, image_url,
+                    ));
+                },
+                Err(()) => {
+                    debug!("{} is not available", url);
+                    return ImageCacheResult::LoadError;
+                },
+            }
         }
 
         let decoded = {
@@ -481,20 +499,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,9 +525,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((image, image_url))) => ImageCacheResult::Available(
+                ImageOrMetadataAvailable::ImageAvailable(image, image_url),
+            ),
+            _ => 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,
@@ -600,13 +659,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))
                     },
                 }
             },
diff --git a/components/net_traits/image_cache.rs b/components/net_traits/image_cache.rs
index e640ca5b292..9c5ca0b1455 100644
--- a/components/net_traits/image_cache.rs
+++ b/components/net_traits/image_cache.rs
@@ -73,14 +73,6 @@ pub enum ImageResponse {
     None,
 }
 
-/// The current state of an image in the cache.
-#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
-pub enum ImageState {
-    Pending(PendingImageId),
-    LoadError,
-    NotRequested(PendingImageId),
-}
-
 /// The unique id for an image that has previously been requested.
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
 pub struct PendingImageId(pub u64);
@@ -101,22 +93,50 @@ pub enum UsePlaceholder {
 // ImageCache public API.
 // ======================================================================
 
+pub enum ImageCacheResult {
+    Available(ImageOrMetadataAvailable),
+    LoadError,
+    Pending(PendingImageId),
+    ReadyForRequest(PendingImageId),
+}
+
 pub trait ImageCache: Sync + Send {
     fn new(webrender_api: WebrenderIpcSender) -> Self
     where
         Self: Sized;
 
-    /// 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(
+    /// Definitively check whether there is a cached, fully loaded image available.
+    fn get_image(
+        &self,
+        url: ServoUrl,
+        origin: ImmutableOrigin,
+        cors_setting: Option<CorsSettings>,
+    ) -> Option<Arc<Image>>;
+
+    fn get_cached_image_status(
+        &self,
+        url: ServoUrl,
+        origin: ImmutableOrigin,
+        cors_setting: Option<CorsSettings>,
+        use_placeholder: UsePlaceholder,
+        can_request: CanRequestImages,
+    ) -> ImageCacheResult;
+
+    /// Add a listener for the provided pending image id, eventually called by
+    /// ImageCacheStore::complete_load.
+    /// If only metadata is available, Available(ImageOrMetadataAvailable) will
+    /// be returned.
+    /// If Available(ImageOrMetadataAvailable::Image) or LoadError is the final value,
+    /// the provided listener will be dropped (consumed & not added to PendingLoad).
+    fn track_image(
         &self,
         url: ServoUrl,
         origin: ImmutableOrigin,
         cors_setting: Option<CorsSettings>,
+        sender: IpcSender<PendingImageResponse>,
         use_placeholder: UsePlaceholder,
         can_request: CanRequestImages,
-    ) -> Result<ImageOrMetadataAvailable, ImageState>;
+    ) -> ImageCacheResult;
 
     /// Add a new listener for the given pending image id. If the image is already present,
     /// the responder will still receive the expected response.
diff --git a/components/script/canvas_state.rs b/components/script/canvas_state.rs
index d0dca9b2d15..2376d6f8d85 100644
--- a/components/script/canvas_state.rs
+++ b/components/script/canvas_state.rs
@@ -38,12 +38,7 @@ use euclid::{
     vec2,
 };
 use ipc_channel::ipc::{self, IpcSender};
-use net_traits::image_cache::CanRequestImages;
-use net_traits::image_cache::ImageCache;
-use net_traits::image_cache::ImageOrMetadataAvailable;
-use net_traits::image_cache::ImageResponse;
-use net_traits::image_cache::ImageState;
-use net_traits::image_cache::UsePlaceholder;
+use net_traits::image_cache::{ImageCache, ImageResponse};
 use net_traits::request::CorsSettings;
 use pixels::PixelFormat;
 use profile_traits::ipc as profiled_ipc;
@@ -261,19 +256,12 @@ impl CanvasState {
         url: ServoUrl,
         cors_setting: Option<CorsSettings>,
     ) -> ImageResponse {
-        let response = self.image_cache.find_image_or_metadata(
-            url.clone(),
-            self.origin.clone(),
-            cors_setting,
-            UsePlaceholder::No,
-            CanRequestImages::No,
-        );
-        match response {
-            Ok(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
-                ImageResponse::Loaded(image, url)
-            },
-            Err(ImageState::Pending(_)) => ImageResponse::None,
-            _ => {
+        match self
+            .image_cache
+            .get_image(url.clone(), self.origin.clone(), cors_setting)
+        {
+            Some(image) => ImageResponse::Loaded(image, url),
+            None => {
                 // Rather annoyingly, we get the same response back from
                 // A load which really failed and from a load which hasn't started yet.
                 self.missing_image_urls.borrow_mut().push(url);
diff --git a/components/script/dom/htmlcanvaselement.rs b/components/script/dom/htmlcanvaselement.rs
index d5327e27dea..9e04585168c 100644
--- a/components/script/dom/htmlcanvaselement.rs
+++ b/components/script/dom/htmlcanvaselement.rs
@@ -432,8 +432,7 @@ impl<'a> From<&'a WebGLContextAttributes> for GLContextAttributes {
 
 pub mod utils {
     use crate::dom::window::Window;
-    use net_traits::image_cache::CanRequestImages;
-    use net_traits::image_cache::{ImageOrMetadataAvailable, ImageResponse, UsePlaceholder};
+    use net_traits::image_cache::ImageResponse;
     use net_traits::request::CorsSettings;
     use servo_url::ServoUrl;
 
@@ -443,18 +442,15 @@ pub mod utils {
         cors_setting: Option<CorsSettings>,
     ) -> ImageResponse {
         let image_cache = window.image_cache();
-        let response = image_cache.find_image_or_metadata(
-            url.into(),
+        let result = image_cache.get_image(
+            url.clone(),
             window.origin().immutable().clone(),
             cors_setting,
-            UsePlaceholder::No,
-            CanRequestImages::No,
         );
-        match response {
-            Ok(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
-                ImageResponse::Loaded(image, url)
-            },
-            _ => ImageResponse::None,
+
+        match result {
+            Some(image) => ImageResponse::Loaded(image, url),
+            None => ImageResponse::None,
         }
     }
 }
diff --git a/components/script/dom/htmlimageelement.rs b/components/script/dom/htmlimageelement.rs
index b5f20784c8e..052b622bb9f 100644
--- a/components/script/dom/htmlimageelement.rs
+++ b/components/script/dom/htmlimageelement.rs
@@ -43,7 +43,7 @@ use crate::dom::values::UNSIGNED_LONG_MAX;
 use crate::dom::virtualmethods::VirtualMethods;
 use crate::dom::window::Window;
 use crate::fetch::create_a_potential_cors_request;
-use crate::image_listener::{add_cache_listener_for_element, ImageCacheListener};
+use crate::image_listener::{generate_cache_listener_for_element, ImageCacheListener};
 use crate::microtask::{Microtask, MicrotaskRunnable};
 use crate::network_listener::{self, NetworkListener, PreInvoke, ResourceTimingListener};
 use crate::script_thread::ScriptThread;
@@ -54,13 +54,15 @@ use dom_struct::dom_struct;
 use euclid::Point2D;
 use html5ever::{LocalName, Prefix, QualName};
 use ipc_channel::ipc;
+use ipc_channel::ipc::IpcSender;
 use ipc_channel::router::ROUTER;
 use mime::{self, Mime};
 use msg::constellation_msg::PipelineId;
 use net_traits::image::base::{Image, ImageMetadata};
-use net_traits::image_cache::UsePlaceholder;
-use net_traits::image_cache::{CanRequestImages, CorsStatus, ImageCache, ImageOrMetadataAvailable};
-use net_traits::image_cache::{ImageResponder, ImageResponse, ImageState, PendingImageId};
+use net_traits::image_cache::{
+    CanRequestImages, CorsStatus, ImageCache, ImageCacheResult, ImageOrMetadataAvailable,
+    ImageResponse, PendingImageId, PendingImageResponse, UsePlaceholder,
+};
 use net_traits::request::{CorsSettings, Destination, Initiator, RequestBuilder};
 use net_traits::{FetchMetadata, FetchResponseListener, FetchResponseMsg, NetworkError};
 use net_traits::{ReferrerPolicy, ResourceFetchTiming, ResourceTimingType};
@@ -317,35 +319,27 @@ impl HTMLImageElement {
     fn fetch_image(&self, img_url: &ServoUrl) {
         let window = window_from_node(self);
         let image_cache = window.image_cache();
-        let response = image_cache.find_image_or_metadata(
-            img_url.clone().into(),
+        let sender = generate_cache_listener_for_element(self);
+        let cache_result = image_cache.track_image(
+            img_url.clone(),
             window.origin().immutable().clone(),
             cors_setting_for_element(self.upcast()),
+            sender,
             UsePlaceholder::Yes,
             CanRequestImages::Yes,
         );
-        match response {
-            Ok(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
-                self.process_image_response(ImageResponse::Loaded(image, url));
-            },
 
-            Ok(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
-                self.process_image_response(ImageResponse::MetadataLoaded(m));
+        match cache_result {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
+                self.process_image_response(ImageResponse::Loaded(image, url))
             },
-
-            Err(ImageState::Pending(id)) => {
-                add_cache_listener_for_element(image_cache, id, self);
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
+                self.process_image_response(ImageResponse::MetadataLoaded(m))
             },
-
-            Err(ImageState::LoadError) => {
-                self.process_image_response(ImageResponse::None);
-            },
-
-            Err(ImageState::NotRequested(id)) => {
-                add_cache_listener_for_element(image_cache, id, self);
-                self.fetch_request(img_url, id);
-            },
-        }
+            ImageCacheResult::Pending(_) => (),
+            ImageCacheResult::ReadyForRequest(id) => self.fetch_request(img_url, id),
+            ImageCacheResult::LoadError => self.process_image_response(ImageResponse::None),
+        };
     }
 
     fn fetch_request(&self, img_url: &ServoUrl, id: PendingImageId) {
@@ -943,14 +937,13 @@ impl HTMLImageElement {
         if let Some(src) = selected_source {
             if let Ok(img_url) = base_url.join(&src) {
                 let image_cache = window.image_cache();
-                let response = image_cache.find_image_or_metadata(
-                    img_url.clone().into(),
+                let response = image_cache.get_image(
+                    img_url.clone(),
                     window.origin().immutable().clone(),
                     cors_setting_for_element(self.upcast()),
-                    UsePlaceholder::No,
-                    CanRequestImages::No,
                 );
-                if let Ok(ImageOrMetadataAvailable::ImageAvailable(image, url)) = response {
+
+                if let Some(image) = response {
                     // Cancel any outstanding tasks that were queued before the src was
                     // set on this element.
                     self.generation.set(self.generation.get() + 1);
@@ -963,7 +956,7 @@ impl HTMLImageElement {
                     self.abort_request(State::CompletelyAvailable, ImageRequestPhase::Current);
                     self.abort_request(State::Unavailable, ImageRequestPhase::Pending);
                     let mut current_request = self.current_request.borrow_mut();
-                    current_request.final_url = Some(url);
+                    current_request.final_url = Some(img_url.clone());
                     current_request.image = Some(image.clone());
                     current_request.metadata = Some(metadata);
                     // Step 6.3.6
@@ -1010,13 +1003,12 @@ impl HTMLImageElement {
     /// Step 2-12 of https://html.spec.whatwg.org/multipage/#img-environment-changes
     fn react_to_environment_changes_sync_steps(&self, generation: u32) {
         // TODO reduce duplicacy of this code
-        fn add_cache_listener_for_element(
-            image_cache: Arc<dyn ImageCache>,
-            id: PendingImageId,
+
+        fn generate_cache_listener_for_element(
             elem: &HTMLImageElement,
             selected_source: String,
             selected_pixel_density: f64,
-        ) {
+        ) -> IpcSender<PendingImageResponse> {
             let trusted_node = Trusted::new(elem);
             let (responder_sender, responder_receiver) = ipc::channel().unwrap();
 
@@ -1025,27 +1017,30 @@ impl HTMLImageElement {
                 .task_manager()
                 .networking_task_source_with_canceller();
             let generation = elem.generation.get();
-            ROUTER.add_route(responder_receiver.to_opaque(), Box::new(move |message| {
-                debug!("Got image {:?}", message);
-                // Return the image via a message to the script thread, which marks
-                // the element as dirty and triggers a reflow.
-                let element = trusted_node.clone();
-                let image = message.to().unwrap();
-                let selected_source_clone = selected_source.clone();
-                let _ = task_source.queue_with_canceller(
-                    task!(process_image_response_for_environment_change: move || {
-                        let element = element.root();
-                        // Ignore any image response for a previous request that has been discarded.
-                        if generation == element.generation.get() {
-                            element.process_image_response_for_environment_change(image,
-                                USVString::from(selected_source_clone), generation, selected_pixel_density);
-                        }
-                    }),
-                    &canceller,
-                );
-            }));
+            ROUTER.add_route(
+                responder_receiver.to_opaque(),
+                Box::new(move |message| {
+                    debug!("Got image {:?}", message);
+                    // Return the image via a message to the script thread, which marks
+                    // the element as dirty and triggers a reflow.
+                    let element = trusted_node.clone();
+                    let image = message.to().unwrap();
+                    let selected_source_clone = selected_source.clone();
+                    let _ = task_source.queue_with_canceller(
+                        task!(process_image_response_for_environment_change: move || {
+                            let element = element.root();
+                            // Ignore any image response for a previous request that has been discarded.
+                            if generation == element.generation.get() {
+                                element.process_image_response_for_environment_change(image,
+                                    USVString::from(selected_source_clone), generation, selected_pixel_density);
+                            }
+                        }),
+                        &canceller,
+                    );
+                }),
+            );
 
-            image_cache.add_listener(id, ImageResponder::new(responder_sender, id));
+            responder_sender
         }
 
         let elem = self.upcast::<Element>();
@@ -1099,25 +1094,32 @@ impl HTMLImageElement {
 
         let window = window_from_node(self);
         let image_cache = window.image_cache();
+
         // Step 14
-        let response = image_cache.find_image_or_metadata(
-            img_url.clone().into(),
+        let sender = generate_cache_listener_for_element(
+            self,
+            selected_source.0.clone(),
+            selected_pixel_density,
+        );
+        let cache_result = image_cache.track_image(
+            img_url.clone(),
             window.origin().immutable().clone(),
             cors_setting_for_element(self.upcast()),
+            sender,
             UsePlaceholder::No,
             CanRequestImages::Yes,
         );
-        match response {
-            Ok(ImageOrMetadataAvailable::ImageAvailable(_image, _url)) => {
+
+        match cache_result {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(_image, _url)) => {
                 // Step 15
                 self.finish_reacting_to_environment_change(
                     selected_source,
                     generation,
                     selected_pixel_density,
-                );
+                )
             },
-
-            Ok(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
                 self.process_image_response_for_environment_change(
                     ImageResponse::MetadataLoaded(m),
                     selected_source,
@@ -1125,18 +1127,7 @@ impl HTMLImageElement {
                     selected_pixel_density,
                 );
             },
-
-            Err(ImageState::Pending(id)) => {
-                add_cache_listener_for_element(
-                    image_cache.clone(),
-                    id,
-                    self,
-                    selected_source.0,
-                    selected_pixel_density,
-                );
-            },
-
-            Err(ImageState::LoadError) => {
+            ImageCacheResult::LoadError => {
                 self.process_image_response_for_environment_change(
                     ImageResponse::None,
                     selected_source,
@@ -1144,17 +1135,8 @@ impl HTMLImageElement {
                     selected_pixel_density,
                 );
             },
-
-            Err(ImageState::NotRequested(id)) => {
-                add_cache_listener_for_element(
-                    image_cache,
-                    id,
-                    self,
-                    selected_source.0,
-                    selected_pixel_density,
-                );
-                self.fetch_request(&img_url, id);
-            },
+            ImageCacheResult::ReadyForRequest(id) => self.fetch_request(&img_url, id),
+            ImageCacheResult::Pending(_) => (),
         }
     }
 
diff --git a/components/script/dom/htmlvideoelement.rs b/components/script/dom/htmlvideoelement.rs
index f83c05bbe0a..a890b49af72 100644
--- a/components/script/dom/htmlvideoelement.rs
+++ b/components/script/dom/htmlvideoelement.rs
@@ -19,16 +19,17 @@ use crate::dom::node::{document_from_node, window_from_node, Node};
 use crate::dom::performanceresourcetiming::InitiatorType;
 use crate::dom::virtualmethods::VirtualMethods;
 use crate::fetch::FetchCanceller;
-use crate::image_listener::{add_cache_listener_for_element, ImageCacheListener};
+use crate::image_listener::{generate_cache_listener_for_element, ImageCacheListener};
 use crate::network_listener::{self, NetworkListener, PreInvoke, ResourceTimingListener};
 use dom_struct::dom_struct;
 use euclid::default::Size2D;
 use html5ever::{LocalName, Prefix};
 use ipc_channel::ipc;
 use ipc_channel::router::ROUTER;
-use net_traits::image_cache::UsePlaceholder;
-use net_traits::image_cache::{CanRequestImages, ImageCache, ImageOrMetadataAvailable};
-use net_traits::image_cache::{ImageResponse, ImageState, PendingImageId};
+use net_traits::image_cache::{
+    CanRequestImages, ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponse,
+    PendingImageId, UsePlaceholder,
+};
 use net_traits::request::{CredentialsMode, Destination, RequestBuilder};
 use net_traits::{
     CoreResourceMsg, FetchChannels, FetchMetadata, FetchResponseListener, FetchResponseMsg,
@@ -155,27 +156,23 @@ impl HTMLVideoElement {
         // network activity as possible.
         let window = window_from_node(self);
         let image_cache = window.image_cache();
-        let response = image_cache.find_image_or_metadata(
-            poster_url.clone().into(),
+        let sender = generate_cache_listener_for_element(self);
+        let cache_result = image_cache.track_image(
+            poster_url.clone(),
             window.origin().immutable().clone(),
             None,
+            sender,
             UsePlaceholder::No,
             CanRequestImages::Yes,
         );
-        match response {
-            Ok(ImageOrMetadataAvailable::ImageAvailable(image, url)) => {
-                self.process_image_response(ImageResponse::Loaded(image, url));
-            },
 
-            Err(ImageState::Pending(id)) => {
-                add_cache_listener_for_element(image_cache, id, self);
+        match cache_result {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable(img, url)) => {
+                self.process_image_response(ImageResponse::Loaded(img, url));
             },
-
-            Err(ImageState::NotRequested(id)) => {
-                add_cache_listener_for_element(image_cache, id, self);
-                self.do_fetch_poster_frame(poster_url, id, cancel_receiver);
+            ImageCacheResult::ReadyForRequest(id) => {
+                self.do_fetch_poster_frame(poster_url, id, cancel_receiver)
             },
-
             _ => (),
         }
     }
diff --git a/components/script/image_listener.rs b/components/script/image_listener.rs
index fffaf3acc03..7bf81c80d76 100644
--- a/components/script/image_listener.rs
+++ b/components/script/image_listener.rs
@@ -8,20 +8,20 @@ use crate::dom::bindings::reflector::DomObject;
 use crate::dom::node::{window_from_node, Node};
 use crate::task_source::TaskSource;
 use ipc_channel::ipc;
+use ipc_channel::ipc::IpcSender;
 use ipc_channel::router::ROUTER;
-use net_traits::image_cache::{ImageCache, ImageResponder, ImageResponse, PendingImageId};
-use std::sync::Arc;
+use net_traits::image_cache::{ImageResponse, PendingImageResponse};
 
 pub trait ImageCacheListener {
     fn generation_id(&self) -> u32;
     fn process_image_response(&self, response: ImageResponse);
 }
 
-pub fn add_cache_listener_for_element<T: ImageCacheListener + DerivedFrom<Node> + DomObject>(
-    image_cache: Arc<dyn ImageCache>,
-    id: PendingImageId,
+pub fn generate_cache_listener_for_element<
+    T: ImageCacheListener + DerivedFrom<Node> + DomObject,
+>(
     elem: &T,
-) {
+) -> IpcSender<PendingImageResponse> {
     let trusted_node = Trusted::new(elem);
     let (responder_sender, responder_receiver) = ipc::channel().unwrap();
 
@@ -49,5 +49,5 @@ pub fn add_cache_listener_for_element<T: ImageCacheListener + DerivedFrom<Node>
         }),
     );
 
-    image_cache.add_listener(id, ImageResponder::new(responder_sender, id));
+    responder_sender
 }