Don't use soon-to-be-deprecated V8 Api (#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).

Author: isheludko

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);

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());
 }
 

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);
   }
 
   {