From 2e9b12d327559e585e61a16f903bac18b70292a8 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 11 Jun 2023 06:53:40 +0200 Subject: [PATCH] LibWeb: Update HTML image loading algorithm with null checks from spec The spec has been updated to fix the missing null checks we found: https://github.com/whatwg/html/commit/8f3d1fc6d14091e098ab81a352b2940c3f27922b --- .../LibWeb/HTML/HTMLImageElement.cpp | 30 ++++++------------- .../Libraries/LibWeb/HTML/ImageRequest.cpp | 23 +++++++++----- Userland/Libraries/LibWeb/HTML/ImageRequest.h | 7 +++-- 3 files changed, 29 insertions(+), 31 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp index b40dc5c231d..6941daa62ae 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp @@ -319,11 +319,8 @@ ErrorOr HTMLImageElement::update_the_image_data(bool restart_animations) entry->ignore_higher_layer_caching = true; // 2. Abort the image request for the current request and the pending request. - m_current_request->abort(realm()); - - // FIXME: Spec bug? Seems like pending request can be null here. - if (m_pending_request) - m_pending_request->abort(realm()); + abort_the_image_request(realm(), m_current_request); + abort_the_image_request(realm(), m_pending_request); // 3. Set pending request to null. m_pending_request = nullptr; @@ -379,11 +376,8 @@ after_step_6: // abort the image request for the current request and the pending request, // and set pending request to null. m_current_request->set_state(ImageRequest::State::Broken); - m_current_request->abort(realm()); - - // FIXME: Spec bug? Seems like the image's pending request can be null here. - if (m_pending_request) - m_pending_request->abort(realm()); + abort_the_image_request(realm(), m_current_request); + abort_the_image_request(realm(), m_pending_request); m_pending_request = nullptr; // 2. Queue an element task on the DOM manipulation task source given the img element and the following steps: @@ -406,11 +400,8 @@ after_step_6: // If that is not successful, then: if (!url_string.is_valid()) { // 1. Abort the image request for the current request and the pending request. - m_current_request->abort(realm()); - - // FIXME: Spec bug? Seems like pending request can be null here. - if (m_pending_request) - m_pending_request->abort(realm()); + abort_the_image_request(realm(), m_current_request); + abort_the_image_request(realm(), m_pending_request); // 2. Set the current request's state to broken. m_current_request->set_state(ImageRequest::State::Broken); @@ -440,9 +431,7 @@ after_step_6: // queue an element task on the DOM manipulation task source given the img element // to restart the animation if restart animation is set, and return. if (url_string == m_current_request->current_url() && m_current_request->state() == ImageRequest::State::PartiallyAvailable) { - // FIXME: Spec bug? Seems like pending request can be null here. - if (m_pending_request) - m_pending_request->abort(realm()); + abort_the_image_request(realm(), m_pending_request); if (restart_animations) { queue_an_element_task(HTML::Task::Source::DOMManipulation, [this] { restart_the_animation(); @@ -452,8 +441,7 @@ after_step_6: } // 14. If the pending request is not null, then abort the image request for the pending request. - if (m_pending_request) - m_pending_request->abort(realm()); + abort_the_image_request(realm(), m_pending_request); // 15. Set image request to a new image request whose current URL is urlString. auto image_request = ImageRequest::create().release_value_but_fixme_should_propagate_errors(); @@ -587,7 +575,7 @@ void HTMLImageElement::handle_successful_fetch(AK::URL const& url_string, String // upgrade the pending request to the current request // and prepare image request for presentation given the img element. if (image_request == m_pending_request) { - m_current_request->abort(realm()); + abort_the_image_request(realm(), m_current_request); upgrade_pending_request_to_current_request(); image_request.prepare_for_presentation(*this); } diff --git a/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp b/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp index 01b6afa6f0f..e9b0a77356d 100644 --- a/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp +++ b/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp @@ -48,17 +48,21 @@ void ImageRequest::set_current_url(AK::URL url) } // https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request -void ImageRequest::abort(JS::Realm& realm) +void abort_the_image_request(JS::Realm& realm, ImageRequest* image_request) { - // 1. Forget image request's image data, if any. - m_image_data = nullptr; + // 1. If image request is null, then return. + if (!image_request) + return; - // 2. Abort any instance of the fetching algorithm for image request, + // 2. Forget image request's image data, if any. + image_request->set_image_data(nullptr); + + // 3. Abort any instance of the fetching algorithm for image request, // discarding any pending tasks generated by that algorithm. - if (m_fetch_controller) - m_fetch_controller->abort(realm, {}); + if (auto fetch_controller = image_request->fetch_controller()) + fetch_controller->abort(realm, {}); - m_fetch_controller = nullptr; + image_request->set_fetch_controller(nullptr); } RefPtr ImageRequest::image_data() const @@ -92,6 +96,11 @@ void ImageRequest::prepare_for_presentation(HTMLImageElement&) // FIXME: 16. Update req's img element's presentation appropriately. } +JS::GCPtr ImageRequest::fetch_controller() +{ + return m_fetch_controller.ptr(); +} + void ImageRequest::set_fetch_controller(JS::GCPtr fetch_controller) { m_fetch_controller = move(fetch_controller); diff --git a/Userland/Libraries/LibWeb/HTML/ImageRequest.h b/Userland/Libraries/LibWeb/HTML/ImageRequest.h index 24bdd5661da..7556cdea138 100644 --- a/Userland/Libraries/LibWeb/HTML/ImageRequest.h +++ b/Userland/Libraries/LibWeb/HTML/ImageRequest.h @@ -37,9 +37,6 @@ public: AK::URL const& current_url() const; void set_current_url(AK::URL); - // https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request - void abort(JS::Realm&); - [[nodiscard]] RefPtr image_data() const; void set_image_data(RefPtr); @@ -52,6 +49,7 @@ public: // https://html.spec.whatwg.org/multipage/images.html#prepare-an-image-for-presentation void prepare_for_presentation(HTMLImageElement&); + [[nodiscard]] JS::GCPtr fetch_controller(); void set_fetch_controller(JS::GCPtr); private: @@ -80,4 +78,7 @@ private: JS::Handle m_fetch_controller; }; +// https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request +void abort_the_image_request(JS::Realm&, ImageRequest*); + }