[inspector] Report [[Prototype]] as internal property.

Previously the inspector was trying to add a special `__proto__`
property to every JSObject, which looked and behaved like a real
data property on the object. But this is confusing to developers
since `__proto__` is not a real data property, but usually an
accessor property on the `Object.prototype`.

Additionally all other internal properties are reported using the
[[Name]] notation, with the [[Prototype]] having been the strange
outlier.

Drive-by-cleanup: Use an ArrayList to collect the name/value pairs
inside Runtime::GetInternalProperties(), which makes this function
more readable and easier to add things.

Bug: chromuium:1162229
Fixed: chromium:1197019
Screenshot: https://imgur.com/a/b7TZ32s.png
Change-Id: Ic4c1e35e2e65f90619fcc12bf3a72806cadb0794
Doc: http://doc/1Xetnc9s6r0yy4LnPbqeCwsnsOtBlvJsV4OCdXMZ1wCM
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2814565
Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#73881}

src/debug/debug-wasm-objects.cc

@@ -1050,78 +1050,57 @@ std::unique_ptr<debug::ScopeIterator> GetWasmScopeIterator(WasmFrame* frame) {
   return std::make_unique<DebugWasmScopeIterator>(frame);
 }
 
-Handle<JSArray> GetWasmInstanceObjectInternalProperties(
+Handle<ArrayList> AddWasmInstanceObjectInternalProperties(
+    Isolate* isolate, Handle<ArrayList> result,
     Handle<WasmInstanceObject> instance) {
-  Isolate* isolate = instance->GetIsolate();
-  Handle<FixedArray> result = isolate->factory()->NewFixedArray(2 * 5);
-  int length = 0;
-
-  Handle<String> module_str =
-      isolate->factory()->NewStringFromAsciiChecked("[[Module]]");
-  Handle<Object> module_obj = handle(instance->module_object(), isolate);
-  result->set(length++, *module_str);
-  result->set(length++, *module_obj);
+  result = ArrayList::Add(
+      isolate, result,
+      isolate->factory()->NewStringFromAsciiChecked("[[Module]]"),
+      handle(instance->module_object(), isolate));
 
   if (FunctionsProxy::Count(isolate, instance) != 0) {
-    Handle<String> functions_str =
-        isolate->factory()->NewStringFromAsciiChecked("[[Functions]]");
-    Handle<Object> functions_obj =
-        GetOrCreateInstanceProxy<FunctionsProxy>(isolate, instance);
-    result->set(length++, *functions_str);
-    result->set(length++, *functions_obj);
+    result = ArrayList::Add(
+        isolate, result,
+        isolate->factory()->NewStringFromAsciiChecked("[[Functions]]"),
+        GetOrCreateInstanceProxy<FunctionsProxy>(isolate, instance));
   }
 
   if (GlobalsProxy::Count(isolate, instance) != 0) {
-    Handle<String> globals_str =
-        isolate->factory()->NewStringFromAsciiChecked("[[Globals]]");
-    Handle<Object> globals_obj =
-        GetOrCreateInstanceProxy<GlobalsProxy>(isolate, instance);
-    result->set(length++, *globals_str);
-    result->set(length++, *globals_obj);
+    result = ArrayList::Add(
+        isolate, result,
+        isolate->factory()->NewStringFromAsciiChecked("[[Globals]]"),
+        GetOrCreateInstanceProxy<GlobalsProxy>(isolate, instance));
   }
 
   if (MemoriesProxy::Count(isolate, instance) != 0) {
-    Handle<String> memories_str =
-        isolate->factory()->NewStringFromAsciiChecked("[[Memories]]");
-    Handle<Object> memories_obj =
-        GetOrCreateInstanceProxy<MemoriesProxy>(isolate, instance);
-    result->set(length++, *memories_str);
-    result->set(length++, *memories_obj);
+    result = ArrayList::Add(
+        isolate, result,
+        isolate->factory()->NewStringFromAsciiChecked("[[Memories]]"),
+        GetOrCreateInstanceProxy<MemoriesProxy>(isolate, instance));
   }
 
   if (TablesProxy::Count(isolate, instance) != 0) {
-    Handle<String> tables_str =
-        isolate->factory()->NewStringFromAsciiChecked("[[Tables]]");
-    Handle<Object> tables_obj =
-        GetOrCreateInstanceProxy<TablesProxy>(isolate, instance);
-    result->set(length++, *tables_str);
-    result->set(length++, *tables_obj);
+    result = ArrayList::Add(
+        isolate, result,
+        isolate->factory()->NewStringFromAsciiChecked("[[Tables]]"),
+        GetOrCreateInstanceProxy<TablesProxy>(isolate, instance));
   }
 
-  return isolate->factory()->NewJSArrayWithElements(result, PACKED_ELEMENTS,
-                                                    length);
+  return result;
 }
 
-Handle<JSArray> GetWasmModuleObjectInternalProperties(
+Handle<ArrayList> AddWasmModuleObjectInternalProperties(
+    Isolate* isolate, Handle<ArrayList> result,
     Handle<WasmModuleObject> module_object) {
-  Isolate* isolate = module_object->GetIsolate();
-  Handle<FixedArray> result = isolate->factory()->NewFixedArray(2 * 2);
-  int length = 0;
-
-  Handle<String> exports_str =
-      isolate->factory()->NewStringFromStaticChars("[[Exports]]");
-  Handle<JSArray> exports_obj = wasm::GetExports(isolate, module_object);
-  result->set(length++, *exports_str);
-  result->set(length++, *exports_obj);
-
-  Handle<String> imports_str =
-      isolate->factory()->NewStringFromStaticChars("[[Imports]]");
-  Handle<JSArray> imports_obj = wasm::GetImports(isolate, module_object);
-  result->set(length++, *imports_str);
-  result->set(length++, *imports_obj);
-
-  return isolate->factory()->NewJSArrayWithElements(result, PACKED_ELEMENTS,
-                                                    length);
+  result = ArrayList::Add(
+      isolate, result,
+      isolate->factory()->NewStringFromStaticChars("[[Exports]]"),
+      wasm::GetExports(isolate, module_object));
+  result = ArrayList::Add(
+      isolate, result,
+      isolate->factory()->NewStringFromStaticChars("[[Imports]]"),
+      wasm::GetImports(isolate, module_object));
+  return result;
 }
 
 }  // namespace internal

src/debug/debug-wasm-objects.h

@@ -28,6 +28,7 @@ class WasmValue;
 
 #include "torque-generated/src/debug/debug-wasm-objects-tq.inc"
 
+class ArrayList;
 class WasmFrame;
 class WasmInstanceObject;
 class WasmModuleObject;
@@ -68,9 +69,11 @@ Handle<JSObject> GetWasmDebugProxy(WasmFrame* frame);
 
 std::unique_ptr<debug::ScopeIterator> GetWasmScopeIterator(WasmFrame* frame);
 
-Handle<JSArray> GetWasmInstanceObjectInternalProperties(
+Handle<ArrayList> AddWasmInstanceObjectInternalProperties(
+    Isolate* isolate, Handle<ArrayList> result,
     Handle<WasmInstanceObject> instance);
-Handle<JSArray> GetWasmModuleObjectInternalProperties(
+Handle<ArrayList> AddWasmModuleObjectInternalProperties(
+    Isolate* isolate, Handle<ArrayList> result,
     Handle<WasmModuleObject> module_object);
 
 }  // namespace internal

src/inspector/value-mirror.cc

@@ -1216,7 +1216,6 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
       return false;
     }
   }
