diff options
| author | Josh Matthews <josh@joshmatthews.net> | 2025-04-19 02:09:03 -0400 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-04-19 06:09:03 +0000 |
| commit | 3ab5b8c4472129798b63cfb40b63ae672763b653 (patch) | |
| tree | 1702e217a31111600f411ecedb8b4a21bb8c569e | |
| parent | c486c6f058ef9ac63e02fd42235e96db64744221 (diff) | |
| download | servo-3ab5b8c4472129798b63cfb40b63ae672763b653.tar.gz servo-3ab5b8c4472129798b63cfb40b63ae672763b653.zip | |
script: Only register one image callback per CSS image in use. (#36612)
When layout encounters a CSS image, the script thread is responsible for
fetching the image from the image cache. When the image is not yet
available, the script thread creates image cache listeners to perform
actions in response to future updates from the image cache.
In the current implementation, a cache listener would iterate over all
nodes using a particular image and mark them as dirty. However, we
mistakenly added one cache listener per node, leading to n^2 runtime
while performing lots of redundant work. For cases like #36480 with over
1000 elements using the same image, this led to a completely
unresponsive script thread.
Testing: Manual testing on the provided testcase, and a new WPT test
that times out without this PR's changes.
Fixes: #36480
Signed-off-by: Josh Matthews <josh@joshmatthews.net>
| -rw-r--r-- | components/script/dom/window.rs | 7 | ||||
| -rw-r--r-- | tests/wpt/mozilla/meta/MANIFEST.json | 7 | ||||
| -rw-r--r-- | tests/wpt/mozilla/tests/mozilla/css-image-batch-callbacks.html | 22 |
3 files changed, 34 insertions, 2 deletions
diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index ce3b5ff6e8c..0308e207e65 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -2019,8 +2019,7 @@ 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)) { + if !images.contains_key(&id) { let trusted_node = Trusted::new(&*node); let sender = self.register_image_cache_listener(id, move |response| { trusted_node @@ -2031,6 +2030,10 @@ impl Window { self.image_cache .add_listener(ImageResponder::new(sender, self.pipeline_id(), id)); + } + + let nodes = images.entry(id).or_default(); + if !nodes.iter().any(|n| std::ptr::eq(&**n, &*node)) { nodes.push(Dom::from_ref(&*node)); } } diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index 7a9b0919885..b6782318356 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -12873,6 +12873,13 @@ ] ] }, + "css-image-batch-callbacks.html": [ + "c449b8bab01d27a41bd1ba472ce6b38d0fa768b0", + [ + null, + {} + ] + ], "custom_auto_rooter.html": [ "3d6f04e85b27bcf957b273e04e4a80b75e714b2f", [ diff --git a/tests/wpt/mozilla/tests/mozilla/css-image-batch-callbacks.html b/tests/wpt/mozilla/tests/mozilla/css-image-batch-callbacks.html new file mode 100644 index 00000000000..c449b8bab01 --- /dev/null +++ b/tests/wpt/mozilla/tests/mozilla/css-image-batch-callbacks.html @@ -0,0 +1,22 @@ +<!doctype html> +<script src="/resources/testharness.js"></script> +<script src="/resources/testharnessreport.js"></script> +<style> + .file_icon:before { +content: url('data:image/gif;base64,R0lGODlhEgAWAKIAAP///8z//5mZmTMzMwAAAAAAAAAAAAAAACH5BAEAAAEALAAAAAASABYAQANHOLq88W+ASSsVmEjLr+7UNg2BCFaCNmBs6w7fuZHRCRB4Htueyac+G9AmodVkoSNIw4QFOa0dDzCcTqpW7FT7k3YfurCYkAAAOw=='); +} +</style> +<div id="content"></div> +<script> + async_test(t => { + let content = document.querySelector('#content'); + for (let i = 0; i < 1000; i++) { + let elem = document.createElement("div"); + elem.className = "file_icon" + content.appendChild(elem); + } + let elem = document.createElement("img"); + elem.src = 'data:image/gif;base64,R0lGODlhEgAWAKIAAP///8z//5mZmTMzMwAAAAAAAAAAAAAAACH5BAEAAAEALAAAAAASABYAQANHOLq88W+ASSsVmEjLr+7UNg2BCFaCNmBs6w7fuZHRCRB4Htueyac+G9AmodVkoSNIw4QFOa0dDzCcTqpW7FT7k3YfurCYkAAAOw=='; + elem.onload = t.step_func_done(); + }, "Large numbers of CSS images do not prevent other async tasks from executing"); +</script> |