Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

The touched code paths are already being exercised by existing
content_browsertests.

R=davidben@chromium.org
BUG=7648
Committed: https://crrev.com/bb028fc67f0835b30bd2ce5d953caa101c15c961
Cr-Commit-Position: refs/heads/master@{#372381}

[asanka, 2016-01-28 20:40:29]

Description was changed from

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after they are created.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both these are not great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just DownloadManager.
Then we can move download handling logic out of RDH into
DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a
download via DownloadManager. The additional overhead should be
negligible since download requests aren't very frequent.

R=davidben@chromium.org
BUG=7648
==========

to

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

R=davidben@chromium.org
BUG=7648
==========

[asanka, 2016-01-28 20:41:09]

Description was changed from

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

R=davidben@chromium.org
BUG=7648
==========

to

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

The touched code paths are already being exercised by existing
content_browsertests.

R=davidben@chromium.org
BUG=7648
==========

[davidben, 2016-01-28 21:25:34]

lgtm

[asanka, 2016-01-28 21:26:24]

On 2016/01/28 21:25:34, davidben (slow) wrote:
> lgtm

Thanks!

[asanka, 2016-01-29 17:17:11]

The CQ bit was checked by asanka@chromium.org

[commit-bot: I haz the power, 2016-01-29 17:17:56]

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

[commit-bot: I haz the power, 2016-01-29 18:28:00]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-01-29 18:29:03]

Description was changed from

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

The touched code paths are already being exercised by existing
content_browsertests.

R=davidben@chromium.org
BUG=7648
==========

to

==========
Use DownloadManager to initiate downloads instead ResourceDispatcherHost

Downloads have two major failure pathways: dealing with failures before
DownloadResourceHandler/DownloadRequestCore creation, and dealing with
failures after.

Somewhat relatedly, ResourceDispatcherHost::BeginDownload() currently is
responsible for implementing some aspects of download initiation that
really don't belong there. For example, download started callbacks are
handled by ResourceDispatcherHost prior to the creation of
DownloadResourceHandler.

Both of these aren't great. In order to unify failure paths and remove
downloads handling code from ResourceDispatcherHost, we need to first
need to reduce RDH::BeginDownload callers down to just
DownloadManagerImpl. Then we can move download handling logic out of RDH
into DownloadManager.

This CL removes the only other caller of RDH::BeginDownload, which is
RenderMessageFilter. It now makes a UI thread hop to initiate a download
via DownloadManager. The overhead of the thread hop should be negligible
since the frequency of download requests are low.

The touched code paths are already being exercised by existing
content_browsertests.

R=davidben@chromium.org
BUG=7648

Committed: https://crrev.com/bb028fc67f0835b30bd2ce5d953caa101c15c961
Cr-Commit-Position: refs/heads/master@{#372381}
==========

[commit-bot: I haz the power, 2016-01-29 18:29:04]

Patchset 2 (id:??) landed as
https://crrev.com/bb028fc67f0835b30bd2ce5d953caa101c15c961
Cr-Commit-Position: refs/heads/master@{#372381}