Don't use soon-to-be-deprecated V8 Api (nodejs#179)

V8 announced deprecation of the following methods: - v8::Objecit::SetAccessor(...) in favor of v8::Object::SetNativeDataProperty(...), - v8::ObjectTemplate::SetNativeDataProperty(...) with AccessControl parameter in favor of v8::ObjectTemplate::SetNativeDataProperty(...) without AccessControl parameter. See https://crrev.com/c/5006387. This CL slightly changes behavior of the following properties: - process.debugPort (for worker processes), - process.title (for worker processes), - process.ppid. The difference is that they will now behave like a regular writable JavaScript data properties - in case setter callback is not provided they will be be reconfigured from a native data property (the one that calls C++ callbacks upon get/set operations) to a real data property (so subsequent reads will no longer trigger C++ getter callbacks).
eti-p-doray · Apr 23, 2024 · 6b056bf · 6b056bf
1 parent 86ea71c
commit 6b056bf
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 20 deletions.
diff --git a/src/node_builtins.cc b/src/node_builtins.cc
@@ -11,7 +11,6 @@ namespace node {
 namespace builtins {
 
 using v8::Context;
-using v8::DEFAULT;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -711,15 +710,13 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
                                 nullptr,
                                 Local<Value>(),
                                 None,
-                                DEFAULT,
                                 SideEffectType::kHasNoSideEffect);
 
   target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "builtinIds"),
                                 BuiltinIdsGetter,
                                 nullptr,
                                 Local<Value>(),
                                 None,
-                                DEFAULT,
                                 SideEffectType::kHasNoSideEffect);
 
   target->SetNativeDataProperty(
@@ -728,15 +725,13 @@ void BuiltinLoader::CreatePerIsolateProperties(IsolateData* isolate_data,
       nullptr,
       Local<Value>(),
       None,
-      DEFAULT,
       SideEffectType::kHasNoSideEffect);
 
   target->SetNativeDataProperty(FIXED_ONE_BYTE_STRING(isolate, "natives"),
                                 GetNatives,
                                 nullptr,
                                 Local<Value>(),
                                 None,
-                                DEFAULT,
                                 SideEffectType::kHasNoSideEffect);
 
   SetMethod(isolate, target, "getCacheUsage", BuiltinLoader::GetCacheUsage);

diff --git a/src/node_process_object.cc b/src/node_process_object.cc
@@ -13,7 +13,6 @@
 
 namespace node {
 using v8::Context;
-using v8::DEFAULT;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -183,13 +182,12 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
 
   // process.title
   CHECK(process
-            ->SetAccessor(
+            ->SetNativeDataProperty(
                 context,
                 FIXED_ONE_BYTE_STRING(isolate, "title"),
                 ProcessTitleGetter,
                 env->owns_process_state() ? ProcessTitleSetter : nullptr,
                 Local<Value>(),
-                DEFAULT,
                 None,
                 SideEffectType::kHasNoSideEffect)
             .FromJust());
@@ -208,9 +206,15 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
   READONLY_PROPERTY(process, "pid",
                     Integer::New(isolate, uv_os_getpid()));
 
-  CHECK(process->SetAccessor(context,
-                             FIXED_ONE_BYTE_STRING(isolate, "ppid"),
-                             GetParentProcessId).FromJust());
+  CHECK(process
+            ->SetNativeDataProperty(context,
+                                    FIXED_ONE_BYTE_STRING(isolate, "ppid"),
+                                    GetParentProcessId,
+                                    nullptr,
+                                    Local<Value>(),
+                                    None,
+                                    SideEffectType::kHasNoSideEffect)
+            .FromJust());
 
   // --security-revert flags
 #define V(code, _, __)                                                        \
@@ -235,11 +239,14 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
 
   // process.debugPort
   CHECK(process
-            ->SetAccessor(context,
-                          FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
-                          DebugPortGetter,
-                          env->owns_process_state() ? DebugPortSetter : nullptr,
-                          Local<Value>())
+            ->SetNativeDataProperty(
+                context,
+                FIXED_ONE_BYTE_STRING(isolate, "debugPort"),
+                DebugPortGetter,
+                env->owns_process_state() ? DebugPortSetter : nullptr,
+                Local<Value>(),
+                None,
+                SideEffectType::kHasNoSideEffect)
             .FromJust());
 }
 

diff --git a/test/parallel/test-worker-unsupported-things.js b/test/parallel/test-worker-unsupported-things.js
@@ -14,14 +14,16 @@ if (!process.env.HAS_STARTED_WORKER) {
 } else {
   {
     const before = process.title;
-    process.title += ' in worker';
-    assert.strictEqual(process.title, before);
+    const after = before + ' in worker';
+    process.title = after;
+    assert.strictEqual(process.title, after);
   }
 
   {
     const before = process.debugPort;
-    process.debugPort++;
-    assert.strictEqual(process.debugPort, before);
+    const after = before + 1;
+    process.debugPort = after;
+    assert.strictEqual(process.debugPort, after);
   }
 
   {