Skip to content
Permalink
Browse files

n-api: change napi_callback to return napi_value

Change `napi_callback` to return `napi_value` directly instead of
requiring `napi_set_return_value`.

When we invoke the callback, we will check the return value and
call `SetReturnValue` ourselves. If the callback returns `NULL`,
we don't set the return value in v8 which would have the same
effect as previously if the callback didn't call
`napi_set_return_value`. Seems to be a more natural way
to handle return values from callbacks. As a consequence,
remove `napi_set_return_value`.

Add a `napi_value` to `napi_property_descriptor` to support string
values which couldn't be passed in the `utf8name` parameter or
symbols as property names. Class names, however, cannot be symbols
so this `napi_value` must be a string type in that case.

Remove all of the `napi_callback_info` helpers except for
`napi_get_cb_info` and make all the parameters to
`napi_get_cb_info` optional except for argc.

Update all the test collateral according to these changes.
Also add `test/addons-napi/common.h` to house some common macros
for wrapping N-API calls and error handling.

PR-URL: #12248
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information...
boingoing authored and addaleax committed Mar 24, 2017
1 parent b470a85 commit ca786c3734f6e23e34cd60f13e6bdaab033c5739
Showing with 939 additions and 1,590 deletions.
  1. +121 −154 src/node_api.cc
  2. +2 −19 src/node_api.h
  3. +5 −2 src/node_api_types.h
  4. +9 −13 test/addons-napi/1_hello_world/binding.c
  5. +16 −37 test/addons-napi/2_function_arguments/binding.c
  6. +18 −26 test/addons-napi/3_callbacks/binding.c
  7. +10 −18 test/addons-napi/4_object_factory/binding.c
  8. +10 −21 test/addons-napi/5_function_factory/binding.c
  9. +72 −124 test/addons-napi/6_object_wrap/myobject.cc
  10. +5 −5 test/addons-napi/6_object_wrap/myobject.h
  11. +10 −19 test/addons-napi/7_factory_wrap/binding.cc
  12. +23 −39 test/addons-napi/7_factory_wrap/myobject.cc
  13. +2 −2 test/addons-napi/7_factory_wrap/myobject.h
  14. +18 −31 test/addons-napi/8_passing_wrapped/binding.cc
  15. +15 −23 test/addons-napi/8_passing_wrapped/myobject.cc
  16. +1 −1 test/addons-napi/8_passing_wrapped/myobject.h
  17. +52 −0 test/addons-napi/common.h
  18. +52 −96 test/addons-napi/test_array/test_array.c
  19. +49 −65 test/addons-napi/test_buffer/test_buffer.c
  20. +39 −72 test/addons-napi/test_constructor/test_constructor.c
  21. +80 −166 test/addons-napi/test_conversions/test_conversions.c
  22. +11 −21 test/addons-napi/test_error/test_error.cc
  23. +28 −44 test/addons-napi/test_exception/test_exception.c
  24. +15 −35 test/addons-napi/test_function/test_function.c
  25. +11 −22 test/addons-napi/test_instanceof/test_instanceof.c
  26. +17 −27 test/addons-napi/test_napi_status/test_napi_status.cc
  27. +16 −36 test/addons-napi/test_number/test_number.c
  28. +77 −169 test/addons-napi/test_object/test_object.c
  29. +31 −60 test/addons-napi/test_properties/test_properties.c
  30. +42 −91 test/addons-napi/test_string/test_string.c
  31. +33 −67 test/addons-napi/test_symbol/test_symbol.c
  32. +49 −85 test/addons-napi/test_typedarray/test_typedarray.c
@@ -36,6 +36,70 @@ class napi_env__ {
napi_extended_error_info last_error;
};

#define RETURN_STATUS_IF_FALSE(env, condition, status) \
do { \
if (!(condition)) { \
return napi_set_last_error((env), (status)); \
} \
} while (0)

#define CHECK_ENV(env) \
if ((env) == nullptr) { \
node::FatalError(__func__, "environment(env) must not be null"); \
}

#define CHECK_ARG(env, arg) \
RETURN_STATUS_IF_FALSE((env), ((arg) != nullptr), napi_invalid_arg)

#define CHECK_MAYBE_EMPTY(env, maybe, status) \
RETURN_STATUS_IF_FALSE((env), !((maybe).IsEmpty()), (status))

