AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "[inspector] Use side-effect free debug evaluate for inherited accessors."

Browse Source 
This reverts commit f9ebad0119.

Reason for revert: suspected root cause of crbug.com/1257806 Additionally, this patch might actually be incorrect as we eagerly evaluate native accessors, which can only happen if the debugger is running.

Original change's description:
> [inspector] Use side-effect free debug evaluate for inherited accessors.
>
> Replace the hard-coded blocklist ("Response.body" and "Request.body") in
> the V8 inspector with proper side-effect free debug evaluate. This is
> otherwise a non-functional change and in particular preserves the
> behavior of reporting accessors as (own) data properties. That will be
> tackled in a follow-up CL.
>
> This CL is possible because with https://crrev.com/c/3056879 Blink now
> properly marks accessors as side-effect free consistently with what the
> V8 inspector had done before.
>
> Doc: http://doc/1gLyyOlssS5zyCSEyybVC-5sp0UnNJj2hBoFyf6ryrTc
> Bug: chromium:829571, chromium:1076820, chromium:1119900
> Change-Id: Idb256accaf4cfb5db5982b3eb06ddcef588be635
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3062573
> Auto-Submit: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Philip Pfaffe <pfaffe@chromium.org>
> Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#76019}

Bug: chromium:829571, chromium:1076820, chromium:1119900, chromium:1257806
Fixed: chromium:1265372
Change-Id: Ia31a3022aaa9ddeae1f01eaa90e345f8bdbb21c9
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3259653
Commit-Queue: Tim van der Lippe <tvanderlippe@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#77708}
This commit is contained in:
Tim van der Lippe 2021-11-04 11:23:52 +00:00 committed by V8 LUCI CQ
parent 6b10881100
commit e9d3611781
4 changed files with 122 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
src/inspector/value-mirror.cc
View File

@ -1179,6 +1179,43 @@ std::unique_ptr<ValueMirror> createNativeSetter(v8::Local<v8::Context> context,
return ValueMirror::create(context, function);
}
bool doesAttributeHaveObservableSideEffectOnGet(v8::Local<v8::Context> context,
v8::Local<v8::Object> object,
v8::Local<v8::Name> name) {
// TODO(dgozman): we should remove this, annotate more embedder properties as
// side-effect free, and call all getters which do not produce side effects.
if (!name->IsString()) return false;
v8::Isolate* isolate = context->GetIsolate();
if (!name.As<v8::String>()->StringEquals(toV8String(isolate, "body"))) {
return false;
}
v8::TryCatch tryCatch(isolate);
v8::Local<v8::Value> request;
if (context->Global()
->GetRealNamedProperty(context, toV8String(isolate, "Request"))
.ToLocal(&request)) {
if (request->IsObject() &&
object->InstanceOf(context, request.As<v8::Object>())
.FromMaybe(false)) {
return true;
}
}
if (tryCatch.HasCaught()) tryCatch.Reset();
v8::Local<v8::Value> response;
if (context->Global()
->GetRealNamedProperty(context, toV8String(isolate, "Response"))
.ToLocal(&response)) {
if (response->IsObject() &&
object->InstanceOf(context, response.As<v8::Object>())
.FromMaybe(false)) {
return true;
}
}
return false;
}
} // anonymous namespace
ValueMirror::~ValueMirror() = default;
@ -1294,12 +1331,12 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
}
isAccessorProperty = getterMirror || setterMirror;
if (name != "__proto__" && !getterFunction.IsEmpty() &&
getterFunction->ScriptId() == v8::UnboundScript::kNoScriptId) {
getterFunction->ScriptId() == v8::UnboundScript::kNoScriptId &&
!doesAttributeHaveObservableSideEffectOnGet(context, object,
v8Name)) {
v8::TryCatch tryCatchFunction(isolate);
v8::Local<v8::Value> value;
if (v8::debug::CallFunctionOn(context, getterFunction, object, 0,
nullptr, true)
.ToLocal(&value)) {
if (object->Get(context, v8Name).ToLocal(&value)) {
if (value->IsPromise() &&
value.As<v8::Promise>()->State() == v8::Promise::kRejected) {
value.As<v8::Promise>()->MarkAsHandled();

53
test/inspector/debugger/object-preview-internal-properties-expected.txt
View File

@ -28,6 +28,11 @@ expression: Object(Symbol(42))
type : symbol
value : Symbol(42)
}
{
name : description
type : string
value : 42
}
expression: Object(BigInt(2))
{
@ -252,6 +257,51 @@ expression: /123/
type : number
value : 0
}
{
name : dotAll
type : boolean
value : false
}
{
name : flags
type : string
value :
}
{
name : global
type : boolean
value : false
}
{
name : hasIndices
type : boolean
value : false
}
{
name : ignoreCase
type : boolean
value : false
}
{
name : multiline
type : boolean
value : false
}
{
name : source
type : string
value : 123
}
{
name : sticky
type : boolean
value : false
}
{
name : unicode
type : boolean
value : false
}
expression: ({})
@ -318,6 +368,5 @@ expression: new Proxy(() => {}, { get: () => x++ })
{
name : name
type : string
value :
value :
}

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

@ -18,7 +18,7 @@ Running test: testObjectPropertiesPreview
[2] : {
name : p3
type : function
value :
value :
}
[3] : {
name : p4
@ -51,12 +51,12 @@ Running test: testArrayPropertiesPreview
[2] : {
name : 4
type : function
value :
value :
}
[3] : {
name : nonEntryFunction
type : function
value :
value :
}
[4] : {
name : 5
@ -142,6 +142,11 @@ Running test: testShortTypedArrayPropertiesPreview
type : number
value : 3
}
[7] : {
name : Symbol(Symbol.toStringTag)
type : string
value : Uint8Array
}
]
subtype : typedarray
type : object

29
test/inspector/runtime/remote-object-expected.txt
View File

@ -519,7 +519,7 @@ Running test: testRegExp
objectId : <objectId>
preview : {
description : /w+/g
overflow : false
overflow : true
properties : [
[0] : {
name : prop
@ -531,6 +531,21 @@ Running test: testRegExp
type : number
value : 0
}
[2] : {
name : dotAll
type : boolean
value : false
}
[3] : {
name : flags
type : string
value : g
}
[4] : {
name : global
type : boolean
value : true
}
]
subtype : regexp
type : object
@ -1209,7 +1224,7 @@ Running test: testWeakMap
[0] : {
name : setTimeout
type : function
value :
value :
}
[1] : {
name : inspector
@ -1293,7 +1308,7 @@ Running test: testWeakSet
[0] : {
name : setTimeout
type : function
value :
value :
}
[1] : {
name : inspector
@ -1751,7 +1766,7 @@ Running test: testTypedArray
objectId : <objectId>
preview : {
description : Uint8Array(2)
overflow : false
overflow : true
properties : [
[0] : {
name : 0
@ -2678,7 +2693,7 @@ Running test: testOtherObjects
[0] : {
name : a
type : function
value :
value :
}
]
type : object
@ -2868,12 +2883,12 @@ Running test: testOtherObjects
[0] : {
name : a1
type : function
value :
value :
}
[1] : {
name : a2
type : function
value :
value :
}
]
type : object