From 9422ae9bb2630f65280c5c1a7e3cd748cafe8ce7 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Thu, 17 Mar 2022 22:40:17 +0000 Subject: [PATCH] LibJS: Add infallible variant of VM::push_execution_context() It makes no sense to require passing a global object and doing a stack space check in some cases where running out of stack is highly unlikely, we can't recover from errors, and currently ignore the result anyway. This is most commonly in constructors and when setting things up, rather than regular function calls. --- Tests/Spreadsheet/test-spreadsheet.cpp | 2 +- Userland/Applications/Spreadsheet/Workbook.cpp | 3 +-- Userland/Libraries/LibJS/Bytecode/Interpreter.cpp | 2 +- Userland/Libraries/LibJS/Interpreter.h | 7 +++---- Userland/Libraries/LibJS/Runtime/VM.h | 5 +++++ Userland/Libraries/LibTest/JavaScriptTestRunner.h | 4 ++-- Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp | 4 ++-- Userland/Libraries/LibWeb/DOM/EventTarget.cpp | 5 +++-- Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp | 2 +- Userland/Libraries/LibWeb/HTML/Worker.cpp | 2 +- Userland/Services/WebContent/WebContentConsoleClient.cpp | 2 +- 11 files changed, 21 insertions(+), 17 deletions(-) diff --git a/Tests/Spreadsheet/test-spreadsheet.cpp b/Tests/Spreadsheet/test-spreadsheet.cpp index e3d27895eaf..425aedac182 100644 --- a/Tests/Spreadsheet/test-spreadsheet.cpp +++ b/Tests/Spreadsheet/test-spreadsheet.cpp @@ -27,7 +27,7 @@ TESTJS_RUN_FILE_FUNCTION(String const&, JS::Interpreter& interpreter, JS::Execut } auto script = result.release_value(); - interpreter.vm().push_execution_context(global_execution_context, interpreter.realm().global_object()); + interpreter.vm().push_execution_context(global_execution_context); MUST(interpreter.run(*script)); interpreter.vm().pop_execution_context(); }; diff --git a/Userland/Applications/Spreadsheet/Workbook.cpp b/Userland/Applications/Spreadsheet/Workbook.cpp index d67394e040e..6756b9eafb9 100644 --- a/Userland/Applications/Spreadsheet/Workbook.cpp +++ b/Userland/Applications/Spreadsheet/Workbook.cpp @@ -39,8 +39,7 @@ Workbook::Workbook(NonnullRefPtrVector&& sheets, GUI::Window& parent_wind m_main_execution_context.variable_environment = &m_interpreter->realm().global_environment(); m_main_execution_context.realm = &m_interpreter->realm(); m_main_execution_context.is_strict_mode = true; - MUST(m_vm->push_execution_context(m_main_execution_context, m_interpreter->global_object())); - + m_vm->push_execution_context(m_main_execution_context); m_vm->enable_default_host_import_module_dynamically_hook(); } diff --git a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp index 6d8f7262d0c..5eb5f5109bc 100644 --- a/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Interpreter.cpp @@ -59,7 +59,7 @@ Interpreter::ValueAndFrame Interpreter::run_and_return_frame(Executable const& e execution_context.realm = &m_realm; // FIXME: How do we know if we're in strict mode? Maybe the Bytecode::Block should know this? // execution_context.is_strict_mode = ???; - MUST(vm().push_execution_context(execution_context, global_object())); + vm().push_execution_context(execution_context); pushed_execution_context = true; } diff --git a/Userland/Libraries/LibJS/Interpreter.h b/Userland/Libraries/LibJS/Interpreter.h index 3bd8325e4f0..79ec10edb39 100644 --- a/Userland/Libraries/LibJS/Interpreter.h +++ b/Userland/Libraries/LibJS/Interpreter.h @@ -56,14 +56,13 @@ public: // 5. Set the ScriptOrModule of newContext to null. (This was done during execution context construction) + // 6. Push newContext onto the execution context stack; newContext is now the running execution context. + vm.push_execution_context(interpreter->m_global_execution_context); + // 7. If the host requires use of an exotic object to serve as realm's global object, let global be such an object created in a host-defined manner. // Otherwise, let global be undefined, indicating that an ordinary object should be created as the global object. auto* global_object = static_cast(interpreter->heap().allocate_without_global_object(forward(args)...)); - // 6. Push newContext onto the execution context stack; newContext is now the running execution context. - // NOTE: This is out of order from the spec, but it shouldn't matter here. - vm.push_execution_context(interpreter->m_global_execution_context, *global_object); - // 8. If the host requires that the this binding in realm's global scope return an object other than the global object, let thisValue be such an object created // in a host-defined manner. Otherwise, let thisValue be undefined, indicating that realm's global this binding should be the global object. if constexpr (IsSame) { diff --git a/Userland/Libraries/LibJS/Runtime/VM.h b/Userland/Libraries/LibJS/Runtime/VM.h index b09127599b0..1295c2f01b1 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.h +++ b/Userland/Libraries/LibJS/Runtime/VM.h @@ -90,6 +90,11 @@ public: return m_stack_info.size_free() < 32 * KiB; } + void push_execution_context(ExecutionContext& context) + { + m_execution_context_stack.append(&context); + } + ThrowCompletionOr push_execution_context(ExecutionContext& context, GlobalObject& global_object) { // Ensure we got some stack space left, so the next function call doesn't kill us. diff --git a/Userland/Libraries/LibTest/JavaScriptTestRunner.h b/Userland/Libraries/LibTest/JavaScriptTestRunner.h index c10bc05049d..7c0cb92d7cb 100644 --- a/Userland/Libraries/LibTest/JavaScriptTestRunner.h +++ b/Userland/Libraries/LibTest/JavaScriptTestRunner.h @@ -363,7 +363,7 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path) JS::Bytecode::Interpreter bytecode_interpreter(interpreter->global_object(), interpreter->realm()); MUST(bytecode_interpreter.run(*executable)); } else { - g_vm->push_execution_context(global_execution_context, interpreter->global_object()); + g_vm->push_execution_context(global_execution_context); MUST(interpreter->run(*test_script)); g_vm->pop_execution_context(); } @@ -382,7 +382,7 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path) (void)bytecode_interpreter.run(*executable); } } else { - g_vm->push_execution_context(global_execution_context, interpreter->global_object()); + g_vm->push_execution_context(global_execution_context); (void)interpreter->run(file_script.value()); g_vm->pop_execution_context(); } diff --git a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp index b74a8d0fa87..e1209646bb0 100644 --- a/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp +++ b/Userland/Libraries/LibWeb/Bindings/MainThreadVM.cpp @@ -138,7 +138,7 @@ JS::VM& main_thread_vm() // 4. If script execution context is not null, then push script execution context onto the JavaScript execution context stack. if (callback_host_defined.active_script_context) - MUST(vm->push_execution_context(*callback_host_defined.active_script_context, callback.callback.cell()->global_object())); + vm->push_execution_context(*callback_host_defined.active_script_context); // 5. Let result be Call(callback.[[Callback]], V, argumentsList). auto result = JS::call(global_object, *callback.callback.cell(), this_value, move(arguments_list)); @@ -228,7 +228,7 @@ JS::VM& main_thread_vm() auto script_record = script_or_module.get>(); dummy_execution_context = JS::ExecutionContext { vm->heap() }; dummy_execution_context->script_or_module = script_or_module; - vm->push_execution_context(dummy_execution_context.value(), script_record->realm().global_object()); + vm->push_execution_context(dummy_execution_context.value()); } // 3. Let result be job(). diff --git a/Userland/Libraries/LibWeb/DOM/EventTarget.cpp b/Userland/Libraries/LibWeb/DOM/EventTarget.cpp index 274531e5c4e..021a5d7199a 100644 --- a/Userland/Libraries/LibWeb/DOM/EventTarget.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventTarget.cpp @@ -372,9 +372,10 @@ Bindings::CallbackType* EventTarget::get_current_value_of_event_handler(FlyStrin return nullptr; } - // 8. Push settings object's realm execution context onto the JavaScript execution context stack; it is now the running JavaScript execution context. auto& global_object = settings_object.global_object(); - global_object.vm().push_execution_context(settings_object.realm_execution_context(), global_object); + + // 8. Push settings object's realm execution context onto the JavaScript execution context stack; it is now the running JavaScript execution context. + global_object.vm().push_execution_context(settings_object.realm_execution_context()); // 9. Let function be the result of calling OrdinaryFunctionCreate, with arguments: // functionPrototype diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp index b804e08793e..76927cb4ff7 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/Environments.cpp @@ -78,7 +78,7 @@ RunScriptDecision EnvironmentSettingsObject::can_run_script() void EnvironmentSettingsObject::prepare_to_run_script() { // 1. Push settings's realm execution context onto the JavaScript execution context stack; it is now the running JavaScript execution context. - global_object().vm().push_execution_context(realm_execution_context(), global_object()); + global_object().vm().push_execution_context(realm_execution_context()); // FIXME: 2. Add settings to the currently running task's script evaluation environment settings object set. } diff --git a/Userland/Libraries/LibWeb/HTML/Worker.cpp b/Userland/Libraries/LibWeb/HTML/Worker.cpp index 0cd10ff0f7b..f0a4fc7c1a8 100644 --- a/Userland/Libraries/LibWeb/HTML/Worker.cpp +++ b/Userland/Libraries/LibWeb/HTML/Worker.cpp @@ -160,7 +160,7 @@ void Worker::run_a_worker(AK::URL& url, EnvironmentSettingsObject& outside_setti m_execution_context.variable_environment = &m_worker_realm->global_environment(); m_execution_context.realm = m_worker_realm; - m_worker_vm->push_execution_context(m_execution_context, *m_worker_scope); + m_worker_vm->push_execution_context(m_execution_context); m_worker_realm->set_global_object(*m_worker_scope, m_worker_scope); // 8. Let worker global scope be the global object of realm execution context's Realm component. diff --git a/Userland/Services/WebContent/WebContentConsoleClient.cpp b/Userland/Services/WebContent/WebContentConsoleClient.cpp index 50758377070..fb742e9a381 100644 --- a/Userland/Services/WebContent/WebContentConsoleClient.cpp +++ b/Userland/Services/WebContent/WebContentConsoleClient.cpp @@ -31,7 +31,7 @@ WebContentConsoleClient::WebContentConsoleClient(JS::Console& console, WeakPtr(*m_interpreter->realm().host_defined()); - vm.push_execution_context(eso.realm_execution_context(), global_object); + vm.push_execution_context(eso.realm_execution_context()); console_global_object->initialize_global_object(); vm.pop_execution_context();