From 1dee21e562a5fdd3af24cc3271f85df23ffa05bd Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 15 Dec 2022 09:18:52 -0500 Subject: [PATCH] WebDriver: Migrate to using local socket files for WebDriver IPC This allows us to use standard Serenity IPC infrastructure rather than manually creating FD-passing sockets. This also lets us use Serenity's WebDriver Session class, removing the copy previously used in Ladybird. This ensures any changes to Session in the future will be picked up by Ladybird for free. --- BrowserWindow.cpp | 6 +- BrowserWindow.h | 4 +- Tab.cpp | 6 +- Tab.h | 2 +- WebContent/main.cpp | 37 +++++------ WebContentView.cpp | 42 ++++++------- WebContentView.h | 4 +- WebDriver/CMakeLists.txt | 2 +- WebDriver/Session.cpp | 130 --------------------------------------- WebDriver/Session.h | 47 -------------- WebDriver/main.cpp | 42 ++++++++++++- main.cpp | 6 +- 12 files changed, 96 insertions(+), 232 deletions(-) delete mode 100644 WebDriver/Session.cpp delete mode 100644 WebDriver/Session.h diff --git a/BrowserWindow.cpp b/BrowserWindow.cpp index ddf3a22e6fc..43733d525cf 100644 --- a/BrowserWindow.cpp +++ b/BrowserWindow.cpp @@ -22,9 +22,9 @@ extern DeprecatedString s_serenity_resource_root; extern Browser::Settings* s_settings; -BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, int webdriver_fd_passing_socket) +BrowserWindow::BrowserWindow(Browser::CookieJar& cookie_jar, StringView webdriver_content_ipc_path) : m_cookie_jar(cookie_jar) - , m_webdriver_fd_passing_socket(webdriver_fd_passing_socket) + , m_webdriver_content_ipc_path(webdriver_content_ipc_path) { m_tabs_container = new QTabWidget(this); m_tabs_container->setElideMode(Qt::TextElideMode::ElideRight); @@ -294,7 +294,7 @@ void BrowserWindow::debug_request(DeprecatedString const& request, DeprecatedStr void BrowserWindow::new_tab() { - auto tab = make(this, m_webdriver_fd_passing_socket); + auto tab = make(this, m_webdriver_content_ipc_path); auto tab_ptr = tab.ptr(); m_tabs.append(std::move(tab)); diff --git a/BrowserWindow.h b/BrowserWindow.h index e011205d1cf..f1fb1571204 100644 --- a/BrowserWindow.h +++ b/BrowserWindow.h @@ -25,7 +25,7 @@ class CookieJar; class BrowserWindow : public QMainWindow { Q_OBJECT public: - explicit BrowserWindow(Browser::CookieJar&, int webdriver_fd_passing_socket); + explicit BrowserWindow(Browser::CookieJar&, StringView webdriver_content_ipc_path); WebContentView& view() const { return m_current_tab->view(); } @@ -52,5 +52,5 @@ public slots: Browser::CookieJar& m_cookie_jar; - int m_webdriver_fd_passing_socket { -1 }; + StringView m_webdriver_content_ipc_path; }; diff --git a/Tab.cpp b/Tab.cpp index 3f38f9a7ff3..f96a37fcb69 100644 --- a/Tab.cpp +++ b/Tab.cpp @@ -20,7 +20,7 @@ extern DeprecatedString s_serenity_resource_root; extern Browser::Settings* s_settings; -Tab::Tab(BrowserWindow* window, int webdriver_fd_passing_socket) +Tab::Tab(BrowserWindow* window, StringView webdriver_content_ipc_path) : QWidget(window) , m_window(window) { @@ -28,7 +28,7 @@ Tab::Tab(BrowserWindow* window, int webdriver_fd_passing_socket) m_layout->setSpacing(0); m_layout->setContentsMargins(0, 0, 0, 0); - m_view = new WebContentView(webdriver_fd_passing_socket); + m_view = new WebContentView(webdriver_content_ipc_path); m_toolbar = new QToolBar(this); m_location_edit = new QLineEdit(this); @@ -151,7 +151,7 @@ Tab::Tab(BrowserWindow* window, int webdriver_fd_passing_socket) // // Note we *don't* do this if we are connected to a WebDriver, as the Set URL command may come in very // quickly, and become replaced by this load. - if (webdriver_fd_passing_socket == -1) { + if (!webdriver_content_ipc_path.is_empty()) { m_is_history_navigation = true; m_view->load("about:blank"sv); } diff --git a/Tab.h b/Tab.h index 77ab896c7f0..be25953fbaa 100644 --- a/Tab.h +++ b/Tab.h @@ -22,7 +22,7 @@ class BrowserWindow; class Tab final : public QWidget { Q_OBJECT public: - Tab(BrowserWindow* window, int webdriver_fd_passing_socket); + Tab(BrowserWindow* window, StringView webdriver_content_ipc_path); WebContentView& view() { return *m_view; } diff --git a/WebContent/main.cpp b/WebContent/main.cpp index 505e1430219..31aef09d75c 100644 --- a/WebContent/main.cpp +++ b/WebContent/main.cpp @@ -44,24 +44,17 @@ struct DeferredInvokerQt final : IPC::DeferredInvoker { } }; -template -static ErrorOr> create_connection_from_passed_socket(int passing_socket_fd, StringView socket_name, QSocketNotifier& notifier, Args&&... args) +template +static void proxy_socket_through_notifier(ClientType& client, QSocketNotifier& notifier) { - auto socket = TRY(Core::take_over_socket_from_system_server(socket_name)); - auto client = TRY(ConnectionType::try_create(move(socket), std::forward(args)...)); - - VERIFY(passing_socket_fd >= 0); - client->set_fd_passing_socket(TRY(Core::Stream::LocalSocket::adopt_fd(passing_socket_fd))); - - notifier.setSocket(client->socket().fd().value()); + notifier.setSocket(client.socket().fd().value()); notifier.setEnabled(true); - QObject::connect(¬ifier, &QSocketNotifier::activated, [client]() mutable { - client->socket().notifier()->on_ready_to_read(); + QObject::connect(¬ifier, &QSocketNotifier::activated, [&client]() mutable { + client.socket().notifier()->on_ready_to_read(); }); - client->set_deferred_invoker(make()); - return client; + client.set_deferred_invoker(make()); } ErrorOr serenity_main(Main::Arguments arguments) @@ -91,20 +84,28 @@ ErrorOr serenity_main(Main::Arguments arguments) dbgln("Failed to load content filters: {}", maybe_content_filter_error.error()); int webcontent_fd_passing_socket { -1 }; - int webdriver_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_fd_passing_socket, "File descriptor of the passing socket for the WebDriver connection", "webdriver-fd-passing-socket", 'd', "webdriver_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); + + auto webcontent_socket = TRY(Core::take_over_socket_from_system_server("WebContent"sv)); + auto webcontent_client = TRY(WebContent::ConnectionFromClient::try_create(move(webcontent_socket))); + webcontent_client->set_fd_passing_socket(TRY(Core::Stream::LocalSocket::adopt_fd(webcontent_fd_passing_socket))); + QSocketNotifier webcontent_notifier(QSocketNotifier::Type::Read); - auto webcontent_client = TRY(create_connection_from_passed_socket(webcontent_fd_passing_socket, "WebContent"sv, webcontent_notifier)); + proxy_socket_through_notifier(*webcontent_client, webcontent_notifier); QSocketNotifier webdriver_notifier(QSocketNotifier::Type::Read); RefPtr webdriver_client; - if (webdriver_fd_passing_socket >= 0) - webdriver_client = TRY(create_connection_from_passed_socket(webdriver_fd_passing_socket, "WebDriver"sv, webdriver_notifier, webcontent_client->page_host())); + 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); + } return app.exec(); } diff --git a/WebContentView.cpp b/WebContentView.cpp index 5ec47f02321..812b352064d 100644 --- a/WebContentView.cpp +++ b/WebContentView.cpp @@ -55,8 +55,8 @@ #include #include -WebContentView::WebContentView(int webdriver_fd_passing_socket) - : m_webdriver_fd_passing_socket(webdriver_fd_passing_socket) +WebContentView::WebContentView(StringView webdriver_content_ipc_path) + : m_webdriver_content_ipc_path(webdriver_content_ipc_path) { setMouseTracking(true); @@ -588,30 +588,30 @@ void WebContentView::create_client() MUST(Core::System::close(ui_fd_passing_fd)); MUST(Core::System::close(ui_fd)); - DeprecatedString takeover_string; - if (auto* socket_takeover = getenv("SOCKET_TAKEOVER")) - takeover_string = DeprecatedString::formatted("{} WebContent:{}", socket_takeover, wc_fd); - else - takeover_string = DeprecatedString::formatted("WebContent:{}", wc_fd); + auto takeover_string = DeprecatedString::formatted("WebContent:{}", wc_fd); MUST(Core::System::setenv("SOCKET_TAKEOVER"sv, takeover_string, true)); auto webcontent_fd_passing_socket_string = DeprecatedString::number(wc_fd_passing_fd); - auto webdriver_fd_passing_socket_string = DeprecatedString::number(m_webdriver_fd_passing_socket); - - char const* argv[] = { - "WebContent", - "--webcontent-fd-passing-socket", - webcontent_fd_passing_socket_string.characters(), - "--webdriver-fd-passing-socket", - webdriver_fd_passing_socket_string.characters(), - nullptr, + + Vector arguments { + "WebContent"sv, + "--webcontent-fd-passing-socket"sv, + webcontent_fd_passing_socket_string }; - auto rc = execvp("./WebContent/WebContent", const_cast(argv)); - if (rc < 0) - rc = execvp((QCoreApplication::applicationDirPath() + "/WebContent").toStdString().c_str(), const_cast(argv)); - if (rc < 0) - perror("execvp"); + if (!m_webdriver_content_ipc_path.is_empty()) { + arguments.append("--webdriver-content-path"sv); + arguments.append(m_webdriver_content_ipc_path); + } + + auto result = Core::System::exec("./WebContent/WebContent"sv, arguments, Core::System::SearchInPath::Yes); + if (result.is_error()) { + auto web_content_path = ak_deprecated_string_from_qstring(QCoreApplication::applicationDirPath() + "/WebContent"); + result = Core::System::exec(web_content_path, arguments, Core::System::SearchInPath::Yes); + } + + if (result.is_error()) + warnln("Could not launch WebContent: {}", result.error()); VERIFY_NOT_REACHED(); } diff --git a/WebContentView.h b/WebContentView.h index bcbee1e6fdc..49e011f98f3 100644 --- a/WebContentView.h +++ b/WebContentView.h @@ -48,7 +48,7 @@ class WebContentView final , public WebView::ViewImplementation { Q_OBJECT public: - explicit WebContentView(int webdriver_fd_passing_socket); + explicit WebContentView(StringView webdriver_content_ipc_path); virtual ~WebContentView() override; void load(AK::URL const&); @@ -219,5 +219,5 @@ class WebContentView final RefPtr m_backup_bitmap; - int m_webdriver_fd_passing_socket { -1 }; + StringView m_webdriver_content_ipc_path; }; diff --git a/WebDriver/CMakeLists.txt b/WebDriver/CMakeLists.txt index ea0d3c07e9f..1719ac2a59e 100644 --- a/WebDriver/CMakeLists.txt +++ b/WebDriver/CMakeLists.txt @@ -2,9 +2,9 @@ set(WEBDRIVER_SOURCE_DIR ${SERENITY_SOURCE_DIR}/Userland/Services/WebDriver) set(SOURCES ${WEBDRIVER_SOURCE_DIR}/Client.cpp + ${WEBDRIVER_SOURCE_DIR}/Session.cpp ${WEBDRIVER_SOURCE_DIR}/WebContentConnection.cpp ../Utilities.cpp - Session.cpp main.cpp ) diff --git a/WebDriver/Session.cpp b/WebDriver/Session.cpp deleted file mode 100644 index 2b3216d591d..00000000000 --- a/WebDriver/Session.cpp +++ /dev/null @@ -1,130 +0,0 @@ -/* - * Copyright (c) 2022, Florent Castelli - * Copyright (c) 2022, Sam Atkins - * Copyright (c) 2022, Tobias Christiansen - * Copyright (c) 2022, Linus Groh - * Copyright (c) 2022, Tim Flynn - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#define AK_DONT_REPLACE_STD - -#include "Session.h" -#include "../Utilities.h" -#include -#include -#include -#include - -namespace WebDriver { - -Session::Session(unsigned session_id, NonnullRefPtr client, Web::WebDriver::LadybirdOptions options) - : m_client(move(client)) - , m_options(move(options)) - , m_id(session_id) -{ -} - -Session::~Session() -{ - if (auto error = stop(); error.is_error()) - warnln("Failed to stop session {}: {}", m_id, error.error()); -} - -ErrorOr Session::start() -{ - int socket_fds[2] {}; - TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, socket_fds)); - auto [webdriver_fd, webcontent_fd] = socket_fds; - - int fd_passing_socket_fds[2] {}; - TRY(Core::System::socketpair(AF_LOCAL, SOCK_STREAM, 0, fd_passing_socket_fds)); - auto [webdriver_fd_passing_fd, webcontent_fd_passing_fd] = fd_passing_socket_fds; - - m_browser_pid = TRY(Core::System::fork()); - - if (m_browser_pid == 0) { - TRY(Core::System::close(webdriver_fd_passing_fd)); - TRY(Core::System::close(webdriver_fd)); - - auto takeover_string = DeprecatedString::formatted("WebDriver:{}", webcontent_fd); - TRY(Core::System::setenv("SOCKET_TAKEOVER"sv, takeover_string, true)); - - auto fd_passing_socket_string = DeprecatedString::number(webcontent_fd_passing_fd); - - if (m_options.headless) { - auto resouces = DeprecatedString::formatted("{}/res", s_serenity_resource_root); - auto error_page = DeprecatedString::formatted("{}/res/html/error.html", s_serenity_resource_root); - auto certs = DeprecatedString::formatted("{}/etc/ca_certs.ini", s_serenity_resource_root); - - char const* argv[] = { - "headless-browser", - "--resources", - resouces.characters(), - "--error-page", - error_page.characters(), - "--certs", - certs.characters(), - "--webdriver-fd-passing-socket", - fd_passing_socket_string.characters(), - "about:blank", - nullptr, - }; - - if (execvp("./_deps/lagom-build/headless-browser", const_cast(argv)) < 0) - perror("execvp"); - } else { - char const* argv[] = { - "ladybird", - "--webdriver-fd-passing-socket", - fd_passing_socket_string.characters(), - nullptr, - }; - - if (execvp("./ladybird", const_cast(argv)) < 0) - perror("execvp"); - } - - VERIFY_NOT_REACHED(); - } - - TRY(Core::System::close(webcontent_fd_passing_fd)); - TRY(Core::System::close(webcontent_fd)); - - auto socket = TRY(Core::Stream::LocalSocket::adopt_fd(webdriver_fd)); - TRY(socket->set_blocking(true)); - - m_web_content_connection = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) WebContentConnection(move(socket), m_client, session_id()))); - m_web_content_connection->set_fd_passing_socket(TRY(Core::Stream::LocalSocket::adopt_fd(webdriver_fd_passing_fd))); - - m_started = true; - return {}; -} - -// https://w3c.github.io/webdriver/#dfn-close-the-session -Web::WebDriver::Response Session::stop() -{ - if (!m_started) - return JsonValue {}; - - // 1. Perform the following substeps based on the remote end’s type: - // NOTE: We perform the "Remote end is an endpoint node" steps in the WebContent process. - m_web_content_connection->close_session(); - - // 2. Remove the current session from active sessions. - // NOTE: Handled by WebDriver::Client. - - // 3. Perform any implementation-specific cleanup steps. - if (m_browser_pid.has_value()) { - MUST(Core::System::kill(*m_browser_pid, SIGTERM)); - m_browser_pid = {}; - } - - m_started = false; - - // 4. If an error has occurred in any of the steps above, return the error, otherwise return success with data null. - return JsonValue {}; -} - -} diff --git a/WebDriver/Session.h b/WebDriver/Session.h deleted file mode 100644 index 0f747b44ae8..00000000000 --- a/WebDriver/Session.h +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (c) 2022, Florent Castelli - * Copyright (c) 2022, Linus Groh - * Copyright (c) 2022, Tim Flynn - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include -#include -#include -#include - -namespace WebDriver { - -class Session { -public: - Session(unsigned session_id, NonnullRefPtr client, Web::WebDriver::LadybirdOptions options); - ~Session(); - - unsigned session_id() const { return m_id; } - - WebContentConnection& web_content_connection() - { - VERIFY(m_web_content_connection); - return *m_web_content_connection; - } - - ErrorOr start(); - Web::WebDriver::Response stop(); - -private: - NonnullRefPtr m_client; - Web::WebDriver::LadybirdOptions m_options; - - bool m_started { false }; - unsigned m_id { 0 }; - - RefPtr m_web_content_connection; - Optional m_browser_pid; -}; - -} diff --git a/WebDriver/main.cpp b/WebDriver/main.cpp index 7a88e4fd6e6..96c62daacdb 100644 --- a/WebDriver/main.cpp +++ b/WebDriver/main.cpp @@ -8,7 +8,9 @@ #include "../Utilities.h" #include +#include #include +#include #include #include #include @@ -16,6 +18,41 @@ extern DeprecatedString s_serenity_resource_root; +static ErrorOr launch_browser(DeprecatedString const& socket_path) +{ + char const* argv[] = { + "ladybird", + "--webdriver-content-path", + socket_path.characters(), + nullptr, + }; + + return Core::System::posix_spawn("./ladybird"sv, nullptr, nullptr, const_cast(argv), environ); +} + +static ErrorOr launch_headless_browser(DeprecatedString const& socket_path) +{ + auto resouces = DeprecatedString::formatted("{}/res", s_serenity_resource_root); + auto error_page = DeprecatedString::formatted("{}/res/html/error.html", s_serenity_resource_root); + auto certs = DeprecatedString::formatted("{}/etc/ca_certs.ini", s_serenity_resource_root); + + char const* argv[] = { + "headless-browser", + "--resources", + resouces.characters(), + "--error-page", + error_page.characters(), + "--certs", + certs.characters(), + "--webdriver-ipc-path", + socket_path.characters(), + "about:blank", + nullptr, + }; + + return Core::System::posix_spawn("./_deps/lagom-build/headless-browser"sv, nullptr, nullptr, const_cast(argv), environ); +} + ErrorOr serenity_main(Main::Arguments arguments) { auto listen_address = "0.0.0.0"sv; @@ -39,6 +76,9 @@ ErrorOr serenity_main(Main::Arguments arguments) platform_init(); + auto webdriver_socket_path = DeprecatedString::formatted("{}/webdriver", TRY(Core::StandardPaths::runtime_directory())); + TRY(Core::Directory::create(webdriver_socket_path, Core::Directory::CreateDirectories::Yes)); + Core::EventLoop loop; auto server = TRY(Core::TCPServer::try_create()); @@ -56,7 +96,7 @@ ErrorOr serenity_main(Main::Arguments arguments) return; } - auto maybe_client = WebDriver::Client::try_create(maybe_buffered_socket.release_value(), server); + auto maybe_client = WebDriver::Client::try_create(maybe_buffered_socket.release_value(), { launch_browser, launch_headless_browser }, server); if (maybe_client.is_error()) { warnln("Could not create a WebDriver client: {}", maybe_client.error()); return; diff --git a/main.cpp b/main.cpp index b9d4a097440..e87e73dc3bb 100644 --- a/main.cpp +++ b/main.cpp @@ -67,17 +67,17 @@ ErrorOr serenity_main(Main::Arguments arguments) Gfx::FontDatabase::set_fixed_width_font_query("Csilla 10 400 0"); StringView raw_url; - int webdriver_fd_passing_socket { -1 }; + StringView webdriver_content_ipc_path; Core::ArgsParser args_parser; args_parser.set_general_help("The Ladybird web browser :^)"); args_parser.add_positional_argument(raw_url, "URL to open", "url", Core::ArgsParser::Required::No); - args_parser.add_option(webdriver_fd_passing_socket, "File descriptor of the passing socket for the WebDriver connection", "webdriver-fd-passing-socket", 'd', "webdriver_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); auto cookie_jar = TRY(Browser::CookieJar::create(*database)); - BrowserWindow window(cookie_jar, webdriver_fd_passing_socket); + BrowserWindow window(cookie_jar, webdriver_content_ipc_path); s_settings = new Browser::Settings(&window); window.setWindowTitle("Ladybird"); window.resize(800, 600);