[inspector] Correctly report private accessor properties.

When calling `Runtime.getProperties` with `accessorPropertiesOnly` we
previously did not report any private fields at all, although it is
possible to define private accessors.

Bug: chromium:1296855
Change-Id: I18b84bfc81449d224738ba3de1f0c41c234025b2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3477112
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Kim-Anh Tran <kimanh@chromium.org>
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#79210}

src/inspector/injected-script.cc

@@ -440,6 +440,7 @@ Response InjectedScript::getProperties(
 
 Response InjectedScript::getInternalAndPrivateProperties(
     v8::Local<v8::Value> value, const String16& groupName,
+    bool accessorPropertiesOnly,
     std::unique_ptr<protocol::Array<InternalPropertyDescriptor>>*
         internalProperties,
     std::unique_ptr<protocol::Array<PrivatePropertyDescriptor>>*
@@ -453,27 +454,31 @@ Response InjectedScript::getInternalAndPrivateProperties(
 
   v8::Local<v8::Context> context = m_context->context();
   int sessionId = m_sessionId;
-  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)
-        ->emplace_back(InternalPropertyDescriptor::create()
-                           .setName(internalProperty.name)
-                           .setValue(std::move(remoteObject))
-                           .build());
+
+  if (!accessorPropertiesOnly) {
+    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)
+          ->emplace_back(InternalPropertyDescriptor::create()
+                             .setName(internalProperty.name)
+                             .setValue(std::move(remoteObject))
+                             .build());
+    }
   }
 
   std::vector<PrivatePropertyMirror> privatePropertyWrappers =
-      ValueMirror::getPrivateProperties(context, value_obj);
+      ValueMirror::getPrivateProperties(context, value_obj,
+                                        accessorPropertiesOnly);
   for (const auto& privateProperty : privatePropertyWrappers) {
     std::unique_ptr<PrivatePropertyDescriptor> descriptor =
         PrivatePropertyDescriptor::create()

src/inspector/injected-script.h

@@ -85,6 +85,7 @@ class InjectedScript final {
 
   Response getInternalAndPrivateProperties(
       v8::Local<v8::Value>, const String16& groupName,
+      bool accessorPropertiesOnly,
       std::unique_ptr<
           protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*
           internalProperties,

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

@@ -452,15 +452,14 @@ Response V8RuntimeAgentImpl::getProperties(
                                        : WrapMode::kNoPreview,
       result, exceptionDetails);
   if (!response.IsSuccess()) return response;
-  if (exceptionDetails->isJust() || accessorPropertiesOnly.fromMaybe(false))
-    return Response::Success();
+  if (exceptionDetails->isJust()) return Response::Success();
   std::unique_ptr<protocol::Array<InternalPropertyDescriptor>>
       internalPropertiesProtocolArray;
   std::unique_ptr<protocol::Array<PrivatePropertyDescriptor>>
       privatePropertiesProtocolArray;
   response = scope.injectedScript()->getInternalAndPrivateProperties(
-      object, scope.objectGroupName(), &internalPropertiesProtocolArray,
-      &privatePropertiesProtocolArray);
+      object, scope.objectGroupName(), accessorPropertiesOnly.fromMaybe(false),
+      &internalPropertiesProtocolArray, &privatePropertiesProtocolArray);
   if (!response.IsSuccess()) return response;
   if (!internalPropertiesProtocolArray->empty())
     *internalProperties = std::move(internalPropertiesProtocolArray);

src/inspector/value-mirror.cc

@@ -893,7 +893,8 @@ void getPrivatePropertiesForPreview(
     int* nameLimit, bool* overflow,
     protocol::Array<PropertyPreview>* privateProperties) {
   std::vector<PrivatePropertyMirror> mirrors =
-      ValueMirror::getPrivateProperties(context, object);
+      ValueMirror::getPrivateProperties(context, object,
+                                        /* accessPropertiesOnly */ false);
   for (auto& mirror : mirrors) {
     std::unique_ptr<PropertyPreview> propertyPreview;
     if (mirror.value) {
@@ -1429,7 +1430,8 @@ void ValueMirror::getInternalProperties(
 
 // static
 std::vector<PrivatePropertyMirror> ValueMirror::getPrivateProperties(
-    v8::Local<v8::Context> context, v8::Local<v8::Object> object) {
+    v8::Local<v8::Context> context, v8::Local<v8::Object> object,
+    bool accessorPropertiesOnly) {
   std::vector<PrivatePropertyMirror> mirrors;
   v8::Isolate* isolate = context->GetIsolate();
   v8::MicrotasksScope microtasksScope(isolate,
@@ -1462,6 +1464,8 @@ std::vector<PrivatePropertyMirror> ValueMirror::getPrivateProperties(
       if (!setter->IsNull()) {
         setterMirror = ValueMirror::create(context, setter);
       }
+    } else if (accessorPropertiesOnly) {
+      continue;
     } else {
       valueMirror = ValueMirror::create(context, value);
     }

src/inspector/value-mirror.h

@@ -81,7 +81,8 @@ class ValueMirror {
       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);
+      v8::Local<v8::Context> context, v8::Local<v8::Object> object,
+      bool accessorPropertiesOnly);
 };
 
 protocol::Response toProtocolValue(v8::Local<v8::Context> context,

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

 Test private class methods
 
 Running test: testScopesPaused
-privateProperties on the base class instance
+private properties on the base class instance
 [
     [0] : {
         name : #inc
@@ -54,6 +54,42 @@ privateProperties on the base class instance
         }
     }
 ]
+private accessors properties on the base class instance
+[
+    [0] : {
+        name : #writeOnly
+        set : {
+            className : Function
+            description : set #writeOnly(val) { this.#field = val; }
+            objectId : <objectId>
+            type : function
+        }
+    }
+    [1] : {
+        get : {
+            className : Function
+            description : get #readOnly() { return this.#field; }
+            objectId : <objectId>
+            type : function
+        }
+        name : #readOnly
+    }
+    [2] : {
+        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
+        }
+    }
+]
 Evaluating private methods
 {
     result : {

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

@@ -56,7 +56,15 @@ InspectorTest.runAsyncTestSuite([
       objectId: frame.this.objectId
     });
 
-    InspectorTest.log('privateProperties on the base class instance');
+    InspectorTest.log('private properties on the base class instance');
+    InspectorTest.logMessage(result.privateProperties);
+
+    ({ result } = await Protocol.Runtime.getProperties({
+      objectId: frame.this.objectId,
+      accessorPropertiesOnly: true,
+    }));
+
+    InspectorTest.log('private accessors properties on the base class instance');
     InspectorTest.logMessage(result.privateProperties);
 
     ({ result } = await Protocol.Debugger.evaluateOnCallFrame({