The "WebCore/platform/" directory situation

[David Hyatt]

After working for a while on the WebCore/platform/ directory, it's become clear that people don't really know what this directory is supposed to contain (and by people I mean pretty much everybody, both inside and outside Apple).  The purpose of the platform/ directory is to serve as a foundation library for WebCore.  It provides a widget toolkit, graphics primitives, basic data structures and wrappers around low level OS services.  It should not depend on any other parts of WebCore, but can depend on JavaScriptCore/wtf.

When considering whether or not to put code in platform/, you need to pretend like platform/ is a separate library.  If you find yourself needing files from page/ or rendering/, then you have two options:
(a) Build an abstract interface that lets you communicate with the rest of WebCore (ScrollbarClient, HostWindow are examples of such interfaces)
(b) Put your file in page/ or somewhere else instead.

As the people who built the primary port that has been used as a reference (Windows) by the other ports, those of us at Apple have actually been the worst offenders at violating the layering abstraction!  We plan to start setting a better example here.  After all, if we violate our own layering all over the place in the Mac and Windows ports, then the rest of you will too.

I think for a long time, we've been saying "It's ok, I'll just dump it in here for now and worry about fixing it later," and that needs to stop.  From now on, when reviewing code that is modifying platform/, check for any new abstraction violations, and DO NOT let the person off the hook in a review.  Modifying code that is already in violation seems fine, but don't let people introduce additional new dependencies that will just have to be cut later.

"Other files in platform/ do it." can't be an excuse any more.

In my experience fixing ScrollView, I've found that taking the time to create a clean separation has actually resulted in a net drop in lines of code, fewer methods bridging over to WebKit, and more cross-platform sharing (and a whole lot of build bustage, but shhhh, let's forget about that part).  There are other files that would similarly benefit from such a refactoring.

In some cases the files are probably just misplaced and should be moved to page/.

I am going to start filing bugs on these layering violations.  I encourage others to get involved with looking for problems in platform/ and filing bugs on these issues.

https://bugs.webkit.org/show_bug.cgi?id=21354 is a tracking bug for platform/ problems.  You can add any bugs you file as blocking it.

I also think it would be worthwhile to discuss options for preventing these layering violations from occurring going forward.  We need to make these violations impossible.  I'd love to hear suggestions on that front (separate library, hacked include paths, etc.).  Whatever we decide should be implementable by Mac, Win, Gtk, Qt, wx and Chromium, since we don't want platform-specific code in platform to violate layering either.

Thanks,

dave

(hyatt@...)

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Maciej Stachowiak]

On Oct 3, 2008, at 9:17 PM, David Hyatt wrote:

> After working for a while on the WebCore/platform/ directory, it's  
> become clear that people don't really know what this directory is  
> supposed to contain (and by people I mean pretty much everybody,  
> both inside and outside Apple).  The purpose of the platform/  
> directory is to serve as a foundation library for WebCore.  It  
> provides a widget toolkit, graphics primitives, basic data  
> structures and wrappers around low level OS services.  It should not  
> depend on any other parts of WebCore, but can depend on  
> JavaScriptCore/wtf.

I agree with Hyatt on this. Although in most respects we don't enforce  
strict modularity in WebCore, this is one case where it is important,  
and we have been lax in maintaining the layering of WebCore/platform.

> I also think it would be worthwhile to discuss options for  
> preventing these layering violations from occurring going forward.  
> We need to make these violations impossible.  I'd love to hear  
> suggestions on that front (separate library, hacked include paths,  
> etc.).  Whatever we decide should be implementable by Mac, Win, Gtk,  
> Qt, wx and Chromium, since we don't want platform-specific code in  
> platform to violate layering either.

I think we should pull WTF and Platform into separate top level  
modules. On Mac OS X they would build static libraries that get linked  
into JavaScriptCore and WebCore respectively. Other ports may want to  
do it slightly differnetly.

We have a lot fewer layering problems with WebCore and WebKit in part  
because they are separate modules, and you have to go out of your way  
and make some Client interfaces to create unfortunate entanglements.

Regards,
Maciej

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Darin Fisher-3]

On Sat, Oct 4, 2008 at 12:27 AM, Maciej Stachowiak <mjs@...> wrote:

On Oct 3, 2008, at 9:17 PM, David Hyatt wrote:

> After working for a while on the WebCore/platform/ directory, it's
> become clear that people don't really know what this directory is
> supposed to contain (and by people I mean pretty much everybody,
> both inside and outside Apple).  The purpose of the platform/
> directory is to serve as a foundation library for WebCore.  It
> provides a widget toolkit, graphics primitives, basic data
> structures and wrappers around low level OS services.  It should not
> depend on any other parts of WebCore, but can depend on
> JavaScriptCore/wtf.

I agree with Hyatt on this. Although in most respects we don't enforce
strict modularity in WebCore, this is one case where it is important,
and we have been lax in maintaining the layering of WebCore/platform.

> I also think it would be worthwhile to discuss options for
> preventing these layering violations from occurring going forward.
> We need to make these violations impossible.  I'd love to hear
> suggestions on that front (separate library, hacked include paths,
> etc.).  Whatever we decide should be implementable by Mac, Win, Gtk,
> Qt, wx and Chromium, since we don't want platform-specific code in
> platform to violate layering either.

I think we should pull WTF and Platform into separate top level
modules. On Mac OS X they would build static libraries that get linked
into JavaScriptCore and WebCore respectively. Other ports may want to
do it slightly differnetly.

We have a lot fewer layering problems with WebCore and WebKit in part
because they are separate modules, and you have to go out of your way
and make some Client interfaces to create unfortunate entanglements.

Regards,
Maciej

