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

Author: Gabriel Schulhof

src/js_native_api_v8.cc

@@ -419,18 +419,36 @@ inline static napi_status Unwrap(napi_env env,
 // calling through N-API.
 // Ref: benchmark/misc/function_call
 // Discussion (incl. perf. data): https://github.com/nodejs/node/pull/21072
-struct CallbackBundle {
+class CallbackBundle {
+ public:
+ // Creates an object to be made available to the static function callback
+ // wrapper, used to retrieve the native callback function and data pointer.
+ static inline v8::Local<v8::Value>
+ New(napi_env env, napi_callback cb, void* data) {
+ CallbackBundle* bundle = new CallbackBundle();
+ bundle->cb = cb;
+ bundle->cb_data = data;
+ bundle->env = env;
+
+ v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
+ Reference::New(env, cbdata, 0, true, Delete, bundle, nullptr);
+ return cbdata;
+ }
  napi_env env; // Necessary to invoke C++ NAPI callback
  void* cb_data; // The user provided callback data
- napi_callback function_or_getter;
- napi_callback setter;
+ napi_callback cb;
+ private:
+ static void Delete(napi_env env, void* data, void* hint) {
+ CallbackBundle* bundle = static_cast<CallbackBundle*>(data);
+ delete bundle;
+ }
 };
 
 // Base class extended by classes that wrap V8 function and property callback
 // info.
 class CallbackWrapper {
  public:
- CallbackWrapper(napi_value this_arg, size_t args_length, void* data)
+ inline CallbackWrapper(napi_value this_arg, size_t args_length, void* data)
  : _this(this_arg), _args_length(args_length), _data(data) {}
 
  virtual napi_value GetNewTarget() = 0;
@@ -449,10 +467,10 @@ class CallbackWrapper {
  void* _data;
 };
 
-template <typename Info, napi_callback CallbackBundle::*FunctionField>
 class CallbackWrapperBase : public CallbackWrapper {
  public:
- CallbackWrapperBase(const Info& cbinfo, const size_t args_length)
+ inline CallbackWrapperBase(const v8::FunctionCallbackInfo<v8::Value>& cbinfo,
+ const size_t args_length)
  : CallbackWrapper(JsValueFromV8LocalValue(cbinfo.This()),
  args_length,
  nullptr),
@@ -462,16 +480,14 @@ class CallbackWrapperBase : public CallbackWrapper {
  _data = _bundle->cb_data;
  }
 
- napi_value GetNewTarget() override { return nullptr; }
-
  protected:
- void InvokeCallback() {
+ inline void InvokeCallback() {
  napi_callback_info cbinfo_wrapper = reinterpret_cast<napi_callback_info>(
  static_cast<CallbackWrapper*>(this));
 
  // All other pointers we need are stored in `_bundle`
  napi_env env = _bundle->env;
- napi_callback cb = _bundle->*FunctionField;
+ napi_callback cb = _bundle->cb;
 
  napi_value result;
  env->CallIntoModule([&](napi_env env) {
@@ -483,19 +499,45 @@ class CallbackWrapperBase : public CallbackWrapper {
  }
  }
 
- const Info& _cbinfo;
+ const v8::FunctionCallbackInfo<v8::Value>& _cbinfo;
  CallbackBundle* _bundle;
 };
 
 class FunctionCallbackWrapper
- : public CallbackWrapperBase<v8::FunctionCallbackInfo<v8::Value>,
- &CallbackBundle::function_or_getter> {
+ : public CallbackWrapperBase {
  public:
  static void Invoke(const v8::FunctionCallbackInfo<v8::Value>& info) {
  FunctionCallbackWrapper cbwrapper(info);
  cbwrapper.InvokeCallback();
  }
 
+ static inline napi_status NewFunction(napi_env env,
+ napi_callback cb,
+ void* cb_data,
+ v8::Local<v8::Function>* result) {
+ v8::Local<v8::Value> cbdata = v8impl::CallbackBundle::New(env, cb, cb_data);
+ RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
+
+ v8::MaybeLocal<v8::Function> maybe_function =
+ v8::Function::New(env->context(), Invoke, cbdata);
+ CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure);
+
+ *result = maybe_function.ToLocalChecked();
+ return napi_clear_last_error(env);
+ }
+
+ static inline napi_status NewTemplate(napi_env env,
+ napi_callback cb,
+ void* cb_data,
+ v8::Local<v8::FunctionTemplate>* result,
+ v8::Local<v8::Signature> sig = v8::Local<v8::Signature>()) {
+ v8::Local<v8::Value> cbdata = v8impl::CallbackBundle::New(env, cb, cb_data);
+ RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
+
+ *result = v8::FunctionTemplate::New(env->isolate, Invoke, cbdata, sig);
+ return napi_clear_last_error(env);
+ }
+
  explicit FunctionCallbackWrapper(
  const v8::FunctionCallbackInfo<v8::Value>& cbinfo)
  : CallbackWrapperBase(cbinfo, cbinfo.Length()) {}
@@ -533,98 +575,6 @@ class FunctionCallbackWrapper
  }
 };
 
-class GetterCallbackWrapper
- : public CallbackWrapperBase<v8::PropertyCallbackInfo<v8::Value>,
- &CallbackBundle::function_or_getter> {
- public:
- static void Invoke(v8::Local<v8::Name> property,
- const v8::PropertyCallbackInfo<v8::Value>& info) {
- GetterCallbackWrapper cbwrapper(info);
- cbwrapper.InvokeCallback();
- }
-
- explicit GetterCallbackWrapper(
- const v8::PropertyCallbackInfo<v8::Value>& cbinfo)
- : CallbackWrapperBase(cbinfo, 0) {}
-
- /*virtual*/
- void Args(napi_value* buffer, size_t buffer_length) override {
- if (buffer_length > 0) {
- napi_value undefined =
- v8impl::JsValueFromV8LocalValue(v8::Undefined(_cbinfo.GetIsolate()));
- for (size_t i = 0; i < buffer_length; i += 1) {
- buffer[i] = undefined;
- }
- }
- }
-
- /*virtual*/
- void SetReturnValue(napi_value value) override {
- v8::Local<v8::Value> val = v8impl::V8LocalValueFromJsValue(value);
- _cbinfo.GetReturnValue().Set(val);
- }
-};
-
-class SetterCallbackWrapper
- : public CallbackWrapperBase<v8::PropertyCallbackInfo<void>,
- &CallbackBundle::setter> {
- public:
- static void Invoke(v8::Local<v8::Name> property,
- v8::Local<v8::Value> value,
- const v8::PropertyCallbackInfo<void>& info) {
- SetterCallbackWrapper cbwrapper(info, value);
- cbwrapper.InvokeCallback();
- }
-
- SetterCallbackWrapper(const v8::PropertyCallbackInfo<void>& cbinfo,
- const v8::Local<v8::Value>& value)
- : CallbackWrapperBase(cbinfo, 1), _value(value) {}
-
- /*virtual*/
- void Args(napi_value* buffer, size_t buffer_length) override {
- if (buffer_length > 0) {
- buffer[0] = v8impl::JsValueFromV8LocalValue(_value);
-
- if (buffer_length > 1) {
- napi_value undefined = v8impl::JsValueFromV8LocalValue(
- v8::Undefined(_cbinfo.GetIsolate()));
- for (size_t i = 1; i < buffer_length; i += 1) {
- buffer[i] = undefined;
- }
- }
- }
- }
-
- /*virtual*/
- void SetReturnValue(napi_value value) override {
- // Ignore any value returned from a setter callback.
- }
-
- private:
- const v8::Local<v8::Value>& _value;
-};
-
-static void DeleteCallbackBundle(napi_env env, void* data, void* hint) {
- CallbackBundle* bundle = static_cast<CallbackBundle*>(data);
- delete bundle;
-}
-
-// Creates an object to be made available to the static function callback
-// wrapper, used to retrieve the native callback function and data pointer.
-static
-v8::Local<v8::Value> CreateFunctionCallbackData(napi_env env,
- napi_callback cb,
- void* data) {
- CallbackBundle* bundle = new CallbackBundle();
- bundle->function_or_getter = cb;
- bundle->cb_data = data;
- bundle->env = env;
- v8::Local<v8::Value> cbdata = v8::External::New(env->isolate, bundle);
- Reference::New(env, cbdata, 0, true, DeleteCallbackBundle, bundle, nullptr);
-
- return cbdata;
-}
-
 enum WrapType {
  retrievable,
  anonymous
@@ -746,22 +696,12 @@ napi_status napi_create_function(napi_env env,
  CHECK_ARG(env, result);
  CHECK_ARG(env, cb);
 
- v8::Isolate* isolate = env->isolate;
  v8::Local<v8::Function> return_value;
- v8::EscapableHandleScope scope(isolate);
- v8::Local<v8::Value> cbdata =
- v8impl::CreateFunctionCallbackData(env, cb, callback_data);
-
- RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
-
- v8::Local<v8::Context> context = env->context();
- v8::MaybeLocal<v8::Function> maybe_function =
- v8::Function::New(context,
- v8impl::FunctionCallbackWrapper::Invoke,
- cbdata);
- CHECK_MAYBE_EMPTY(env, maybe_function, napi_generic_failure);
-
- return_value = scope.Escape(maybe_function.ToLocalChecked());
+ v8::EscapableHandleScope scope(env->isolate);
+ v8::Local<v8::Function> fn;
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
+ env, cb, callback_data, &fn));
+ return_value = scope.Escape(fn);
 
  if (utf8name != nullptr) {
  v8::Local<v8::String> name_string;
@@ -793,13 +733,9 @@ napi_status napi_define_class(napi_env env,
  v8::Isolate* isolate = env->isolate;
 
  v8::EscapableHandleScope scope(isolate);
- v8::Local<v8::Value> cbdata =
- v8impl::CreateFunctionCallbackData(env, constructor, callback_data);
-
- RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
-
- v8::Local<v8::FunctionTemplate> tpl = v8::FunctionTemplate::New(
- isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);
+ v8::Local<v8::FunctionTemplate> tpl;
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
+ env, constructor, callback_data, &tpl));
 
  v8::Local<v8::String> name_string;
  CHECK_NEW_FROM_UTF8_LEN(env, name_string, utf8name, length);
@@ -829,18 +765,12 @@ napi_status napi_define_class(napi_env env,
  v8::Local<v8::FunctionTemplate> getter_tpl;
  v8::Local<v8::FunctionTemplate> setter_tpl;
  if (p->getter != nullptr) {
- v8::Local<v8::Value> getter_data =
- v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
-
- getter_tpl = v8::FunctionTemplate::New(
- isolate, v8impl::FunctionCallbackWrapper::Invoke, getter_data);
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
+ env, p->getter, p->data, &getter_tpl));
  }
  if (p->setter != nullptr) {
- v8::Local<v8::Value> setter_data =
- v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
-
- setter_tpl = v8::FunctionTemplate::New(
- isolate, v8impl::FunctionCallbackWrapper::Invoke, setter_data);
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
+ env, p->setter, p->data, &setter_tpl));
  }
 
  tpl->PrototypeTemplate()->SetAccessorProperty(
@@ -850,16 +780,9 @@ napi_status napi_define_class(napi_env env,
  attributes,
  v8::AccessControl::DEFAULT);
  } else if (p->method != nullptr) {
- v8::Local<v8::Value> cbdata =
- v8impl::CreateFunctionCallbackData(env, p->method, p->data);
-
- RETURN_STATUS_IF_FALSE(env, !cbdata.IsEmpty(), napi_generic_failure);
-
- v8::Local<v8::FunctionTemplate> t =
- v8::FunctionTemplate::New(isolate,
- v8impl::FunctionCallbackWrapper::Invoke,
- cbdata,
- v8::Signature::New(isolate, tpl));
+ v8::Local<v8::FunctionTemplate> t;
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewTemplate(
+ env, p->method, p->data, &t, v8::Signature::New(isolate, tpl)));
 
  tpl->PrototypeTemplate()->Set(property_name, t, attributes);
  } else {
@@ -1264,33 +1187,16 @@ napi_status napi_define_properties(napi_env env,
  STATUS_CALL(v8impl::V8NameFromPropertyDescriptor(env, p, &property_name));
 
  if (p->getter != nullptr || p->setter != nullptr) {
- v8::Local<v8::Value> local_getter;
- v8::Local<v8::Value> local_setter;
+ v8::Local<v8::Function> local_getter;
+ v8::Local<v8::Function> local_setter;
 
  if (p->getter != nullptr) {
- v8::Local<v8::Value> getter_data =
- v8impl::CreateFunctionCallbackData(env, p->getter, p->data);
- CHECK_MAYBE_EMPTY(env, getter_data, napi_generic_failure);
-
- v8::MaybeLocal<v8::Function> maybe_getter =
- v8::Function::New(context,
- v8impl::FunctionCallbackWrapper::Invoke,
- getter_data);
- CHECK_MAYBE_EMPTY(env, maybe_getter, napi_generic_failure);
-
- local_getter = maybe_getter.ToLocalChecked();
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
+ env, p->getter, p->data, &local_getter));
  }
  if (p->setter != nullptr) {
- v8::Local<v8::Value> setter_data =
- v8impl::CreateFunctionCallbackData(env, p->setter, p->data);
- CHECK_MAYBE_EMPTY(env, setter_data, napi_generic_failure);
-
- v8::MaybeLocal<v8::Function> maybe_setter =
- v8::Function::New(context,
- v8impl::FunctionCallbackWrapper::Invoke,
- setter_data);
- CHECK_MAYBE_EMPTY(env, maybe_setter, napi_generic_failure);
- local_setter = maybe_setter.ToLocalChecked();
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
+ env, p->setter, p->data, &local_setter));
  }
 
  v8::PropertyDescriptor descriptor(local_getter, local_setter);
@@ -1305,19 +1211,10 @@ napi_status napi_define_properties(napi_env env,
  return napi_set_last_error(env, napi_invalid_arg);
  }
  } else if (p->method != nullptr) {
- v8::Local<v8::Value> cbdata =
- v8impl::CreateFunctionCallbackData(env, p->method, p->data);
-
- CHECK_MAYBE_EMPTY(env, cbdata, napi_generic_failure);
-
- v8::MaybeLocal<v8::Function> maybe_fn =
- v8::Function::New(context,
- v8impl::FunctionCallbackWrapper::Invoke,
- cbdata);
-
- CHECK_MAYBE_EMPTY(env, maybe_fn, napi_generic_failure);
-
- v8::PropertyDescriptor descriptor(maybe_fn.ToLocalChecked(),
+ v8::Local<v8::Function> method;
+ STATUS_CALL(v8impl::FunctionCallbackWrapper::NewFunction(
+ env, p->method, p->data, &method));
+ v8::PropertyDescriptor descriptor(method,
  (p->attributes & napi_writable) != 0);
  descriptor.set_enumerable((p->attributes & napi_enumerable) != 0);
  descriptor.set_configurable((p->attributes & napi_configurable) != 0);