Don't create parallel request if download is about to complete

Don't create parallel request if download is about to complete

For fast networks, 2MB slice limit maybe too small.
This change adds a new requirement that if a download is about to complete,
don't create the parallel request.
This solves the issue for faster networks.

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

[qinmin, 2017-04-07 23:40:54]

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

[qinmin, 2017-04-07 23:40:54]

PTAL

[xingliu, 2017-04-08 02:21:05]

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

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job_unittest.cc:199: // File size:
100 bytes.
nit: Can we add a TODO here to pass the minimum remaining time here as  a
parameter in CreateParallelJob after it's a finch configuration? 

So other test cases don't need to change with this configuration accordingly.

[qinmin, 2017-04-10 19:04:48]

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

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job_unittest.cc:199: // File size:
100 bytes.
On 2017/04/08 02:21:05, xingliu wrote:
> nit: Can we add a TODO here to pass the minimum remaining time here as  a
> parameter in CreateParallelJob after it's a finch configuration? 
> 
> So other test cases don't need to change with this configuration accordingly.

Done.

[xingliu, 2017-04-10 19:05:38]

lgtm.

[qinmin, 2017-04-10 19:32:03]

The CQ bit was checked by qinmin@chromium.org

[commit-bot: I haz the power, 2017-04-10 19:33:23]

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

[David Trainor- moved to gerrit, 2017-04-10 20:10:00]

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

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:157: int64_t current_speed =
download_item_->CurrentSpeed() == 0
std::max(1, download_item_->CurrentSpeed())?  Also current_bytes_per_second?

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:162: if (remaining_bytes /
current_speed >
Do we want an UMA stat for when we decide/don't decide to do this?  Maybe even a
histogram of typical seconds left?

[qinmin, 2017-04-10 20:22:44]

The CQ bit was unchecked by qinmin@chromium.org

[qinmin, 2017-04-10 21:43:39]

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

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:157: int64_t current_speed =
download_item_->CurrentSpeed() == 0
On 2017/04/10 20:10:00, David Trainor-ping if over 24h wrote:
> std::max(1, download_item_->CurrentSpeed())?  Also current_bytes_per_second?

Done.

https://codereview.chromium.org/2806923002/diff/1/content/browser/download/pa...
content/browser/download/parallel_download_job.cc:162: if (remaining_bytes /
current_speed >
On 2017/04/10 20:10:00, David Trainor-ping if over 24h wrote:
> Do we want an UMA stat for when we decide/don't decide to do this?  Maybe even
a
> histogram of typical seconds left?

will add the decion about parallel download creation as an enum after xingliu@'s
change is submitted.

Added a metric to record the remaining time.

[qinmin, 2017-04-10 21:43:55]

qinmin@chromium.org changed reviewers:
+ isherman@chromium.org

[qinmin, 2017-04-10 21:43:56]

+isherman for UMA changes

[Ilya Sherman, 2017-04-10 21:47:42]

Metrics LGTM

[qinmin, 2017-04-10 23:59:39]

Patchset #3 (id:40001) has been deleted

[qinmin, 2017-04-11 21:25:19]

The CQ bit was checked by qinmin@chromium.org

[qinmin, 2017-04-11 21:25:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
xingliu@chromium.org, isherman@chromium.org
Link to the patchset: https://codereview.chromium.org/2806923002/#ps80001
(title: "add UMA and finch")

[commit-bot: I haz the power, 2017-04-11 21:25:45]

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-11 22:45:55]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491945919887150,
"parent_rev": "a70a095814dfc2530cc653d92962f1bc0b83987d", "commit_rev":
"cbb485045d95ea0934cce5a99e3796d4dcbfb596"}

[commit-bot: I haz the power, 2017-04-11 22:46:47]

Description was changed from

==========
Don't create parallel request if download is about to complete

For fast networks, 2MB slice limit maybe too small.
This change adds a new requirement that if a download is about to complete,
don't create the parallel request.
This solves the issue for faster networks.

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

to

==========
Don't create parallel request if download is about to complete

For fast networks, 2MB slice limit maybe too small.
This change adds a new requirement that if a download is about to complete,
don't create the parallel request.
This solves the issue for faster networks.

BUG=644352

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

[commit-bot: I haz the power, 2017-04-11 22:46:48]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/cbb485045d95ea0934cce5a99e37...