Commit f9ebad01 authored by Benedikt Meurer's avatar Benedikt Meurer Committed by V8 LUCI CQ

[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: 's avatarPhilip Pfaffe <pfaffe@chromium.org>
Cr-Commit-Position: refs/heads/master@{#76019}
parent 6ca9684f
......@@ -1170,43 +1170,6 @@ 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;
......@@ -1309,25 +1272,24 @@ bool ValueMirror::getProperties(v8::Local<v8::Context> context,
if (!descriptor.value.IsEmpty()) {
valueMirror = ValueMirror::create(context, descriptor.value);
}
bool getterIsNativeFunction = false;
v8::Local<v8::Function> getterFunction;
if (!descriptor.get.IsEmpty()) {
v8::Local<v8::Value> get = descriptor.get;
getterMirror = ValueMirror::create(context, get);
getterIsNativeFunction =
get->IsFunction() && get.As<v8::Function>()->ScriptId() ==
v8::UnboundScript::kNoScriptId;
if (get->IsFunction()) getterFunction = get.As<v8::Function>();
}
if (!descriptor.set.IsEmpty()) {
setterMirror = ValueMirror::create(context, descriptor.set);
}
isAccessorProperty = getterMirror || setterMirror;
if (name != "__proto__" && getterIsNativeFunction &&
formatAccessorsAsProperties &&
!doesAttributeHaveObservableSideEffectOnGet(context, object,
v8Name)) {
if (formatAccessorsAsProperties && name != "__proto__" &&
!getterFunction.IsEmpty() &&
getterFunction->ScriptId() == v8::UnboundScript::kNoScriptId) {
v8::TryCatch tryCatch(isolate);
v8::Local<v8::Value> value;
if (object->Get(context, v8Name).ToLocal(&value)) {
if (v8::debug::CallFunctionOn(context, getterFunction, object, 0,
nullptr, true)
.ToLocal(&value)) {
valueMirror = ValueMirror::create(context, value);
isOwn = true;
setterMirror = nullptr;
......
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