Bug 1148750, part 1 - Factor out the lookup common to three branches at the top of NativeDefineProperty. r=efaust.

The existing setup saves a branch. We can't keep it. All that code is about to be completely rewritten. In the standard algorithms, this check is not immediately followed by a branch on this particular condition (desc.hasValue()). Furthermore, to deal with resolve hooks properly, we will later change the condition of this if-statement to something like `if (resolving)`, which will not be something we can common up with any other branch in this function.
This commit is contained in:
Jason Orendorff 2015-03-23 14:32:30 -05:00
parent d0a5e0a8f9
commit fbeaf9d086

View File

@ -1298,16 +1298,33 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
}
}
Rooted<PropertyDescriptor> desc(cx, desc_);
// 9.1.6.1 OrdinaryDefineOwnProperty steps 1-2.
RootedShape shape(cx);
if (desc_.hasValue()) {
// If we did a normal lookup here, it would cause resolve hook recursion in
// the following case. Suppose the first script we run in a lazy global is
// |parseInt()|.
// - js::InitNumberClass is called to resolve parseInt.
// - js::InitNumberClass tries to define the Number constructor on the
// global.
// - We end up here.
// - This lookup for 'Number' triggers the global resolve hook.
// - js::InitNumberClass is called again, this time to resolve Number.
// - It creates a second Number constructor, which trips an assertion.
//
// Therefore we do a special lookup that does not call the resolve hook.
NativeLookupOwnPropertyNoResolve(cx, obj, id, &shape);
} else {
if (!NativeLookupOwnProperty<CanGC>(cx, obj, id, &shape))
return false;
}
Rooted<PropertyDescriptor> desc(cx, desc_);
// If defining a getter or setter, we must check for its counterpart and
// update the attributes and property ops. A getter or setter is really
// only half of a property.
if (desc.isAccessorDescriptor()) {
if (!NativeLookupOwnProperty<CanGC>(cx, obj, id, &shape))
return false;
if (shape) {
// If we are defining a getter whose setter was already defined, or
// vice versa, finish the job via obj->changeProperty.
@ -1351,20 +1368,6 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
// must both be filled in.
desc.attributesRef() |= JSPROP_GETTER | JSPROP_SETTER;
} else if (desc.hasValue()) {
// If we did a normal lookup here, it would cause resolve hook recursion in
// the following case. Suppose the first script we run in a lazy global is
// |parseInt()|.
// - js::InitNumberClass is called to resolve parseInt.
// - js::InitNumberClass tries to define the Number constructor on the
// global.
// - We end up here.
// - This lookup for 'Number' triggers the global resolve hook.
// - js::InitNumberClass is called again, this time to resolve Number.
// - It creates a second Number constructor, which trips an assertion.
//
// Therefore we do a special lookup that does not call the resolve hook.
NativeLookupOwnPropertyNoResolve(cx, obj, id, &shape);
if (shape) {
// If any other JSPROP_IGNORE_* attributes are present, copy the
// corresponding JSPROP_* attributes from the existing property.
@ -1383,9 +1386,6 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId
}
} else {
// We have been asked merely to update JSPROP_PERMANENT and/or JSPROP_ENUMERATE.
if (!NativeLookupOwnProperty<CanGC>(cx, obj, id, &shape))
return false;
if (shape) {
// Don't forget about arrays.
if (IsImplicitDenseOrTypedArrayElement(shape)) {