diff --git a/Libraries/LibCore/Process.cpp b/Libraries/LibCore/Process.cpp index 3d630a04e9c..73b6f18f8b3 100644 --- a/Libraries/LibCore/Process.cpp +++ b/Libraries/LibCore/Process.cpp @@ -121,7 +121,7 @@ ErrorOr Process::spawn(ProcessSpawnOptions const& options) return Process { pid }; } -ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, ByteString working_directory, KeepAsChild keep_as_child) +ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, ByteString working_directory, KeepAsChild keep_as_child) { auto process = TRY(spawn({ .executable = path, @@ -131,14 +131,11 @@ ErrorOr Process::spawn(StringView path, ReadonlySpan argument if (keep_as_child == KeepAsChild::No) TRY(process.disown()); - else { - // FIXME: This won't be needed if return value is changed to Process. - process.m_should_disown = false; - } - return process.pid(); + + return process; } -ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, ByteString working_directory, KeepAsChild keep_as_child) +ErrorOr Process::spawn(StringView path, ReadonlySpan arguments, ByteString working_directory, KeepAsChild keep_as_child) { Vector backing_strings; backing_strings.ensure_capacity(arguments.size()); @@ -153,9 +150,8 @@ ErrorOr Process::spawn(StringView path, ReadonlySpan argument if (keep_as_child == KeepAsChild::No) TRY(process.disown()); - else - process.m_should_disown = false; - return process.pid(); + + return process; } ErrorOr Process::get_name() diff --git a/Libraries/LibCore/Process.h b/Libraries/LibCore/Process.h index 8a63825f727..23deabbd5d2 100644 --- a/Libraries/LibCore/Process.h +++ b/Libraries/LibCore/Process.h @@ -74,9 +74,8 @@ public: 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); - static ErrorOr spawn(StringView path, ReadonlySpan arguments, ByteString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); + static ErrorOr spawn(StringView path, ReadonlySpan arguments, ByteString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); + static ErrorOr spawn(StringView path, ReadonlySpan arguments, ByteString working_directory = {}, KeepAsChild keep_as_child = KeepAsChild::No); static ErrorOr get_name(); enum class SetThreadName { @@ -96,7 +95,7 @@ public: ErrorOr wait_for_termination(); private: - Process(pid_t pid) + Process(pid_t pid = -1) : m_pid(pid) , m_should_disown(true) { diff --git a/Services/WebDriver/Client.h b/Services/WebDriver/Client.h index 573806abb5e..2030d556617 100644 --- a/Services/WebDriver/Client.h +++ b/Services/WebDriver/Client.h @@ -19,8 +19,8 @@ namespace WebDriver { struct LaunchBrowserCallbacks { - Function(ByteString const&)> launch_browser; - Function(ByteString const&)> launch_headless_browser; + Function(ByteString const&)> launch_browser; + Function(ByteString const&)> launch_headless_browser; }; class Client final : public Web::WebDriver::Client { diff --git a/Services/WebDriver/Session.cpp b/Services/WebDriver/Session.cpp index 4cfc160b146..391182e4da0 100644 --- a/Services/WebDriver/Session.cpp +++ b/Services/WebDriver/Session.cpp @@ -43,10 +43,9 @@ Session::~Session() // from active sessions // 3. Perform any implementation-specific cleanup steps. - if (m_browser_pid.has_value()) { - MUST(Core::System::kill(*m_browser_pid, SIGTERM)); - m_browser_pid = {}; - } + if (m_browser_process.has_value()) + MUST(Core::System::kill(m_browser_process->pid(), SIGTERM)); + if (m_web_content_socket_path.has_value()) { MUST(Core::System::unlink(*m_web_content_socket_path)); m_web_content_socket_path = {}; @@ -172,9 +171,9 @@ ErrorOr Session::start(LaunchBrowserCallbacks const& callbacks) m_web_content_server = TRY(create_server(promise)); if (m_options.headless) - m_browser_pid = TRY(callbacks.launch_headless_browser(*m_web_content_socket_path)); + m_browser_process = TRY(callbacks.launch_headless_browser(*m_web_content_socket_path)); else - m_browser_pid = TRY(callbacks.launch_browser(*m_web_content_socket_path)); + m_browser_process = TRY(callbacks.launch_browser(*m_web_content_socket_path)); // FIXME: Allow this to be more asynchronous. For now, this at least allows us to propagate // errors received while accepting the Browser and WebContent sockets. diff --git a/Services/WebDriver/Session.h b/Services/WebDriver/Session.h index 6c8bd05e212..63c2a740324 100644 --- a/Services/WebDriver/Session.h +++ b/Services/WebDriver/Session.h @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -95,7 +96,7 @@ private: String m_current_window_handle; Optional m_web_content_socket_path; - Optional m_browser_pid; + Optional m_browser_process; RefPtr m_web_content_server; diff --git a/Services/WebDriver/main.cpp b/Services/WebDriver/main.cpp index 088311c54d4..4bf0d61c7b7 100644 --- a/Services/WebDriver/main.cpp +++ b/Services/WebDriver/main.cpp @@ -19,11 +19,11 @@ static Vector certificates; -static ErrorOr launch_process(StringView application, ReadonlySpan arguments) +static ErrorOr launch_process(StringView application, ReadonlySpan arguments) { auto paths = TRY(WebView::get_paths_for_helper_process(application)); - ErrorOr result = -1; + ErrorOr result = Error::from_string_literal("All paths failed to launch"); for (auto const& path : paths) { auto path_view = path.view(); result = Core::Process::spawn(path_view, arguments, {}, Core::Process::KeepAsChild::Yes); @@ -56,13 +56,13 @@ static Vector create_arguments(ByteString const& socket_path, bool f return arguments; } -static ErrorOr launch_browser(ByteString const& socket_path, bool force_cpu_painting) +static ErrorOr launch_browser(ByteString const& socket_path, bool force_cpu_painting) { auto arguments = create_arguments(socket_path, force_cpu_painting); return launch_process("Ladybird"sv, arguments.span()); } -static ErrorOr launch_headless_browser(ByteString const& socket_path, bool force_cpu_painting) +static ErrorOr launch_headless_browser(ByteString const& socket_path, bool force_cpu_painting) { auto arguments = create_arguments(socket_path, force_cpu_painting); return launch_process("headless-browser"sv, arguments.span());