-  bool shouldSkipProto = internalType == V8InternalValueType::kScopeList;
 
   bool formatAccessorsAsProperties =
       clientFor(context)->formatAccessorsAsProperties(object);
@@ -1304,8 +1303,7 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
           bool isSymbolDescription =
               object->IsSymbol() && name == "description";
           if (isSymbolDescription ||
-              (name != "__proto__" && getterIsNativeFunction &&
-               formatAccessorsAsProperties &&
+              (getterIsNativeFunction && formatAccessorsAsProperties &&
                !doesAttributeHaveObservableSideEffectOnGet(context, object,
                                                            v8Name))) {
             v8::TryCatch tryCatch(isolate);
@@ -1321,7 +1319,6 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
       }
     }
     if (accessorPropertiesOnly && !isAccessorProperty) continue;
-    if (name == "__proto__") shouldSkipProto = true;
     auto mirror = PropertyMirror{name,
                                  writable,
                                  configurable,
@@ -1340,16 +1337,6 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
       return false;
     }
   }
-  if (!shouldSkipProto && ownProperties && !object->IsProxy() &&
-      !accessorPropertiesOnly) {
-    v8::Local<v8::Value> prototype = object->GetPrototype();
-    if (prototype->IsObject()) {
-      accumulator->Add(PropertyMirror{String16("__proto__"), true, true, false,
-                                      true, false,
-                                      ValueMirror::create(context, prototype),
-                                      nullptr, nullptr, nullptr, nullptr});
-    }
-  }
   return true;
 }
 

