Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit 79b2ba6

Browse filesBrowse files
Gabriel Schulhofcodebytere
authored andcommitted
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
Copy full SHA for 79b2ba6

File tree

Expand file treeCollapse file tree

1 file changed

+80
-183
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

1 file changed

+80
-183
lines changed
Open diff view settings
Collapse file

‎src/js_native_api_v8.cc‎

Copy file name to clipboardExpand all lines: src/js_native_api_v8.cc
+80-183Lines changed: 80 additions & 183 deletions
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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.