[api] Prefer Descriptor interceptor over Getter in GetPropertyAttributes

Also fix GetPropertyDescriptorWithInterceptor so that it only calls the
interceptor once.

R=ahaas@chromium.org, franzih@chromium.org

Bug: node:17480, node:17481
Change-Id: I2c3813f80df2962ec909bae7267884ce0b8ccbef
Reviewed-on: https://chromium-review.googlesource.com/816515
Commit-Queue: Franziska Hinkelmann <franzih@chromium.org>
Reviewed-by: Franziska Hinkelmann <franzih@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50260}
This commit is contained in:
Timothy Gu 2017-12-20 06:53:25 -08:00 committed by Commit Bot
parent 5d10735e18
commit d5fbf7c5c3
3 changed files with 67 additions and 12 deletions

View File

@ -135,6 +135,7 @@ Sanjoy Das <sanjoy@playingwithpointers.com>
Seo Sanghyeon <sanxiyn@gmail.com>
Stefan Penner <stefan.penner@gmail.com>
Sylvestre Ledru <sledru@mozilla.com>
Timothy Gu <timothygu99@gmail.com>
Tobias Burnus <burnus@net-b.de>
Victor Costan <costan@gmail.com>
Vlad Burlik <vladbph@gmail.com>

View File

@ -1895,6 +1895,22 @@ Maybe<PropertyAttributes> GetPropertyAttributesWithInterceptorInternal(
CHECK(result->ToInt32(&value));
return Just(static_cast<PropertyAttributes>(value));
}
} else if (!interceptor->descriptor()->IsUndefined(isolate)) {
Handle<Object> result;
if (it->IsElement()) {
result = args.CallIndexedDescriptor(interceptor, it->index());
} else {
result = args.CallNamedDescriptor(interceptor, it->name());
}
if (!result.is_null()) {
PropertyDescriptor desc;
Utils::ApiCheck(
PropertyDescriptor::ToPropertyDescriptor(isolate, result, &desc),
it->IsElement() ? "v8::IndexedPropertyDescriptorCallback"
: "v8::NamedPropertyDescriptorCallback",
"Invalid property descriptor.");
return Just(desc.ToAttributes());
}
} else if (!interceptor->getter()->IsUndefined(isolate)) {
// TODO(verwaest): Use GetPropertyWithInterceptor?
Handle<Object> result;
@ -7622,13 +7638,17 @@ namespace {
Maybe<bool> GetPropertyDescriptorWithInterceptor(LookupIterator* it,
PropertyDescriptor* desc) {
bool has_access = true;
if (it->state() == LookupIterator::ACCESS_CHECK) {
has_access = it->HasAccess() || JSObject::AllCanRead(it);
it->Next();
if (it->HasAccess()) {
it->Next();
} else if (!JSObject::AllCanRead(it) ||
it->state() != LookupIterator::INTERCEPTOR) {
it->Restart();
return Just(false);
}
}
if (has_access && it->state() == LookupIterator::INTERCEPTOR) {
if (it->state() == LookupIterator::INTERCEPTOR) {
Isolate* isolate = it->isolate();
Handle<InterceptorInfo> interceptor = it->GetInterceptor();
if (!interceptor->descriptor()->IsUndefined(isolate)) {
@ -7660,9 +7680,9 @@ Maybe<bool> GetPropertyDescriptorWithInterceptor(LookupIterator* it,
return Just(true);
}
it->Next();
}
}
it->Restart();
return Just(false);
}
} // namespace

View File

@ -2029,14 +2029,18 @@ THREADED_TEST(PropertyDefinerCallbackWithSetter) {
}
namespace {
int empty_descriptor_called;
void EmptyPropertyDescriptorCallback(
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
// Do not intercept by not calling info.GetReturnValue().Set().
empty_descriptor_called++;
}
int intercepting_descriptor_called;
void InterceptingPropertyDescriptorCallback(
Local<Name> name, const v8::PropertyCallbackInfo<v8::Value>& info) {
// Intercept the callback by setting a different descriptor.
intercepting_descriptor_called++;
const char* code =
"var desc = {value: 42};"
"desc;";
@ -2045,6 +2049,12 @@ void InterceptingPropertyDescriptorCallback(
.ToLocalChecked();
info.GetReturnValue().Set(descriptor);
}
template <class S, class T>
void CrashingPropertyCallback(Local<S> name,
const v8::PropertyCallbackInfo<T>& info) {
UNREACHABLE();
}
} // namespace
THREADED_TEST(PropertyDescriptorCallback) {
@ -2052,10 +2062,11 @@ THREADED_TEST(PropertyDescriptorCallback) {
LocalContext env;
{ // Normal behavior of getOwnPropertyDescriptor() with empty callback.
empty_descriptor_called = 0;
v8::Local<v8::FunctionTemplate> templ =
v8::FunctionTemplate::New(CcTest::isolate());
templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
0, 0, EmptyPropertyDescriptorCallback, 0, 0, 0));
CrashingPropertyCallback, 0, EmptyPropertyDescriptorCallback, 0, 0, 0));
env->Global()
->Set(env.local(), v8_str("obj"), templ->GetFunction(env.local())
.ToLocalChecked()
@ -2071,9 +2082,21 @@ THREADED_TEST(PropertyDescriptorCallback) {
.ToLocalChecked()
->Int32Value(env.local())
.FromJust());
CHECK_EQ(false, v8_compile("'empty' in obj;")
->Run(env.local())
.ToLocalChecked()
->BooleanValue(env.local())
.FromJust());
CHECK_EQ(true, v8_compile("'x' in obj;")
->Run(env.local())
.ToLocalChecked()
->BooleanValue(env.local())
.FromJust());
CHECK_EQ(3, empty_descriptor_called);
}
{ // Intercept getOwnPropertyDescriptor().
intercepting_descriptor_called = 0;
v8::Local<v8::FunctionTemplate> templ =
v8::FunctionTemplate::New(CcTest::isolate());
templ->InstanceTemplate()->SetHandler(v8::NamedPropertyHandlerConfiguration(
@ -2093,6 +2116,17 @@ THREADED_TEST(PropertyDescriptorCallback) {
.ToLocalChecked()
->Int32Value(env.local())
.FromJust());
CHECK_EQ(true, v8_compile("'empty' in obj;")
->Run(env.local())
.ToLocalChecked()
->BooleanValue(env.local())
.FromJust());
CHECK_EQ(true, v8_compile("'x' in obj;")
->Run(env.local())
.ToLocalChecked()
->BooleanValue(env.local())
.FromJust());
CHECK_EQ(3, intercepting_descriptor_called);
}
}
@ -4714,7 +4748,7 @@ TEST(NamedAllCanReadInterceptor) {
ExpectInt32("checked.whatever", 17);
CHECK(!CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')")
->IsUndefined());
CHECK_EQ(6, access_check_data.count);
CHECK_EQ(5, access_check_data.count);
access_check_data.result = false;
ExpectInt32("checked.whatever", intercept_data_0.value);
@ -4723,7 +4757,7 @@ TEST(NamedAllCanReadInterceptor) {
CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')");
CHECK(try_catch.HasCaught());
}
CHECK_EQ(9, access_check_data.count);
CHECK_EQ(8, access_check_data.count);
intercept_data_1.should_intercept = true;
ExpectInt32("checked.whatever", intercept_data_1.value);
@ -4732,7 +4766,7 @@ TEST(NamedAllCanReadInterceptor) {
CompileRun("Object.getOwnPropertyDescriptor(checked, 'whatever')");
CHECK(try_catch.HasCaught());
}
CHECK_EQ(12, access_check_data.count);
CHECK_EQ(11, access_check_data.count);
g_access_check_data = nullptr;
}
@ -4801,7 +4835,7 @@ TEST(IndexedAllCanReadInterceptor) {
ExpectInt32("checked[15]", 17);
CHECK(!CompileRun("Object.getOwnPropertyDescriptor(checked, '15')")
->IsUndefined());
CHECK_EQ(6, access_check_data.count);
CHECK_EQ(5, access_check_data.count);
access_check_data.result = false;
ExpectInt32("checked[15]", intercept_data_0.value);
@ -4810,7 +4844,7 @@ TEST(IndexedAllCanReadInterceptor) {
CompileRun("Object.getOwnPropertyDescriptor(checked, '15')");
CHECK(try_catch.HasCaught());
}
CHECK_EQ(9, access_check_data.count);
CHECK_EQ(8, access_check_data.count);
intercept_data_1.should_intercept = true;
ExpectInt32("checked[15]", intercept_data_1.value);
@ -4819,7 +4853,7 @@ TEST(IndexedAllCanReadInterceptor) {
CompileRun("Object.getOwnPropertyDescriptor(checked, '15')");
CHECK(try_catch.HasCaught());
}
CHECK_EQ(12, access_check_data.count);
CHECK_EQ(11, access_check_data.count);
g_access_check_data = nullptr;
}