[Webkit-unassigned] [Bug 33159] Make JSObject::getPropertyNames() non-virtual

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=33159


Kent Hansen <kent.hansen at nokia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #45807|                            |review?
               Flag|                            |




--- Comment #1 from Kent Hansen <kent.hansen at nokia.com>  2010-01-04 10:56:01 PST ---
Created an attachment (id=45807)
 --> (https://bugs.webkit.org/attachment.cgi?id=45807)
Proposed patch

>From the C API perspective, the change has no impact, since
JSObjectGetPropertyNamesCallback already has the semantics of
getOwnPropertyNames().

OverridesGetPropertyNames flag renamed to OverridesGetOwnPropertyNames. (I
don't know why the flag is set for several classes which _don't_ reimplement
it? Are they just out of sync?)

DebuggerActivation: I think it's a bug that getOwnPropertyNames() was calling
m_activation->getPropertyNames() instead of
m_activation->getOwnPropertyNames()?

JSC::Bindings::Instance and friends: When enumerating such objects, the
properties in the prototype chain will now be included as well. I'm not sure
whether this is desirable or not. E.g. if you use an NPObject in a for-in
statement, should the properties in the prototype chain be included? I would
think "Yes", since the NP API has no knowledge of the prototype at all, so it's
not up to it to block its properties. I'm seeing NPN_Enumerate as another "own"
properties variation.

The RuntimeObjectImp::getOwnPropertyNames() implementation seemed broken; it
simply called itself recursively with the same arguments, causing a stack
overflow if you called e.g. Object.keys() on such an object.

JSDOMWindow: There will no longer be an "early cut-off" when getPropertyNames()
is called on a JSDOMWindow that's not accessible from the current context;
instead the prototype chain will be followed. This is bad and reason to reject
this patch, but I wish there was a different way to handle the security checks
(e.g. SpiderMonkey has a checkAccess callback in the base JS "class", so the
check can be initiated from the engine side). Well, as it stands, I guess this
disproves the claim in the task description. :]
>From looking at the JSC C API, it seems that JSObjectGetPrototype() doesn't do
a security check, so it's going to behave differently from
JSObjectGetProperty("__proto__")? Is that fine? Having the security check in
JSObject::prototype() would fix the issue with this part of the patch as
well... (I realize there would probably be performance implications to
investigate.)

JSQuarantinedObjectWrapper: Same issue as JSDOMWindow.

All the existing tests seem to pass, anyway.

Maybe it makes sense to split this bug into smaller tasks to deal with the
various issues.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.