From 1b6e5cf79d1be903d3a9b0081ced570ec4f9adb7 Mon Sep 17 00:00:00 2001 From: ishell Date: Mon, 25 Mar 2024 15:45:41 +0100 Subject: [PATCH 1/2] src: do not use deprecated V8 API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Namely: - `v8::ObjectTemplate::SetAccessor(v8::Local, ...);` - `v8::ObjectTemplate::SetNativeDataProperty` with `AccessControl` Refs: https://github.com/v8/v8/commit/46c241eb99557fe8205acac5c526650c3847d180 Refs: https://github.com/v8/v8/commit/6ec883986bd417e2a42ddb960bd9449deb7e4639 Co-authored-by: Michaël Zasso --- src/base_object-inl.h | 6 +++--- src/base_object.h | 4 ++-- src/node_builtins.cc | 5 ----- src/node_external_reference.h | 2 -- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index da8fed7b3013..518b22dabef0 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -132,14 +132,14 @@ v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const { template void BaseObject::InternalFieldGet( - v8::Local property, + v8::Local property, const v8::PropertyCallbackInfo& info) { info.GetReturnValue().Set( info.This()->GetInternalField(Field).As()); } -template -void BaseObject::InternalFieldSet(v8::Local property, +template +void BaseObject::InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info) { // This could be e.g. value->IsFunction(). diff --git a/src/base_object.h b/src/base_object.h index b0ada1d7f38f..fd6d62a99ce0 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -111,10 +111,10 @@ class BaseObject : public MemoryRetainer { // Setter/Getter pair for internal fields that can be passed to SetAccessor. template - static void InternalFieldGet(v8::Local property, + static void InternalFieldGet(v8::Local property, const v8::PropertyCallbackInfo& info); template - static void InternalFieldSet(v8::Local property, + static void InternalFieldSet(v8::Local property, v8::Local value, const v8::PropertyCallbackInfo& info); diff --git a/src/node_builtins.cc b/src/node_builtins.cc index 81d3c9f7511e..1ffe2e13aae4 100644 --- a/src/node_builtins.cc +++ b/src/node_builtins.cc @@ -11,7 +11,6 @@ namespace node { namespace builtins { using v8::Context; -using v8::DEFAULT; using v8::EscapableHandleScope; using v8::Function; using v8::FunctionCallbackInfo; @@ -710,7 +709,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data, nullptr, Local(), None, - DEFAULT, SideEffectType::kHasNoSideEffect); target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "builtinIds"), @@ -718,7 +716,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data, nullptr, Local(), None, - DEFAULT, SideEffectType::kHasNoSideEffect); target->SetNativeDataProperty( @@ -727,7 +724,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data, nullptr, Local(), None, - DEFAULT, SideEffectType::kHasNoSideEffect); target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "natives"), @@ -735,7 +731,6 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data, nullptr, Local(), None, - DEFAULT, SideEffectType::kHasNoSideEffect); SetMethod(isolate, target, "getCacheUsage", BuiltinLoader::GetCacheUsage); diff --git a/src/node_external_reference.h b/src/node_external_reference.h index be4386dcf712..6bbeffb8941e 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -68,8 +68,6 @@ class ExternalReferenceRegistry { V(CFunctionWithBool) \ V(const v8::CFunctionInfo*) \ V(v8::FunctionCallback) \ - V(v8::AccessorGetterCallback) \ - V(v8::AccessorSetterCallback) \ V(v8::AccessorNameGetterCallback) \ V(v8::AccessorNameSetterCallback) \ V(v8::NamedPropertyGetterCallback) \ From 3f16877d741a764ab179fbadd35cbb69af88e55f Mon Sep 17 00:00:00 2001 From: Igor Sheludko Date: Tue, 30 Apr 2024 15:22:06 +0200 Subject: [PATCH 2/2] src: use new V8 API to define stream accessor Define XxxStream.prototype.onread as an accessor in JavaScript sense. Previously it was defined via soon-to-be-deprecated `v8::ObjectTemplate::SetAccessor(..)` which used to call setter even for property stores via stream object. The replacement V8 API `v8::ObjectTemplate::SetNativeDataProperty(..)` defines a properly behaving data property and thus a store to a stream object will not trigger the "onread" setter callback. In order to preserve the desired behavior of storing the value in the receiver's internal field the "onread" property should be defined as a proper JavaScript accessor. --- src/base_object-inl.h | 15 +++++++-------- src/base_object.h | 7 ++----- src/stream_base.cc | 36 +++++++++++++++++++++++++++++++----- src/stream_base.h | 7 +++++++ 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 518b22dabef0..61f30b3cfbdb 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -132,19 +132,18 @@ v8::EmbedderGraph::Node::Detachedness BaseObject::GetDetachedness() const { template void BaseObject::InternalFieldGet( - v8::Local property, - const v8::PropertyCallbackInfo& info) { - info.GetReturnValue().Set( - info.This()->GetInternalField(Field).As()); + const v8::FunctionCallbackInfo& args) { + args.GetReturnValue().Set( + args.This()->GetInternalField(Field).As()); } template -void BaseObject::InternalFieldSet(v8::Local property, - v8::Local value, - const v8::PropertyCallbackInfo& info) { +void BaseObject::InternalFieldSet( + const v8::FunctionCallbackInfo& args) { + v8::Local value = args[0]; // This could be e.g. value->IsFunction(). CHECK(((*value)->*typecheck)()); - info.This()->SetInternalField(Field, value); + args.This()->SetInternalField(Field, value); } bool BaseObject::has_pointer_data() const { diff --git a/src/base_object.h b/src/base_object.h index fd6d62a99ce0..546d968e5ca4 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -111,12 +111,9 @@ class BaseObject : public MemoryRetainer { // Setter/Getter pair for internal fields that can be passed to SetAccessor. template - static void InternalFieldGet(v8::Local property, - const v8::PropertyCallbackInfo& info); + static void InternalFieldGet(const v8::FunctionCallbackInfo& args); template - static void InternalFieldSet(v8::Local property, - v8::Local value, - const v8::PropertyCallbackInfo& info); + static void InternalFieldSet(const v8::FunctionCallbackInfo& args); // This is a bit of a hack. See the override in async_wrap.cc for details. virtual bool IsDoneInitializing() const; diff --git a/src/stream_base.cc b/src/stream_base.cc index af714734f49b..a8f9f98d413c 100644 --- a/src/stream_base.cc +++ b/src/stream_base.cc @@ -492,6 +492,29 @@ Local StreamBase::GetObject() { return GetAsyncWrap()->object(); } +void StreamBase::AddAccessor(v8::Isolate* isolate, + v8::Local signature, + enum v8::PropertyAttribute attributes, + v8::Local t, + JSMethodFunction* getter, + JSMethodFunction* setter, + v8::Local string) { + Local getter_templ = + NewFunctionTemplate(isolate, + getter, + signature, + ConstructorBehavior::kThrow, + SideEffectType::kHasNoSideEffect); + Local setter_templ = + NewFunctionTemplate(isolate, + setter, + signature, + ConstructorBehavior::kThrow, + SideEffectType::kHasSideEffect); + t->PrototypeTemplate()->SetAccessorProperty( + string, getter_templ, setter_templ, attributes); +} + void StreamBase::AddMethod(Isolate* isolate, Local signature, enum PropertyAttribute attributes, @@ -561,11 +584,14 @@ void StreamBase::AddMethods(IsolateData* isolate_data, JSMethod<&StreamBase::WriteString>); t->PrototypeTemplate()->Set(FIXED_ONE_BYTE_STRING(isolate, "isStreamBase"), True(isolate)); - t->PrototypeTemplate()->SetAccessor( - FIXED_ONE_BYTE_STRING(isolate, "onread"), - BaseObject::InternalFieldGet, - BaseObject::InternalFieldSet); + AddAccessor(isolate, + sig, + static_cast(DontDelete | DontEnum), + t, + BaseObject::InternalFieldGet, + BaseObject::InternalFieldSet, + FIXED_ONE_BYTE_STRING(isolate, "onread")); } void StreamBase::RegisterExternalReferences( diff --git a/src/stream_base.h b/src/stream_base.h index 62a8928e144a..ccbd769ceaf3 100644 --- a/src/stream_base.h +++ b/src/stream_base.h @@ -413,6 +413,13 @@ class StreamBase : public StreamResource { EmitToJSStreamListener default_listener_; void SetWriteResult(const StreamWriteResult& res); + static void AddAccessor(v8::Isolate* isolate, + v8::Local sig, + enum v8::PropertyAttribute attributes, + v8::Local t, + JSMethodFunction* getter, + JSMethodFunction* setter, + v8::Local str); static void AddMethod(v8::Isolate* isolate, v8::Local sig, enum v8::PropertyAttribute attributes,