Polish the cleaning up url request code for parallel download.

Polish the cleaning up url request code for parallel download.

This CL addresses code review feedbacks in
https://codereview.chromium.org/2811293004/.

The flow to close a particular request is:

DownloadFileImpl ==> DownloadItem ==> DownloadJob ==> Close url request.

The flow is covered in ParallelDownloadComplete browser test.


The reason that we do this, is because we terminate some of the requests
in parallel download before it completes while not canceling the whole
download. And after we null out the callback of ByteStreamReader, the
actual url request is not closed.

BUG=644352
Review-Url: https://codereview.chromium.org/2829853011
Cr-Commit-Position: refs/heads/master@{#467402}
Committed: https://chromium.googlesource.com/chromium/src/+/aca52687045830c9b42e288df02ddcf4a01618f7

[xingliu, 2017-04-22 00:05:48]

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

[xingliu, 2017-04-22 00:06:04]

xingliu@chromium.org changed reviewers:
+ asanka@chromium.org, dtrainor@chromium.org, qinmin@chromium.org

[xingliu, 2017-04-22 00:06:05]

Hi, PTAL, thanks.

[commit-bot: I haz the power, 2017-04-22 00:06:37]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-22 01:07:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-22 01:07:37]

Dry run: This issue passed the CQ dry run.

[qinmin, 2017-04-24 21:27:31]

lgtm % nit

https://codereview.chromium.org/2829853011/diff/1/content/browser/download/pa...
File content/browser/download/parallel_download_job.cc (right):

https://codereview.chromium.org/2829853011/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:104: if (it != workers_.end())
nit: you can remove this if statement.

[qinmin, 2017-04-24 21:27:32]

lgtm % nit

[xingliu, 2017-04-24 22:07:12]

Thanks.

https://codereview.chromium.org/2829853011/diff/1/content/browser/download/pa...
File content/browser/download/parallel_download_job.cc (right):

https://codereview.chromium.org/2829853011/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:104: if (it != workers_.end())
On 2017/04/24 21:27:31, qinmin wrote:
> nit: you can remove this if statement.

Done.

[asanka, 2017-04-25 03:16:37]

lgtm

[David Trainor- moved to gerrit, 2017-04-25 03:52:05]

lgtm

[xingliu, 2017-04-26 17:54:50]

Thanks all for the review!

[xingliu, 2017-04-26 17:54:55]

The CQ bit was checked by xingliu@chromium.org

[xingliu, 2017-04-26 17:54:55]

The patchset sent to the CQ was uploaded after l-g-t-m from qinmin@chromium.org
Link to the patchset: https://codereview.chromium.org/2829853011/#ps20001
(title: "Work on feedbacks.")

[commit-bot: I haz the power, 2017-04-26 17:55:51]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-26 18:59:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493229295372130,
"parent_rev": "da917a493d3c94e2e2003029a97f3811f3cd05c5", "commit_rev":
"aca52687045830c9b42e288df02ddcf4a01618f7"}

[commit-bot: I haz the power, 2017-04-26 18:59:34]

Description was changed from

==========
Polish the cleaning up url request code for parallel download.

This CL addresses code review feedbacks in
https://codereview.chromium.org/2811293004/.

The flow to close a particular request is:

DownloadFileImpl ==> DownloadItem ==> DownloadJob ==> Close url request.

The flow is covered in ParallelDownloadComplete browser test.


The reason that we do this, is because we terminate some of the requests
in parallel download before it completes while not canceling the whole
download. And after we null out the callback of ByteStreamReader, the
actual url request is not closed.

BUG=644352
==========

to

==========
Polish the cleaning up url request code for parallel download.

This CL addresses code review feedbacks in
https://codereview.chromium.org/2811293004/.

The flow to close a particular request is:

DownloadFileImpl ==> DownloadItem ==> DownloadJob ==> Close url request.

The flow is covered in ParallelDownloadComplete browser test.


The reason that we do this, is because we terminate some of the requests
in parallel download before it completes while not canceling the whole
download. And after we null out the callback of ByteStreamReader, the
actual url request is not closed.

BUG=644352

Review-Url: https://codereview.chromium.org/2829853011
Cr-Commit-Position: refs/heads/master@{#467402}
Committed:
https://chromium.googlesource.com/chromium/src/+/aca52687045830c9b42e288df02d...
==========

[commit-bot: I haz the power, 2017-04-26 18:59:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/aca52687045830c9b42e288df02d...