Bug 51385 - Move V8 to common implementation of window.open
Summary: Move V8 to common implementation of window.open
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-12-21 00:58 PST by Adam Barth
Modified: 2010-12-21 19:10 PST (History)
4 users (show)

See Also:

Attachments
Patch (11.43 KB, patch)
2010-12-21 01:00 PST, Adam Barth 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2010-12-21 00:58:09 PST 
Move V8 to common implementation of window.open
Comment 1 Adam Barth 2010-12-21 01:00:44 PST 
Created attachment 77091 [details]
Patch
Comment 2 anton muhin 2010-12-21 05:29:48 PST 
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?
Comment 3 anton muhin 2010-12-21 05:30:05 PST 
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?
Comment 4 Adam Barth 2010-12-21 15:16:02 PST 
> Adam, maybe you can give it a quick informal try just in Inspector?

Will do.
Comment 5 Darin Adler 2010-12-21 15:18:43 PST 
Comment on attachment 77091 [details]
Patch

Looks good to me.
Comment 6 WebKit Commit Bot 2010-12-21 19:10:20 PST 
Comment on attachment 77091 [details]
Patch

Clearing flags on attachment: 77091

Committed r74449: <http://trac.webkit.org/changeset/74449>
Comment 7 WebKit Commit Bot 2010-12-21 19:10:27 PST 
All reviewed patches have been landed.  Closing bug.