#define CHECK_MAYBE_NOTHING(env, maybe, status) \
RETURN_STATUS_IF_FALSE((env), !((maybe).IsNothing()), (status))

// NAPI_PREAMBLE is not wrapped in do..while: try_catch must have function scope
#define NAPI_PREAMBLE(env) \
CHECK_ENV((env)); \
RETURN_STATUS_IF_FALSE((env), (env)->last_exception.IsEmpty(), \
napi_pending_exception); \
napi_clear_last_error((env)); \
v8impl::TryCatch try_catch((env))

#define CHECK_TO_TYPE(env, type, context, result, src, status) \
do { \
auto maybe = v8impl::V8LocalValueFromJsValue((src))->To##type((context)); \
CHECK_MAYBE_EMPTY((env), maybe, (status)); \
(result) = maybe.ToLocalChecked(); \
} while (0)

#define CHECK_TO_OBJECT(env, context, result, src) \
CHECK_TO_TYPE((env), Object, (context), (result), (src), napi_object_expected)

#define CHECK_TO_STRING(env, context, result, src) \
CHECK_TO_TYPE((env), String, (context), (result), (src), napi_string_expected)

#define CHECK_TO_NUMBER(env, context, result, src) \
CHECK_TO_TYPE((env), Number, (context), (result), (src), napi_number_expected)

#define CHECK_TO_BOOL(env, context, result, src) \
CHECK_TO_TYPE((env), Boolean, (context), (result), (src), \
napi_boolean_expected)

#define CHECK_NEW_FROM_UTF8_LEN(env, result, str, len) \
do { \
auto str_maybe = v8::String::NewFromUtf8( \
(env)->isolate, (str), v8::NewStringType::kInternalized, (len)); \
CHECK_MAYBE_EMPTY((env), str_maybe, napi_generic_failure); \
(result) = str_maybe.ToLocalChecked(); \
} while (0)

#define CHECK_NEW_FROM_UTF8(env, result, str) \
CHECK_NEW_FROM_UTF8_LEN((env), (result), (str), -1)

#define GET_RETURN_STATUS(env) \
(!try_catch.HasCaught() ? napi_ok \
: napi_set_last_error((env), napi_pending_exception))

