Skip to content

Commit 5519ac3

Browse files
author
ci_lynx
committed
[Feature] Add spec-compliant API: napi_set_instance_data_spec_compliant
Since the current behavior of PrimJS's `napi_set_instance_data` API is inconsistent with the standard, the NAPI Adapter requires a new API to achieve alignment with the standard. To avoid breaking changes for existing users, the new `napi_set_instance_data_spec_compliant` API is added. issue: m-6565624125
1 parent 9be5128 commit 5519ac3

File tree

7 files changed

+91
-13
lines changed

7 files changed

+91
-13
lines changed

‎src/napi/adapter/js_native_api_adapter.cc‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,28 @@
2020

2121
#include "js_native_api_types.h"
2222
#include "napi_runtime.h"
23+
#include "napi_state.h"
2324

2425
#ifdef USE_PRIMJS_NAPI
2526
#include "primjs_napi_defines.h"
2627
#endif
2728

29+
#define CHECK_ENV(env) \
30+
do { \
31+
if ((env) == nullptr) { \
32+
return napi_set_last_error(nullptr, napi_invalid_arg); \
33+
} \
34+
} while (0)
35+
36+
#define RETURN_STATUS_IF_FALSE(env, condition, status) \
37+
do { \
38+
if (!(condition)) { \
39+
return napi_set_last_error((env), (status)); \
40+
} \
41+
} while (0)
42+
43+
#define CHECK_ARG(env, arg) \
44+
RETURN_STATUS_IF_FALSE((env), ((arg) != nullptr), napi_invalid_arg)
2845
EXTERN_C_START
2946

3047
napi_status napi_get_version_primjs(napi_env env, uint32_t* result) {
@@ -549,16 +566,20 @@ napi_status napi_add_finalizer_primjs(napi_env env, napi_value js_object,
549566
finalize_hint, result);
550567
}
551568

