Object.getOwnPropertyNames considers enumerable getter inherited properties as own (636989, r=jorendorff/gal).

This commit is contained in:
Brendan Eich 2011-06-17 00:01:21 -07:00
parent e81d952262
commit a4caa74fea
5 changed files with 48 additions and 113 deletions

View File

@ -172,37 +172,25 @@ NewKeyValuePair(JSContext *cx, jsid id, const Value &val, Value *rval)
static inline bool
Enumerate(JSContext *cx, JSObject *obj, JSObject *pobj, jsid id,
bool enumerable, bool sharedPermanent, uintN flags, IdSet& ht,
AutoIdVector *props)
bool enumerable, uintN flags, IdSet& ht, AutoIdVector *props)
{
IdSet::AddPtr p = ht.lookupForAdd(id);
JS_ASSERT_IF(obj == pobj && !obj->isProxy(), !p);
JS_ASSERT_IF(flags & JSITER_OWNONLY, obj == pobj);
/* If we've already seen this, we definitely won't add it. */
if (JS_UNLIKELY(!!p))
return true;
if (!(flags & JSITER_OWNONLY) || pobj->isProxy()) {
IdSet::AddPtr p = ht.lookupForAdd(id);
JS_ASSERT_IF(obj == pobj && !obj->isProxy(), !p);
/*
* It's not necessary to add properties to the hash table at the end of the
* prototype chain -- but a proxy might return duplicated properties, so
* always add for them.
*/
if ((pobj->getProto() || pobj->isProxy()) && !ht.add(p, id))
return false;
/* If we've already seen this, we definitely won't add it. */
if (JS_UNLIKELY(!!p))
return true;
if (JS_UNLIKELY(flags & JSITER_OWNONLY)) {
/*
* Shared-permanent hack: If this property is shared permanent
* and pobj and obj have the same class, then treat it as an own
* property of obj, even if pobj != obj. (But see bug 575997.)
*
* Omit the magic __proto__ property so that JS code can use
* Object.getOwnPropertyNames without worrying about it.
* It's not necessary to add properties to the hash table at the end of
* the prototype chain, but a proxy might return duplicated properties,
* so always add if pobj is a proxy.
*/
if (!pobj->getProto() && id == ATOM_TO_JSID(cx->runtime->atomState.protoAtom))
return true;
if (pobj != obj && !(sharedPermanent && pobj->getClass() == obj->getClass()))
return true;
if ((pobj->getProto() || pobj->isProxy()) && !ht.add(p, id))
return false;
}
if (enumerable || (flags & JSITER_HIDDEN))
@ -223,8 +211,7 @@ EnumerateNativeProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uintN fl
if (!JSID_IS_DEFAULT_XML_NAMESPACE(shape.propid) &&
!shape.isAlias() &&
!Enumerate(cx, obj, pobj, shape.propid, shape.enumerable(),
shape.isSharedPermanent(), flags, ht, props))
!Enumerate(cx, obj, pobj, shape.propid, shape.enumerable(), flags, ht, props))
{
return false;
}
@ -238,7 +225,7 @@ static bool
EnumerateDenseArrayProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uintN flags,
IdSet &ht, AutoIdVector *props)
{
if (!Enumerate(cx, obj, pobj, ATOM_TO_JSID(cx->runtime->atomState.lengthAtom), false, true,
if (!Enumerate(cx, obj, pobj, ATOM_TO_JSID(cx->runtime->atomState.lengthAtom), false,
flags, ht, props)) {
return false;
}
@ -249,7 +236,7 @@ EnumerateDenseArrayProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uint
for (size_t i = 0; i < capacity; ++i, ++vp) {
if (!vp->isMagic(JS_ARRAY_HOLE)) {
/* Dense arrays never get so large that i would not fit into an integer id. */
if (!Enumerate(cx, obj, pobj, INT_TO_JSID(i), true, false, flags, ht, props))
if (!Enumerate(cx, obj, pobj, INT_TO_JSID(i), true, flags, ht, props))
return false;
}
}
@ -261,11 +248,6 @@ EnumerateDenseArrayProperties(JSContext *cx, JSObject *obj, JSObject *pobj, uint
static bool
Snapshot(JSContext *cx, JSObject *obj, uintN flags, AutoIdVector *props)
{
/*
* FIXME: Bug 575997 - We won't need to initialize this hash table if
* (flags & JSITER_OWNONLY) when we eliminate inheritance of
* shared-permanent properties as own properties.
*/
IdSet ht(cx);
if (!ht.init(32))
return NULL;
@ -299,7 +281,7 @@ Snapshot(JSContext *cx, JSObject *obj, uintN flags, AutoIdVector *props)
return false;
}
for (size_t n = 0, len = proxyProps.length(); n < len; n++) {
if (!Enumerate(cx, obj, pobj, proxyProps[n], true, false, flags, ht, props))
if (!Enumerate(cx, obj, pobj, proxyProps[n], true, flags, ht, props))
return false;
}
/* Proxy objects enumerate the prototype on their own, so we are done here. */
@ -319,13 +301,13 @@ Snapshot(JSContext *cx, JSObject *obj, uintN flags, AutoIdVector *props)
return false;
if (state.isNull())
break;
if (!Enumerate(cx, obj, pobj, id, true, false, flags, ht, props))
if (!Enumerate(cx, obj, pobj, id, true, flags, ht, props))
return false;
}
}
}
if (JS_UNLIKELY(pobj->isXML()))
if ((flags & JSITER_OWNONLY) || pobj->isXML())
break;
} while ((pobj = pobj->getProto()) != NULL);

View File

