[inspector] Add nonIndexedPropertiesOnly to Runtime.getProperties.

This introduces a new, optional `nonIndexedPropertiesOnly` flag to the
`Runtime.getProperties` inspector request, which tells the inspector to
only report properties whose name is not an (typed) array index. This is
to support retrieving all properties except for the indexed ones when
the DevTools front-end decides to use the array bucketing mechanism.
Previously the DevTools front-end had some quite complicated logic in
place to simulate this via injected JavaScript, but that logic didn't
pick up internal properties and was also interfering with the inherited
accessor mechanism. With this new flag, it's straight-forward to
implement the correct behavior in the DevTools front-end.

The corresponding devtools-frontend CL is https://crrev.com/c/3099011.

Before: https://imgur.com/hMX6vaV.png
After: https://imgur.com/MGgiuJQ.png
Bug: chromium:1199701
Change-Id: Iacbe9756ed8a2e6982efaebe1e7c606d37c05379
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3099686
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76360}

include/js_protocol.pdl

@@ -1427,6 +1427,8 @@ domain Runtime
       experimental optional boolean accessorPropertiesOnly
       # Whether preview should be generated for the results.
       experimental optional boolean generatePreview
+      # If true, returns non-indexed properties only.
+      experimental optional boolean nonIndexedPropertiesOnly
     returns
       # Object properties.
       array of PropertyDescriptor result

src/inspector/injected-script.cc

