Bug 112965

Summary: Prefer 'KURL(ParsedURLString, String)' when dealing with known-good data.
Product: WebKit Reporter: Mike West <mkwst>
Component: New BugsAssignee: Mike West <mkwst>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, esprehn+autocc, mkwst+watchlist, ojan.autocc, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Mike West
Reported 2013-03-21 14:33:47 PDT
Prefer 'KURL(ParsedURLString, String)' when dealing with known-good data.
Attachments
Patch (5.07 KB, patch)
2013-03-21 14:39 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Patch for landing (5.02 KB, patch)
2013-03-22 01:46 PDT, Mike West 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mike West
Comment 1 2013-03-21 14:39:30 PDT
Created attachment 194340 [details] Patch
Alexey Proskuryakov
Comment 2 2013-03-21 20:00:11 PDT
Comment on attachment 194340 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194340&action=review We still don't have a class that wraps a known good string without KURL size overhead? > Source/WebCore/page/SecurityOrigin.cpp:532 > + return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port))); This isn't normalized to append a slash? Please watch KURL based debug testers. > Source/WebCore/page/SecurityOrigin.cpp:540 > + return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port))); Ditto.
Mike West
Comment 3 2013-03-22 01:46:10 PDT
(In reply to comment #2) > (From update of attachment 194340 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=194340&action=review > > We still don't have a class that wraps a known good string without KURL size overhead? > > > Source/WebCore/page/SecurityOrigin.cpp:532 > > + return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port))); > > This isn't normalized to append a slash? Please watch KURL based debug testers. > > > Source/WebCore/page/SecurityOrigin.cpp:540 > > + return create(KURL(ParsedURLString, protocol + "://" + host + ":" + String::number(port))); > > Ditto. Good eye, thank you. I've added a trailing slash to both, which I think is indeed necessary. I'll CQ the patch, but keep my finger on the revert button if it chokes a bot.
Mike West
Comment 4 2013-03-22 01:46:59 PDT
Created attachment 194471 [details] Patch for landing
WebKit Review Bot
Comment 5 2013-03-22 02:23:13 PDT
Comment on attachment 194471 [details] Patch for landing Clearing flags on attachment: 194471 Committed r146580: <http://trac.webkit.org/changeset/146580>
WebKit Review Bot
Comment 6 2013-03-22 02:23:16 PDT
All reviewed patches have been landed. Closing bug.
Alexey Proskuryakov
Comment 7 2013-04-04 14:33:52 PDT
This has caused bug 113961.
Mike West
Comment 8 2013-04-05 03:04:58 PDT
(In reply to comment #7) > This has caused bug 113961. Thank you for fixing the crash, and apologies that I missed that case. :/
Note You need to log in before you can comment on or make changes to this bug.