script: Do not set up an IPC route for every image load (#35041)

Instead of setting up a route for every image load in the DOM / Layout,
route all incoming image cache responses through the `ScriptThread`.
This avoids creating a set of file descriptor for every image that is
loaded.

This change requires having the `ImageCache` track the `PipelineId` of
the original the listener so that the `ScriptThread` can route it
properly to the correct `Window`.

Signed-off-by: Martin Robinson <mrobinson@igalia.com>

Cargo.lock

@@ -6675,6 +6675,7 @@ dependencies = [
  "euclid",
  "http 1.2.0",
  "indexmap",
+ "ipc-channel",
  "keyboard-types",
  "malloc_size_of",
  "markup5ever",

components/layout/context.rs

@@ -137,7 +137,7 @@ impl LayoutContext<'_> {
                     Some(image_info)
                 }
             },
-            None | Some(ImageOrMetadataAvailable::MetadataAvailable(_)) => None,
+            None | Some(ImageOrMetadataAvailable::MetadataAvailable(..)) => None,
         }
     }
 }

components/layout/fragment.rs

@@ -440,8 +440,8 @@ impl ImageFragmentInfo {
                             ImageOrMetadataAvailable::ImageAvailable { image, .. } => {
                                 ImageOrMetadata::Image(image)
                             },
-                            ImageOrMetadataAvailable::MetadataAvailable(m) => {
+                            ImageOrMetadataAvailable::MetadataAvailable(metadata, _id) => {
-                                ImageOrMetadata::Metadata(m)
+                                ImageOrMetadata::Metadata(metadata)
                             },
                         })
                 })

components/layout_2020/context.rs

@@ -129,7 +129,7 @@ impl LayoutContext<'_> {
                     Some(image_info)
                 }
             },
-            None | Some(ImageOrMetadataAvailable::MetadataAvailable(_)) => None,
+            None | Some(ImageOrMetadataAvailable::MetadataAvailable(..)) => None,
         }
     }
 }

components/layout_2020/replaced.rs

