[class] Expose private fields through GetPrivateFields

This will allow the devtools UI to display private fields on the scope
panel.

Instead of extending GetInternalProperties, we expose a separate
GetPrivateFields method on the debug interface. This allows us to do
better type checking, for example, we can directly cast to a
v8::Private as this can only contain private fields.

This also allows us to have better constraints on the input type --
v8::Object, as opposed to a v8::Value.

The KeyAccumulator is extended to collect private names for the
PRIVATE_NAMES_ONLY PropertyFilter.

Bug: v8:8773
Change-Id: Id47c551186c59dae9a06721074ef78144f25892f
Reviewed-on: https://chromium-review.googlesource.com/c/1475664
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Daniel Ehrenberg <littledan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#59920}

src/api.cc

@@ -9060,6 +9060,18 @@ MaybeLocal<Array> debug::GetInternalProperties(Isolate* v8_isolate,
   return Utils::ToLocal(result);
 }
 
+MaybeLocal<Array> debug::GetPrivateFields(Local<Context> context,
+                                          Local<Object> value) {
+  PREPARE_FOR_EXECUTION(context, debug, GetPrivateFields, Array);
+  i::Handle<i::JSReceiver> val = Utils::OpenHandle(*value);
+  i::Handle<i::JSArray> result;
+  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  has_pending_exception =
+      !(internal_isolate->debug()->GetPrivateFields(val).ToHandle(&result));
+  RETURN_ON_FAILED_EXECUTION(Array);
+  RETURN_ESCAPED(Utils::ToLocal(result));
+}
+
 void debug::ChangeBreakOnException(Isolate* isolate, ExceptionBreakState type) {
   i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
   internal_isolate->debug()->ChangeBreakOnException(

src/counters.h

@@ -727,6 +727,7 @@ class RuntimeCallTimer final {
   V(Date_New)                                              \
   V(Date_NumberValue)                                      \
   V(Debug_Call)                                            \
+  V(debug_GetPrivateFields)                                \
   V(Error_New)                                             \
   V(External_New)                                          \
   V(Float32Array_New)                                      \

src/debug/debug-interface.h

@@ -49,6 +49,13 @@ void ClearBreakOnNextFunctionCall(Isolate* isolate);
  */
 MaybeLocal<Array> GetInternalProperties(Isolate* isolate, Local<Value> value);
 
+/**
+ * Returns array of private fields specific to the value type. Result has
+ * the following format: [<name>, <value>,...,<name>, <value>]. Result array
+ * will be allocated in the current context.
+ */
+MaybeLocal<Array> GetPrivateFields(Local<Context> context, Local<Object> value);
+
 enum ExceptionBreakState {
   NoBreakOnException = 0,
   BreakOnUncaughtException = 1,

src/debug/debug.cc

@@ -1366,6 +1366,22 @@ bool Debug::GetPossibleBreakpoints(Handle<Script> script, int start_position,
   UNREACHABLE();
 }
 
+MaybeHandle<JSArray> Debug::GetPrivateFields(Handle<JSReceiver> receiver) {
+  Factory* factory = isolate_->factory();
+
+  Handle<FixedArray> internal_fields;
+  ASSIGN_RETURN_ON_EXCEPTION(isolate_, internal_fields,
+                             JSReceiver::GetPrivateEntries(isolate_, receiver),
+                             JSArray);
+
+  int nof_internal_fields = internal_fields->length();
+  if (nof_internal_fields == 0) {
+    return factory->NewJSArray(0);
+  }
+
+  return factory->NewJSArrayWithElements(internal_fields);
+}
+
 class SharedFunctionInfoFinder {
  public:
   explicit SharedFunctionInfoFinder(int target_position)

src/debug/debug.h

@@ -262,6 +262,8 @@ class Debug {
                               int end_position, bool restrict_to_function,
                               std::vector<BreakLocation>* locations);
 
+  MaybeHandle<JSArray> GetPrivateFields(Handle<JSReceiver> receiver);
+
   bool IsBlackboxed(Handle<SharedFunctionInfo> shared);
 
   bool CanBreakAtEntry(Handle<SharedFunctionInfo> shared);

src/inspector/value-mirror.cc

@@ -809,6 +809,63 @@ void getInternalPropertiesForPreview(
   }
 }
 
+void getPrivateFieldsForPreview(v8::Local<v8::Context> context,
+                                v8::Local<v8::Object> object, int* nameLimit,
+                                bool* overflow,
+                                protocol::Array<PropertyPreview>* properties) {
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::MicrotasksScope microtasksScope(isolate,
+                                      v8::MicrotasksScope::kDoNotRunMicrotasks);
+  v8::TryCatch tryCatch(isolate);
+  v8::Local<v8::Array> privateFields;
+
+  if (!v8::debug::GetPrivateFields(context, object).ToLocal(&privateFields)) {
+    return;
+  }
+
+  for (uint32_t i = 0; i < privateFields->Length(); i += 2) {
+    v8::Local<v8::Data> name;
+    if (!privateFields->Get(context, i).ToLocal(&name)) {
+      tryCatch.Reset();
+      continue;
+    }
+
+    // Weirdly, v8::Private is set to be a subclass of v8::Data and
+    // not v8::Value, meaning, we first need to upcast to v8::Data
+    // and then downcast to v8::Private. Changing the hierarchy is a
+    // breaking change now. Not sure if that's possible.
+    //
+    // TODO(gsathya): Add an IsPrivate method to the v8::Private and
+    // assert here.
+    v8::Local<v8::Private> private_field = v8::Local<v8::Private>::Cast(name);
+    v8::Local<v8::Value> private_name = private_field->Name();
+    CHECK(!private_name->IsUndefined());
+
+    v8::Local<v8::Value> value;
+    if (!privateFields->Get(context, i + 1).ToLocal(&value)) {
+      tryCatch.Reset();
+      continue;
+    }
+
+    auto wrapper = ValueMirror::create(context, value);
+    if (wrapper) {
+      std::unique_ptr<PropertyPreview> propertyPreview;
+      wrapper->buildPropertyPreview(
+          context,
+          toProtocolStringWithTypeCheck(context->GetIsolate(), private_name),
+          &propertyPreview);
+      if (propertyPreview) {
+        if (!*nameLimit) {
+          *overflow = true;
+          return;
+        }
+        --*nameLimit;
+        properties->addItem(std::move(propertyPreview));
+      }
+    }
+  }
+}
+
 class ObjectMirror final : public ValueMirror {
  public:
   ObjectMirror(v8::Local<v8::Value> value, const String16& description)
@@ -899,6 +956,7 @@ class ObjectMirror final : public ValueMirror {
 
     v8::Local<v8::Value> value = m_value;
     while (value->IsProxy()) value = value.As<v8::Proxy>()->GetTarget();
+
     if (value->IsObject() && !value->IsProxy()) {
       v8::Local<v8::Object> objectForPreview = value.As<v8::Object>();
       std::vector<InternalPropertyMirror> internalProperties;
@@ -913,6 +971,9 @@ class ObjectMirror final : public ValueMirror {
         }
       }
 
+      getPrivateFieldsForPreview(context, objectForPreview, nameLimit,
+                                 &overflow, properties.get());
+
       std::vector<PropertyMirror> mirrors;
       if (getPropertiesForPreview(context, objectForPreview, nameLimit,
                                   indexLimit, &overflow, &mirrors)) {

src/keys.cc

@@ -69,12 +69,17 @@ void KeyAccumulator::AddKey(Object key, AddKeyConversion convert) {
 }
 
 void KeyAccumulator::AddKey(Handle<Object> key, AddKeyConversion convert) {
-  if (key->IsSymbol()) {
+  if (filter_ == PRIVATE_NAMES_ONLY) {
+    if (!key->IsSymbol()) return;
+    if (!Symbol::cast(*key)->is_private_name()) return;
+  } else if (key->IsSymbol()) {
     if (filter_ & SKIP_SYMBOLS) return;
-    if (Handle<Symbol>::cast(key)->is_private()) return;
+
+    if (Symbol::cast(*key)->is_private()) return;
   } else if (filter_ & SKIP_STRINGS) {
     return;
   }
+
   if (IsShadowed(key)) return;
   if (keys_.is_null()) {
     keys_ = OrderedHashSet::Allocate(isolate_, 16);
@@ -711,6 +716,23 @@ Maybe<bool> KeyAccumulator::CollectOwnPropertyNames(Handle<JSReceiver> receiver,
   return CollectInterceptorKeys(receiver, object, this, kNamed);
 }
 
+void KeyAccumulator::CollectPrivateNames(Handle<JSReceiver> receiver,
+                                         Handle<JSObject> object) {
+  if (object->HasFastProperties()) {
+    int limit = object->map()->NumberOfOwnDescriptors();
+    Handle<DescriptorArray> descs(object->map()->instance_descriptors(),
+                                  isolate_);
+    CollectOwnPropertyNamesInternal<false>(object, this, descs, 0, limit);
+  } else if (object->IsJSGlobalObject()) {
+    GlobalDictionary::CollectKeysTo(
+        handle(JSGlobalObject::cast(*object)->global_dictionary(), isolate_),
+        this);
+  } else {
+    NameDictionary::CollectKeysTo(
+        handle(object->property_dictionary(), isolate_), this);
+  }
+}
+
 Maybe<bool> KeyAccumulator::CollectAccessCheckInterceptorKeys(
     Handle<AccessCheckInfo> access_check_info, Handle<JSReceiver> receiver,
     Handle<JSObject> object) {
@@ -765,6 +787,11 @@ Maybe<bool> KeyAccumulator::CollectOwnKeys(Handle<JSReceiver> receiver,
     }
     filter_ = static_cast<PropertyFilter>(filter_ | ONLY_ALL_CAN_READ);
   }
+  if (filter_ & PRIVATE_NAMES_ONLY) {
+    CollectPrivateNames(receiver, object);
+    return Just(true);
+  }
+
   MAYBE_RETURN(CollectOwnElementIndices(receiver, object), Nothing<bool>());
   MAYBE_RETURN(CollectOwnPropertyNames(receiver, object), Nothing<bool>());
   return Just(true);
@@ -808,6 +835,12 @@ class NameComparator {
 Maybe<bool> KeyAccumulator::CollectOwnJSProxyKeys(Handle<JSReceiver> receiver,
                                                   Handle<JSProxy> proxy) {
   STACK_CHECK(isolate_, Nothing<bool>());
+  if (filter_ == PRIVATE_NAMES_ONLY) {
+    NameDictionary::CollectKeysTo(
+        handle(proxy->property_dictionary(), isolate_), this);
+    return Just(true);
+  }
+
   // 1. Let handler be the value of the [[ProxyHandler]] internal slot of O.
   Handle<Object> handler(proxy->handler(), isolate_);
   // 2. If handler is null, throw a TypeError exception.

src/keys.h

@@ -52,6 +52,8 @@ class KeyAccumulator final {
                                        Handle<JSObject> object);
   Maybe<bool> CollectOwnPropertyNames(Handle<JSReceiver> receiver,
                                       Handle<JSObject> object);
+  void CollectPrivateNames(Handle<JSReceiver> receiver,
+                           Handle<JSObject> object);
   Maybe<bool> CollectAccessCheckInterceptorKeys(
       Handle<AccessCheckInfo> access_check_info, Handle<JSReceiver> receiver,
       Handle<JSObject> object);

src/objects-inl.h

@@ -412,8 +412,12 @@ bool Object::HasValidElements() {
 
 bool Object::FilterKey(PropertyFilter filter) {
   DCHECK(!IsPropertyCell());
-  if (IsSymbol()) {
+  if (filter == PRIVATE_NAMES_ONLY) {
+    if (!IsSymbol()) return true;
+    return !Symbol::cast(*this)->is_private_name();
+  } else if (IsSymbol()) {
     if (filter & SKIP_SYMBOLS) return true;
+
     if (Symbol::cast(*this)->is_private()) return true;
   } else {
     if (filter & SKIP_STRINGS) return true;

src/objects.cc

@@ -459,6 +459,10 @@ Handle<String> Object::NoSideEffectsToString(Isolate* isolate,
     // -- S y m b o l
     Handle<Symbol> symbol = Handle<Symbol>::cast(input);
 
+    if (symbol->is_private_name()) {
+      return Handle<String>(String::cast(symbol->name()), isolate);
+    }
+
     IncrementalStringBuilder builder(isolate);
     builder.AppendCString("Symbol(");
     if (symbol->name()->IsString()) {
@@ -8436,5 +8440,36 @@ void JSFinalizationGroup::Cleanup(
   }
 }
 
+MaybeHandle<FixedArray> JSReceiver::GetPrivateEntries(
+    Isolate* isolate, Handle<JSReceiver> receiver) {
+  PropertyFilter key_filter = static_cast<PropertyFilter>(PRIVATE_NAMES_ONLY);
+
+  Handle<FixedArray> keys;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate, keys,
+      KeyAccumulator::GetKeys(receiver, KeyCollectionMode::kOwnOnly, key_filter,
+                              GetKeysConversion::kConvertToString),
+      MaybeHandle<FixedArray>());
+
+  Handle<FixedArray> entries =
+      isolate->factory()->NewFixedArray(keys->length() * 2);
+  int length = 0;
+
+  for (int i = 0; i < keys->length(); ++i) {
+    Handle<Object> obj_key = handle(keys->get(i), isolate);
+    Handle<Symbol> key(Symbol::cast(*obj_key), isolate);
+    CHECK(key->is_private_name());
+    Handle<Object> value;
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+        isolate, value, Object::GetProperty(isolate, receiver, key),
+        MaybeHandle<FixedArray>());
+
+    entries->set(length++, *key);
+    entries->set(length++, *value);
+  }
+  DCHECK_EQ(length, entries->length());
+  return FixedArray::ShrinkOrEmpty(isolate, entries, length);
+}
+
 }  // namespace internal
 }  // namespace v8

src/objects/js-objects.h

@@ -271,6 +271,9 @@ class JSReceiver : public HeapObject {
 
   bool HasComplexElements();
 
+  V8_WARN_UNUSED_RESULT static MaybeHandle<FixedArray> GetPrivateEntries(
+      Isolate* isolate, Handle<JSReceiver> receiver);
+
   OBJECT_CONSTRUCTORS(JSReceiver, HeapObject);
 };
 

src/property-details.h

@@ -32,7 +32,6 @@ enum PropertyAttributes {
   // a non-existent property.
 };
 
-
 enum PropertyFilter {
   ALL_PROPERTIES = 0,
   ONLY_WRITABLE = 1,
@@ -41,6 +40,7 @@ enum PropertyFilter {
   SKIP_STRINGS = 8,
   SKIP_SYMBOLS = 16,
   ONLY_ALL_CAN_READ = 32,
+  PRIVATE_NAMES_ONLY = 64,
   ENUMERABLE_STRINGS = ONLY_ENUMERABLE | SKIP_SYMBOLS,
 };
 // Enable fast comparisons of PropertyAttributes against PropertyFilters.

test/cctest/test-debug.cc

@@ -4503,3 +4503,99 @@ TEST(Regress517592) {
   CHECK_EQ(delegate.break_count(), 1);
   v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
 }
+
+TEST(GetPrivateFields) {
+  LocalContext env;
+  v8::Isolate* v8_isolate = CcTest::isolate();
+  v8::internal::Isolate* isolate = CcTest::i_isolate();
+  v8::HandleScope scope(v8_isolate);
+  v8::Local<v8::Context> context = env.local();
+  v8::internal::FLAG_harmony_class_fields = true;
+  v8::internal::FLAG_harmony_private_fields = true;
+  v8::Local<v8::String> source = v8_str(
+      "var X = class {\n"
+      "  #foo = 1;\n"
+      "  #bar = function() {};\n"
+      "}\n"
+      "var x = new X()");
+  CompileRun(source);
+  v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(
+      env->Global()
+          ->Get(context, v8_str(env->GetIsolate(), "x"))
+          .ToLocalChecked());
+  v8::Local<v8::Array> private_names =
+      v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+
+  for (int i = 0; i < 4; i = i + 2) {
+    Handle<v8::internal::JSReceiver> private_name =
+        v8::Utils::OpenHandle(*private_names->Get(context, i)
+                                   .ToLocalChecked()
+                                   ->ToObject(context)
+                                   .ToLocalChecked());
+    Handle<v8::internal::JSValue> private_value =
+        Handle<v8::internal::JSValue>::cast(private_name);
+    Handle<v8::internal::Symbol> priv_symbol(
+        v8::internal::Symbol::cast(private_value->value()), isolate);
+    CHECK(priv_symbol->is_private_name());
+  }
+
+  source = v8_str(
+      "var Y = class {\n"
+      "  #baz = 2;\n"
+      "}\n"
+      "var X = class extends Y{\n"
+      "  #foo = 1;\n"
+      "  #bar = function() {};\n"
+      "}\n"
+      "var x = new X()");
+  CompileRun(source);
+  object = v8::Local<v8::Object>::Cast(
+      env->Global()
+          ->Get(context, v8_str(env->GetIsolate(), "x"))
+          .ToLocalChecked());
+  private_names = v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+
+  for (int i = 0; i < 6; i = i + 2) {
+    Handle<v8::internal::JSReceiver> private_name =
+        v8::Utils::OpenHandle(*private_names->Get(context, i)
+                                   .ToLocalChecked()
+                                   ->ToObject(context)
+                                   .ToLocalChecked());
+    Handle<v8::internal::JSValue> private_value =
+        Handle<v8::internal::JSValue>::cast(private_name);
+    Handle<v8::internal::Symbol> priv_symbol(
+        v8::internal::Symbol::cast(private_value->value()), isolate);
+    CHECK(priv_symbol->is_private_name());
+  }
+
+  source = v8_str(
+      "var Y = class {\n"
+      "  constructor() {"
+      "    return new Proxy({}, {});"
+      "  }"
+      "}\n"
+      "var X = class extends Y{\n"
+      "  #foo = 1;\n"
+      "  #bar = function() {};\n"
+      "}\n"
+      "var x = new X()");
+  CompileRun(source);
+  object = v8::Local<v8::Object>::Cast(
+      env->Global()
+          ->Get(context, v8_str(env->GetIsolate(), "x"))
+          .ToLocalChecked());
+  private_names = v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+
+  for (int i = 0; i < 4; i = i + 2) {
+    Handle<v8::internal::JSReceiver> private_name =
+        v8::Utils::OpenHandle(*private_names->Get(context, i)
+                                   .ToLocalChecked()
+                                   ->ToObject(context)
+                                   .ToLocalChecked());
+    Handle<v8::internal::JSValue> private_value =
+        Handle<v8::internal::JSValue>::cast(private_name);
+    Handle<v8::internal::Symbol> priv_symbol(
+        v8::internal::Symbol::cast(private_value->value()), isolate);
+    CHECK(priv_symbol->is_private_name());
+  }
+}

test/inspector/debugger/object-preview-internal-properties-expected.txt

@@ -259,3 +259,39 @@ expression: Promise.resolve(42)
     value : 42
 }
 
+Running test: privateNames
+expression: new class { #foo = 1; #bar = 2; baz = 3;}
+{
+    name : #foo
+    type : number
+    value : 1
+}
+{
+    name : #bar
+    type : number
+    value : 2
+}
+{
+    name : baz
+    type : number
+    value : 3
+}
+
+expression: new class extends class { #baz = 3; } { #foo = 1; #bar = 2; }
+{
+    name : #baz
+    type : number
+    value : 3
+}
+{
+    name : #foo
+    type : number
+    value : 1
+}
+{
+    name : #bar
+    type : number
+    value : 2
+}
+
+expression: new class extends class { constructor() { return new Proxy({}, {}); } } { #foo = 1; #bar = 2; }

test/inspector/debugger/object-preview-internal-properties.js

 // Copyright 2016 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+//
+// Flags: --harmony-class-fields
 
 let {session, contextGroup, Protocol} = InspectorTest.start("Check internal properties reported in object preview.");
 
@@ -72,6 +74,14 @@ InspectorTest.runTestSuite([
     Protocol.Runtime.evaluate({ expression: "Array.prototype.__defineGetter__(\"0\",() => { throw new Error() }) "})
       .then(() => checkExpression("Promise.resolve(42)"))
       .then(next);
+  },
+
+  function privateNames(next)
+  {
+    checkExpression("new class { #foo = 1; #bar = 2; baz = 3;}")
+      .then(() => checkExpression("new class extends class { #baz = 3; } { #foo = 1; #bar = 2; }"))
+      .then(() => checkExpression("new class extends class { constructor() { return new Proxy({}, {}); } } { #foo = 1; #bar = 2; }"))
+      .then(next);
   }
 ]);