Skip to content
This repository has been archived by the owner on Oct 15, 2020. It is now read-only.

Commit

Permalink
chakrashim: fixing ObjectTemplate regression
Browse files Browse the repository at this point in the history 
During some adhoc testing I found that HasOwnProperty behavior
regressed for objects created with value properties.  I reworked the
callback to early exit when a property descriptor callback returns a
valid descriptor and restored the rest of the function logic back.

Refs: 5fed2d6

PR-URL: #435
Reviewed-By: Jimmy Thomson <jithomso@microsoft.com>
  • Loading branch information
@kfarnung
kfarnung committed Nov 29, 2017
1 parent e72ff2e commit cf8b577
Showing 1 changed file with 76 additions and 62 deletions.
138 changes: 76 additions & 62 deletions deps/chakrashim/src/v8objecttemplate.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -622,115 +622,129 @@ JsValueRef CHAKRA_CALLBACK Utils::GetOwnPropertyDescriptorCallback(

if (isPropIntType) {
if (objectData->setterGetterInterceptor != nullptr) {
if (objectData->setterGetterInterceptor->
indexedPropertyDescriptor != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->
indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->indexedPropertyDescriptor(
index, info);
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());

if (descriptor != JS_INVALID_REFERENCE) {
return descriptor;
}
}

if (objectData->setterGetterInterceptor->
indexedPropertyQuery != nullptr) {
HandleScope scope(nullptr);
PropertyCallbackInfo<Integer> info(
*(objectData->setterGetterInterceptor->
indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
*(objectData->setterGetterInterceptor->
indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->indexedPropertyQuery(index, info);
queryResult = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
}

if (objectData->setterGetterInterceptor->
indexedPropertyGetter != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->
indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
*(objectData->setterGetterInterceptor->
indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->indexedPropertyGetter(index, info);
value = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
}
}

// if queryResult valid, ensure value available
if (queryResult != JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
if (jsrt::GetIndexedProperty(object, index, &value) != JsNoError) {
return jsrt::GetUndefined();
// if queryResult valid, ensure value available
if (queryResult != JS_INVALID_REFERENCE && value ==
JS_INVALID_REFERENCE) {
if (jsrt::GetIndexedProperty(object, index, &value) != JsNoError) {
return jsrt::GetUndefined();
}
}
}

// We should not have both a Descriptor and a Query.
if (objectData->setterGetterInterceptor != nullptr &&
objectData->setterGetterInterceptor->
indexedPropertyDescriptor != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->indexedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->
indexedPropertyDescriptor(index, info);
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
}
} else { // named property...
if (objectData->setterGetterInterceptor != nullptr) {
if (objectData->setterGetterInterceptor->
namedPropertyDescriptor != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->
namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->namedPropertyDescriptor(
reinterpret_cast<String*>(prop), info);
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());

if (descriptor != JS_INVALID_REFERENCE) {
return descriptor;
}
}

// query the property descriptor if there is such, and then get the value
// from the proxy in order to go through the interceptor
if (objectData->setterGetterInterceptor->namedPropertyQuery != nullptr) {
HandleScope scope(nullptr);
PropertyCallbackInfo<Integer> info(
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
*(objectData->setterGetterInterceptor->
namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->
namedPropertyQuery(reinterpret_cast<String*>(prop), info);
namedPropertyQuery(reinterpret_cast<String*>(prop), info);
queryResult = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
}

if (objectData->setterGetterInterceptor->namedPropertyGetter != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
*(objectData->setterGetterInterceptor->
namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->
namedPropertyGetter(reinterpret_cast<String*>(prop), info);
namedPropertyGetter(reinterpret_cast<String*>(prop), info);
value = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
}
}

// if queryResult valid, ensure value available
if (queryResult != JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
if (jsrt::GetProperty(object, prop, &value) != JsNoError) {
return jsrt::GetUndefined();
// if queryResult valid, ensure value available
if (queryResult != JS_INVALID_REFERENCE && value ==
JS_INVALID_REFERENCE) {
if (jsrt::GetProperty(object, prop, &value) != JsNoError) {
return jsrt::GetUndefined();
}
}
}
}

if (objectData->setterGetterInterceptor != nullptr &&
objectData->setterGetterInterceptor->namedPropertyDescriptor != nullptr) {
PropertyCallbackInfo<Value> info(
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
reinterpret_cast<Object*>(object),
/*holder*/reinterpret_cast<Object*>(object));
objectData->setterGetterInterceptor->namedPropertyDescriptor(
reinterpret_cast<String*>(prop), info);
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
// if neither is intercepted, fallback to default
if (queryResult == JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
if (jsrt::GetOwnPropertyDescriptor(object, prop,
&descriptor) != JsNoError) {
return jsrt::GetUndefined();
}

return descriptor;
}

int queryResultInt = v8::PropertyAttribute::DontEnum;
if (queryResult != JS_INVALID_REFERENCE) {
if (jsrt::ValueToIntLikely(queryResult, &queryResultInt) != JsNoError) {
return jsrt::GetUndefined();
}

v8::PropertyAttribute attributes =
static_cast<v8::PropertyAttribute>(queryResultInt);
if (jsrt::CreatePropertyDescriptor(attributes, value, JS_INVALID_REFERENCE,
JS_INVALID_REFERENCE,
&descriptor) != JsNoError) {
return jsrt::GetUndefined();
}
}

// if nothing is intercepted, fallback to default
if (descriptor == JS_INVALID_REFERENCE) {
if (jsrt::GetOwnPropertyDescriptor(object, prop,
&descriptor) != JsNoError) {
return jsrt::GetUndefined();
}
v8::PropertyAttribute attributes =
static_cast<v8::PropertyAttribute>(queryResultInt);
if (jsrt::CreatePropertyDescriptor(attributes, value, JS_INVALID_REFERENCE,
JS_INVALID_REFERENCE,
&descriptor) != JsNoError) {
return jsrt::GetUndefined();
}

return descriptor;
Expand Down

0 comments on commit cf8b577

Please sign in to comment.