From 3894d8efec8fff16e9c5eb6d3e54e30dbb7019be Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 6 Jun 2025 07:16:28 -0400 Subject: [PATCH] headless-browser: Don't go through Application to create web views We currently create web views through the headless Application, so that the Application can store the views for easy iteration/management. We would like to move HeadlessWebView into LibWebView to make headlessness a feature of the primary Ladybird binaries. In order to do so, we need to remove this indirection, as we won't have this test-only Application class in Ladybird. --- UI/Headless/Application.cpp | 26 +------------------------- UI/Headless/Application.h | 20 +------------------- UI/Headless/HeadlessWebView.cpp | 12 +++++------- UI/Headless/HeadlessWebView.h | 2 ++ UI/Headless/Test.cpp | 33 ++++++++++++++++++--------------- UI/Headless/main.cpp | 8 ++++---- 6 files changed, 31 insertions(+), 70 deletions(-) diff --git a/UI/Headless/Application.cpp b/UI/Headless/Application.cpp index 9c732e7ccfb..72fabdce8b8 100644 --- a/UI/Headless/Application.cpp +++ b/UI/Headless/Application.cpp @@ -1,18 +1,15 @@ /* - * Copyright (c) 2024, Tim Flynn + * Copyright (c) 2024-2025, Tim Flynn * * SPDX-License-Identifier: BSD-2-Clause */ -#include #include #include #include -#include #include #include #include -#include namespace Ladybird { @@ -127,25 +124,4 @@ ErrorOr Application::launch_test_fixtures() return {}; } -HeadlessWebView& Application::create_web_view(Core::AnonymousBuffer theme, Web::DevicePixelSize window_size) -{ - auto web_view = HeadlessWebView::create(move(theme), window_size); - m_web_views.append(move(web_view)); - - return *m_web_views.last(); -} - -HeadlessWebView& Application::create_child_web_view(HeadlessWebView& parent, u64 page_index) -{ - auto web_view = HeadlessWebView::create_child(parent, page_index); - m_web_views.append(move(web_view)); - - return *m_web_views.last(); -} - -void Application::destroy_web_views() -{ - m_web_views.clear(); -} - } diff --git a/UI/Headless/Application.h b/UI/Headless/Application.h index 279cb179c27..4386bfd07b9 100644 --- a/UI/Headless/Application.h +++ b/UI/Headless/Application.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Tim Flynn + * Copyright (c) 2024-2025, Tim Flynn * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,15 +8,11 @@ #include #include -#include #include -#include #include namespace Ladybird { -class HeadlessWebView; - class Application : public WebView::Application { WEB_VIEW_APPLICATION(Application) @@ -33,17 +29,6 @@ public: ErrorOr launch_test_fixtures(); - HeadlessWebView& create_web_view(Core::AnonymousBuffer theme, Web::DevicePixelSize window_size); - HeadlessWebView& create_child_web_view(HeadlessWebView&, u64 page_index); - void destroy_web_views(); - - template - void for_each_web_view(Callback&& callback) - { - for (auto& web_view : m_web_views) - callback(*web_view); - } - static constexpr u8 VERBOSITY_LEVEL_LOG_TEST_DURATION = 1; static constexpr u8 VERBOSITY_LEVEL_LOG_SLOWEST_TESTS = 2; static constexpr u8 VERBOSITY_LEVEL_LOG_SKIPPED_TESTS = 3; @@ -66,9 +51,6 @@ public: int per_test_timeout_in_seconds { 30 }; int width { 800 }; int height { 600 }; - -private: - Vector> m_web_views; }; } diff --git a/UI/Headless/HeadlessWebView.cpp b/UI/Headless/HeadlessWebView.cpp index c70ecb758ad..47cb9a26bbe 100644 --- a/UI/Headless/HeadlessWebView.cpp +++ b/UI/Headless/HeadlessWebView.cpp @@ -6,7 +6,6 @@ #include #include -#include #include namespace Ladybird { @@ -19,13 +18,12 @@ HeadlessWebView::HeadlessWebView(Core::AnonymousBuffer theme, Web::DevicePixelSi , m_test_promise(TestPromise::construct()) { on_new_web_view = [this](auto, auto, Optional page_index) { - if (page_index.has_value()) { - auto& web_view = Application::the().create_child_web_view(*this, *page_index); - return web_view.handle(); - } + auto web_view = page_index.has_value() + ? HeadlessWebView::create_child(*this, *page_index) + : HeadlessWebView::create(m_theme, m_viewport_size); - auto& web_view = Application::the().create_web_view(m_theme, m_viewport_size); - return web_view.handle(); + m_child_web_views.append(move(web_view)); + return m_child_web_views.last()->handle(); }; on_reposition_window = [this](auto position) { diff --git a/UI/Headless/HeadlessWebView.h b/UI/Headless/HeadlessWebView.h index fcf3bbb6355..ef2bd71f769 100644 --- a/UI/Headless/HeadlessWebView.h +++ b/UI/Headless/HeadlessWebView.h @@ -54,6 +54,8 @@ private: // FIXME: We should implement UI-agnostic platform APIs to interact with the system clipboard. Optional m_clipboard; + + Vector> m_child_web_views; }; } diff --git a/UI/Headless/Test.cpp b/UI/Headless/Test.cpp index 8f2c5d54dfd..78aa85fcd97 100644 --- a/UI/Headless/Test.cpp +++ b/UI/Headless/Test.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2024, Tim Flynn + * Copyright (c) 2024-2025, Tim Flynn * * SPDX-License-Identifier: BSD-2-Clause */ @@ -539,9 +539,14 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize auto concurrency = min(app.test_concurrency, tests.size()); size_t loaded_web_views = 0; + Vector> views; + views.ensure_capacity(concurrency); + for (size_t i = 0; i < concurrency; ++i) { - auto& view = app.create_web_view(theme, window_size); - view.on_load_finish = [&](auto const&) { ++loaded_web_views; }; + auto view = HeadlessWebView::create(theme, window_size); + view->on_load_finish = [&](auto const&) { ++loaded_web_views; }; + + views.unchecked_append(move(view)); } // We need to wait for the initial about:blank load to complete before starting the tests, otherwise we may load the @@ -567,9 +572,9 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize Vector non_passing_tests; - app.for_each_web_view([&](auto& view) { - set_ui_callbacks_for_tests(view); - view.clear_content_filters(); + for (auto& view : views) { + set_ui_callbacks_for_tests(*view); + view->clear_content_filters(); auto run_next_test = [&]() { auto index = current_test++; @@ -589,13 +594,13 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize Core::deferred_invoke([&]() mutable { if (s_skipped_tests.contains_slow(test.input_path)) - view.on_test_complete({ test, TestResult::Skipped }); + view->on_test_complete({ test, TestResult::Skipped }); else - run_test(view, test, app); + run_test(*view, test, app); }); }; - view.test_promise().when_resolved([&, run_next_test](auto result) { + view->test_promise().when_resolved([&, run_next_test](auto result) { result.test.end_time = UnixDateTime::now(); if (app.verbosity >= Application::VERBOSITY_LEVEL_LOG_TEST_DURATION) { @@ -636,7 +641,7 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize Core::deferred_invoke([run_next_test]() { run_next_test(); }); - }); + } MUST(all_tests_complete->await()); @@ -672,16 +677,14 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize } if (app.dump_gc_graph) { - app.for_each_web_view([&](auto& view) { - if (auto path = view.dump_gc_graph(); path.is_error()) + for (auto& view : views) { + if (auto path = view->dump_gc_graph(); path.is_error()) warnln("Failed to dump GC graph: {}", path.error()); else outln("GC graph dumped to {}", path.value()); - }); + } } - app.destroy_web_views(); - if (all_tests_ok) return {}; diff --git a/UI/Headless/main.cpp b/UI/Headless/main.cpp index 76ecc566bfc..f6a99c5ee49 100644 --- a/UI/Headless/main.cpp +++ b/UI/Headless/main.cpp @@ -78,22 +78,22 @@ ErrorOr serenity_main(Main::Arguments arguments) return 0; } - auto& view = app->create_web_view(move(theme), window_size); + auto view = Ladybird::HeadlessWebView::create(move(theme), window_size); VERIFY(!WebView::Application::browser_options().urls.is_empty()); auto const& url = WebView::Application::browser_options().urls.first(); if (app->dump_layout_tree || app->dump_text) { Ladybird::Test test { app->dump_layout_tree ? Ladybird::TestMode::Layout : Ladybird::TestMode::Text }; - Ladybird::run_dump_test(view, test, url, app->per_test_timeout_in_seconds * 1000); + Ladybird::run_dump_test(*view, test, url, app->per_test_timeout_in_seconds * 1000); - auto completion = MUST(view.test_promise().await()); + auto completion = MUST(view->test_promise().await()); return completion.result == Ladybird::TestResult::Pass ? 0 : 1; } RefPtr timer; if (!WebView::Application::browser_options().webdriver_content_ipc_path.has_value()) - timer = TRY(load_page_for_screenshot_and_exit(Core::EventLoop::current(), view, url, app->screenshot_timeout, app->screenshot_path)); + timer = TRY(load_page_for_screenshot_and_exit(Core::EventLoop::current(), *view, url, app->screenshot_timeout, app->screenshot_path)); return app->execute(); }