[webkit-dev] Cleaning up directories in WebCore

[Adam Barth]

On Fri, Nov 18, 2011 at 1:47 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> On Fri, Nov 18, 2011 at 11:21 AM, Adam Barth <abarth at webkit.org> wrote:
>> 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.
>
> Okay. Making each component self-contained by itself seems like a good idea
> but I'm not sure if re-organizing directories and files is the best way to
> accomplish this. For example, each component can still have lots of bad
> dependencies within each file since we don't control header inclusion per
> directory.
>>
>> 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.
>
> Still, if I see a crash in shipped version of Chrome, then I have to
> remember to map between different paths, which will be a huge cognitive
> overhead for the next couple of months if we start the move now. Also, there
> are zillion WebKit crash bugs with stack trace. Those file paths need to be
> moved as well. In addition, all work-in-progress patches and local git
> branches will likely to conflict with these moves.

We move files around all the time.  If we're afraid to move files for
these reasons, we'll be stuck with bad organizational choices we made
in the distant past.  WebKit's philosophy is to think of the future as
large, so the future benefits of these sorts of changes often outweigh
the transition costs.

>> 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.
>
> Hm... what happens if we decide to enable it on all platforms and get rid of
> the ENABLE flag? Should we then move those files to a different directory?

We should keep these files in those directories.  The looser coupling
is valuable even for "always on" features.

>> 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.
>
> I agree this is a good goal. I'm yet convinced that the proposal directory
> reorg would accomplish this though. Perhaps a better way to accomplish this
> is to introduce some sort of header inclusion rules.

I've done some analysis of header inclusion dependencies, and they're
actually pretty good for these directories.  The main issues at the
moment are "hub" classes like Page, Frame, and Document, which need to
be touched by many features.  One of the things I'm planning to do is
remove more ifdefs from these files, which, as a side effect, will
improve the dependencies.

> Having said that, I re-iterate my agreement that moving manual-tests and
> platform out of WebCore would be a clear improvement.

Thanks.

On Fri, Nov 18, 2011 at 2:04 PM, Benjamin Poulain <benjamin at webkit.org> wrote:
> Along that change, what about moving some of the tests to follow the
> same directory structures as the Features/Modules?:
> LayoutTests/Features/notifications, etc
>
> This would make it easier to find a good subset of tests to run while
> developing.

I agree that cleaning up the organization of the LayoutTests would be
valuable.  There's been some discussion of moving the conformance
tests into some recognizable place so folks can know that certain
tests are from, for example, W3C conformance suites.  We also have
this "fast" directory, which is pretty meaningless at this point.

If you're interested in working on this, I'd encourage you look at the
tests we currently have and send a proposal to webkit-dev for how
you'd like to clean them up.

Adam