node-api: disambiguate napi_add_finalizer

napi_add_finalizer doesn't support any operations related to
napi_wrap. Remove the ambiguous statements in the doc about
napi_wrap and avoid reusing the v8impl::Wrap call.

PR-URL: nodejs#45401
Reviewed-By: Michael Dawson <[email protected]>

Author: legendecas

doc/api/n-api.md

@@ -2382,12 +2382,7 @@ is used to pass external data through JavaScript code, so it can be retrieved
 later by native code using [`napi_get_value_external`][].
 
 The API adds a `napi_finalize` callback which will be called when the JavaScript
-object just created is ready for garbage collection. It is similar to
-`napi_wrap()` except that:
-
-* the native data cannot be retrieved later using `napi_unwrap()`,
-* nor can it be removed later using `napi_remove_wrap()`, and
-* the object created by the API can be used with `napi_wrap()`.
+object just created has been garbage collected.
 
 The created value is not an object, and therefore does not support additional
 properties. It is considered a distinct value type: calling `napi_typeof()` with
@@ -2441,12 +2436,7 @@ managed. The caller must ensure that the byte buffer remains valid until the
 finalize callback is called.
 
 The API adds a `napi_finalize` callback which will be called when the JavaScript
-object just created is ready for garbage collection. It is similar to
-`napi_wrap()` except that:
-
-* the native data cannot be retrieved later using `napi_unwrap()`,
-* nor can it be removed later using `napi_remove_wrap()`, and
-* the object created by the API can be used with `napi_wrap()`.
+object just created has been garbage collected.
 
 JavaScript `ArrayBuffer`s are described in
 [Section 24.1][] of the ECMAScript Language Specification.
@@ -2497,12 +2487,7 @@ backed by the passed in buffer. While this is still a fully-supported data
 structure, in most cases using a `TypedArray` will suffice.
 
 The API adds a `napi_finalize` callback which will be called when the JavaScript
-object just created is ready for garbage collection. It is similar to
-`napi_wrap()` except that:
-
-* the native data cannot be retrieved later using `napi_unwrap()`,
-* nor can it be removed later using `napi_remove_wrap()`, and
-* the object created by the API can be used with `napi_wrap()`.
+object just created has been garbage collected.
 
 For Node.js >=4 `Buffers` are `Uint8Array`s.
 
