RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 12:57:35 +00:00
Code Issues Activity Actions

LibWeb+WebContent: Do not reference-count file request objects

Browse source 
There is currently a memory leak with these file request objects due to
the callback on_file_request_finish referencing itself in its capture
list. This object does not need to be reference counted or allocated on
the heap. It is only ever stored in a HashMap until a response is
received from the browser, and it is not shared.
This commit is contained in:
Timothy Flynn 2023-01-30 16:35:47 -05:00 • committed by Linus Groh
parent 9bb469f324
commit 96f409ec1e
9 changed files with 29 additions and 26 deletions
Download patch file Download diff file Expand all files Collapse all files

5
Userland/Libraries/LibWeb/Loader/FileRequest.cpp
View file

@ -8,8 +8,9 @@
namespace Web {
FileRequest::FileRequest(DeprecatedString path)
: m_path(move(path))
FileRequest::FileRequest(DeprecatedString path, Function<void(ErrorOr<i32>)> on_file_request_finish_callback)
: on_file_request_finish(move(on_file_request_finish_callback))
, m_path(move(path))
{
}

5
Userland/Libraries/LibWeb/Loader/FileRequest.h
View file

@ -9,13 +9,12 @@
#include <AK/DeprecatedString.h>
#include <AK/Error.h>
#include <AK/Function.h>
#include <AK/RefCounted.h>
namespace Web {
class FileRequest : public RefCounted<FileRequest> {
class FileRequest {
public:
explicit FileRequest(DeprecatedString path);
FileRequest(DeprecatedString path, Function<void(ErrorOr<i32>)> on_file_request_finish);
DeprecatedString path() const;

10
Userland/Libraries/LibWeb/Loader/ResourceLoader.cpp
View file

@ -236,9 +236,8 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, Has
if (!m_page.has_value())
return;
VERIFY(m_page.has_value());
auto file_ref = make_ref_counted<FileRequest>(url.path());
file_ref->on_file_request_finish = [this, success_callback = move(success_callback), error_callback = move(error_callback), log_success, log_failure, request, file_ref](ErrorOr<i32> file_or_error) {
FileRequest file_request(url.path(), [this, success_callback = move(success_callback), error_callback = move(error_callback), log_success, log_failure, request](ErrorOr<i32> file_or_error) {
--m_pending_loads;
if (on_load_counter_change)
on_load_counter_change();
@ -271,8 +270,9 @@ void ResourceLoader::load(LoadRequest& request, Function<void(ReadonlyBytes, Has
auto data = maybe_data.release_value();
log_success(request);
success_callback(data, {}, {});
};
m_page->client().request_file(file_ref);
});
m_page->client().request_file(move(file_request));
++m_pending_loads;
if (on_load_counter_change)

2
Userland/Libraries/LibWeb/Page/Page.h
View file

@ -195,7 +195,7 @@ public:
virtual void page_did_update_resource_count(i32) { }
virtual void page_did_close_browsing_context(HTML::BrowsingContext const&) { }
virtual void request_file(NonnullRefPtr<FileRequest>&) = 0;
virtual void request_file(FileRequest) = 0;
// https://html.spec.whatwg.org/multipage/input.html#show-the-picker,-if-applicable
virtual void page_did_request_file_picker(WeakPtr<DOM::EventTarget>, [[maybe_unused]] bool multiple) {};