[v19.x backport] src,lib: group properties used as constants from util binding

lib/buffer.js

@@ -67,11 +67,11 @@ const {
   kStringMaxLength
 } = internalBinding('buffer');
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ALL_PROPERTIES,
-    ONLY_ENUMERABLE
+    ONLY_ENUMERABLE,
   },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 const {
   customInspectSymbol,

lib/internal/bootstrap/node.js

@@ -75,8 +75,9 @@ const {
   exposeInterface,
 } = require('internal/util');
 const {
-  exiting_aliased_Uint32Array,
-  getHiddenValue,
+  privateSymbols: {
+    exiting_aliased_Uint32Array,
+  },
 } = internalBinding('util');
 
 setupProcessObject();
@@ -86,8 +87,7 @@ setupBuffer();
 
 process.domain = null;
 {
-  const exitingAliasedUint32Array =
-    getHiddenValue(process, exiting_aliased_Uint32Array);
+  const exitingAliasedUint32Array = process[exiting_aliased_Uint32Array];
   ObjectDefineProperty(process, '_exiting', {
     __proto__: null,
     get() {

lib/internal/buffer.js

@@ -32,9 +32,11 @@ const {
   utf8Write,
   getZeroFillToggle
 } = internalBinding('buffer');
+
 const {
-  untransferable_object_private_symbol,
-  setHiddenValue,
+  privateSymbols: {
+    untransferable_object_private_symbol,
+  },
 } = internalBinding('util');
 
 // Temporary buffers to convert numbers.
@@ -1048,7 +1050,7 @@ function addBufferPrototypeMethods(proto) {
 function markAsUntransferable(obj) {
   if ((typeof obj !== 'object' && typeof obj !== 'function') || obj === null)
     return;  // This object is a primitive and therefore already untransferable.
-  setHiddenValue(obj, untransferable_object_private_symbol, true);
+  obj[untransferable_object_private_symbol] = true;
 }
 
 // A toggle used to access the zero fill setting of the array buffer allocator

lib/internal/errors.js

@@ -809,16 +809,14 @@ const fatalExceptionStackEnhancers = {
   }
 };
 
+const {
+  privateSymbols: {
+    arrow_message_private_symbol,
+  }
+} = internalBinding('util');
 // Ensures the printed error line is from user code.
-let _kArrowMessagePrivateSymbol, _setHiddenValue;
 function setArrowMessage(err, arrowMessage) {
-  if (!_kArrowMessagePrivateSymbol) {
-    ({
-      arrow_message_private_symbol: _kArrowMessagePrivateSymbol,
-      setHiddenValue: _setHiddenValue,
-    } = internalBinding('util'));
-  }
-  _setHiddenValue(err, _kArrowMessagePrivateSymbol, arrowMessage);
+  err[arrow_message_private_symbol] = arrowMessage;
 }
 
 // Hide stack lines before the first user code line.

lib/internal/util.js

@@ -42,10 +42,10 @@ const {
 } = require('internal/errors');
 const { signals } = internalBinding('constants').os;
 const {
-  getHiddenValue,
-  setHiddenValue,
-  arrow_message_private_symbol: kArrowMessagePrivateSymbolIndex,
-  decorated_private_symbol: kDecoratedPrivateSymbolIndex,
+  privateSymbols: {
+    arrow_message_private_symbol,
+    decorated_private_symbol,
+  },
   sleep: _sleep,
   toUSVString: _toUSVString,
 } = internalBinding('util');
@@ -143,15 +143,14 @@ function deprecate(fn, msg, code, useEmitSync) {
 }
 
 function decorateErrorStack(err) {
-  if (!(isError(err) && err.stack) ||
-      getHiddenValue(err, kDecoratedPrivateSymbolIndex) === true)
+  if (!(isError(err) && err.stack) || err[decorated_private_symbol])
     return;
 
-  const arrow = getHiddenValue(err, kArrowMessagePrivateSymbolIndex);
+  const arrow = err[arrow_message_private_symbol];
 
   if (arrow) {
     err.stack = arrow + err.stack;
-    setHiddenValue(err, kDecoratedPrivateSymbolIndex, true);
+    err[decorated_private_symbol] = true;
   }
 }
 

lib/internal/util/comparisons.js

@@ -46,11 +46,11 @@ const {
   isFloat64Array,
 } = types;
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ONLY_ENUMERABLE,
-    SKIP_SYMBOLS
-  }
+    SKIP_SYMBOLS,
+  },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 
 const kStrict = true;

lib/internal/util/inspect.js

@@ -97,18 +97,18 @@ const {
 } = primordials;
 
 const {
+  constants: {
+    ALL_PROPERTIES,
+    ONLY_ENUMERABLE,
+    kPending,
+    kRejected,
+  },
   getOwnNonIndexProperties,
   getPromiseDetails,
   getProxyDetails,
-  kPending,
-  kRejected,
   previewEntries,
   getConstructorName: internalGetConstructorName,
   getExternalValue,
-  propertyFilter: {
-    ALL_PROPERTIES,
-    ONLY_ENUMERABLE
-  }
 } = internalBinding('util');
 
 const {

lib/internal/webstreams/util.js

@@ -40,8 +40,10 @@ const {
 } = require('util');
 
 const {
+  constants: {
+    kPending,
+  },
   getPromiseDetails,
-  kPending,
 } = internalBinding('util');
 
 const assert = require('internal/assert');

lib/repl.js

@@ -166,11 +166,11 @@ const {
   setupReverseSearch,
 } = require('internal/repl/utils');
 const {
-  getOwnNonIndexProperties,
-  propertyFilter: {
+  constants: {
     ALL_PROPERTIES,
-    SKIP_SYMBOLS
-  }
+    SKIP_SYMBOLS,
+  },
+  getOwnNonIndexProperties,
 } = internalBinding('util');
 const {
   startSigintWatchdog,

src/node_util.cc

@@ -26,7 +26,6 @@ using v8::Object;
 using v8::ONLY_CONFIGURABLE;
 using v8::ONLY_ENUMERABLE;
 using v8::ONLY_WRITABLE;
-using v8::Private;
 using v8::Promise;
 using v8::PropertyFilter;
 using v8::Proxy;
@@ -159,44 +158,6 @@ static void PreviewEntries(const FunctionCallbackInfo<Value>& args) {
       Array::New(env->isolate(), ret, arraysize(ret)));
 }
 
-inline Local<Private> IndexToPrivateSymbol(Environment* env, uint32_t index) {
-#define V(name, _) &Environment::name,
-  static Local<Private> (Environment::*const methods[])() const = {
-    PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
-  };
-#undef V
-  CHECK_LT(index, arraysize(methods));
-  return (env->*methods[index])();
-}
-
-static void GetHiddenValue(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  CHECK(args[0]->IsObject());
-  CHECK(args[1]->IsUint32());
-
-  Local<Object> obj = args[0].As<Object>();
-  uint32_t index = args[1].As<Uint32>()->Value();
-  Local<Private> private_symbol = IndexToPrivateSymbol(env, index);
-  Local<Value> ret;
-  if (obj->GetPrivate(env->context(), private_symbol).ToLocal(&ret))
-    args.GetReturnValue().Set(ret);
-}
-
-static void SetHiddenValue(const FunctionCallbackInfo<Value>& args) {
-  Environment* env = Environment::GetCurrent(args);
-
-  CHECK(args[0]->IsObject());
-  CHECK(args[1]->IsUint32());
-
-  Local<Object> obj = args[0].As<Object>();
-  uint32_t index = args[1].As<Uint32>()->Value();
-  Local<Private> private_symbol = IndexToPrivateSymbol(env, index);
-  bool ret;
-  if (obj->SetPrivate(env->context(), private_symbol, args[2]).To(&ret))
-    args.GetReturnValue().Set(ret);
-}
-
 static void Sleep(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[0]->IsUint32());
   uint32_t msec = args[0].As<Uint32>()->Value();
@@ -379,8 +340,6 @@ static void ToUSVString(const FunctionCallbackInfo<Value>& args) {
 }
 
 void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
-  registry->Register(GetHiddenValue);
-  registry->Register(SetHiddenValue);
   registry->Register(GetPromiseDetails);
   registry->Register(GetProxyDetails);
   registry->Register(PreviewEntries);
@@ -404,28 +363,54 @@ void Initialize(Local<Object> target,
   Environment* env = Environment::GetCurrent(context);
   Isolate* isolate = env->isolate();
 
-#define V(name, _)                                                            \
-  target->Set(context,                                                        \
-              FIXED_ONE_BYTE_STRING(env->isolate(), #name),                   \
-              Integer::NewFromUnsigned(env->isolate(), index++)).Check();
   {
-    uint32_t index = 0;
+    Local<v8::ObjectTemplate> tmpl = v8::ObjectTemplate::New(isolate);
+#define V(PropertyName, _)                                                     \
+  tmpl->Set(FIXED_ONE_BYTE_STRING(env->isolate(), #PropertyName),              \
+            env->PropertyName());
+
     PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V)
+#undef V
+
+    target
+        ->Set(context,
+              FIXED_ONE_BYTE_STRING(isolate, "privateSymbols"),
+              tmpl->NewInstance(context).ToLocalChecked())
+        .Check();
   }
+
+  {
+    Local<Object> constants = Object::New(isolate);
+#define V(name)                                                                \
+  constants                                                                    \
+      ->Set(context,                                                           \
+            FIXED_ONE_BYTE_STRING(isolate, #name),                             \
+            Integer::New(isolate, Promise::PromiseState::name))                \
+      .Check();
+
+    V(kPending);
+    V(kFulfilled);
+    V(kRejected);
 #undef V
 
-#define V(name)                                                               \
-  target->Set(context,                                                        \
-              FIXED_ONE_BYTE_STRING(env->isolate(), #name),                   \
-              Integer::New(env->isolate(), Promise::PromiseState::name))      \
-    .FromJust()
-  V(kPending);
-  V(kFulfilled);
-  V(kRejected);
+#define V(name)                                                                \
+  constants                                                                    \
+      ->Set(context,                                                           \
+            FIXED_ONE_BYTE_STRING(isolate, #name),                             \
+            Integer::New(isolate, PropertyFilter::name))                       \
+      .Check();
+
+    V(ALL_PROPERTIES);
+    V(ONLY_WRITABLE);
+    V(ONLY_ENUMERABLE);
+    V(ONLY_CONFIGURABLE);
+    V(SKIP_STRINGS);
+    V(SKIP_SYMBOLS);
 #undef V
 
-  SetMethodNoSideEffect(context, target, "getHiddenValue", GetHiddenValue);
-  SetMethod(context, target, "setHiddenValue", SetHiddenValue);
+    target->Set(context, env->constants_string(), constants).Check();
+  }
+
   SetMethodNoSideEffect(
       context, target, "getPromiseDetails", GetPromiseDetails);
   SetMethodNoSideEffect(context, target, "getProxyDetails", GetProxyDetails);
@@ -439,16 +424,6 @@ void Initialize(Local<Object> target,
 
   SetMethod(
       context, target, "arrayBufferViewHasBuffer", ArrayBufferViewHasBuffer);
-  Local<Object> constants = Object::New(env->isolate());
-  NODE_DEFINE_CONSTANT(constants, ALL_PROPERTIES);
-  NODE_DEFINE_CONSTANT(constants, ONLY_WRITABLE);
-  NODE_DEFINE_CONSTANT(constants, ONLY_ENUMERABLE);
-  NODE_DEFINE_CONSTANT(constants, ONLY_CONFIGURABLE);
-  NODE_DEFINE_CONSTANT(constants, SKIP_STRINGS);
-  NODE_DEFINE_CONSTANT(constants, SKIP_SYMBOLS);
-  target->Set(context,
-              FIXED_ONE_BYTE_STRING(env->isolate(), "propertyFilter"),
-              constants).Check();
 
   Local<String> should_abort_on_uncaught_toggle =
       FIXED_ONE_BYTE_STRING(env->isolate(), "shouldAbortOnUncaughtToggle");

test/parallel/test-internal-util-decorate-error-stack.js

@@ -5,12 +5,14 @@ const fixtures = require('../common/fixtures');
 const assert = require('assert');
 const internalUtil = require('internal/util');
 const { internalBinding } = require('internal/test/binding');
-const binding = internalBinding('util');
+const {
+  privateSymbols: {
+    arrow_message_private_symbol,
+    decorated_private_symbol,
+  }
+} = internalBinding('util');
 const spawnSync = require('child_process').spawnSync;
 
-const kArrowMessagePrivateSymbolIndex = binding.arrow_message_private_symbol;
-const kDecoratedPrivateSymbolIndex = binding.decorated_private_symbol;
-
 const decorateErrorStack = internalUtil.decorateErrorStack;
 
 // Verify that decorateErrorStack does not throw with non-objects.
@@ -73,9 +75,8 @@ const arrowMessage = 'arrow_message';
 err = new Error('foo');
 originalStack = err.stack;
 
-binding.setHiddenValue(err, kArrowMessagePrivateSymbolIndex, arrowMessage);
+err[arrow_message_private_symbol] = arrowMessage;
 decorateErrorStack(err);
 
 assert.strictEqual(err.stack, `${arrowMessage}${originalStack}`);
-assert.strictEqual(
-  binding.getHiddenValue(err, kDecoratedPrivateSymbolIndex), true);
+assert.strictEqual(err[decorated_private_symbol], true);