Bug 82587 – [Chromium] Move createURLLoader() into Platform

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

[Adam Barth]

Move createURLLoader() into Platform

[Adam Barth]

Created attachment 134532 [details]
Patch

[WebKit Review Bot]

Please wait for approval from [e-mail], [e-mail], [e-mail], [e-mail] or [e-mail] before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.

[James Robinson]

Comment on attachment 134532 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=134532&action=review

R=me but one question about headers in the comments.

> Source/WebKit/chromium/public/platform/Platform.h:26
> +#include "../../../../Platform/chromium/public/Platform.h"

do we need this forwarding header? I've been assuming that we only need a forwarding header in here if we expect users of the client API to need to see the header and don't want them to have to put both APIs on their include path. I don't think that's true here, since we don't expect users of the API to be using Platform.h directly, right?

If we do need this to be exposed for users of the client API then I think it should live in WebKit/chromium/public/

> Source/WebKit/chromium/public/platform/WebKitPlatformSupport.h:34
> +#include "Platform.h"

could this be #include <public/Platform.h> or "../../../Platform/chromium/public/Platform.h"  to avoid the forwarding header?

[Adam Barth]

(In reply to comment #3)
> (From update of attachment 134532 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=134532&action=review
> 
> R=me but one question about headers in the comments.
> 
> > Source/WebKit/chromium/public/platform/Platform.h:26
> > +#include "../../../../Platform/chromium/public/Platform.h"
> 
> do we need this forwarding header? I've been assuming that we only need a forwarding header in here if we expect users of the client API to need to see the header and don't want them to have to put both APIs on their include path. I don't think that's true here, since we don't expect users of the API to be using Platform.h directly, right?

The issue is that Platform is a base class for WebKitPlatformSupport, so embedders need to see the header so that they can see all of WebKitPlatformSupport.

> If we do need this to be exposed for users of the client API then I think it should live in WebKit/chromium/public/
> 
> > Source/WebKit/chromium/public/platform/WebKitPlatformSupport.h:34
> > +#include "Platform.h"
> 
> could this be #include <public/Platform.h> or "../../../Platform/chromium/public/Platform.h"  to avoid the forwarding header?

#include <public/Platform.h> doesn't work because clients who include WebKitPlatformSupport.h wouldn't be able to use this include path.  The second thing works, but I thought it was cleaner to use a forwarding header.

I'm not 100% sure what we want to do with these forwarding headers in the long term.  My thinking was that I'd just make this one fit the pattern used by the others and we can worry about what to do with the forwarding headers once we're further along.

[James Robinson]

The other headers have that pattern because they are part of the client API - i.e. someone who wants to talk to WebKit needs to be able to see the WebString header.  Right now Platform.h is just an implementation detail for people using the client API, so I think it makes more sense for now to "hide" it inside WebKitPlatformSupport.h instead of having a dedicated forwarding header.

[Adam Barth]

Ok.  Will do.

[Adam Barth]

Committed r112563: <http://trac.webkit.org/changeset/112563>