From 795786387b0d3da94d27222931e5c36315d63d96 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 7 Jul 2021 19:34:26 +0300 Subject: [PATCH] LibJS: Remove the NativeProperty mechanism from LibJS These were an ad-hoc way to implement special behaviour when reading or writing to specific object properties. Because these were effectively replaced by the abillity to override the internal methods of Object, they are no longer needed. --- Userland/Libraries/LibJS/CMakeLists.txt | 1 - Userland/Libraries/LibJS/Forward.h | 1 - Userland/Libraries/LibJS/MarkupGenerator.cpp | 2 +- .../LibJS/Runtime/NativeProperty.cpp | 36 ----------- .../Libraries/LibJS/Runtime/NativeProperty.h | 29 --------- Userland/Libraries/LibJS/Runtime/Object.cpp | 63 ++----------------- Userland/Libraries/LibJS/Runtime/Object.h | 11 +--- Userland/Libraries/LibJS/Runtime/Value.cpp | 2 - Userland/Libraries/LibJS/Runtime/Value.h | 17 +---- Userland/Libraries/LibWeb/DOM/Document.cpp | 2 +- Userland/Utilities/js.cpp | 2 +- 11 files changed, 9 insertions(+), 157 deletions(-) delete mode 100644 Userland/Libraries/LibJS/Runtime/NativeProperty.cpp delete mode 100644 Userland/Libraries/LibJS/Runtime/NativeProperty.h diff --git a/Userland/Libraries/LibJS/CMakeLists.txt b/Userland/Libraries/LibJS/CMakeLists.txt index 3971875f4a..3d127f8921 100644 --- a/Userland/Libraries/LibJS/CMakeLists.txt +++ b/Userland/Libraries/LibJS/CMakeLists.txt @@ -82,7 +82,6 @@ set(SOURCES Runtime/MarkedValueList.cpp Runtime/MathObject.cpp Runtime/NativeFunction.cpp - Runtime/NativeProperty.cpp Runtime/NumberConstructor.cpp Runtime/NumberObject.cpp Runtime/NumberPrototype.cpp diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index 4b47465211..c9c1214e42 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -143,7 +143,6 @@ class HeapBlock; class Interpreter; class MarkedValueList; class NativeFunction; -class NativeProperty; class ObjectEnvironment; class PrimitiveString; class PromiseReaction; diff --git a/Userland/Libraries/LibJS/MarkupGenerator.cpp b/Userland/Libraries/LibJS/MarkupGenerator.cpp index bbe739b55e..91c9b05cd8 100644 --- a/Userland/Libraries/LibJS/MarkupGenerator.cpp +++ b/Userland/Libraries/LibJS/MarkupGenerator.cpp @@ -148,7 +148,7 @@ void MarkupGenerator::error_to_html(const Object& object, StringBuilder& html_ou auto& vm = object.vm(); auto name = object.get_without_side_effects(vm.names.name).value_or(JS::js_undefined()); auto message = object.get_without_side_effects(vm.names.message).value_or(JS::js_undefined()); - if (name.is_accessor() || name.is_native_property() || message.is_accessor() || message.is_native_property()) { + if (name.is_accessor() || message.is_accessor()) { html_output.append(wrap_string_in_style(JS::Value(&object).to_string_without_side_effects(), StyleType::Invalid)); } else { auto name_string = name.to_string_without_side_effects(); diff --git a/Userland/Libraries/LibJS/Runtime/NativeProperty.cpp b/Userland/Libraries/LibJS/Runtime/NativeProperty.cpp deleted file mode 100644 index 47c9857e8a..0000000000 --- a/Userland/Libraries/LibJS/Runtime/NativeProperty.cpp +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (c) 2020, Andreas Kling - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#include - -namespace JS { - -NativeProperty::NativeProperty(Function getter, Function setter) - : m_getter(move(getter)) - , m_setter(move(setter)) -{ -} - -NativeProperty::~NativeProperty() -{ -} - -Value NativeProperty::get(VM& vm, GlobalObject& global_object) const -{ - if (!m_getter) - return js_undefined(); - return m_getter(vm, global_object); -} - -void NativeProperty::set(VM& vm, GlobalObject& global_object, Value value) -{ - if (!m_setter) - return; - m_setter(vm, global_object, move(value)); -} - -} diff --git a/Userland/Libraries/LibJS/Runtime/NativeProperty.h b/Userland/Libraries/LibJS/Runtime/NativeProperty.h deleted file mode 100644 index aaa0389b5e..0000000000 --- a/Userland/Libraries/LibJS/Runtime/NativeProperty.h +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (c) 2020, Andreas Kling - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#pragma once - -#include -#include - -namespace JS { - -class NativeProperty final : public Cell { -public: - NativeProperty(Function getter, Function setter); - virtual ~NativeProperty() override; - - Value get(VM&, GlobalObject&) const; - void set(VM&, GlobalObject&, Value); - -private: - virtual const char* class_name() const override { return "NativeProperty"; } - - Function m_getter; - Function m_setter; -}; - -} diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 7b11081653..b84aa73635 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -15,7 +15,6 @@ #include #include #include -#include #include #include #include @@ -903,7 +902,7 @@ bool Object::set_immutable_prototype(Object* prototype) return false; } -Optional Object::storage_get(PropertyName const& property_name, CallNativeProperty call_native_property) const +Optional Object::storage_get(PropertyName const& property_name) const { VERIFY(property_name.is_valid()); @@ -923,8 +922,6 @@ Optional Object::storage_get(PropertyName const& property_na value = m_storage[metadata->offset]; attributes = metadata->attributes; } - if (value.is_native_property() && call_native_property == CallNativeProperty::Yes) - value = call_native_property_getter(value.as_native_property(), this); return ValueAndAttributes { .value = value, .attributes = attributes }; } @@ -944,15 +941,7 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c if (property_name.is_number()) { auto index = property_name.as_number(); - if (value.is_native_property()) { - m_indexed_properties.put(index, value, attributes); - } else { - auto existing_value = m_indexed_properties.get(index); - if (existing_value.has_value() && existing_value->value.is_native_property()) - call_native_property_setter(existing_value->value.as_native_property(), this, value); - else - m_indexed_properties.put(index, value, attributes); - } + m_indexed_properties.put(index, value, attributes); return; } @@ -998,15 +987,7 @@ void Object::storage_set(PropertyName const& property_name, ValueAndAttributes c VERIFY(metadata.has_value()); } - if (value.is_native_property()) { - m_storage[metadata->offset] = value; - } else { - auto existing_value = m_storage[metadata->offset]; - if (existing_value.is_native_property()) - call_native_property_setter(existing_value.as_native_property(), this, value); - else - m_storage[metadata->offset] = value; - } + m_storage[metadata->offset] = value; } void Object::storage_delete(PropertyName const& property_name) @@ -1090,7 +1071,7 @@ Value Object::get_without_side_effects(const PropertyName& property_name) const { auto* object = this; while (object) { - auto value_and_attributes = object->storage_get(property_name, CallNativeProperty::No); + auto value_and_attributes = object->storage_get(property_name); if (value_and_attributes.has_value()) return value_and_attributes->value; object = object->prototype(); @@ -1113,11 +1094,6 @@ void Object::define_native_function(PropertyName const& property_name, Function< define_direct_property(property_name, function, attribute); } -void Object::define_native_property(PropertyName const& property_name, Function getter, Function setter, PropertyAttributes attribute) -{ - define_direct_property(property_name, heap().allocate_without_global_object(move(getter), move(setter)), attribute); -} - // 20.1.2.3.1 ObjectDefineProperties ( O, Properties ), https://tc39.es/ecma262/#sec-objectdefineproperties Object* Object::define_properties(Value properties) { @@ -1238,35 +1214,4 @@ Value Object::invoke_internal(const StringOrSymbol& property_name, Optionalvm(); - ExecutionContext execution_context; - if (auto* interpreter = vm.interpreter_if_exists()) - execution_context.current_node = interpreter->current_node(); - execution_context.is_strict_mode = vm.in_strict_mode(); - execution_context.this_value = this_value; - vm.push_execution_context(execution_context, global_object()); - if (vm.exception()) - return {}; - auto result = property.get(vm, global_object()); - vm.pop_execution_context(); - return result; -} - -void Object::call_native_property_setter(NativeProperty& property, Value this_value, Value setter_value) const -{ - auto& vm = this->vm(); - ExecutionContext execution_context; - if (auto* interpreter = vm.interpreter_if_exists()) - execution_context.current_node = interpreter->current_node(); - execution_context.is_strict_mode = vm.in_strict_mode(); - execution_context.this_value = this_value; - vm.push_execution_context(execution_context, global_object()); - if (vm.exception()) - return; - property.set(vm, global_object(), setter_value); - vm.pop_execution_context(); -} - } diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index be08581bf3..665c6312bf 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -111,12 +111,7 @@ public: // Implementation-specific storage abstractions - enum class CallNativeProperty { - Yes, - No, - }; - - Optional storage_get(PropertyName const&, CallNativeProperty = CallNativeProperty::Yes) const; + Optional storage_get(PropertyName const&) const; bool storage_has(PropertyName const&) const; void storage_set(PropertyName const&, ValueAndAttributes const&); void storage_delete(PropertyName const&); @@ -129,7 +124,6 @@ public: void define_direct_accessor(PropertyName const&, FunctionObject* getter, FunctionObject* setter, PropertyAttributes attributes); void define_native_function(PropertyName const&, Function, i32 length, PropertyAttributes attributes); - void define_native_property(PropertyName const&, Function getter, Function setter, PropertyAttributes attributes); void define_native_accessor(PropertyName const&, Function getter, Function setter, PropertyAttributes attributes); virtual bool is_function() const { return false; } @@ -196,9 +190,6 @@ protected: bool m_has_parameter_map { false }; private: - Value call_native_property_getter(NativeProperty& property, Value this_value) const; - void call_native_property_setter(NativeProperty& property, Value this_value, Value) const; - void set_shape(Shape&); Object* prototype() { return shape().prototype(); } diff --git a/Userland/Libraries/LibJS/Runtime/Value.cpp b/Userland/Libraries/LibJS/Runtime/Value.cpp index 6b5a6a08f8..c60a7f8157 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.cpp +++ b/Userland/Libraries/LibJS/Runtime/Value.cpp @@ -318,8 +318,6 @@ String Value::to_string_without_side_effects() const return String::formatted("[object {}]", as_object().class_name()); case Type::Accessor: return ""; - case Type::NativeProperty: - return ""; default: VERIFY_NOT_REACHED(); } diff --git a/Userland/Libraries/LibJS/Runtime/Value.h b/Userland/Libraries/LibJS/Runtime/Value.h index 7013027c94..b694129b2a 100644 --- a/Userland/Libraries/LibJS/Runtime/Value.h +++ b/Userland/Libraries/LibJS/Runtime/Value.h @@ -41,7 +41,6 @@ public: Symbol, Accessor, BigInt, - NativeProperty, }; enum class PreferredType { @@ -60,9 +59,8 @@ public: bool is_symbol() const { return m_type == Type::Symbol; } bool is_accessor() const { return m_type == Type::Accessor; }; bool is_bigint() const { return m_type == Type::BigInt; }; - bool is_native_property() const { return m_type == Type::NativeProperty; } bool is_nullish() const { return is_null() || is_undefined(); } - bool is_cell() const { return is_string() || is_accessor() || is_object() || is_bigint() || is_symbol() || is_native_property(); } + bool is_cell() const { return is_string() || is_accessor() || is_object() || is_bigint() || is_symbol(); } bool is_array(GlobalObject&) const; bool is_function() const; bool is_constructor() const; @@ -164,12 +162,6 @@ public: m_value.as_bigint = const_cast(bigint); } - Value(const NativeProperty* native_property) - : m_type(Type::NativeProperty) - { - m_value.as_native_property = const_cast(native_property); - } - explicit Value(Type type) : m_type(type) { @@ -245,12 +237,6 @@ public: return *m_value.as_bigint; } - NativeProperty& as_native_property() - { - VERIFY(is_native_property()); - return *m_value.as_native_property; - } - Array& as_array(); FunctionObject& as_function(); @@ -318,7 +304,6 @@ private: Cell* as_cell; Accessor* as_accessor; BigInt* as_bigint; - NativeProperty* as_native_property; u64 encoded; } m_value { .encoded = 0 }; diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 62871e8630..8c3a1711dc 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -646,7 +646,7 @@ JS::Interpreter& Document::interpreter() auto& object = value.as_object(); auto name = object.get_without_side_effects(vm.names.name).value_or(JS::js_undefined()); auto message = object.get_without_side_effects(vm.names.message).value_or(JS::js_undefined()); - if (name.is_accessor() || name.is_native_property() || message.is_accessor() || message.is_native_property()) { + if (name.is_accessor() || message.is_accessor()) { // The result is not going to be useful, let's just print the value. This affects DOMExceptions, for example. dbgln("Unhandled JavaScript exception: {}", value); } else { diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index dd6ffd3daf..7fb8f1a876 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -255,7 +255,7 @@ static void print_error(JS::Object const& object, HashTable& seen_o { auto name = object.get_without_side_effects(vm->names.name).value_or(JS::js_undefined()); auto message = object.get_without_side_effects(vm->names.message).value_or(JS::js_undefined()); - if (name.is_accessor() || name.is_native_property() || message.is_accessor() || message.is_native_property()) { + if (name.is_accessor() || message.is_accessor()) { print_value(&object, seen_objects); } else { auto name_string = name.to_string_without_side_effects();