[webkit-dev] Cleaning up directories in WebCore

[Adam Barth]

On Fri, Nov 18, 2011 at 2:06 AM, Ryosuke Niwa <rniwa at webkit.org>
wrote:> What's the benefit of this re-org?
The benefit is mostly to improve the hackability of WebKit by grouping
related code together and separating unrelated code.  My hope is that
the code in the "Features" directory will become more self-contained
over time, helping us increase the functionality provided by WebKit
with less implementation complexity.

> Is it really worth the trouble to move these files around? This would mean> that we may need to navigate multiple pages on trac to see revision logs.> Other tools may suffer from such file moves as well. I remember I had to> create new git checkout when Source move happened.
These moves should be less disruptive than the Source move.  The total
volume of code is an order of magnitude less, and the changes can be
made in smaller increments.

> Also, moving inspector, notifications, etc... under Features would mean that> folks working on those components now need to type at last 2-3 more> characters even with tab completion to find their files. Isn't that worse> than the current situation?
I don't see that as a big cost, but perhaps I'm spoiled by using
TextMate and Xcode, which make it easy to navigate files.

> And how would one decide whether a directory belongs to WebCore vs.> WebCore/Features?
One good rule of thumb is if you've creating a bunch of files (i.e., a
decent block code) behind an ENABLE ifdef, then it should probably go
in the Features directory, especially if it corresponds to a
"standalone" specification, e.g., from the WebApps working group.

One of the benefits of this approach is to encourage the folks writing
these blocks of code to make them more self-contained so that the core
part of the engine has fewer ifdefs.

> Though I do agree that moving manual-test out of Source/WebCore seems like a> good idea. It should probably live at trunk/ManualTests.
That sounds like the place we should start.

On Fri, Nov 18, 2011 at 2:17 AM, Kenneth Rohde Christiansen
<kenneth.christiansen at gmail.com> wrote:
> I also agree with moving Platform out. That would make the
> dependencies easier to understand and hopefully avoid more layering
> violations.

That's also the hardest part, which means I'll probably do it last.  :)
On Fri, Nov 18, 2011 at 9:54 AM, Darin Adler <darin at apple.com> wrote:
> On Nov 18, 2011, at 1:51 AM, Adam Barth wrote:
>
>> Features/  <-- New folder for (roughly) self-contained features
>
> I like the concept, but not the name. I’ll try to think on this and suggest a name I like better than “features”.

Yeah, "features" is too general a name.  Here are some more ideas:

- Modules (emphasizing that these are standalone and can be added or
removed from the build)
-

>>  notifications/
>>  storage/
>>  fileapi/
>>  webaudio/
>>  mediastream/
>>  workers/
>>  websockets/
>>  inspector/
>
> Not sure these are all the best names. In some cases we include the “web” prefix from the name of the specification and in other cases we omit it. But maybe these match ENABLE names that are already long-established.

Another option is to align them more closely with the TR name of the
specification they implement.  That would make it easier to understand
the connection with their specifications.

>> manual-tests/  <-- Move outside of Source
>
> Seems clearly right and different from the other suggestions; since these files are not involved in builds I think we can easily do this move sooner than other moves.

Yes.  I'll do this first.

>> dom/
>>  events/  <-- New folder for the 106 files that match dom/*Event*
>
> Seems OK, but sort of cuts against the feature organization. This could separate
>
> What about “device motion”, “device orientation”, “micro data”, “messages”, and “clipboard”? Many of these don’t seem to fit well into the dom directory even though the code is all there.

I was worried that approach would create directories with too few files in them.

>> page/
>>  history/  <-- Moved from top-level
>
> The name history is ambiguous. Is it the back/forward list (called “history” from the point of view of JavaScript, but something we are careful to never call history in WebKit API on Mac at least) or global history (the visited link machinery)?

It's the BackForward list and the PageCache.  It's pretty small.
Maybe we should just merge it with page or name it backforward?

>> platform/  <-- Move to Source/Platform
>
> Good important idea.
>
>> The "html" and "page" directories also contain a bunch of unrelated files (presumably because they're attractive locations for folks to add files).  Once we do the directory moves above, I'll send out another proposal for cleaning up those directories a bit.
>
> I think it’s helpful for at least one or two additional people to review a list of the files to move before we do each of these moves.

That sounds like something that's best done in the bug tracker.  Once
the discussion here as settled down, I'll file bugs about each of
these with a list of files and solicit reviews.

> I think one useful planning tool would be a list of directories and numbers of files in them. Logical organization is important, but size is also relevant to judge the benefits of a new possible organization.

Features/
 notifications/ (8 files)
 storage/ (195 files)
 fileapi/ (115 files)
 webaudio/ (103 files)
 mediastream/ (28 files; under active development)
 workers/ (43 files)
 websockets/ (20 files)
 inspector/ (426 files)
manual-tests/ (580 files)
dom/
 events/ (106 files)
page/
 history/ (21 files)
platform/ (2182 files)

For a sense of scale, WebCore has ~7932 files total, so moving
manual-tests and platform out of WebCore will reduce the total number
of files by 30%.  (That's not quite right because some of the files in
platform will actually end up in WebCore proper because they currently
violate layering rules, but it's a good ballpark number.)

Adam