Bug 38470 – Add first WTFURL file

• Component: New Bugs
• Status: RESOLVED FIXED

[Adam Barth]

Add empty folder to house URL parser

[Adam Barth]

Created attachment 54932 [details]
Patch

[Adam Barth]

I wasn't sure whether to put this at the root level or in the JavaScriptCore/wtf folder.

[Darin Adler]

Comment on attachment 54932 [details]
Patch

Change log looks fine. There's no directory in the patch, but rs=me on adding the directory.

[Adam Barth]

Comment on attachment 54932 [details]
Patch

I landed this patch and then reverted it in r58683 because Darin Adler would prefer that we not create a new top-level directory for this code.

[Darin Adler]

It’s a lot more convenient for Apple if the new code goes in some existing directory and shares in the existing build system in that directory. While I wouldn’t rule out making a new top-level project entirely, I’d prefer that this be somewhere in either JavaScriptCore or WebCore.

[Adam Barth]

Here are some locations that have been floated:

JavaScriptCore/wtf/url
JavaScriptCore/wtf/URLCore
JavaScriptCore/URLCore
WebCore/URLCore

It seems a bit silly to put the code in JavaScriptCore because it has very little to do with JavaScript.  It has more to do with WebCore, so maybe that makes the most sense.  My guess is that fishd's main concern is that the code not accrete dependencies on WebCore.

Perhaps WebCore/URLCore with its own ChangeLog file makes the most sense?  We already have folders in WebCore with special dependency rules (e.g., platform).

[Alexey Proskuryakov]

If it's going to be in WebCore, why not put it in WebCore/platform?

[Adam Barth]

(In reply to comment #7)
> If it's going to be in WebCore, why not put it in WebCore/platform?

You're suggesting WebCore/platform/url ?

Here's some more info from email:

On Mon, May 3, 2010 at 11:41 AM, Darin Adler <[e-mail]> wrote:
>
> On May 3, 2010, at 10:17 AM, Adam Barth wrote:
>
> > How about JavaScriptCore/wtf/url ?
> >
> > I originally used the name "URLCore" to fit in with the rest of the top-level "Core" directories.  It seems a bit odd to have the code inside the JavaScriptCore folder, but that might make sense if/when wtf is moved into its own top-level directory.
>
> Seems OK.
>
> I think the real choice isn’t WebCore vs. JavaScriptCore, but rather WebCore’s platform vs. WTF. Both of those are conceptually separate libraries although they live inside the WebCore and JavaScriptCore directory. At some point I’d like to make those link and compile separately to enforce correct layering, but there are costs associated so I am not in a rush to do that right away.
>
> For Chromium’s sake, I think WTF is right since they are basically treating URL as a low level thing, and probably don’t want to have the networking library depend on the WebCore platform layer.

[Adam Barth]

Created attachment 55000 [details]
Patch

[Maciej Stachowiak]

Comment on attachment 55000 [details]
Patch

JavaScriptCore/wtf/url/src/URLComponent.h:66
 +      int m_length; // Will be -1 if the component is unspecified.
I'm not a fan of in-band signalling like this. It tends to be error-prone. Is the size of this class critical? In any case, I guess it's fine to initially land like this.

r=me

[Adam Barth]

Committed r58740: <http://trac.webkit.org/changeset/58740>

[Darin Fisher (:fishd, Google)]

(In reply to comment #5)
> It’s a lot more convenient for Apple if the new code goes in some existing
> directory and shares in the existing build system in that directory. While I
> wouldn’t rule out making a new top-level project entirely, I’d prefer that this
> be somewhere in either JavaScriptCore or WebCore.

A goal for Chromium at least is to be able to make use of this code from our network stack without having the network stack depend on any other parts of WebKit.  This is why I would prefer a new top-level directory, so the separation is clearer to developers.

[Adam Barth]

The two issues seem somewhat separable:

1) Where the code lives.
2) What the dependency rules are.

My goal is to make the code have as few dependencies as possible.  Actually, I'm aiming for zero (or maybe just ICU), like the current code has.

[Darin Fisher (:fishd, Google)]

(In reply to comment #13)
> The two issues seem somewhat separable:
> 
> 1) Where the code lives.
> 2) What the dependency rules are.
> 
> My goal is to make the code have as few dependencies as possible.  Actually,
> I'm aiming for zero (or maybe just ICU), like the current code has.

OK.  There is a third concern:  how do we keep it that way?

[Adam Barth]

> OK.  There is a third concern:  how do we keep it that way?

The same way we keep most of our invariants: buildbots.

[Adam Barth]

To be more specific, I imagine that the Chromium port will re-map JavaScriptCore/wtf/url to WebKit/chromium/wtfurl in the same way it current maps googleurl into that location.  The main Chromium build will probably re-map it to src/wtfurl (again, where googleurl is currently mapped).  By building the library using wtfurl.gyp, the Chromium buildbots will ensure that its dependencies remain in check.

[Brett Wilson (Google)]

Currently the ICU dependency is in one source file only. It does require the headers to compile. We did this so other projects could use this coded without taking a 10MB hit, by supplying a simple converter file (assuming they don't want IDN). It is used by other projects at Google in this way.

I think we should try to maintain it being separate from ICU in this way when it lives in WebCore as well.

[Darin Fisher (:fishd, Google)]

(In reply to comment #16)
> To be more specific, I imagine that the Chromium port will re-map
> JavaScriptCore/wtf/url to WebKit/chromium/wtfurl in the same way it current
> maps googleurl into that location.  The main Chromium build will probably
> re-map it to src/wtfurl (again, where googleurl is currently mapped).  By
> building the library using wtfurl.gyp, the Chromium buildbots will ensure that
> its dependencies remain in check.

It would be nice to avoid the extra duplication of svn checkouts.  What does re-mapping buy us?  (Maybe we should discuss this in another forum.)