test/debugger/test-api.js

@@ -463,7 +463,12 @@ class DebugWrapper {
         "Runtime.getProperties", { objectId : objectId, ownProperties: true });
     this.sendMessage(msg);
     const reply = this.takeReplyChecked(msgid);
-    return Object(reply.result.internalProperties[0].value.value);
+    for (const internalProperty of reply.result.internalProperties) {
+      if (internalProperty.name === '[[PrimitiveValue]]') {
+        return Object(internalProperty.value.value);
+      }
+    }
+    throw new Error('Remote object is not a value wrapper');
   }
 
   reconstructRemoteObject(obj) {

test/inspector/debugger/get-properties-paused-expected.txt

 Checks Runtime.getProperties method while debugger is paused.
 
 Running test: testObject5
-  __proto__ own object undefined
   foo own string cat
 Internal properties
   [[PrimitiveValue]] number 5
+  [[Prototype]] object undefined
 
 Running test: testNotOwn
   __defineGetter__ inherited function undefined
@@ -23,6 +23,8 @@ Running test: testNotOwn
   toLocaleString inherited function undefined
   toString inherited function undefined
   valueOf inherited function undefined
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testAccessorsOnly
   b own no value, getter, setter
@@ -32,31 +34,34 @@ Running test: testArray
   0 own string red
   1 own string green
   2 own string blue
-  __proto__ own object undefined
   length own number 3
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testBound
-  __proto__ own function undefined
   length own number 0
   name own string bound Number
 Internal properties
   [[BoundArgs]] object undefined
   [[BoundThis]] object undefined
+  [[Prototype]] function undefined
   [[TargetFunction]] function undefined
 
 Running test: testObjectThrowsLength
-  __proto__ own object undefined
   length own no value, getter
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testTypedArrayWithoutLength
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testArrayBuffer
 
 Running test: testArrayBufferWithBrokenUintCtor
-  __proto__ own object undefined
 Internal properties
   [[ArrayBufferByteLength]] number 7
   [[ArrayBufferData]] string 0x...
   [[Int8Array]] object undefined
+  [[Prototype]] object undefined
   [[Uint8Array]] object undefined

test/inspector/debugger/get-properties-paused.js

@@ -42,8 +42,6 @@ let { Protocol } = InspectorTest.start('Checks Runtime.getProperties method whil
       let objectId = await evaluateToObjectId('new Uint8Array([1, 1, 1, 1, 1, 1, 1, 1]).buffer');
       let props = await Protocol.Runtime.getProperties({ objectId, ownProperties: true });
       for (let prop of props.result.result) {
-        if (prop.name === '__proto__')
-          continue;
         InspectorTest.log(prop.name);
         await logGetPropertiesResult(prop.value.objectId);
       }

test/inspector/debugger/set-variable-value-expected.txt

@@ -54,6 +54,17 @@ Running test: testSetVariableValueMain
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Object
+                    description : Object
+                    objectId : <objectId>
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -67,19 +78,6 @@ Running test: testSetVariableValueMain
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Object
-                    description : Object
-                    objectId : <objectId>
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }
@@ -98,6 +96,18 @@ Running test: testSetVariableValueMain
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Array
+                    description : Array(0)
+                    objectId : <objectId>
+                    subtype : array
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -144,20 +154,6 @@ Running test: testSetVariableValueMain
                 }
                 writable : true
             }
