LibWebView+LibCore: Manage process lifecycle using a SIGCHLD handler

This large commit also refactors LibWebView's process handling to use a top-level Application class that uses a new WebView::Process class to encapsulate the IPC-centric nature of each helper process.
ADKaster · Jun 30, 2024 · 3a383d9 · 3a383d9
1 parent f027530
commit 3a383d9
Show file tree

Hide file tree

Showing 24 changed files with 354 additions and 168 deletions.
diff --git a/AK/Debug.h.in b/AK/Debug.h.in
@@ -290,6 +290,10 @@
 #    cmakedefine01 WEBGL_CONTEXT_DEBUG
 #endif
 
+#ifndef WEBVIEW_PROCESS_DEBUG
+#    cmakedefine01 WEBVIEW_PROCESS_DEBUG
+#endif
+
 #ifndef WEB_FETCH_DEBUG
 #    cmakedefine01 WEB_FETCH_DEBUG
 #endif

diff --git a/Ladybird/AppKit/UI/TaskManager.mm b/Ladybird/AppKit/UI/TaskManager.mm
@@ -6,7 +6,7 @@
 
 #include <AK/String.h>
 #include <LibCore/Timer.h>
-#include <LibWebView/ProcessManager.h>
+#include <LibWebView/Application.h>
 
 #import <UI/LadybirdWebView.h>
 #import <UI/TaskManager.h>
@@ -77,8 +77,8 @@ - (instancetype)init
 
 - (void)updateStatistics
 {
-    WebView::ProcessManager::the().update_all_processes();
-    [self.web_view loadHTML:WebView::ProcessManager::the().generate_html()];
+    WebView::Application::the().update_process_statistics();
+    [self.web_view loadHTML:WebView::Application::the().generate_process_statistics_html()];
 }
 
 @end
diff --git a/Ladybird/AppKit/main.mm b/Ladybird/AppKit/main.mm
@@ -10,13 +10,12 @@
 #include <Ladybird/Types.h>
 #include <Ladybird/Utilities.h>
 #include <LibCore/ArgsParser.h>
-#include <LibCore/EventLoop.h>
 #include <LibGfx/Font/FontDatabase.h>
 #include <LibMain/Main.h>
+#include <LibWebView/Application.h>
 #include <LibWebView/ChromeProcess.h>
 #include <LibWebView/CookieJar.h>
 #include <LibWebView/Database.h>
-#include <LibWebView/ProcessManager.h>
 #include <LibWebView/URL.h>
 #include <LibWebView/ViewImplementation.h>
 #include <LibWebView/WebContentClient.h>
