Commit 0e2d3413 authored by Michael Achenbach's avatar Michael Achenbach Committed by Commit Bot

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

This reverts commit 2b94e567.

Reason for revert: Speculative based on layout test failures on
win and mac which could block the roll:
https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Win/5294
https://ci.chromium.org/p/v8/builders/ci/V8%20Blink%20Mac/4955

Original change's description:
> [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}

Bug: chromuium:1162229
Change-Id: Ia893ad672eb370fa6fce7eddf2947bf8f6755831
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2818386
Auto-Submit: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#73886}
parent a958fd78
......@@ -1050,57 +1050,78 @@ std::unique_ptr<debug::ScopeIterator> GetWasmScopeIterator(WasmFrame* frame) {
return std::make_unique<DebugWasmScopeIterator>(frame);
}
Handle<ArrayList> AddWasmInstanceObjectInternalProperties(
Isolate* isolate, Handle<ArrayList> result,
Handle<JSArray> GetWasmInstanceObjectInternalProperties(
Handle<WasmInstanceObject> instance) {
result = ArrayList::Add(
isolate, result,
isolate->factory()->NewStringFromAsciiChecked("[[Module]]"),
handle(instance->module_object(), isolate));
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);
if (FunctionsProxy::Count(isolate, instance) != 0) {
result = ArrayList::Add(
isolate, result,
isolate->factory()->NewStringFromAsciiChecked("[[Functions]]"),
GetOrCreateInstanceProxy<FunctionsProxy>(isolate, instance));
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);
}
if (GlobalsProxy::Count(isolate, instance) != 0) {
result = ArrayList::Add(
isolate, result,
isolate->factory()->NewStringFromAsciiChecked("[[Globals]]"),
GetOrCreateInstanceProxy<GlobalsProxy>(isolate, instance));
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);
}
if (MemoriesProxy::Count(isolate, instance) != 0) {
result = ArrayList::Add(
isolate, result,
isolate->factory()->NewStringFromAsciiChecked("[[Memories]]"),
GetOrCreateInstanceProxy<MemoriesProxy>(isolate, instance));
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);
}
if (TablesProxy::Count(isolate, instance) != 0) {
result = ArrayList::Add(
isolate, result,
isolate->factory()->NewStringFromAsciiChecked("[[Tables]]"),
GetOrCreateInstanceProxy<TablesProxy>(isolate, instance));
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);
}
return result;
return isolate->factory()->NewJSArrayWithElements(result, PACKED_ELEMENTS,
length);
}
Handle<ArrayList> AddWasmModuleObjectInternalProperties(
Isolate* isolate, Handle<ArrayList> result,
Handle<JSArray> GetWasmModuleObjectInternalProperties(
Handle<WasmModuleObject> module_object) {
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;
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);
}
} // namespace internal
......
......@@ -28,7 +28,6 @@ class WasmValue;
#include "torque-generated/src/debug/debug-wasm-objects-tq.inc"
class ArrayList;
class WasmFrame;
class WasmInstanceObject;
class WasmModuleObject;
......@@ -69,11 +68,9 @@ Handle<JSObject> GetWasmDebugProxy(WasmFrame* frame);
std::unique_ptr<debug::ScopeIterator> GetWasmScopeIterator(WasmFrame* frame);
Handle<ArrayList> AddWasmInstanceObjectInternalProperties(
Isolate* isolate, Handle<ArrayList> result,
Handle<JSArray> GetWasmInstanceObjectInternalProperties(
Handle<WasmInstanceObject> instance);
Handle<ArrayList> AddWasmModuleObjectInternalProperties(
Isolate* isolate, Handle<ArrayList> result,
Handle<JSArray> GetWasmModuleObjectInternalProperties(
Handle<WasmModuleObject> module_object);
} // namespace internal
......
......@@ -1216,6 +1216,7 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
return false;
}
}
bool shouldSkipProto = internalType == V8InternalValueType::kScopeList;
bool formatAccessorsAsProperties =
clientFor(context)->formatAccessorsAsProperties(object);
......@@ -1303,7 +1304,8 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
bool isSymbolDescription =
object->IsSymbol() && name == "description";
if (isSymbolDescription ||
(getterIsNativeFunction && formatAccessorsAsProperties &&
(name != "__proto__" && getterIsNativeFunction &&
formatAccessorsAsProperties &&
!doesAttributeHaveObservableSideEffectOnGet(context, object,
v8Name))) {
v8::TryCatch tryCatch(isolate);
......@@ -1319,6 +1321,7 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
}
}
if (accessorPropertiesOnly && !isAccessorProperty) continue;
if (name == "__proto__") shouldSkipProto = true;
auto mirror = PropertyMirror{name,
writable,
configurable,
......@@ -1337,6 +1340,16 @@ 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;
}
......
This diff is collapsed.
......@@ -463,12 +463,7 @@ class DebugWrapper {
"Runtime.getProperties", { objectId : objectId, ownProperties: true });
this.sendMessage(msg);
const reply = this.takeReplyChecked(msgid);
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');
return Object(reply.result.internalProperties[0].value.value);
}
reconstructRemoteObject(obj) {
......
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,8 +23,6 @@ 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
......@@ -34,34 +32,31 @@ 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
Internal properties
[[Prototype]] object undefined
__proto__ own 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
......@@ -42,6 +42,8 @@ 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);
}
......
......@@ -54,17 +54,6 @@ Running test: testSetVariableValueMain
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
}
]
result : [
[0] : {
configurable : true
......@@ -78,6 +67,19 @@ 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
}
]
}
}
......@@ -96,18 +98,6 @@ 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
......@@ -154,6 +144,20 @@ 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
}
]
}
}
......@@ -171,17 +175,6 @@ Running test: testSetVariableValueMain
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
}
]
result : [
[0] : {
configurable : true
......@@ -195,6 +188,19 @@ 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
}
]
}
}
......
......@@ -824,11 +824,6 @@ Running test: testObjInheritsGetterProperty
type : number
value : NaN
}
[1] : {
name : __proto__
type : object
value : Object
}
]
type : object
}
......
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,8 +23,6 @@ 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
......@@ -34,38 +32,33 @@ 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
Internal properties
[[Prototype]] object undefined
__proto__ own 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
......@@ -79,13 +72,6 @@ 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
......@@ -95,8 +81,7 @@ Internal properties
5 own number 1
6 own number 1
7 own number 1
Internal properties
[[Prototype]] object undefined
__proto__ own object undefined
[[Uint8Array]]
0 own number 1
1 own number 1
......@@ -106,31 +91,21 @@ Internal properties
5 own number 1
6 own number 1
7 own number 1
Internal properties
[[Prototype]] object undefined
__proto__ own object undefined
[[Int16Array]]
0 own number 257
1 own number 257
2 own number 257
3 own number 257
Internal properties
[[Prototype]] object undefined
__proto__ own object undefined
[[Int32Array]]
0 own number 16843009
1 own number 16843009
Internal properties
[[Prototype]] object undefined
__proto__ own 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]]
......@@ -138,22 +113,18 @@ Internal properties
[[ArrayBufferByteLength]]
[[ArrayBufferData]]
[[WebAssemblyMemory]]
Internal properties
[[Prototype]] object undefined
__proto__ own 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
......@@ -45,6 +45,8 @@ 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);
}
......@@ -59,6 +61,8 @@ 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);
}
......@@ -80,6 +84,8 @@ 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);
}
......
......@@ -4,17 +4,6 @@ Running test: testObject
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
}
]
result : [
[0] : {
configurable : true
......@@ -28,23 +17,25 @@ Running test: testObject
}
writable : true
}
]
}
}
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
[1] : {
configurable : true
enumerable : false
isOwn : true
name : __proto__
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
writable : true
}
]
}
}
{
id : <messageId>
result : {
result : [
[0] : {
configurable : false
......@@ -129,6 +120,19 @@ 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
}
]
}
}
......@@ -5,17 +5,6 @@ Retrieving properties in 2
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
}
]
result : [
[0] : {
configurable : true
......@@ -29,24 +18,26 @@ Retrieving properties in 2
}
writable : true
}
]
}
}
Retrieving properties in 1
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
[1] : {
configurable : true
enumerable : false
isOwn : true
name : __proto__
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
writable : true
}
]
}
}
Retrieving properties in 1
{
id : <messageId>
result : {
result : [
[0] : {
configurable : true
......@@ -60,25 +51,27 @@ Retrieving properties in 1
}
writable : true
}
]
}
}
Disconnecting 2
Retrieving properties in 1
{
id : <messageId>
result : {
internalProperties : [
[0] : {
name : [[Prototype]]
[1] : {
configurable : true
enumerable : false
isOwn : true
name : __proto__
value : {
className : Object
description : Object
objectId : <objectId>
type : object
}
writable : true
}
]
}
}
Disconnecting 2
Retrieving properties in 1
{
id : <messageId>
result : {
result : [
[0] : {
configurable : true
......@@ -92,6 +85,19 @@ 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
}
]
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment