[class] implement inspector support for private instance methods

This patch implements inspector support for private instance methods:

- Previously to implement brand checking for instances with private
  instance methods we store the brand both as the value with the brand
  itself as the key in the stances. Now we make the value the context
  associated with the class instead.
- To retrieve the private instance methods and accessors from the
  instances at runtime, we look into the contexts stored with the
  brands, and analyze the scope info to get the names as well as
  context slot indices of them.
- This patch extends the `PrivatePropertyDescriptor` in the inspector
  protocol to include optional `get` and `set` fields, and make the
  `value` field optional (similar to `PropertyDescriptor`s).
  Private fields or private instance methods are returned in the
  `value` field while private accessors are returned in the `get`
  and/or `set` field. Property previews for the instaces containing
  private instance methods and accessors are also updated similarly,
  although no additional protocol change is necessary since the
  `PropertyPreview` type can already be used to display accessors.

Design doc: https://docs.google.com/document/d/1N91LObhQexnB0eE7EvGe57HsvNMFX16CaWu-XCTnnmY/edit

Bug: v8:9839, v8:8330
Change-Id: If37090bd23833a18f75deb1249ca5c4405ca2bf2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1934407
Commit-Queue: Joyee Cheung <joyee@igalia.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Reviewed-by: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65337}

include/js_protocol.pdl

@@ -1073,7 +1073,13 @@ domain Runtime
       # Private property name.
       string name
       # The value associated with the private property.
-      RemoteObject value
+      optional RemoteObject value
+      # A function which serves as a getter for the private property,
+      # or `undefined` if there is no getter (accessor descriptors only).
+      optional RemoteObject get
+      # A function which serves as a setter for the private property,
+      # or `undefined` if there is no setter (accessor descriptors only).
+      optional RemoteObject set
 
   # Represents function call argument. Either remote object id `objectId`, primitive `value`,
   # unserializable primitive value or neither of (for undefined) them should be specified.

src/api/api-inl.h

@@ -66,6 +66,7 @@ inline Local<To> Utils::Convert(v8::internal::Handle<From> obj) {
     return Convert<v8::internal::JSTypedArray, v8::Type##Array>(obj); \
   }
 
+MAKE_TO_LOCAL(ToLocal, AccessorPair, debug::AccessorPair)
 MAKE_TO_LOCAL(ToLocal, Context, Context)
 MAKE_TO_LOCAL(ToLocal, Object, Value)
 MAKE_TO_LOCAL(ToLocal, Module, Module)

src/api/api.cc

@@ -3761,6 +3761,12 @@ void v8::WasmModuleObject::CheckCast(Value* that) {
                   "Could not convert to wasm module object");
 }
 
