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.
LadybirdBrowser · Dec 15, 2022 · 1dee21e · 1dee21e
1 parent e79d9ec
commit 1dee21e
Show file tree

Hide file tree

Showing 12 changed files with 96 additions and 232 deletions.
diff --git 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<Tab>(this, m_webdriver_fd_passing_socket);
+    auto tab = make<Tab>(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
@@ -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
@@ -20,15 +20,15 @@
 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)
 {
     m_layout = new QBoxLayout(QBoxLayout::Direction::TopToBottom, this);
     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
@@ -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
@@ -44,24 +44,17 @@ struct DeferredInvokerQt final : IPC::DeferredInvoker {
     }
 };
 
-template<typename ConnectionType, typename... Args>
-static ErrorOr<NonnullRefPtr<ConnectionType>> create_connection_from_passed_socket(int passing_socket_fd, StringView socket_name, QSocketNotifier& notifier, Args&&... args)
+template<typename ClientType>
+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>(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(&notifier, &QSocketNotifier::activated, [client]() mutable {
-        client->socket().notifier()->on_ready_to_read();
+    QObject::connect(&notifier, &QSocketNotifier::activated, [&client]() mutable {
+        client.socket().notifier()->on_ready_to_read();
     });
 
-    client->set_deferred_invoker(make<DeferredInvokerQt>());
-    return client;
+    client.set_deferred_invoker(make<DeferredInvokerQt>());
 }
 
 ErrorOr<int> serenity_main(Main::Arguments arguments)
@@ -91,20 +84,28 @@ ErrorOr<int> 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::ConnectionFromClient>(webcontent_fd_passing_socket, "WebContent"sv, webcontent_notifier));
+    proxy_socket_through_notifier(*webcontent_client, webcontent_notifier);
 
     QSocketNotifier webdriver_notifier(QSocketNotifier::Type::Read);
     RefPtr<WebContent::WebDriverConnection> webdriver_client;
-    if (webdriver_fd_passing_socket >= 0)
-        webdriver_client = TRY(create_connection_from_passed_socket<WebContent::WebDriverConnection>(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
@@ -55,8 +55,8 @@
 #include <QTreeView>
 #include <QVBoxLayout>
 
-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<StringView> arguments {
+            "WebContent"sv,
+            "--webcontent-fd-passing-socket"sv,
+            webcontent_fd_passing_socket_string
         };
 
-        auto rc = execvp("./WebContent/WebContent", const_cast<char**>(argv));
-        if (rc < 0)
-            rc = execvp((QCoreApplication::applicationDirPath() + "/WebContent").toStdString().c_str(), const_cast<char**>(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
@@ -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<Gfx::Bitmap> 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
@@ -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