From e6d920d87ddd7b02b4fbbca3d6d6e98b9c8107dd Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 9 Apr 2020 22:15:26 +0200 Subject: LibJS: Add Object.defineProperty() and start caring about attributes We now care (a little bit) about the "configurable" and "writable" property attributes. Property attributes are stored together with the property name in the Shape object. Forward transitions are not attribute-savvy and will cause poor Shape reuse in the case of multiple same-name properties with different attributes. Oh, and this patch also adds Object.getOwnPropertyDescriptor() :^) --- Libraries/LibJS/Runtime/Object.cpp | 16 ++++++--- Libraries/LibJS/Runtime/Object.h | 10 ++++-- Libraries/LibJS/Runtime/ObjectConstructor.cpp | 46 ++++++++++++++++++++++++-- Libraries/LibJS/Runtime/ObjectConstructor.h | 2 ++ Libraries/LibJS/Runtime/Shape.cpp | 12 +++---- Libraries/LibJS/Runtime/Shape.h | 12 +++++-- Libraries/LibJS/Tests/Object.defineProperty.js | 39 ++++++++++++++++++++++ 7 files changed, 121 insertions(+), 16 deletions(-) create mode 100644 Libraries/LibJS/Tests/Object.defineProperty.js (limited to 'Libraries') diff --git a/Libraries/LibJS/Runtime/Object.cpp b/Libraries/LibJS/Runtime/Object.cpp index 774ba51af4..df4205fe72 100644 --- a/Libraries/LibJS/Runtime/Object.cpp +++ b/Libraries/LibJS/Runtime/Object.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -97,14 +98,22 @@ void Object::set_shape(Shape& new_shape) m_shape = &new_shape; } -bool Object::put_own_property(Object& this_object, const FlyString& property_name, Value value) +void Object::put_own_property(Object& this_object, const FlyString& property_name, u8 attributes, Value value, PutOwnPropertyMode mode) { auto metadata = shape().lookup(property_name); if (!metadata.has_value()) { - auto* new_shape = m_shape->create_put_transition(property_name, 0); + auto* new_shape = m_shape->create_put_transition(property_name, attributes); set_shape(*new_shape); metadata = shape().lookup(property_name); ASSERT(metadata.has_value()); + } else if (!(metadata.value().attributes & Attribute::Writable)) { + dbg() << "Disallow write to non-writable property"; + return; + } + if (mode == PutOwnPropertyMode::DefineProperty && !(metadata.value().attributes & Attribute::Configurable) && attributes != metadata.value().attributes) { + dbg() << "Disallow reconfig of non-configurable property"; + interpreter().throw_exception("TypeError", String::format("Cannot redefine property '%s'", property_name.characters())); + return; } auto value_here = m_storage[metadata.value().offset]; @@ -118,7 +127,6 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam } else { m_storage[metadata.value().offset] = value; } - return true; } Optional Object::get_by_index(i32 property_index) const @@ -201,7 +209,7 @@ void Object::put(const FlyString& property_name, Value value) } object = object->prototype(); } - put_own_property(*this, property_name, value); + put_own_property(*this, property_name, Attribute::Configurable | Attribute::Enumerable | Attribute::Writable, value, PutOwnPropertyMode::Put); } void Object::put(PropertyName property_name, Value value) diff --git a/Libraries/LibJS/Runtime/Object.h b/Libraries/LibJS/Runtime/Object.h index 7b9c8dcdfb..90fe25b852 100644 --- a/Libraries/LibJS/Runtime/Object.h +++ b/Libraries/LibJS/Runtime/Object.h @@ -52,8 +52,14 @@ public: void put(const FlyString& property_name, Value); void put(PropertyName, Value); - virtual Optional get_own_property(const Object& this_object, const FlyString& property_name) const; - virtual bool put_own_property(Object& this_object, const FlyString& property_name, Value); + Optional get_own_property(const Object& this_object, const FlyString& property_name) const; + + enum class PutOwnPropertyMode { + Put, + DefineProperty, + }; + + void put_own_property(Object& this_object, const FlyString& property_name, u8 attributes, Value, PutOwnPropertyMode); void put_native_function(const FlyString& property_name, AK::Function, i32 length = 0); void put_native_property(const FlyString& property_name, AK::Function getter, AK::Function setter); diff --git a/Libraries/LibJS/Runtime/ObjectConstructor.cpp b/Libraries/LibJS/Runtime/ObjectConstructor.cpp index bef6ceff1e..5d3b54fdfb 100644 --- a/Libraries/LibJS/Runtime/ObjectConstructor.cpp +++ b/Libraries/LibJS/Runtime/ObjectConstructor.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -37,6 +38,8 @@ ObjectConstructor::ObjectConstructor() { put("prototype", interpreter().object_prototype()); + put_native_function("defineProperty", define_property, 3); + put_native_function("getOwnPropertyDescriptor", get_own_property_descriptor, 2); put_native_function("getOwnPropertyNames", get_own_property_names, 1); put_native_function("getPrototypeOf", get_prototype_of, 1); put_native_function("setPrototypeOf", set_prototype_of, 2); @@ -88,8 +91,6 @@ Value ObjectConstructor::set_prototype_of(Interpreter& interpreter) { if (interpreter.argument_count() < 2) return {}; - if (!interpreter.argument(0).is_object()) - return {}; auto* object = interpreter.argument(0).to_object(interpreter.heap()); if (interpreter.exception()) return {}; @@ -97,4 +98,45 @@ Value ObjectConstructor::set_prototype_of(Interpreter& interpreter) return {}; } +Value ObjectConstructor::get_own_property_descriptor(Interpreter& interpreter) +{ + if (interpreter.argument_count() < 2) + return interpreter.throw_exception("TypeError", "Object.getOwnPropertyDescriptor() needs 2 arguments"); + if (!interpreter.argument(0).is_object()) + return interpreter.throw_exception("TypeError", "Object argument is not an object"); + auto& object = interpreter.argument(0).as_object(); + auto metadata = object.shape().lookup(interpreter.argument(1).to_string()); + if (!metadata.has_value()) + return js_undefined(); + auto* descriptor = interpreter.heap().allocate(); + descriptor->put("configurable", Value(!!(metadata.value().attributes & Attribute::Configurable))); + descriptor->put("enumerable", Value(!!(metadata.value().attributes & Attribute::Enumerable))); + descriptor->put("writable", Value(!!(metadata.value().attributes & Attribute::Writable))); + descriptor->put("value", object.get(interpreter.argument(1).to_string()).value_or(js_undefined())); + return descriptor; +} + +Value ObjectConstructor::define_property(Interpreter& interpreter) +{ + if (interpreter.argument_count() < 3) + return interpreter.throw_exception("TypeError", "Object.defineProperty() needs 3 arguments"); + if (!interpreter.argument(0).is_object()) + return interpreter.throw_exception("TypeError", "Object argument is not an object"); + if (!interpreter.argument(2).is_object()) + return interpreter.throw_exception("TypeError", "Descriptor argument is not an object"); + auto& object = interpreter.argument(0).as_object(); + auto& descriptor = interpreter.argument(2).as_object(); + + Value value = descriptor.get("value").value_or(js_undefined()); + u8 configurable = descriptor.get("configurable").value_or(Value(false)).to_boolean() * Attribute::Configurable; + u8 enumerable = descriptor.get("enumerable").value_or(Value(false)).to_boolean() * Attribute::Enumerable; + u8 writable = descriptor.get("writable").value_or(Value(false)).to_boolean() * Attribute::Writable; + u8 attributes = configurable | enumerable | writable; + + dbg() << "Defining new property " << interpreter.argument(1).to_string() << " with descriptor { " << configurable << ", " << enumerable << ", " << writable << ", attributes=" << attributes << " }"; + + object.put_own_property(object, interpreter.argument(1).to_string(), attributes, value, PutOwnPropertyMode::DefineProperty); + return &object; +} + } diff --git a/Libraries/LibJS/Runtime/ObjectConstructor.h b/Libraries/LibJS/Runtime/ObjectConstructor.h index 06fd81ba01..06fdac9c9e 100644 --- a/Libraries/LibJS/Runtime/ObjectConstructor.h +++ b/Libraries/LibJS/Runtime/ObjectConstructor.h @@ -42,6 +42,8 @@ private: virtual bool has_constructor() const override { return true; } virtual const char* class_name() const override { return "ObjectConstructor"; } + static Value define_property(Interpreter&); + static Value get_own_property_descriptor(Interpreter&); static Value get_own_property_names(Interpreter&); static Value get_prototype_of(Interpreter&); static Value set_prototype_of(Interpreter&); diff --git a/Libraries/LibJS/Runtime/Shape.cpp b/Libraries/LibJS/Runtime/Shape.cpp index fadb073281..8dbbd34a2e 100644 --- a/Libraries/LibJS/Runtime/Shape.cpp +++ b/Libraries/LibJS/Runtime/Shape.cpp @@ -29,12 +29,12 @@ namespace JS { -Shape* Shape::create_put_transition(const FlyString& property_name, u8 property_attributes) +Shape* Shape::create_put_transition(const FlyString& property_name, u8 attributes) { auto* new_shape = m_forward_transitions.get(property_name).value_or(nullptr); - if (new_shape && new_shape->m_property_attributes == property_attributes) + if (new_shape && new_shape->m_attributes == attributes) return new_shape; - new_shape = heap().allocate(this, property_name, property_attributes); + new_shape = heap().allocate(this, property_name, attributes); m_forward_transitions.set(property_name, new_shape); return new_shape; } @@ -48,10 +48,10 @@ Shape::Shape() { } -Shape::Shape(Shape* previous_shape, const FlyString& property_name, u8 property_attributes) +Shape::Shape(Shape* previous_shape, const FlyString& property_name, u8 attributes) : m_previous(previous_shape) , m_property_name(property_name) - , m_property_attributes(property_attributes) + , m_attributes(attributes) , m_prototype(previous_shape->m_prototype) { } @@ -114,7 +114,7 @@ void Shape::ensure_property_table() const // Ignore prototype transitions as they don't affect the key map. continue; } - m_property_table->set(shape->m_property_name, { next_offset++, shape->m_property_attributes }); + m_property_table->set(shape->m_property_name, { next_offset++, shape->m_attributes }); } } diff --git a/Libraries/LibJS/Runtime/Shape.h b/Libraries/LibJS/Runtime/Shape.h index 66613160ec..6c98a86bf8 100644 --- a/Libraries/LibJS/Runtime/Shape.h +++ b/Libraries/LibJS/Runtime/Shape.h @@ -35,6 +35,14 @@ namespace JS { +struct Attribute { + enum { + Configurable = 1 << 0, + Enumerable = 1 << 1, + Writable = 1 << 2, + }; +}; + struct PropertyMetadata { size_t offset { 0 }; u8 attributes { 0 }; @@ -45,7 +53,7 @@ public: virtual ~Shape() override; Shape(); - Shape(Shape* previous_shape, const FlyString& property_name, u8 property_attributes); + Shape(Shape* previous_shape, const FlyString& property_name, u8 attributes); Shape(Shape* previous_shape, Object* new_prototype); Shape* create_put_transition(const FlyString& name, u8 attributes); @@ -71,7 +79,7 @@ private: HashMap m_forward_transitions; Shape* m_previous { nullptr }; FlyString m_property_name; - u8 m_property_attributes { 0 }; + u8 m_attributes { 0 }; Object* m_prototype { nullptr }; }; diff --git a/Libraries/LibJS/Tests/Object.defineProperty.js b/Libraries/LibJS/Tests/Object.defineProperty.js new file mode 100644 index 0000000000..4f3b7049a6 --- /dev/null +++ b/Libraries/LibJS/Tests/Object.defineProperty.js @@ -0,0 +1,39 @@ +function assert(x) { if (!x) throw 1; } + +try { + + var o = {}; + Object.defineProperty(o, "foo", { value: 1, writable: false, enumerable: false }); + + assert(o.foo === 1); + o.foo = 2; + assert(o.foo === 1); + + var d = Object.getOwnPropertyDescriptor(o, "foo"); + assert(d.configurable === false); + assert(d.enumerable === false); + assert(d.writable === false); + assert(d.value === 1); + + Object.defineProperty(o, "bar", { value: "hi", writable: true, enumerable: true }); + + assert(o.bar === "hi"); + o.bar = "ho"; + assert(o.bar === "ho"); + + d = Object.getOwnPropertyDescriptor(o, "bar"); + assert(d.configurable === false); + assert(d.enumerable === true); + assert(d.writable === true); + assert(d.value === "ho"); + + try { + Object.defineProperty(o, "bar", { value: "xx", enumerable: false }); + } catch (e) { + assert(e.name === "TypeError"); + } + + console.log("PASS"); +} catch (e) { + console.log(e) +} -- cgit v1.2.3