From 700ad6bf350f9e74044c015157f8ad7072c7f04c Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 16 Mar 2023 08:50:22 -0400 Subject: [PATCH] WebContent+LibWebView: Consolidate the way browsers connect to WebDriver Currently, on Serenity, we connect to WebDriver from the browser-side of the WebContent connection for both Browser and headless-browser. On Lagom, we connect from within the WebContent process itself, signaled by a command line flag. This patch changes Lagom browsers to connect to WebDriver the same way that Serenity browsers do. This will ensure we can do other initializers in the same order across all platforms and browsers. --- Ladybird/WebContent/main.cpp | 10 +++------- Ladybird/WebContentView.cpp | 5 ++++- Userland/Libraries/LibWebView/ViewImplementation.cpp | 9 ++------- Userland/Libraries/LibWebView/ViewImplementation.h | 2 +- Userland/Services/WebContent/PageHost.cpp | 4 ++++ Userland/Services/WebContent/PageHost.h | 1 + Userland/Utilities/headless-browser.cpp | 8 ++++---- 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/Ladybird/WebContent/main.cpp b/Ladybird/WebContent/main.cpp index d1ab62ad9b5..f30f6723787 100644 --- a/Ladybird/WebContent/main.cpp +++ b/Ladybird/WebContent/main.cpp @@ -83,11 +83,9 @@ ErrorOr serenity_main(Main::Arguments arguments) dbgln("Failed to load content filters: {}", maybe_content_filter_error.error()); int webcontent_fd_passing_socket { -1 }; - DeprecatedString webdriver_content_ipc_path; Core::ArgsParser args_parser; args_parser.add_option(webcontent_fd_passing_socket, "File descriptor of the passing socket for the WebContent connection", "webcontent-fd-passing-socket", 'c', "webcontent_fd_passing_socket"); - args_parser.add_option(webdriver_content_ipc_path, "Path to WebDriver IPC for WebContent", "webdriver-content-path", 0, "path"); args_parser.parse(arguments); VERIFY(webcontent_fd_passing_socket >= 0); @@ -100,11 +98,9 @@ ErrorOr serenity_main(Main::Arguments arguments) proxy_socket_through_notifier(*webcontent_client, webcontent_notifier); QSocketNotifier webdriver_notifier(QSocketNotifier::Type::Read); - RefPtr webdriver_client; - if (!webdriver_content_ipc_path.is_empty()) { - webdriver_client = TRY(WebContent::WebDriverConnection::connect(webcontent_client->page_host(), webdriver_content_ipc_path)); - proxy_socket_through_notifier(*webdriver_client, webdriver_notifier); - } + webcontent_client->page_host().on_webdriver_connection = [&](auto& webdriver) { + proxy_socket_through_notifier(webdriver, webdriver_notifier); + }; return app.exec(); } diff --git a/Ladybird/WebContentView.cpp b/Ladybird/WebContentView.cpp index acc1457ffe5..f6394145bf5 100644 --- a/Ladybird/WebContentView.cpp +++ b/Ladybird/WebContentView.cpp @@ -603,7 +603,7 @@ void WebContentView::create_client() m_client_state = {}; auto candidate_web_content_paths = get_paths_for_helper_process("WebContent"sv).release_value_but_fixme_should_propagate_errors(); - auto new_client = launch_web_content_process(candidate_web_content_paths, m_webdriver_content_ipc_path).release_value_but_fixme_should_propagate_errors(); + auto new_client = launch_web_content_process(candidate_web_content_paths).release_value_but_fixme_should_propagate_errors(); m_web_content_notifier.setSocket(new_client->socket().fd().value()); m_web_content_notifier.setEnabled(true); @@ -636,6 +636,9 @@ void WebContentView::create_client() // FIXME: Get the screen rect. // client().async_update_screen_rects(GUI::Desktop::the().rects(), GUI::Desktop::the().main_screen_index()); + + if (!m_webdriver_content_ipc_path.is_empty()) + client().async_connect_to_webdriver(m_webdriver_content_ipc_path); } void WebContentView::handle_web_content_process_crash() diff --git a/Userland/Libraries/LibWebView/ViewImplementation.cpp b/Userland/Libraries/LibWebView/ViewImplementation.cpp index 85332ee979b..c77b8c27f9b 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.cpp +++ b/Userland/Libraries/LibWebView/ViewImplementation.cpp @@ -126,7 +126,7 @@ void ViewImplementation::run_javascript(StringView js_source) #if !defined(AK_OS_SERENITY) -ErrorOr> ViewImplementation::launch_web_content_process(ReadonlySpan candidate_web_content_paths, StringView webdriver_content_ipc_path) +ErrorOr> ViewImplementation::launch_web_content_process(ReadonlySpan candidate_web_content_paths) { int socket_fds[2] {}; TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, socket_fds)); @@ -149,17 +149,12 @@ ErrorOr> ViewImplementation::launch_web auto webcontent_fd_passing_socket_string = TRY(String::number(wc_fd_passing_fd)); - Vector arguments { + auto arguments = Array { "WebContent"sv, "--webcontent-fd-passing-socket"sv, webcontent_fd_passing_socket_string }; - if (!webdriver_content_ipc_path.is_empty()) { - TRY(arguments.try_append("--webdriver-content-path"sv)); - TRY(arguments.try_append(webdriver_content_ipc_path)); - } - ErrorOr result; for (auto const& path : candidate_web_content_paths) { result = Core::System::exec(path, arguments, Core::System::SearchInPath::Yes); diff --git a/Userland/Libraries/LibWebView/ViewImplementation.h b/Userland/Libraries/LibWebView/ViewImplementation.h index d23e4f7ef4e..d065f14c7c4 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.h +++ b/Userland/Libraries/LibWebView/ViewImplementation.h @@ -118,7 +118,7 @@ protected: virtual void update_zoom() = 0; #if !defined(AK_OS_SERENITY) - ErrorOr> launch_web_content_process(ReadonlySpan candidate_web_content_paths, StringView webdriver_content_ipc_path); + ErrorOr> launch_web_content_process(ReadonlySpan candidate_web_content_paths); #endif struct SharedBitmap { diff --git a/Userland/Services/WebContent/PageHost.cpp b/Userland/Services/WebContent/PageHost.cpp index ae30fd7cf2b..582d731735c 100644 --- a/Userland/Services/WebContent/PageHost.cpp +++ b/Userland/Services/WebContent/PageHost.cpp @@ -93,6 +93,10 @@ ErrorOr PageHost::connect_to_webdriver(DeprecatedString const& webdriver_i { VERIFY(!m_webdriver); m_webdriver = TRY(WebDriverConnection::connect(*this, webdriver_ipc_path)); + + if (on_webdriver_connection) + on_webdriver_connection(*m_webdriver); + return {}; } diff --git a/Userland/Services/WebContent/PageHost.h b/Userland/Services/WebContent/PageHost.h index b7f3c133c4e..fafbb8f676d 100644 --- a/Userland/Services/WebContent/PageHost.h +++ b/Userland/Services/WebContent/PageHost.h @@ -43,6 +43,7 @@ public: Web::DevicePixelSize content_size() const { return m_content_size; } ErrorOr connect_to_webdriver(DeprecatedString const& webdriver_ipc_path); + Function on_webdriver_connection; void alert_closed(); void confirm_closed(bool accepted); diff --git a/Userland/Utilities/headless-browser.cpp b/Userland/Utilities/headless-browser.cpp index 498a23104fc..5ce84c9c396 100644 --- a/Userland/Utilities/headless-browser.cpp +++ b/Userland/Utilities/headless-browser.cpp @@ -53,12 +53,9 @@ public: #if defined(AK_OS_SERENITY) view->m_client_state.client = TRY(WebView::WebContentClient::try_create(*view)); - - if (!web_driver_ipc_path.is_empty()) - view->client().async_connect_to_webdriver(web_driver_ipc_path); #else auto candidate_web_content_paths = TRY(get_paths_for_helper_process("WebContent"sv)); - view->m_client_state.client = TRY(view->launch_web_content_process(candidate_web_content_paths, web_driver_ipc_path)); + view->m_client_state.client = TRY(view->launch_web_content_process(candidate_web_content_paths)); #endif view->client().async_update_system_theme(move(theme)); @@ -67,6 +64,9 @@ public: view->client().async_set_viewport_rect({ { 0, 0 }, window_size }); view->client().async_set_window_size(window_size); + if (!web_driver_ipc_path.is_empty()) + view->client().async_connect_to_webdriver(web_driver_ipc_path); + return view; }