dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibWeb: Fix stale image request callbacks corrupting newer requests

Browse source 
When img.src is changed rapidly (e.g., YouTube Music sets a GIF
placeholder then swaps to a real URL via IntersectionObserver), the
failure callback from the stale first request could corrupt the newer
request by calling abort_the_image_request on the now-reassigned
m_current_request.

Fix this by using the existing m_update_the_image_data_count generation
counter to detect stale fetch callbacks.

This fixes thumbnail loading on YouTube Music.
This commit is contained in:
Aliaksandr Kalenik 2026-03-01 10:25:37 +01:00 committed by Alexander Kalenik
parent a7b70b0042
commit b49c243abd
4 changed files with 51 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

24
Libraries/LibWeb/HTML/HTMLImageElement.cpp
View file

@ -838,7 +838,7 @@ after_step_7:
if (delay_load_event)
m_load_event_delayer.emplace(document());
add_callbacks_to_image_request(*image_request, maybe_omit_events, *url_string, previous_url);
add_callbacks_to_image_request(*image_request, maybe_omit_events, *url_string, previous_url, update_the_image_data_count);
// AD-HOC: If the image request is already available or fetching, no need to start another fetch.
if (image_request->is_available() || image_request->is_fetching())
@ -883,11 +883,11 @@ after_step_7:
}));
}
void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> image_request, bool maybe_omit_events, String const& url_string, String const& previous_url)
void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> image_request, bool maybe_omit_events, String const& url_string, String const& previous_url, u64 update_the_image_data_count)
{
image_request->add_callbacks(
[this, image_request, maybe_omit_events, url_string, previous_url]() {
batching_dispatcher().enqueue(GC::create_function(realm().heap(), [this, image_request, maybe_omit_events, url_string, previous_url] {
[this, image_request, maybe_omit_events, url_string, previous_url, update_the_image_data_count]() {
batching_dispatcher().enqueue(GC::create_function(realm().heap(), [this, image_request, maybe_omit_events, url_string, previous_url, update_the_image_data_count] {
// AD-HOC: Bail out if the document became inactive (e.g. iframe removed or navigated)
// between when the fetch completed and when this batched callback runs.
if (!document().is_fully_active()) {
@ -895,6 +895,13 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
return;
}
// AD-HOC: If another instance of update_the_image_data was started after the one that initiated this
// request, this callback is stale. Bail out to avoid corrupting the state of the newer request.
if (update_the_image_data_count != m_update_the_image_data_count) {
m_load_event_delayer.clear();
return;
}
VERIFY(image_request->shared_resource_request());
auto image_data = image_request->shared_resource_request()->image_data();
image_request->set_image_data(image_data);
@ -936,7 +943,7 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
m_load_event_delayer.clear();
}));
},
[this, image_request, maybe_omit_events, url_string, previous_url]() {
[this, image_request, maybe_omit_events, url_string, previous_url, update_the_image_data_count]() {
// AD-HOC: Bail out if the document became inactive (e.g. iframe removed or navigated)
// between when the fetch completed and when this failure callback runs.
if (!document().is_fully_active()) {
@ -946,6 +953,13 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
// The image data is not in a supported file format;
// AD-HOC: If another instance of update_the_image_data was started after the one that initiated this
// request, this callback is stale. Bail out to avoid corrupting the state of the newer request.
if (update_the_image_data_count != m_update_the_image_data_count) {
m_load_event_delayer.clear();
return;
}
// the user agent must set image request's state to broken,
image_request->set_state(ImageRequest::State::Broken);

2
Libraries/LibWeb/HTML/HTMLImageElement.h
View file

@ -144,7 +144,7 @@ private:
void handle_successful_fetch(URL::URL const&, StringView mime_type, ImageRequest&, ByteBuffer, bool maybe_omit_events, URL::URL const& previous_url);
void handle_failed_fetch();
void add_callbacks_to_image_request(GC::Ref<ImageRequest>, bool maybe_omit_events, String const& url_string, String const& previous_url);
void add_callbacks_to_image_request(GC::Ref<ImageRequest>, bool maybe_omit_events, String const& url_string, String const& previous_url, u64 update_the_image_data_count);
void animate();

2
Tests/LibWeb/Text/expected/HTML/img-src-change-during-failed-decode.txt Normal file
View file

@ -0,0 +1,2 @@
naturalWidth: 1
naturalHeight: 1

29
Tests/LibWeb/Text/input/HTML/img-src-change-during-failed-decode.html Normal file
View file

@ -0,0 +1,29 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<script>
// Test that changing img.src to a valid image while a previous invalid src is still
// being decoded works correctly. The stale failure callback from the first (invalid)
// src must not corrupt the state of the second (valid) image request.
asyncTest((done) => {
let img = document.createElement("img");
document.body.appendChild(img);
// Set src to data that will fail to decode (not valid image data).
img.src = "data:image/png;base64,dGhpcyBpcyBub3QgYW4gaW1hZ2U=";
// After a task boundary, the microtask for the first src will have run and
// started the decode via ImageDecoder. Change src to a valid 1x1 red PNG.
setTimeout(() => {
img.onload = () => {
println(`naturalWidth: ${img.naturalWidth}`);
println(`naturalHeight: ${img.naturalHeight}`);
done();
};
img.onerror = () => {
println("FAIL: error event fired for valid image");
done();
};
img.src = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwADhQGAWjR9awAAAABJRU5ErkJggg==";
}, 0);
});
</script>