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

Commit cf8b577

Browse files
kfarnungkfarnung
committed
chakrashim: fixing ObjectTemplate regression
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>
1 parent e72ff2e commit cf8b577

File tree

1 file changed

+76
-62
lines changed

1 file changed

+76
-62
lines changed

‎deps/chakrashim/src/v8objecttemplate.cc‎

Lines changed: 76 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -622,115 +622,129 @@ JsValueRef CHAKRA_CALLBACK Utils::GetOwnPropertyDescriptorCallback(
622622

623623
if (isPropIntType) {
624624
if (objectData->setterGetterInterceptor != nullptr) {
625+
if (objectData->setterGetterInterceptor->
626+
indexedPropertyDescriptor != nullptr) {
627+
PropertyCallbackInfo<Value> info(
628+
*(objectData->setterGetterInterceptor->
629+
indexedPropertyInterceptorData),
630+
reinterpret_cast<Object*>(object),
631+
/*holder*/reinterpret_cast<Object*>(object));
632+
objectData->setterGetterInterceptor->indexedPropertyDescriptor(
633+
index, info);
634+
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
635+
636+
if (descriptor != JS_INVALID_REFERENCE) {
637+
return descriptor;
638+
}
639+
}
640+
625641
if (objectData->setterGetterInterceptor->
626642
indexedPropertyQuery != nullptr) {
627643
HandleScope scope(nullptr);
628644
PropertyCallbackInfo<Integer> info(
629-
*(objectData->setterGetterInterceptor->
630-
indexedPropertyInterceptorData),
631-
reinterpret_cast<Object*>(object),
632-
/*holder*/reinterpret_cast<Object*>(object));
645+
*(objectData->setterGetterInterceptor->
646+
indexedPropertyInterceptorData),
647+
reinterpret_cast<Object*>(object),
648+
/*holder*/reinterpret_cast<Object*>(object));
633649
objectData->setterGetterInterceptor->indexedPropertyQuery(index, info);
634650
queryResult = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
635651
}
636652

637653
if (objectData->setterGetterInterceptor->
638654
indexedPropertyGetter != nullptr) {
639655
PropertyCallbackInfo<Value> info(
640-
*(objectData->setterGetterInterceptor->
641-
indexedPropertyInterceptorData),
642-
reinterpret_cast<Object*>(object),
643-
/*holder*/reinterpret_cast<Object*>(object));
656+
*(objectData->setterGetterInterceptor->
657+
indexedPropertyInterceptorData),
658+
reinterpret_cast<Object*>(object),
659+
/*holder*/reinterpret_cast<Object*>(object));
644660
objectData->setterGetterInterceptor->indexedPropertyGetter(index, info);
645661
value = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
646662
}
647-
}
648663

649-
// if queryResult valid, ensure value available
650-
if (queryResult != JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
651-
if (jsrt::GetIndexedProperty(object, index, &value) != JsNoError) {
652-
return jsrt::GetUndefined();
664+
// if queryResult valid, ensure value available
665+
if (queryResult != JS_INVALID_REFERENCE && value ==
666+
JS_INVALID_REFERENCE) {
667+
if (jsrt::GetIndexedProperty(object, index, &value) != JsNoError) {
668+
return jsrt::GetUndefined();
669+
}
653670
}
654671
}
655-
656-
// We should not have both a Descriptor and a Query.
657-
if (objectData->setterGetterInterceptor != nullptr &&
658-
objectData->setterGetterInterceptor->
659-
indexedPropertyDescriptor != nullptr) {
660-
PropertyCallbackInfo<Value> info(
661-
*(objectData->setterGetterInterceptor->indexedPropertyInterceptorData),
662-
reinterpret_cast<Object*>(object),
663-
/*holder*/reinterpret_cast<Object*>(object));
664-
objectData->setterGetterInterceptor->
665-
indexedPropertyDescriptor(index, info);
666-
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
667-
}
668672
} else { // named property...
669673
if (objectData->setterGetterInterceptor != nullptr) {
674+
if (objectData->setterGetterInterceptor->
675+
namedPropertyDescriptor != nullptr) {
676+
PropertyCallbackInfo<Value> info(
677+
*(objectData->setterGetterInterceptor->
678+
namedPropertyInterceptorData),
679+
reinterpret_cast<Object*>(object),
680+
/*holder*/reinterpret_cast<Object*>(object));
681+
objectData->setterGetterInterceptor->namedPropertyDescriptor(
682+
reinterpret_cast<String*>(prop), info);
683+
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
684+
685+
if (descriptor != JS_INVALID_REFERENCE) {
686+
return descriptor;
687+
}
688+
}
689+
670690
// query the property descriptor if there is such, and then get the value
671691
// from the proxy in order to go through the interceptor
672692
if (objectData->setterGetterInterceptor->namedPropertyQuery != nullptr) {
673693
HandleScope scope(nullptr);
674694
PropertyCallbackInfo<Integer> info(
675-
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
676-
reinterpret_cast<Object*>(object),
677-
/*holder*/reinterpret_cast<Object*>(object));
695+
*(objectData->setterGetterInterceptor->
696+
namedPropertyInterceptorData),
697+
reinterpret_cast<Object*>(object),
698+
/*holder*/reinterpret_cast<Object*>(object));
678699
objectData->setterGetterInterceptor->
679-
namedPropertyQuery(reinterpret_cast<String*>(prop), info);
700+
namedPropertyQuery(reinterpret_cast<String*>(prop), info);
680701
queryResult = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
681702
}
682703

683704
if (objectData->setterGetterInterceptor->namedPropertyGetter != nullptr) {
684705
PropertyCallbackInfo<Value> info(
685-
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
686-
reinterpret_cast<Object*>(object),
687-
/*holder*/reinterpret_cast<Object*>(object));
706+
*(objectData->setterGetterInterceptor->
707+
namedPropertyInterceptorData),
708+
reinterpret_cast<Object*>(object),
709+
/*holder*/reinterpret_cast<Object*>(object));
688710
objectData->setterGetterInterceptor->
689-
namedPropertyGetter(reinterpret_cast<String*>(prop), info);
711+
namedPropertyGetter(reinterpret_cast<String*>(prop), info);
690712
value = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
691713
}
692-
}
693714

