[webkit-reviews] review granted: [Bug 94713] [V8] Remove V8Proxy from V8DOMWrapper::instantiateV8Object() : [Attachment 159939] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Kentaro Hara
<haraken at chromium.org>'s request for review:
Bug 94713: [V8] Remove V8Proxy from V8DOMWrapper::instantiateV8Object()
https://bugs.webkit.org/show_bug.cgi?id=94713

Attachment 159939: Patch
https://bugs.webkit.org/attachment.cgi?id=159939&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=159939&action=review


> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:163
> -v8::Local<v8::Object> V8DOMWrapper::instantiateV8Object(V8Proxy* proxy,
WrapperTypeInfo* type, void* impl)
> +v8::Local<v8::Object> V8DOMWrapper::instantiateV8Object(Frame* frame,
WrapperTypeInfo* type, void* impl)

This should actually take a ScriptExecutionContext, but Frame is better than
V8Proxy :)

> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:175
> -		   Frame* frame =
V8DOMWindow::toNative(globalPrototype)->frame();
> -		   if (frame &&
frame->script()->canExecuteScripts(NotAboutToExecuteScript))
> -		       proxy = frame->script()->proxy();
> +		   Frame* globalFrame =
V8DOMWindow::toNative(globalPrototype)->frame();
> +		   if (globalFrame &&
globalFrame->script()->canExecuteScripts(NotAboutToExecuteScript))
> +		       frame = globalFrame;

This code is wrong, but we can fix that in another patch.  Can you file a bug
about V8DOMWrapper::instantiateV8Object needing to take a
ScriptExecutionContext ?