Or at least, compile platform/ with an include path that only has wtf/ and subdirectories of platform/.  Separate static libraries might be the easiest way to accomplish that?

-Darin

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[David Hyatt]

This sounds good to me, although I'm not sure the files in platform/  
need to move.  I think the directory is fine under WebCore/ still.

dave

On Oct 4, 2008, at 2:56 AM, Eric Seidel wrote:

> +1 for making wtf and platform their own projects/targets.
>
> -eric
>
> On Sat, Oct 4, 2008 at 12:27 AM, Maciej Stachowiak <mjs@...>  
> wrote:
>>
>> On Oct 3, 2008, at 9:17 PM, David Hyatt wrote:
>>
>>> After working for a while on the WebCore/platform/ directory, it's
>>> become clear that people don't really know what this directory is
>>> supposed to contain (and by people I mean pretty much everybody,
>>> both inside and outside Apple).  The purpose of the platform/
>>> directory is to serve as a foundation library for WebCore.  It
>>> provides a widget toolkit, graphics primitives, basic data
>>> structures and wrappers around low level OS services.  It should not
>>> depend on any other parts of WebCore, but can depend on
>>> JavaScriptCore/wtf.
>>
>> I agree with Hyatt on this. Although in most respects we don't  
>> enforce
>> strict modularity in WebCore, this is one case where it is important,
>> and we have been lax in maintaining the layering of WebCore/platform.
>>
>>> I also think it would be worthwhile to discuss options for
>>> preventing these layering violations from occurring going forward.
>>> We need to make these violations impossible.  I'd love to hear
>>> suggestions on that front (separate library, hacked include paths,
>>> etc.).  Whatever we decide should be implementable by Mac, Win, Gtk,
>>> Qt, wx and Chromium, since we don't want platform-specific code in
>>> platform to violate layering either.
>>
>> I think we should pull WTF and Platform into separate top level
>> modules. On Mac OS X they would build static libraries that get  
>> linked
>> into JavaScriptCore and WebCore respectively. Other ports may want to
>> do it slightly differnetly.
>>
>> We have a lot fewer layering problems with WebCore and WebKit in part
>> because they are separate modules, and you have to go out of your way
>> and make some Client interfaces to create unfortunate entanglements.
>>
>> Regards,
>> Maciej
>>
>> _______________________________________________
>> webkit-dev mailing list
>> webkit-dev@...
>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Maciej Stachowiak]

On Oct 4, 2008, at 2:23 AM, David Hyatt wrote:

> This sounds good to me, although I'm not sure the files in platform/  
> need to move.  I think the directory is fine under WebCore/ still.

I could go either way, but I kind of like moving the platform  
directory to a top level module because that makes it more crystal  
clear that you shouldn't have inverse dependencies.

  - Maicje

>
>
> dave
>
> On Oct 4, 2008, at 2:56 AM, Eric Seidel wrote:
>
>> +1 for making wtf and platform their own projects/targets.
>>
>> -eric
>>
>> On Sat, Oct 4, 2008 at 12:27 AM, Maciej Stachowiak <mjs@...>  
>> wrote:
>>>
>>> On Oct 3, 2008, at 9:17 PM, David Hyatt wrote:
>>>
>>>> After working for a while on the WebCore/platform/ directory, it's
>>>> become clear that people don't really know what this directory is
>>>> supposed to contain (and by people I mean pretty much everybody,
>>>> both inside and outside Apple).  The purpose of the platform/
>>>> directory is to serve as a foundation library for WebCore.  It
>>>> provides a widget toolkit, graphics primitives, basic data
>>>> structures and wrappers around low level OS services.  It should  
>>>> not
>>>> depend on any other parts of WebCore, but can depend on
>>>> JavaScriptCore/wtf.
>>>
>>> I agree with Hyatt on this. Although in most respects we don't  
>>> enforce
>>> strict modularity in WebCore, this is one case where it is  
>>> important,
>>> and we have been lax in maintaining the layering of WebCore/
>>> platform.
>>>
>>>> I also think it would be worthwhile to discuss options for
>>>> preventing these layering violations from occurring going forward.
>>>> We need to make these violations impossible.  I'd love to hear
>>>> suggestions on that front (separate library, hacked include paths,
>>>> etc.).  Whatever we decide should be implementable by Mac, Win,  
>>>> Gtk,
>>>> Qt, wx and Chromium, since we don't want platform-specific code in
>>>> platform to violate layering either.
>>>
>>> I think we should pull WTF and Platform into separate top level
>>> modules. On Mac OS X they would build static libraries that get  
>>> linked
>>> into JavaScriptCore and WebCore respectively. Other ports may want  
>>> to
>>> do it slightly differnetly.
>>>
>>> We have a lot fewer layering problems with WebCore and WebKit in  
>>> part
>>> because they are separate modules, and you have to go out of your  
>>> way
>>> and make some Client interfaces to create unfortunate entanglements.
>>>
>>> Regards,
>>> Maciej
>>>
>>> _______________________________________________
>>> webkit-dev mailing list
>>> webkit-dev@...
>>> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>>>
>

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

[Mark Rowe-5]

On Oct 4, 2008, at 12:27 AM, Maciej Stachowiak wrote:

> We have a lot fewer layering problems with WebCore and WebKit in part
> because they are separate modules, and you have to go out of your way
> and make some Client interfaces to create unfortunate entanglements.

Some ports do have a problem with this separation too.  Mac and  
Windows enforce the separation between WebCore and WebKit by building  
each as separate libraries, but some of the other ports build WebCore  
and WebKit into a single module and have been using WebKit-level  
classes inside WebCore.

- Mark

_______________________________________________
webkit-dev mailing list
webkit-dev@...
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev