[class] Expose private class fields in inspector protocol

This allows the devtools to preview the private fields that are
installed on an object.

Change-Id: I6d8aad7ad0e51cdf18f6139b4bb8665e4b606aa5
Bug: v8:8773, v8:8337
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1487914
Commit-Queue: Sathya Gunasekaran <gsathya@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60134}

src/inspector/injected-script.cc

@@ -57,10 +57,11 @@ static bool isResolvableNumberLike(String16 query) {
 }  // namespace
 
 using protocol::Array;
-using protocol::Runtime::PropertyDescriptor;
+using protocol::Maybe;
 using protocol::Runtime::InternalPropertyDescriptor;
+using protocol::Runtime::PrivatePropertyDescriptor;
+using protocol::Runtime::PropertyDescriptor;
 using protocol::Runtime::RemoteObject;
-using protocol::Maybe;
 
 class InjectedScript::ProtocolPromiseHandler {
  public:
@@ -355,30 +356,55 @@ Response InjectedScript::getProperties(
   return Response::OK();
 }
 
-Response InjectedScript::getInternalProperties(
+Response InjectedScript::getInternalAndPrivateProperties(
     v8::Local<v8::Value> value, const String16& groupName,
-    std::unique_ptr<protocol::Array<InternalPropertyDescriptor>>* result) {
-  *result = protocol::Array<InternalPropertyDescriptor>::create();
+    std::unique_ptr<protocol::Array<InternalPropertyDescriptor>>*
+        internalProperties,
+    std::unique_ptr<protocol::Array<PrivatePropertyDescriptor>>*
+        privateProperties) {
+  *internalProperties = protocol::Array<InternalPropertyDescriptor>::create();
+  *privateProperties = protocol::Array<PrivatePropertyDescriptor>::create();
+
+  if (!value->IsObject()) return Response::OK();
+
+  v8::Local<v8::Object> value_obj = value.As<v8::Object>();
+
   v8::Local<v8::Context> context = m_context->context();
   int sessionId = m_sessionId;
-  std::vector<InternalPropertyMirror> wrappers;
-  if (value->IsObject()) {
-    ValueMirror::getInternalProperties(m_context->context(),
-                                       value.As<v8::Object>(), &wrappers);
+  std::vector<InternalPropertyMirror> internalPropertiesWrappers;
+  ValueMirror::getInternalProperties(m_context->context(), value_obj,
+                                     &internalPropertiesWrappers);
+  for (const auto& internalProperty : internalPropertiesWrappers) {
+    std::unique_ptr<RemoteObject> remoteObject;
+    Response response = internalProperty.value->buildRemoteObject(
+        m_context->context(), WrapMode::kNoPreview, &remoteObject);
+    if (!response.isSuccess()) return response;
+    response = bindRemoteObjectIfNeeded(sessionId, context,
+                                        internalProperty.value->v8Value(),
+                                        groupName, remoteObject.get());
+    if (!response.isSuccess()) return response;
+    (*internalProperties)
+        ->addItem(InternalPropertyDescriptor::create()
+                      .setName(internalProperty.name)
+                      .setValue(std::move(remoteObject))
+                      .build());
   }
-  for (size_t i = 0; i < wrappers.size(); ++i) {
+  std::vector<PrivatePropertyMirror> privatePropertyWrappers =
+      ValueMirror::getPrivateProperties(m_context->context(), value_obj);
+  for (const auto& privateProperty : privatePropertyWrappers) {
     std::unique_ptr<RemoteObject> remoteObject;
-    Response response = wrappers[i].value->buildRemoteObject(
+    Response response = privateProperty.value->buildRemoteObject(
         m_context->context(), WrapMode::kNoPreview, &remoteObject);
     if (!response.isSuccess()) return response;
     response = bindRemoteObjectIfNeeded(sessionId, context,
-                                        wrappers[i].value->v8Value(), groupName,
-                                        remoteObject.get());
+                                        privateProperty.value->v8Value(),
+                                        groupName, remoteObject.get());
     if (!response.isSuccess()) return response;
-    (*result)->addItem(InternalPropertyDescriptor::create()
-                           .setName(wrappers[i].name)
-                           .setValue(std::move(remoteObject))
-                           .build());
+    (*privateProperties)
+        ->addItem(PrivatePropertyDescriptor::create()
+                      .setName(privateProperty.name)
+                      .setValue(std::move(remoteObject))
+                      .build());
   }
   return Response::OK();
 }

src/inspector/injected-script.h

@@ -78,11 +78,14 @@ class InjectedScript final {
           result,
       Maybe<protocol::Runtime::ExceptionDetails>*);
 
-  Response getInternalProperties(
+  Response getInternalAndPrivateProperties(
       v8::Local<v8::Value>, const String16& groupName,
       std::unique_ptr<
           protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*
-          result);
+          internalProperties,
+      std::unique_ptr<
+          protocol::Array<protocol::Runtime::PrivatePropertyDescriptor>>*
+          privateProperties);
 
   void releaseObject(const String16& objectId);
 

src/inspector/js_protocol.pdl

@@ -1021,6 +1021,14 @@ domain Runtime
       # The value associated with the property.
       optional RemoteObject value
 
+  # Object private field descriptor.
+  experimental type PrivatePropertyDescriptor extends object
+    properties
+      # Private property name.
+      string name
+      # The value associated with the private property.
+      RemoteObject value
+
   # Represents function call argument. Either remote object id `objectId`, primitive `value`,
   # unserializable primitive value or neither of (for undefined) them should be specified.
   type CallArgument extends object
@@ -1262,6 +1270,8 @@ domain Runtime
       array of PropertyDescriptor result
       # Internal object properties (only of the element itself).
       optional array of InternalPropertyDescriptor internalProperties
+      # Object private properties.
+      experimental optional array of PrivatePropertyDescriptor privateProperties
       # Exception details.
       optional ExceptionDetails exceptionDetails
 

src/inspector/v8-runtime-agent-impl.cc

@@ -385,8 +385,11 @@ Response V8RuntimeAgentImpl::getProperties(
         result,
     Maybe<protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*
         internalProperties,
+    Maybe<protocol::Array<protocol::Runtime::PrivatePropertyDescriptor>>*
+        privateProperties,
     Maybe<protocol::Runtime::ExceptionDetails>* exceptionDetails) {
   using protocol::Runtime::InternalPropertyDescriptor;
+  using protocol::Runtime::PrivatePropertyDescriptor;
 
   InjectedScript::ObjectScope scope(m_session, objectId);
   Response response = scope.initialize();
@@ -409,12 +412,17 @@ Response V8RuntimeAgentImpl::getProperties(
   if (exceptionDetails->isJust() || accessorPropertiesOnly.fromMaybe(false))
     return Response::OK();
   std::unique_ptr<protocol::Array<InternalPropertyDescriptor>>
-      propertiesProtocolArray;
-  response = scope.injectedScript()->getInternalProperties(
-      object, scope.objectGroupName(), &propertiesProtocolArray);
+      internalPropertiesProtocolArray;
+  std::unique_ptr<protocol::Array<PrivatePropertyDescriptor>>
+      privatePropertiesProtocolArray;
+  response = scope.injectedScript()->getInternalAndPrivateProperties(
+      object, scope.objectGroupName(), &internalPropertiesProtocolArray,
+      &privatePropertiesProtocolArray);
   if (!response.isSuccess()) return response;
-  if (propertiesProtocolArray->length())
-    *internalProperties = std::move(propertiesProtocolArray);
+  if (internalPropertiesProtocolArray->length())
+    *internalProperties = std::move(internalPropertiesProtocolArray);
+  if (privatePropertiesProtocolArray->length())
+    *privateProperties = std::move(privatePropertiesProtocolArray);
   return Response::OK();
 }
 

src/inspector/v8-runtime-agent-impl.h

@@ -87,6 +87,8 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
           result,
       Maybe<protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*
           internalProperties,
+      Maybe<protocol::Array<protocol::Runtime::PrivatePropertyDescriptor>>*
+          privateProperties,
       Maybe<protocol::Runtime::ExceptionDetails>*) override;
   Response releaseObjectGroup(const String16& objectGroup) override;
   Response runIfWaitingForDebugger() override;

src/inspector/value-mirror.cc

@@ -15,11 +15,10 @@
 namespace v8_inspector {
 
 using protocol::Response;
-using protocol::Runtime::RemoteObject;
+using protocol::Runtime::EntryPreview;
 using protocol::Runtime::ObjectPreview;
 using protocol::Runtime::PropertyPreview;
-using protocol::Runtime::EntryPreview;
-using protocol::Runtime::InternalPropertyDescriptor;
+using protocol::Runtime::RemoteObject;
 
 namespace {
 V8InspectorClient* clientFor(v8::Local<v8::Context> context) {
@@ -809,60 +808,23 @@ 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));
-      }
+void getPrivatePropertiesForPreview(
+    v8::Local<v8::Context> context, v8::Local<v8::Object> object,
+    int* nameLimit, bool* overflow,
+    protocol::Array<PropertyPreview>* privateProperties) {
+  std::vector<PrivatePropertyMirror> mirrors =
+      ValueMirror::getPrivateProperties(context, object);
+  std::vector<String16> whitelist;
+  for (auto& mirror : mirrors) {
+    std::unique_ptr<PropertyPreview> propertyPreview;
+    mirror.value->buildPropertyPreview(context, mirror.name, &propertyPreview);
+    if (!propertyPreview) continue;
+    if (!*nameLimit) {
+      *overflow = true;
+      return;
     }
+    --*nameLimit;
+    privateProperties->addItem(std::move(propertyPreview));
   }
 }
 
@@ -971,8 +933,8 @@ class ObjectMirror final : public ValueMirror {
         }
       }
 
-      getPrivateFieldsForPreview(context, objectForPreview, nameLimit,
-                                 &overflow, properties.get());
+      getPrivatePropertiesForPreview(context, objectForPreview, nameLimit,
+                                     &overflow, properties.get());
 
       std::vector<PropertyMirror> mirrors;
       if (getPropertiesForPreview(context, objectForPreview, nameLimit,
@@ -1411,6 +1373,53 @@ void ValueMirror::getInternalProperties(
   }
 }
 
+// static
+std::vector<PrivatePropertyMirror> ValueMirror::getPrivateProperties(
+    v8::Local<v8::Context> context, v8::Local<v8::Object> object) {
+  std::vector<PrivatePropertyMirror> mirrors;
+  v8::Isolate* isolate = context->GetIsolate();
+  v8::MicrotasksScope microtasksScope(isolate,
+                                      v8::MicrotasksScope::kDoNotRunMicrotasks);
+  v8::TryCatch tryCatch(isolate);
+  v8::Local<v8::Array> privateProperties;
+
+  if (!v8::debug::GetPrivateFields(context, object).ToLocal(&privateProperties))
+    return mirrors;
+
+  for (uint32_t i = 0; i < privateProperties->Length(); i += 2) {
+    v8::Local<v8::Value> name;
+    if (!privateProperties->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();
+    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;
+}
+
 String16 descriptionForNode(v8::Local<v8::Context> context,
                             v8::Local<v8::Value> value) {
   if (!value->IsObject()) return String16();

src/inspector/value-mirror.h

@@ -20,6 +20,11 @@ namespace v8_inspector {
 class ValueMirror;
 enum class WrapMode;
 
+struct PrivatePropertyMirror {
+  String16 name;
+  std::unique_ptr<ValueMirror> value;
+};
+
 struct InternalPropertyMirror {
   String16 name;
   std::unique_ptr<ValueMirror> value;
@@ -72,6 +77,8 @@ class ValueMirror {
   static void getInternalProperties(
       v8::Local<v8::Context> context, v8::Local<v8::Object> object,
       std::vector<InternalPropertyMirror>* mirrors);
+  static std::vector<PrivatePropertyMirror> getPrivateProperties(
+      v8::Local<v8::Context> context, v8::Local<v8::Object> object);
 };
 }  // namespace v8_inspector
 

test/inspector/runtime/get-properties-expected.txt

@@ -51,6 +51,26 @@ Running test: testObjectThrowsLength
 Running test: testTypedArrayWithoutLength
   __proto__ own object undefined
 
+Running test: testClassWithPrivateFields
+  __proto__ own object undefined
+  baz own number 4
+Private properties
+  #bar number 3
+  #foo number 2
+  __proto__ own object undefined
+  baz own number 4
+Private properties
+  #bar number 3
+  #baz number 1
+  #foo number 2
+Internal properties
+  [[Handler]] object undefined
+  [[IsRevoked]] boolean false
+  [[Target]] object undefined
+Private properties
+  #bar number 3
+  #foo number 2
+
 Running test: testArrayBuffer
 [[Int8Array]]
   0 own number 1

test/inspector/runtime/get-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-private-fields
 
 let {session, contextGroup, Protocol} = InspectorTest.start('Checks Runtime.getProperties method');
 
@@ -33,6 +35,12 @@ InspectorTest.runAsyncTestSuite([
     return logExpressionProperties('({__proto__: Uint8Array.prototype})');
   },
 
+  function testClassWithPrivateFields() {
+    return logExpressionProperties('new class { #foo = 2; #bar = 3; baz = 4; }')
+      .then(() => logExpressionProperties('new class extends class { #baz = 1 } { #foo = 2; #bar = 3; baz = 4; }'))
+      .then(() => logExpressionProperties('new class extends class { #hidden = 1; constructor() { return new Proxy({}, {}); } } { #foo = 2; #bar = 3; baz = 4; }'));
+  },
+
   async function testArrayBuffer() {
     let objectId = await evaluateToObjectId('new Uint8Array([1, 1, 1, 1, 1, 1, 1, 1]).buffer');
     let props = await Protocol.Runtime.getProperties({ objectId, ownProperties: true });
@@ -87,17 +95,21 @@ async function logGetPropertiesResult(objectId, flags = { ownProperties: true })
       InspectorTest.log("  " + p.name + " " + own + " no value" +
         (hasGetterSetter(p, "get") ? ", getter" : "") + (hasGetterSetter(p, "set") ? ", setter" : ""));
   }
-  var internalPropertyArray = props.result.internalProperties;
-  if (internalPropertyArray) {
-    InspectorTest.log("Internal properties");
-    internalPropertyArray.sort(NamedThingComparator);
-    for (var i = 0; i < internalPropertyArray.length; i++) {
-      var p = internalPropertyArray[i];
+
+  function printFields(type, array) {
+    if (!array) { return; }
+    InspectorTest.log(type);
+    array.sort(NamedThingComparator);
+    for (var i = 0; i < array.length; i++) {
+      var p = array[i];
       var v = p.value;
       InspectorTest.log('  ' + p.name + ' ' + v.type + ' ' + v.value);
     }
   }
 
+  printFields("Internal properties", props.result.internalProperties);
+  printFields("Private properties", props.result.privateProperties);
+
   function NamedThingComparator(o1, o2) {
     return o1.name === o2.name ? 0 : (o1.name < o2.name ? -1 : 1);
   }