From 129c7ae33dd34ffae5b4dd06762eed4e232e8335 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 26 May 2020 08:32:26 -0700 Subject: [PATCH 1/3] n-api: remove `napi_env::CallIntoModuleThrow` Give `napi_env::CallIntoModule` the thrower used by `CallIntoModuleThrow` as its default second argument. That way we do not need two different methods on `napi_env` for calling into the addon. --- src/js_native_api_v8.cc | 4 ++-- src/js_native_api_v8.h | 18 +++++++++--------- src/node_api.cc | 8 ++++---- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/js_native_api_v8.cc b/src/js_native_api_v8.cc index a09969b9adf28f..db53a104d6b8ef 100644 --- a/src/js_native_api_v8.cc +++ b/src/js_native_api_v8.cc @@ -267,7 +267,7 @@ class RefBase : protected Finalizer, RefTracker { protected: inline void Finalize(bool is_env_teardown = false) override { if (_finalize_callback != nullptr) { - _env->CallIntoModuleThrow([&](napi_env env) { + _env->CallIntoModule([&](napi_env env) { _finalize_callback( env, _finalize_data, @@ -475,7 +475,7 @@ class CallbackWrapperBase : public CallbackWrapper { napi_callback cb = _bundle->*FunctionField; napi_value result; - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { result = cb(env, cbinfo_wrapper); }); diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 32fc16f155a7fe..29c341c6d3f1c1 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -82,8 +82,15 @@ struct napi_env__ { return v8::Just(true); } - template - void CallIntoModule(T&& call, U&& handle_exception) { + static inline void + HandleThrow(napi_env env, v8::Local value) { + env->isolate->ThrowException(value); + } + using ThrowHandler = std::function)>; + + template + inline void CallIntoModule(T&& call, + ThrowHandler handle_exception = HandleThrow) { int open_handle_scopes_before = open_handle_scopes; int open_callback_scopes_before = open_callback_scopes; napi_clear_last_error(this); @@ -96,13 +103,6 @@ struct napi_env__ { } } - template - void CallIntoModuleThrow(T&& call) { - CallIntoModule(call, [&](napi_env env, v8::Local value) { - env->isolate->ThrowException(value); - }); - } - v8impl::Persistent last_exception; // We store references in two different lists, depending on whether they have diff --git a/src/node_api.cc b/src/node_api.cc index b87690752b3d9a..fe24eca1b8e2d8 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -57,7 +57,7 @@ class BufferFinalizer : private Finalizer { v8::HandleScope handle_scope(finalizer->_env->isolate); v8::Context::Scope context_scope(finalizer->_env->context()); - finalizer->_env->CallIntoModuleThrow([&](napi_env env) { + finalizer->_env->CallIntoModule([&](napi_env env) { finalizer->_finalize_callback( env, finalizer->_finalize_data, @@ -308,7 +308,7 @@ class ThreadSafeFunction : public node::AsyncResource { v8::Local::New(env->isolate, ref); js_callback = v8impl::JsValueFromV8LocalValue(js_cb); } - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { call_js_cb(env, js_callback, context, data); }); } @@ -318,7 +318,7 @@ class ThreadSafeFunction : public node::AsyncResource { v8::HandleScope scope(env->isolate); if (finalize_cb) { CallbackScope cb_scope(this); - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { finalize_cb(env, finalize_data, context); }); } @@ -455,7 +455,7 @@ void napi_module_register_by_symbol(v8::Local exports, napi_env env = v8impl::NewEnv(context); napi_value _exports; - env->CallIntoModuleThrow([&](napi_env env) { + env->CallIntoModule([&](napi_env env) { _exports = init(env, v8impl::JsValueFromV8LocalValue(exports)); }); From 19a518f62527cc4e7da5299207981f9546097f33 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 26 May 2020 12:00:05 -0700 Subject: [PATCH 2/3] restore template argument and remove std::function --- src/js_native_api_v8.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 29c341c6d3f1c1..830c2b6f55d554 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -86,11 +86,10 @@ struct napi_env__ { HandleThrow(napi_env env, v8::Local value) { env->isolate->ThrowException(value); } - using ThrowHandler = std::function)>; + typedef void (*ThrowHandler)(napi_env, v8::Local); - template - inline void CallIntoModule(T&& call, - ThrowHandler handle_exception = HandleThrow) { + template + inline void CallIntoModule(T&& call, U&& handle_exception = HandleThrow) { int open_handle_scopes_before = open_handle_scopes; int open_callback_scopes_before = open_callback_scopes; napi_clear_last_error(this); From 7743f469f675d06884fc9134808ca17dcd4bea7e Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Tue, 26 May 2020 12:43:56 -0700 Subject: [PATCH 3/3] derive the type of the throw handler using decltype --- src/js_native_api_v8.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/js_native_api_v8.h b/src/js_native_api_v8.h index 830c2b6f55d554..9c737f3c9cc9fc 100644 --- a/src/js_native_api_v8.h +++ b/src/js_native_api_v8.h @@ -86,9 +86,8 @@ struct napi_env__ { HandleThrow(napi_env env, v8::Local value) { env->isolate->ThrowException(value); } - typedef void (*ThrowHandler)(napi_env, v8::Local); - template + template inline void CallIntoModule(T&& call, U&& handle_exception = HandleThrow) { int open_handle_scopes_before = open_handle_scopes; int open_callback_scopes_before = open_callback_scopes;