Bug 38633 – [WTFURL] Add Chromium front-end to the URL parser

• Component: New Bugs
• Status: RESOLVED WONTFIX

[Adam Barth]

[WTFURL] Add Chromium front-end to the URL parser

[Adam Barth]

Created attachment 55218 [details]
Patch

[Adam Barth]

It's probably most productive for fishd to review this patch.

[WebKit Review Bot]

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

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
Last 3072 characters of output:
romium/parser.h:34:  Alphabetical sorting problem.  [build/include_order] [4]
JavaScriptCore/wtf/url/chromium/parser.h:57:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:58:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:64:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:65:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:69:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:70:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:73:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:74:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:98:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:99:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:112:  port_num is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:118:  port_num is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:141:  file_name is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:144:  file_name is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:192:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:193:  after_scheme is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:196:  spec_len is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:197:  after_scheme is incorrectly named. Don't use underscores in your identifier names.  [readability/naming] [4]
JavaScriptCore/wtf/url/chromium/parser.h:200:  One space before end of line comments  [whitespace/comments] [5]
JavaScriptCore/wtf/url/chromium/parser.h:202:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 22 in 3 files


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

[Darin Fisher (:fishd, Google)]

It seems like this should live in the Chromium repository since it is not used by anything in WebKit.  It will also be easier to maintain in the Chromium repository since it uses Google C++ style, and it also digs into Chromium's base/ directory for headers and types.  I really like to avoid dependencies back onto the Chromium repository since it makes it hard to change things in Chromium.

What is the advantage to having this live in svn.webkit.org?

[Adam Barth]

Actually, we don't need the dependency on base.  I forgot to remove the include.  Discussing this patch with fishd on IRC.

[Adam Barth]

Comment on attachment 55218 [details]
Patch

I spoke to fishd on the phone.  We're going to do this in two layers:

1) A stable opaque interface in WebKit style in svn.webkit.org.
2) A re-syndication of that interface in Chromium style in svn.chromium.org.

By using inline functions, there shouldn't be any runtime impact of using two layers.  There's just a bit of complexity, but we get a clear definition of the dependencies in return.

[Brett Wilson (Google)]

Thanks for the explanation in the other bug of the header file stuff. But won't we still get two copies of the code in the inlined templatized header file, one for this Chromium interface and one for the WebKit one?

[Adam Barth]

(In reply to comment #7)
> Thanks for the explanation in the other bug of the header file stuff. But won't
> we still get two copies of the code in the inlined templatized header file, one
> for this Chromium interface and one for the WebKit one?

I'm not sure how to avoid that given that the two want to instantiate the templates with different types (i.e., UChar versus char16).  These are both the same at the binary level, so maybe there's something nice we can do.  I need to see how this works today and see if we can replicate that.

[Stephen Chenney]

LayoutTest failures for Chromium are being marked WontFix. The Bug is still accessible and referenced from TestExpectations.