diff --git a/src/binding.cc b/src/binding.cc index 69140db0..cdf3370e 100644 --- a/src/binding.cc +++ b/src/binding.cc @@ -1126,17 +1126,15 @@ void v8__ObjectTemplate__SetInternalFieldCount(const v8::ObjectTemplate& self, ptr_to_local(&self)->SetInternalFieldCount(value); } -void v8__ObjectTemplate__SetAccessor(const v8::ObjectTemplate& self, - const v8::Name& key, - v8::AccessorNameGetterCallback getter) { - ptr_to_local(&self)->SetAccessor(ptr_to_local(&key), getter); -} - -void v8__ObjectTemplate__SetAccessorWithSetter( +void v8__ObjectTemplate__SetAccessor( const v8::ObjectTemplate& self, const v8::Name& key, v8::AccessorNameGetterCallback getter, - v8::AccessorNameSetterCallback setter) { - ptr_to_local(&self)->SetAccessor(ptr_to_local(&key), getter, setter); + v8::AccessorNameSetterCallback setter, + const v8::Value* data_or_null, + v8::PropertyAttribute attr) { + ptr_to_local(&self)->SetAccessor( + ptr_to_local(&key), getter, setter, ptr_to_local(data_or_null), v8::AccessControl::DEFAULT, + attr); } void v8__ObjectTemplate__SetNamedPropertyHandler( @@ -1273,17 +1271,13 @@ MaybeBool v8__Object__DefineProperty(const v8::Object& self, MaybeBool v8__Object__SetAccessor(const v8::Object& self, const v8::Context& context, const v8::Name& key, - v8::AccessorNameGetterCallback getter) { + v8::AccessorNameGetterCallback getter, + v8::AccessorNameSetterCallback setter, + const v8::Value* data_or_null, + v8::PropertyAttribute attr) { return maybe_to_maybe_bool(ptr_to_local(&self)->SetAccessor( - ptr_to_local(&context), ptr_to_local(&key), getter)); -} - -MaybeBool v8__Object__SetAccessorWithSetter( - const v8::Object& self, const v8::Context& context, const v8::Name& key, - v8::AccessorNameGetterCallback getter, - v8::AccessorNameSetterCallback setter) { - return maybe_to_maybe_bool(ptr_to_local(&self)->SetAccessor( - ptr_to_local(&context), ptr_to_local(&key), getter, setter)); + ptr_to_local(&context), ptr_to_local(&key), getter, setter, + ptr_to_local(data_or_null), v8::AccessControl::DEFAULT,attr)); } v8::Isolate* v8__Object__GetIsolate(const v8::Object& self) { diff --git a/src/object.rs b/src/object.rs index 83124f3c..d289f66f 100644 --- a/src/object.rs +++ b/src/object.rs @@ -2,6 +2,7 @@ use crate::isolate::Isolate; use crate::support::int; use crate::support::MapFnTo; use crate::support::MaybeBool; +use crate::AccessorConfiguration; use crate::AccessorNameGetterCallback; use crate::AccessorNameSetterCallback; use crate::Array; @@ -25,6 +26,7 @@ use crate::Value; use std::convert::TryFrom; use std::ffi::c_void; use std::num::NonZeroI32; +use std::ptr::null; extern "C" { fn v8__Object__New(isolate: *mut Isolate) -> *const Object; @@ -40,13 +42,9 @@ extern "C" { context: *const Context, key: *const Name, getter: AccessorNameGetterCallback, - ) -> MaybeBool; - fn v8__Object__SetAccessorWithSetter( - this: *const Object, - context: *const Context, - key: *const Name, - getter: AccessorNameGetterCallback, - setter: AccessorNameSetterCallback, + setter: Option, + data_or_null: *const Value, + attr: PropertyAttribute, ) -> MaybeBool; fn v8__Object__Get( this: *const Object, @@ -435,15 +433,11 @@ impl Object { name: Local, getter: impl for<'s> MapFnTo>, ) -> Option { - unsafe { - v8__Object__SetAccessor( - self, - &*scope.get_current_context(), - &*name, - getter.map_fn_to(), - ) - } - .into() + self.set_accessor_with_configuration( + scope, + name, + AccessorConfiguration::new(getter), + ) } #[inline(always)] @@ -453,14 +447,29 @@ impl Object { name: Local, getter: impl for<'s> MapFnTo>, setter: impl for<'s> MapFnTo>, + ) -> Option { + self.set_accessor_with_configuration( + scope, + name, + AccessorConfiguration::new(getter).setter(setter), + ) + } + #[inline(always)] + pub fn set_accessor_with_configuration( + &self, + scope: &mut HandleScope, + name: Local, + configuration: AccessorConfiguration, ) -> Option { unsafe { - v8__Object__SetAccessorWithSetter( + v8__Object__SetAccessor( self, &*scope.get_current_context(), &*name, - getter.map_fn_to(), - setter.map_fn_to(), + configuration.getter, + configuration.setter, + configuration.data.map_or_else(null, |p| &*p), + configuration.property_attribute, ) } .into() diff --git a/src/template.rs b/src/template.rs index 38069ed8..f477f256 100644 --- a/src/template.rs +++ b/src/template.rs @@ -91,16 +91,14 @@ extern "C" { this: *const ObjectTemplate, value: int, ); + fn v8__ObjectTemplate__SetAccessor( this: *const ObjectTemplate, key: *const Name, getter: AccessorNameGetterCallback, - ); - fn v8__ObjectTemplate__SetAccessorWithSetter( - this: *const ObjectTemplate, - key: *const Name, - getter: AccessorNameGetterCallback, - setter: AccessorNameSetterCallback, + setter: Option, + data_or_null: *const Value, + attr: PropertyAttribute, ); fn v8__ObjectTemplate__SetAccessorProperty( this: *const ObjectTemplate, @@ -135,6 +133,46 @@ extern "C" { fn v8__ObjectTemplate__SetImmutableProto(this: *const ObjectTemplate); } +pub struct AccessorConfiguration<'s> { + pub(crate) getter: AccessorNameGetterCallback<'s>, + pub(crate) setter: Option>, + pub(crate) data: Option>, + pub(crate) property_attribute: PropertyAttribute, +} + +impl<'s> AccessorConfiguration<'s> { + pub fn new(getter: impl MapFnTo>) -> Self { + Self { + getter: getter.map_fn_to(), + setter: None, + data: None, + property_attribute: NONE, + } + } + + pub fn setter( + mut self, + setter: impl MapFnTo>, + ) -> Self { + self.setter = Some(setter.map_fn_to()); + self + } + + pub fn property_attribute( + mut self, + property_attribute: PropertyAttribute, + ) -> Self { + self.property_attribute = property_attribute; + self + } + + /// Set the associated data. The default is no associated data. + pub fn data(mut self, data: Local<'s, Value>) -> Self { + self.data = Some(data); + self + } +} + #[derive(Default)] pub struct NamedPropertyHandlerConfiguration<'s> { pub(crate) getter: Option>, @@ -620,7 +658,8 @@ impl ObjectTemplate { key: Local, getter: impl for<'s> MapFnTo>, ) { - unsafe { v8__ObjectTemplate__SetAccessor(self, &*key, getter.map_fn_to()) } + self + .set_accessor_with_configuration(key, AccessorConfiguration::new(getter)) } #[inline(always)] @@ -629,13 +668,27 @@ impl ObjectTemplate { key: Local, getter: impl for<'s> MapFnTo>, setter: impl for<'s> MapFnTo>, + ) { + self.set_accessor_with_configuration( + key, + AccessorConfiguration::new(getter).setter(setter), + ); + } + + #[inline(always)] + pub fn set_accessor_with_configuration( + &self, + key: Local, + configuration: AccessorConfiguration, ) { unsafe { - v8__ObjectTemplate__SetAccessorWithSetter( + v8__ObjectTemplate__SetAccessor( self, &*key, - getter.map_fn_to(), - setter.map_fn_to(), + configuration.getter, + configuration.setter, + configuration.data.map_or_else(null, |p| &*p), + configuration.property_attribute, ) } } diff --git a/tests/test_api.rs b/tests/test_api.rs index a8a5c641..e8eddc60 100644 --- a/tests/test_api.rs +++ b/tests/test_api.rs @@ -14,10 +14,10 @@ use std::ptr::{addr_of, NonNull}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; use std::sync::Mutex; -use v8::fast_api; use v8::fast_api::CType; use v8::fast_api::Type::*; use v8::inspector::ChannelBase; +use v8::{fast_api, AccessorConfiguration}; // TODO(piscisaureus): Ideally there would be no need to import this trait. use v8::MapFnTo; @@ -1801,6 +1801,43 @@ fn object_template_set_accessor() { assert!(this.set_internal_field(0, value)); }; + let getter_with_data = + |scope: &mut v8::HandleScope, + key: v8::Local, + args: v8::PropertyCallbackArguments, + mut rv: v8::ReturnValue| { + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_string()); + assert!(!args.should_throw_on_error()); + assert_eq!(args.data().to_rust_string_lossy(scope), "data"); + + let expected_key = v8::String::new(scope, "key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + rv.set(this.get_internal_field(scope, 0).unwrap()); + }; + + let setter_with_data = + |scope: &mut v8::HandleScope, + key: v8::Local, + value: v8::Local, + args: v8::PropertyCallbackArguments| { + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_string()); + assert!(!args.should_throw_on_error()); + assert_eq!(args.data().to_rust_string_lossy(scope), "data"); + + let expected_key = v8::String::new(scope, "key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + assert!(value.is_int32()); + assert!(this.set_internal_field(0, value)); + }; + let key = v8::String::new(scope, "key").unwrap(); let name = v8::String::new(scope, "obj").unwrap(); @@ -1840,6 +1877,34 @@ fn object_template_set_accessor() { // Falls back on standard setter assert!(eval(scope, "obj.key2 = null; obj.key2").unwrap().is_null()); + // Getter + setter + data + + let templ = v8::ObjectTemplate::new(scope); + templ.set_internal_field_count(1); + let data = v8::String::new(scope, "data").unwrap(); + templ.set_accessor_with_configuration( + key.into(), + AccessorConfiguration::new(getter_with_data) + .setter(setter_with_data) + .data(data.into()), + ); + + let obj = templ.new_instance(scope).unwrap(); + obj.set_internal_field(0, int.into()); + scope.get_current_context().global(scope).set( + scope, + name.into(), + obj.into(), + ); + let new_int = v8::Integer::new(scope, 9); + eval(scope, "obj.key = 9"); + assert!(obj + .get_internal_field(scope, 0) + .unwrap() + .strict_equals(new_int.into())); + // Falls back on standard setter + assert!(eval(scope, "obj.key2 = null; obj.key2").unwrap().is_null()); + // Accessor property let getter = v8::FunctionTemplate::new(scope, fortytwo_callback); fn property_setter( @@ -2621,6 +2686,218 @@ fn object_set_accessor_with_setter() { } } +#[test] +fn object_set_accessor_with_setter_with_property() { + let _setup_guard = setup::parallel_test(); + let isolate = &mut v8::Isolate::new(Default::default()); + let scope = &mut v8::HandleScope::new(isolate); + let context = v8::Context::new(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + { + static CALL_COUNT: AtomicUsize = AtomicUsize::new(0); + + let getter = |scope: &mut v8::HandleScope, + key: v8::Local, + args: v8::PropertyCallbackArguments, + mut rv: v8::ReturnValue| { + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_undefined()); + assert!(!args.should_throw_on_error()); + + let expected_key = v8::String::new(scope, "getter_setter_key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = this.get(scope, int_key.into()).unwrap(); + let int_value = v8::Local::::try_from(int_value).unwrap(); + assert_eq!(int_value.value(), 42); + + let s = v8::String::new(scope, "hello").unwrap(); + assert!(rv.get(scope).is_undefined()); + rv.set(s.into()); + + CALL_COUNT.fetch_add(1, Ordering::SeqCst); + }; + + let setter = |scope: &mut v8::HandleScope, + key: v8::Local, + value: v8::Local, + args: v8::PropertyCallbackArguments| { + println!("setter called"); + + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_undefined()); + assert!(!args.should_throw_on_error()); + + let expected_key = v8::String::new(scope, "getter_setter_key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = this.get(scope, int_key.into()).unwrap(); + let int_value = v8::Local::::try_from(int_value).unwrap(); + assert_eq!(int_value.value(), 42); + + let new_value = v8::Local::::try_from(value).unwrap(); + this.set(scope, int_key.into(), new_value.into()); + + CALL_COUNT.fetch_add(1, Ordering::SeqCst); + }; + + let obj = v8::Object::new(scope); + + let getter_setter_key = + v8::String::new(scope, "getter_setter_key").unwrap(); + obj.set_accessor_with_configuration( + scope, + getter_setter_key.into(), + AccessorConfiguration::new(getter) + .setter(setter) + .property_attribute(v8::READ_ONLY), + ); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = v8::Integer::new(scope, 42); + obj.set(scope, int_key.into(), int_value.into()); + + let obj_name = v8::String::new(scope, "obj").unwrap(); + context + .global(scope) + .set(scope, obj_name.into(), obj.into()); + + let actual = eval(scope, "obj.getter_setter_key").unwrap(); + let expected = v8::String::new(scope, "hello").unwrap(); + assert!(actual.strict_equals(expected.into())); + + eval(scope, "obj.getter_setter_key = 123").unwrap(); + assert_eq!( + obj + .get(scope, int_key.into()) + .unwrap() + .to_integer(scope) + .unwrap() + .value(), + 42 //Since it is read only + ); + + assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 1); + } +} + +#[test] +fn object_set_accessor_with_data() { + let _setup_guard = setup::parallel_test(); + let isolate = &mut v8::Isolate::new(Default::default()); + let scope = &mut v8::HandleScope::new(isolate); + let context = v8::Context::new(scope); + let scope = &mut v8::ContextScope::new(scope, context); + + { + static CALL_COUNT: AtomicUsize = AtomicUsize::new(0); + + let getter = |scope: &mut v8::HandleScope, + key: v8::Local, + args: v8::PropertyCallbackArguments, + mut rv: v8::ReturnValue| { + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_string()); + assert!(!args.should_throw_on_error()); + + let data = v8::String::new(scope, "data").unwrap(); + assert!(data.strict_equals(args.data())); + + let expected_key = v8::String::new(scope, "getter_setter_key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = this.get(scope, int_key.into()).unwrap(); + let int_value = v8::Local::::try_from(int_value).unwrap(); + assert_eq!(int_value.value(), 42); + + let s = v8::String::new(scope, "hello").unwrap(); + assert!(rv.get(scope).is_undefined()); + rv.set(s.into()); + + CALL_COUNT.fetch_add(1, Ordering::SeqCst); + }; + + let setter = |scope: &mut v8::HandleScope, + key: v8::Local, + value: v8::Local, + args: v8::PropertyCallbackArguments| { + println!("setter called"); + + let this = args.this(); + + assert_eq!(args.holder(), this); + assert!(args.data().is_string()); + assert!(!args.should_throw_on_error()); + + let data = v8::String::new(scope, "data").unwrap(); + assert!(data.strict_equals(args.data())); + + let expected_key = v8::String::new(scope, "getter_setter_key").unwrap(); + assert!(key.strict_equals(expected_key.into())); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = this.get(scope, int_key.into()).unwrap(); + let int_value = v8::Local::::try_from(int_value).unwrap(); + assert_eq!(int_value.value(), 42); + + let new_value = v8::Local::::try_from(value).unwrap(); + this.set(scope, int_key.into(), new_value.into()); + + CALL_COUNT.fetch_add(1, Ordering::SeqCst); + }; + + let obj = v8::Object::new(scope); + + let getter_setter_key = + v8::String::new(scope, "getter_setter_key").unwrap(); + + let data = v8::String::new(scope, "data").unwrap(); + obj.set_accessor_with_configuration( + scope, + getter_setter_key.into(), + AccessorConfiguration::new(getter) + .setter(setter) + .data(data.into()), + ); + + let int_key = v8::String::new(scope, "int_key").unwrap(); + let int_value = v8::Integer::new(scope, 42); + obj.set(scope, int_key.into(), int_value.into()); + + let obj_name = v8::String::new(scope, "obj").unwrap(); + context + .global(scope) + .set(scope, obj_name.into(), obj.into()); + + let actual = eval(scope, "obj.getter_setter_key").unwrap(); + let expected = v8::String::new(scope, "hello").unwrap(); + assert!(actual.strict_equals(expected.into())); + + eval(scope, "obj.getter_setter_key = 123").unwrap(); + assert_eq!( + obj + .get(scope, int_key.into()) + .unwrap() + .to_integer(scope) + .unwrap() + .value(), + 123 + ); + + assert_eq!(CALL_COUNT.load(Ordering::SeqCst), 2); + } +} + #[test] fn promise_resolved() { let _setup_guard = setup::parallel_test();