@@ -5141,7 +5126,7 @@ napi_status napi_wrap(napi_env env,
 * `[in] native_object`: The native instance that will be wrapped in the
   JavaScript object.
 * `[in] finalize_cb`: Optional native callback that can be used to free the
-  native instance when the JavaScript object is ready for garbage-collection.
+  native instance when the JavaScript object has been garbage-collected.
   [`napi_finalize`][] provides more details.
 * `[in] finalize_hint`: Optional contextual hint that is passed to the
   finalize callback.
@@ -5303,7 +5288,7 @@ napiVersion: 5
 ```c
 napi_status napi_add_finalizer(napi_env env,
                                napi_value js_object,
-                               void* native_object,
+                               void* finalize_data,
                                napi_finalize finalize_cb,
                                void* finalize_hint,
                                napi_ref* result);
@@ -5312,10 +5297,9 @@ napi_status napi_add_finalizer(napi_env env,
 * `[in] env`: The environment that the API is invoked under.
 * `[in] js_object`: The JavaScript object to which the native data will be
   attached.
-* `[in] native_object`: The native data that will be attached to the JavaScript
-  object.
+* `[in] finalize_data`: Optional data to be passed to `finalize_cb`.
 * `[in] finalize_cb`: Native callback that will be used to free the
-  native data when the JavaScript object is ready for garbage-collection.
+  native data when the JavaScript object has been garbage-collected.
   [`napi_finalize`][] provides more details.
 * `[in] finalize_hint`: Optional contextual hint that is passed to the
   finalize callback.
@@ -5324,14 +5308,9 @@ napi_status napi_add_finalizer(napi_env env,
 Returns `napi_ok` if the API succeeded.
 
 Adds a `napi_finalize` callback which will be called when the JavaScript object
-in `js_object` is ready for garbage collection. This API is similar to
-`napi_wrap()` except that:
-
-* the native data cannot be retrieved later using `napi_unwrap()`,
-* nor can it be removed later using `napi_remove_wrap()`, and
-* the API can be called multiple times with different data items in order to
-  attach each of them to the JavaScript object, and
-* the object manipulated by the API can be used with `napi_wrap()`.
+in `js_object` has been garbage-collected.
+
+This API can be called multiple times on a single JavaScript object.
 
 _Caution_: The optional returned reference (if obtained) should be deleted via
 [`napi_delete_reference`][] ONLY in response to the finalize callback

src/js_native_api.h

@@ -492,7 +492,7 @@ NAPI_EXTERN napi_status NAPI_CDECL napi_get_date_value(napi_env env,
 // Add finalizer for pointer
 NAPI_EXTERN napi_status NAPI_CDECL napi_add_finalizer(napi_env env,
                                                       napi_value js_object,
-                                                      void* native_object,
+                                                      void* finalize_data,
                                                       napi_finalize finalize_cb,
                                                       void* finalize_hint,
                                                       napi_ref* result);

src/js_native_api_v8.cc

@@ -401,9 +401,6 @@ class FunctionCallbackWrapper : public CallbackWrapperBase {
   }
 };
 
-enum WrapType { retrievable, anonymous };
-
-template <WrapType wrap_type>
 inline napi_status Wrap(napi_env env,
                         napi_value js_object,
                         void* native_object,
@@ -419,17 +416,11 @@ inline napi_status Wrap(napi_env env,
   RETURN_STATUS_IF_FALSE(env, value->IsObject(), napi_invalid_arg);
   v8::Local<v8::Object> obj = value.As<v8::Object>();
 
-  if (wrap_type == retrievable) {
-    // If we've already wrapped this object, we error out.
-    RETURN_STATUS_IF_FALSE(
-        env,
-        !obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper))
-             .FromJust(),
-        napi_invalid_arg);
-  } else if (wrap_type == anonymous) {
-    // If no finalize callback is provided, we error out.
-    CHECK_ARG(env, finalize_cb);
-  }
+  // If we've already wrapped this object, we error out.
+  RETURN_STATUS_IF_FALSE(
+      env,
+      !obj->HasPrivate(context, NAPI_PRIVATE_KEY(context, wrapper)).FromJust(),
+      napi_invalid_arg);
 
   v8impl::Reference* reference = nullptr;
   if (result != nullptr) {
@@ -458,12 +449,10 @@ inline napi_status Wrap(napi_env env,
         finalize_cb == nullptr ? nullptr : finalize_hint);
   }
 
-  if (wrap_type == retrievable) {
-    CHECK(obj->SetPrivate(context,
-                          NAPI_PRIVATE_KEY(context, wrapper),
-                          v8::External::New(env->isolate, reference))
-              .FromJust());
-  }
+  CHECK(obj->SetPrivate(context,
+                        NAPI_PRIVATE_KEY(context, wrapper),
+                        v8::External::New(env->isolate, reference))
+            .FromJust());
 
   return GET_RETURN_STATUS(env);
 }
@@ -2289,7 +2278,7 @@ napi_status NAPI_CDECL napi_wrap(napi_env env,
                                  napi_finalize finalize_cb,
                                  void* finalize_hint,
                                  napi_ref* result) {
-  return v8impl::Wrap<v8impl::retrievable>(
+  return v8impl::Wrap(
       env, js_object, native_object, finalize_cb, finalize_hint, result);
 }
 
@@ -3110,12 +3099,31 @@ napi_status NAPI_CDECL napi_run_script(napi_env env,
 
 napi_status NAPI_CDECL napi_add_finalizer(napi_env env,
                                           napi_value js_object,
-                                          void* native_object,
+                                          void* finalize_data,
                                           napi_finalize finalize_cb,
                                           void* finalize_hint,
                                           napi_ref* result) {
-  return v8impl::Wrap<v8impl::anonymous>(
-      env, js_object, native_object, finalize_cb, finalize_hint, result);
+  // Omit NAPI_PREAMBLE and GET_RETURN_STATUS because V8 calls here cannot throw
+  // JS exceptions.
+  CHECK_ENV(env);
+  CHECK_ARG(env, js_object);
+  CHECK_ARG(env, finalize_cb);
+
+  v8::Local<v8::Value> v8_value = v8impl::V8LocalValueFromJsValue(js_object);
+  RETURN_STATUS_IF_FALSE(env, v8_value->IsObject(), napi_invalid_arg);
+
+  // Create a self-deleting reference if the optional out-param result is not
+  // set.
+  v8impl::Ownership ownership = result == nullptr
+                                    ? v8impl::Ownership::kRuntime
+                                    : v8impl::Ownership::kUserland;
+  v8impl::Reference* reference = v8impl::Reference::New(
+      env, v8_value, 0, ownership, finalize_cb, finalize_data, finalize_hint);
+
+  if (result != nullptr) {
+    *result = reinterpret_cast<napi_ref>(reference);
+  }
+  return napi_clear_last_error(env);
 }
 
 napi_status NAPI_CDECL napi_adjust_external_memory(napi_env env,