[webkit-reviews] review granted: [Bug 27276] Some constructor objects exposed on Window have the wrong prototype chain : [Attachment 32739] patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Jul 15 23:31:38 PDT 2009


Adam Barth <abarth at webkit.org> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 27276: Some constructor objects exposed on Window have the wrong prototype
chain
https://bugs.webkit.org/show_bug.cgi?id=27276

Attachment 32739: patch
https://bugs.webkit.org/attachment.cgi?id=32739&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
This looks great!  A few nits below.

> +for (property in window) {
> +    windowProperites.push(property);
> +}

No braces here.

I'm surprised we don't see a lot of junk from the testing code.  Maybe it's
better to enumerate the subframe's properties?

> +for (var x = 0; x < windowProperites.length; x++) {

Preincrement.

>  JSAudioConstructor::JSAudioConstructor(ExecState* exec, JSDOMGlobalObject*
globalObject)
> -    :
DOMObject(JSAudioConstructor::createStructure(exec->lexicalGlobalObject()->obje
ctPrototype()))
> -    , m_globalObject(globalObject)
> +    :
DOMObject(JSAudioConstructor::createStructure(globalObject->objectPrototype()))

> +	  , m_globalObject(globalObject)

Extra space added here.

> -JSXSLTProcessorConstructor::JSXSLTProcessorConstructor(ExecState* exec)
> -    :
DOMObject(JSXSLTProcessorConstructor::createStructure(exec->lexicalGlobalObject
()->objectPrototype()))
> +JSXSLTProcessorConstructor::JSXSLTProcessorConstructor(ExecState* exec,
JSDOMGlobalObject* globalObject)
> +    :
DOMObject(JSXSLTProcessorConstructor::createStructure(globalObject->objectProto
type()))
>  {
> -    putDirect(exec->propertyNames().prototype,
JSXSLTProcessorPrototype::self(exec, exec->lexicalGlobalObject()), None);
> +    putDirect(exec->propertyNames().prototype,
JSXSLTProcessorPrototype::self(exec, globalObject), None);
> +    putDirect(exec->propertyNames().length, jsNumber(exec, 1),
ReadOnly|DontDelete|DontEnum);

Where did this length line come from?


More information about the webkit-reviews mailing list