Don't create cached functions that access lexicalGlobalObject()

• Component: WebCore JavaScript
• Status: RESOLVED FIXED

[Michael Saboff]

The current implementation of lexicalGlobalObject() uses the JSScope stored in the current call frame.  For native functions, the JSScope is copied from the caller's frame.  These two behaviors together mean that a native function that calls lexicalGlobalObject() will not get their true lexicalGlobalObject, but their caller's.  As part of eliminating the JSScope slot from the call frame, we need to clean up code that depends on this behavior.  Therefore native functions that access their lexicalGlobalObject need to always be created for each instance of their prototype object, so that their instance has their lexicalGlobalObject.

[Michael Saboff]

Created attachment 240045 [details]
Patch

[Geoffrey Garen]

Comment on attachment 240045 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240045&action=review

r=me

> LayoutTests/ChangeLog:17
> +        a parents and child windows instances are the same.  Now they are always different.

"parent's"
"window's"

[Michael Saboff]

Committed r174847: <http://trac.webkit.org/changeset/174847>

[Darin Adler]

Comment on attachment 240045 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=240045&action=review

> Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:65
> +    if (propertyName == Identifier(exec, "open")) {

I believe this is less efficient than:

    if (propertyName == "open") {

> Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:76
> +    const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName);
> +    if (entry) {

Would be nice to put this definition inside the if.

[Michael Saboff]

(In reply to comment #4)
> Comment on attachment 240045 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240045&action=review
> 
> > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:65
> > +    if (propertyName == Identifier(exec, "open")) {
> 
> I believe this is less efficient than:
> 
>     if (propertyName == "open") {
> 
> > Source/WebCore/bindings/js/JSHTMLDocumentCustom.cpp:76
> > +    const HashTableValue* entry = JSHTMLDocument::info()->staticPropHashTable->entry(propertyName);
> > +    if (entry) {
> 
> Would be nice to put this definition inside the if.

Tracked in <https://bugs.webkit.org/show_bug.cgi?id=137885>