Move URLFetcherImpl to net/

Move URLFetcherImpl to net/

Add some TODOs for follow-up tasks.

Leave URLFetcher::Create in content, but move it to url_fetcher.cc.

BUG=118220
TEST=


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

[akalin, 2012-06-14 03:36:17]

+wtc for review
+jam for content owners (gyp files, will TBR if necessary)

[jam, 2012-06-14 04:12:51]

lgtm

now that this is in net, do you still need a URLFetcher and URLFetcherImpl? I
had added this separation because this is part of the Content API (like WebKit
API), but it's not like that generally in net afaik.

[akalin, 2012-06-14 04:17:50]

On 2012/06/14 04:12:51, John Abd-El-Malek wrote:
> lgtm
> 
> now that this is in net, do you still need a URLFetcher and URLFetcherImpl? I
> had added this separation because this is part of the Content API (like WebKit
> API), but it's not like that generally in net afaik.

Hmm, there's TestURLFetcher, so I think there's still a need for an
interface/implementation separation, at least for testing.

[jam, 2012-06-14 04:23:26]

On 2012/06/14 04:17:50, akalin wrote:
> On 2012/06/14 04:12:51, John Abd-El-Malek wrote:
> > lgtm
> > 
> > now that this is in net, do you still need a URLFetcher and URLFetcherImpl?
I
> > had added this separation because this is part of the Content API (like
WebKit
> > API), but it's not like that generally in net afaik.
> 
> Hmm, there's TestURLFetcher, so I think there's still a need for an
> interface/implementation separation, at least for testing.

good point. that used to derive from the implementation, but I had converted
that to derive from an interface (after I added it). I guess now it's fine to
leave the interface because of this, but I'll defer to net folks and you.

[wtc, 2012-06-14 19:52:28]

Patch set 3 LGTM.  Thanks.

https://chromiumcodereview.appspot.com/10534154/diff/9002/content/common/net/...
File content/common/net/url_fetcher.cc (right):

https://chromiumcodereview.appspot.com/10534154/diff/9002/content/common/net/...
content/common/net/url_fetcher.cc:27: net::URLFetcherFactory* const factory =
net::URLFetcherImpl::factory();

Nit: the use of 'const' here seems a little excessive.
It means 'factory' won't be changed after initialization, right?

[akalin, 2012-06-15 03:42:16]

Committing via commitbot

https://chromiumcodereview.appspot.com/10534154/diff/9002/content/common/net/...
File content/common/net/url_fetcher.cc (right):

https://chromiumcodereview.appspot.com/10534154/diff/9002/content/common/net/...
content/common/net/url_fetcher.cc:27: net::URLFetcherFactory* const factory =
net::URLFetcherImpl::factory();
On 2012/06/14 19:52:28, wtc wrote:
> 
> Nit: the use of 'const' here seems a little excessive.
> It means 'factory' won't be changed after initialization, right?

Done.

[commit-bot: I haz the power, 2012-06-15 03:42:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/10534154/8004

[commit-bot: I haz the power, 2012-06-15 05:16:15]

Try job failure for 10534154-8004 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2012-06-15 05:29:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/10534154/8004

[commit-bot: I haz the power, 2012-06-15 06:24:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akalin@chromium.org/10534154/9019

[commit-bot: I haz the power, 2012-06-15 07:51:47]

Change committed as 142355