694-
// if queryResult valid, ensure value available
695-
if (queryResult != JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
696-
if (jsrt::GetProperty(object, prop, &value) != JsNoError) {
697-
return jsrt::GetUndefined();
715+
// if queryResult valid, ensure value available
716+
if (queryResult != JS_INVALID_REFERENCE && value ==
717+
JS_INVALID_REFERENCE) {
718+
if (jsrt::GetProperty(object, prop, &value) != JsNoError) {
719+
return jsrt::GetUndefined();
720+
}
698721
}
699722
}
723+
}
700724

701-
if (objectData->setterGetterInterceptor != nullptr &&
702-
objectData->setterGetterInterceptor->namedPropertyDescriptor != nullptr) {
703-
PropertyCallbackInfo<Value> info(
704-
*(objectData->setterGetterInterceptor->namedPropertyInterceptorData),
705-
reinterpret_cast<Object*>(object),
706-
/*holder*/reinterpret_cast<Object*>(object));
707-
objectData->setterGetterInterceptor->namedPropertyDescriptor(
708-
reinterpret_cast<String*>(prop), info);
709-
descriptor = reinterpret_cast<JsValueRef>(info.GetReturnValue().Get());
725+
// if neither is intercepted, fallback to default
726+
if (queryResult == JS_INVALID_REFERENCE && value == JS_INVALID_REFERENCE) {
727+
if (jsrt::GetOwnPropertyDescriptor(object, prop,
728+
&descriptor) != JsNoError) {
729+
return jsrt::GetUndefined();
710730
}
731+
732+
return descriptor;
711733
}
712734

713735
int queryResultInt = v8::PropertyAttribute::DontEnum;
714736
if (queryResult != JS_INVALID_REFERENCE) {
715737
if (jsrt::ValueToIntLikely(queryResult, &queryResultInt) != JsNoError) {
716738
return jsrt::GetUndefined();
717739
}
718-
719-
v8::PropertyAttribute attributes =
720-
static_cast<v8::PropertyAttribute>(queryResultInt);
721-
if (jsrt::CreatePropertyDescriptor(attributes, value, JS_INVALID_REFERENCE,
722-
JS_INVALID_REFERENCE,
723-
&descriptor) != JsNoError) {
724-
return jsrt::GetUndefined();
725-
}
726740
}
727741

728-
// if nothing is intercepted, fallback to default
729-
if (descriptor == JS_INVALID_REFERENCE) {
730-
if (jsrt::GetOwnPropertyDescriptor(object, prop,
731-
&descriptor) != JsNoError) {
732-
return jsrt::GetUndefined();
733-
}
742+
v8::PropertyAttribute attributes =
743+
static_cast<v8::PropertyAttribute>(queryResultInt);
744+
if (jsrt::CreatePropertyDescriptor(attributes, value, JS_INVALID_REFERENCE,
745+
JS_INVALID_REFERENCE,
746+
&descriptor) != JsNoError) {
747+
return jsrt::GetUndefined();
734748
}
735749

736750
return descriptor;

0 commit comments

Comments
 (0)