Bug 27276 – Some constructor objects exposed on Window have the wrong prototype chain

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Eric Seidel (no email)]

Some constructor objects exposed on Window have the wrong prototype chain

I'm going to start by making the constructors on Window be consistent in their prototype chains.  Fixing the more general case will require adding a backpointer from every DOMObject to the global object it was created from.

[Eric Seidel (no email)]

Created attachment 32739 [details]
patch

[Eric Seidel (no email)]

CCing participants from the webkit-dev discussion.

[Adam Barth]

Comment on attachment 32739 [details]
patch

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()->objectPrototype()))
> -    , 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->objectPrototype()))
>  {
> -    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?

[Adam Barth]

Assigning to Eric because he has some nits to fix, making this not actionable by anyone else.

[Eric Seidel (no email)]

(In reply to comment #3)
> (From update of attachment 32739 [details])
> This looks great!  A few nits below.
> 
> > +for (property in window) {
> > +    windowProperites.push(property);
> > +}
> 
> No braces here.

Fixed.

> 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.

Doesn't matter.  Why?  (The JS engine will actually make it a pre-increment under the covers anyway.)

> >  JSAudioConstructor::JSAudioConstructor(ExecState* exec, JSDOMGlobalObject* globalObject)
> > -    : DOMObject(JSAudioConstructor::createStructure(exec->lexicalGlobalObject()->objectPrototype()))
> > -    , m_globalObject(globalObject)
> > +    : DOMObject(JSAudioConstructor::createStructure(globalObject->objectPrototype()))
> > +       , m_globalObject(globalObject)
> 
> Extra space added here.

Fixed.

> > -JSXSLTProcessorConstructor::JSXSLTProcessorConstructor(ExecState* exec)
> > -    : DOMObject(JSXSLTProcessorConstructor::createStructure(exec->lexicalGlobalObject()->objectPrototype()))
> > +JSXSLTProcessorConstructor::JSXSLTProcessorConstructor(ExecState* exec, JSDOMGlobalObject* globalObject)
> > +    : DOMObject(JSXSLTProcessorConstructor::createStructure(globalObject->objectPrototype()))
> >  {
> > -    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?

It was missing a length member, so I guessed.  But it's an unrelated change, so I'll remove it.

[Eric Seidel (no email)]

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/dom/prototype-inheritance-expected.txt
	A	LayoutTests/fast/dom/prototype-inheritance.html
	A	LayoutTests/fast/dom/resources/prototype-inheritance.js
	M	WebCore/ChangeLog
	M	WebCore/bindings/js/JSAudioConstructor.cpp
	M	WebCore/bindings/js/JSDOMBinding.h
	M	WebCore/bindings/js/JSDOMGlobalObject.h
	M	WebCore/bindings/js/JSDOMWindowCustom.cpp
	M	WebCore/bindings/js/JSImageConstructor.cpp
	M	WebCore/bindings/js/JSMessageChannelConstructor.cpp
	M	WebCore/bindings/js/JSOptionConstructor.cpp
	M	WebCore/bindings/js/JSWebKitCSSMatrixConstructor.cpp
	M	WebCore/bindings/js/JSWebKitCSSMatrixConstructor.h
	M	WebCore/bindings/js/JSWebKitPointConstructor.cpp
	M	WebCore/bindings/js/JSWebKitPointConstructor.h
	M	WebCore/bindings/js/JSWorkerConstructor.cpp
	M	WebCore/bindings/js/JSWorkerConstructor.h
	M	WebCore/bindings/js/JSXMLHttpRequestConstructor.cpp
	M	WebCore/bindings/js/JSXSLTProcessorConstructor.cpp
	M	WebCore/bindings/js/JSXSLTProcessorConstructor.h
	M	WebCore/bindings/scripts/CodeGeneratorJS.pm
Committed r46068
	M	WebCore/ChangeLog
	M	WebCore/bindings/scripts/CodeGeneratorJS.pm
	M	WebCore/bindings/js/JSXSLTProcessorConstructor.h
	M	WebCore/bindings/js/JSWorkerConstructor.cpp
	M	WebCore/bindings/js/JSDOMGlobalObject.h
	M	WebCore/bindings/js/JSWebKitPointConstructor.cpp
	M	WebCore/bindings/js/JSOptionConstructor.cpp
	M	WebCore/bindings/js/JSXMLHttpRequestConstructor.cpp
	M	WebCore/bindings/js/JSImageConstructor.cpp
	M	WebCore/bindings/js/JSWebKitPointConstructor.h
	M	WebCore/bindings/js/JSDOMBinding.h
	M	WebCore/bindings/js/JSAudioConstructor.cpp
	M	WebCore/bindings/js/JSWebKitCSSMatrixConstructor.cpp
	M	WebCore/bindings/js/JSDOMWindowCustom.cpp
	M	WebCore/bindings/js/JSWebKitCSSMatrixConstructor.h
	M	WebCore/bindings/js/JSXSLTProcessorConstructor.cpp
	M	WebCore/bindings/js/JSMessageChannelConstructor.cpp
	M	WebCore/bindings/js/JSWorkerConstructor.h
	M	LayoutTests/ChangeLog
	A	LayoutTests/fast/dom/resources/prototype-inheritance.js
	A	LayoutTests/fast/dom/prototype-inheritance-expected.txt
	A	LayoutTests/fast/dom/prototype-inheritance.html
r46068 = 29ffe78e086c9016bbf5bd143fd4d2feedde6138 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46068