[Chromium] Move WebKitPlatformSupport.h and dependencies to new public/platform directory
Created attachment 116474 [details] Patch
Please wait for approval from fishd@chromium.org before submitting because this patch contains changes to the Chromium public API.
This smells like all kinds of awesome. What about the idea of moving these headers into Source/Platform/chromium/public/? Is WebKit/chromium/public/platform/ staging ground for that eventual move?
Comment on attachment 116474 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=116474&action=review > Source/WebKit/chromium/public/platform/WebSerializedScriptValue.h:49 > +// FIXME: Should this class be in platform? good question.
> good question. My initial read is "no", but I need to investigate more why it's used by WebKitPlatformSupport.
Thanks for the review.
Committed r101122: <http://trac.webkit.org/changeset/101122>
> This smells like all kinds of awesome. What about the idea of moving these headers into Source/Platform/chromium/public/? Is WebKit/chromium/public/platform/ staging ground for that eventual move? Yes, that's the idea. I'm going to try to sort out which headers belong on which side of the split. Using forwarding headers lets us feel this out without impacting the rest of Chromium too much. I'd also like to split up the CPP files in src, which will require use to be a bit more honest about our dependencies.
(In reply to comment #8) > > This smells like all kinds of awesome. What about the idea of moving these headers into Source/Platform/chromium/public/? Is WebKit/chromium/public/platform/ staging ground for that eventual move? > > Yes, that's the idea. I'm going to try to sort out which headers belong on which side of the split. Using forwarding headers lets us feel this out without impacting the rest of Chromium too much. > > I'd also like to split up the CPP files in src, which will require use to be a bit more honest about our dependencies. Sounds great. Hopefully, there is an end-state that permits WebCore (from Chromium-specific .cpp files) to talk directly to these platform/ headers.
> Hopefully, there is an end-state that permits WebCore (from Chromium-specific .cpp files) to talk directly to these platform/ headers. My thought was that Chromium-specific code in Source/Platform could talk to them. WebCore-proper code would still need to go through some sort of port-agnostic Platform abstraction. This should also let us solve the problem of having WebCore-namespaced code in WebKit/chromium/src (although I need to study all uses of this to make sure we can handle them all this way).
(In reply to comment #10) > > Hopefully, there is an end-state that permits WebCore (from Chromium-specific .cpp files) to talk directly to these platform/ headers. > > My thought was that Chromium-specific code in Source/Platform could talk to them. WebCore-proper code would still need to go through some sort of port-agnostic Platform abstraction. Is this presupposing that code that today exists in Source/WebCore/platform moves to Source/Platform? Concretely I'd like to implement WebCore::GraphicsLayer in terms of interfaces in this platform/ section. What needs to happen in order to do that? Can I have an implementation in platform/graphics/chromium/GraphicsLayerChromium.cpp talk directly to types in this new platform directory or not? > > This should also let us solve the problem of having WebCore-namespaced code in WebKit/chromium/src (although I need to study all uses of this to make sure we can handle them all this way).
Also, do we plan to keep using the Web* prefix for platform APIs? I don't think it makes as much sense for platform stuff as it does for client interfaces, personally.
> Is this presupposing that code that today exists in Source/WebCore/platform moves to Source/Platform? Yes, but we don't need to wait. I was just trying to think ahead. > Concretely I'd like to implement WebCore::GraphicsLayer in terms of interfaces in this platform/ section. What needs to happen in order to do that? Mostly adding the include path to the WebCore.gyp file. Past that, it's mostly aesthetics. > Can I have an implementation in platform/graphics/chromium/GraphicsLayerChromium.cpp talk directly to types in this new platform directory or not? Currently that's not allowed, but we could change our minds about what's allowed. IMHO, that's the direction we should move in. > Also, do we plan to keep using the Web* prefix for platform APIs? I don't think it makes as much sense for platform stuff as it does for client interfaces, personally. That sounds like a question for fishd.
(In reply to comment #13) > > Can I have an implementation in platform/graphics/chromium/GraphicsLayerChromium.cpp talk directly to types in this new platform directory or not? > > Currently that's not allowed, but we could change our minds about what's allowed. IMHO, that's the direction we should move in. +1 > > Also, do we plan to keep using the Web* prefix for platform APIs? I don't think it makes as much sense for platform stuff as it does for client interfaces, personally. > > That sounds like a question for fishd. I had wrestled with this a bit, but I came back to sticking with the Web* prefix. Reason: WebString and friends will need to live in the platform/ directory. If we are going to have some Web* in the platform directory, then we might as well be consistent about using it for all interfaces. I think the rule for platform/ APIs should be that they are _only_ APIs that are needed to support WebCore. There should be no static functions at this level in other words.
(In reply to comment #14) > There should be no static functions at this level in other words. ^^^ Whoops, scratch that. WebString::fromUTF8 is an obvious counter example.