Bug 97405 – Integrate most of GoogleURL in WTFURL

• Component: Web Template Framework
• Status: RESOLVED FIXED

[Benjamin Poulain]

Let's get this project going.

[Benjamin Poulain]

Created attachment 165280 [details]
Patch

[WebKit Review Bot]

Attachment 165280 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xc..." exit_code: 1
Source/WTF/wtf/url/src/URLCanonInternal.cpp:285:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.cpp:302:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:363:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:364:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:368:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:374:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:380:  _strtoui64 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 7 in 35 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Benjamin Poulain]

Created attachment 165281 [details]
Patch

[WebKit Review Bot]

Attachment 165281 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xc..." exit_code: 1
Source/WTF/wtf/url/src/URLCanonInternal.cpp:285:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.cpp:302:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:363:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:364:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:368:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:374:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:380:  _strtoui64 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 7 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 165281 [details]
Patch

Attachment 165281 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13984349

[Benjamin Poulain]

Created attachment 165282 [details]
Patch

[WebKit Review Bot]

Attachment 165282 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xc..." exit_code: 1
Source/WTF/wtf/url/src/URLCanonInternal.cpp:285:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.cpp:302:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:363:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:364:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:368:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:374:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:380:  _strtoui64 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 7 in 34 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Kenneth Rohde Christiansen]

Comment on attachment 165282 [details]
Patch

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

> Source/WTF/wtf/url/api/ParsedURL.cpp:184
> +//    ASSERT_WITH_MESSAGE(!segment.isEmpty(), "A valid URL component should not be empty.");

:-)

> Source/WTF/wtf/url/src/URLCanon.h:540
> +bool ReplaceStandardURL(const char* base,
> +                                 const URLSegments& baseParsed,
> +                                 const Replacements<char>&,
> +                                 CharsetConverter* queryConverter,
> +                                 URLBuffer<char>&,
> +                                 URLSegments* outputParsed);

Shouldn't we put this on one line. That seems to be webkit style

> Source/WTF/wtf/url/src/URLCanonFileurl.cpp:3
> +// Copyright 2007, Google Inc. All rights reserved.
> +// Copyright 2012, Apple Inc. All rights reserved.
> +//

Shouldnt we fix the license style to be like the rest of webkit? like no comma after the year etc.

/*
 *  Copyright (C) 2007 Google Inc. All rights reserved.
 *  Copyright (C) 2012  Apple Inc. All rights reserved.
 *

[Adam Barth]

Ben and I discussed this patch on #webkit for a bit before he posted this patch.  He would like to land this patch, get things wired together, and then work on refactoring the code in place.  This approach isn't the one we typically use when adding code to WebKit, but our thinking is that this code is more like a third-party library like the ones in Source/ThirdParty.

Perhaps we should first land the code in Source/ThirdParty, refactor it there, and then move it to into WebKit proper when its ready?  The downside of that approach is that the build system integration is trickier.

I'm going to rs=me this patch, but please consider whether it would be better to stage the code in Source/ThirdParty for a while.

[Benjamin Poulain]

> Ben and I discussed this patch on #webkit for a bit before he posted this patch.  He would like to land this patch, get things wired together, and then work on refactoring the code in place.  This approach isn't the one we typically use when adding code to WebKit, but our thinking is that this code is more like a third-party library like the ones in Source/ThirdParty.

The problem is this code is in desperate need for cleaning and refactoring. Doing so without relying on the tests is working in the dark.

Every hour I spent renaming stuff would be better spent refactoring the code toward the template version.

All the code is contained in the url/ directory, so it is quite self contained already.

Next, I want to have all the tests passing, add new tests to cover the component parsing. Then eventually refactor everything and fix performance.

[Benjamin Poulain]

Created attachment 165294 [details]
Patch

[Benjamin Poulain]

Same patch, more cleaning.

[WebKit Review Bot]

Attachment 165294 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/WTF.xc..." exit_code: 1
Source/WTF/wtf/url/src/URLCanonInternal.cpp:267:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.cpp:284:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:365:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:366:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:370:  _itoa_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:376:  _itow_s is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Source/WTF/wtf/url/src/URLCanonInternal.h:382:  _strtoui64 is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 7 in 36 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Adam Barth]

Comment on attachment 165294 [details]
Patch

ok

[Benjamin Poulain]

Comment on attachment 165294 [details]
Patch

Clearing flags on attachment: 165294

Committed r129412: <http://trac.webkit.org/changeset/129412>

[Benjamin Poulain]

All reviewed patches have been landed.  Closing bug.