Create new image cache per document (#36832)

Rather than sharing the full image cache in a script_thread, the image
cache is now unique per document. This ensures that CSP factors no
longer affect whether the image is retrieved from the cache incorrectly.

To do so, the thread_pool is shared across all caches, but the store is
fresh. Except for the place_holder{image,url}, which are cloned. That's
because the `rippy_data` is only available in the constellation and no
longer accessible at the point that we need to create the document in
the script_thread.

Contrary to the description in #36505, the script_thread still has an
image_cache for this reason: so it has access to the store and
thread_pool to clone it.

With these changes, the two CSP tests no longer flake. Confirmed with
running the following commmand:

```
./mach test-wpt tests/wpt/tests/content-security-policy/generic/ --rerun=10
```

Fixes #36505

Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
This commit is contained in:
Tim van der Lippe 2025-05-04 22:20:56 +02:00 committed by GitHub
parent 3db0194e5a
commit 8a837778d9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 68 additions and 95 deletions

View file

@ -452,6 +452,8 @@ impl Layout for LayoutThread {
.map(|tree| tree.conditional_size_of(ops))
.unwrap_or_default(),
});
reports.push(self.image_cache.memory_report(formatted_url, ops));
}
fn set_quirks_mode(&mut self, quirks_mode: QuirksMode) {

View file

@ -431,7 +431,7 @@ pub struct ImageCacheImpl {
store: Arc<Mutex<ImageCacheStore>>,
/// Thread pool for image decoding
thread_pool: CoreResourceThreadPool,
thread_pool: Arc<CoreResourceThreadPool>,
}
impl ImageCache for ImageCacheImpl {
@ -454,7 +454,10 @@ impl ImageCache for ImageCacheImpl {
placeholder_url: ServoUrl::parse("chrome://resources/rippy.png").unwrap(),
compositor_api,
})),
thread_pool: CoreResourceThreadPool::new(thread_count, "ImageCache".to_string()),
thread_pool: Arc::new(CoreResourceThreadPool::new(
thread_count,
"ImageCache".to_string(),
)),
}
}
@ -651,6 +654,25 @@ impl ImageCache for ImageCacheImpl {
},
}
}
fn create_new_image_cache(
&self,
compositor_api: CrossProcessCompositorApi,
) -> Arc<dyn ImageCache> {
let store = self.store.lock().unwrap();
let placeholder_image = store.placeholder_image.clone();
let placeholder_url = store.placeholder_url.clone();
Arc::new(ImageCacheImpl {
store: Arc::new(Mutex::new(ImageCacheStore {
pending_loads: AllPendingLoads::new(),
completed_loads: HashMap::new(),
placeholder_image,
placeholder_url,
compositor_api,
})),
thread_pool: self.thread_pool.clone(),
})
}
}
impl ImageCacheImpl {

View file

@ -745,7 +745,9 @@ impl ScriptThread {
.senders
.pipeline_to_constellation_sender
.clone(),
image_cache: script_thread.image_cache.clone(),
image_cache: script_thread
.image_cache
.create_new_image_cache(script_thread.compositor_api.clone()),
#[cfg(feature = "webgpu")]
gpu_id_hub: script_thread.gpu_id_hub.clone(),
inherited_secure_context: script_thread.inherited_secure_context,
@ -2443,8 +2445,6 @@ impl ScriptThread {
let prefix = format!("url({urls})");
reports.extend(self.get_cx().get_reports(prefix.clone(), ops));
reports.push(self.image_cache.memory_report(&prefix, ops));
});
reports_chan.send(ProcessReports::new(reports));
@ -3145,13 +3145,17 @@ impl ScriptThread {
self.resource_threads.clone(),
));
let image_cache = self
.image_cache
.create_new_image_cache(self.compositor_api.clone());
let layout_config = LayoutConfig {
id: incomplete.pipeline_id,
webview_id: incomplete.webview_id,
url: final_url.clone(),
is_iframe: incomplete.parent_info.is_some(),
script_chan: self.senders.constellation_sender.clone(),
image_cache: self.image_cache.clone(),
image_cache: image_cache.clone(),
font_context: font_context.clone(),
time_profiler_chan: self.senders.time_profiler_sender.clone(),
compositor_api: self.compositor_api.clone(),
@ -3166,7 +3170,7 @@ impl ScriptThread {
self.layout_factory.create(layout_config),
font_context,
self.senders.image_cache_sender.clone(),
self.image_cache.clone(),
image_cache.clone(),
self.resource_threads.clone(),
#[cfg(feature = "bluetooth")]
self.senders.bluetooth_sender.clone(),

View file

@ -141,4 +141,10 @@ pub trait ImageCache: Sync + Send {
/// Inform the image cache about a response for a pending request.
fn notify_pending_response(&self, id: PendingImageId, action: FetchResponseMsg);
/// Create new image cache based on this one, while reusing the existing thread_pool.
fn create_new_image_cache(
&self,
compositor_api: CrossProcessCompositorApi,
) -> Arc<dyn ImageCache>;
}