-            [4] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Array
-                    description : Array(0)
-                    objectId : <objectId>
-                    subtype : array
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }
@@ -175,6 +171,17 @@ Running test: testSetVariableValueMain
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Object
+                    description : Object
+                    objectId : <objectId>
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -188,19 +195,6 @@ Running test: testSetVariableValueMain
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Object
-                    description : Object
-                    objectId : <objectId>
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }

test/inspector/runtime/evaluate-with-generate-preview-expected.txt

@@ -824,6 +824,11 @@ Running test: testObjInheritsGetterProperty
             type : number
             value : NaN
         }
+        [1] : {
+            name : __proto__
+            type : object
+            value : Object
+        }
     ]
     type : object
 }

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

 Checks Runtime.getProperties method
 
 Running test: testObject5
-  __proto__ own object undefined
   foo own string cat
 Internal properties
   [[PrimitiveValue]] number 5
+  [[Prototype]] object undefined
 
 Running test: testNotOwn
   __defineGetter__ inherited function undefined
@@ -23,6 +23,8 @@ Running test: testNotOwn
   toLocaleString inherited function undefined
   toString inherited function undefined
   valueOf inherited function undefined
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testAccessorsOnly
   b own no value, getter, setter
@@ -32,33 +34,38 @@ Running test: testArray
   0 own string red
   1 own string green
   2 own string blue
-  __proto__ own object undefined
   length own number 3
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testBound
-  __proto__ own function undefined
   length own number 0
   name own string bound Number
 Internal properties
   [[BoundArgs]] object undefined
   [[BoundThis]] object undefined
+  [[Prototype]] function undefined
   [[TargetFunction]] function undefined
 
 Running test: testObjectThrowsLength
-  __proto__ own object undefined
   length own no value, getter
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testTypedArrayWithoutLength
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testClassWithPrivateFields
-  __proto__ own object undefined
   baz own number 4
+Internal properties
+  [[Prototype]] object undefined
 Private properties
   #bar number 3
   #foo number 2
-  __proto__ own object undefined
   baz own number 4
+Internal properties
+  [[Prototype]] object undefined
 Private properties
   #bar number 3
   #baz number 1
@@ -72,6 +79,13 @@ Private properties
   #foo number 2
 
 Running test: testArrayBuffer
+[[Prototype]]
+  Symbol(Symbol.toStringTag) own string ArrayBuffer
+  byteLength own no value, getter
+  constructor own function undefined
+  slice own function undefined
+Internal properties
+  [[Prototype]] object undefined
 [[Int8Array]]
   0 own number 1
   1 own number 1
@@ -81,7 +95,8 @@ Running test: testArrayBuffer
   5 own number 1
   6 own number 1
   7 own number 1
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 [[Uint8Array]]
   0 own number 1
   1 own number 1
@@ -91,21 +106,31 @@ Running test: testArrayBuffer
   5 own number 1
   6 own number 1
   7 own number 1
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 [[Int16Array]]
   0 own number 257
   1 own number 257
   2 own number 257
   3 own number 257
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 [[Int32Array]]
   0 own number 16843009
   1 own number 16843009
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 [[ArrayBufferByteLength]]
 [[ArrayBufferData]]
 
 Running test: testArrayBufferFromWebAssemblyMemory
+[[Prototype]]
+  Symbol(Symbol.toStringTag) own string ArrayBuffer
+  byteLength own no value, getter
+  constructor own function undefined
+  slice own function undefined
+Internal properties
+  [[Prototype]] object undefined
 [[Int8Array]]
 [[Uint8Array]]
 [[Int16Array]]
@@ -113,18 +138,22 @@ Running test: testArrayBufferFromWebAssemblyMemory
 [[ArrayBufferByteLength]]
 [[ArrayBufferData]]
 [[WebAssemblyMemory]]
-  __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined
 
 Running test: testDetachedArrayBuffer
+[[Prototype]] undefined
 [[IsDetached]] true
 
 Running test: testArrayBufferWithBrokenUintCtor
