Bug 132721

Summary: JSDOMWindow should have a WatchpointSet to fire on window close
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 132705    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch fpizlo: review+

Description Mark Hahnenberg 2014-05-08 18:00:41 PDT
This would allow us to reset the inline caches that assumed that they could skip the first part of JSDOMWindow::getOwnPropertySlot.
Comment 1 Mark Hahnenberg 2014-05-08 18:24:15 PDT
Created attachment 231119 [details]
Patch
Comment 2 Mark Hahnenberg 2014-05-08 19:06:23 PDT
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;
Comment 3 Mark Hahnenberg 2014-05-08 19:09:12 PDT
Created attachment 231121 [details]
Patch
Comment 4 Geoffrey Garen 2014-05-09 00:03:24 PDT
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!
Comment 5 Mark Hahnenberg 2014-05-09 11:22:45 PDT
(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.
Comment 6 Mark Hahnenberg 2014-05-09 11:49:35 PDT
Created attachment 231172 [details]
Patch
Comment 7 Mark Hahnenberg 2014-05-09 12:19:08 PDT
Created attachment 231177 [details]
Patch
Comment 8 Filip Pizlo 2014-05-09 12:52:57 PDT
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().
Comment 9 Mark Hahnenberg 2014-05-09 13:26:48 PDT
Committed r168548: <http://trac.webkit.org/changeset/168548>