From dfe776b7225e1d49fc74e9142c970f99fe020a87 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 10 Aug 2025 16:34:51 +0200 Subject: [PATCH] WebDriver: Stop using the ancient Core::EventReceiver parent/child API Before this change, clients were kept alive by making them children of the TCPServer object. This ownership model is going away (and this was the only remaining use of it!) so let's just put the clients in a hash table instead. --- Libraries/LibWeb/WebDriver/Client.cpp | 10 ++++++---- Libraries/LibWeb/WebDriver/Client.h | 4 +++- Services/WebDriver/Client.cpp | 8 ++++---- Services/WebDriver/Client.h | 4 ++-- Services/WebDriver/main.cpp | 10 +++++++++- 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/Libraries/LibWeb/WebDriver/Client.cpp b/Libraries/LibWeb/WebDriver/Client.cpp index 1640932ab0a..ebfd663dc89 100644 --- a/Libraries/LibWeb/WebDriver/Client.cpp +++ b/Libraries/LibWeb/WebDriver/Client.cpp @@ -176,9 +176,8 @@ static JsonValue make_success_response(JsonValue value) return result; } -Client::Client(NonnullOwnPtr socket, Core::EventReceiver* parent) - : Core::EventReceiver(parent) - , m_socket(move(socket)) +Client::Client(NonnullOwnPtr socket) + : m_socket(move(socket)) { m_socket->on_ready_to_read = [this] { if (auto result = on_ready_to_read(); result.is_error()) @@ -194,7 +193,10 @@ Client::~Client() void Client::die() { // We defer removing this connection to avoid closing its socket while we are inside the on_ready_to_read callback. - deferred_invoke([this] { remove_from_parent(); }); + deferred_invoke([this] { + if (on_death) + on_death(); + }); } ErrorOr Client::on_ready_to_read() diff --git a/Libraries/LibWeb/WebDriver/Client.h b/Libraries/LibWeb/WebDriver/Client.h index 966b2a849d6..0c5015f2ee8 100644 --- a/Libraries/LibWeb/WebDriver/Client.h +++ b/Libraries/LibWeb/WebDriver/Client.h @@ -116,8 +116,10 @@ public: // 18. Print, https://w3c.github.io/webdriver/#print virtual Response print_page(Parameters parameters, JsonValue payload) = 0; + Function on_death; + protected: - Client(NonnullOwnPtr, Core::EventReceiver* parent); + explicit Client(NonnullOwnPtr); private: using WrappedError = Variant; diff --git a/Services/WebDriver/Client.cpp b/Services/WebDriver/Client.cpp index 5052e77f713..758750bdca0 100644 --- a/Services/WebDriver/Client.cpp +++ b/Services/WebDriver/Client.cpp @@ -21,17 +21,17 @@ namespace WebDriver { -ErrorOr> Client::try_create(NonnullOwnPtr socket, LaunchBrowserCallback launch_browser_callback, Core::EventReceiver* parent) +ErrorOr> Client::try_create(NonnullOwnPtr socket, LaunchBrowserCallback launch_browser_callback) { if (!launch_browser_callback) return Error::from_string_literal("The callback to launch the browser must be provided"); TRY(socket->set_blocking(true)); - return adopt_nonnull_ref_or_enomem(new (nothrow) Client(move(socket), move(launch_browser_callback), parent)); + return adopt_nonnull_ref_or_enomem(new (nothrow) Client(move(socket), move(launch_browser_callback))); } -Client::Client(NonnullOwnPtr socket, LaunchBrowserCallback launch_browser_callback, Core::EventReceiver* parent) - : Web::WebDriver::Client(move(socket), parent) +Client::Client(NonnullOwnPtr socket, LaunchBrowserCallback launch_browser_callback) + : Web::WebDriver::Client(move(socket)) , m_launch_browser_callback(move(launch_browser_callback)) { } diff --git a/Services/WebDriver/Client.h b/Services/WebDriver/Client.h index 73eb10b2d31..7a73b8f042f 100644 --- a/Services/WebDriver/Client.h +++ b/Services/WebDriver/Client.h @@ -23,13 +23,13 @@ class Client final : public Web::WebDriver::Client { C_OBJECT_ABSTRACT(Client); public: - static ErrorOr> try_create(NonnullOwnPtr, LaunchBrowserCallback, Core::EventReceiver* parent); + static ErrorOr> try_create(NonnullOwnPtr, LaunchBrowserCallback); virtual ~Client() override; LaunchBrowserCallback const& launch_browser_callback() const { return m_launch_browser_callback; } private: - Client(NonnullOwnPtr, LaunchBrowserCallback, Core::EventReceiver* parent); + Client(NonnullOwnPtr, LaunchBrowserCallback); virtual Web::WebDriver::Response new_session(Web::WebDriver::Parameters parameters, JsonValue payload) override; virtual Web::WebDriver::Response delete_session(Web::WebDriver::Parameters parameters, JsonValue payload) override; diff --git a/Services/WebDriver/main.cpp b/Services/WebDriver/main.cpp index 15f4e2b29be..e28d2af9235 100644 --- a/Services/WebDriver/main.cpp +++ b/Services/WebDriver/main.cpp @@ -106,6 +106,8 @@ ErrorOr ladybird_main(Main::Arguments arguments) Core::EventLoop loop; auto server = TRY(Core::TCPServer::try_create()); + HashTable> clients; + // FIXME: Propagate errors server->on_ready_to_accept = [&] { auto maybe_client_socket = server->accept(); @@ -125,11 +127,17 @@ ErrorOr ladybird_main(Main::Arguments arguments) return launch_process("Ladybird"sv, arguments.span()); }; - auto maybe_client = WebDriver::Client::try_create(maybe_buffered_socket.release_value(), move(launch_browser_callback), server); + auto maybe_client = WebDriver::Client::try_create(maybe_buffered_socket.release_value(), move(launch_browser_callback)); if (maybe_client.is_error()) { warnln("Could not create a WebDriver client: {}", maybe_client.error()); return; } + + auto client = maybe_client.release_value(); + client->on_death = [&] { + clients.remove(client); + }; + clients.set(move(client)); }; TRY(server->listen(ipv4_address.value(), port, Core::TCPServer::AllowAddressReuse::Yes));