Bug 73890

Summary: Simplify KURL's checkEncodedString()
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: WebCore Misc.Assignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: kling, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 73915    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch none

Description Benjamin Poulain 2011-12-05 23:11:44 PST 
Simplify KURL's checkEncodedString()
Comment 1 Benjamin Poulain 2011-12-05 23:36:01 PST 
Created attachment 117997 [details]
Patch
Comment 2 Benjamin Poulain 2011-12-05 23:42:45 PST 
Comment on attachment 117997 [details]
Patch

Thanks
Comment 3 WebKit Review Bot 2011-12-06 05:05:01 PST 
Comment on attachment 117997 [details]
Patch

Clearing flags on attachment: 117997

Committed r102123: <http://trac.webkit.org/changeset/102123>
Comment 4 WebKit Review Bot 2011-12-06 05:05:05 PST 
All reviewed patches have been landed.  Closing bug.
Comment 5 Csaba Osztrogonác 2011-12-06 05:43:59 PST 
It made all tests fail in debug mode. (GTK and Qt too)

ASSERTION FAILED: !url.isEmpty() || isSchemeFirstChar(url[0])
../../../../Source/WebCore/platform/KURL.cpp(300) : void WebCore::checkEncodedString(const WTF::String&)

Could you guys fix it as soon as possible?
Comment 6 Andreas Kling 2011-12-06 06:02:27 PST 
Assertion fixed with <http://trac.webkit.org/changeset/102128>
Comment 7 Benjamin Poulain 2011-12-06 09:31:31 PST 
Thanks Andreas and Ossy. I was a bit too fast on that one...
Comment 8 Darin Adler 2011-12-06 09:56:17 PST 
Comment on attachment 117997 [details]
Patch

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

> Source/WebCore/platform/KURL.cpp:300
> +    UNUSED_PARAM(url);
> +    ASSERT(url.containsOnlyASCII());
> +    ASSERT(!url.isEmpty() || isSchemeFirstChar(url[0]));

This should use ASSERT_UNUSED instead of a separate UNUSED_PARAM.

In the future, UNUSED_PARAM will be changed to warn if the parameter is used, so we don’t want to compile it in, in any cases where the parameter is used.
Comment 9 Benjamin Poulain 2011-12-06 10:02:02 PST 
(In reply to comment #8)
> In the future, UNUSED_PARAM will be changed to warn if the parameter is used, so we don’t want to compile it in, in any cases where the parameter is used.

I did not know that is the plan. I will update then.
Comment 10 Benjamin Poulain 2011-12-06 16:15:51 PST 
Reopening to attach new patch.
Comment 11 Benjamin Poulain 2011-12-06 16:15:53 PST 
Created attachment 118135 [details]
Patch
Comment 12 WebKit Review Bot 2011-12-06 20:24:58 PST 
Comment on attachment 118135 [details]
Patch

Clearing flags on attachment: 118135

Committed r102214: <http://trac.webkit.org/changeset/102214>
Comment 13 WebKit Review Bot 2011-12-06 20:25:03 PST 
All reviewed patches have been landed.  Closing bug.