@@ -217,7 +217,7 @@ impl ReplacedContents {
                 Some(ImageOrMetadataAvailable::ImageAvailable { image, .. }) => {
                     (Some(image.clone()), image.width as f32, image.height as f32)
                 },
-                Some(ImageOrMetadataAvailable::MetadataAvailable(metadata)) => {
+                Some(ImageOrMetadataAvailable::MetadataAvailable(metadata, _id)) => {
                     (None, metadata.width as f32, metadata.height as f32)
                 },
                 None => return None,

components/malloc_size_of/Cargo.toml

@@ -18,6 +18,7 @@ crossbeam-channel = { workspace = true }
 euclid = { workspace = true }
 http = { workspace = true }
 indexmap = { workspace = true }
+ipc-channel = { workspace = true }
 keyboard-types = { workspace = true }
 markup5ever = { workspace = true }
 serde = { workspace = true }

components/malloc_size_of/lib.rs

@@ -674,6 +674,12 @@ impl<T> MallocSizeOf for tokio::sync::mpsc::UnboundedSender<T> {
     }
 }
 
+impl<T> MallocSizeOf for ipc_channel::ipc::IpcSender<T> {
+    fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize {
+        0
+    }
+}
+
 impl<T: MallocSizeOf> MallocSizeOf for accountable_refcell::RefCell<T> {
     fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
         self.borrow().size_of(ops)

components/net/image_cache.rs

@@ -8,11 +8,11 @@ use std::sync::{Arc, Mutex};
 use std::{mem, thread};
 
 use imsz::imsz_from_reader;
-use ipc_channel::ipc::{IpcSender, IpcSharedMemory};
+use ipc_channel::ipc::IpcSharedMemory;
 use log::{debug, warn};
 use net_traits::image_cache::{
     ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponder, ImageResponse,
-    PendingImageId, PendingImageResponse, UsePlaceholder,
+    PendingImageId, UsePlaceholder,
 };
 use net_traits::request::CorsSettings;
 use net_traits::{FetchMetadata, FetchResponseMsg, FilteredMetadata, NetworkError};
@@ -497,7 +497,7 @@ impl ImageCache for ImageCacheImpl {
                     (&None, Some(meta)) => {
                         debug!("Metadata available for {} ({:?})", url, key);
                         return ImageCacheResult::Available(
-                            ImageOrMetadataAvailable::MetadataAvailable(meta.clone()),
+                            ImageOrMetadataAvailable::MetadataAvailable(meta.clone(), key),
                         );
                     },
                     (&Some(Err(_)), _) | (&None, &None) => {
@@ -534,46 +534,11 @@ impl ImageCache for ImageCacheImpl {
         }
     }
 
-    fn track_image(
-        &self,
-        url: ServoUrl,
-        origin: ImmutableOrigin,
-        cors_setting: Option<CorsSettings>,
-        sender: IpcSender<PendingImageResponse>,
-        use_placeholder: UsePlaceholder,
-    ) -> ImageCacheResult {
-        debug!("Track image for {} ({:?})", url, origin);
-        let cache_result = self.get_cached_image_status(
-            url.clone(),
-            origin.clone(),
-            cors_setting,
-            use_placeholder,
-        );
-
-        match cache_result {
-            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(_)) => {
-                let mut store = self.store.lock().unwrap();
-                let id = *store
-                    .pending_loads
-                    .url_to_load_key
-                    .get(&(url, origin, cors_setting))
-                    .unwrap();
-                self.add_listener_with_store(&mut store, 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) {
+    fn add_listener(&self, listener: ImageResponder) {
         let mut store = self.store.lock().unwrap();
-        self.add_listener_with_store(&mut store, id, listener);
+        self.add_listener_with_store(&mut store, listener);
     }
 
     /// Inform the image cache about a response for a pending request.
@@ -658,12 +623,8 @@ impl ImageCache for ImageCacheImpl {
 
 impl ImageCacheImpl {
     /// Require self.store.lock() before calling.
-    fn add_listener_with_store(
+    fn add_listener_with_store(&self, store: &mut ImageCacheStore, listener: ImageResponder) {
-        &self,
+        let id = listener.id;
-        store: &mut ImageCacheStore,
-        id: PendingImageId,
-        listener: ImageResponder,
-    ) {
         if let Some(load) = store.pending_loads.get_by_key_mut(&id) {
             if let Some(ref metadata) = load.metadata {
                 listener.respond(ImageResponse::MetadataLoaded(metadata.clone()));

components/script/dom/globalscope.rs

@@ -233,7 +233,6 @@ pub(crate) struct GlobalScope {
     inline_module_map: DomRefCell<HashMap<ScriptId, Rc<ModuleTree>>>,
 
     /// For providing instructions to an optional devtools server.
-    #[ignore_malloc_size_of = "channels are hard"]
     #[no_trace]
     devtools_chan: Option<IpcSender<ScriptToDevtoolsControlMsg>>,
 

components/script/dom/htmlimageelement.rs

@@ -15,16 +15,13 @@ use cssparser::{Parser, ParserInput};
 use dom_struct::dom_struct;
 use euclid::Point2D;
 use html5ever::{local_name, namespace_url, ns, LocalName, Prefix, QualName};
-use ipc_channel::ipc;
-use ipc_channel::ipc::IpcSender;
-use ipc_channel::router::ROUTER;
 use js::jsapi::JSAutoRealm;
 use js::rust::HandleObject;
 use mime::{self, Mime};
 use net_traits::http_status::HttpStatus;
 use net_traits::image_cache::{
-    ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponse, PendingImageId,
+    ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponder, ImageResponse,
-    PendingImageResponse, UsePlaceholder,
+    PendingImageId, UsePlaceholder,
 };
 use net_traits::request::{
     CorsSettings, Destination, Initiator, Referrer, RequestBuilder, RequestId,
@@ -89,7 +86,6 @@ 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::{generate_cache_listener_for_element, ImageCacheListener};
 use crate::microtask::{Microtask, MicrotaskRunnable};
 use crate::network_listener::{self, PreInvoke, ResourceTimingListener};
 use crate::realms::enter_realm;
@@ -355,13 +351,11 @@ impl HTMLImageElement {
     /// Update the current image with a valid URL.
     fn fetch_image(&self, img_url: &ServoUrl, can_gc: CanGc) {
         let window = self.owner_window();
-        let image_cache = window.image_cache();
+
-        let sender = generate_cache_listener_for_element(self);
+        let cache_result = window.image_cache().get_cached_image_status(
-        let cache_result = image_cache.track_image(
             img_url.clone(),
             window.origin().immutable().clone(),
             cors_setting_for_element(self.upcast()),
-            sender,
             UsePlaceholder::Yes,
         );
 
@@ -380,15 +374,63 @@ impl HTMLImageElement {
                     self.process_image_response(ImageResponse::Loaded(image, url), can_gc)
                 }
             },
-            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(
-                self.process_image_response(ImageResponse::MetadataLoaded(m), can_gc)
+                metadata,
+                id,
+            )) => {
+                self.process_image_response(ImageResponse::MetadataLoaded(metadata), can_gc);
+                self.register_image_cache_callback(id, ChangeType::Element);
+            },
+            ImageCacheResult::Pending(id) => {
+                self.register_image_cache_callback(id, ChangeType::Element);
+            },
+            ImageCacheResult::ReadyForRequest(id) => {
+                self.fetch_request(img_url, id);
+                self.register_image_cache_callback(id, ChangeType::Element);
             },
-            ImageCacheResult::Pending(_) => (),
-            ImageCacheResult::ReadyForRequest(id) => self.fetch_request(img_url, id),
             ImageCacheResult::LoadError => self.process_image_response(ImageResponse::None, can_gc),
         };
     }
 
+    fn register_image_cache_callback(&self, id: PendingImageId, change_type: ChangeType) {
+        let trusted_node = Trusted::new(self);
+        let generation = self.generation_id();
+        let window = self.owner_window();
+        let sender = window.register_image_cache_listener(id, move |response| {
+            let trusted_node = trusted_node.clone();
+            let window = trusted_node.root().owner_window();
+            let callback_type = change_type.clone();
+
+            window
+                .as_global_scope()
+                .task_manager()
+                .networking_task_source()
+                .queue(task!(process_image_response: move || {
+                let element = trusted_node.root();
+
+                // Ignore any image response for a previous request that has been discarded.
+                if generation != element.generation_id() {
+                    return;
+                }
+
+                match callback_type {
+                    ChangeType::Element => {
+                        element.process_image_response(response.response, CanGc::note());
+                    }
+                    ChangeType::Environment { selected_source, selected_pixel_density } => {
+                        element.process_image_response_for_environment_change(
+                            response.response, selected_source, generation, selected_pixel_density, CanGc::note()
+                        );
+                    }
+                }
+            }));
+        });
+
+        window
+            .image_cache()
+            .add_listener(ImageResponder::new(sender, window.pipeline_id(), id));
+    }
+
     fn fetch_request(&self, img_url: &ServoUrl, id: PendingImageId) {
         let document = self.owner_document();
         let window = self.owner_window();
@@ -1054,48 +1096,6 @@ 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, can_gc: CanGc) {
-        // TODO reduce duplicacy of this code
-
-        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();
-            let task_source = elem
-                .owner_global()
-                .task_manager()
-                .networking_task_source()
-                .to_sendable();
-            let generation = elem.generation.get();
-
-            ROUTER.add_typed_route(
-                responder_receiver,
-                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: PendingImageResponse = message.unwrap();
-                    let selected_source_clone = selected_source.clone();
-                    task_source.queue(
-                        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.response,
-                                    USVString::from(selected_source_clone), generation,
-                                    selected_pixel_density, CanGc::note());
-                            }
-                        })
-                    );
-                }),
-            );
-
-            responder_sender
-        }
-
         let elem = self.upcast::<Element>();
         let document = elem.owner_document();
         let has_pending_request = matches!(self.image_request.get(), ImageRequestPhase::Pending);
@@ -1143,23 +1143,20 @@ impl HTMLImageElement {
             can_gc,
         );
 
-        let window = self.owner_window();
-        let image_cache = window.image_cache();
-
         // Step 14
-        let sender = generate_cache_listener_for_element(
+        let window = self.owner_window();
-            self,
+        let cache_result = window.image_cache().get_cached_image_status(
-            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,
         );
 
+        let change_type = ChangeType::Environment {
+            selected_source: selected_source.clone(),
+            selected_pixel_density,
+        };
+
         match cache_result {
             ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable { .. }) => {
                 // Step 15
@@ -1169,7 +1166,7 @@ impl HTMLImageElement {
                     selected_pixel_density,
                 )
             },
-            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m)) => {
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(m, id)) => {
                 self.process_image_response_for_environment_change(
                     ImageResponse::MetadataLoaded(m),
                     selected_source,
@@ -1177,6 +1174,7 @@ impl HTMLImageElement {
                     selected_pixel_density,
                     can_gc,
                 );
+                self.register_image_cache_callback(id, change_type);
             },
             ImageCacheResult::LoadError => {
                 self.process_image_response_for_environment_change(
@@ -1187,8 +1185,13 @@ impl HTMLImageElement {
                     can_gc,
                 );
             },
-            ImageCacheResult::ReadyForRequest(id) => self.fetch_request(&img_url, id),
+            ImageCacheResult::ReadyForRequest(id) => {
-            ImageCacheResult::Pending(_) => (),
+                self.fetch_request(&img_url, id);
+                self.register_image_cache_callback(id, change_type);
+            },
+            ImageCacheResult::Pending(id) => {
+                self.register_image_cache_callback(id, change_type);
+            },
         }
     }
 
@@ -1383,6 +1386,10 @@ impl HTMLImageElement {
             .as_ref()
             .is_some_and(|url| url.scheme() == "data" || url.origin().same_origin(origin))
     }
+
+    fn generation_id(&self) -> u32 {
+        self.generation.get()
+    }
 }
 
 #[derive(JSTraceable, MallocSizeOf)]
@@ -1882,16 +1889,6 @@ impl FormControl for HTMLImageElement {
     }
 }
 
-impl ImageCacheListener for HTMLImageElement {
-    fn generation_id(&self) -> u32 {
-        self.generation.get()
-    }
-
-    fn process_image_response(&self, response: ImageResponse, can_gc: CanGc) {
-        self.process_image_response(response, can_gc);
-    }
-}
-
 fn image_dimension_setter(element: &Element, attr: LocalName, value: u32, can_gc: CanGc) {
     // This setter is a bit weird: the IDL type is unsigned long, but it's parsed as
     // a dimension for rendering.
@@ -2186,3 +2183,12 @@ pub fn parse_a_srcset_attribute(input: &str) -> Vec<ImageSource> {
     }
     candidates
 }
+
+#[derive(Clone)]
+enum ChangeType {
+    Environment {
+        selected_source: USVString,
+        selected_pixel_density: f64,
+    },
+    Element,
+}

components/script/dom/htmlvideoelement.rs

@@ -11,8 +11,8 @@ use html5ever::{local_name, namespace_url, ns, LocalName, Prefix};
 use ipc_channel::ipc;
 use js::rust::HandleObject;
 use net_traits::image_cache::{
-    ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponse, PendingImageId,
+    ImageCache, ImageCacheResult, ImageOrMetadataAvailable, ImageResponder, ImageResponse,
-    UsePlaceholder,
+    PendingImageId, UsePlaceholder,
 };
 use net_traits::request::{CredentialsMode, Destination, RequestBuilder, RequestId};
 use net_traits::{
@@ -41,7 +41,6 @@ use crate::dom::node::{Node, NodeTraits};
 use crate::dom::performanceresourcetiming::InitiatorType;
 use crate::dom::virtualmethods::VirtualMethods;
 use crate::fetch::FetchCanceller;
-use crate::image_listener::{generate_cache_listener_for_element, ImageCacheListener};
 use crate::network_listener::{self, PreInvoke, ResourceTimingListener};
 use crate::script_runtime::CanGc;
 
@@ -177,28 +176,47 @@ impl HTMLVideoElement {
         // network activity as possible.
         let window = self.owner_window();
         let image_cache = window.image_cache();
-        let sender = generate_cache_listener_for_element(self);
+        let cache_result = image_cache.get_cached_image_status(
-        let cache_result = image_cache.track_image(
             poster_url.clone(),
             window.origin().immutable().clone(),
             None,
-            sender,
             UsePlaceholder::No,
         );
 
-        match cache_result {
+        let id = match cache_result {
             ImageCacheResult::Available(ImageOrMetadataAvailable::ImageAvailable {
                 image,
                 url,
                 ..
             }) => {
                 self.process_image_response(ImageResponse::Loaded(image, url), can_gc);
+                return;
             },
+            ImageCacheResult::Available(ImageOrMetadataAvailable::MetadataAvailable(_, id)) => id,
             ImageCacheResult::ReadyForRequest(id) => {
                 self.do_fetch_poster_frame(poster_url, id, can_gc);
+                id
             },
-            _ => (),
+            ImageCacheResult::LoadError => {
+                self.process_image_response(ImageResponse::None, can_gc);
+                return;
+            },
+            ImageCacheResult::Pending(id) => id,
+        };
+
+        let trusted_node = Trusted::new(self);
+        let generation = self.generation_id();
+        let sender = window.register_image_cache_listener(id, move |response| {
+            let element = trusted_node.root();
+
+            // Ignore any image response for a previous request that has been discarded.
+            if generation != element.generation_id() {
+                return;
             }
+            element.process_image_response(response.response, CanGc::note());
+        });
+
+        image_cache.add_listener(ImageResponder::new(sender, window.pipeline_id(), id));
     }
 
     /// <https://html.spec.whatwg.org/multipage/#poster-frame>
@@ -228,6 +246,26 @@ impl HTMLVideoElement {
         let context = PosterFrameFetchContext::new(self, poster_url, id, request.id);
         self.owner_document().fetch_background(request, context);
     }
+
+    fn generation_id(&self) -> u32 {
+        self.generation_id.get()
+    }
+
+    fn process_image_response(&self, response: ImageResponse, can_gc: CanGc) {
+        match response {
+            ImageResponse::Loaded(image, url) => {
+                debug!("Loaded poster image for video element: {:?}", url);
+                self.htmlmediaelement.process_poster_image_loaded(image);
+                LoadBlocker::terminate(&self.load_blocker, can_gc);
+            },
+            ImageResponse::MetadataLoaded(..) => {},
+            // The image cache may have loaded a placeholder for an invalid poster url
+            ImageResponse::PlaceholderLoaded(..) | ImageResponse::None => {
+                // A failed load should unblock the document load.
+                LoadBlocker::terminate(&self.load_blocker, can_gc);
+            },
+        }
+    }
 }
 
 impl HTMLVideoElementMethods<crate::DomTypeHolder> for HTMLVideoElement {
@@ -289,28 +327,6 @@ impl VirtualMethods for HTMLVideoElement {
     }
 }
 
-impl ImageCacheListener for HTMLVideoElement {
-    fn generation_id(&self) -> u32 {
-        self.generation_id.get()
-    }
-
-    fn process_image_response(&self, response: ImageResponse, can_gc: CanGc) {
-        match response {
-            ImageResponse::Loaded(image, url) => {
-                debug!("Loaded poster image for video element: {:?}", url);
-                self.htmlmediaelement.process_poster_image_loaded(image);
-                LoadBlocker::terminate(&self.load_blocker, can_gc);
-            },
-            ImageResponse::MetadataLoaded(..) => {},
-            // The image cache may have loaded a placeholder for an invalid poster url
-            ImageResponse::PlaceholderLoaded(..) | ImageResponse::None => {
-                // A failed load should unblock the document load.
-                LoadBlocker::terminate(&self.load_blocker, can_gc);
-            },
-        }
-    }
-}
-
 struct PosterFrameFetchContext {
     /// Reference to the script thread image cache.
     image_cache: Arc<dyn ImageCache>,

components/script/dom/window.rs

@@ -30,7 +30,6 @@ use euclid::default::{Point2D as UntypedPoint2D, Rect as UntypedRect};
 use euclid::{Point2D, Rect, Scale, Size2D, Vector2D};
 use fonts::FontContext;
 use ipc_channel::ipc::{self, IpcSender};
-use ipc_channel::router::ROUTER;
 use js::conversions::ToJSValConvertible;
 use js::glue::DumpJSStack;
 use js::jsapi::{
@@ -148,9 +147,7 @@ use crate::dom::windowproxy::{WindowProxy, WindowProxyHandler};
 use crate::dom::worklet::Worklet;
 use crate::dom::workletglobalscope::WorkletGlobalScopeType;
 use crate::layout_image::fetch_image_for_layout;
-use crate::messaging::{
+use crate::messaging::{MainThreadScriptMsg, ScriptEventLoopReceiver, ScriptEventLoopSender};
-    ImageCacheMsg, MainThreadScriptMsg, ScriptEventLoopReceiver, ScriptEventLoopSender,
-};
 use crate::microtask::MicrotaskQueue;
 use crate::realms::{enter_realm, InRealm};
 use crate::script_runtime::{CanGc, JSContext, Runtime};
@@ -160,6 +157,16 @@ use crate::unminify::unminified_path;
 use crate::webdriver_handlers::jsval_to_webdriver;
 use crate::{fetch, window_named_properties};
 
+/// A callback to call when a response comes back from the `ImageCache`.
+///
+/// This is wrapped in a struct so that we can implement `MallocSizeOf`
+/// for this type.
+#[derive(MallocSizeOf)]
+pub struct PendingImageCallback(
+    #[ignore_malloc_size_of = "dyn Fn is currently impossible to measure"]
+    Box<dyn Fn(PendingImageResponse) + 'static>,
+);
+
 /// Current state of the window object
 #[derive(Clone, Copy, Debug, JSTraceable, MallocSizeOf, PartialEq)]
 enum WindowState {
@@ -216,7 +223,7 @@ pub(crate) struct Window {
     #[no_trace]
     image_cache: Arc<dyn ImageCache>,
     #[no_trace]
-    image_cache_chan: Sender<ImageCacheMsg>,
+    image_cache_sender: IpcSender<PendingImageResponse>,
     window_proxy: MutNullableDom<WindowProxy>,
     document: MutNullableDom<Document>,
     location: MutNullableDom<Location>,
@@ -234,7 +241,6 @@ pub(crate) struct Window {
     /// no devtools server
     #[no_trace]
     devtools_markers: DomRefCell<HashSet<TimelineMarkerType>>,
-    #[ignore_malloc_size_of = "channels are hard"]
     #[no_trace]
     devtools_marker_sender: DomRefCell<Option<IpcSender<Option<TimelineMarker>>>>,
 
@@ -262,7 +268,6 @@ pub(crate) struct Window {
     window_size: Cell<WindowSizeData>,
 
     /// A handle for communicating messages to the bluetooth thread.
-    #[ignore_malloc_size_of = "channels are hard"]
     #[no_trace]
     bluetooth_thread: IpcSender<BluetoothRequest>,
 
@@ -280,7 +285,6 @@ pub(crate) struct Window {
     layout_blocker: Cell<LayoutBlocker>,
 
     /// A channel for communicating results of async scripts back to the webdriver server
-    #[ignore_malloc_size_of = "channels are hard"]
     #[no_trace]
     webdriver_script_chan: DomRefCell<Option<IpcSender<WebDriverJSResult>>>,
 
@@ -311,6 +315,12 @@ pub(crate) struct Window {
     #[cfg(feature = "webxr")]
     webxr_registry: Option<webxr_api::Registry>,
 
+    /// When an element triggers an image load or starts watching an image load from the
+    /// `ImageCache` it adds an entry to this list. When those loads are triggered from
+    /// layout, they also add an etry to [`Self::pending_layout_images`].
+    #[no_trace]
+    pending_image_callbacks: DomRefCell<HashMap<PendingImageId, Vec<PendingImageCallback>>>,
+
     /// All of the elements that have an outstanding image request that was
     /// initiated by layout during a reflow. They are stored in the script thread
     /// to ensure that the element can be marked dirty when the image data becomes
@@ -532,10 +542,20 @@ impl Window {
         Worklet::new(self, WorkletGlobalScopeType::Paint)
     }
 
-    pub(crate) fn pending_image_notification(&self, response: PendingImageResponse) {
+    pub(crate) fn register_image_cache_listener(
-        //XXXjdm could be more efficient to send the responses to layout,
+        &self,
-        //       rather than making layout talk to the image cache to
+        id: PendingImageId,
-        //       obtain the same data.
+        callback: impl Fn(PendingImageResponse) + 'static,
+    ) -> IpcSender<PendingImageResponse> {
+        self.pending_image_callbacks
+            .borrow_mut()
+            .entry(id)
+            .or_default()
+            .push(PendingImageCallback(Box::new(callback)));
+        self.image_cache_sender.clone()
+    }
+
+    fn pending_layout_image_notification(&self, response: PendingImageResponse) {
         let mut images = self.pending_layout_images.borrow_mut();
         let nodes = images.entry(response.id);
         let nodes = match nodes {
@@ -555,6 +575,33 @@ impl Window {
         }
     }
 
+    pub(crate) fn pending_image_notification(&self, response: PendingImageResponse) {
+        // We take the images here, in order to prevent maintaining a mutable borrow when
+        // image callbacks are called. These, in turn, can trigger garbage collection.
+        // Normally this shouldn't trigger more pending image notifications, but just in
+        // case we do not want to cause a double borrow here.
+        let mut images = std::mem::take(&mut *self.pending_image_callbacks.borrow_mut());
+        let Entry::Occupied(callbacks) = images.entry(response.id) else {
+            let _ = std::mem::replace(&mut *self.pending_image_callbacks.borrow_mut(), images);
+            return;
+        };
+
+        for callback in callbacks.get() {
+            callback.0(response.clone());
+        }
+
+        match response.response {
+            ImageResponse::MetadataLoaded(_) => {},
+            ImageResponse::Loaded(_, _) |
+            ImageResponse::PlaceholderLoaded(_, _) |
+            ImageResponse::None => {
+                callbacks.remove();
+            },
+        }
+
+        let _ = std::mem::replace(&mut *self.pending_image_callbacks.borrow_mut(), images);
+    }
+
     pub(crate) fn compositor_api(&self) -> &CrossProcessCompositorApi {
         &self.compositor_api
     }
@@ -1924,17 +1971,16 @@ impl Window {
             let mut images = self.pending_layout_images.borrow_mut();
             let nodes = images.entry(id).or_default();
             if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) {
-                let (responder, responder_listener) =
+                let trusted_node = Trusted::new(&*node);
-                    ProfiledIpc::channel(self.global().time_profiler_chan().clone()).unwrap();
+                let sender = self.register_image_cache_listener(id, move |response| {
-                let image_cache_chan = self.image_cache_chan.clone();
+                    trusted_node
-                ROUTER.add_typed_route(
+                        .root()
-                    responder_listener.to_ipc_receiver(),
+                        .owner_window()
-                    Box::new(move |message| {
+                        .pending_layout_image_notification(response);
-                        let _ = image_cache_chan.send((pipeline_id, message.unwrap()));
+                });
-                    }),
+
-                );
                 self.image_cache
-                    .add_listener(id, ImageResponder::new(responder, id));
+                    .add_listener(ImageResponder::new(sender, self.pipeline_id(), id));
                 nodes.push(Dom::from_ref(&*node));
             }
         }
@@ -2696,7 +2742,7 @@ impl Window {
         script_chan: Sender<MainThreadScriptMsg>,
         layout: Box<dyn Layout>,
         font_context: Arc<FontContext>,
-        image_cache_chan: Sender<ImageCacheMsg>,
+        image_cache_sender: IpcSender<PendingImageResponse>,
         image_cache: Arc<dyn ImageCache>,
         resource_threads: ResourceThreads,
         bluetooth_thread: IpcSender<BluetoothRequest>,
@@ -2760,7 +2806,7 @@ impl Window {
             script_chan,
             layout: RefCell::new(layout),
             font_context,
-            image_cache_chan,
+            image_cache_sender,
             image_cache,
             navigator: Default::default(),
             location: Default::default(),
@@ -2795,6 +2841,7 @@ impl Window {
             webgl_chan,
             #[cfg(feature = "webxr")]
             webxr_registry,
+            pending_image_callbacks: Default::default(),
             pending_layout_images: Default::default(),
             unminified_css_dir: Default::default(),
             local_script_source,

components/script/image_listener.rs

@@ -1,53 +0,0 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
-
-use ipc_channel::ipc;
-use ipc_channel::ipc::IpcSender;
-use ipc_channel::router::ROUTER;
-use net_traits::image_cache::{ImageResponse, PendingImageResponse};
-
-use crate::dom::bindings::conversions::DerivedFrom;
-use crate::dom::bindings::refcounted::Trusted;
-use crate::dom::bindings::reflector::DomObject;
-use crate::dom::node::{Node, NodeTraits};
-use crate::script_runtime::CanGc;
-
-pub(crate) trait ImageCacheListener {
-    fn generation_id(&self) -> u32;
-    fn process_image_response(&self, response: ImageResponse, can_gc: CanGc);
-}
-
-pub(crate) 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();
-
-    let task_source = elem
-        .owner_global()
-        .task_manager()
-        .networking_task_source()
-        .to_sendable();
-    let generation = elem.generation_id();
-
-    ROUTER.add_typed_route(
-        responder_receiver,
-        Box::new(move |message| {
-            let element = trusted_node.clone();
-            let image: PendingImageResponse = message.unwrap();
-            debug!("Got image {:?}", image);
-            task_source.queue(task!(process_image_response: move || {
-                let element = element.root();
-                // Ignore any image response for a previous request that has been discarded.
-                if generation == element.generation_id() {
-                    element.process_image_response(image.response, CanGc::note());
-                }
-            }));
-        }),
-    );
-
-    responder_sender
-}

components/script/lib.rs

@@ -37,7 +37,6 @@ pub(crate) mod document_loader;
 mod dom;
 mod canvas_state;
 pub(crate) mod fetch;
-mod image_listener;
 mod init;
 mod layout_image;
 

components/script/messaging.rs

@@ -32,14 +32,12 @@ use crate::task::TaskBox;
 use crate::task_queue::{QueuedTask, QueuedTaskConversion, TaskQueue};
 use crate::task_source::TaskSourceName;
 
-pub(crate) type ImageCacheMsg = (PipelineId, PendingImageResponse);
-
 #[derive(Debug)]
 pub(crate) enum MixedMessage {
     FromConstellation(ConstellationControlMsg),
     FromScript(MainThreadScriptMsg),
     FromDevtools(DevtoolScriptControlMsg),
-    FromImageCache((PipelineId, PendingImageResponse)),
+    FromImageCache(PendingImageResponse),
     #[cfg(feature = "webgpu")]
     FromWebGPUServer(WebGPUMsg),
     TimerFired,
@@ -101,7 +99,7 @@ impl MixedMessage {
                 MainThreadScriptMsg::Inactive => None,
                 MainThreadScriptMsg::WakeUp => None,
             },
-            MixedMessage::FromImageCache((pipeline_id, _)) => Some(*pipeline_id),
+            MixedMessage::FromImageCache(response) => Some(response.pipeline_id),
             MixedMessage::FromDevtools(_) | MixedMessage::TimerFired => None,
             #[cfg(feature = "webgpu")]
             MixedMessage::FromWebGPUServer(..) => None,
@@ -321,9 +319,11 @@ pub(crate) struct ScriptThreadSenders {
     #[no_trace]
     pub(crate) layout_to_constellation_ipc_sender: IpcSender<LayoutMsg>,
 
-    /// The [`Sender`] on which messages can be sent to the `ImageCache`.
+    /// The shared [`IpcSender`] which is sent to the `ImageCache` when requesting an image. The
+    /// messages on this channel are routed to crossbeam [`Sender`] on the router thread, which
+    /// in turn sends messages to [`ScriptThreadReceivers::image_cache_receiver`].
     #[no_trace]
-    pub(crate) image_cache_sender: Sender<ImageCacheMsg>,
+    pub(crate) image_cache_sender: IpcSender<PendingImageResponse>,
 
     /// For providing contact with the time profiler.
     #[no_trace]
@@ -352,7 +352,7 @@ pub(crate) struct ScriptThreadReceivers {
 
     /// The [`Receiver`] which receives incoming messages from the `ImageCache`.
     #[no_trace]
-    pub(crate) image_cache_receiver: Receiver<ImageCacheMsg>,
+    pub(crate) image_cache_receiver: Receiver<PendingImageResponse>,
 
     /// For receiving commands from an optional devtools server. Will be ignored if no such server
     /// exists. When devtools are not active this will be [`crossbeam_channel::never()`].

components/script/script_thread.rs

@@ -847,7 +847,6 @@ impl ScriptThread {
             .map(|_| ROUTER.route_ipc_receiver_to_new_crossbeam_receiver(ipc_devtools_receiver))
             .unwrap_or_else(crossbeam_channel::never);
 
-        let (image_cache_sender, image_cache_receiver) = unbounded();
         let task_queue = TaskQueue::new(self_receiver, self_sender.clone());
 
         let closing = Arc::new(AtomicBool::new(false));
@@ -863,6 +862,15 @@ impl ScriptThread {
             Some(Box::new(background_hang_monitor_exit_signal)),
         );
 
+        let (image_cache_sender, image_cache_receiver) = unbounded();
+        let (ipc_image_cache_sender, ipc_image_cache_receiver) = ipc::channel().unwrap();
+        ROUTER.add_typed_route(
+            ipc_image_cache_receiver,
+            Box::new(move |message| {
+                let _ = image_cache_sender.send(message.unwrap());
+            }),
+        );
+
         let receivers = ScriptThreadReceivers {
             constellation_receiver,
             image_cache_receiver,
@@ -878,7 +886,7 @@ impl ScriptThread {
             constellation_sender: state.constellation_sender,
             pipeline_to_constellation_sender: state.pipeline_to_constellation_sender.sender.clone(),
             layout_to_constellation_ipc_sender: state.layout_to_constellation_ipc_sender,
-            image_cache_sender,
+            image_cache_sender: ipc_image_cache_sender,
             time_profiler_sender: state.time_profiler_sender,
             memory_profiler_sender: state.memory_profiler_sender,
             devtools_server_sender,
@@ -2078,8 +2086,8 @@ impl ScriptThread {
         }
     }
 
-    fn handle_msg_from_image_cache(&self, (id, response): (PipelineId, PendingImageResponse)) {
+    fn handle_msg_from_image_cache(&self, response: PendingImageResponse) {
-        let window = self.documents.borrow().find_window(id);
+        let window = self.documents.borrow().find_window(response.pipeline_id);
         if let Some(ref window) = window {
             window.pending_image_notification(response);
         }

components/shared/net/image_cache.rs

@@ -4,6 +4,7 @@
 
 use std::sync::Arc;
 
+use base::id::PipelineId;
 use ipc_channel::ipc::IpcSender;
 use log::debug;
 use malloc_size_of_derive::MallocSizeOf;
@@ -28,7 +29,7 @@ pub enum ImageOrMetadataAvailable {
         url: ServoUrl,
         is_placeholder: bool,
     },
-    MetadataAvailable(ImageMetadata),
+    MetadataAvailable(ImageMetadata, PendingImageId),
 }
 
 /// This is optionally passed to the image cache when requesting
@@ -37,13 +38,22 @@ pub enum ImageOrMetadataAvailable {
 /// and/or repaint.
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct ImageResponder {
-    id: PendingImageId,
+    pipeline_id: PipelineId,
+    pub id: PendingImageId,
     sender: IpcSender<PendingImageResponse>,
 }
 
 impl ImageResponder {
-    pub fn new(sender: IpcSender<PendingImageResponse>, id: PendingImageId) -> ImageResponder {
+    pub fn new(
-        ImageResponder { sender, id }
+        sender: IpcSender<PendingImageResponse>,
+        pipeline_id: PipelineId,
+        id: PendingImageId,
+    ) -> ImageResponder {
+        ImageResponder {
+            pipeline_id,
+            sender,
+            id,
+        }
     }
 
     pub fn respond(&self, response: ImageResponse) {
@@ -52,6 +62,7 @@ impl ImageResponder {
         // That's not a case that's worth warning about.
         // TODO(#15501): are there cases in which we should perform cleanup?
         let _ = self.sender.send(PendingImageResponse {
+            pipeline_id: self.pipeline_id,
             response,
             id: self.id,
         });
@@ -75,8 +86,9 @@ pub enum ImageResponse {
 #[derive(Clone, Copy, Debug, Deserialize, Eq, Hash, MallocSizeOf, PartialEq, Serialize)]
 pub struct PendingImageId(pub u64);
 
-#[derive(Debug, Deserialize, Serialize)]
+#[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct PendingImageResponse {
+    pub pipeline_id: PipelineId,
     pub response: ImageResponse,
     pub id: PendingImageId,
 }
@@ -119,24 +131,9 @@ pub trait ImageCache: Sync + Send {
         use_placeholder: UsePlaceholder,
     ) -> 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,
-    ) -> 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.
-    fn add_listener(&self, id: PendingImageId, listener: ImageResponder);
+    fn add_listener(&self, listener: ImageResponder);
 
     /// Inform the image cache about a response for a pending request.
     fn notify_pending_response(&self, id: PendingImageId, action: FetchResponseMsg);