@ -1540,39 +1540,26 @@ js_PropertyIsEnumerable(JSContext *cx, JSObject *obj, jsid id, Value *vp)
JSObject *pobj;
JSProperty *prop;
if (!obj->lookupProperty(cx, id, &pobj, &prop))
return JS_FALSE;
return false;
if (!prop) {
vp->setBoolean(false);
return JS_TRUE;
return true;
}
/*
* XXX ECMA spec error compatible: return false unless hasOwnProperty.
* The ECMA spec really should be fixed so propertyIsEnumerable and the
* for..in loop agree on whether prototype properties are enumerable,
* obviously by fixing this method (not by breaking the for..in loop!).
*
* We check here for shared permanent prototype properties, which should
* be treated as if they are local to obj. They are an implementation
* technique used to satisfy ECMA requirements; users should not be able
* to distinguish a shared permanent proto-property from a local one.
* ECMA spec botch: return false unless hasOwnProperty. Leaving "own" out
* of propertyIsEnumerable's name was a mistake.
*/
bool shared;
uintN attrs;
if (pobj->isNative()) {
Shape *shape = (Shape *) prop;
shared = shape->isSharedPermanent();
attrs = shape->attributes();
} else {
shared = false;
if (!pobj->getAttributes(cx, id, &attrs))
return false;
}
if (pobj != obj && !shared) {
if (pobj != obj) {
vp->setBoolean(false);
return true;
}
uintN attrs;
if (!pobj->getAttributes(cx, id, &attrs))
return false;
vp->setBoolean((attrs & JSPROP_ENUMERATE) != 0);
return true;
}
@ -2059,23 +2046,6 @@ DefinePropertyOnObject(JSContext *cx, JSObject *obj, const jsid &id, const PropD
JS_ASSERT(!obj->getOps()->defineProperty);
/*
* If we find a shared permanent property in a different object obj2 from
* obj, then if the property is shared permanent (an old hack to optimize
* per-object properties into one prototype property), ignore that lookup
* result (null current).
*
* FIXME: bug 575997 (see also bug 607863).
*/
if (current && obj2 != obj && obj2->isNative()) {
/* See same assertion with comment further below. */
JS_ASSERT(obj2->getClass() == obj->getClass());
Shape *shape = (Shape *) current;
if (shape->isSharedPermanent())
current = NULL;
}
/* 8.12.9 steps 2-4. */
if (!current) {
if (!obj->isExtensible())
@ -2108,15 +2078,7 @@ DefinePropertyOnObject(JSContext *cx, JSObject *obj, const jsid &id, const PropD
/* 8.12.9 steps 5-6 (note 5 is merely a special case of 6). */
Value v = UndefinedValue();
/*
* In the special case of shared permanent properties, the "own" property
* can be found on a different object. In that case the returned property
* might not be native, except: the shared permanent property optimization
* is not applied if the objects have different classes (bug 320854), as
* must be enforced by js_HasOwnProperty for the Shape cast below to be
* safe.
*/
JS_ASSERT(obj->getClass() == obj2->getClass());
JS_ASSERT(obj == obj2);
const Shape *shape = reinterpret_cast<Shape *>(current);
do {
@ -5788,25 +5750,8 @@ js_DeleteProperty(JSContext *cx, JSObject *obj, jsid id, Value *rval, JSBool str
return false;
if (!prop || proto != obj) {
/*
* If the property was found in a native prototype, check whether it's
* shared and permanent. Such a property stands for direct properties
* in all delegating objects, matching ECMA semantics without bloating
* each delegating object.
*/
if (prop && proto->isNative()) {
shape = (Shape *)prop;
if (shape->isSharedPermanent()) {
if (strict)
return obj->reportNotConfigurable(cx, id);
rval->setBoolean(false);
return true;
}
}
/*
* If no property, or the property comes unshared or impermanent from
* a prototype, call the class's delProperty hook, passing rval as the
* result parameter.
* If no property, or the property comes from a prototype, call the
* class's delProperty hook, passing rval as the result parameter.
*/
return CallJSPropertyOp(cx, obj->getClass()->delProperty, obj, id, rval);
}

View File

@ -555,8 +555,6 @@ struct Shape : public js::gc::Cell
bool get(JSContext* cx, JSObject *receiver, JSObject *obj, JSObject *pobj, js::Value* vp) const;
bool set(JSContext* cx, JSObject *obj, bool strict, js::Value* vp) const;
inline bool isSharedPermanent() const;
bool hasSlot() const { return (attrs & JSPROP_SHARED) == 0; }
uint8 attributes() const { return attrs; }
@ -760,12 +758,6 @@ Shape::search(JSRuntime *rt, js::Shape **startp, jsid id, bool adding)
#undef METER
inline bool
Shape::isSharedPermanent() const
{
return (~attrs & (JSPROP_SHARED | JSPROP_PERMANENT)) == 0;
}
} // namespace js
#ifdef _MSC_VER

View File

@ -0,0 +1,15 @@
/*
* Any copyright is dedicated to the Public Domain.
* http://creativecommons.org/licenses/publicdomain/
*/
assertEq(function testcase() {
var proto = {};
Object.defineProperty(proto, "prop", {get: function () {return {};}, enumerable: true});
var ConstructFun = function () {};
ConstructFun.prototype = proto;
var child = new ConstructFun;
return Object.getOwnPropertyNames(child).indexOf('prop');
}(), -1);
reportCompare(0, 0, "ok");

View File

@ -45,3 +45,4 @@ script preventExtensions-idempotent.js
script isPrototypeOf.js
script propertyIsEnumerable.js
script toLocaleString.js
script gOPD-vs-prototype-accessor.js