Skip to content

Commit 79b2ba6

Browse files
Gabriel Schulhofcodebytere
Gabriel Schulhof
authored and
codebytere
committed
n-api: clean up binding creation
* Remove dead code for `GetterCallbackWrapper` and `SetterCallbackWrapper`. * Factor out creation of new `v8::Function`s. * Factor out creation of new `v8::FunctionTemplate`s. * Turn `CallbackBundle` into a class, internalizing creation of new instances and garbage collection. Signed-off-by: Gabriel Schulhof <gabriel.schulhof@intel.com> PR-URL: #36170 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com>
1 parent 5698cc0 commit 79b2ba6

File tree

1 file changed

+80
-183
lines changed

1 file changed

+80
-183
lines changed

‎src/js_native_api_v8.cc

+80-183
Original file line numberDiff line numberDiff line change
@@ -418,18 +418,36 @@ inline static napi_status Unwrap(napi_env env,
418418
// calling through N-API.
419419
// Ref: benchmark/misc/function_call
420420
// Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072
421-
struct CallbackBundle {
421+
class CallbackBundle {
422+
public:
423+
// Creates an object to be made available to the static function callback
424+
// wrapper, used to retrieve the native callback function and data pointer.
425+
static inline v8::Local<v8::Value>
426+
New(napi_env env, napi_callback cb, void* data) {
427+
CallbackBundle* bundle = new CallbackBundle();
428+
bundle->cb = cb;
429+
bundle->cb_data = data;
430+
bundle->env = env;
431+
432+
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
433+
Reference::New(env, cbdata, 0, true, Delete, bundle, nullptr);
434+
return cbdata;
435+
}
422436
napi_env env; // Necessary to invoke C++ NAPI callback
423437
void* cb_data; // The user provided callback data
424-
napi_callback function_or_getter;
425-
napi_callback setter;
438+
napi_callback cb;
439+
private:
440+
static void Delete(napi_env env, void* data, void* hint) {
441+
CallbackBundle* bundle = static_cast<CallbackBundle*>(data);
442+
delete bundle;
443+
}
426444
};
427445

428446
// Base class extended by classes that wrap V8 function and property callback
429447
// info.
430448
class CallbackWrapper {
431449
public:
432-
CallbackWrapper(napi_value this_arg, size_t args_length, void* data)
450+
inline CallbackWrapper(napi_value this_arg, size_t args_length, void* data)
433451
: _this(this_arg), _args_length(args_length), _data(data) {}
434452

435453
virtual napi_value GetNewTarget() = 0;
@@ -448,10 +466,10 @@ class CallbackWrapper {
448466
void* _data;
449467
};
450468

451-
template <typename Info, napi_callback CallbackBundle::*FunctionField>
452469
class CallbackWrapperBase : public CallbackWrapper {
453470
public:
454-
CallbackWrapperBase(const Info& cbinfo, const size_t args_length)
471+
inline CallbackWrapperBase(const v8::FunctionCallbackInfo<v8::Value>& cbinfo,
472+
const size_t args_length)
455473
: CallbackWrapper(JsValueFromV8LocalValue(cbinfo.This()),
456474
args_length,
457475
nullptr),
@@ -461,16 +479,14 @@ class CallbackWrapperBase : public CallbackWrapper {
461479
_data = _bundle->cb_data;
462480
}
463481

464-
napi_value GetNewTarget() override { return nullptr; }
465-
466482
protected:
467-
void InvokeCallback() {
483+
inline void InvokeCallback() {
468484
napi_callback_info cbinfo_wrapper = reinterpret_cast<napi_callback_info>(
469485
static_cast<CallbackWrapper*>(this));
470486

471487
// All other pointers we need are stored in `_bundle`
472488
napi_env env = _bundle->env;
473-
napi_callback cb = _bundle->*FunctionField;
489+
napi_callback cb = _bundle->cb;
474490

475491
napi_value result;
476492
env->CallIntoModule([&](napi_env env) {
@@ -482,19 +498,45 @@ class CallbackWrapperBase : public CallbackWrapper {
482498
}
483499
}
484500

485-
const Info& _cbinfo;
501+
const v8::FunctionCallbackInfo<v8::Value>& _cbinfo;
486502
CallbackBundle* _bundle;
487503
};
488504

489505
class FunctionCallbackWrapper
490-
: public CallbackWrapperBase<v8::FunctionCallbackInfo<v8::Value>,
491-
&CallbackBundle::function_or_getter> {
506+
: public CallbackWrapperBase {
492507
public:
493508
static void Invoke(const v8::FunctionCallbackInfo<v8::Value>& info) {
494509
FunctionCallbackWrapper cbwrapper(info);
495510
cbwrapper.InvokeCallback();
496511
}
497512

513+
static inline napi_status NewFunction(napi_env env,
514+
napi_callback cb,
515+
void* cb_data,
516+
v8::Local<v8::Function>* result) {
517+
v8::Local<v8::Value> cbdata = v8impl::CallbackBundle::New(env, cb, cb_data);
518+
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
519+
520+
v8::MaybeLocal<v8::Function> maybe_function =
521+
v8::Function::New(env->context(), Invoke, cbdata);
522+
CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure);
523+
524+
*result = maybe_function.ToLocalChecked();
525+
return napi_clear_last_error(env);
526+
}
527+
528+
static inline napi_status NewTemplate(napi_env env,
529+
napi_callback cb,
530+
void* cb_data,
531+
v8::Local<v8::FunctionTemplate>* result,
532+
v8::Local<v8::Signature> sig = v8::Local<v8::Signature>()) {
533+
v8::Local<v8::Value> cbdata = v8impl::CallbackBundle::New(env, cb, cb_data);
534+
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
535+
536+
*result = v8::FunctionTemplate::New(env->isolate, Invoke, cbdata, sig);
537+
return napi_clear_last_error(env);
538+
}
539+
498540
explicit FunctionCallbackWrapper(
499541
const v8::FunctionCallbackInfo<v8::Value>& cbinfo)
500542
: CallbackWrapperBase(cbinfo, cbinfo.Length()) {}
@@ -532,98 +574,6 @@ class FunctionCallbackWrapper
532574
}
533575
};
534576

535-
class GetterCallbackWrapper
536-
: public CallbackWrapperBase<v8::PropertyCallbackInfo<v8::Value>,
537-
&CallbackBundle::function_or_getter> {
538-
public:
539-
static void Invoke(v8::Local<v8::Name> property,
540-
const v8::PropertyCallbackInfo<v8::Value>& info) {
541-
GetterCallbackWrapper cbwrapper(info);
542-
cbwrapper.InvokeCallback();
543-
}
544-
545-
explicit GetterCallbackWrapper(
546-
const v8::PropertyCallbackInfo<v8::Value>& cbinfo)
547-
: CallbackWrapperBase(cbinfo, 0) {}
548-
549-
/*virtual*/
550-
void Args(napi_value* buffer, size_t buffer_length) override {
551-
if (buffer_length > 0) {
552-
napi_value undefined =
553-
v8impl::JsValueFromV8LocalValue(v8::Undefined(_cbinfo.GetIsolate()));
554-
for (size_t i = 0; i < buffer_length; i += 1) {
555-
buffer[i] = undefined;
556-
}
557-
}
558-
}
559-
560-
/*virtual*/
561-
void SetReturnValue(napi_value value) override {
562-
v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
563-
_cbinfo.GetReturnValue().Set(val);
564-
}
565-
};
566-
567-
class SetterCallbackWrapper
568-
: public CallbackWrapperBase<v8::PropertyCallbackInfo<void>,
569-
&CallbackBundle::setter> {
570-
public:
571-
static void Invoke(v8::Local<v8::Name> property,
572-
v8::Local<v8::Value> value,
573-
const v8::PropertyCallbackInfo<void>& info) {
574-
SetterCallbackWrapper cbwrapper(info, value);
575-
cbwrapper.InvokeCallback();
576-
}
577-
578-
SetterCallbackWrapper(const v8::PropertyCallbackInfo<void>& cbinfo,
579-
const v8::Local<v8::Value>& value)
580-
: CallbackWrapperBase(cbinfo, 1), _value(value) {}
581-
582-
/*virtual*/
583-
void Args(napi_value* buffer, size_t buffer_length) override {
584-
if (buffer_length > 0) {
585-
buffer[0] = v8impl::JsValueFromV8LocalValue(_value);
586-
587-
if (buffer_length > 1) {
588-
napi_value undefined = v8impl::JsValueFromV8LocalValue(
589-
v8::Undefined(_cbinfo.GetIsolate()));
590-
for (size_t i = 1; i < buffer_length; i += 1) {
591-
buffer[i] = undefined;
592-
}
593-
}
594-
}
595-
}
596-
597-
/*virtual*/
598-
void SetReturnValue(napi_value value) override {
599-
// Ignore any value returned from a setter callback.
600-
}
601-
602-
private:
603-
const v8::Local<v8::Value>& _value;
604-
};
605-
606-
static void DeleteCallbackBundle(napi_env env, void* data, void* hint) {
607-
CallbackBundle* bundle = static_cast<CallbackBundle*>(data);
608-
delete bundle;
609-
}
610-
611-
// Creates an object to be made available to the static function callback
612-
// wrapper, used to retrieve the native callback function and data pointer.
613-
static
614-
v8::Local<v8::Value> CreateFunctionCallbackData(napi_env env,
615-
napi_callback cb,
616-
void* data) {
617-
CallbackBundle* bundle = new CallbackBundle();
618-
bundle->function_or_getter = cb;
619-
bundle->cb_data = data;
620-
bundle->env = env;
621-
v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
622-
Reference::New(env, cbdata, 0, true, DeleteCallbackBundle, bundle, nullptr);
623-
624-
return cbdata;
625-
}
626-
627577
enum WrapType {
628578
retrievable,
629579
anonymous
@@ -745,22 +695,12 @@ napi_status napi_create_function(napi_env env,
745695
CHECK_ARG(env, result);
746696
CHECK_ARG(env, cb);
747697

748-
v8::Isolate* isolate = env->isolate;
749698
v8::Local<v8::Function> return_value;
750-
v8::EscapableHandleScope scope(isolate);
751-
v8::Local<v8::Value> cbdata =
752-
v8impl::CreateFunctionCallbackData(env, cb, callback_data);
753-
754-
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
755-
756-
v8::Local<v8::Context> context = env->context();
757-
v8::MaybeLocal<v8::Function> maybe_function =
758-
v8::Function::New(context,
759-
v8impl::FunctionCallbackWrapper::Invoke,
760-
cbdata);
761-
CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure);
762-
763-
return_value = scope.Escape(maybe_function.ToLocalChecked());
699+
v8::EscapableHandleScope scope(env->isolate);
700+
v8::Local<v8::Function> fn;
701+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
702+
env, cb, callback_data, &fn));
703+
return_value = scope.Escape(fn);
764704

765705
if (utf8name != nullptr) {
766706
v8::Local<v8::String> name_string;
@@ -792,13 +732,9 @@ napi_status napi_define_class(napi_env env,
792732
v8::Isolate* isolate = env->isolate;
793733

794734
v8::EscapableHandleScope scope(isolate);
795-
v8::Local<v8::Value> cbdata =
796-
v8impl::CreateFunctionCallbackData(env, constructor, callback_data);
797-
798-
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
799-
800-
v8::Local<v8::FunctionTemplate> tpl = v8::FunctionTemplate::New(
801-
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
735+
v8::Local<v8::FunctionTemplate> tpl;
736+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
737+
env, constructor, callback_data, &tpl));
802738

803739
v8::Local<v8::String> name_string;
804740
CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length);
@@ -828,18 +764,12 @@ napi_status napi_define_class(napi_env env,
828764
v8::Local<v8::FunctionTemplate> getter_tpl;
829765
v8::Local<v8::FunctionTemplate> setter_tpl;
830766
if (p->getter != nullptr) {
831-
v8::Local<v8::Value> getter_data =
832-
v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
833-
834-
getter_tpl = v8::FunctionTemplate::New(
835-
isolate, v8impl::FunctionCallbackWrapper::Invoke, getter_data);
767+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
768+
env, p->getter, p->data, &getter_tpl));
836769
}
837770
if (p->setter != nullptr) {
838-
v8::Local<v8::Value> setter_data =
839-
v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
840-
841-
setter_tpl = v8::FunctionTemplate::New(
842-
isolate, v8impl::FunctionCallbackWrapper::Invoke, setter_data);
771+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
772+
env, p->setter, p->data, &setter_tpl));
843773
}
844774

845775
tpl->PrototypeTemplate()->SetAccessorProperty(
@@ -849,16 +779,9 @@ napi_status napi_define_class(napi_env env,
849779
attributes,
850780
v8::AccessControl::DEFAULT);
851781
} else if (p->method != nullptr) {
852-
v8::Local<v8::Value> cbdata =
853-
v8impl::CreateFunctionCallbackData(env, p->method, p->data);
854-
855-
RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
856-
857-
v8::Local<v8::FunctionTemplate> t =
858-
v8::FunctionTemplate::New(isolate,
859-
v8impl::FunctionCallbackWrapper::Invoke,
860-
cbdata,
861-
v8::Signature::New(isolate, tpl));
782+
v8::Local<v8::FunctionTemplate> t;
783+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
784+
env, p->method, p->data, &t, v8::Signature::New(isolate, tpl)));
862785

