From 43dc0f52a6b08265aea446f704621cfd3e0507fe Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Sun, 19 Jan 2025 13:13:59 -0500 Subject: [PATCH] LibWeb: Do not run microtasks when the event loop is paused For example, running `alert(1)` will pause the event loop, during which time no JavaScript should execute. This patch extends this disruption to microtasks. This avoids a crash inside the microtask executor, which asserts the JS execution context stack is empty. This makes us behave the same as Firefox in the following page: Before the aforementioned assertion was added, we would execute that microtask before showing the alert. Firefox does not do this, and now we don't either. --- Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp | 6 ++- Tests/LibWeb/Text/expected/alert.txt | 1 + Tests/LibWeb/Text/input/alert.html | 7 +++ UI/Headless/Test.cpp | 49 +++++++++++-------- 4 files changed, 42 insertions(+), 21 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/alert.txt create mode 100644 Tests/LibWeb/Text/input/alert.html diff --git a/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp b/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp index 13cc1bca9b..4e981fad1f 100644 --- a/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp +++ b/Libraries/LibWeb/HTML/EventLoop/EventLoop.cpp @@ -503,6 +503,9 @@ void perform_a_microtask_checkpoint() // https://html.spec.whatwg.org/#perform-a-microtask-checkpoint void EventLoop::perform_a_microtask_checkpoint() { + if (execution_paused()) + return; + // NOTE: This assertion is per requirement 9.5 of the ECMA-262 spec, see: https://tc39.es/ecma262/#sec-jobs // > At some future point in time, when there is no running context in the agent for which the job is scheduled and that agent's execution context stack is empty... VERIFY(vm().execution_context_stack().is_empty()); @@ -653,6 +656,8 @@ EventLoop::PauseHandle::~PauseHandle() // https://html.spec.whatwg.org/multipage/webappapis.html#pause EventLoop::PauseHandle EventLoop::pause() { + m_execution_paused = true; + // 1. Let global be the current global object. auto& global = current_principal_global_object(); @@ -667,7 +672,6 @@ EventLoop::PauseHandle EventLoop::pause() // not run further tasks, and any script in the currently running task must block. User agents should remain // responsive to user input while paused, however, albeit in a reduced capacity since the event loop will not be // doing anything. - m_execution_paused = true; return PauseHandle { *this, global, time_before_pause }; } diff --git a/Tests/LibWeb/Text/expected/alert.txt b/Tests/LibWeb/Text/expected/alert.txt new file mode 100644 index 0000000000..aaecaf93c4 --- /dev/null +++ b/Tests/LibWeb/Text/expected/alert.txt @@ -0,0 +1 @@ +PASS (didn't crash) diff --git a/Tests/LibWeb/Text/input/alert.html b/Tests/LibWeb/Text/input/alert.html new file mode 100644 index 0000000000..94d51ad8ec --- /dev/null +++ b/Tests/LibWeb/Text/input/alert.html @@ -0,0 +1,7 @@ + + diff --git a/UI/Headless/Test.cpp b/UI/Headless/Test.cpp index aece2eeb33..0ddd599a65 100644 --- a/UI/Headless/Test.cpp +++ b/UI/Headless/Test.cpp @@ -379,6 +379,30 @@ static void run_test(HeadlessWebView& view, Test& test, Application& app) view.on_text_test_finish = {}; + promise->when_resolved([&view, &test, &app](auto) { + auto url = URL::create_with_file_scheme(MUST(FileSystem::real_path(test.input_path))); + + switch (test.mode) { + case TestMode::Text: + case TestMode::Layout: + run_dump_test(view, test, url, app.per_test_timeout_in_seconds * 1000); + return; + case TestMode::Ref: + run_ref_test(view, test, url, app.per_test_timeout_in_seconds * 1000); + return; + case TestMode::Crash: + run_dump_test(view, test, url, app.per_test_timeout_in_seconds * 1000); + return; + } + + VERIFY_NOT_REACHED(); + }); + + view.load("about:blank"sv); +} + +static void set_ui_callbacks_for_tests(HeadlessWebView& view) +{ view.on_request_file_picker = [&](auto const& accepted_file_types, auto allow_multiple_files) { // Create some dummy files for tests. Vector selected_files; @@ -420,26 +444,10 @@ static void run_test(HeadlessWebView& view, Test& test, Application& app) view.file_picker_closed(move(selected_files)); }; - promise->when_resolved([&view, &test, &app](auto) { - auto url = URL::create_with_file_scheme(MUST(FileSystem::real_path(test.input_path))); - - switch (test.mode) { - case TestMode::Text: - case TestMode::Layout: - run_dump_test(view, test, url, app.per_test_timeout_in_seconds * 1000); - return; - case TestMode::Ref: - run_ref_test(view, test, url, app.per_test_timeout_in_seconds * 1000); - return; - case TestMode::Crash: - run_dump_test(view, test, url, app.per_test_timeout_in_seconds * 1000); - return; - } - - VERIFY_NOT_REACHED(); - }); - - view.load("about:blank"sv); + view.on_request_alert = [&](auto const&) { + // For tests, just close the alert right away to unblock JS execution. + view.alert_closed(); + }; } ErrorOr run_tests(Core::AnonymousBuffer const& theme, Web::DevicePixelSize window_size) @@ -516,6 +524,7 @@ 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(); auto run_next_test = [&]() {