From 70ce8046c3001e72a38d4ede09a04edc75614707 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 12 Nov 2024 08:50:54 -0500 Subject: [PATCH] headless-browser: Handle WebContent crashes similar to the graphical UIs Instead of bringing the whole browser down, let's re-initialize the WebContent client so we can move on. This is particularly needed for WPT. --- UI/Headless/HeadlessWebView.cpp | 9 +++++--- UI/Headless/HeadlessWebView.h | 3 +++ UI/Headless/Test.cpp | 39 +++++++++++++++++++++++++-------- UI/Headless/Test.h | 3 +++ 4 files changed, 42 insertions(+), 12 deletions(-) diff --git a/UI/Headless/HeadlessWebView.cpp b/UI/Headless/HeadlessWebView.cpp index c4996de5efb..aea811b8889 100644 --- a/UI/Headless/HeadlessWebView.cpp +++ b/UI/Headless/HeadlessWebView.cpp @@ -168,9 +168,12 @@ void HeadlessWebView::initialize_client(CreateNewClient create_new_client) client().async_connect_to_webdriver(m_client_state.page_index, *web_driver_ipc_path); m_client_state.client->on_web_content_process_crash = [this] { - warnln("\033[31;1mWebContent Crashed!!\033[0m"); - warnln(" Last page loaded: {}", url()); - VERIFY_NOT_REACHED(); + Core::deferred_invoke([this] { + handle_web_content_process_crash(LoadErrorPage::No); + + if (on_web_content_crashed) + on_web_content_crashed(); + }); }; } diff --git a/UI/Headless/HeadlessWebView.h b/UI/Headless/HeadlessWebView.h index ed013690c52..b8e043759ef 100644 --- a/UI/Headless/HeadlessWebView.h +++ b/UI/Headless/HeadlessWebView.h @@ -7,6 +7,7 @@ #pragma once #include +#include #include #include #include @@ -31,6 +32,8 @@ public: TestPromise& test_promise() { return *m_test_promise; } void on_test_complete(TestCompletion); + Function on_web_content_crashed; + private: HeadlessWebView(Core::AnonymousBuffer theme, Gfx::IntSize viewport_size); diff --git a/UI/Headless/Test.cpp b/UI/Headless/Test.cpp index 33005543ee4..7a6d7762d74 100644 --- a/UI/Headless/Test.cpp +++ b/UI/Headless/Test.cpp @@ -98,6 +98,13 @@ static ErrorOr collect_ref_tests(Vector& tests, StringView path, Str return {}; } +static void clear_test_callbacks(HeadlessWebView& view) +{ + view.on_load_finish = {}; + view.on_text_test_finish = {}; + view.on_web_content_crashed = {}; +} + void run_dump_test(HeadlessWebView& view, Test& test, URL::URL const& url, int timeout_in_milliseconds) { auto timer = Core::Timer::create_single_shot(timeout_in_milliseconds, [&view, &test]() { @@ -160,17 +167,22 @@ void run_dump_test(HeadlessWebView& view, Test& test, URL::URL const& url, int t }; auto on_test_complete = [&view, &test, timer, handle_completed_test]() { + clear_test_callbacks(view); timer->stop(); - view.on_load_finish = {}; - view.on_text_test_finish = {}; - if (auto result = handle_completed_test(); result.is_error()) view.on_test_complete({ test, TestResult::Fail }); else view.on_test_complete({ test, result.value() }); }; + view.on_web_content_crashed = [&view, &test, timer]() { + clear_test_callbacks(view); + timer->stop(); + + view.on_test_complete({ test, TestResult::Crashed }); + }; + if (test.mode == TestMode::Layout) { view.on_load_finish = [&view, &test, url, on_test_complete = move(on_test_complete)](auto const& loaded_url) { // We don't want subframe loads to trigger the test finish. @@ -257,17 +269,22 @@ static void run_ref_test(HeadlessWebView& view, Test& test, URL::URL const& url, }; auto on_test_complete = [&view, &test, timer, handle_completed_test]() { + clear_test_callbacks(view); timer->stop(); - view.on_load_finish = {}; - view.on_text_test_finish = {}; - if (auto result = handle_completed_test(); result.is_error()) view.on_test_complete({ test, TestResult::Fail }); else view.on_test_complete({ test, result.value() }); }; + view.on_web_content_crashed = [&view, &test, timer]() { + clear_test_callbacks(view); + timer->stop(); + + view.on_test_complete({ test, TestResult::Crashed }); + }; + view.on_load_finish = [&view, &test, on_test_complete = move(on_test_complete)](auto const&) { if (test.actual_screenshot) { view.take_screenshot()->when_resolved([&test, on_test_complete = move(on_test_complete)](RefPtr screenshot) { @@ -418,6 +435,7 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Gfx::IntSize window_ size_t pass_count = 0; size_t fail_count = 0; size_t timeout_count = 0; + size_t crashed_count = 0; size_t skipped_count = 0; bool is_tty = isatty(STDOUT_FILENO); @@ -473,6 +491,9 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Gfx::IntSize window_ case TestResult::Timeout: ++timeout_count; break; + case TestResult::Crashed: + ++crashed_count; + break; case TestResult::Skipped: ++skipped_count; break; @@ -497,9 +518,9 @@ ErrorOr run_tests(Core::AnonymousBuffer const& theme, Gfx::IntSize window_ if (is_tty) outln("\33[2K\rDone!"); - outln("=================================================="); - outln("Pass: {}, Fail: {}, Skipped: {}, Timeout: {}", pass_count, fail_count, skipped_count, timeout_count); - outln("=================================================="); + outln("=========================================================="); + outln("Pass: {}, Fail: {}, Skipped: {}, Timeout: {}, Crashed: {}", pass_count, fail_count, skipped_count, timeout_count, crashed_count); + outln("=========================================================="); for (auto const& non_passing_test : non_passing_tests) { if (non_passing_test.result == TestResult::Skipped && !app.verbose) diff --git a/UI/Headless/Test.h b/UI/Headless/Test.h index ea723462915..85995203c44 100644 --- a/UI/Headless/Test.h +++ b/UI/Headless/Test.h @@ -34,6 +34,7 @@ enum class TestResult { Fail, Skipped, Timeout, + Crashed, }; static constexpr StringView test_result_to_string(TestResult result) @@ -47,6 +48,8 @@ static constexpr StringView test_result_to_string(TestResult result) return "Skipped"sv; case TestResult::Timeout: return "Timeout"sv; + case TestResult::Crashed: + return "Crashed"sv; } VERIFY_NOT_REACHED(); }