On Sat, Sep 11, 2010 at 2:49 AM, Adam Barth <abarth@webkit.org> wrote:
That makes a certain amount of sense.  It seems like we have three
ways for WebCore to talk to Webkit/foo:

1) Client interfaces
2) PlatformBridge
3) Header-in-WebCore, implementation-in-WebKit/foo

Is there some systematic way of determining which way we should use in
a given circumstance?

Good question.

Historically, we started off using #2 exclusively, except for ResourceHandle.  There was probably not a good reason for ResourceHandle being different at the time.

PlatformBridge (previously named ChromiumBridge) was invented to help solve the very problem that led you here.  I wanted us to have all WebCore symbols (except for the ChromiumBridge symbols) defined in WebCore.  This way if you found something implemented in terms of ChromiumBridge functions, you would know to follow the trail to the WebKit layer's ChromiumBridge implementation.  ChromiumBridge was to be the enumeration of the WebCore symbols left undefined by WebCore.

One issue with ChromiumBridge is that people are forced to replicate functions on ChromiumBridge that correspond to other WebCore/platform class methods (e.g., see PlatformScreenChromium.cpp).  This is unfortunate and tedious for people.  The ResourceHandle model was less work to implement and less work to maintain, so that model became popular.  See WebKit/chromium/src/GraphicsContext3D.cpp for example!


 
 If we like pattern (3), maybe we should replace
PlatformBridge with (3)?

Yes, perhaps we should do that.  In concert with that, it would be good to create a subdirectory in WebKit/chromium/src for these WebCore class implementations.

I'm concerned that the PlatformStrategies approach adds too much maintenance overhead given the number of interfaces we'd need to add to it.

I like low cost and easy to maintain solutions that are intuitive for developers.  I think solution #3 as currently implemented suffers from not being intuitive.  If we can fix that, then we should be good.

-Darin

 
 I think there was some talk earlier of
WebKit2 using something called a "strategy," which seemed a bit like
(2), but maybe more modular.

To be clear, this came up in my trying to wrap my mine around the
loader.  I'm not sure there's anything wrong with (3), it just took me
a while to find Chromium's implementation of ResourceHandle because it
wasn't where I expected it to be.

Adam



 


On Fri, Sep 10, 2010 at 11:24 PM, Darin Fisher <darin@chromium.org> wrote:
> We use this pattern a lot.  You can see that ResourceHandle is implemented
> in terms of WebKit API (WebURLLoader).
> The alternative is to introduce extra interfaces between WebCore and WebKit.
>  That has the benefit of isolating WebCore as you say, but it adds the cost
> of more layering and more indirection.  We already have a lot of layering
> and indirection when you consider what lives on the other side of the WebKit
> API (in the Chromium repository).
> One small thing I have wanted to see done is to move all such classes to a
> special subdirectory of WebKit/chromium/src to make it clearer that they are
> in this special category.
> I've noticed that other ports that have their ResourceHandle implementation
> in WebCore/platform resort to using WebKit level classes and interfaces
> directly to achieve a similar effect.  Doing so has similar
> abstraction-breaking issues.
> With the advent of WebKit2, there was a discussion about this topic as well.
>  The desire for WebKit2 to support WebCore being built as a separate DSO was
> one of the primary reasons given for adding the extra layer of indirection
> between WebCore and WebKit for platform level things.  (It has been a
> non-goal of the Chromium port to build WebCore as a separate DSO.)
> There are a lot more interfaces that would be needed besides just
> ResourceHandle{Client}.  At last count it was fairly substantial.  It would
> be good to take stock of the entire set before considering a change.
> -Darin
>
> On Fri, Sep 10, 2010 at 5:41 PM, Adam Barth <abarth@webkit.org> wrote:
>>
>> In investigating the loader, I noticed that Chromium's
>> ResourceHandle.cpp is in WebKit/chromium/src.  ResourceHandle is a
>> WebCore/platform abstract.  Most of the other implementations of
>> ResourceHandle are in
>> WebCore/platform/mumble/ResourceHandleMumble.cpp.
>>
>> Is there a reason why Chromium uses a different design?  Normally, we
>> need a use a virtual function to jump from WebCore to WebKit/chromium.
>>  It looks like we're avoiding that in this case, at the cost of
>> blurring the layer boundaries.
>>
>> Adam
>
>