Browse Source

LibWeb: Forbid sharing image requests across documents

This change addresses the bug where images unable to load when the
reload button in the UI is clicked repeatedly. Before this fix, it was
possible to use SharedImageRequests across multiple documents. However,
when the document that initiated the request is gone, tasks scheduled
on the event loop remain in the fetching state because the originating
document is no longer active. Furthermore, another reason to prohibit
the sharing of image requests across documents is that the "Origin"
header in an image request is dependent on the document.
Aliaksandr Kalenik 1 year ago
parent
commit
8ebb4e8047

+ 5 - 0
Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -3555,4 +3555,9 @@ void Document::update_for_history_step_application(JS::NonnullGCPtr<HTML::Sessio
     }
     }
 }
 }
 
 
+HashMap<AK::URL, HTML::SharedImageRequest*>& Document::shared_image_requests()
+{
+    return m_shared_image_requests;
+}
+
 }
 }

+ 5 - 0
Userland/Libraries/LibWeb/DOM/Document.h

@@ -30,6 +30,7 @@
 #include <LibWeb/HTML/Origin.h>
 #include <LibWeb/HTML/Origin.h>
 #include <LibWeb/HTML/SandboxingFlagSet.h>
 #include <LibWeb/HTML/SandboxingFlagSet.h>
 #include <LibWeb/HTML/Scripting/Environments.h>
 #include <LibWeb/HTML/Scripting/Environments.h>
+#include <LibWeb/HTML/SharedImageRequest.h>
 #include <LibWeb/HTML/VisibilityState.h>
 #include <LibWeb/HTML/VisibilityState.h>
 #include <LibWeb/WebIDL/ExceptionOr.h>
 #include <LibWeb/WebIDL/ExceptionOr.h>
 
 
@@ -531,6 +532,8 @@ public:
 
 
     void update_for_history_step_application(JS::NonnullGCPtr<HTML::SessionHistoryEntry>, bool do_not_reactive, size_t script_history_length, size_t script_history_index);
     void update_for_history_step_application(JS::NonnullGCPtr<HTML::SessionHistoryEntry>, bool do_not_reactive, size_t script_history_length, size_t script_history_index);
 
 
+    HashMap<AK::URL, HTML::SharedImageRequest*>& shared_image_requests();
+
 protected:
 protected:
     virtual void initialize(JS::Realm&) override;
     virtual void initialize(JS::Realm&) override;
     virtual void visit_edges(Cell::Visitor&) override;
     virtual void visit_edges(Cell::Visitor&) override;
@@ -735,6 +738,8 @@ private:
 
 
     // https://html.spec.whatwg.org/multipage/browsing-the-web.html#latest-entry
     // https://html.spec.whatwg.org/multipage/browsing-the-web.html#latest-entry
     JS::GCPtr<HTML::SessionHistoryEntry> m_latest_entry;
     JS::GCPtr<HTML::SessionHistoryEntry> m_latest_entry;
+
+    HashMap<AK::URL, HTML::SharedImageRequest*> m_shared_image_requests;
 };
 };
 
 
 template<>
 template<>

+ 11 - 11
Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp

@@ -18,36 +18,36 @@
 
 
 namespace Web::HTML {
 namespace Web::HTML {
 
 
-static HashMap<AK::URL, SharedImageRequest*>& shared_image_requests()
-{
-    static HashMap<AK::URL, SharedImageRequest*> requests;
-    return requests;
-}
-
 JS::NonnullGCPtr<SharedImageRequest> SharedImageRequest::get_or_create(JS::Realm& realm, Page& page, AK::URL const& url)
 JS::NonnullGCPtr<SharedImageRequest> SharedImageRequest::get_or_create(JS::Realm& realm, Page& page, AK::URL const& url)
 {
 {
-    if (auto it = shared_image_requests().find(url); it != shared_image_requests().end())
+    auto document = Bindings::host_defined_environment_settings_object(realm).responsible_document();
+    VERIFY(document);
+    auto& shared_image_requests = document->shared_image_requests();
+    if (auto it = shared_image_requests.find(url); it != shared_image_requests.end())
         return *it->value;
         return *it->value;
-    auto request = realm.heap().allocate<SharedImageRequest>(realm, page, url);
-    shared_image_requests().set(url, request);
+    auto request = realm.heap().allocate<SharedImageRequest>(realm, page, url, *document);
+    shared_image_requests.set(url, request);
     return request;
     return request;
 }
 }
 
 
-SharedImageRequest::SharedImageRequest(Page& page, AK::URL url)
+SharedImageRequest::SharedImageRequest(Page& page, AK::URL url, JS::NonnullGCPtr<DOM::Document> document)
     : m_page(page)
     : m_page(page)
     , m_url(move(url))
     , m_url(move(url))
+    , m_document(document)
 {
 {
 }
 }
 
 
 SharedImageRequest::~SharedImageRequest()
 SharedImageRequest::~SharedImageRequest()
 {
 {
-    shared_image_requests().remove(m_url);
+    auto& shared_image_requests = m_document->shared_image_requests();
+    shared_image_requests.remove(m_url);
 }
 }
 
 
 void SharedImageRequest::visit_edges(JS::Cell::Visitor& visitor)
 void SharedImageRequest::visit_edges(JS::Cell::Visitor& visitor)
 {
 {
     Base::visit_edges(visitor);
     Base::visit_edges(visitor);
     visitor.visit(m_fetch_controller);
     visitor.visit(m_fetch_controller);
+    visitor.visit(m_document);
     for (auto& callback : m_callbacks) {
     for (auto& callback : m_callbacks) {
         visitor.visit(callback.on_finish);
         visitor.visit(callback.on_finish);
         visitor.visit(callback.on_fail);
         visitor.visit(callback.on_fail);

+ 3 - 1
Userland/Libraries/LibWeb/HTML/SharedImageRequest.h

@@ -42,7 +42,7 @@ public:
     virtual void visit_edges(JS::Cell::Visitor&) override;
     virtual void visit_edges(JS::Cell::Visitor&) override;
 
 
 private:
 private:
-    explicit SharedImageRequest(Page&, AK::URL);
+    explicit SharedImageRequest(Page&, AK::URL, JS::NonnullGCPtr<DOM::Document>);
 
 
     void handle_successful_fetch(AK::URL const&, StringView mime_type, ByteBuffer data);
     void handle_successful_fetch(AK::URL const&, StringView mime_type, ByteBuffer data);
     void handle_failed_fetch();
     void handle_failed_fetch();
@@ -67,6 +67,8 @@ private:
     AK::URL m_url;
     AK::URL m_url;
     RefPtr<DecodedImageData const> m_image_data;
     RefPtr<DecodedImageData const> m_image_data;
     JS::GCPtr<Fetch::Infrastructure::FetchController> m_fetch_controller;
     JS::GCPtr<Fetch::Infrastructure::FetchController> m_fetch_controller;
+
+    JS::GCPtr<DOM::Document> m_document;
 };
 };
 
 
 }
 }