+void v8::debug::AccessorPair::CheckCast(Value* that) {
+  i::Handle<i::Object> obj = Utils::OpenHandle(that);
+  Utils::ApiCheck(obj->IsAccessorPair(), "v8::AccessorPair::Cast",
+                  "Could not convert to AccessorPair");
+}
+
 v8::BackingStore::~BackingStore() {
   auto i_this = reinterpret_cast<const i::BackingStore*>(this);
   i_this->~BackingStore();  // manually call internal destructor
@@ -9252,16 +9258,91 @@ MaybeLocal<Array> debug::GetInternalProperties(Isolate* v8_isolate,
   return Utils::ToLocal(result);
 }
 
-MaybeLocal<Array> debug::GetPrivateFields(Local<Context> context,
+bool debug::GetPrivateMembers(Local<Context> context, Local<Object> value,
-                                          Local<Object> value) {
+                              std::vector<Local<Value>>* names_out,
-  PREPARE_FOR_EXECUTION(context, debug, GetPrivateFields, Array);
+                              std::vector<Local<Value>>* values_out) {
-  i::Handle<i::JSReceiver> val = Utils::OpenHandle(*value);
+  i::Isolate* isolate = reinterpret_cast<i::Isolate*>(context->GetIsolate());
-  i::Handle<i::JSArray> result;
+  LOG_API(isolate, debug, GetPrivateMembers);
-  i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
+  ENTER_V8_NO_SCRIPT_NO_EXCEPTION(isolate);
-  has_pending_exception =
+  i::Handle<i::JSReceiver> receiver = Utils::OpenHandle(*value);
-      !(internal_isolate->debug()->GetPrivateFields(val).ToHandle(&result));
+  i::Handle<i::JSArray> names;
-  RETURN_ON_FAILED_EXECUTION(Array);
+  i::Handle<i::FixedArray> values;
-  RETURN_ESCAPED(Utils::ToLocal(result));
+
+  i::PropertyFilter key_filter =
+      static_cast<i::PropertyFilter>(i::PropertyFilter::PRIVATE_NAMES_ONLY);
+  i::Handle<i::FixedArray> keys;
+  ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+      isolate, keys,
+      i::KeyAccumulator::GetKeys(receiver, i::KeyCollectionMode::kOwnOnly,
+                                 key_filter,
+                                 i::GetKeysConversion::kConvertToString),
+      false);
+
+  // Estimate number of private entries to return in the FixedArray.
+  int private_entries_count = 0;
+  for (int i = 0; i < keys->length(); ++i) {
+    // Exclude the private brand symbols.
+    i::Handle<i::Symbol> key(i::Symbol::cast(keys->get(i)), isolate);
+    if (key->is_private_brand()) {
+      i::Handle<i::Object> value;
+      ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+          isolate, value, i::Object::GetProperty(isolate, receiver, key),
+          false);
+
+      i::Handle<i::Context> context(i::Context::cast(*value), isolate);
+      i::Handle<i::ScopeInfo> scope_info(context->scope_info(), isolate);
+      // At least one slot contains the brand symbol so it does not count.
+      private_entries_count += (scope_info->ContextLocalCount() - 1);
+    } else {
+      private_entries_count++;
+    }
+  }
+
+  DCHECK(names_out->empty());
+  names_out->reserve(private_entries_count);
+  DCHECK(values_out->empty());
+  values_out->reserve(private_entries_count);
+  for (int i = 0; i < keys->length(); ++i) {
+    i::Handle<i::Object> obj_key(keys->get(i), isolate);
+    i::Handle<i::Symbol> key(i::Symbol::cast(*obj_key), isolate);
+    CHECK(key->is_private_name());
+    i::Handle<i::Object> value;
+    ASSIGN_RETURN_ON_EXCEPTION_VALUE(
+        isolate, value, i::Object::GetProperty(isolate, receiver, key), false);
+
+    if (key->is_private_brand()) {
+      DCHECK(value->IsContext());
+      i::Handle<i::Context> context(i::Context::cast(*value), isolate);
+      i::Handle<i::ScopeInfo> scope_info(context->scope_info(), isolate);
+      int local_count = scope_info->ContextLocalCount();
+
+      for (int j = 0; j < local_count; ++j) {
+        i::VariableMode mode = scope_info->ContextLocalMode(j);
+        if (!i::IsPrivateMethodOrAccessorVariableMode(mode)) {
+          continue;
+        }
+
+        i::Handle<i::String> name(scope_info->ContextLocalName(j), isolate);
+        int context_index = scope_info->ContextHeaderLength() + j;
+        i::Handle<i::Object> slot_value(context->get(context_index), isolate);
+        DCHECK_IMPLIES(mode == i::VariableMode::kPrivateMethod,
+                       slot_value->IsJSFunction());
+        DCHECK_IMPLIES(mode != i::VariableMode::kPrivateMethod,
+                       slot_value->IsAccessorPair());
+        names_out->push_back(Utils::ToLocal(name));
+        values_out->push_back(Utils::ToLocal(slot_value));
+      }
+    } else {  // Private fields
+      i::Handle<i::String> name(
+          i::String::cast(i::Symbol::cast(*key).description()), isolate);
+      names_out->push_back(Utils::ToLocal(name));
+      values_out->push_back(Utils::ToLocal(value));
+    }
+  }
+
+  DCHECK_EQ(names_out->size(), values_out->size());
+  DCHECK_LE(names_out->size(), private_entries_count);
+  return true;
 }
 
 Local<Context> debug::GetCreationContext(Local<Object> value) {
@@ -10140,6 +10221,25 @@ debug::WeakMap* debug::WeakMap::Cast(v8::Value* value) {
   return static_cast<debug::WeakMap*>(value);
 }
 
+Local<Value> debug::AccessorPair::getter() {
+  i::Handle<i::AccessorPair> accessors = Utils::OpenHandle(this);
+  i::Isolate* isolate = accessors->GetIsolate();
+  i::Handle<i::Object> getter(accessors->getter(), isolate);
+  return Utils::ToLocal(getter);
+}
+
+Local<Value> debug::AccessorPair::setter() {
+  i::Handle<i::AccessorPair> accessors = Utils::OpenHandle(this);
+  i::Isolate* isolate = accessors->GetIsolate();
+  i::Handle<i::Object> setter(accessors->setter(), isolate);
+  return Utils::ToLocal(setter);
+}
+
+bool debug::AccessorPair::IsAccessorPair(Local<Value> that) {
+  i::Handle<i::Object> obj = Utils::OpenHandle(*that);
+  return obj->IsAccessorPair();
+}
+
 const char* CpuProfileNode::GetFunctionNameStr() const {
   const i::ProfileNode* node = reinterpret_cast<const i::ProfileNode*>(this);
   return node->entry()->name();

src/api/api.h

@@ -30,6 +30,7 @@ class JSArrayBufferView;
 }  // namespace internal
 
 namespace debug {
+class AccessorPair;
 class GeneratorObject;
 class Script;
 class WeakMap;
@@ -127,6 +128,7 @@ class RegisteredExtension {
   V(debug::GeneratorObject, JSGeneratorObject) \
   V(debug::Script, Script)                     \
   V(debug::WeakMap, JSWeakMap)                 \
+  V(debug::AccessorPair, AccessorPair)         \
   V(Promise, JSPromise)                        \
   V(Primitive, Object)                         \
   V(PrimitiveArray, FixedArray)                \
@@ -143,6 +145,8 @@ class Utils {
   static void ReportOOMFailure(v8::internal::Isolate* isolate,
                                const char* location, bool is_heap_oom);
 
+  static inline Local<debug::AccessorPair> ToLocal(
+      v8::internal::Handle<v8::internal::AccessorPair> obj);
   static inline Local<Context> ToLocal(
       v8::internal::Handle<v8::internal::Context> obj);
   static inline Local<Value> ToLocal(

src/debug/debug-interface.h

@@ -52,12 +52,20 @@ V8_EXPORT_PRIVATE void ClearBreakOnNextFunctionCall(Isolate* isolate);
 MaybeLocal<Array> GetInternalProperties(Isolate* isolate, Local<Value> value);
 
 /**
- * Returns array of private fields specific to the value type. Result has
+ * Returns through the out parameters names_out a vector of names
- * the following format: [<name>, <value>,...,<name>, <value>]. Result array
+ * in v8::String for private members, including fields, methods,
- * will be allocated in the current context.
+ * accessors specific to the value type.
+ * The values are returned through the out parameter values_out in the
+ * corresponding indices. Private fields and methods are returned directly
+ * while accessors are returned as v8::debug::AccessorPair. Missing components
+ * in the accessor pairs are null.
+ * If an exception occurs, false is returned. Otherwise true is returned.
+ * Results will be allocated in the current context and handle scope.
  */
-V8_EXPORT_PRIVATE MaybeLocal<Array> GetPrivateFields(Local<Context> context,
+V8_EXPORT_PRIVATE bool GetPrivateMembers(Local<Context> context,
-                                                     Local<Object> value);
+                                         Local<Object> value,
+                                         std::vector<Local<Value>>* names_out,
+                                         std::vector<Local<Value>>* values_out);
 
 /**
  * Forwards to v8::Object::CreationContext, but with special handling for
@@ -513,6 +521,25 @@ class WeakMap : public v8::Object {
   WeakMap();
 };
 
+/**
+ * Pairs of accessors.
+ *
+ * In the case of private accessors, getters and setters are either null or
+ * Functions.
+ */
+class AccessorPair : public v8::Value {
+ public:
+  V8_EXPORT_PRIVATE v8::Local<v8::Value> getter();
+  V8_EXPORT_PRIVATE v8::Local<v8::Value> setter();
+
+  V8_EXPORT_PRIVATE static bool IsAccessorPair(v8::Local<v8::Value> obj);
+  V8_INLINE V8_EXPORT_PRIVATE static AccessorPair* Cast(v8::Value* obj);
+
+ private:
+  AccessorPair();
+  static void CheckCast(v8::Value* obj);
+};
+
 struct PropertyDescriptor {
   bool enumerable : 1;
   bool has_enumerable : 1;
@@ -545,6 +572,14 @@ class PropertyIterator {
   virtual bool is_own() = 0;
   virtual bool is_array_index() = 0;
 };
+
+AccessorPair* AccessorPair::Cast(v8::Value* value) {
+#ifdef V8_ENABLE_CHECKS
+  CheckCast(value);
+#endif
+  return static_cast<AccessorPair*>(value);
+}
+
 }  // namespace debug
 }  // namespace v8
 

src/debug/debug.cc

@@ -1409,22 +1409,6 @@ 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

@@ -267,8 +267,6 @@ class V8_EXPORT_PRIVATE 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/injected-script.cc

@@ -393,22 +393,53 @@ Response InjectedScript::getInternalAndPrivateProperties(
                            .setValue(std::move(remoteObject))
                            .build());
   }
+
   std::vector<PrivatePropertyMirror> privatePropertyWrappers =
-      ValueMirror::getPrivateProperties(m_context->context(), value_obj);
+      ValueMirror::getPrivateProperties(context, value_obj);
   for (const auto& privateProperty : privatePropertyWrappers) {
+    std::unique_ptr<PrivatePropertyDescriptor> descriptor =
+        PrivatePropertyDescriptor::create()
+            .setName(privateProperty.name)
+            .build();
+
     std::unique_ptr<RemoteObject> remoteObject;
-    Response response = privateProperty.value->buildRemoteObject(
+    Response response;
-        m_context->context(), WrapMode::kNoPreview, &remoteObject);
+    DCHECK((privateProperty.getter || privateProperty.setter) ^
-    if (!response.isSuccess()) return response;
+           (!!privateProperty.value));
-    response = bindRemoteObjectIfNeeded(sessionId, context,
+    if (privateProperty.value) {
-                                        privateProperty.value->v8Value(),
+      response = privateProperty.value->buildRemoteObject(
-                                        groupName, remoteObject.get());
+          context, WrapMode::kNoPreview, &remoteObject);
-    if (!response.isSuccess()) return response;
+      if (!response.isSuccess()) return response;
-    (*privateProperties)
+      response = bindRemoteObjectIfNeeded(sessionId, context,
-        ->emplace_back(PrivatePropertyDescriptor::create()
+                                          privateProperty.value->v8Value(),
-                           .setName(privateProperty.name)
+                                          groupName, remoteObject.get());
-                           .setValue(std::move(remoteObject))
+      if (!response.isSuccess()) return response;
-                           .build());
+      descriptor->setValue(std::move(remoteObject));
+    }
+
+    if (privateProperty.getter) {
+      response = privateProperty.getter->buildRemoteObject(
+          context, WrapMode::kNoPreview, &remoteObject);
+      if (!response.isSuccess()) return response;
+      response = bindRemoteObjectIfNeeded(sessionId, context,
+                                          privateProperty.getter->v8Value(),
+                                          groupName, remoteObject.get());
+      if (!response.isSuccess()) return response;
+      descriptor->setGet(std::move(remoteObject));
+    }
+
+    if (privateProperty.setter) {
+      response = privateProperty.setter->buildRemoteObject(
+          context, WrapMode::kNoPreview, &remoteObject);
+      if (!response.isSuccess()) return response;
+      response = bindRemoteObjectIfNeeded(sessionId, context,
+                                          privateProperty.setter->v8Value(),
+                                          groupName, remoteObject.get());
+      if (!response.isSuccess()) return response;
+      descriptor->setSet(std::move(remoteObject));
+    }
+
+    (*privateProperties)->emplace_back(std::move(descriptor));
   }
   return Response::OK();
 }

src/inspector/value-mirror.cc

@@ -841,7 +841,15 @@ void getPrivatePropertiesForPreview(
   std::vector<String16> whitelist;
   for (auto& mirror : mirrors) {
     std::unique_ptr<PropertyPreview> propertyPreview;
-    mirror.value->buildPropertyPreview(context, mirror.name, &propertyPreview);
+    if (mirror.value) {
+      mirror.value->buildPropertyPreview(context, mirror.name,
+                                         &propertyPreview);
+    } else {
+      propertyPreview = PropertyPreview::create()
+                            .setName(mirror.name)
+                            .setType(PropertyPreview::TypeEnum::Accessor)
+                            .build();
+    }
     if (!propertyPreview) continue;
     if (!*nameLimit) {
       *overflow = true;
@@ -1415,40 +1423,40 @@ std::vector<PrivatePropertyMirror> ValueMirror::getPrivateProperties(
   v8::TryCatch tryCatch(isolate);
   v8::Local<v8::Array> privateProperties;
 
-  if (!v8::debug::GetPrivateFields(context, object).ToLocal(&privateProperties))
+  std::vector<v8::Local<v8::Value>> names;
+  std::vector<v8::Local<v8::Value>> values;
+  if (!v8::debug::GetPrivateMembers(context, object, &names, &values))
     return mirrors;
 
-  for (uint32_t i = 0; i < privateProperties->Length(); i += 2) {
+  size_t len = values.size();
-    v8::Local<v8::Value> name;
+  for (size_t i = 0; i < len; i++) {
-    if (!privateProperties->Get(context, i).ToLocal(&name)) {
+    v8::Local<v8::Value> name = names[i];
-      tryCatch.Reset();
+    DCHECK(name->IsString());
-      continue;
+    v8::Local<v8::Value> value = values[i];
+
+    std::unique_ptr<ValueMirror> valueMirror;
+    std::unique_ptr<ValueMirror> getterMirror;
+    std::unique_ptr<ValueMirror> setterMirror;
+    if (v8::debug::AccessorPair::IsAccessorPair(value)) {
+      v8::Local<v8::debug::AccessorPair> accessors =
+          value.As<v8::debug::AccessorPair>();
+      v8::Local<v8::Value> getter = accessors->getter();
+      v8::Local<v8::Value> setter = accessors->setter();
+      if (!getter->IsNull()) {
+        getterMirror = ValueMirror::create(context, getter);
+      }
+      if (!setter->IsNull()) {
+        setterMirror = ValueMirror::create(context, setter);
+      }
+    } else {
+      valueMirror = ValueMirror::create(context, value);
     }
 
-    // Weirdly, v8::Private is set to be a subclass of v8::Data and
+    mirrors.emplace_back(PrivatePropertyMirror{
-    // not v8::Value, meaning, we first need to upcast to v8::Data
+        toProtocolStringWithTypeCheck(context->GetIsolate(), name),
-    // and then downcast to v8::Private. Changing the hierarchy is a
+        std::move(valueMirror), std::move(getterMirror),
-    // breaking change now. Not sure if that's possible.
+        std::move(setterMirror)});
-    //
-    // 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();
-    DCHECK(!private_name->IsUndefined());
-
-    v8::Local<v8::Value> value;
-    if (!privateProperties->Get(context, i + 1).ToLocal(&value)) {
-      tryCatch.Reset();
-      continue;
-    }
-    auto wrapper = ValueMirror::create(context, value);
-    if (wrapper) {
-      mirrors.emplace_back(PrivatePropertyMirror{
-          toProtocolStringWithTypeCheck(context->GetIsolate(), private_name),
-          std::move(wrapper)});
-    }
   }
-
   return mirrors;
 }
 

src/inspector/value-mirror.h

@@ -23,6 +23,8 @@ enum class WrapMode;
 struct PrivatePropertyMirror {
   String16 name;
   std::unique_ptr<ValueMirror> value;
+  std::unique_ptr<ValueMirror> getter;
+  std::unique_ptr<ValueMirror> setter;
 };
 
 struct InternalPropertyMirror {

src/interpreter/bytecode-generator.cc

@@ -2368,13 +2368,16 @@ void BytecodeGenerator::BuildInvalidPropertyAccess(MessageTemplate tmpl,
 }
 
 void BytecodeGenerator::BuildPrivateBrandInitialization(Register receiver) {
-  RegisterList brand_args = register_allocator()->NewRegisterList(2);
+  RegisterList brand_args = register_allocator()->NewRegisterList(3);
   Variable* brand = info()->scope()->outer_scope()->AsClassScope()->brand();
-  DCHECK_NOT_NULL(brand);
+  int depth = execution_context()->ContextChainDepth(brand->scope());
+  ContextScope* class_context = execution_context()->Previous(depth);
+
   BuildVariableLoad(brand, HoleCheckMode::kElided);
   builder()
       ->StoreAccumulatorInRegister(brand_args[1])
       .MoveRegister(receiver, brand_args[0])
+      .MoveRegister(class_context->reg(), brand_args[2])
       .CallRuntime(Runtime::kAddPrivateBrand, brand_args);
 }
 

src/logging/counters.h

@@ -730,6 +730,7 @@ class RuntimeCallTimer final {
   TRACER_BACKGROUND_SCOPES(V)
 
 #define FOR_EACH_API_COUNTER(V)                            \
+  V(AccessorPair_New)                                      \
   V(ArrayBuffer_Cast)                                      \
   V(ArrayBuffer_Detach)                                    \
   V(ArrayBuffer_New)                                       \
@@ -749,7 +750,7 @@ class RuntimeCallTimer final {
   V(Date_New)                                              \
   V(Date_NumberValue)                                      \
   V(Debug_Call)                                            \
-  V(debug_GetPrivateFields)                                \
+  V(debug_GetPrivateMembers)                               \
   V(Error_New)                                             \
   V(External_New)                                          \
   V(Float32Array_New)                                      \

src/objects/js-objects.h

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

src/objects/objects.cc

@@ -8085,52 +8085,5 @@ Maybe<bool> JSFinalizationGroup::Cleanup(
   return Just(true);
 }
 
-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>());
-
-  // Calculate number of private entries to return in the FixedArray.
-  // TODO(v8:9839): take the number of private methods/accessors into account.
-  int private_brand_count = 0;
-  for (int i = 0; i < keys->length(); ++i) {
-    // Exclude the private brand symbols.
-    if (Symbol::cast(keys->get(i)).is_private_brand()) {
-      private_brand_count++;
-    }
-  }
-  int private_entries_count = keys->length() - private_brand_count;
-
-  Handle<FixedArray> entries =
-      isolate->factory()->NewFixedArray(private_entries_count * 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());
-    if (key->is_private_brand()) {
-      // TODO(v8:9839): get the private methods/accessors of the instance
-      // using the brand and add them to the entries.
-      continue;
-    }
-    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/struct-inl.h

@@ -24,6 +24,8 @@ TQ_OBJECT_CONSTRUCTORS_IMPL(Tuple2)
 TQ_OBJECT_CONSTRUCTORS_IMPL(Tuple3)
 TQ_OBJECT_CONSTRUCTORS_IMPL(AccessorPair)
 
+NEVER_READ_ONLY_SPACE_IMPL(AccessorPair)
+
 TQ_OBJECT_CONSTRUCTORS_IMPL(ClassPositions)
 
 void Struct::InitializeBody(int object_size) {

src/objects/struct.h

@@ -49,6 +49,7 @@ class Tuple3 : public TorqueGeneratedTuple3<Tuple3, Struct> {
 //   * null: an accessor which has not been set
 class AccessorPair : public TorqueGeneratedAccessorPair<AccessorPair, Struct> {
  public:
+  NEVER_READ_ONLY_SPACE
   static Handle<AccessorPair> Copy(Isolate* isolate, Handle<AccessorPair> pair);
 
   inline Object get(AccessorComponent component);

src/runtime/runtime-object.cc

@@ -1241,9 +1241,10 @@ RUNTIME_FUNCTION(Runtime_CreatePrivateAccessors) {
 
 RUNTIME_FUNCTION(Runtime_AddPrivateBrand) {
   HandleScope scope(isolate);
-  DCHECK_EQ(args.length(), 2);
+  DCHECK_EQ(args.length(), 3);
   CONVERT_ARG_HANDLE_CHECKED(JSReceiver, receiver, 0);
   CONVERT_ARG_HANDLE_CHECKED(Symbol, brand, 1);
+  CONVERT_ARG_HANDLE_CHECKED(Context, context, 2);
   DCHECK(brand->is_private_name());
 
   LookupIterator it = LookupIterator::PropertyOrElement(
@@ -1256,9 +1257,7 @@ RUNTIME_FUNCTION(Runtime_AddPrivateBrand) {
 
   PropertyAttributes attributes =
       static_cast<PropertyAttributes>(DONT_ENUM | DONT_DELETE | READ_ONLY);
-  // TODO(joyee): we could use this slot to store something useful. For now,
+  CHECK(Object::AddDataProperty(&it, context, attributes, Just(kDontThrow),
-  // store the brand itself.
-  CHECK(Object::AddDataProperty(&it, brand, attributes, Just(kDontThrow),
                                 StoreOrigin::kMaybeKeyed)
             .FromJust());
   return *receiver;

src/runtime/runtime.h

@@ -282,7 +282,7 @@ namespace internal {
 #define FOR_EACH_INTRINSIC_OBJECT(F, I)                         \
   F(AddDictionaryProperty, 3, 1)                                \
   F(AddPrivateField, 3, 1)                                      \
-  F(AddPrivateBrand, 2, 1)                                      \
+  F(AddPrivateBrand, 3, 1)                                      \
   F(AllocateHeapNumber, 0, 1)                                   \
   F(ClassOf, 1, 1)                                              \
   F(CollectTypeProfile, 3, 1)                                   \

test/cctest/interpreter/bytecode_expectations/PrivateAccessorAccess.golden

@@ -22,43 +22,44 @@ snippet: "
   var test = A;
   new test;
 "
-frame size: 6
+frame size: 7
 parameter count: 1
-bytecode array length: 95
+bytecode array length: 98
 bytecodes: [
   /*   67 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   76 S> */ B(LdaCurrentContextSlot), U8(2),
-                B(Star), R(3),
+                B(Star), R(4),
                 B(LdaCurrentContextSlot), U8(3),
   /*   81 E> */ B(LdaKeyedProperty), R(this), U8(0),
-                B(CallRuntime), U16(Runtime::kLoadPrivateGetter), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kLoadPrivateGetter), R(4), U8(1),
-                B(Star), R(4),
-                B(CallProperty0), R(4), R(this), U8(2),
-                B(Inc), U8(4),
-                B(Star), R(4),
-  /*   83 E> */ B(CallRuntime), U16(Runtime::kLoadPrivateSetter), R(3), U8(1),
                 B(Star), R(5),
-                B(CallProperty1), R(5), R(this), R(4), U8(5),
+                B(CallProperty0), R(5), R(this), U8(2),
+                B(Inc), U8(4),
+                B(Star), R(5),
+  /*   83 E> */ B(CallRuntime), U16(Runtime::kLoadPrivateSetter), R(4), U8(1),
+                B(Star), R(6),
+                B(CallProperty1), R(6), R(this), R(5), U8(5),
   /*   91 S> */ B(LdaSmi), I8(1),
-                B(Star), R(2),
+                B(Star), R(3),
                 B(LdaCurrentContextSlot), U8(2),
-                B(Star), R(4),
+                B(Star), R(5),
                 B(LdaCurrentContextSlot), U8(3),
   /*   96 E> */ B(LdaKeyedProperty), R(this), U8(7),
-                B(CallRuntime), U16(Runtime::kLoadPrivateSetter), R(4), U8(1),
+                B(CallRuntime), U16(Runtime::kLoadPrivateSetter), R(5), U8(1),
-                B(Star), R(5),
+                B(Star), R(6),
-                B(CallProperty1), R(5), R(this), R(2), U8(9),
+                B(CallProperty1), R(6), R(this), R(3), U8(9),
   /*  108 S> */ B(LdaCurrentContextSlot), U8(2),
-                B(Star), R(3),
+                B(Star), R(4),
                 B(LdaCurrentContextSlot), U8(3),
   /*  120 E> */ B(LdaKeyedProperty), R(this), U8(11),
-                B(CallRuntime), U16(Runtime::kLoadPrivateGetter), R(3), U8(1),
+                B(CallRuntime), U16(Runtime::kLoadPrivateGetter), R(4), U8(1),
-                B(Star), R(4),
+                B(Star), R(5),
-                B(CallProperty0), R(4), R(this), U8(13),
+                B(CallProperty0), R(5), R(this), U8(13),
   /*  123 S> */ B(Return),
 ]
 constant pool: [
@@ -75,20 +76,21 @@ snippet: "
   var test = B;
   new test;
 "
-frame size: 4
+frame size: 5
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   48 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   53 S> */ B(Wide), B(LdaSmi), I16(264),
-                B(Star), R(2),
-                B(LdaConstant), U8(0),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kNewTypeError), R(2), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
                 B(Throw),
 ]
 constant pool: [
@@ -106,20 +108,21 @@ snippet: "
   var test = C;
   new test;
 "
-frame size: 4
+frame size: 5
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   41 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   46 S> */ B(Wide), B(LdaSmi), I16(263),
-                B(Star), R(2),
-                B(LdaConstant), U8(0),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kNewTypeError), R(2), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
                 B(Throw),
 ]
 constant pool: [
@@ -137,20 +140,21 @@ snippet: "
   var test = D;
   new test;
 "
-frame size: 4
+frame size: 5
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   48 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   53 S> */ B(Wide), B(LdaSmi), I16(264),
-                B(Star), R(2),
-                B(LdaConstant), U8(0),
                 B(Star), R(3),
-  /*   61 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(2), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+  /*   61 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
                 B(Throw),
 ]
 constant pool: [
@@ -168,20 +172,21 @@ snippet: "
   var test = E;
   new test;
 "
-frame size: 5
+frame size: 6
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   41 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   46 S> */ B(Wide), B(LdaSmi), I16(263),
-                B(Star), R(3),
-                B(LdaConstant), U8(0),
                 B(Star), R(4),
-                B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(5),
+                B(CallRuntime), U16(Runtime::kNewTypeError), R(4), U8(2),
                 B(Throw),
 ]
 constant pool: [

test/cctest/interpreter/bytecode_expectations/PrivateMethodAccess.golden

@@ -17,20 +17,21 @@ snippet: "
   var test = A;
   new A;
 "
-frame size: 3
+frame size: 4
 parameter count: 1
-bytecode array length: 28
+bytecode array length: 31
 bytecodes: [
   /*   44 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   49 S> */ B(LdaCurrentContextSlot), U8(3),
   /*   61 E> */ B(LdaKeyedProperty), R(this), U8(0),
                 B(LdaCurrentContextSlot), U8(2),
-                B(Star), R(2),
+                B(Star), R(3),
-  /*   63 E> */ B(CallAnyReceiver), R(2), R(this), U8(1), U8(2),
+  /*   63 E> */ B(CallAnyReceiver), R(3), R(this), U8(1), U8(2),
   /*   66 S> */ B(Return),
 ]
 constant pool: [
@@ -48,20 +49,21 @@ snippet: "
   var test = B;
   new test;
 "
-frame size: 4
+frame size: 5
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   44 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   49 S> */ B(Wide), B(LdaSmi), I16(262),
-                B(Star), R(2),
-                B(LdaConstant), U8(0),
                 B(Star), R(3),
-  /*   57 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(2), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+  /*   57 E> */ B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
                 B(Throw),
 ]
 constant pool: [
@@ -80,20 +82,21 @@ snippet: "
   var test = C;
   new test;
 "
-frame size: 4
+frame size: 5
 parameter count: 1
-bytecode array length: 29
+bytecode array length: 32
 bytecodes: [
   /*   44 E> */ B(StackCheck),
                 B(LdaCurrentContextSlot), U8(3),
                 B(Star), R(1),
                 B(Mov), R(this), R(0),
-                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(2),
+                B(Mov), R(context), R(2),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(0), U8(3),
   /*   49 S> */ B(Wide), B(LdaSmi), I16(262),
-                B(Star), R(2),
-                B(LdaConstant), U8(0),
                 B(Star), R(3),
-                B(CallRuntime), U16(Runtime::kNewTypeError), R(2), U8(2),
+                B(LdaConstant), U8(0),
+                B(Star), R(4),
+                B(CallRuntime), U16(Runtime::kNewTypeError), R(3), U8(2),
                 B(Throw),
 ]
 constant pool: [
@@ -102,3 +105,46 @@ constant pool: [
 handlers: [
 ]
 
+---
+snippet: "
+  class D {
+    #d() { return 1; }
+    constructor() { (() => this)().#d(); }
+  }
+  
+  var test = D;
+  new test;
+"
+frame size: 6
+parameter count: 1
+bytecode array length: 58
+bytecodes: [
+  /*   44 E> */ B(StackCheck),
+                B(CreateFunctionContext), U8(0), U8(1),
+                B(PushContext), R(0),
+                B(Ldar), R(this),
+                B(StaCurrentContextSlot), U8(2),
+                B(LdaContextSlot), R(0), U8(3), U8(0),
+                B(Star), R(2),
+                B(Mov), R(this), R(1),
+                B(Mov), R(0), R(3),
+                B(CallRuntime), U16(Runtime::kAddPrivateBrand), R(1), U8(3),
+  /*   49 S> */ B(CreateClosure), U8(1), U8(0), U8(2),
+                B(Star), R(5),
+  /*   61 E> */ B(CallUndefinedReceiver0), R(5), U8(0),
+                B(Star), R(5),
+                B(LdaContextSlot), R(0), U8(3), U8(0),
+  /*   63 E> */ B(LdaKeyedProperty), R(5), U8(2),
+                B(LdaContextSlot), R(0), U8(2), U8(0),
+                B(Star), R(4),
+  /*   66 E> */ B(CallAnyReceiver), R(4), R(5), U8(1), U8(4),
+                B(LdaUndefined),
+  /*   70 S> */ B(Return),
+]
+constant pool: [
+  SCOPE_INFO_TYPE,
+  SHARED_FUNCTION_INFO_TYPE,
+]
+handlers: [
+]
+

test/cctest/interpreter/test-bytecode-generator.cc

@@ -2821,6 +2821,14 @@ TEST(PrivateMethodAccess) {
       "}\n"
       "\n"
       "var test = C;\n"
+      "new test;\n",
+
+      "class D {\n"
+      "  #d() { return 1; }\n"
+      "  constructor() { (() => this)().#d(); }\n"
+      "}\n"
+      "\n"
+      "var test = D;\n"
       "new test;\n"};
 
   CHECK(CompareTexts(BuildActual(printer, snippets),

test/cctest/test-debug.cc

@@ -4710,16 +4710,22 @@ TEST(Regress517592) {
   v8::debug::SetDebugDelegate(env->GetIsolate(), nullptr);
 }
 
+namespace {
+std::string FromString(v8::Isolate* isolate, v8::Local<v8::String> str) {
+  v8::String::Utf8Value utf8(isolate, str);
+  return std::string(*utf8);
+}
+}  // namespace
+
 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::Local<v8::String> source = v8_str(
       "var X = class {\n"
-      "  #foo = 1;\n"
+      "  #field_number = 1;\n"
-      "  #bar = function() {};\n"
+      "  #field_function = function() {};\n"
       "}\n"
       "var x = new X()");
   CompileRun(source);
@@ -4727,49 +4733,55 @@ TEST(GetPrivateFields) {
       env->Global()
           ->Get(context, v8_str(env->GetIsolate(), "x"))
           .ToLocalChecked());
-  v8::Local<v8::Array> private_names =
+  std::vector<v8::Local<v8::Value>> names;
-      v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+  std::vector<v8::Local<v8::Value>> values;
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
 
-  for (int i = 0; i < 4; i = i + 2) {
+  CHECK_EQ(names.size(), 2);
-    Handle<v8::internal::JSReceiver> private_name =
+  for (int i = 0; i < 2; i++) {
-        v8::Utils::OpenHandle(*private_names->Get(context, i)
+    v8::Local<v8::Value> name = names[i];
-                                   .ToLocalChecked()
+    v8::Local<v8::Value> value = values[i];
-                                   ->ToObject(context)
+    CHECK(name->IsString());
-                                   .ToLocalChecked());
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
-    Handle<v8::internal::JSPrimitiveWrapper> private_value =
+    if (name_str == "#field_number") {
-        Handle<v8::internal::JSPrimitiveWrapper>::cast(private_name);
+      CHECK(value->Equals(context, v8_num(1)).FromJust());
-    Handle<v8::internal::Symbol> priv_symbol(
+    } else {
-        v8::internal::Symbol::cast(private_value->value()), isolate);
+      CHECK_EQ(name_str, "#field_function");
-    CHECK(priv_symbol->is_private_name());
+      CHECK(value->IsFunction());
+    }
   }
 
   source = v8_str(
       "var Y = class {\n"
-      "  #baz = 2;\n"
+      "  #base_field_number = 2;\n"
       "}\n"
       "var X = class extends Y{\n"
-      "  #foo = 1;\n"
+      "  #field_number = 1;\n"
-      "  #bar = function() {};\n"
+      "  #field_function = function() {};\n"
       "}\n"
       "var x = new X()");
   CompileRun(source);
+  names.clear();
+  values.clear();
   object = v8::Local<v8::Object>::Cast(
       env->Global()
           ->Get(context, v8_str(env->GetIsolate(), "x"))
           .ToLocalChecked());
-  private_names = v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
 
-  for (int i = 0; i < 6; i = i + 2) {
+  CHECK_EQ(names.size(), 3);
-    Handle<v8::internal::JSReceiver> private_name =
+  for (int i = 0; i < 3; i++) {
-        v8::Utils::OpenHandle(*private_names->Get(context, i)
+    v8::Local<v8::Value> name = names[i];
-                                   .ToLocalChecked()
+    v8::Local<v8::Value> value = values[i];
-                                   ->ToObject(context)
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
-                                   .ToLocalChecked());
+    if (name_str == "#base_field_number") {
-    Handle<v8::internal::JSPrimitiveWrapper> private_value =
+      CHECK(value->Equals(context, v8_num(2)).FromJust());
-        Handle<v8::internal::JSPrimitiveWrapper>::cast(private_name);
+    } else if (name_str == "#field_number") {
-    Handle<v8::internal::Symbol> priv_symbol(
+      CHECK(value->Equals(context, v8_num(1)).FromJust());
-        v8::internal::Symbol::cast(private_value->value()), isolate);
+    } else {
-    CHECK(priv_symbol->is_private_name());
+      CHECK_EQ(name_str, "#field_function");
+      CHECK(value->IsFunction());
+    }
   }
 
   source = v8_str(
@@ -4779,27 +4791,167 @@ TEST(GetPrivateFields) {
       "  }"
       "}\n"
       "var X = class extends Y{\n"
-      "  #foo = 1;\n"
+      "  #field_number = 1;\n"
-      "  #bar = function() {};\n"
+      "  #field_function = function() {};\n"
       "}\n"
       "var x = new X()");
   CompileRun(source);
+  names.clear();
+  values.clear();
   object = v8::Local<v8::Object>::Cast(
       env->Global()
           ->Get(context, v8_str(env->GetIsolate(), "x"))
           .ToLocalChecked());
-  private_names = v8::debug::GetPrivateFields(context, object).ToLocalChecked();
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
 
-  for (int i = 0; i < 4; i = i + 2) {
+  CHECK_EQ(names.size(), 2);
-    Handle<v8::internal::JSReceiver> private_name =
+  for (int i = 0; i < 2; i++) {
-        v8::Utils::OpenHandle(*private_names->Get(context, i)
+    v8::Local<v8::Value> name = names[i];
-                                   .ToLocalChecked()
+    v8::Local<v8::Value> value = values[i];
-                                   ->ToObject(context)
+    CHECK(name->IsString());
-                                   .ToLocalChecked());
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
-    Handle<v8::internal::JSPrimitiveWrapper> private_value =
+    if (name_str == "#field_number") {
-        Handle<v8::internal::JSPrimitiveWrapper>::cast(private_name);
+      CHECK(value->Equals(context, v8_num(1)).FromJust());
-    Handle<v8::internal::Symbol> priv_symbol(
+    } else {
-        v8::internal::Symbol::cast(private_value->value()), isolate);
+      CHECK_EQ(name_str, "#field_function");
-    CHECK(priv_symbol->is_private_name());
+      CHECK(value->IsFunction());
+    }
+  }
+}
+
+TEST(GetPrivateMethodsAndAccessors) {
+  i::FLAG_harmony_private_methods = true;
+  LocalContext env;
+  v8::Isolate* v8_isolate = CcTest::isolate();
+  v8::HandleScope scope(v8_isolate);
+  v8::Local<v8::Context> context = env.local();
+
+  v8::Local<v8::String> source = v8_str(
+      "var X = class {\n"
+      "  #method() { }\n"
+      "  get #accessor() { }\n"
+      "  set #accessor(val) { }\n"
+      "  get #readOnly() { }\n"
+      "  set #writeOnly(val) { }\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());
+  std::vector<v8::Local<v8::Value>> names;
+  std::vector<v8::Local<v8::Value>> values;
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
+
+  CHECK_EQ(names.size(), 4);
+  for (int i = 0; i < 4; i++) {
+    v8::Local<v8::Value> name = names[i];
+    v8::Local<v8::Value> value = values[i];
+    CHECK(name->IsString());
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
+    if (name_str == "#method") {
+      CHECK(value->IsFunction());
+    } else {
+      CHECK(v8::debug::AccessorPair::IsAccessorPair(value));
+      v8::Local<v8::debug::AccessorPair> accessors =
+          value.As<v8::debug::AccessorPair>();
+      if (name_str == "#accessor") {
+        CHECK(accessors->getter()->IsFunction());
+        CHECK(accessors->setter()->IsFunction());
+      } else if (name_str == "#readOnly") {
+        CHECK(accessors->getter()->IsFunction());
+        CHECK(accessors->setter()->IsNull());
+      } else {
+        CHECK_EQ(name_str, "#writeOnly");
+        CHECK(accessors->getter()->IsNull());
+        CHECK(accessors->setter()->IsFunction());
+      }
+    }
+  }
+
+  source = v8_str(
+      "var Y = class {\n"
+      "  #method() {}\n"
+      "  get #accessor() {}\n"
+      "  set #accessor(val) {};\n"
+      "}\n"
+      "var X = class extends Y{\n"
+      "  get #readOnly() {}\n"
+      "  set #writeOnly(val) {};\n"
+      "}\n"
+      "var x = new X()");
+  CompileRun(source);
+  names.clear();
+  values.clear();
+  object = v8::Local<v8::Object>::Cast(
+      env->Global()
+          ->Get(context, v8_str(env->GetIsolate(), "x"))
+          .ToLocalChecked());
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
+
+  CHECK_EQ(names.size(), 4);
+  for (int i = 0; i < 4; i++) {
+    v8::Local<v8::Value> name = names[i];
+    v8::Local<v8::Value> value = values[i];
+    CHECK(name->IsString());
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
+    if (name_str == "#method") {
+      CHECK(value->IsFunction());
+    } else {
+      CHECK(v8::debug::AccessorPair::IsAccessorPair(value));
+      v8::Local<v8::debug::AccessorPair> accessors =
+          value.As<v8::debug::AccessorPair>();
+      if (name_str == "#accessor") {
+        CHECK(accessors->getter()->IsFunction());
+        CHECK(accessors->setter()->IsFunction());
+      } else if (name_str == "#readOnly") {
+        CHECK(accessors->getter()->IsFunction());
+        CHECK(accessors->setter()->IsNull());
+      } else {
+        CHECK_EQ(name_str, "#writeOnly");
+        CHECK(accessors->getter()->IsNull());
+        CHECK(accessors->setter()->IsFunction());
+      }
+    }
+  }
+
+  source = v8_str(
+      "var Y = class {\n"
+      "  constructor() {"
+      "    return new Proxy({}, {});"
+      "  }"
+      "}\n"
+      "var X = class extends Y{\n"
+      "  #method() {}\n"
+      "  get #accessor() {}\n"
+      "  set #accessor(val) {};\n"
+      "}\n"
+      "var x = new X()");
+  CompileRun(source);
+  names.clear();
+  values.clear();
+  object = v8::Local<v8::Object>::Cast(
+      env->Global()
+          ->Get(context, v8_str(env->GetIsolate(), "x"))
+          .ToLocalChecked());
+  CHECK(v8::debug::GetPrivateMembers(context, object, &names, &values));
+
+  CHECK_EQ(names.size(), 2);
+  for (int i = 0; i < 2; i++) {
+    v8::Local<v8::Value> name = names[i];
+    v8::Local<v8::Value> value = values[i];
+    CHECK(name->IsString());
+    std::string name_str = FromString(v8_isolate, name.As<v8::String>());
+    if (name_str == "#method") {
+      CHECK(value->IsFunction());
+    } else {
+      CHECK_EQ(name_str, "#accessor");
+      CHECK(v8::debug::AccessorPair::IsAccessorPair(value));
+      v8::Local<v8::debug::AccessorPair> accessors =
+          value.As<v8::debug::AccessorPair>();
+      CHECK(accessors->getter()->IsFunction());
+      CHECK(accessors->setter()->IsFunction());
+    }
   }
 }

test/inspector/debugger/class-private-methods-expected.txt

@@ -1,8 +1,51 @@
 Test private class methods
 
 Running test: testScopesPaused
+privateProperties on the base class instance
 [
     [0] : {
+        name : #inc
+        value : {
+            className : Function
+            description : #inc() { this.#field++; return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [1] : {
+        name : #writeOnly
+        set : {
+            className : Function
+            description : set #writeOnly(val) { this.#field = val; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [2] : {
+        get : {
+            className : Function
+            description : get #readOnly() { return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+        name : #readOnly
+    }
+    [3] : {
+        get : {
+            className : Function
+            description : get #accessor() { return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+        name : #accessor
+        set : {
+            className : Function
+            description : set #accessor(val) { this.#field = val; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [4] : {
         name : #field
         value : {
             description : 2
@@ -11,6 +54,7 @@ Running test: testScopesPaused
         }
     }
 ]
+Evaluating private methods
 {
     result : {
         description : 3
@@ -18,3 +62,144 @@ Running test: testScopesPaused
         value : 3
     }
 }
+Evaluating private methods
+{
+    result : {
+        description : 4
+        type : number
+        value : 4
+    }
+}
+Evaluating private accessors
+{
+    result : {
+        description : 5
+        type : number
+        value : 5
+    }
+}
+Evaluating read-only accessor
+{
+    result : {
+        description : 5
+        type : number
+        value : 5
+    }
+}
+Evaluating write-only accessor
+{
+    result : {
+        description : 0
+        type : number
+        value : 0
+    }
+}
+privateProperties on the subclass instance
+[
+    [0] : {
+        name : #inc
+        value : {
+            className : Function
+            description : #inc() { this.#field++; return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [1] : {
+        name : #writeOnly
+        set : {
+            className : Function
+            description : set #writeOnly(val) { this.#field = val; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [2] : {
+        get : {
+            className : Function
+            description : get #readOnly() { return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+        name : #readOnly
+    }
+    [3] : {
+        get : {
+            className : Function
+            description : get #accessor() { return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+        name : #accessor
+        set : {
+            className : Function
+            description : set #accessor(val) { this.#field = val; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [4] : {
+        name : #field
+        value : {
+            description : 2
+            type : number
+            value : 2
+        }
+    }
+    [5] : {
+        name : #subclassMethod
+        value : {
+            className : Function
+            description : #subclassMethod() { return 'subclassMethod'; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [6] : {
+        name : #inc
+        value : {
+            className : Function
+            description : #inc() { return 'subclass #inc'; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+]
+Evaluating private methods in the base class from the subclass
+{
+    exceptionDetails : {
+        columnNumber : 4
+        exception : {
+            className : SyntaxError
+            description : SyntaxError: Private field '#subclassMethod' must be declared in an enclosing class     at B.fn (<anonymous>:16:7)     at run (<anonymous>:30:5)     at <anonymous>:1:1
+            objectId : <objectId>
+            subtype : error
+            type : object
+        }
+        exceptionId : <exceptionId>
+        lineNumber : 0
+        scriptId : <scriptId>
+        text : Uncaught
+    }
+    result : {
+        className : SyntaxError
+        description : SyntaxError: Private field '#subclassMethod' must be declared in an enclosing class     at B.fn (<anonymous>:16:7)     at run (<anonymous>:30:5)     at <anonymous>:1:1
+        objectId : <objectId>
+        subtype : error
+        type : object
+    }
+}
+Evaluating private method in the subclass from the subclass
+{
+    result : {
+        type : string
+        value : subclassMethod
+    }
+}
+Evaluating private method shadowing the base class method
+{
+    result : {
+        type : string
+        value : subclass #inc
+    }
+}

test/inspector/debugger/class-private-methods-preview-expected.txt

@@ -0,0 +1,30 @@
+Check private methods in object preview.
+
+Running test: testPrivateMethods
+expression: new class extends class { constructor() { return new Proxy({}, {}); } } { #method() { return 1; } }
+[
+]
+expression: new class { #method() { return 1; } get #accessor() { } set #accessor(val) { } }
+[
+    [0] : {
+        name : #method
+        type : function
+        value : 
+    }
+    [1] : {
+        name : #accessor
+        type : accessor
+    }
+]
+expression: new class extends class { #method() { return 1; } } { get #accessor() { } set #accessor(val) { } }
+[
+    [0] : {
+        name : #method
+        type : function
+        value : 
+    }
+    [1] : {
+        name : #accessor
+        type : accessor
+    }
+]

test/inspector/debugger/class-private-methods-preview.js

@@ -0,0 +1,38 @@
+// 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-private-methods
+
+let {session, contextGroup, Protocol} = InspectorTest.start("Check private methods in object preview.");
+
+Protocol.Debugger.enable();
+Protocol.Runtime.enable();
+Protocol.Runtime.onConsoleAPICalled(dumpInternalProperties);
+
+contextGroup.setupInjectedScriptEnvironment();
+
+InspectorTest.runAsyncTestSuite([
+  async function testPrivateMethods() {
+    const expressions = [
+        "new class extends class { constructor() { return new Proxy({}, {}); } } { #method() { return 1; } }",
+        "new class { #method() { return 1; } get #accessor() { } set #accessor(val) { } }",
+        "new class extends class { #method() { return 1; } } { get #accessor() { } set #accessor(val) { } }",
+    ];
+    for (const expression of expressions) {
+      InspectorTest.log(`expression: ${expression}`);
+      await Protocol.Runtime.evaluate({
+        expression: `console.table(${expression})`,
+        generatePreview: true
+      });
+    }
+  }
+]);
+
+function dumpInternalProperties(message) {
+  try {
+    InspectorTest.logMessage(message.params.args[0].preview.properties);
+  } catch {
+    InspectorTest.logMessage(message);
+  }
+}

test/inspector/debugger/class-private-methods.js

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 // Flags: --harmony-private-methods
-// TODO(v8:9839): return private methods and accessors
 
 let { session, contextGroup, Protocol } = InspectorTest.start(
   "Test private class methods"
@@ -13,15 +12,33 @@ contextGroup.addScript(`
 function run() {
   class A {
     #field = 2;
+
     #inc() { this.#field++; return this.#field; }
-    get #getter() { return this.#field; }
+
-    set #setter(val) { this.#field = val; }
+    set #writeOnly(val) { this.#field = val; }
-    fn () {
+
+    get #readOnly() { return this.#field; }
+
+    get #accessor() { return this.#field; }
+    set #accessor(val) { this.#field = val; }
+
+    fn() {
       debugger;
     }
   };
   const a = new A();
   a.fn();
+
+
+  class B extends A {
+    #subclassMethod() { return 'subclassMethod'; }
+    #inc() { return 'subclass #inc'; }
+    test() { debugger; }
+  }
+
+  const b = new B();
+  b.fn();
+  b.test();
 }`);
 
 InspectorTest.runAsyncTestSuite([
@@ -31,17 +48,91 @@ InspectorTest.runAsyncTestSuite([
 
     let {
       params: { callFrames }
-    } = await Protocol.Debugger.oncePaused(); // inside fn()
+    } = await Protocol.Debugger.oncePaused(); // inside a.fn()
-    const frame = callFrames[0];
+    let frame = callFrames[0];
-    let { result  } = await Protocol.Runtime.getProperties({
+    let { result } = await Protocol.Runtime.getProperties({
       objectId: frame.this.objectId
     });
+
+    InspectorTest.log('privateProperties on the base class instance');
     InspectorTest.logMessage(result.privateProperties);
-    let { result: result2 } = await Protocol.Debugger.evaluateOnCallFrame({
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
       expression: 'this.#inc();',
       callFrameId: callFrames[0].callFrameId
-    });
+    }));
-    InspectorTest.logObject(result2);
+
+    InspectorTest.log('Evaluating private methods');
+    InspectorTest.logObject(result);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#inc();',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating private methods');
+    InspectorTest.logObject(result);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: '++this.#accessor;',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating private accessors');
+    InspectorTest.logObject(result);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#readOnly;',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating read-only accessor');
+    InspectorTest.logObject(result);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#writeOnly = 0; this.#field;',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating write-only accessor');
+    InspectorTest.logObject(result);
+
+    Protocol.Debugger.resume();
+    ({ params: { callFrames }  } = await Protocol.Debugger.oncePaused());  // b.fn();
+    frame = callFrames[0];
+
+    ({ result } = await Protocol.Runtime.getProperties({
+      objectId: frame.this.objectId
+    }));
+    InspectorTest.log('privateProperties on the subclass instance');
+    InspectorTest.logMessage(result.privateProperties);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#subclassMethod();',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating private methods in the base class from the subclass');
+    InspectorTest.logMessage(result);
+
+    Protocol.Debugger.resume();
+    ({ params: { callFrames }  } = await Protocol.Debugger.oncePaused());  // b.test();
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#subclassMethod();',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating private method in the subclass from the subclass');
+    InspectorTest.logObject(result);
+
+    ({ result } = await Protocol.Debugger.evaluateOnCallFrame({
+      expression: 'this.#inc();',
+      callFrameId: callFrames[0].callFrameId
+    }));
+
+    InspectorTest.log('Evaluating private method shadowing the base class method');
+    InspectorTest.logObject(result);
+
     Protocol.Debugger.resume();
     Protocol.Debugger.disable();
   }