Bug 94460

Summary:

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

Product:

WebKit

Reporter:

Kentaro Hara <haraken>

Component:

WebCore JavaScript

Assignee:

Kentaro Hara <haraken>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, dcarney, japhet, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

85330

Attachments:

Description	Flags
Patch	none

Description Kentaro Hara 2012-08-20 02:48:11 PDT

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

Comment 1 Kentaro Hara 2012-08-20 03:26:13 PDT

Created attachment 159386 [details]
Patch

Comment 2 Adam Barth 2012-08-20 11:35:42 PDT

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.

Comment 3 Adam Barth 2012-08-20 11:36:39 PDT

This patch might be of interest to dcanrey.

Comment 4 Adam Barth 2012-08-20 11:37:05 PDT

> This patch might be of interest to dcanrey.

(sorry for typoing your name)

Comment 5 Kentaro Hara 2012-08-20 17:31:35 PDT

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

Comment 6 Kentaro Hara 2012-08-20 17:33:02 PDT

(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!