@@ -354,8 +354,8 @@ class PropertyAccumulator : public ValueMirror::PropertyAccumulator {
 
 Response InjectedScript::getProperties(
     v8::Local<v8::Object> object, const String16& groupName, bool ownProperties,
-    bool accessorPropertiesOnly, WrapMode wrapMode,
-    std::unique_ptr<Array<PropertyDescriptor>>* properties,
+    bool accessorPropertiesOnly, bool nonIndexedPropertiesOnly,
+    WrapMode wrapMode, std::unique_ptr<Array<PropertyDescriptor>>* properties,
     Maybe<protocol::Runtime::ExceptionDetails>* exceptionDetails) {
   v8::HandleScope handles(m_context->isolate());
   v8::Local<v8::Context> context = m_context->context();
@@ -367,7 +367,8 @@ Response InjectedScript::getProperties(
   std::vector<PropertyMirror> mirrors;
   PropertyAccumulator accumulator(&mirrors);
   if (!ValueMirror::getProperties(context, object, ownProperties,
-                                  accessorPropertiesOnly, &accumulator)) {
+                                  accessorPropertiesOnly,
+                                  nonIndexedPropertiesOnly, &accumulator)) {
     return createExceptionDetails(tryCatch, groupName, exceptionDetails);
   }
   for (const PropertyMirror& mirror : mirrors) {

src/inspector/injected-script.h

@@ -76,7 +76,8 @@ class InjectedScript final {
 
   Response getProperties(
       v8::Local<v8::Object>, const String16& groupName, bool ownProperties,
-      bool accessorPropertiesOnly, WrapMode wrapMode,
+      bool accessorPropertiesOnly, bool nonIndexedPropertiesOnly,
+      WrapMode wrapMode,
       std::unique_ptr<protocol::Array<protocol::Runtime::PropertyDescriptor>>*
           result,
       Maybe<protocol::Runtime::ExceptionDetails>*);

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

@@ -418,6 +418,7 @@ void V8RuntimeAgentImpl::callFunctionOn(
 Response V8RuntimeAgentImpl::getProperties(
     const String16& objectId, Maybe<bool> ownProperties,
     Maybe<bool> accessorPropertiesOnly, Maybe<bool> generatePreview,
+    Maybe<bool> nonIndexedPropertiesOnly,
     std::unique_ptr<protocol::Array<protocol::Runtime::PropertyDescriptor>>*
         result,
     Maybe<protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*
@@ -442,6 +443,7 @@ Response V8RuntimeAgentImpl::getProperties(
   response = scope.injectedScript()->getProperties(
       object, scope.objectGroupName(), ownProperties.fromMaybe(false),
       accessorPropertiesOnly.fromMaybe(false),
+      nonIndexedPropertiesOnly.fromMaybe(false),
       generatePreview.fromMaybe(false) ? WrapMode::kWithPreview
                                        : WrapMode::kNoPreview,
       result, exceptionDetails);

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

@@ -88,6 +88,7 @@ class V8RuntimeAgentImpl : public protocol::Runtime::Backend {
   Response getProperties(
       const String16& objectId, Maybe<bool> ownProperties,
       Maybe<bool> accessorPropertiesOnly, Maybe<bool> generatePreview,
+      Maybe<bool> nonIndexedPropertiesOnly,
       std::unique_ptr<protocol::Array<protocol::Runtime::PropertyDescriptor>>*
           result,
       Maybe<protocol::Array<protocol::Runtime::InternalPropertyDescriptor>>*

src/inspector/value-mirror.cc

@@ -844,7 +844,7 @@ bool getPropertiesForPreview(v8::Local<v8::Context> context,
                       : -1;
   PreviewPropertyAccumulator accumulator(blocklist, skipIndex, nameLimit,
                                          indexLimit, overflow, properties);
-  return ValueMirror::getProperties(context, object, false, false,
+  return ValueMirror::getProperties(context, object, false, false, false,
                                     &accumulator);
 }
 
@@ -1178,6 +1178,7 @@ ValueMirror::~ValueMirror() = default;
 bool ValueMirror::getProperties(v8::Local<v8::Context> context,
                                 v8::Local<v8::Object> object,
                                 bool ownProperties, bool accessorPropertiesOnly,
+                                bool nonIndexedPropertiesOnly,
                                 PropertyAccumulator* accumulator) {
   v8::Isolate* isolate = context->GetIsolate();
   v8::TryCatch tryCatch(isolate);
@@ -1209,6 +1210,14 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
   while (!iterator->Done()) {
     bool isOwn = iterator->is_own();
     if (!isOwn && ownProperties) break;
+    bool isIndex = iterator->is_array_index();
+    if (isIndex && nonIndexedPropertiesOnly) {
+      if (!iterator->Advance().FromMaybe(false)) {
+        CHECK(tryCatch.HasCaught());
+        return false;
+      }
+      continue;
+    }
     v8::Local<v8::Name> v8Name = iterator->name();
     v8::Maybe<bool> result = set->Has(context, v8Name);
     if (result.IsNothing()) return false;
@@ -1301,7 +1310,7 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
                                  configurable,
                                  enumerable,
                                  isOwn,
-                                 iterator->is_array_index(),
+                                 isIndex,
                                  isAccessorProperty && valueMirror,
                                  std::move(valueMirror),
                                  std::move(getterMirror),

src/inspector/value-mirror.h

@@ -75,6 +75,7 @@ class ValueMirror {
   static bool getProperties(v8::Local<v8::Context> context,
                             v8::Local<v8::Object> object, bool ownProperties,
                             bool accessorPropertiesOnly,
+                            bool nonIndexedPropertiesOnly,
                             PropertyAccumulator* accumulator);
   static void getInternalProperties(
       v8::Local<v8::Context> context, v8::Local<v8::Object> object,

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

@@ -185,3 +185,12 @@ Running test: testObjectWithProtoProperty
   __proto__ own object undefined
 Internal properties
   [[Prototype]] object undefined
+
+Running test: testArrayNonIndexedPropertiesOnly
+  length own number 2
+Internal properties
+  [[Prototype]] object undefined
+
+Running test: testTypedArrayNonIndexedPropertiesOnly
+Internal properties
+  [[Prototype]] object undefined

test/inspector/runtime/get-properties.js

@@ -106,6 +106,14 @@ InspectorTest.runAsyncTestSuite([
 
   async function testObjectWithProtoProperty() {
     await logExpressionProperties('Object.defineProperty({}, "__proto__", {enumerable: true, value: {b:"aaa"}})');
+  },
+
+  function testArrayNonIndexedPropertiesOnly() {
+    return logExpressionProperties('[1, 2]', {nonIndexedPropertiesOnly: true, ownProperties: true});
+  },
+
+  function testTypedArrayNonIndexedPropertiesOnly() {
+    return logExpressionProperties('new Int8Array(1)', {nonIndexedPropertiesOnly: true, ownProperties: true});
   }
 ]);