From 3a383d9b2098b7a4bbbfde292ddd001577b43ad1 Mon Sep 17 00:00:00 2001 From: Andrew Kaster Date: Sat, 29 Jun 2024 22:24:01 -0600 Subject: [PATCH] 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. --- AK/Debug.h.in | 4 + Ladybird/AppKit/UI/TaskManager.mm | 6 +- Ladybird/AppKit/main.mm | 17 ++- Ladybird/HelperProcess.cpp | 4 +- Ladybird/Qt/TaskManagerWindow.cpp | 7 +- Ladybird/Qt/main.cpp | 17 ++- Meta/CMake/all_the_debug_macros.cmake | 1 + Userland/Libraries/LibCore/Forward.h | 1 + .../Libraries/LibCore/Platform/ProcessInfo.h | 8 -- .../LibCore/Platform/ProcessStatistics.h | 4 +- Userland/Libraries/LibCore/Process.cpp | 7 ++ Userland/Libraries/LibCore/Process.h | 1 + .../LibImageDecoderClient/Client.cpp | 3 - Userland/Libraries/LibWebView/Application.cpp | 100 +++++++++++++++++ Userland/Libraries/LibWebView/Application.h | 51 +++++++++ Userland/Libraries/LibWebView/CMakeLists.txt | 4 +- Userland/Libraries/LibWebView/Process.cpp | 25 +++++ Userland/Libraries/LibWebView/Process.h | 46 ++++++++ Userland/Libraries/LibWebView/ProcessInfo.h | 41 ------- .../Libraries/LibWebView/ProcessManager.cpp | 105 +++++++----------- .../Libraries/LibWebView/ProcessManager.h | 26 +++-- Userland/Libraries/LibWebView/ProcessType.h | 21 ++++ .../Libraries/LibWebView/WebContentClient.cpp | 16 +-- Userland/Utilities/headless-browser.cpp | 7 +- 24 files changed, 354 insertions(+), 168 deletions(-) create mode 100644 Userland/Libraries/LibWebView/Application.cpp create mode 100644 Userland/Libraries/LibWebView/Application.h create mode 100644 Userland/Libraries/LibWebView/Process.cpp create mode 100644 Userland/Libraries/LibWebView/Process.h delete mode 100644 Userland/Libraries/LibWebView/ProcessInfo.h create mode 100644 Userland/Libraries/LibWebView/ProcessType.h diff --git a/AK/Debug.h.in b/AK/Debug.h.in index 03c9da1b5366c..bf6c55a504459 100644 --- 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 index a72b5f134b617..32bb0f16e481f 100644 --- a/Ladybird/AppKit/UI/TaskManager.mm +++ b/Ladybird/AppKit/UI/TaskManager.mm @@ -6,7 +6,7 @@ #include #include -#include +#include #import #import @@ -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 index 9bf170dcc8de2..9a17470e65bcf 100644 --- a/Ladybird/AppKit/main.mm +++ b/Ladybird/AppKit/main.mm @@ -10,13 +10,12 @@ #include #include #include -#include #include #include +#include #include #include #include -#include #include #include #include @@ -77,7 +76,7 @@ static void open_urls_from_client(Vector 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 const& raw_urls, NewWindow open_urls_from_client(raw_urls, NewWindow::Yes); }; - WebView::ProcessManager::initialize(); - auto mach_port_server = make(); 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 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 index 48b77a879f551..26afaef75c061 100644 --- a/Ladybird/HelperProcess.cpp +++ b/Ladybird/HelperProcess.cpp @@ -8,7 +8,7 @@ #include "Utilities.h" #include #include -#include +#include template static ErrorOr> launch_server_process( @@ -45,7 +45,7 @@ static ErrorOr> 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 index 728e03e874ca9..59b07bfeb4101 100644 --- a/Ladybird/Qt/TaskManagerWindow.cpp +++ b/Ladybird/Qt/TaskManagerWindow.cpp @@ -5,7 +5,7 @@ */ #include "TaskManagerWindow.h" -#include +#include #include 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 index 1f0becd0af3ae..3f91044f30e94 100644 --- a/Ladybird/Qt/main.cpp +++ b/Ladybird/Qt/main.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -76,8 +77,8 @@ ErrorOr serenity_main(Main::Arguments arguments) Ladybird::Application app(arguments.argc, arguments.argv); Core::EventLoopManager::install(*new Ladybird::EventLoopManagerQt); - Core::EventLoop event_loop; - static_cast(event_loop.impl()).set_main_loop(); + WebView::Application webview_app(arguments.argc, arguments.argv); + static_cast(Core::EventLoop::current().impl()).set_main_loop(); TRY(handle_attached_debugger()); @@ -142,17 +143,15 @@ ErrorOr serenity_main(Main::Arguments arguments) window.view().load(file_url); }; - WebView::ProcessManager::initialize(); - #if defined(AK_OS_MACOS) auto mach_port_server = make(); 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 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 index 01e6415fe2565..90b4cea1fa7fc 100644 --- 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 index 6805d60ba6d05..d9455edf896a1 100644 --- 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 index 937b1f64c8676..d6a97237340b8 100644 --- 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 index f3b54846bf0b9..ccd51a7313259 100644 --- a/Userland/Libraries/LibCore/Platform/ProcessStatistics.h +++ b/Userland/Libraries/LibCore/Platform/ProcessStatistics.h @@ -15,11 +15,11 @@ namespace Core::Platform { struct ProcessStatistics { - template + template void for_each_process(Callback&& callback) { for (auto& process : processes) - callback(verify_cast(*process)); + callback(*process); } u64 total_time_scheduled { 0 }; diff --git a/Userland/Libraries/LibCore/Process.cpp b/Userland/Libraries/LibCore/Process.cpp index 65d0c8fb095fe..11f988b95f8d1 100644 --- 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::spawn(ProcessSpawnOptions const& options) { #define CHECK(invocation) \ diff --git a/Userland/Libraries/LibCore/Process.h b/Userland/Libraries/LibCore/Process.h index 7b71fcb3115f6..ebb2ca54b074d 100644 --- a/Userland/Libraries/LibCore/Process.h +++ b/Userland/Libraries/LibCore/Process.h @@ -74,6 +74,7 @@ class Process { } static ErrorOr spawn(ProcessSpawnOptions const& options); + static Process current(); // FIXME: Make the following 2 functions return Process instance or delete them. static ErrorOr spawn(StringView path, ReadonlySpan arguments, ByteString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); diff --git a/Userland/Libraries/LibImageDecoderClient/Client.cpp b/Userland/Libraries/LibImageDecoderClient/Client.cpp index b204b14982871..f9f2e23d9d22e 100644 --- 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> Client::decode_image(ReadonlyBytes encoded_data, Function(DecodedImage&)> on_resolved, Function on_rejected, Optional ideal_size, Optional mime_type) diff --git a/Userland/Libraries/LibWebView/Application.cpp b/Userland/Libraries/LibWebView/Application.cpp new file mode 100644 index 0000000000000..7e243b9098652 --- /dev/null +++ b/Userland/Libraries/LibWebView/Application.cpp @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2024, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include + +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 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(); 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(); 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(); + } +} + +} diff --git a/Userland/Libraries/LibWebView/Application.h b/Userland/Libraries/LibWebView/Application.h new file mode 100644 index 0000000000000..3ccbabf66508c --- /dev/null +++ b/Userland/Libraries/LibWebView/Application.h @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2024, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace WebView { + +class Application { + AK_MAKE_NONCOPYABLE(Application); + +public: + Application(int argc, char** argv); + virtual ~Application(); + + int exec(); + + static Application& the() { return *s_the; } + + Core::EventLoop& event_loop() { return m_event_loop; } + + void add_child_process(Process&&); + + // FIXME: Should these methods be part of Application, instead of deferring to ProcessManager? +#if defined(AK_OS_MACH) + void set_process_mach_port(pid_t, Core::MachPort&&); +#endif + Optional find_process(pid_t); + + // FIXME: Should we just expose the ProcessManager via a getter? + void update_process_statistics(); + String generate_process_statistics_html(); + +protected: + virtual void process_did_exit(Process&&); + +private: + static Application* s_the; + + Core::EventLoop m_event_loop; + ProcessManager m_process_manager; + bool m_in_shutdown { false }; +}; + +} diff --git a/Userland/Libraries/LibWebView/CMakeLists.txt b/Userland/Libraries/LibWebView/CMakeLists.txt index ba3bd70ae620c..d649c9bfcb31d 100644 --- a/Userland/Libraries/LibWebView/CMakeLists.txt +++ b/Userland/Libraries/LibWebView/CMakeLists.txt @@ -1,12 +1,14 @@ include(${SerenityOS_SOURCE_DIR}/Meta/CMake/public_suffix.cmake) set(SOURCES + Application.cpp Attribute.cpp ChromeProcess.cpp CookieJar.cpp Database.cpp InspectorClient.cpp ProcessHandle.cpp + Process.cpp ProcessManager.cpp RequestServerAdapter.cpp SearchEngine.cpp @@ -46,7 +48,7 @@ set(GENERATED_SOURCES ) serenity_lib(LibWebView webview) -target_link_libraries(LibWebView PRIVATE LibCore LibFileSystem LibGfx LibIPC LibProtocol LibJS LibWeb LibUnicode LibURL) +target_link_libraries(LibWebView PRIVATE LibCore LibFileSystem LibGfx LibImageDecoderClient LibIPC LibProtocol LibJS LibWeb LibUnicode LibURL) target_compile_definitions(LibWebView PRIVATE ENABLE_PUBLIC_SUFFIX=$) # Third-party diff --git a/Userland/Libraries/LibWebView/Process.cpp b/Userland/Libraries/LibWebView/Process.cpp new file mode 100644 index 0000000000000..49ff8480d73e6 --- /dev/null +++ b/Userland/Libraries/LibWebView/Process.cpp @@ -0,0 +1,25 @@ +/* + * Copyright (c) 2024, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +namespace WebView { + +Process::Process(ProcessType type, RefPtr connection, Core::Process process) + : m_process(move(process)) + , m_type(type) + , m_connection(move(connection)) +{ +} + +Process::~Process() +{ + if (m_connection) + m_connection->shutdown(); +} + +} diff --git a/Userland/Libraries/LibWebView/Process.h b/Userland/Libraries/LibWebView/Process.h new file mode 100644 index 0000000000000..ae9346c7dd040 --- /dev/null +++ b/Userland/Libraries/LibWebView/Process.h @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2024, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include +#include +#include + +namespace WebView { + +class Process { + AK_MAKE_NONCOPYABLE(Process); + AK_MAKE_DEFAULT_MOVABLE(Process); + +public: + Process(ProcessType type, RefPtr connection, Core::Process process); + ~Process(); + + ProcessType type() const { return m_type; } + Optional const& title() const { return m_title; } + void set_title(Optional title) { m_title = move(title); } + + template + Optional client() + { + if (auto strong_connection = m_connection.strong_ref()) + return verify_cast(*strong_connection); + return {}; + } + + pid_t pid() const { return m_process.pid(); } + +private: + Core::Process m_process; + ProcessType m_type; + Optional m_title; + WeakPtr m_connection; +}; + +} diff --git a/Userland/Libraries/LibWebView/ProcessInfo.h b/Userland/Libraries/LibWebView/ProcessInfo.h deleted file mode 100644 index c48cb2a1da506..0000000000000 --- a/Userland/Libraries/LibWebView/ProcessInfo.h +++ /dev/null @@ -1,41 +0,0 @@ -/* - * Copyright (c) 2024, Andrew Kaster - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include -#include -#include - -#if defined(AK_OS_MACH) -# include -#endif - -namespace WebView { - -enum class ProcessType { - Chrome, - WebContent, - WebWorker, - RequestServer, - ImageDecoder, -}; - -struct ProcessInfo : public Core::Platform::ProcessInfo { - using Core::Platform::ProcessInfo::ProcessInfo; - - ProcessInfo(ProcessType type, pid_t pid) - : Core::Platform::ProcessInfo(pid) - , type(type) - { - } - - ProcessType type { ProcessType::WebContent }; - Optional title; -}; - -} diff --git a/Userland/Libraries/LibWebView/ProcessManager.cpp b/Userland/Libraries/LibWebView/ProcessManager.cpp index 33d2c62c48c86..492b18a0066a1 100644 --- a/Userland/Libraries/LibWebView/ProcessManager.cpp +++ b/Userland/Libraries/LibWebView/ProcessManager.cpp @@ -12,8 +12,6 @@ namespace WebView { -static sig_atomic_t s_received_sigchld = 0; - ProcessType process_type_from_name(StringView name) { if (name == "Chrome"sv) @@ -49,96 +47,74 @@ StringView process_name_from_type(ProcessType type) } ProcessManager::ProcessManager() + : on_process_exited([](Process&&) {}) { -} - -ProcessManager::~ProcessManager() -{ -} - -ProcessManager& ProcessManager::the() -{ - static ProcessManager s_the; - return s_the; -} - -void ProcessManager::initialize() -{ - // FIXME: Should we change this to call EventLoop::register_signal? - // Note that only EventLoopImplementationUnix has a working register_signal - - struct sigaction action { }; - action.sa_flags = SA_RESTART; - action.sa_sigaction = [](int, auto*, auto) { - s_received_sigchld = 1; - }; + m_signal_handle = Core::EventLoop::register_signal(SIGCHLD, [this](int) { + auto result = Core::System::waitpid(-1, WNOHANG); + while (!result.is_error() && result.value().pid > 0) { + auto& [pid, status] = result.value(); + if (WIFEXITED(status) || WIFSIGNALED(status)) { + if (auto process = remove_process(pid); process.has_value()) + on_process_exited(process.release_value()); + } + result = Core::System::waitpid(-1, WNOHANG); + } + }); - MUST(Core::System::sigaction(SIGCHLD, &action, nullptr)); + add_process(Process(WebView::ProcessType::Chrome, nullptr, Core::Process::current())); - the().add_process(WebView::ProcessType::Chrome, getpid()); #ifdef AK_OS_MACH auto self_send_port = mach_task_self(); auto res = mach_port_mod_refs(mach_task_self(), self_send_port, MACH_PORT_RIGHT_SEND, +1); VERIFY(res == KERN_SUCCESS); - the().add_process(getpid(), Core::MachPort::adopt_right(self_send_port, Core::MachPort::PortRight::Send)); + set_process_mach_port(getpid(), Core::MachPort::adopt_right(self_send_port, Core::MachPort::PortRight::Send)); #endif } -ProcessInfo* ProcessManager::find_process(pid_t pid) +ProcessManager::~ProcessManager() { - if (auto existing_process = m_statistics.processes.find_if([&](auto& info) { return info->pid == pid; }); !existing_process.is_end()) - return verify_cast(existing_process->ptr()); + Core::EventLoop::unregister_signal(m_signal_handle); +} - return nullptr; +Optional ProcessManager::find_process(pid_t pid) +{ + return m_processes.get(pid); } -void ProcessManager::add_process(ProcessType type, pid_t pid) +void ProcessManager::add_process(WebView::Process&& process) { Threading::MutexLocker locker { m_lock }; - if (auto* existing_process = find_process(pid)) { - existing_process->type = type; - return; - } - m_statistics.processes.append(make(type, pid)); + + auto pid = process.pid(); + auto result = m_processes.set(pid, move(process)); + VERIFY(result == AK::HashSetResult::InsertedNewEntry); + m_statistics.processes.append(make(pid)); } #if defined(AK_OS_MACH) -void ProcessManager::add_process(pid_t pid, Core::MachPort&& port) +void ProcessManager::set_process_mach_port(pid_t pid, Core::MachPort&& port) { Threading::MutexLocker locker { m_lock }; - if (auto* existing_process = find_process(pid)) { - existing_process->child_task_port = move(port); - return; + for (auto const& info : m_statistics.processes) { + if (info->pid == pid) { + info->child_task_port = move(port); + return; + } } - m_statistics.processes.append(make(pid, move(port))); } #endif -void ProcessManager::remove_process(pid_t pid) +Optional ProcessManager::remove_process(pid_t pid) { Threading::MutexLocker locker { m_lock }; m_statistics.processes.remove_first_matching([&](auto const& info) { - if (info->pid == pid) { - return true; - } - return false; + return (info->pid == pid); }); + return m_processes.take(pid); } -void ProcessManager::update_all_processes() +void ProcessManager::update_all_process_statistics() { - if (s_received_sigchld) { - s_received_sigchld = 0; - auto result = Core::System::waitpid(-1, WNOHANG); - while (!result.is_error() && result.value().pid > 0) { - auto& [pid, status] = result.value(); - if (WIFEXITED(status) || WIFSIGNALED(status)) { - remove_process(pid); - } - result = Core::System::waitpid(-1, WNOHANG); - } - } - Threading::MutexLocker locker { m_lock }; (void)update_process_statistics(m_statistics); } @@ -198,12 +174,13 @@ String ProcessManager::generate_html() )"sv); - m_statistics.for_each_process([&](auto const& process) { + m_statistics.for_each_process([&](auto const& process) { builder.append(""sv); builder.append(""sv); - builder.append(WebView::process_name_from_type(process.type)); - if (process.title.has_value()) - builder.appendff(" - {}", escape_html_entities(*process.title)); + auto& process_handle = this->find_process(process.pid).value(); + builder.append(WebView::process_name_from_type(process_handle.type())); + if (process_handle.title().has_value()) + builder.appendff(" - {}", escape_html_entities(*process_handle.title())); builder.append(""sv); builder.append(""sv); builder.append(MUST(String::number(process.pid))); diff --git a/Userland/Libraries/LibWebView/ProcessManager.h b/Userland/Libraries/LibWebView/ProcessManager.h index 87a5ab1ab87f4..21f5bf435fe02 100644 --- a/Userland/Libraries/LibWebView/ProcessManager.h +++ b/Userland/Libraries/LibWebView/ProcessManager.h @@ -12,7 +12,8 @@ #include #include #include -#include +#include +#include namespace WebView { @@ -20,26 +21,29 @@ ProcessType process_type_from_name(StringView); StringView process_name_from_type(ProcessType type); class ProcessManager { + AK_MAKE_NONCOPYABLE(ProcessManager); + public: - static ProcessManager& the(); - static void initialize(); + ProcessManager(); + ~ProcessManager(); - void add_process(WebView::ProcessType, pid_t); - void remove_process(pid_t); - ProcessInfo* find_process(pid_t); + void add_process(Process&&); + Optional remove_process(pid_t); + Optional find_process(pid_t); #if defined(AK_OS_MACH) - void add_process(pid_t, Core::MachPort&&); + void set_process_mach_port(pid_t, Core::MachPort&&); #endif - void update_all_processes(); + void update_all_process_statistics(); String generate_html(); -private: - ProcessManager(); - ~ProcessManager(); + Function on_process_exited; +private: Core::Platform::ProcessStatistics m_statistics; + HashMap m_processes; + int m_signal_handle { -1 }; Threading::Mutex m_lock; }; diff --git a/Userland/Libraries/LibWebView/ProcessType.h b/Userland/Libraries/LibWebView/ProcessType.h new file mode 100644 index 0000000000000..7ab98c8e20d40 --- /dev/null +++ b/Userland/Libraries/LibWebView/ProcessType.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2024, Andrew Kaster + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace WebView { + +enum class ProcessType : u8 { + Chrome, + WebContent, + WebWorker, + RequestServer, + ImageDecoder, +}; + +} diff --git a/Userland/Libraries/LibWebView/WebContentClient.cpp b/Userland/Libraries/LibWebView/WebContentClient.cpp index 3b36a777bcf22..61f36297f7833 100644 --- a/Userland/Libraries/LibWebView/WebContentClient.cpp +++ b/Userland/Libraries/LibWebView/WebContentClient.cpp @@ -5,7 +5,7 @@ */ #include "WebContentClient.h" -#include "ProcessManager.h" +#include "Application.h" #include "ViewImplementation.h" #include @@ -36,8 +36,7 @@ WebContentClient::~WebContentClient() void WebContentClient::die() { - VERIFY(on_web_content_process_crash); - on_web_content_process_crash(); + // Intentionally empty. Restart is handled at another level. } void WebContentClient::register_view(u64 page_id, ViewImplementation& view) @@ -49,6 +48,9 @@ void WebContentClient::register_view(u64 page_id, ViewImplementation& view) void WebContentClient::unregister_view(u64 page_id) { m_views.remove(page_id); + if (m_views.is_empty()) { + on_web_content_process_crash = nullptr; + } } void WebContentClient::did_paint(u64 page_id, Gfx::IntRect const& rect, i32 bitmap_id) @@ -59,8 +61,8 @@ void WebContentClient::did_paint(u64 page_id, Gfx::IntRect const& rect, i32 bitm void WebContentClient::did_start_loading(u64 page_id, URL::URL const& url, bool is_redirect) { - if (auto* process = WebView::ProcessManager::the().find_process(m_process_handle.pid)) - process->title.clear(); + if (auto process = WebView::Application::the().find_process(m_process_handle.pid); process.has_value()) + process->set_title(OptionalNone {}); if (auto view = view_for_page_id(page_id); view.has_value()) { view->set_url({}, url); @@ -143,8 +145,8 @@ void WebContentClient::did_layout(u64 page_id, Gfx::IntSize content_size) void WebContentClient::did_change_title(u64 page_id, ByteString const& title) { - if (auto* process = WebView::ProcessManager::the().find_process(m_process_handle.pid)) - process->title = MUST(String::from_byte_string(title)); + if (auto process = WebView::Application::the().find_process(m_process_handle.pid); process.has_value()) + process->set_title(MUST(String::from_byte_string(title))); if (auto view = view_for_page_id(page_id); view.has_value()) { if (!view->on_title_change) diff --git a/Userland/Utilities/headless-browser.cpp b/Userland/Utilities/headless-browser.cpp index bb1ccaff2a772..6fee9692eed94 100644 --- a/Userland/Utilities/headless-browser.cpp +++ b/Userland/Utilities/headless-browser.cpp @@ -51,6 +51,7 @@ #include #include #include +#include #include #include #include @@ -631,7 +632,7 @@ static ErrorOr run_tests(HeadlessWebContentView& view, StringView test_root ErrorOr serenity_main(Main::Arguments arguments) { - Core::EventLoop event_loop; + WebView::Application app(arguments.argc, arguments.argv); int screenshot_timeout = 1; StringView raw_url; @@ -704,8 +705,8 @@ ErrorOr serenity_main(Main::Arguments arguments) } if (web_driver_ipc_path.is_empty()) { - auto timer = TRY(load_page_for_screenshot_and_exit(event_loop, *view, url.value(), screenshot_timeout)); - return event_loop.exec(); + auto timer = TRY(load_page_for_screenshot_and_exit(Core::EventLoop::current(), *view, url.value(), screenshot_timeout)); + return app.exec(); } return 0;