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.

Author: legendecas

doc/api/n-api.md

@@ -5285,7 +5285,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);
@@ -5294,10 +5294,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 is been garbage-collected.
   [`napi_finalize`][] provides more details.
 * `[in] finalize_hint`: Optional contextual hint that is passed to the
   finalize callback.
@@ -5306,14 +5305,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:
+in `js_object` is been garbage-collected.
 
-* 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()`.
+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

@@ -392,9 +392,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,
@@ -410,47 +407,36 @@ 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;
+  // Create a self-deleting reference if the optional out-param result is not
+  // set.
+  bool delete_self = true;
   if (result != nullptr) {
     // The returned reference should be deleted via napi_delete_reference()
     // ONLY in response to the finalize callback invocation. (If it is deleted
     // before then, then the finalize callback will never be invoked.)
     // Therefore a finalize callback is required when returning a reference.
     CHECK_ARG(env, finalize_cb);
-    reference = v8impl::Reference::New(
-        env, obj, 0, false, finalize_cb, native_object, finalize_hint);
+    delete_self = false;
+  }
+
+  v8impl::Reference* reference = v8impl::Reference::New(
+      env, obj, 0, delete_self, finalize_cb, native_object, finalize_hint);
+
+  if (result != nullptr) {
     *result = reinterpret_cast<napi_ref>(reference);
-  } else {
-    // Create a self-deleting reference.
-    reference = v8impl::Reference::New(
-        env,
-        obj,
-        0,
-        true,
-        finalize_cb,
-        native_object,
-        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);
 }
 
@@ -2360,7 +2346,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);
 }
 
@@ -3176,12 +3162,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);
+  if (!(v8_value->IsObject() || v8_value->IsFunction())) {
+    return napi_set_last_error(env, napi_invalid_arg);
+  }
+
+  // Create a self-deleting reference if the optional out-param result is not
+  // set.
+  bool delete_self = result == nullptr;
+  v8impl::Reference* reference = v8impl::Reference::New(
+      env, v8_value, 0, delete_self, 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,