From 235eb0b1adba5920ce914e5a908b5940c2ddf9f8 Mon Sep 17 00:00:00 2001 From: Ali Mohammad Pur Date: Sun, 21 Nov 2021 05:04:55 +0330 Subject: [PATCH] Spreadsheet: Replace hacky JS VM configuration with a more correct one Now we give each sheet its own interpreter and realm, and only make them share the VM. This is to prepare for the next commit, which will be refactoring a bunch of things to propagate exceptions via ThrowCompletionOr. --- .../Spreadsheet/JSIntegration.cpp | 2 +- .../Applications/Spreadsheet/Spreadsheet.cpp | 12 ++++----- .../Applications/Spreadsheet/Spreadsheet.h | 2 ++ .../Applications/Spreadsheet/Workbook.cpp | 27 ++++++++++--------- Userland/Applications/Spreadsheet/Workbook.h | 10 +++---- 5 files changed, 27 insertions(+), 26 deletions(-) diff --git a/Userland/Applications/Spreadsheet/JSIntegration.cpp b/Userland/Applications/Spreadsheet/JSIntegration.cpp index 85d4f35346d..0015f6eff2a 100644 --- a/Userland/Applications/Spreadsheet/JSIntegration.cpp +++ b/Userland/Applications/Spreadsheet/JSIntegration.cpp @@ -356,7 +356,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::get_column_bound) } WorkbookObject::WorkbookObject(Workbook& workbook) - : JS::Object(*JS::Object::create(workbook.global_object(), workbook.global_object().object_prototype())) + : JS::Object(*JS::Object::create(workbook.vm().interpreter().global_object(), workbook.vm().interpreter().global_object().object_prototype())) , m_workbook(workbook) { } diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.cpp b/Userland/Applications/Spreadsheet/Spreadsheet.cpp index beaa6fc38e1..83f8296c546 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Userland/Applications/Spreadsheet/Spreadsheet.cpp @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -38,16 +39,13 @@ Sheet::Sheet(StringView name, Workbook& workbook) Sheet::Sheet(Workbook& workbook) : m_workbook(workbook) + , m_interpreter(JS::Interpreter::create(m_workbook.vm(), *this)) { - JS::DeferGC defer_gc(m_workbook.interpreter().heap()); - m_global_object = m_workbook.interpreter().heap().allocate_without_global_object(*this); - global_object().initialize_global_object(); + JS::DeferGC defer_gc(m_workbook.vm().heap()); + m_global_object = static_cast(&m_interpreter->global_object()); global_object().define_direct_property("workbook", m_workbook.workbook_object(), JS::default_attributes); global_object().define_direct_property("thisSheet", &global_object(), JS::default_attributes); // Self-reference is unfortunate, but required. - // Note: We have to set the global object here otherwise the functions in runtime.js are not registered correctly. - interpreter().realm().set_global_object(global_object(), &global_object()); - // Sadly, these have to be evaluated once per sheet. auto file_or_error = Core::File::open("/res/js/Spreadsheet/runtime.js", Core::OpenMode::ReadOnly); if (!file_or_error.is_error()) { @@ -77,7 +75,7 @@ Sheet::~Sheet() JS::Interpreter& Sheet::interpreter() const { - return m_workbook.interpreter(); + return const_cast(*m_interpreter); } size_t Sheet::add_row() diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.h b/Userland/Applications/Spreadsheet/Spreadsheet.h index 69248dff8b1..213131185df 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.h +++ b/Userland/Applications/Spreadsheet/Spreadsheet.h @@ -147,6 +147,8 @@ private: Workbook& m_workbook; mutable SheetGlobalObject* m_global_object; + NonnullOwnPtr m_interpreter; + Cell* m_current_cell_being_evaluated { nullptr }; HashTable m_visited_cells_in_update; diff --git a/Userland/Applications/Spreadsheet/Workbook.cpp b/Userland/Applications/Spreadsheet/Workbook.cpp index 65f269a209d..b6d90dd0afb 100644 --- a/Userland/Applications/Spreadsheet/Workbook.cpp +++ b/Userland/Applications/Spreadsheet/Workbook.cpp @@ -17,21 +17,24 @@ namespace Spreadsheet { -static JS::VM& global_vm() -{ - static RefPtr vm; - if (!vm) - vm = JS::VM::create(); - return *vm; -} - Workbook::Workbook(NonnullRefPtrVector&& sheets) : m_sheets(move(sheets)) - , m_interpreter(JS::Interpreter::create(global_vm())) - , m_interpreter_scope(JS::VM::InterpreterExecutionScope(interpreter())) + , m_vm(JS::VM::create()) + , m_interpreter(JS::Interpreter::create(m_vm)) + , m_interpreter_scope(*m_interpreter) + , m_main_execution_context(m_vm->heap()) { - m_workbook_object = interpreter().heap().allocate(global_object(), *this); - global_object().define_direct_property("workbook", workbook_object(), JS::default_attributes); + m_workbook_object = m_vm->heap().allocate(m_interpreter->global_object(), *this); + m_interpreter->global_object().define_direct_property("workbook", workbook_object(), JS::default_attributes); + + m_main_execution_context.current_node = nullptr; + m_main_execution_context.this_value = &m_interpreter->global_object(); + m_main_execution_context.function_name = "(global execution context)"sv; + m_main_execution_context.lexical_environment = &m_interpreter->realm().global_environment(); + 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())); } bool Workbook::set_filename(const String& filename) diff --git a/Userland/Applications/Spreadsheet/Workbook.h b/Userland/Applications/Spreadsheet/Workbook.h index a2831ace4e3..99206d0e6bf 100644 --- a/Userland/Applications/Spreadsheet/Workbook.h +++ b/Userland/Applications/Spreadsheet/Workbook.h @@ -37,19 +37,17 @@ public: return *sheet; } - JS::Interpreter& interpreter() { return *m_interpreter; } - const JS::Interpreter& interpreter() const { return *m_interpreter; } - - JS::GlobalObject& global_object() { return m_interpreter->global_object(); } - const JS::GlobalObject& global_object() const { return m_interpreter->global_object(); } - WorkbookObject* workbook_object() { return m_workbook_object; } + JS::VM& vm() { return *m_vm; } + JS::VM const& vm() const { return *m_vm; } private: NonnullRefPtrVector m_sheets; + NonnullRefPtr m_vm; NonnullOwnPtr m_interpreter; JS::VM::InterpreterExecutionScope m_interpreter_scope; WorkbookObject* m_workbook_object { nullptr }; + JS::ExecutionContext m_main_execution_context; String m_current_filename; bool m_dirty { false };