Bug 94460 – [V8] Move retrieve{Window,Frame,PerContextData}() from V8Proxy to V8Binding

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Kentaro Hara]

To kill V8Proxy, we move retrieve{Window,Frame,PerContextData}() from V8Proxy to V8Binding.

[Kentaro Hara]

Created attachment 159386 [details]
Patch

[Adam Barth]

Comment on attachment 159386 [details]
Patch

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

> Source/WebCore/bindings/v8/V8Binding.cpp:385
> +DOMWindow* retrieveWindow(v8::Handle<v8::Context> context)

I might rename retrieveWindow to

DOMWindow* toDOMWindow(v8::Handle<v8::Context>)

> Source/WebCore/bindings/v8/V8Binding.cpp:394
> +Frame* retrieveFrame(v8::Handle<v8::Context> context)

retrieveFrame -> toFrameIfNotDetached ?

It would be nice if callers understood why the conversion might fail.

> Source/WebCore/bindings/v8/V8Binding.cpp:405
> +V8PerContextData* retrievePerContextData(Frame* frame)

retrievePerContextData -> perContextDataForCurrentWorld ?

Really this function should take a Frame and a DOMWrapperWorld, but that's something for dcarney.

> Source/WebCore/bindings/v8/V8Binding.h:375
> +    // Returns the frame object of the window object associated with
> +    // a context.

... if the DOMWindow is currently being displayed in the Frame.

> Source/WebCore/bindings/v8/V8Binding.h:378
> +    // Returns the PerContextData associated with a frame.

... for the current isolated world.

[Adam Barth]

This patch might be of interest to dcanrey.

[Adam Barth]

> This patch might be of interest to dcanrey.

(sorry for typoing your name)

[Kentaro Hara]

Committed r126103: <http://trac.webkit.org/changeset/126103>

[Kentaro Hara]

(In reply to comment #2)
> (From update of attachment 159386 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=159386&action=review
> 
> > Source/WebCore/bindings/v8/V8Binding.cpp:385
> > +DOMWindow* retrieveWindow(v8::Handle<v8::Context> context)
> 
> I might rename retrieveWindow to
> 
> DOMWindow* toDOMWindow(v8::Handle<v8::Context>)
> 
> > Source/WebCore/bindings/v8/V8Binding.cpp:394
> > +Frame* retrieveFrame(v8::Handle<v8::Context> context)
> 
> retrieveFrame -> toFrameIfNotDetached ?
> 
> It would be nice if callers understood why the conversion might fail.
> 
> > Source/WebCore/bindings/v8/V8Binding.cpp:405
> > +V8PerContextData* retrievePerContextData(Frame* frame)
> 
> retrievePerContextData -> perContextDataForCurrentWorld ?
> 
> Really this function should take a Frame and a DOMWrapperWorld, but that's something for dcarney.
> 
> > Source/WebCore/bindings/v8/V8Binding.h:375
> > +    // Returns the frame object of the window object associated with
> > +    // a context.
> 
> ... if the DOMWindow is currently being displayed in the Frame.
> 
> > Source/WebCore/bindings/v8/V8Binding.h:378
> > +    // Returns the PerContextData associated with a frame.
> 
> ... for the current isolated world.

All done. Thanks for reviewing!