Bug 132721 – JSDOMWindow should have a WatchpointSet to fire on window close

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Mark Hahnenberg]

This would allow us to reset the inline caches that assumed that they could skip the first part of JSDOMWindow::getOwnPropertySlot.

[Mark Hahnenberg]

Created attachment 231119 [details]
Patch

[Mark Hahnenberg]

Comment on attachment 231119 [details]
Patch

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

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:280
> +            return;

continue;

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:283
> +            return;

continue;

[Mark Hahnenberg]

Created attachment 231121 [details]
Patch

[Geoffrey Garen]

Comment on attachment 231121 [details]
Patch

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

r=me

> Source/JavaScriptCore/jit/Repatch.cpp:334
> +    if (watchpointSet)
> +        watchpointSet->add(stubInfo.addWatchpoint(codeBlock));
> +

Is this really the only place we'll ever specialize a window object property access? Is there some kind of struct flag to guarantee that? For example, what prevents the DFG from fully inlining the property access?

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:275
> +    clientData->getAllWorlds(wrapperWorlds);

If DOMWindow were ScriptWrappable, you would need a special case to clear its inline wrapper. But it isn't. So yay!

[Mark Hahnenberg]

(In reply to comment #4)
> (From update of attachment 231121 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231121&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jit/Repatch.cpp:334
> > +    if (watchpointSet)
> > +        watchpointSet->add(stubInfo.addWatchpoint(codeBlock));
> > +
> 
> Is this really the only place we'll ever specialize a window object property access? Is there some kind of struct flag to guarantee that? For example, what prevents the DFG from fully inlining the property access?

Now that you mention it, the DFG definitely needs to know about this. This will require more infrastructure, so I'll post a new patch.

[Mark Hahnenberg]

Created attachment 231172 [details]
Patch

[Mark Hahnenberg]

Created attachment 231177 [details]
Patch

[Filip Pizlo]

Comment on attachment 231177 [details]
Patch

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

> Source/JavaScriptCore/bytecode/PolymorphicGetByIdList.h:84
>      bool doesCalls() const { return type() == Getter || type() == CustomGetter; }
> +    bool isWatched() const { return type() == WatchedStub; }

Add an isSimple() and assert that instead of the places where we currently assert !doesCalls().

[Mark Hahnenberg]

Committed r168548: <http://trac.webkit.org/changeset/168548>