Bug 51385 – Move V8 to common implementation of window.open

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Move V8 to common implementation of window.open

[Adam Barth]

Created attachment 77091 [details]
Patch

[anton muhin]

Comment on attachment 77091 [details]
Patch

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

> WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:519
> +    EXCEPTION_BLOCK(String, urlString, toWebCoreStringWithNullOrUndefinedCheck(args[0]));

I am not sure EXCEPTION_BLOCK does what it promises in the case of string conversion: if memory serves, our functions do eat thrown exceptions.  If it's indeed the case, I would recommend using STRING_TO_V8PARAMETER_EXCEPTION_BLOCK.

Adam, maybe you can give it a quick informal try just in Inspector?

[anton muhin]

And overall LGTM

(In reply to comment #2)
> (From update of attachment 77091 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=77091&action=review
> 
> > WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp:519
> > +    EXCEPTION_BLOCK(String, urlString, toWebCoreStringWithNullOrUndefinedCheck(args[0]));
> 
> I am not sure EXCEPTION_BLOCK does what it promises in the case of string conversion: if memory serves, our functions do eat thrown exceptions.  If it's indeed the case, I would recommend using STRING_TO_V8PARAMETER_EXCEPTION_BLOCK.
> 
> Adam, maybe you can give it a quick informal try just in Inspector?

[Adam Barth]

> Adam, maybe you can give it a quick informal try just in Inspector?

Will do.

[Darin Adler]

Comment on attachment 77091 [details]
Patch

Looks good to me.

[WebKit Commit Bot]

Comment on attachment 77091 [details]
Patch

Clearing flags on attachment: 77091

Committed r74449: <http://trac.webkit.org/changeset/74449>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.