Bug 99483 – Evaluate whether Chromium can use PlatformStrategies like all the other ports

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[WebKit Review Bot]

Evaluate whether Chromium can use PlatformStrategies like all the other ports
Requested by abarth on #webkit.

[Adam Barth]

I forget who asked me this question, but I think it was beidson.

The short answer is "yes", but PlatformStrategies seem to contain layering violations currently.  For example, PlatformStrategies.h is in WebCore/platform, but it mentions PluginStrategy, which is in WebCore/plugins.

The long answer is that PlatformStrategies would introduce an extra layer of indirection that isn't necessary for Chromium.  PlatformStrategies is playing a similar role to

http://trac.webkit.org/browser/trunk/Source/Platform/chromium/public/Platform.h

The main difference is that we jump directly to the embedder rather than thunking though the WebKit/WebKit2 layer.

[Brady Eidson]

(In reply to comment #1)
> I forget who asked me this question, but I think it was beidson.
> 
> The short answer is "yes", but PlatformStrategies seem to contain layering violations currently.  For example, PlatformStrategies.h is in WebCore/platform, but it mentions PluginStrategy, which is in WebCore/plugins.

This "layering" violation seems tenuous, as all of WebCore is generally considered to be the same layer.  Are you suggesting that anywhere else we "cross directories" within WebCore we're committing a layering violation?

> 
> The long answer is that PlatformStrategies would introduce an extra layer of indirection that isn't necessary for Chromium.  PlatformStrategies is playing a similar role to
> 
> http://trac.webkit.org/browser/trunk/Source/Platform/chromium/public/Platform.h

How exactly does the chromium "Platform" class get instantiated and plugged in to WebCore?
 
> The main difference is that we jump directly to the embedder rather than thunking though the WebKit/WebKit2 layer.

On the surface it seems to me like "class Platform" from the above header is a pseudo-WebKit port in and of itself.

Do you have any alternate suggestions for how "All of the WebKit ports besides Chromium" and "Chromium" might adopt the same mechanism for what PlatformStrategies currently accomplishes?

As it is there are a few scattered "#if USE(PLATFORM_STRATEGIES)" blocks that basically exist to support Chromium, but Alexey and I are each working on two separate projects that threaten to add more.

[Adam Barth]

> Are you suggesting that anywhere else we "cross directories" within WebCore we're committing a layering violation?

Nope.  WebCore/platform is special in that sense.  Both ap and sam have r-'ed patches of mine for such layering violations.

> How exactly does the chromium "Platform" class get instantiated and plugged in to WebCore?

The embedder supplies an implementation of Platform when initializing WebKit:

http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/public/WebKit.h#L45

(Minor detail: Actually the embedder supplies an implementation of WebKitPlatformSupport, which is a subclass of Platform.  We're in the process of deleting WebKitPlatformSupport, and once we're done the embedder will supply a WebKit::Platform object directly.)

The implementation gets stored in a static, similar to teh s_platformStrategies in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/PlatformStrategies.cpp#L34

> > The main difference is that we jump directly to the embedder rather than thunking though the WebKit/WebKit2 layer.
> 
> On the surface it seems to me like "class Platform" from the above header is a pseudo-WebKit port in and of itself.

That is not the case.

> Do you have any alternate suggestions for how "All of the WebKit ports besides Chromium" and "Chromium" might adopt the same mechanism for what PlatformStrategies currently accomplishes?

Nope.  The requirements are different because WebKit2 needs to do extra work that would just be thunks to the Platform API in Chromium.

> As it is there are a few scattered "#if USE(PLATFORM_STRATEGIES)" blocks that basically exist to support Chromium, but Alexey and I are each working on two separate projects that threaten to add more.

It would be nice to remove those ifdefs, but I don't see a way to do it without introducing an extra layer of indirection for Chromium.

I'd have to look at exactly what you're doing, but I suspect you're introducing an extra layer of indirection for WebKit1 as well.  Perhaps that's less important to you?  The root issue is that Chromium does not wish to adopt WebKit2 at this time.

[Brady Eidson]

(In reply to comment #3)
> > Are you suggesting that anywhere else we "cross directories" within WebCore we're committing a layering violation?
> 
> Nope.  WebCore/platform is special in that sense.  Both ap and sam have r-'ed patches of mine for such layering violations.

Discussed this with Alexey on IRC.  It's - amazingly - the first I've heard of this rule.  I wonder if it's documented anywhere.

Despite the countless violations of it that are in WebCore/platform, it does make sense.

> > How exactly does the chromium "Platform" class get instantiated and plugged in to WebCore?
> 
> The embedder supplies an implementation of Platform when initializing WebKit:
> 
> http://trac.webkit.org/browser/trunk/Source/WebKit/chromium/public/WebKit.h#L45
> 
> (Minor detail: Actually the embedder supplies an implementation of WebKitPlatformSupport, which is a subclass of Platform.  We're in the process of deleting WebKitPlatformSupport, and once we're done the embedder will supply a WebKit::Platform object directly.)
> 
> The implementation gets stored in a static, similar to teh s_platformStrategies in http://trac.webkit.org/browser/trunk/Source/WebCore/platform/PlatformStrategies.cpp#L34

So it sounds like the embedder provides a class that implements these necessary functions for Chromium WebCore's sake...

> > Do you have any alternate suggestions for how "All of the WebKit ports besides Chromium" and "Chromium" might adopt the same mechanism for what PlatformStrategies currently accomplishes?
> 
> Nope.  The requirements are different because WebKit2 needs to do extra work that would just be thunks to the Platform API in Chromium.

...but I must be confused, because it sounds like what the other ports do with it could easily be precisely what WebKitPlatformSupport does...?

Also, I think you mischaracterize this as being about WebKit2.  It's about the WebCore porting layer that makes the WebKit project possible.  It is used in at least 5 WebKit1 ports, including all of the top-tier ports that have build bots.  Except Chromium.

> > As it is there are a few scattered "#if USE(PLATFORM_STRATEGIES)" blocks that basically exist to support Chromium, but Alexey and I are each working on two separate projects that threaten to add more.
> 
> It would be nice to remove those ifdefs, but I don't see a way to do it without introducing an extra layer of indirection for Chromium.

What is the concern about this layer of indirection?  The only one I can think of is a speed cost which seems like it could be negated by implementing Chromium's strategies in 100% inlined headers.

Am I missing something else obvious?

> I'd have to look at exactly what you're doing, but I suspect you're introducing an extra layer of indirection for WebKit1 as well.  Perhaps that's less important to you? 

The PlatformStrategies layer of customization already exists for EVERY WebKit port inside Source/WebKit that maintains a build bot...  exception Chromium.
It also exists for Source/WebKit2, but for the sake of this discussion WebKit2 is "just another WebKit port"

> The root issue is that Chromium does not wish to adopt WebKit2 at this time.

As previously stated, this is not specifically about WebKit2.

[Adam Barth]

> So it sounds like the embedder provides a class that implements these necessary functions for Chromium WebCore's sake...

Yes.

> > > Do you have any alternate suggestions for how "All of the WebKit ports besides Chromium" and "Chromium" might adopt the same mechanism for what PlatformStrategies currently accomplishes?
> > 
> > Nope.  The requirements are different because WebKit2 needs to do extra work that would just be thunks to the Platform API in Chromium.
> 
> ...but I must be confused, because it sounds like what the other ports do with it could easily be precisely what WebKitPlatformSupport does...?

The difference is that WebKit::Platform is a public API for Chromium and PlatformStrategies is not.  In Chromium, WebCore talks directly to the public API rather than indirecting through the WebKit layer.  The WebKit layer doesn't add any value for Chromium, unlike in WebKit2 where WebKit2 does the process-hopping work.

> Also, I think you mischaracterize this as being about WebKit2.  It's about the WebCore porting layer that makes the WebKit project possible.  It is used in at least 5 WebKit1 ports, including all of the top-tier ports that have build bots.  Except Chromium.

Let's take VisitedLinkStrategy::isLinkVisited as an example.  Here's the WebKit1 implementation of VisitedLinkStrategy:isLinkVisited on apple-mac:

bool WebPlatformStrategies::isLinkVisited(Page* page, LinkHash hash, const KURL&, const AtomicString&)
{
    return page->group().isLinkVisited(hash);
}

Notice that from WebKit1's perspective VisitedLinkStrategy::isLinkVisited is just unnecessary indirection.  The callsite for this function is in SelectionChecker.cpp:

#if USE(PLATFORM_STRATEGIES)
    return platformStrategies()->visitedLinkStrategy()->isLinkVisited(page, hash, m_document->baseURL(), *attribute) ? InsideVisitedLink : InsideUnvisitedLink;
#else
    return page->group().isLinkVisited(hash) ? InsideVisitedLink : InsideUnvisitedLink;
#endif

Without USE(PLATFORM_STRATEGIES) defined, we just skip the extra indirection through the WebKit layer.
 
> > > As it is there are a few scattered "#if USE(PLATFORM_STRATEGIES)" blocks that basically exist to support Chromium, but Alexey and I are each working on two separate projects that threaten to add more.
> > 
> > It would be nice to remove those ifdefs, but I don't see a way to do it without introducing an extra layer of indirection for Chromium.
> 
> What is the concern about this layer of indirection?  The only one I can think of is a speed cost which seems like it could be negated by implementing Chromium's strategies in 100% inlined headers.

That's not possible given that these are virtual functions.

> Am I missing something else obvious?

Consider how Chromium implements isLinkVisited.  There's a port-independent header file called VisitedLinks.h.  VisitedLinksChromium.cpp implements that header and calls the API directly:

bool VisitedLinks::isLinkVisited(LinkHash visitedLinkHash)
{
    return WebKit::Platform::current()->isLinkVisited(visitedLinkHash);
}

That's one virtual function call (to reach the embedder) rather than two virtual function calls (one to reach the WebKit-layer and then another to reach the embedder).

> > I'd have to look at exactly what you're doing, but I suspect you're introducing an extra layer of indirection for WebKit1 as well.  Perhaps that's less important to you? 
> 
> The PlatformStrategies layer of customization already exists for EVERY WebKit port inside Source/WebKit that maintains a build bot...  exception Chromium.

As far as I can tell, PlatformStrategies exists for the benefit of WebKit2 at the expense of WebKit1.  That makes sense for a port like apple-mac that is focused on WebKit2, but Chromium does not wish to adopt WebKit2 at this time and wishes to continue using WebKit1.

I'm certainly willing to believe that isLinkVisited is a bad example.  Is there a better example I should look at instead?

> > The root issue is that Chromium does not wish to adopt WebKit2 at this time.
> 
> As previously stated, this is not specifically about WebKit2.

That doesn't match my reading of the code.  What value does VisitedLinkStrategy::isLinkVisited provide to WebKit1 on apple-mac that doesn't involve some reference to WebKit2?  As far as I can tell, it just introduces an extra level of indirection.

[Sam Weinig]

(In reply to comment #5)
> > So it sounds like the embedder provides a class that implements these necessary functions for Chromium WebCore's sake...
> 
> Yes.
> 
> > > > Do you have any alternate suggestions for how "All of the WebKit ports besides Chromium" and "Chromium" might adopt the same mechanism for what PlatformStrategies currently accomplishes?
> > > 
> > > Nope.  The requirements are different because WebKit2 needs to do extra work that would just be thunks to the Platform API in Chromium.
> > 
> > ...but I must be confused, because it sounds like what the other ports do with it could easily be precisely what WebKitPlatformSupport does...?
> 
> The difference is that WebKit::Platform is a public API for Chromium and PlatformStrategies is not.  In Chromium, WebCore talks directly to the public API rather than indirecting through the WebKit layer.  The WebKit layer doesn't add any value for Chromium, unlike in WebKit2 where WebKit2 does the process-hopping work.

This is unfortunate, and I hope you reconsider this for the sake of the other ports and being WebKit citizen.

> 
> > Also, I think you mischaracterize this as being about WebKit2.  It's about the WebCore porting layer that makes the WebKit project possible.  It is used in at least 5 WebKit1 ports, including all of the top-tier ports that have build bots.  Except Chromium.
> 
> Let's take VisitedLinkStrategy::isLinkVisited as an example.  Here's the WebKit1 implementation of VisitedLinkStrategy:isLinkVisited on apple-mac:
> 
> bool WebPlatformStrategies::isLinkVisited(Page* page, LinkHash hash, const KURL&, const AtomicString&)
> {
>     return page->group().isLinkVisited(hash);
> }
> 
> Notice that from WebKit1's perspective VisitedLinkStrategy::isLinkVisited is just unnecessary indirection.  The callsite for this function is in SelectionChecker.cpp:
> 
> #if USE(PLATFORM_STRATEGIES)
>     return platformStrategies()->visitedLinkStrategy()->isLinkVisited(page, hash, m_document->baseURL(), *attribute) ? InsideVisitedLink : InsideUnvisitedLink;
> #else
>     return page->group().isLinkVisited(hash) ? InsideVisitedLink : InsideUnvisitedLink;
> #endif
> 
> Without USE(PLATFORM_STRATEGIES) defined, we just skip the extra indirection through the WebKit layer.

You are right, this abstraction exists to accommodate WebCore being able to exist in a world with more than one WebKit layer (WebKit1 and WebKit2 in this case).  That said, I have tried hard to only use this where I did not believe it would have significant performance impact.  If the performance is critical there, we should probably fix it for all ports, not just for yours.

> 
> > > > As it is there are a few scattered "#if USE(PLATFORM_STRATEGIES)" blocks that basically exist to support Chromium, but Alexey and I are each working on two separate projects that threaten to add more.
> > > 
> > > It would be nice to remove those ifdefs, but I don't see a way to do it without introducing an extra layer of indirection for Chromium.
> > 
> > What is the concern about this layer of indirection?  The only one I can think of is a speed cost which seems like it could be negated by implementing Chromium's strategies in 100% inlined headers.
> 
> That's not possible given that these are virtual functions.
> 
> > Am I missing something else obvious?
> 
> Consider how Chromium implements isLinkVisited.  There's a port-independent header file called VisitedLinks.h.  VisitedLinksChromium.cpp implements that header and calls the API directly:
> 
> bool VisitedLinks::isLinkVisited(LinkHash visitedLinkHash)
> {
>     return WebKit::Platform::current()->isLinkVisited(visitedLinkHash);
> }
> 
> That's one virtual function call (to reach the embedder) rather than two virtual function calls (one to reach the WebKit-layer and then another to reach the embedder).
> 
> > > I'd have to look at exactly what you're doing, but I suspect you're introducing an extra layer of indirection for WebKit1 as well.  Perhaps that's less important to you? 
> > 
> > The PlatformStrategies layer of customization already exists for EVERY WebKit port inside Source/WebKit that maintains a build bot...  exception Chromium.
> 
> As far as I can tell, PlatformStrategies exists for the benefit of WebKit2 at the expense of WebKit1.  That makes sense for a port like apple-mac that is focused on WebKit2, but Chromium does not wish to adopt WebKit2 at this time and wishes to continue using WebKit1.
> 
> I'm certainly willing to believe that isLinkVisited is a bad example.  Is there a better example I should look at instead?
> 
> > > The root issue is that Chromium does not wish to adopt WebKit2 at this time.
> > 
> > As previously stated, this is not specifically about WebKit2.
> 
> That doesn't match my reading of the code.  What value does VisitedLinkStrategy::isLinkVisited provide to WebKit1 on apple-mac that doesn't involve some reference to WebKit2?  As far as I can tell, it just introduces an extra level of indirection.

Indeed, there a many places where we abstract to allow for multiple ports but keep a clean WebCore (all of the clients, ChromeClient, FrameLoaderClient for example).  The intent of PlatformStrategies is to extend that model to places where there is no Page to hang the clients off of.  By not adopting it, you are making the code a more #ifdeffy place, which is a bit sad.

[Adam Barth]

Created attachment 170058 [details]
An approach without extra virtual functin calls or ifdefs

[Adam Barth]

> This is unfortunate, and I hope you reconsider this for the sake of the other ports and being WebKit citizen.

I'd rather find a solution that doesn't involving adding extra virtual function calls to the Chromium port.  Attachment 170058 [details] gives an example of how we might achieve this, at least for this particular case.  In that patch, Chromium is still works differently from the WebKit2 ports, but we're not going to be able to change that without either introducing extra indirection or having Chromium adopt WebKit2.

[Sam Weinig]

(In reply to comment #8)
> > This is unfortunate, and I hope you reconsider this for the sake of the other ports and being WebKit citizen.
> 
> I'd rather find a solution that doesn't involving adding extra virtual function calls to the Chromium port.  Attachment 170058 [details] gives an example of how we might achieve this, at least for this particular case.  In that patch, Chromium is still works differently from the WebKit2 ports, but we're not going to be able to change that without either introducing extra indirection or having Chromium adopt WebKit2.

I am ok with an approach that doesn't require a virtual function for ports that don't need to support two WebKits, but I still don't understand why you feel this is so performance critical.

[Adam Barth]

> I am ok with an approach that doesn't require a virtual function for ports that don't need to support two WebKits, but I still don't understand why you feel this is so performance critical.

As Maciej wrote yesterday:

[[
Just as for behavior-affecting changes it is the patch submitter's job to provide evidence that the patch is correct, with performance-affecting patches it is the patch submitter's job to provide evidence that the patch improves performance as intended and does not regress things that it might be feared to regress.
]]

https://bugs.webkit.org/show_bug.cgi?id=100057#c6

In a broader context, this topic was already discussed in detail in June 2010 in this thread on webkit-dev:

http://lists.webkit.org/pipermail/webkit-dev/2010-June/013270.html

In that thread, Darin Fisher wrote:

[[
It's not so much about performance.  There's a cognitive cost to having so many layers.  But, given (c) above, I understand that you really don't have a choice.  In contrast, the Chromium project doesn't build WebCore as a standalone DLL, so we don't have the same constraint.  I don't have a problem replacing ChromiumBridge / PlatformBridge with a virtual table (or a set of virtual tables) if that means that we can share more code with other ports.
]]

What's changed since then is that we've deleted PlatformBridge from the Chromium port and removed this cognitive cost.  That was a fair amount of work because we needed to factor the Chromium WebKit API into two pieces, the Client API and the Platform API, as discussed on webkit-dev:

http://lists.webkit.org/pipermail/webkit-dev/2012-March/020166.html

Re-introducing PlatformBridge into the Chromium port renamed to PlatformStrategies and with a slower implementation is a net loss.

[Maciej Stachowiak]

(In reply to comment #2)
> (In reply to comment #1)
> > I forget who asked me this question, but I think it was beidson.
> > 
> > The short answer is "yes", but PlatformStrategies seem to contain layering violations currently.  For example, PlatformStrategies.h is in WebCore/platform, but it mentions PluginStrategy, which is in WebCore/plugins.
> 
> This "layering" violation seems tenuous, as all of WebCore is generally considered to be the same layer.  Are you suggesting that anywhere else we "cross directories" within WebCore we're committing a layering violation?

WebCore/platform is usually considered a layer below all of the rest of WebCore, so it shouldn't depend on other subdirectories of WebCore. We should fix this. (Probably worth a separate bug).

[Maciej Stachowiak]

(In reply to comment #10)
> > I am ok with an approach that doesn't require a virtual function for ports that don't need to support two WebKits, but I still don't understand why you feel this is so performance critical.
> 
> As Maciej wrote yesterday:
> 
> [[
> Just as for behavior-affecting changes it is the patch submitter's job to provide evidence that the patch is correct, with performance-affecting patches it is the patch submitter's job to provide evidence that the patch improves performance as intended and does not regress things that it might be feared to regress.
> ]]

FWIW when we first adopted the PlatformStrategies approach, we confirmed that it did not measurably regress our core benchmarks, even using the classic Mac WebKit API where all it does is add a level of indirection through a virtual function call. The benchmarks we tested specifically included page load speed benchmarks (PLT, iBench HTML).

If you are worried that the virtual call overhead being worse in the Chromium context there is probably some test that can be done to find out.


Of course, it is ultimately up to Chromium folks you whether you adopt the PlatformStrategies mechanism. I have no personal interest in being pushy about it. I just hope that can be an informed choice.



> What's changed since then is that we've deleted PlatformBridge from the Chromium port and removed this cognitive cost.  That was a fair amount of work because we needed to factor the Chromium WebKit API into two pieces, the Client API and the Platform API, as discussed on webkit-dev:

There's also a cognitive cost to Chromium being different from other ports in unnecessary ways, for everyone working on the project. I think that is the motive behind the suggestion in this bug. I agree that you shouldn't have to incur a measurable perf hit to avoid this cognitive cost, though, if indeed there is one.

> 
> http://lists.webkit.org/pipermail/webkit-dev/2012-March/020166.html
> 
> Re-introducing PlatformBridge into the Chromium port renamed to PlatformStrategies and with a slower implementation is a net loss.

Another thing I said yesterday was that optimization without measuring was a well-known bad engineering practice, and I think that applies to assuming virtual calls will be slower without testing.

If anyone does try any kind of A/B test, I'd be interested in seeing the results. We can also try digging up old versions of Safari/WebKit to retest the pert impact of the PlatformStrategies change at the time it was added.

[Adam Barth]

I put the (hopefully!) non-controversial part of this change in bug 100156.

[Adam Barth]

This discussion appears to have run its course.

The summary from my perspective is that PlatformStrategies has been designed with only the needs of WebKit2 in mind.  The main thing that has changed since we last discussed this topic in 2010 is that we've put some amount of effort into the Chromium port to remove precisely the extra thunks that adopting PlatformStrategies would introduce.

It might be appropriate to revisit this topic in the future if (1) you all are interested in redesigning PlatformStrategies to provide value to non-WebKit2 ports or (2) Chromium decides to adopt WebKit2.

[Maciej Stachowiak]

(In reply to comment #14)
> This discussion appears to have run its course.
> 
> The summary from my perspective is that PlatformStrategies has been designed with only the needs of WebKit2 in mind.  The main thing that has changed since we last discussed this topic in 2010 is that we've put some amount of effort into the Chromium port to remove precisely the extra thunks that adopting PlatformStrategies would introduce.
> 
> It might be appropriate to revisit this topic in the future if (1) you all are interested in redesigning PlatformStrategies to provide value to non-WebKit2 ports or (2) Chromium decides to adopt WebKit2.

We'd definitely change PlatformStrategies if we could make it usable for Chromium while still serving its current purpose. Removing unnecessary architectural divergences is an important goal for us. 

Do you have any specific suggestions for changes that would help? Is there any obstacle besides the extra level of virtual call? Would it be productive to discuss in person how we might make a mechanism that meets all our needs?

If virtual call is the only issue, then it seems like it may be possible to make PlatformStrategies only conditionally virtual, so that indirection via virtual method calls is introduced only for ports that actually need runtime switchable strategies. I am skeptical that the virtual calls in this case actually have a measurable runtime cost, but there's no reason to impose them in cases where they have no value.

If you'd still be concerned about an approach with no extra virtual calls for Chromium solely because of an extra layer of indirection, then I guess there is not much we can do, but I hope would consider the tradeoff of the cognitive cost of unnecessary Chromium-specific differences vs the cognitive cost of an extra non-virtual layer of indirection for Chromium (that already exists for other ports). I tend to think total cognitive cost of the first option is higher.

If you have a suggestion for how a more Chromium-like approach could work for non-Chromium ports then we'd be open to considering that too.

(BTW it seems like given your comment the correct resolution to this bug is WONTFIX, not FIXED).

[Adam Barth]

I think the main thing that would make PlatformStrategies work well in Chromium would be if the Chromium embedder could supply a strategy directly, in much the same way that WebKit2 supplies strategies.  The main benefit of this approach is that it avoids the extra thunk.

The main cost to this approach is that it's more difficult to modify strategy APIs because Chromium will have dependencies on the API from outside svn.webkit.org.  Relatedly, we'd need to express the API in terms of API types (e.g., types that don't leak all of WebCore's implementation details).  Chromium is more flexible in these regards than other ports because we control the code on both sides of the API, so we can certainly change the APIs over time (and switch to whatever types are convenient)---there's just a bit more friction.

Maybe a good path forward is to work though a simple example and then to see how that might scale to other examples?  The simplest one to start with might be VisitedLinkStrategy.  In VisitedLinkStrategy.h, we have:

    typedef uint64_t LinkHash;
    virtual bool isLinkVisited(Page*, LinkHash, const KURL& baseURL, const AtomicString& attributeURL) = 0;

In <http://trac.webkit.org/browser/trunk/Source/Platform/chromium/public/Platform.h> we have:

    virtual bool isLinkVisited(unsigned long long linkHash) { return false; }

Interestingly, in WebPlatformStrategies.cpp, the implementation of isLinkVisited for WebKit2 is:

bool WebPlatformStrategies::isLinkVisited(Page*, LinkHash linkHash, const KURL&, const AtomicString&)
{
    return WebProcess::shared().isLinkVisited(linkHash);
}

Notice that the WebKit2 implementation drops all the parameters except the linkHash.

The way I would approach merging these concepts is to create a new public directory (potentially in Source/Platform/public) with a header similar to the following:

namespace Platform {

class VisitedLinkStrategy {
public:
    virtual bool isLinkVisited(unsigned long long linkHash) { return false; }
};

}

The idea would be that these headers wouldn't have any dependencies outside themselves, much like the Source/Platform/chromium/public headers have entirely self-contained dependencies.  Chromium could then implement VisitedLinkStrategy in the embedder and WebKit2 could implement it in the WebKit2 layer.

Over time, we'd likely end up migrating headers between Source/Platform/chromium/public and Source/Platform/public so that Source/Platform/public contains the Platform abstraction that's shared by all the ports (e.g., WebCookieJar---likely renamed and merged with CookieStrategy) and Source/Platform/chromium/public contains the Chromium-specific parts (e.g., WebCompositorSupport).