mirror of
https://github.com/LadybirdBrowser/ladybird.git
synced 2025-12-08 06:09:58 +00:00
test-web: Do not clear the WebContent crash handler between tests
Clearing the callback opens a window for the WebContent process to crash while we do not have a callback set. In practice, we see this with ASAN crashes, where the crash occurs after we've already signaled that the test has completed. We now set the crash handler once during init. This required moving the clearing of other callbacks to the test completion handler (we were clearing these callbacks in several different ways anyways, so now we will at least be consistent).
This commit is contained in:
parent
59c963f98f
commit
165afd8ad1
Notes:
github-actions[bot]
2025-11-22 13:10:38 +00:00
Author: https://github.com/trflynn89
Commit: 165afd8ad1
Pull-request: https://github.com/LadybirdBrowser/ladybird/pull/6902
Reviewed-by: https://github.com/gmta ✅
2 changed files with 30 additions and 53 deletions
|
|
@ -14,6 +14,7 @@
|
|||
#include <AK/String.h>
|
||||
#include <AK/Time.h>
|
||||
#include <AK/Vector.h>
|
||||
#include <LibCore/Forward.h>
|
||||
#include <LibCore/Promise.h>
|
||||
#include <LibGfx/Forward.h>
|
||||
|
||||
|
|
@ -59,6 +60,8 @@ struct Test {
|
|||
|
||||
RefPtr<Gfx::Bitmap const> actual_screenshot {};
|
||||
RefPtr<Gfx::Bitmap const> expectation_screenshot {};
|
||||
|
||||
RefPtr<Core::Timer> timeout_timer {};
|
||||
};
|
||||
|
||||
struct TestCompletion {
|
||||
|
|
|
|||
|
|
@ -38,7 +38,7 @@ namespace TestWeb {
|
|||
|
||||
static RefPtr<Core::Promise<Empty>> s_all_tests_complete;
|
||||
static Vector<ByteString> s_skipped_tests;
|
||||
static HashMap<WebView::ViewImplementation const*, Test const*> s_test_by_view;
|
||||
static HashMap<WebView::ViewImplementation const*, Test*> s_test_by_view;
|
||||
|
||||
static constexpr StringView test_result_to_string(TestResult result)
|
||||
{
|
||||
|
|
@ -152,13 +152,6 @@ static ErrorOr<void> collect_crash_tests(Application const& app, Vector<Test>& t
|
|||
return {};
|
||||
}
|
||||
|
||||
static void clear_test_callbacks(TestWebView& view)
|
||||
{
|
||||
view.on_load_finish = {};
|
||||
view.on_test_finish = {};
|
||||
view.on_web_content_crashed = {};
|
||||
}
|
||||
|
||||
static String generate_wait_for_test_string(StringView wait_class)
|
||||
{
|
||||
return MUST(String::formatted(R"(
|
||||
|
|
@ -195,12 +188,7 @@ static auto wait_for_reftest_completion = generate_wait_for_test_string("reftest
|
|||
|
||||
static void run_dump_test(TestWebView& view, Test& test, URL::URL const& url, int timeout_in_milliseconds)
|
||||
{
|
||||
auto timer = Core::Timer::create_single_shot(timeout_in_milliseconds, [&view, &test]() {
|
||||
view.on_load_finish = {};
|
||||
view.on_test_finish = {};
|
||||
view.on_set_test_timeout = {};
|
||||
view.reset_zoom();
|
||||
|
||||
test.timeout_timer = Core::Timer::create_single_shot(timeout_in_milliseconds, [&view, &test]() {
|
||||
view.on_test_complete({ test, TestResult::Timeout });
|
||||
});
|
||||
|
||||
|
|
@ -259,24 +247,13 @@ static void run_dump_test(TestWebView& view, Test& test, URL::URL const& url, in
|
|||
return TestResult::Fail;
|
||||
};
|
||||
|
||||
auto on_test_complete = [&view, &test, timer, handle_completed_test]() {
|
||||
view.reset_zoom();
|
||||
clear_test_callbacks(view);
|
||||
timer->stop();
|
||||
|
||||
auto on_test_complete = [&view, &test, handle_completed_test]() {
|
||||
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.
|
||||
|
|
@ -342,25 +319,18 @@ static void run_dump_test(TestWebView& view, Test& test, URL::URL const& url, in
|
|||
};
|
||||
}
|
||||
|
||||
view.on_set_test_timeout = [timer, timeout_in_milliseconds](double milliseconds) {
|
||||
if (milliseconds <= timeout_in_milliseconds)
|
||||
return;
|
||||
timer->stop();
|
||||
timer->start(milliseconds);
|
||||
view.on_set_test_timeout = [&test, timeout_in_milliseconds](double milliseconds) {
|
||||
if (milliseconds > timeout_in_milliseconds)
|
||||
test.timeout_timer->restart(milliseconds);
|
||||
};
|
||||
|
||||
view.load(url);
|
||||
timer->start();
|
||||
test.timeout_timer->start();
|
||||
}
|
||||
|
||||
static void run_ref_test(TestWebView& view, Test& test, URL::URL const& url, int timeout_in_milliseconds)
|
||||
{
|
||||
auto timer = Core::Timer::create_single_shot(timeout_in_milliseconds, [&view, &test]() {
|
||||
view.on_load_finish = {};
|
||||
view.on_test_finish = {};
|
||||
view.on_set_test_timeout = {};
|
||||
view.reset_zoom();
|
||||
|
||||
view.on_test_complete({ test, TestResult::Timeout });
|
||||
});
|
||||
|
||||
|
|
@ -394,23 +364,13 @@ static void run_ref_test(TestWebView& view, Test& test, URL::URL const& url, int
|
|||
return TestResult::Fail;
|
||||
};
|
||||
|
||||
auto on_test_complete = [&view, &test, timer, handle_completed_test]() {
|
||||
clear_test_callbacks(view);
|
||||
timer->stop();
|
||||
|
||||
auto on_test_complete = [&view, &test, handle_completed_test]() {
|
||||
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](auto const&) {
|
||||
view.run_javascript(wait_for_reftest_completion);
|
||||
};
|
||||
|
|
@ -483,11 +443,9 @@ static void run_ref_test(TestWebView& view, Test& test, URL::URL const& url, int
|
|||
view.load(URL::Parser::basic_parse(reference_to_load).release_value());
|
||||
};
|
||||
|
||||
view.on_set_test_timeout = [timer, timeout_in_milliseconds](double milliseconds) {
|
||||
if (milliseconds <= timeout_in_milliseconds)
|
||||
return;
|
||||
timer->stop();
|
||||
timer->start(milliseconds);
|
||||
view.on_set_test_timeout = [&test, timeout_in_milliseconds](double milliseconds) {
|
||||
if (milliseconds > timeout_in_milliseconds)
|
||||
test.timeout_timer->restart(milliseconds);
|
||||
};
|
||||
|
||||
view.load(url);
|
||||
|
|
@ -580,6 +538,11 @@ static void set_ui_callbacks_for_tests(TestWebView& view)
|
|||
// For tests, just close the alert right away to unblock JS execution.
|
||||
view.alert_closed();
|
||||
};
|
||||
|
||||
view.on_web_content_crashed = [&view]() {
|
||||
if (auto test = s_test_by_view.get(&view); test.has_value())
|
||||
view.on_test_complete({ *test.value(), TestResult::Crashed });
|
||||
};
|
||||
}
|
||||
|
||||
static ErrorOr<int> run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize window_size)
|
||||
|
|
@ -699,6 +662,17 @@ static ErrorOr<int> run_tests(Core::AnonymousBuffer const& theme, Web::DevicePix
|
|||
};
|
||||
|
||||
view->test_promise().when_resolved([&, run_next_test, view_id](auto result) {
|
||||
view->on_load_finish = {};
|
||||
view->on_test_finish = {};
|
||||
view->on_reference_test_metadata = {};
|
||||
view->on_set_test_timeout = {};
|
||||
view->reset_zoom();
|
||||
|
||||
if (result.test.timeout_timer) {
|
||||
result.test.timeout_timer->stop();
|
||||
result.test.timeout_timer.clear();
|
||||
}
|
||||
|
||||
result.test.end_time = UnixDateTime::now();
|
||||
s_test_by_view.remove(view);
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue