From afcfea20015d59a5776ed644ad4c3b2a008fefc6 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Thu, 25 Jun 2020 14:00:13 +0100 Subject: [PATCH] LibJS: Handle "receiver" argument in Reflect.{get,set}() --- Libraries/LibJS/Runtime/Object.cpp | 16 ++++++++++------ Libraries/LibJS/Runtime/Object.h | 6 +++--- Libraries/LibJS/Runtime/ProxyObject.cpp | 4 ++-- Libraries/LibJS/Runtime/ProxyObject.h | 4 ++-- Libraries/LibJS/Runtime/ReflectObject.cpp | 12 ++++++++---- Libraries/LibJS/Tests/Reflect.get.js | 17 +++++++++++++++++ Libraries/LibJS/Tests/Reflect.set.js | 18 ++++++++++++++++++ 7 files changed, 60 insertions(+), 17 deletions(-) diff --git a/Libraries/LibJS/Runtime/Object.cpp b/Libraries/LibJS/Runtime/Object.cpp index 0624840eff..0b87bc57fd 100644 --- a/Libraries/LibJS/Runtime/Object.cpp +++ b/Libraries/LibJS/Runtime/Object.cpp @@ -152,7 +152,7 @@ bool Object::prevent_extensions() return true; } -Value Object::get_own_property(const Object& this_object, PropertyName property_name) const +Value Object::get_own_property(const Object& this_object, PropertyName property_name, Value receiver) const { Value value_here; @@ -170,7 +170,7 @@ Value Object::get_own_property(const Object& this_object, PropertyName property_ ASSERT(!value_here.is_empty()); if (value_here.is_accessor()) { - return value_here.as_accessor().call_getter(Value(const_cast(this))); + return value_here.as_accessor().call_getter(receiver); } if (value_here.is_native_property()) return call_native_property_getter(const_cast(&this_object), value_here); @@ -605,7 +605,7 @@ Value Object::get_by_index(u32 property_index) const return {}; } -Value Object::get(PropertyName property_name) const +Value Object::get(PropertyName property_name, Value receiver) const { if (property_name.is_number()) return get_by_index(property_name.as_number()); @@ -617,7 +617,9 @@ Value Object::get(PropertyName property_name) const const Object* object = this; while (object) { - auto value = object->get_own_property(*this, property_name); + if (receiver.is_empty()) + receiver = Value(const_cast(this)); + auto value = object->get_own_property(*this, property_name, receiver); if (interpreter().exception()) return {}; if (!value.is_empty()) @@ -656,7 +658,7 @@ bool Object::put_by_index(u32 property_index, Value value) return put_own_property_by_index(*this, property_index, value, default_attributes, PutOwnPropertyMode::Put); } -bool Object::put(PropertyName property_name, Value value) +bool Object::put(PropertyName property_name, Value value, Value receiver) { if (property_name.is_number()) return put_by_index(property_name.as_number(), value); @@ -676,7 +678,9 @@ bool Object::put(PropertyName property_name, Value value) if (metadata.has_value()) { auto value_here = object->m_storage[metadata.value().offset]; if (value_here.is_accessor()) { - value_here.as_accessor().call_setter(Value(this), value); + if (receiver.is_empty()) + receiver = Value(this); + value_here.as_accessor().call_setter(receiver, value); return true; } if (value_here.is_native_property()) { diff --git a/Libraries/LibJS/Runtime/Object.h b/Libraries/LibJS/Runtime/Object.h index cdc5381b02..4cf78f822d 100644 --- a/Libraries/LibJS/Runtime/Object.h +++ b/Libraries/LibJS/Runtime/Object.h @@ -84,14 +84,14 @@ public: GlobalObject& global_object() const { return shape().global_object(); } - virtual Value get(PropertyName) const; + virtual Value get(PropertyName, Value receiver = {}) const; virtual bool has_property(PropertyName) const; bool has_own_property(PropertyName) const; - virtual bool put(PropertyName, Value); + virtual bool put(PropertyName, Value, Value receiver = {}); - Value get_own_property(const Object& this_object, PropertyName) const; + Value get_own_property(const Object& this_object, PropertyName, Value receiver) const; Value get_own_properties(const Object& this_object, GetOwnPropertyMode, bool only_enumerable_properties = false) const; virtual Optional get_own_property_descriptor(PropertyName) const; Value get_own_property_descriptor_object(PropertyName) const; diff --git a/Libraries/LibJS/Runtime/ProxyObject.cpp b/Libraries/LibJS/Runtime/ProxyObject.cpp index 2f9809c4ad..6c46dd8fa4 100644 --- a/Libraries/LibJS/Runtime/ProxyObject.cpp +++ b/Libraries/LibJS/Runtime/ProxyObject.cpp @@ -363,7 +363,7 @@ bool ProxyObject::has_property(PropertyName name) const return trap_result; } -Value ProxyObject::get(PropertyName name) const +Value ProxyObject::get(PropertyName name, Value) const { if (m_is_revoked) { interpreter().throw_exception(ErrorType::ProxyRevoked); @@ -395,7 +395,7 @@ Value ProxyObject::get(PropertyName name) const return trap_result; } -bool ProxyObject::put(PropertyName name, Value value) +bool ProxyObject::put(PropertyName name, Value value, Value) { if (m_is_revoked) { interpreter().throw_exception(ErrorType::ProxyRevoked); diff --git a/Libraries/LibJS/Runtime/ProxyObject.h b/Libraries/LibJS/Runtime/ProxyObject.h index ce8c3ef69d..f1b2c7bb7a 100644 --- a/Libraries/LibJS/Runtime/ProxyObject.h +++ b/Libraries/LibJS/Runtime/ProxyObject.h @@ -50,8 +50,8 @@ public: virtual Optional get_own_property_descriptor(PropertyName) const override; virtual bool define_property(const FlyString& property_name, const Object& descriptor, bool throw_exceptions = true) override; virtual bool has_property(PropertyName name) const override; - virtual Value get(PropertyName name) const override; - virtual bool put(PropertyName name, Value value) override; + virtual Value get(PropertyName name, Value receiver) const override; + virtual bool put(PropertyName name, Value value, Value receiver) override; virtual Value delete_property(PropertyName name) override; void revoke() { m_is_revoked = true; } diff --git a/Libraries/LibJS/Runtime/ReflectObject.cpp b/Libraries/LibJS/Runtime/ReflectObject.cpp index b013591876..01a0936c8a 100644 --- a/Libraries/LibJS/Runtime/ReflectObject.cpp +++ b/Libraries/LibJS/Runtime/ReflectObject.cpp @@ -178,14 +178,16 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::delete_property) JS_DEFINE_NATIVE_FUNCTION(ReflectObject::get) { - // FIXME: There's a third argument, receiver, for getters - use it once we have those. auto* target = get_target_object_from(interpreter, "get"); if (!target) return {}; auto property_key = interpreter.argument(1).to_string(interpreter); if (interpreter.exception()) return {}; - return target->get(property_key).value_or(js_undefined()); + Value receiver = {}; + if (interpreter.argument_count() > 2) + receiver = interpreter.argument(2); + return target->get(property_key, receiver).value_or(js_undefined()); } JS_DEFINE_NATIVE_FUNCTION(ReflectObject::get_own_property_descriptor) @@ -244,7 +246,6 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::prevent_extensions) JS_DEFINE_NATIVE_FUNCTION(ReflectObject::set) { - // FIXME: There's a fourth argument, receiver, for setters - use it once we have those. auto* target = get_target_object_from(interpreter, "set"); if (!target) return {}; @@ -252,7 +253,10 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::set) if (interpreter.exception()) return {}; auto value = interpreter.argument(2); - return Value(target->put(property_key, value)); + Value receiver = {}; + if (interpreter.argument_count() > 3) + receiver = interpreter.argument(3); + return Value(target->put(property_key, value, receiver)); } JS_DEFINE_NATIVE_FUNCTION(ReflectObject::set_prototype_of) diff --git a/Libraries/LibJS/Tests/Reflect.get.js b/Libraries/LibJS/Tests/Reflect.get.js index d3b28398f1..0ad987132f 100644 --- a/Libraries/LibJS/Tests/Reflect.get.js +++ b/Libraries/LibJS/Tests/Reflect.get.js @@ -33,6 +33,23 @@ try { assert(Reflect.get(new String("foo"), 2) === "o"); assert(Reflect.get(new String("foo"), 3) === undefined); + const foo = { + get prop() { + this.getPropCalled = true; + } + }; + const bar = {}; + Object.setPrototypeOf(bar, foo); + + assert(foo.getPropCalled === undefined); + assert(bar.getPropCalled === undefined); + Reflect.get(bar, "prop"); + assert(foo.getPropCalled === undefined); + assert(bar.getPropCalled === true); + Reflect.get(bar, "prop", foo); + assert(foo.getPropCalled === true); + assert(bar.getPropCalled === true); + console.log("PASS"); } catch (e) { console.log("FAIL: " + e); diff --git a/Libraries/LibJS/Tests/Reflect.set.js b/Libraries/LibJS/Tests/Reflect.set.js index d498996a16..08f095f191 100644 --- a/Libraries/LibJS/Tests/Reflect.set.js +++ b/Libraries/LibJS/Tests/Reflect.set.js @@ -48,6 +48,24 @@ try { assert(a[3] === undefined); assert(a[4] === "bar"); + + const foo = { + set prop(value) { + this.setPropCalled = true; + } + }; + const bar = {}; + Object.setPrototypeOf(bar, foo); + + assert(foo.setPropCalled === undefined); + assert(bar.setPropCalled === undefined); + Reflect.set(bar, "prop", 42); + assert(foo.setPropCalled === undefined); + assert(bar.setPropCalled === true); + Reflect.set(bar, "prop", 42, foo); + assert(foo.setPropCalled === true); + assert(bar.setPropCalled === true); + console.log("PASS"); } catch (e) { console.log("FAIL: " + e);