552-
napi_status napi_set_instance_data_primjs(napi_env env, uint64_t key,
553-
void* data, napi_finalize finalize_cb,
569+
static const uint64_t kNapiAdapterInstanceDataKey =
570+
reinterpret_cast<uint64_t>(&kNapiAdapterInstanceDataKey);
571+
napi_status napi_set_instance_data_primjs(napi_env env, void* data,
572+
napi_finalize finalize_cb,
554573
void* finalize_hint) {
555-
return env->napi_set_instance_data(env, key, data, finalize_cb,
556-
finalize_hint);
574+
CHECK_ENV(env);
575+
return env->napi_set_instance_data_spec_compliant(
576+
env, kNapiAdapterInstanceDataKey, data, finalize_cb, finalize_hint);
557577
}
558578

559-
napi_status napi_get_instance_data_primjs(napi_env env, uint64_t key,
560-
void** data) {
561-
return env->napi_get_instance_data(env, key, data);
579+
napi_status napi_get_instance_data_primjs(napi_env env, void** data) {
580+
CHECK_ENV(env);
581+
CHECK_ARG(env, data);
582+
return env->napi_get_instance_data(env, kNapiAdapterInstanceDataKey, data);
562583
}
563584

564585
napi_status napi_get_last_error_info_primjs(

‎src/napi/adapter/js_native_api_adapter.h‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -356,12 +356,10 @@ NAPI_EXTERN napi_status napi_add_finalizer_primjs(
356356
napi_finalize finalize_cb, void* finalize_hint, napi_ref* result);
357357

358358
// Instance data
359-
NAPI_EXTERN napi_status napi_set_instance_data_primjs(napi_env env,
360-
uint64_t key, void* data,
359+
NAPI_EXTERN napi_status napi_set_instance_data_primjs(napi_env env, void* data,
361360
napi_finalize finalize_cb,
362361
void* finalize_hint);
363362
NAPI_EXTERN napi_status napi_get_instance_data_primjs(napi_env env,
364-
uint64_t key,
365363
void** data);
366364

367365
// Error information

‎src/napi/harmony/js_native_api_harmony.cc‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,12 @@ static napi_status napi_set_instance_data(napi_env env, uint64_t key,
12191219
return napi_clear_last_error(env);
12201220
}
12211221

1222+
static napi_status napi_set_instance_data_spec_compliant(
1223+
napi_env env, uint64_t key, void *data, napi_finalize finalize_cb,
1224+
void *finalize_hint) {
1225+
return napi_set_instance_data(env, key, data, finalize_cb, finalize_hint);
1226+
}
1227+
12221228
static napi_status napi_get_instance_data(napi_env env, uint64_t key,
12231229
void **data) {
12241230
auto &map = env->ctx->instance_data_;

‎src/napi/js_native_api.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,10 @@ struct napi_env__ {
501501
size_t script_len, const uint8_t** data,
502502
int* length);
503503
#endif // ENABLE_CODECACHE
504+
505+
napi_status (*napi_set_instance_data_spec_compliant)(
506+
napi_env env, uint64_t key, void* data, napi_finalize finalize_cb,
507+
void* finalize_hint);
504508
};
505509

506510
#ifdef ENABLE_CODECACHE
@@ -627,6 +631,7 @@ struct napi_env__ {
627631
V(open_context_scope) \
628632
V(close_context_scope) \
629633
V(get_own_property_descriptor) \
634+
V(set_instance_data_spec_compliant) \
630635
NAPI_ENGINE_CACHE_CALL(V)
631636

632637
// These functions share same implementations across JS engines

‎src/napi/jsc/js_native_api_JavaScriptCore.cc‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -376,7 +376,6 @@ class RefBase : protected Finalizer, RefTracker {
376376

377377
inline uint32_t RefCount() { return _refcount; }
378378

379-
protected:
380379
inline void Finalize(bool is_env_teardown = false) override {
381380
if (is_env_teardown && RefCount() > 0) _refcount = 0;
382381

@@ -2786,6 +2785,23 @@ napi_status napi_set_instance_data(napi_env env, uint64_t key, void* data,
27862785
return napi_clear_last_error(env);
27872786
}
27882787

2788+
napi_status napi_set_instance_data_spec_compliant(napi_env env, uint64_t key,
2789+
void* data,
2790+
napi_finalize finalize_cb,
2791+
void* finalize_hint) {
2792+
auto it = env->ctx->instance_data_registry.find(key);
2793+
if (it != env->ctx->instance_data_registry.end()) {
2794+
jscimpl::RefBase* idata = static_cast<jscimpl::RefBase*>(it->second);
2795+
env->ctx->instance_data_registry.erase(it);
2796+
idata->Finalize(false);
2797+
}
2798+
2799+
env->ctx->instance_data_registry[key] =
2800+
jscimpl::RefBase::New(env, 0, true, finalize_cb, data, finalize_hint);
2801+
2802+
return napi_clear_last_error(env);
2803+
}
2804+
27892805
napi_status napi_get_instance_data(napi_env env, uint64_t key, void** data) {
27902806
auto it = env->ctx->instance_data_registry.find(key);
27912807
if (it == env->ctx->instance_data_registry.end()) {

‎src/napi/quickjs/js_native_api_QuickJS.cc‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,6 @@ class RefBase : protected Finalizer, RefTracker {
281281

282282
inline uint32_t RefCount() { return _refcount; }
283283

284-
protected:
285284
inline void Finalize(bool is_env_teardown = false) override {
286285
if (is_env_teardown && RefCount() > 0) _refcount = 0;
287286

@@ -2532,6 +2531,23 @@ napi_status napi_set_instance_data(napi_env env, uint64_t key, void* data,
25322531
return napi_clear_last_error(env);
25332532
}
25342533

2534+
napi_status napi_set_instance_data_spec_compliant(napi_env env, uint64_t key,
2535+
void* data,
2536+
napi_finalize finalize_cb,
2537+
void* finalize_hint) {
2538+
auto it = env->ctx->instance_data_registry.find(key);
2539+
if (it != env->ctx->instance_data_registry.end()) {
2540+
qjsimpl::RefBase* idata = static_cast<qjsimpl::RefBase*>(it->second);
2541+
env->ctx->instance_data_registry.erase(it);
2542+
idata->Finalize(false);
2543+
}
2544+
2545+
env->ctx->instance_data_registry[key] =
2546+
qjsimpl::RefBase::New(env, 0, true, finalize_cb, data, finalize_hint);
2547+
2548+
return napi_clear_last_error(env);
2549+
}
2550+
25352551
napi_status napi_get_instance_data(napi_env env, uint64_t key, void** data) {
25362552
auto it = env->ctx->instance_data_registry.find(key);
25372553
if (it == env->ctx->instance_data_registry.end()) {

‎src/napi/v8/js_native_api_v8.cc.h‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ class RefBase : protected Finalizer, RefTracker {
284284

285285
inline uint32_t RefCount() { return _refcount; }
286286

287-
protected:
288287
inline void Finalize(bool is_env_teardown = false) override {
289288
// In addition to being called during environment teardown, this method is
290289
// also the entry point for the garbage collector. During environment
@@ -2503,6 +2502,23 @@ napi_status napi_set_instance_data(napi_env env, uint64_t key, void* data,
25032502
return napi_clear_last_error(env);
25042503
}
25052504

2505+
napi_status napi_set_instance_data_spec_compliant(napi_env env, uint64_t key,
2506+
void* data,
2507+
napi_finalize finalize_cb,
2508+
void* finalize_hint) {
2509+
auto it = env->ctx->instance_data_registry.find(key);
2510+
if (it != env->ctx->instance_data_registry.end()) {
2511+
v8impl::RefBase* idata = static_cast<v8impl::RefBase*>(it->second);
2512+
env->ctx->instance_data_registry.erase(it);
2513+
idata->Finalize(false);
2514+
}
2515+
2516+
env->ctx->instance_data_registry[key] =
2517+
v8impl::RefBase::New(env, 0, true, finalize_cb, data, finalize_hint);
2518+
2519+
return napi_clear_last_error(env);
2520+
}
2521+
25062522
napi_status napi_get_instance_data(napi_env env, uint64_t key, void** data) {
25072523
auto it = env->ctx->instance_data_registry.find(key);
25082524
if (it == env->ctx->instance_data_registry.end()) {

0 commit comments

Comments
 (0)