863786
tpl->PrototypeTemplate()->Set(property_name, t, attributes);
864787
} else {
@@ -1263,33 +1186,16 @@ napi_status napi_define_properties(napi_env env,
12631186
STATUS_CALL(v8impl::V8NameFromPropertyDescriptor(env, p, &property_name));
12641187

12651188
if (p->getter != nullptr || p->setter != nullptr) {
1266-
v8::Local<v8::Value> local_getter;
1267-
v8::Local<v8::Value> local_setter;
1189+
v8::Local<v8::Function> local_getter;
1190+
v8::Local<v8::Function> local_setter;
12681191

12691192
if (p->getter != nullptr) {
1270-
v8::Local<v8::Value> getter_data =
1271-
v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
1272-
CHECK_MAYBE_EMPTY(env, getter_data, napi_generic_failure);
1273-
1274-
v8::MaybeLocal<v8::Function> maybe_getter =
1275-
v8::Function::New(context,
1276-
v8impl::FunctionCallbackWrapper::Invoke,
1277-
getter_data);
1278-
CHECK_MAYBE_EMPTY(env, maybe_getter, napi_generic_failure);
1279-
1280-
local_getter = maybe_getter.ToLocalChecked();
1193+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
1194+
env, p->getter, p->data, &local_getter));
12811195
}
12821196
if (p->setter != nullptr) {
1283-
v8::Local<v8::Value> setter_data =
1284-
v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
1285-
CHECK_MAYBE_EMPTY(env, setter_data, napi_generic_failure);
1286-
1287-
v8::MaybeLocal<v8::Function> maybe_setter =
1288-
v8::Function::New(context,
1289-
v8impl::FunctionCallbackWrapper::Invoke,
1290-
setter_data);
1291-
CHECK_MAYBE_EMPTY(env, maybe_setter, napi_generic_failure);
1292-
local_setter = maybe_setter.ToLocalChecked();
1197+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
1198+
env, p->setter, p->data, &local_setter));
12931199
}
12941200

12951201
v8::PropertyDescriptor descriptor(local_getter, local_setter);
@@ -1304,19 +1210,10 @@ napi_status napi_define_properties(napi_env env,
13041210
return napi_set_last_error(env, napi_invalid_arg);
13051211
}
13061212
} else if (p->method != nullptr) {
1307-
v8::Local<v8::Value> cbdata =
1308-
v8impl::CreateFunctionCallbackData(env, p->method, p->data);
1309-
1310-
CHECK_MAYBE_EMPTY(env, cbdata, napi_generic_failure);
1311-
1312-
v8::MaybeLocal<v8::Function> maybe_fn =
1313-
v8::Function::New(context,
1314-
v8impl::FunctionCallbackWrapper::Invoke,
1315-
cbdata);
1316-
1317-
CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure);
1318-
1319-
v8::PropertyDescriptor descriptor(maybe_fn.ToLocalChecked(),
1213+
v8::Local<v8::Function> method;
1214+
STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
1215+
env, p->method, p->data, &method));
1216+
v8::PropertyDescriptor descriptor(method,
13201217
(p->attributes & napi_writable) != 0);
13211218
descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
13221219
descriptor.set_configurable((p->attributes & napi_configurable) != 0);

0 commit comments

Comments
 (0)