Create a content::UrlFetcher interface that lives in content/public/common and convert users to i...

Create a content::UrlFetcher interface that lives in content/public/common and convert users to it. I have added a static create function, but will switch instantiations to use it in another change since this has grown a lot. Basically this change converts function names away from unix_hacker style, which they shouldn't have been using anyways since some are virtual, and made all of the other functions virtual.

BUG=98716
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107151

[willchan no longer on Chromium, 2011-10-25 16:59:49]

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
File content/common/net/url_fetcher.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
content/common/net/url_fetcher.h:24: class CONTENT_EXPORT URLFetcher : public
content::URLFetcher{
This is still going to be named URLFetcher? I'd be a bit nervous about that.
What about URLFetcherImpl or something?

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
content/common/net/url_fetcher.h:98: static void CancelAll();
CancelAll() is in both places?

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
File content/public/common/url_fetcher.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:38: //   URLFetcher* fetcher =
URLFetcher::create("http://www.google.com",
URLFetcher::Create

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:68: static const int
kInvalidHttpResponseCode;
I don't think this actually needs to be public. I think it's local to
url_fetcher.cc. I'd get rid of this.

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:81: content::URLFetcherDelegate* d);
no need for content::

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
File content/public/common/url_fetcher_delegate.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher_delegate.h:20: virtual void
OnURLFetchComplete(const content::URLFetcher* source) = 0;
Is this content:: necessary? I suspect name resolution will choose the name
within content first rather than the global namespace's.

[jam, 2011-10-25 17:16:20]

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
File content/common/net/url_fetcher.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
content/common/net/url_fetcher.h:24: class CONTENT_EXPORT URLFetcher : public
content::URLFetcher{
On 2011/10/25 16:59:49, willchan wrote:
> This is still going to be named URLFetcher? I'd be a bit nervous about that.
> What about URLFetcherImpl or something?

yep it'll be renamed to UrlFetcherImpl (in the next cl)

http://codereview.chromium.org/8375039/diff/1186/content/common/net/url_fetch...
content/common/net/url_fetcher.h:98: static void CancelAll();
On 2011/10/25 16:59:49, willchan wrote:
> CancelAll() is in both places?

the problem is that the static content::UrlFetcher function can't call Core's
CancelAll because that's a private member of UrlFetcher.

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
File content/public/common/url_fetcher.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:38: //   URLFetcher* fetcher =
URLFetcher::create("http://www.google.com",
On 2011/10/25 16:59:49, willchan wrote:
> URLFetcher::Create

Done.

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:68: static const int
kInvalidHttpResponseCode;
On 2011/10/25 16:59:49, willchan wrote:
> I don't think this actually needs to be public. I think it's local to
> url_fetcher.cc. I'd get rid of this.

it's referenced below in GetResponseCode

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:81: content::URLFetcherDelegate* d);
On 2011/10/25 16:59:49, willchan wrote:
> no need for content::

Done.

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
File content/public/common/url_fetcher_delegate.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher_delegate.h:20: virtual void
OnURLFetchComplete(const content::URLFetcher* source) = 0;
On 2011/10/25 16:59:49, willchan wrote:
> Is this content:: necessary? I suspect name resolution will choose the name
> within content first rather than the global namespace's.

no it's not, done.

[willchan no longer on Chromium, 2011-10-25 17:22:57]

lgtm

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
File content/public/common/url_fetcher.h (right):

http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
content/public/common/url_fetcher.h:68: static const int
kInvalidHttpResponseCode;
On 2011/10/25 17:16:20, John Abd-El-Malek wrote:
> On 2011/10/25 16:59:49, willchan wrote:
> > I don't think this actually needs to be public. I think it's local to
> > url_fetcher.cc. I'd get rid of this.
> 
> it's referenced below in GetResponseCode

Oh, good point. I just did a grep and no one's checking for this, heh. Maybe
it's better off just to use an enum set to -1. Whatever.

[jam, 2011-10-25 17:27:10]

On 2011/10/25 17:22:57, willchan wrote:
> lgtm
> 
>
http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
> File content/public/common/url_fetcher.h (right):
> 
>
http://codereview.chromium.org/8375039/diff/1186/content/public/common/url_fe...
> content/public/common/url_fetcher.h:68: static const int
> kInvalidHttpResponseCode;
> On 2011/10/25 17:16:20, John Abd-El-Malek wrote:
> > On 2011/10/25 16:59:49, willchan wrote:
> > > I don't think this actually needs to be public. I think it's local to
> > > url_fetcher.cc. I'd get rid of this.
> > 
> > it's referenced below in GetResponseCode
> 
> Oh, good point. I just did a grep and no one's checking for this, heh. Maybe
> it's better off just to use an enum set to -1. Whatever.

good idea, done