From f57310999dd8d01249d7fd2c2b8fe3a929805d31 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Thu, 6 Jul 2023 07:43:23 -0400 Subject: [PATCH] LibWeb: Update workarounds for the empty execution context stack Use the new helper class to perform this workaround. --- Userland/Libraries/LibWeb/Fetch/Body.cpp | 22 +++---------- .../Libraries/LibWeb/Fetch/FetchMethod.cpp | 12 ++----- Userland/Libraries/LibWeb/FileAPI/Blob.cpp | 10 ++---- .../LibWeb/HTML/HTMLMediaElement.cpp | 25 +++++---------- .../LibWeb/HTML/Scripting/Fetching.cpp | 8 ++--- Userland/Libraries/LibWeb/Page/Page.cpp | 31 +++++-------------- .../LibWeb/WebDriver/ExecuteScript.cpp | 10 ++---- 7 files changed, 30 insertions(+), 88 deletions(-) diff --git a/Userland/Libraries/LibWeb/Fetch/Body.cpp b/Userland/Libraries/LibWeb/Fetch/Body.cpp index 50a5f3bb9a..7596f2a663 100644 --- a/Userland/Libraries/LibWeb/Fetch/Body.cpp +++ b/Userland/Libraries/LibWeb/Fetch/Body.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -165,15 +166,9 @@ WebIDL::ExceptionOr> consume_body(JS::Realm& realm // 3. Let errorSteps given error be to reject promise with error. // NOTE: `promise` and `realm` is protected by JS::SafeFunction. auto error_steps = [promise, &realm](JS::GCPtr error) { - // NOTE: Not part of the spec, but we need to have an execution context on the stack to call native functions. - // (In this case, Promise's reject function) - auto& environment_settings_object = Bindings::host_defined_environment_settings_object(realm); - environment_settings_object.prepare_to_run_script(); - + // AD-HOC: An execution context is required for Promise's reject function. + HTML::TemporaryExecutionContext execution_context { Bindings::host_defined_environment_settings_object(realm) }; WebIDL::reject_promise(realm, promise, error); - - // See above NOTE. - environment_settings_object.clean_up_after_running_script(); }; // 4. Let successSteps given a byte sequence data be to resolve promise with the result of running convertBytesToJSValue @@ -183,15 +178,8 @@ WebIDL::ExceptionOr> consume_body(JS::Realm& realm auto success_steps = [promise, &realm, &object, type](ByteBuffer const& data) { auto& vm = realm.vm(); - // NOTE: Not part of the spec, but we need to have an execution context on the stack to call native functions. - // (In this case, Promise's reject function and JSON.parse) - auto& environment_settings_object = Bindings::host_defined_environment_settings_object(realm); - environment_settings_object.prepare_to_run_script(); - - ScopeGuard guard = [&]() { - // See above NOTE. - environment_settings_object.clean_up_after_running_script(); - }; + // AD-HOC: An execution context is required for Promise's reject function and JSON.parse. + HTML::TemporaryExecutionContext execution_context { Bindings::host_defined_environment_settings_object(realm) }; auto value_or_error = Bindings::throw_dom_exception_if_needed(vm, [&]() -> WebIDL::ExceptionOr { return package_data(realm, data, type, TRY_OR_THROW_OOM(vm, object.mime_type_impl())); diff --git a/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp b/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp index 0a5b4a7e35..28afe8da7d 100644 --- a/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp +++ b/Userland/Libraries/LibWeb/Fetch/FetchMethod.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include @@ -85,15 +86,8 @@ JS::NonnullGCPtr fetch(JS::VM& vm, RequestInfo const& input, Reques if (locally_aborted->value()) return; - // NOTE: Not part of the spec, but we need to have an execution context on the stack to call native functions. - // (In this case, Promise functions) - auto& environment_settings_object = Bindings::host_defined_environment_settings_object(relevant_realm); - environment_settings_object.prepare_to_run_script(); - - ScopeGuard guard = [&]() { - // See above NOTE. - environment_settings_object.clean_up_after_running_script(); - }; + // AD-HOC: An execution context is required for Promise functions. + HTML::TemporaryExecutionContext execution_context { Bindings::host_defined_environment_settings_object(relevant_realm) }; // 2. If response’s aborted flag is set, then: if (response->aborted()) { diff --git a/Userland/Libraries/LibWeb/FileAPI/Blob.cpp b/Userland/Libraries/LibWeb/FileAPI/Blob.cpp index 2ffae24e70..e2205b91cd 100644 --- a/Userland/Libraries/LibWeb/FileAPI/Blob.cpp +++ b/Userland/Libraries/LibWeb/FileAPI/Blob.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -282,14 +283,7 @@ WebIDL::ExceptionOr> Blob::get_stream( // 2. Queue a global task on the file reading task source given blob’s relevant global object to perform the following steps: HTML::queue_global_task(HTML::Task::Source::FileReading, realm.global_object(), [stream, bytes = move(bytes)]() { - // NOTE: Not part of the spec, but we need to have an execution context on the stack to call native functions. - auto& environment_settings_object = Bindings::host_defined_environment_settings_object(stream->realm()); - environment_settings_object.prepare_to_run_script(); - - ScopeGuard guard = [&]() { - // See above NOTE. - environment_settings_object.clean_up_after_running_script(); - }; + HTML::TemporaryExecutionContext execution_context { Bindings::host_defined_environment_settings_object(stream->realm()) }; // 1. If bytes is failure, then error stream with a failure reason and abort these steps. // 2. Let chunk be a new Uint8Array wrapping an ArrayBuffer containing bytes. If creating the ArrayBuffer throws an exception, then error stream with that exception and abort these steps. diff --git a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp index 216f7bd88f..76ca7f30ad 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLMediaElement.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -368,12 +369,8 @@ WebIDL::ExceptionOr HTMLMediaElement::pause() WebIDL::ExceptionOr HTMLMediaElement::toggle_playback() { - // FIXME: This runs from outside the context of any user script, so we do not have a running execution - // context. This pushes one to allow the promise creation hook to run. - auto& environment_settings = document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); - - ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } }; + // AD-HOC: An execution context is required for Promise creation hooks. + TemporaryExecutionContext execution_context { document().relevant_settings_object() }; if (potentially_playing()) TRY(pause()); @@ -1826,17 +1823,13 @@ void HTMLMediaElement::resolve_pending_play_promises(ReadonlySpanrealm(); - // FIXME: This AO runs from the media element task queue, at which point we do not have a running execution - // context. This pushes one to allow the promise resolving hook to run. - auto& environment_settings = document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); + // AD-HOC: An execution context is required for Promise resolving hooks. + TemporaryExecutionContext execution_context { document().relevant_settings_object() }; // To resolve pending play promises for a media element with a list of promises promises, the user agent // must resolve each promise in promises with undefined. for (auto const& promise : promises) WebIDL::resolve_promise(realm, promise, JS::js_undefined()); - - environment_settings.clean_up_after_running_script(); } // https://html.spec.whatwg.org/multipage/media.html#reject-pending-play-promises @@ -1844,17 +1837,13 @@ void HTMLMediaElement::reject_pending_play_promises(ReadonlySpanrealm(); - // FIXME: This AO runs from the media element task queue, at which point we do not have a running execution - // context. This pushes one to allow the promise rejection hook to run. - auto& environment_settings = document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); + // AD-HOC: An execution context is required for Promise rejection hooks. + TemporaryExecutionContext execution_context { document().relevant_settings_object() }; // To reject pending play promises for a media element with a list of promise promises and an exception name // error, the user agent must reject each promise in promises with error. for (auto const& promise : promises) WebIDL::reject_promise(realm, promise, error); - - environment_settings.clean_up_after_running_script(); } WebIDL::ExceptionOr HTMLMediaElement::handle_keydown(Badge, KeyCode key) diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp index 4151fda074..4740160fdd 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Fetching.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -568,9 +569,7 @@ void fetch_descendants_of_and_link_a_module_script(JavaScriptModuleScript& modul return; } - // FIXME: This is an ad-hoc hack to make sure that there's an execution context on the VM stack in case linking throws an exception. - auto& vm = fetch_client_settings_object.vm(); - vm.push_execution_context(fetch_client_settings_object.realm_execution_context()); + TemporaryExecutionContext execution_context { fetch_client_settings_object }; // FIXME: 2. Let parse error be the result of finding the first parse error given result. @@ -591,9 +590,6 @@ void fetch_descendants_of_and_link_a_module_script(JavaScriptModuleScript& modul TODO(); } - // FIXME: This undoes the ad-hoc hack above. - vm.pop_execution_context(); - // 5. Run onComplete given result. on_complete(result); }); diff --git a/Userland/Libraries/LibWeb/Page/Page.cpp b/Userland/Libraries/LibWeb/Page/Page.cpp index 8072c690dd..61a4179d26 100644 --- a/Userland/Libraries/LibWeb/Page/Page.cpp +++ b/Userland/Libraries/LibWeb/Page/Page.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -300,12 +301,8 @@ WebIDL::ExceptionOr Page::toggle_media_play_state() if (!media_element) return {}; - // FIXME: This runs from outside the context of any user script, so we do not have a running execution - // context. This pushes one to allow the promise creation hook to run. - auto& environment_settings = media_element->document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); - - ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } }; + // AD-HOC: An execution context is required for Promise creation hooks. + HTML::TemporaryExecutionContext execution_context { media_element->document().relevant_settings_object() }; if (media_element->potentially_playing()) TRY(media_element->pause()); @@ -321,12 +318,9 @@ void Page::toggle_media_mute_state() if (!media_element) return; - // FIXME: This runs from outside the context of any user script, so we do not have a running execution - // context. This pushes one to allow the promise creation hook to run. - auto& environment_settings = media_element->document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); + // AD-HOC: An execution context is required for Promise creation hooks. + HTML::TemporaryExecutionContext execution_context { media_element->document().relevant_settings_object() }; - ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } }; media_element->set_muted(!media_element->muted()); } @@ -336,12 +330,8 @@ WebIDL::ExceptionOr Page::toggle_media_loop_state() if (!media_element) return {}; - // FIXME: This runs from outside the context of any user script, so we do not have a running execution - // context. This pushes one to allow the promise creation hook to run. - auto& environment_settings = media_element->document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); - - ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } }; + // AD-HOC: An execution context is required for Promise creation hooks. + HTML::TemporaryExecutionContext execution_context { media_element->document().relevant_settings_object() }; if (media_element->has_attribute(HTML::AttributeNames::loop)) media_element->remove_attribute(HTML::AttributeNames::loop); @@ -357,12 +347,7 @@ WebIDL::ExceptionOr Page::toggle_media_controls_state() if (!media_element) return {}; - // FIXME: This runs from outside the context of any user script, so we do not have a running execution - // context. This pushes one to allow the promise creation hook to run. - auto& environment_settings = media_element->document().relevant_settings_object(); - environment_settings.prepare_to_run_script(); - - ScopeGuard guard { [&] { environment_settings.clean_up_after_running_script(); } }; + HTML::TemporaryExecutionContext execution_context { media_element->document().relevant_settings_object() }; if (media_element->has_attribute(HTML::AttributeNames::controls)) media_element->remove_attribute(HTML::AttributeNames::controls); diff --git a/Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp b/Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp index d0f35677e6..86ccc847d3 100644 --- a/Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp +++ b/Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -325,18 +326,13 @@ ExecuteScriptResultSerialized execute_script(Web::Page& page, DeprecatedString c ExecuteScriptResultSerialized execute_async_script(Web::Page& page, DeprecatedString const& body, JS::MarkedVector arguments, Optional const& timeout) { auto* document = page.top_level_browsing_context().active_document(); - auto& settings_object = document->relevant_settings_object(); auto* window = page.top_level_browsing_context().active_window(); auto& realm = window->realm(); auto& vm = window->vm(); auto start = MonotonicTime::now(); - // NOTE: We need to push an execution context in order to make create_resolving_functions() succeed. - vm.push_execution_context(settings_object.realm_execution_context()); - ScopeGuard pop_guard = [&] { - VERIFY(&settings_object.realm_execution_context() == &vm.running_execution_context()); - vm.pop_execution_context(); - }; + // AD-HOC: An execution context is required for Promise creation hooks. + HTML::TemporaryExecutionContext execution_context { document->relevant_settings_object() }; // 4. Let promise be a new Promise. auto promise_capability = WebIDL::create_promise(realm);