[Downloads] Factor out request handling logic between DRH and UD.

[Downloads] Factor out request handling logic between DRH and UD.

DownloadResourceHandler and UrlDownloader both have a lot in common since they
are both responsible for driving a URL request. The only difference being that
DRH is written to be a part of the ResourceLoader stack, while UrlDownloader
drives a URLRequest on its own. This CL factors out the common bits in both so
that upcoming fixes to downloads resumption can be made and tested in one
location.

R=svaldez@chromium.org
BUG=7648
BUG=225901
Committed: https://crrev.com/64114156487081d877b793d3a501a8658743141d
Cr-Commit-Position: refs/heads/master@{#366459}

[asanka, 2015-12-16 19:27:14]

Could you have a look?

The existing tests should cover all the changes being made here. But they don't.
So I'll be adding some more tests to cover the DownloadRequestHandle changes. It
should be possible to call Pause/Resume for downloads that are running off of
UrlDownloader.

I'm doing this because there's a bunch of changes that needs to go into how we
deal with redirects and a few other error cases, and it's a bit painful to make
those changes in both DRH and DRC.

[svaldez, 2015-12-17 16:53:33]

LGTM. Mostly just nits. I took a quick look to make sure that there isn't
anything missing from the refactor, and can take another look after today's
launch review.

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/download_create_info.cc (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_create_info.cc:30: make_scoped_ptr(new
DownloadSaveInfo())) {}
(new DownloadSaveInfo)

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_create_info.cc:42: total_bytes);
nit: Possibly revert to separate lines since that's slightly clearer.

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/download_manager_impl_unittest.cc (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_manager_impl_unittest.cc:522:
info.request_handle.reset(new DownloadRequestHandle());
nit: (new DownloadRequestHandle);

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/url_downloader.h (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/url_downloader.h:19: struct DownloadCreateInfo;
Are these still needed?

[asanka, 2015-12-21 20:13:51]

The CQ bit was checked by asanka@chromium.org to run a CQ dry run

[asanka, 2015-12-21 20:14:02]

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/download_create_info.cc (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_create_info.cc:30: make_scoped_ptr(new
DownloadSaveInfo())) {}
On 2015/12/17 16:53:32, svaldez wrote:
> (new DownloadSaveInfo)

Done.

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_create_info.cc:42: total_bytes);
On 2015/12/17 16:53:32, svaldez wrote:
> nit: Possibly revert to separate lines since that's slightly clearer.

Unfortunately, this is clang format's handywork.

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/download_manager_impl_unittest.cc (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/download_manager_impl_unittest.cc:522:
info.request_handle.reset(new DownloadRequestHandle());
On 2015/12/17 16:53:32, svaldez wrote:
> nit: (new DownloadRequestHandle);

Done.

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
File content/browser/download/url_downloader.h (right):

https://codereview.chromium.org/1533583002/diff/60001/content/browser/downloa...
content/browser/download/url_downloader.h:19: struct DownloadCreateInfo;
On 2015/12/17 16:53:32, svaldez wrote:
> Are these still needed?

Nope. Only DownloadManagerImpl is needed. Thanks!

[commit-bot: I haz the power, 2015-12-21 20:14:11]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533583002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533583002/80001

[asanka, 2015-12-21 21:02:04]

The CQ bit was checked by asanka@chromium.org

[asanka, 2015-12-21 21:02:05]

The patchset sent to the CQ was uploaded after l-g-t-m from svaldez@chromium.org
Link to the patchset: https://codereview.chromium.org/1533583002/#ps80001
(title: "Address comments and change .Pass() -> std::move(...) per PRESUBMIT
check")

[commit-bot: I haz the power, 2015-12-21 21:02:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1533583002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1533583002/80001

[commit-bot: I haz the power, 2015-12-21 21:28:56]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-12-21 21:29:36]

Description was changed from

==========
[Downloads] Factor out request handling logic between DRH and UD.

DownloadResourceHandler and UrlDownloader both have a lot in common since they
are both responsible for driving a URL request. The only difference being that
DRH is written to be a part of the ResourceLoader stack, while UrlDownloader
drives a URLRequest on its own. This CL factors out the common bits in both so
that upcoming fixes to downloads resumption can be made and tested in one
location.

R=svaldez@chromium.org
BUG=7648
BUG=225901
==========

to

==========
[Downloads] Factor out request handling logic between DRH and UD.

DownloadResourceHandler and UrlDownloader both have a lot in common since they
are both responsible for driving a URL request. The only difference being that
DRH is written to be a part of the ResourceLoader stack, while UrlDownloader
drives a URLRequest on its own. This CL factors out the common bits in both so
that upcoming fixes to downloads resumption can be made and tested in one
location.

R=svaldez@chromium.org
BUG=7648
BUG=225901

Committed: https://crrev.com/64114156487081d877b793d3a501a8658743141d
Cr-Commit-Position: refs/heads/master@{#366459}
==========

[commit-bot: I haz the power, 2015-12-21 21:29:37]

Patchset 5 (id:??) landed as
https://crrev.com/64114156487081d877b793d3a501a8658743141d
Cr-Commit-Position: refs/heads/master@{#366459}