namespace v8impl {

// convert from n-api property attributes to v8::PropertyAttribute
@@ -127,6 +191,22 @@ v8::Local<v8::Value> V8LocalValueFromJsValue(napi_value v) {
return local;
}

static inline napi_status V8NameFromPropertyDescriptor(napi_env env,
const napi_property_descriptor* p,
v8::Local<v8::Name>* result) {
if (p->utf8name != nullptr) {
CHECK_NEW_FROM_UTF8(env, *result, p->utf8name);
} else {
v8::Local<v8::Value> property_value =
v8impl::V8LocalValueFromJsValue(p->name);

RETURN_STATUS_IF_FALSE(env, property_value->IsName(), napi_name_expected);
*result = property_value.As<v8::Name>();
}

return napi_ok;
}

// Adapter for napi_finalize callbacks.
class Finalizer {
protected:
@@ -361,13 +441,19 @@ class CallbackWrapperBase : public CallbackWrapper {
v8::Local<v8::External>::Cast(
_cbdata->GetInternalField(kInternalFieldIndex))->Value());
v8::Isolate* isolate = _cbinfo.GetIsolate();

napi_env env = static_cast<napi_env>(
v8::Local<v8::External>::Cast(
_cbdata->GetInternalField(kEnvIndex))->Value());

// Make sure any errors encountered last time we were in N-API are gone.
napi_clear_last_error(env);
cb(env, cbinfo_wrapper);

napi_value result = cb(env, cbinfo_wrapper);

if (result != nullptr) {
this->SetReturnValue(result);
}

if (!env->last_exception.IsEmpty()) {
isolate->ThrowException(
@@ -608,75 +694,12 @@ void napi_module_register(napi_module* mod) {
node::node_module_register(nm);
}

#define RETURN_STATUS_IF_FALSE(env, condition, status) \
do { \
if (!(condition)) { \
return napi_set_last_error((env), (status)); \
} \
} while (0)

#define CHECK_ENV(env) \
if ((env) == nullptr) { \
node::FatalError(__func__, "environment(env) must not be null"); \
}

#define CHECK_ARG(env, arg) \
RETURN_STATUS_IF_FALSE((env), ((arg) != nullptr), napi_invalid_arg)

#define CHECK_MAYBE_EMPTY(env, maybe, status) \
RETURN_STATUS_IF_FALSE((env), !((maybe).IsEmpty()), (status))

#define CHECK_MAYBE_NOTHING(env, maybe, status) \
RETURN_STATUS_IF_FALSE((env), !((maybe).IsNothing()), (status))

// NAPI_PREAMBLE is not wrapped in do..while: try_catch must have function scope
#define NAPI_PREAMBLE(env) \
CHECK_ENV((env)); \
RETURN_STATUS_IF_FALSE((env), (env)->last_exception.IsEmpty(), \
napi_pending_exception); \
napi_clear_last_error((env)); \
v8impl::TryCatch try_catch((env))

#define CHECK_TO_TYPE(env, type, context, result, src, status) \
do { \
auto maybe = v8impl::V8LocalValueFromJsValue((src))->To##type((context)); \
CHECK_MAYBE_EMPTY((env), maybe, (status)); \
(result) = maybe.ToLocalChecked(); \
} while (0)

#define CHECK_TO_OBJECT(env, context, result, src) \
CHECK_TO_TYPE((env), Object, (context), (result), (src), napi_object_expected)

#define CHECK_TO_STRING(env, context, result, src) \
CHECK_TO_TYPE((env), String, (context), (result), (src), napi_string_expected)

#define CHECK_TO_NUMBER(env, context, result, src) \
CHECK_TO_TYPE((env), Number, (context), (result), (src), napi_number_expected)

#define CHECK_TO_BOOL(env, context, result, src) \
CHECK_TO_TYPE((env), Boolean, (context), (result), (src), \
napi_boolean_expected)

#define CHECK_NEW_FROM_UTF8_LEN(env, result, str, len) \
do { \
auto str_maybe = v8::String::NewFromUtf8( \
(env)->isolate, (str), v8::NewStringType::kInternalized, (len)); \
CHECK_MAYBE_EMPTY((env), str_maybe, napi_generic_failure); \
result = str_maybe.ToLocalChecked(); \
} while (0)

#define CHECK_NEW_FROM_UTF8(env, result, str) \
CHECK_NEW_FROM_UTF8_LEN((env), (result), (str), -1)

#define GET_RETURN_STATUS(env) \
(!try_catch.HasCaught() ? napi_ok \
: napi_set_last_error((env), napi_pending_exception))

// Warning: Keep in-sync with napi_status enum
const char* error_messages[] = {nullptr,
"Invalid pointer passed as argument",
"An object was expected",
"A string was expected",
"A string or symbol was expected",
"A function was expected",
"A number was expected",
"A boolean was expected",
@@ -793,8 +816,14 @@ napi_status napi_define_class(napi_env env,
continue;
}

v8::Local<v8::String> property_name;
CHECK_NEW_FROM_UTF8(env, property_name, p->utf8name);
v8::Local<v8::Name> property_name;
napi_status status =
v8impl::V8NameFromPropertyDescriptor(env, p, &property_name);

if (status != napi_ok) {
return napi_set_last_error(env, status);
}

v8::PropertyAttribute attributes =
v8impl::V8PropertyAttributesFromDescriptor(p);

@@ -822,7 +851,6 @@ napi_status napi_define_class(napi_env env,
v8impl::FunctionCallbackWrapper::Invoke,
cbdata,
v8::Signature::New(isolate, tpl));
t->SetClassName(property_name);

tpl->PrototypeTemplate()->Set(property_name, t, attributes);
} else {
@@ -855,18 +883,6 @@ napi_status napi_define_class(napi_env env,
return GET_RETURN_STATUS(env);
}

napi_status napi_set_return_value(napi_env env,
napi_callback_info cbinfo,
napi_value value) {
NAPI_PREAMBLE(env);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);

info->SetReturnValue(value);
return GET_RETURN_STATUS(env);
}

napi_status napi_get_property_names(napi_env env,
napi_value object,
napi_value* result) {
@@ -1102,11 +1118,16 @@ napi_status napi_define_properties(napi_env env,
for (size_t i = 0; i < property_count; i++) {
const napi_property_descriptor* p = &properties[i];

v8::Local<v8::Name> name;
CHECK_NEW_FROM_UTF8(env, name, p->utf8name);
v8::Local<v8::Name> property_name;
napi_status status =
v8impl::V8NameFromPropertyDescriptor(env, p, &property_name);

if (status != napi_ok) {
return napi_set_last_error(env, status);
}

v8::PropertyAttribute attributes =
v8impl::V8PropertyAttributesFromDescriptor(p);
v8impl::V8PropertyAttributesFromDescriptor(p);

if (p->getter != nullptr || p->setter != nullptr) {
v8::Local<v8::Object> cbdata = v8impl::CreateAccessorCallbackData(
@@ -1117,7 +1138,7 @@ napi_status napi_define_properties(napi_env env,

auto set_maybe = obj->SetAccessor(
context,
name,
property_name,
p->getter ? v8impl::GetterCallbackWrapper::Invoke : nullptr,
p->setter ? v8impl::SetterCallbackWrapper::Invoke : nullptr,
cbdata,
@@ -1136,8 +1157,8 @@ napi_status napi_define_properties(napi_env env,
v8::Local<v8::FunctionTemplate> t = v8::FunctionTemplate::New(
isolate, v8impl::FunctionCallbackWrapper::Invoke, cbdata);

auto define_maybe =
obj->DefineOwnProperty(context, name, t->GetFunction(), attributes);
auto define_maybe = obj->DefineOwnProperty(
context, property_name, t->GetFunction(), attributes);

if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_generic_failure);
@@ -1146,7 +1167,7 @@ napi_status napi_define_properties(napi_env env,
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(p->value);

auto define_maybe =
obj->DefineOwnProperty(context, name, value, attributes);
obj->DefineOwnProperty(context, property_name, value, attributes);

if (!define_maybe.FromMaybe(false)) {
return napi_set_last_error(env, napi_invalid_arg);
@@ -1439,33 +1460,24 @@ napi_status napi_get_cb_info(
napi_value* this_arg, // [out] Receives the JS 'this' arg for the call
void** data) { // [out] Receives the data pointer for the callback.
CHECK_ENV(env);
CHECK_ARG(env, argc);
CHECK_ARG(env, argv);
CHECK_ARG(env, this_arg);
CHECK_ARG(env, data);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);

info->Args(argv, std::min(*argc, info->ArgsLength()));
*argc = info->ArgsLength();
*this_arg = info->This();
*data = info->Data();

return napi_ok;
}

napi_status napi_get_cb_args_length(napi_env env,
napi_callback_info cbinfo,
size_t* result) {
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because no V8 APIs are called.
CHECK_ENV(env);
CHECK_ARG(env, result);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);
if (argv != nullptr) {
CHECK_ARG(env, argc);
info->Args(argv, std::min(*argc, info->ArgsLength()));
}
if (argc != nullptr) {
*argc = info->ArgsLength();
}
if (this_arg != nullptr) {
*this_arg = info->This();
}
if (data != nullptr) {
*data = info->Data();
}

*result = info->ArgsLength();
return napi_ok;
}

@@ -1483,51 +1495,6 @@ napi_status napi_is_construct_call(napi_env env,
return napi_ok;
}

// copy encoded arguments into provided buffer or return direct pointer to
// encoded arguments array?
napi_status napi_get_cb_args(napi_env env,
napi_callback_info cbinfo,
napi_value* buf,
size_t bufsize) {
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because no V8 APIs are called.
CHECK_ENV(env);
CHECK_ARG(env, buf);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);

info->Args(buf, bufsize);
return napi_ok;
}

napi_status napi_get_cb_this(napi_env env,
napi_callback_info cbinfo,
napi_value* result) {
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because no V8 APIs are called.
CHECK_ENV(env);
CHECK_ARG(env, result);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);

*result = info->This();
return napi_ok;
}

napi_status napi_get_cb_data(napi_env env,
napi_callback_info cbinfo,
void** result) {
// Omit NAPI_PREAMBLE and GET_RETURN_STATUS because no V8 APIs are called.
CHECK_ENV(env);
CHECK_ARG(env, result);

v8impl::CallbackWrapper* info =
reinterpret_cast<v8impl::CallbackWrapper*>(cbinfo);

*result = info->Data();
return napi_ok;
}

napi_status napi_call_function(napi_env env,
napi_value recv,
napi_value func,

0 comments on commit ca786c3

Please sign in to comment.
You can’t perform that action at this time.