@@ -77,7 +76,7 @@ static void open_urls_from_client(Vector<ByteString> const& raw_urls, NewWindow
     Application* application = [Application sharedApplication];
 
     Core::EventLoopManager::install(*new Ladybird::CFEventLoopManager);
-    Core::EventLoop event_loop;
+    WebView::Application web_view_app(arguments.argc, arguments.argv);
 
     platform_init();
 
@@ -118,16 +117,14 @@ static void open_urls_from_client(Vector<ByteString> const& raw_urls, NewWindow
         open_urls_from_client(raw_urls, NewWindow::Yes);
     };
 
-    WebView::ProcessManager::initialize();
-
     auto mach_port_server = make<Ladybird::MachPortServer>();
     set_mach_server_name(mach_port_server->server_port_name());
-    mach_port_server->on_receive_child_mach_port = [](auto pid, auto port) {
-        WebView::ProcessManager::the().add_process(pid, move(port));
+    mach_port_server->on_receive_child_mach_port = [&web_view_app](auto pid, auto port) {
+        web_view_app.set_process_mach_port(pid, move(port));
     };
     mach_port_server->on_receive_backing_stores = [](Ladybird::MachPortServer::BackingStoresMessage message) {
-        auto view = WebView::WebContentClient::view_for_pid_and_page_id(message.pid, message.page_id);
-        view->did_allocate_iosurface_backing_stores(message.front_backing_store_id, move(message.front_backing_store_port), message.back_backing_store_id, move(message.back_backing_store_port));
+        if (auto view = WebView::WebContentClient::view_for_pid_and_page_id(message.pid, message.page_id); view.has_value())
+            view->did_allocate_iosurface_backing_stores(message.front_backing_store_id, move(message.front_backing_store_port), message.back_backing_store_id, move(message.back_backing_store_port));
     };
 
     auto database = TRY(WebView::Database::create());
@@ -158,5 +155,5 @@ static void open_urls_from_client(Vector<ByteString> const& raw_urls, NewWindow
 
     [NSApp setDelegate:delegate];
 
-    return event_loop.exec();
+    return web_view_app.exec();
 }
diff --git a/Ladybird/HelperProcess.cpp b/Ladybird/HelperProcess.cpp
@@ -8,7 +8,7 @@
 #include "Utilities.h"
 #include <AK/Enumerate.h>
 #include <LibCore/Process.h>
-#include <LibWebView/ProcessManager.h>
+#include <LibWebView/Application.h>
 
 template<typename ClientType, typename... ClientArguments>
 static ErrorOr<NonnullRefPtr<ClientType>> launch_server_process(
@@ -45,7 +45,7 @@ static ErrorOr<NonnullRefPtr<ClientType>> launch_server_process(
             if constexpr (requires { process.client->set_pid(pid_t {}); })
                 process.client->set_pid(process.process.pid());
 
-            WebView::ProcessManager::the().add_process(WebView::process_type_from_name(server_name), process.process.pid());
+            WebView::Application::the().add_child_process(WebView::Process { WebView::process_type_from_name(server_name), process.client, move(process.process) });
 
             if (enable_callgrind_profiling == Ladybird::EnableCallgrindProfiling::Yes) {
                 dbgln();

diff --git a/Ladybird/Qt/TaskManagerWindow.cpp b/Ladybird/Qt/TaskManagerWindow.cpp
@@ -5,7 +5,7 @@
  */
 
 #include "TaskManagerWindow.h"
-#include <LibWebView/ProcessManager.h>
+#include <LibWebView/Application.h>
 #include <QVBoxLayout>
 
 namespace Ladybird {
@@ -41,9 +41,8 @@ void TaskManagerWindow::hideEvent(QHideEvent*)
 
 void TaskManagerWindow::update_statistics()
 {
-
-    WebView::ProcessManager::the().update_all_processes();
-    m_web_view->load_html(WebView::ProcessManager::the().generate_html());
+    WebView::Application::the().update_process_statistics();
+    m_web_view->load_html(WebView::Application::the().generate_process_statistics_html());
 }
 
 }
diff --git a/Ladybird/Qt/main.cpp b/Ladybird/Qt/main.cpp
@@ -17,6 +17,7 @@
 #include <LibCore/System.h>
 #include <LibGfx/Font/FontDatabase.h>
 #include <LibMain/Main.h>
+#include <LibWebView/Application.h>
 #include <LibWebView/ChromeProcess.h>
 #include <LibWebView/CookieJar.h>
 #include <LibWebView/Database.h>
@@ -76,8 +77,8 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
     Ladybird::Application app(arguments.argc, arguments.argv);
 
     Core::EventLoopManager::install(*new Ladybird::EventLoopManagerQt);
-    Core::EventLoop event_loop;
-    static_cast<Ladybird::EventLoopImplementationQt&>(event_loop.impl()).set_main_loop();
+    WebView::Application webview_app(arguments.argc, arguments.argv);
+    static_cast<Ladybird::EventLoopImplementationQt&>(Core::EventLoop::current().impl()).set_main_loop();
 
     TRY(handle_attached_debugger());
 
@@ -142,17 +143,15 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
         window.view().load(file_url);
     };
 
-    WebView::ProcessManager::initialize();
-
 #if defined(AK_OS_MACOS)
     auto mach_port_server = make<Ladybird::MachPortServer>();
     set_mach_server_name(mach_port_server->server_port_name());
-    mach_port_server->on_receive_child_mach_port = [](auto pid, auto port) {
-        WebView::ProcessManager::the().add_process(pid, move(port));
+    mach_port_server->on_receive_child_mach_port = [&webview_app](auto pid, auto port) {
+        webview_app.set_process_mach_port(pid, move(port));
     };
     mach_port_server->on_receive_backing_stores = [](Ladybird::MachPortServer::BackingStoresMessage message) {
-        auto view = WebView::WebContentClient::view_for_pid_and_page_id(message.pid, message.page_id);
-        view->did_allocate_iosurface_backing_stores(message.front_backing_store_id, move(message.front_backing_store_port), message.back_backing_store_id, move(message.back_backing_store_port));
+        if (auto view = WebView::WebContentClient::view_for_pid_and_page_id(message.pid, message.page_id); view.has_value())
+            view->did_allocate_iosurface_backing_stores(message.front_backing_store_id, move(message.front_backing_store_port), message.back_backing_store_id, move(message.back_backing_store_port));
     };
 #endif
 
@@ -204,5 +203,5 @@ ErrorOr<int> serenity_main(Main::Arguments arguments)
 
     window.show();
 
-    return event_loop.exec();
+    return webview_app.exec();
 }
diff --git a/Meta/CMake/all_the_debug_macros.cmake b/Meta/CMake/all_the_debug_macros.cmake
@@ -68,6 +68,7 @@ set(WASM_VALIDATOR_DEBUG ON)
 set(WEBDRIVER_DEBUG ON)
 set(WEBDRIVER_ROUTE_DEBUG ON)
 set(WEBGL_CONTEXT_DEBUG ON)
+set(WEBVIEW_PROCESS_DEBUG ON)
 set(WEB_FETCH_DEBUG ON)
 set(WEB_WORKER_DEBUG ON)
 set(WEBP_DEBUG ON)

diff --git a/Userland/Libraries/LibCore/Forward.h b/Userland/Libraries/LibCore/Forward.h
@@ -31,6 +31,7 @@ class MimeData;
 class NetworkJob;
 class NetworkResponse;
 class Notifier;
+class Process;
 class ProcessStatisticsReader;
 class Resource;
 class ResourceImplementation;

diff --git a/Userland/Libraries/LibCore/Platform/ProcessInfo.h b/Userland/Libraries/LibCore/Platform/ProcessInfo.h
@@ -20,8 +20,6 @@ struct ProcessInfo {
     {
     }
 
-    virtual ~ProcessInfo() = default;
-
     pid_t pid { 0 };
 
     u64 memory_usage_bytes { 0 };
@@ -30,12 +28,6 @@ struct ProcessInfo {
     u64 time_spent_in_process { 0 };
 
 #if defined(AK_OS_MACH)
-    ProcessInfo(pid_t pid, Core::MachPort&& port)
-        : pid(pid)
-        , child_task_port(move(port))
-    {
-    }
-
     Core::MachPort child_task_port;
 #endif
 };

diff --git a/Userland/Libraries/LibCore/Platform/ProcessStatistics.h b/Userland/Libraries/LibCore/Platform/ProcessStatistics.h
@@ -15,11 +15,11 @@
 namespace Core::Platform {
 
 struct ProcessStatistics {
-    template<typename ProcessInfoType, typename Callback>
+    template<typename Callback>
     void for_each_process(Callback&& callback)
     {
         for (auto& process : processes)
-            callback(verify_cast<ProcessInfoType>(*process));
+            callback(*process);
     }
 
     u64 total_time_scheduled { 0 };

diff --git a/Userland/Libraries/LibCore/Process.cpp b/Userland/Libraries/LibCore/Process.cpp
@@ -64,6 +64,13 @@ struct ArgvList {
     }
 };
 
+Process Process::current()
+{
+    auto p = Process { getpid() };
+    p.m_should_disown = false;
+    return p;
+}
+
 ErrorOr<Process> Process::spawn(ProcessSpawnOptions const& options)
 {
 #define CHECK(invocation)                  \

diff --git a/Userland/Libraries/LibCore/Process.h b/Userland/Libraries/LibCore/Process.h
@@ -74,6 +74,7 @@ class Process {
     }
 
     static ErrorOr<Process> spawn(ProcessSpawnOptions const& options);
+    static Process current();
 
     // FIXME: Make the following 2 functions return Process instance or delete them.
     static ErrorOr<pid_t> spawn(StringView path, ReadonlySpan<ByteString> arguments, ByteString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No);

diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp
@@ -20,9 +20,6 @@ void Client::die()
         promise->reject(Error::from_string_literal("ImageDecoder disconnected"));
     }
     m_pending_decoded_images.clear();
-
-    if (on_death)
-        on_death();
 }
 
 NonnullRefPtr<Core::Promise<DecodedImage>> Client::decode_image(ReadonlyBytes encoded_data, Function<ErrorOr<void>(DecodedImage&)> on_resolved, Function<void(Error&)> on_rejected, Optional<Gfx::IntSize> ideal_size, Optional<ByteString> mime_type)

diff --git a/Userland/Libraries/LibWebView/Application.cpp b/Userland/Libraries/LibWebView/Application.cpp
@@ -0,0 +1,100 @@
+/*
+ * Copyright (c) 2024, Andrew Kaster <akaster@serenityos.org>
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <AK/Debug.h>
+#include <LibImageDecoderClient/Client.h>
+#include <LibWebView/Application.h>
+#include <LibWebView/WebContentClient.h>
+
+namespace WebView {
+
+Application* Application::s_the = nullptr;
+
+Application::Application(int, char**)
+{
+    VERIFY(!s_the);
+    s_the = this;
+
+    m_process_manager.on_process_exited = [this](Process&& process) {
+        process_did_exit(move(process));
+    };
+}
+
+Application::~Application()
+{
+    s_the = nullptr;
+}
+
+int Application::exec()
+{
+    int ret = m_event_loop.exec();
+    m_in_shutdown = true;
+    return ret;
+}
+
+void Application::add_child_process(WebView::Process&& process)
+{
+    m_process_manager.add_process(move(process));
+}
+
+#if defined(AK_OS_MACH)
+void Application::set_process_mach_port(pid_t pid, Core::MachPort&& port)
+{
+    m_process_manager.set_process_mach_port(pid, move(port));
+}
+#endif
+
+Optional<Process&> Application::find_process(pid_t pid)
+{
+    return m_process_manager.find_process(pid);
+}
+
+void Application::update_process_statistics()
+{
+    m_process_manager.update_all_process_statistics();
+}
+
+String Application::generate_process_statistics_html()
+{
+    return m_process_manager.generate_html();
+}
+
+void Application::process_did_exit(Process&& process)
+{
+    if (m_in_shutdown)
+        return;
+
+    dbgln_if(WEBVIEW_PROCESS_DEBUG, "Process {} died, type: {}", process.pid(), process_name_from_type(process.type()));
+
+    switch (process.type()) {
+    case ProcessType::ImageDecoder:
+        if (auto client = process.client<ImageDecoderClient::Client>(); client.has_value()) {
+            dbgln_if(WEBVIEW_PROCESS_DEBUG, "Restart ImageDecoder process");
+            if (auto on_death = move(client->on_death)) {
+                on_death();
+            }
+        }
+        break;
+    case ProcessType::RequestServer:
+        dbgln_if(WEBVIEW_PROCESS_DEBUG, "FIXME: Restart request server");
+        break;
+    case ProcessType::WebContent:
+        if (auto client = process.client<WebContentClient>(); client.has_value()) {
+            dbgln_if(WEBVIEW_PROCESS_DEBUG, "Restart WebContent process");
+            if (auto on_web_content_process_crash = move(client->on_web_content_process_crash))
+                on_web_content_process_crash();
+        }
+        break;
+    case ProcessType::WebWorker:
+        dbgln_if(WEBVIEW_PROCESS_DEBUG, "WebWorker {} died, not sure what to do.", process.pid());
+        break;
+    case ProcessType::Chrome:
+        dbgln("Invalid process type to be dying: Chrome");
+        VERIFY_NOT_REACHED();
+    }
+}
+
+}