From 759800aab6e2cbc95a568e00337bf3e572b5b136 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Thu, 2 Jan 2020 12:40:59 -0800 Subject: [PATCH] perform some renames to better reflect semantics --- napi-inl.h | 32 ++++++++++++------------ napi.h | 6 ++--- test/objectwrap_constructor_exception.js | 9 +------ 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index f51c4a790..5367a0bec 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2702,35 +2702,35 @@ inline Object FunctionReference::New(const std::vector& args) const // CallbackInfo class //////////////////////////////////////////////////////////////////////////////// -class ObjectWrapCleanup { +class ObjectWrapConstructionContext { public: - ObjectWrapCleanup(CallbackInfo* info) { - info->_object_wrap_cleanup = this; + ObjectWrapConstructionContext(CallbackInfo* info) { + info->_objectWrapConstructionContext = this; } - static inline void MarkWrapOK(const CallbackInfo& info) { - if (info._object_wrap_cleanup == nullptr) { - Napi::Error::Fatal("ObjectWrapCleanup::MarkWrapOK", - "_object_wrap_cleanup is NULL"); + static inline void SetObjectWrapped(const CallbackInfo& info) { + if (info._objectWrapConstructionContext == nullptr) { + Napi::Error::Fatal("ObjectWrapConstructionContext::SetObjectWrapped", + "_objectWrapConstructionContext is NULL"); } - info._object_wrap_cleanup->_wrap_worked = true; + info._objectWrapConstructionContext->_objectWrapped = true; } - inline void RemoveWrap(const CallbackInfo& info) { - if (_wrap_worked) { + inline void Cleanup(const CallbackInfo& info) { + if (_objectWrapped) { napi_status status = napi_remove_wrap(info.Env(), info.This(), nullptr); // There's already a pending exception if we are at this point, so we have // no choice but to fatally fail here. NAPI_FATAL_IF_FAILED(status, - "ObjectWrapCleanup::RemoveWrap", + "ObjectWrapConstructionContext::Cleanup", "Failed to remove wrap from unsuccessfully " "constructed ObjectWrap instance"); } } private: - bool _wrap_worked = false; + bool _objectWrapped = false; }; inline CallbackInfo::CallbackInfo(napi_env env, napi_callback_info info) @@ -3140,7 +3140,7 @@ inline ObjectWrap::ObjectWrap(const Napi::CallbackInfo& callbackInfo) { status = napi_wrap(env, wrapper, this, FinalizeCallback, nullptr, &ref); NAPI_THROW_IF_FAILED_VOID(env, status); - ObjectWrapCleanup::MarkWrapOK(callbackInfo); + ObjectWrapConstructionContext::SetObjectWrapped(callbackInfo); Reference* instanceRef = this; *instanceRef = Reference(env, ref); } @@ -3716,13 +3716,13 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( napi_value wrapper = details::WrapCallback([&] { CallbackInfo callbackInfo(env, info); - ObjectWrapCleanup cleanup(&callbackInfo); + ObjectWrapConstructionContext constructionContext(&callbackInfo); #ifdef NAPI_CPP_EXCEPTIONS try { new T(callbackInfo); } catch (const Error& e) { // Re-throw the error after removing the failed wrap. - cleanup.RemoveWrap(callbackInfo); + constructionContext.Cleanup(callbackInfo); throw e; } #else @@ -3730,7 +3730,7 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( if (callbackInfo.Env().IsExceptionPending()) { // We need to clear the exception so that removing the wrap might work. Error e = callbackInfo.Env().GetAndClearPendingException(); - cleanup.RemoveWrap(callbackInfo); + constructionContext.Cleanup(callbackInfo); e.ThrowAsJavaScriptException(); delete instance; } diff --git a/napi.h b/napi.h index 876b14b64..61375b7c6 100644 --- a/napi.h +++ b/napi.h @@ -138,7 +138,7 @@ namespace Napi { class CallbackInfo; class TypedArray; template class TypedArrayOf; - class ObjectWrapCleanup; + class ObjectWrapConstructionContext; typedef TypedArrayOf Int8Array; ///< Typed-array of signed 8-bit integers typedef TypedArrayOf Uint8Array; ///< Typed-array of unsigned 8-bit integers @@ -1403,7 +1403,7 @@ namespace Napi { class CallbackInfo { public: - friend class ObjectWrapCleanup; + friend class ObjectWrapConstructionContext; CallbackInfo(napi_env env, napi_callback_info info); ~CallbackInfo(); @@ -1429,7 +1429,7 @@ namespace Napi { napi_value _staticArgs[6]; napi_value* _dynamicArgs; void* _data; - ObjectWrapCleanup* _object_wrap_cleanup; + ObjectWrapConstructionContext* _objectWrapConstructionContext; }; class PropertyDescriptor { diff --git a/test/objectwrap_constructor_exception.js b/test/objectwrap_constructor_exception.js index db64e2e05..1615ed363 100644 --- a/test/objectwrap_constructor_exception.js +++ b/test/objectwrap_constructor_exception.js @@ -4,15 +4,8 @@ const assert = require('assert'); const test = (binding) => { const { ConstructorExceptionTest } = binding.objectwrapConstructorException; - let gotException = false; - try { - new ConstructorExceptionTest(); - } catch (anException) { - gotException = true; - } + assert.throws(() => (new ConstructorExceptionTest())); global.gc(); - - assert.strictEqual(gotException, true); } test(require(`./build/${buildType}/binding.node`));