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

Kentaro Hara

Reported 2012-08-20 02:48:11 PDT

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

Attachments
Patch (16.48 KB, patch) 2012-08-20 03:26 PDT, Kentaro Hara	no flags	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Kentaro Hara

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

Created attachment 159386 [details] Patch

Adam Barth

Comment 2 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.

Adam Barth

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

This patch might be of interest to dcanrey.

Adam Barth

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

> This patch might be of interest to dcanrey. (sorry for typoing your name)

Kentaro Hara

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

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

Kentaro Hara

Comment 6 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!

Note You need to log in before you can comment on or make changes to this bug.