-  __proto__ own object undefined
 Internal properties
   [[ArrayBufferByteLength]] number 7
   [[ArrayBufferData]] string 0x...
   [[Int8Array]] object undefined
+  [[Prototype]] object undefined
   [[Uint8Array]] object undefined
 
 Running test: testObjectWithProtoProperty
   __proto__ own object undefined
+Internal properties
+  [[Prototype]] object undefined

test/inspector/runtime/get-properties.js

@@ -45,8 +45,6 @@ InspectorTest.runAsyncTestSuite([
     let objectId = await evaluateToObjectId('new Uint8Array([1, 1, 1, 1, 1, 1, 1, 1]).buffer');
     let props = await Protocol.Runtime.getProperties({ objectId, ownProperties: true });
     for (let prop of props.result.result) {
-      if (prop.name === '__proto__')
-        continue;
       InspectorTest.log(prop.name);
       await logGetPropertiesResult(prop.value.objectId);
     }
@@ -61,8 +59,6 @@ InspectorTest.runAsyncTestSuite([
     let objectId = await evaluateToObjectId('new WebAssembly.Memory({initial: 1}).buffer');
     let props = await Protocol.Runtime.getProperties({ objectId, ownProperties: true });
     for (let prop of props.result.result) {
-      if (prop.name === '__proto__')
-        continue;
       InspectorTest.log(prop.name);
       await logGetPropertiesResult(prop.value.objectId);
     }
@@ -84,8 +80,6 @@ InspectorTest.runAsyncTestSuite([
     await Protocol.Runtime.evaluate({ expression: 'b', generatePreview: true })
     let props = await Protocol.Runtime.getProperties({ objectId, ownProperties: true });
     for (let prop of props.result.result) {
-      if (prop.name === '__proto__')
-        continue;
       InspectorTest.log(prop.name);
       await logGetPropertiesResult(prop.value.objectId);
     }

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

@@ -4,6 +4,17 @@ Running test: testObject
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Object
+                    description : Object
+                    objectId : <objectId>
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -17,25 +28,23 @@ Running test: testObject
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
+        ]
+    }
+}
+{
+    id : <messageId>
+    result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
                 value : {
                     className : Object
                     description : Object
                     objectId : <objectId>
                     type : object
                 }
-                writable : true
             }
         ]
-    }
-}
-{
-    id : <messageId>
-    result : {
         result : [
             [0] : {
                 configurable : false
@@ -120,19 +129,6 @@ Running test: testObject
                 }
                 writable : false
             }
-            [6] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Object
-                    description : Object
-                    objectId : <objectId>
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }

test/inspector/sessions/runtime-remote-object-expected.txt

@@ -5,6 +5,17 @@ Retrieving properties in 2
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Object
+                    description : Object
+                    objectId : <objectId>
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -18,26 +29,24 @@ Retrieving properties in 2
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
+        ]
+    }
+}
+Retrieving properties in 1
+{
+    id : <messageId>
+    result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
                 value : {
                     className : Object
                     description : Object
                     objectId : <objectId>
                     type : object
                 }
-                writable : true
             }
         ]
-    }
-}
-Retrieving properties in 1
-{
-    id : <messageId>
-    result : {
         result : [
             [0] : {
                 configurable : true
@@ -51,19 +60,6 @@ Retrieving properties in 1
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Object
-                    description : Object
-                    objectId : <objectId>
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }
@@ -72,6 +68,17 @@ Retrieving properties in 1
 {
     id : <messageId>
     result : {
+        internalProperties : [
+            [0] : {
+                name : [[Prototype]]
+                value : {
+                    className : Object
+                    description : Object
+                    objectId : <objectId>
+                    type : object
+                }
+            }
+        ]
         result : [
             [0] : {
                 configurable : true
@@ -85,19 +92,6 @@ Retrieving properties in 1
                 }
                 writable : true
             }
-            [1] : {
-                configurable : true
-                enumerable : false
-                isOwn : true
-                name : __proto__
-                value : {
-                    className : Object
-                    description : Object
-                    objectId : <objectId>
-                    type : object
-                }
-                writable : true
-            }
         ]
     }
 }