New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 644352 link

Starred by 9 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

Implement support for parallel requests for downloads

Project Member Reported by asanka@chromium.org, Sep 6 2016

Issue description

We should investigate the effects of having multiple active requests for the same resource where each request is fetching different ranges.

 
This was a popular feature request for IE in the late '00s, as it was a common feature of download managers of the era. 

Our investigation at the time concluded that it only reliably improved performance in the case that the server was deliberately throttling per-connection throughput (e.g. to entice the user to pay for an upgrade); it frequently failed on such sites though because they caught on and started using one-time tokens for downloads such that the parallel attempt would be redirected to the download page.

In a world of multiplexed HTTP2 connections, I wonder whether there would still be any benefit.
#1. Understood. Thanks!

The investigative portion of this bug is to understand the effects of this technique. We currently don't have much data on how something like this would perform in practice.

I'm also conjecturing that support for multiple parallel requests will make it possible for us to speculatively reconnect to servers where the existing connection is likely stalled. Currently there are situations where we can't reliably tell when a connection is dead without waiting for it to timeout. Since the current stack is only capable of dealing with one request, we have to wait for the existing connection to fail before starting a new one.

In this bug, a new connection implies a new socket. I don't think anything useful would come of starting a new multiplexed request over an existing socket.

Comment 3 by asanka@chromium.org, Sep 22 2016

Cc: asanka@chromium.org cbentzel@chromium.org mdw@chromium.org
 Issue 649467  has been merged into this issue.

Comment 4 by asanka@chromium.org, Sep 22 2016

Labels: M-55

Comment 5 by asanka@chromium.org, Dec 16 2016

Labels: -Pri-2 -M-55 Pri-3
Owner: ----
Status: Untriaged (was: Assigned)
I'm no longer working on downloads. Putting back on the triage queue so that it'll get prioritized during the next triage round.
Cc: qin...@chromium.org
Labels: -Pri-3 M-59 Pri-1
Owner: xingliu@chromium.org
Status: Assigned (was: Untriaged)
Assigning to Xing and Min to divide and conquer. :)
Project Member

Comment 7 by bugdroid1@chromium.org, Jan 25 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/18268bbb2566da62040b06c46f370f9ed6ea8964

commit 18268bbb2566da62040b06c46f370f9ed6ea8964
Author: qinmin <qinmin@chromium.org>
Date: Wed Jan 25 08:04:37 2017

Adding a new feature to enable/disable parallel downloading

will add the UI flag when more things are added

BUG=644352

Review-Url: https://codereview.chromium.org/2648393002
Cr-Commit-Position: refs/heads/master@{#445975}

[modify] https://crrev.com/18268bbb2566da62040b06c46f370f9ed6ea8964/chrome/common/chrome_features.cc
[modify] https://crrev.com/18268bbb2566da62040b06c46f370f9ed6ea8964/chrome/common/chrome_features.h

Project Member

Comment 8 by bugdroid1@chromium.org, Feb 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fed7466d012f7593c11843b2a98d3c4326c54ac9

commit fed7466d012f7593c11843b2a98d3c4326c54ac9
Author: xingliu <xingliu@chromium.org>
Date: Thu Feb 09 01:06:02 2017

Range request support for parallel download in DownloadRequestCore.

1. Add range request handling for parallel download, controlled
by DownloadUrlParameters. Currently put the logic in
DownloadRequestCore, if requests logic defers too much, we can pull out
to a new builder class.

Download resumption Range header: Range:100-
Parallel download Range header: Range:100-149

2. Add a unit test for DownloadRequestCore to ensure requests are
correctly built.

BUG=644352

Review-Url: https://codereview.chromium.org/2660783002
Cr-Commit-Position: refs/heads/master@{#449171}

[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/browser/download/download_request_core.cc
[add] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/browser/download/download_request_core_unittest.cc
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/public/browser/download_save_info.cc
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/public/browser/download_save_info.h
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/public/browser/download_url_parameters.h
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/content/test/BUILD.gn
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/net/http/http_request_headers.cc
[modify] https://crrev.com/fed7466d012f7593c11843b2a98d3c4326c54ac9/net/http/http_request_headers.h

Project Member

Comment 9 by bugdroid1@chromium.org, Feb 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfb00e35e0d1cff74c3b402029fb5310c95bed7c

commit bfb00e35e0d1cff74c3b402029fb5310c95bed7c
Author: qinmin <qinmin@chromium.org>
Date: Thu Feb 09 21:42:55 2017

Move parallDownloading feature to content/

This feature should not be dependant on the embedder.
Additionally, most of the download code rest in content/browser/.
Move the feature from chrome/ to content/

BUG=644352

Review-Url: https://codereview.chromium.org/2687973002
Cr-Commit-Position: refs/heads/master@{#449423}

[modify] https://crrev.com/bfb00e35e0d1cff74c3b402029fb5310c95bed7c/chrome/common/chrome_features.cc
[modify] https://crrev.com/bfb00e35e0d1cff74c3b402029fb5310c95bed7c/chrome/common/chrome_features.h
[modify] https://crrev.com/bfb00e35e0d1cff74c3b402029fb5310c95bed7c/content/public/common/content_features.cc
[modify] https://crrev.com/bfb00e35e0d1cff74c3b402029fb5310c95bed7c/content/public/common/content_features.h

Project Member

Comment 10 by bugdroid1@chromium.org, Feb 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5

commit b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5
Author: qinmin <qinmin@chromium.org>
Date: Wed Feb 15 06:47:47 2017

add a download slices table into history download db

To support parallel downloads, a download item may have multiple jobs.
For each job, it will write to the target file with a different offset. we need to store the information for each piece of slice so download resumption can work.
This CL adds a new table to store the slice information.
This change also increases the history db version by 1

BUG=644352

Review-Url: https://codereview.chromium.org/2665243003
Cr-Commit-Position: refs/heads/master@{#450606}

[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/chrome/browser/download/download_history.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/chrome/browser/safe_browsing/incident_reporting/last_download_finder_unittest.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/BUILD.gn
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_database.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_database.h
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_row.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_row.h
[add] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_slice_info.cc
[add] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_slice_info.h
[add] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_slice_info_unittest.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_types.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/download_types.h
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/history_backend_db_unittest.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/browser/history_database.cc
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/history/core/test/history_backend_db_base_test.cc
[add] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/components/test/data/history/history.32.sql
[modify] https://crrev.com/b2925e8fc7cacf09215b6c9c12b975fb7a9bdee5/tools/metrics/histograms/histograms.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f456c408998b5178f9f72810f417d57ffacb6da2

commit f456c408998b5178f9f72810f417d57ffacb6da2
Author: qinmin <qinmin@chromium.org>
Date: Fri Feb 17 10:20:54 2017

Refactor BaseFile class to support  sparse files

To support parallel downloading, a file might have multiple writers at the same time.
This CL refactors the BaseFile class to support sparse files.
It adds a method to allow callers to write to arbitrary offsets.
And this CL skips all the hash validation logic if the |is_sparse_file_| is true.

BUG=644352

Review-Url: https://codereview.chromium.org/2695153002
Cr-Commit-Position: refs/heads/master@{#451276}

[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/base_file.cc
[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/base_file.h
[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/base_file_unittest.cc
[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/base_file_win_unittest.cc
[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/f456c408998b5178f9f72810f417d57ffacb6da2/content/browser/download/save_file.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b

commit 468824d8e6ce6def7e144e9a72ae706f4b0b1b2b
Author: xingliu <xingliu@chromium.org>
Date: Tue Feb 28 02:59:25 2017

Introduce ParallelDownloadJob.

1. Add ParallelDownloadJob and DownloadUrlJob, both inherits DownloadJob
owned by DownloadItemImpl, currently we only do necessary refactoring
for parallel download feature, and failure handling or wiring to file
IO logic is not implemented yet.

2. Modify DownloadJob, remove any function we don't need. Since we need
to access many things in DownloadItemImpl, now it cache a raw pointer,
and is a friend class of DownloadItemImpl.

3. ParallelDownloadJob holds a list of DownloadUrlTask, which uses
UrlDownloader to send requests.

4. Tweak for UrlDownloader, previously UrlDownloader is coupled with
DownloadManagerImpl and everything has to go through
DownloadManagerImpl, now it uses a weak pointer delegate to communicate
to its owner, i.e. DownloadManagerImpl or DownloadUrlTask.

5. Pull out MockDownloadItemImpl to a seperate source file.

BUG=644352

Review-Url: https://codereview.chromium.org/2689373003
Cr-Commit-Position: refs/heads/master@{#453478}

[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/BUILD.gn
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_create_info.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_create_info.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_item_impl.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job.h
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job_factory.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job_factory.h
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job_impl.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job_impl.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_job_unittest.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_manager_impl.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_request_core.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_worker.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/download_worker.h
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/mock_download_item_impl.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/mock_download_item_impl.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/mock_download_job.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/mock_download_job.h
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/parallel_download_job.cc
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/parallel_download_job.h
[add] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/url_downloader.cc
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/browser/download/url_downloader.h
[modify] https://crrev.com/468824d8e6ce6def7e144e9a72ae706f4b0b1b2b/content/test/BUILD.gn

Project Member

Comment 15 by bugdroid1@chromium.org, Mar 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/992dacf666e6750a1911caebe43b29c93ea4d65f

commit 992dacf666e6750a1911caebe43b29c93ea4d65f
Author: xingliu <xingliu@chromium.org>
Date: Fri Mar 03 23:28:46 2017

Make DownloadFileImpl handle multiple byte streams.

DownloadFileImpl now hold multiple byte streams, add a wrapper class
for each stream reader.

Didn't hook to slice info or parallel job.

BUG=644352

Review-Url: https://codereview.chromium.org/2712713007
Cr-Commit-Position: refs/heads/master@{#454709}

[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_file.h
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_file_factory.cc
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_file_impl.h
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/download_stats.h
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/browser/download/mock_download_file.h
[modify] https://crrev.com/992dacf666e6750a1911caebe43b29c93ea4d65f/content/public/test/test_file_error_injector.cc

Project Member

Comment 16 by bugdroid1@chromium.org, Mar 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4373e6482a70511b1598f3f4de0170f357c06ae1

commit 4373e6482a70511b1598f3f4de0170f357c06ae1
Author: qinmin <qinmin@chromium.org>
Date: Mon Mar 06 22:47:09 2017

Pass slice info to DownloadFileImpl

When DownloadFileImpl is constructed, some slices may have already been downloaded.
Pass such information to it so it can check if all file content are downloaded.

BUG=644352

Review-Url: https://codereview.chromium.org/2734493003
Cr-Commit-Position: refs/heads/master@{#454985}

[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_file_factory.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_file_factory.h
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_file_impl.h
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/4373e6482a70511b1598f3f4de0170f357c06ae1/content/public/test/test_file_error_injector.cc

Project Member

Comment 17 by bugdroid1@chromium.org, Mar 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1

commit 2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1
Author: qinmin <qinmin@chromium.org>
Date: Tue Mar 07 21:10:18 2017

Change FindNextSliceToDownload() into FindSlicesToDownload()

For parallel downloads, we need a helper method to return a list of slices to download.
This CL changes FindNextSliceToDownload() into FindSlicesToDownload().
And it will return an array of slices to be downloaded.
This allows the ParallelDownloadJob to fork new request for each slice.

BUG=644352

Review-Url: https://codereview.chromium.org/2727143005
Cr-Commit-Position: refs/heads/master@{#455221}

[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/download_job_factory.cc
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/2bb2241a9aa5907ede2c661f1d4d1a8d9da017a1/content/browser/download/parallel_download_utils_unittest.cc

Project Member

Comment 18 by bugdroid1@chromium.org, Mar 8 2017

Project Member

Comment 19 by bugdroid1@chromium.org, Mar 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bc81898122086cf8a510c854583eefee34c1f022

commit bc81898122086cf8a510c854583eefee34c1f022
Author: qinmin <qinmin@chromium.org>
Date: Wed Mar 08 23:45:41 2017

Initialize the download file with correct bytes_so_far value

When parallel download is enabled, the bytes_so_far value is not equal to the offset in save_info.
Get the information from received_slices_ instead.

BUG=644352

Review-Url: https://codereview.chromium.org/2736613009
Cr-Commit-Position: refs/heads/master@{#455598}

[modify] https://crrev.com/bc81898122086cf8a510c854583eefee34c1f022/content/browser/download/download_file_impl.cc

Project Member

Comment 20 by bugdroid1@chromium.org, Mar 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cde3c3a4b12244b382390ee59b5484074e364b6e

commit cde3c3a4b12244b382390ee59b5484074e364b6e
Author: qinmin <qinmin@chromium.org>
Date: Tue Mar 14 19:52:32 2017

Update the received slices vector when stream is written to disk

Whenever new data is written to disk, we should update the received slices vector.
This could impact existing SourceStream objects as their length could get truncated.
This CL introduces the logic to update both the received slices vector and update existing SourceStream objects.
Because the update is very frequent, SourceStream now holds an index to the vector.
So that they can update the vector efficiently.

BUG=644352

Review-Url: https://codereview.chromium.org/2737033002
Cr-Commit-Position: refs/heads/master@{#456796}

[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/download_file_impl.h
[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/cde3c3a4b12244b382390ee59b5484074e364b6e/content/browser/download/parallel_download_utils_unittest.cc

Project Member

Comment 21 by bugdroid1@chromium.org, Mar 15 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c8aefe72593bb63ccc20dd34fae9ea034b3d709

commit 9c8aefe72593bb63ccc20dd34fae9ea034b3d709
Author: qinmin <qinmin@chromium.org>
Date: Wed Mar 15 00:05:22 2017

Verify that all slice are downloaded when a stream complete

To determine if a download finishes, we check if all streams are completed.
However, some of the byte streams might not have been added to DownloadFileImpl.
As a result, this CL checks if all the slices are written.

BUG=644352

Review-Url: https://codereview.chromium.org/2738063002
Cr-Commit-Position: refs/heads/master@{#456903}

[modify] https://crrev.com/9c8aefe72593bb63ccc20dd34fae9ea034b3d709/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/9c8aefe72593bb63ccc20dd34fae9ea034b3d709/content/browser/download/download_file_impl.h
[modify] https://crrev.com/9c8aefe72593bb63ccc20dd34fae9ea034b3d709/content/browser/download/download_file_unittest.cc

Project Member

Comment 22 by bugdroid1@chromium.org, Mar 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a217ace927455448460006a47fc00331dd4ca30a

commit a217ace927455448460006a47fc00331dd4ca30a
Author: xingliu <xingliu@chromium.org>
Date: Thu Mar 16 21:36:35 2017

Add a delay to send the parallel requests.

After the response of the original request, and a delay, parallel
requests are sent.

The delay is finch configurable, and default value is 0, so unit tests
don't take the delay.

BUG=644352

Review-Url: https://codereview.chromium.org/2750853004
Cr-Commit-Position: refs/heads/master@{#457564}

[modify] https://crrev.com/a217ace927455448460006a47fc00331dd4ca30a/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/a217ace927455448460006a47fc00331dd4ca30a/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/a217ace927455448460006a47fc00331dd4ca30a/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/a217ace927455448460006a47fc00331dd4ca30a/content/browser/download/parallel_download_utils.h

Project Member

Comment 23 by bugdroid1@chromium.org, Mar 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/457f1988af83ecf544aa76a8ee8126f6ac980c41

commit 457f1988af83ecf544aa76a8ee8126f6ac980c41
Author: qinmin <qinmin@chromium.org>
Date: Fri Mar 17 19:08:06 2017

Move the logic to determine how much data can be written to another function

The logic to calculate how much data to write is getting bigger,
move to a separate function.
This CL also adds a check if the target location is already written.
If so, it will request the current stream to terminate.
Unit test will pass once https://codereview.chromium.org/2737033002/ lands

BUG=644352

Review-Url: https://codereview.chromium.org/2744793003
Cr-Commit-Position: refs/heads/master@{#457839}

[modify] https://crrev.com/457f1988af83ecf544aa76a8ee8126f6ac980c41/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/457f1988af83ecf544aa76a8ee8126f6ac980c41/content/browser/download/download_file_impl.h
[modify] https://crrev.com/457f1988af83ecf544aa76a8ee8126f6ac980c41/content/browser/download/download_file_unittest.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Mar 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6719c20584c6c916a4f5820a0f3b458ab612dfad

commit 6719c20584c6c916a4f5820a0f3b458ab612dfad
Author: xingliu <xingliu@chromium.org>
Date: Sat Mar 18 03:45:21 2017

Glue parallel download job and download file together.

1. Connect the parallel download job to download file, so we can
generate requests and push the byte streams to download file.

2. Move 2 utility functions into parallel_download_util.

3. Tweak the test fixture for parallel download job, now it can take
a ReceivedSlice. And we target to test BuildParallelRequests now.
So we can add unittests for resumption later. The unittests for
communication between classes will be added later.

4. Refactor download_file_unittests, now it may accept more streams.

BUG=644352

Review-Url: https://codereview.chromium.org/2742093002
Cr-Commit-Position: refs/heads/master@{#457952}

[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_create_info.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_create_info.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_file.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_file_impl.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_job.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_job.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_job_factory.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_request_core.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_worker.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/download_worker.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/mock_download_file.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/mock_download_item_impl.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/mock_download_item_impl.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/6719c20584c6c916a4f5820a0f3b458ab612dfad/content/public/browser/download_item.h

Project Member

Comment 25 by bugdroid1@chromium.org, Mar 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33

commit 0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33
Author: qinmin <qinmin@chromium.org>
Date: Mon Mar 20 22:56:48 2017

Add UMA for estimating disk bandwidth and the time saved with parallel download

This CL adds UMAs for disk bandwiths for parallel download when:
1. there are only 1 request active
2. there are multiple requests active

It also adds a UMA to estimate the time saved through parallel download

BUG=644352

Review-Url: https://codereview.chromium.org/2750713003
Cr-Commit-Position: refs/heads/master@{#458217}

[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_file.h
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_file_impl.h
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_stats.cc
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/download_stats.h
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/content/browser/download/mock_download_file.h
[modify] https://crrev.com/0c5570d5cf495210b600a0b6fa3f3ec0b53a7b33/tools/metrics/histograms/histograms.xml

Project Member

Comment 29 by bugdroid1@chromium.org, Mar 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6dbfc80be8169ac5e0ffc7f051432ab8681de7bb

commit 6dbfc80be8169ac5e0ffc7f051432ab8681de7bb
Author: qinmin <qinmin@chromium.org>
Date: Thu Mar 23 04:40:30 2017

Allow the initial request to take over failed parallel requests.

When a parallel request fails, DownloadFileImpl will send an error and cause download to be interrupted.
This could cause a potential problem:
If a server always fail addtional requests, these requests will cause the download to interrupt.
As a result, the initial request and the download can go nowhere.
This CL addresses the issue by allowing the initial request to continue,
if it is still alive and can cover the slice to be downloaded by the failed request.
If the initial request cannot download the slice left by the parallel request, an error will be sent.

Will add a test once https://codereview.chromium.org/2742093002/ lands

BUG=644352

Review-Url: https://codereview.chromium.org/2748103014
Cr-Commit-Position: refs/heads/master@{#459005}

[modify] https://crrev.com/6dbfc80be8169ac5e0ffc7f051432ab8681de7bb/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/6dbfc80be8169ac5e0ffc7f051432ab8681de7bb/content/browser/download/download_file_impl.h

Project Member

Comment 30 by bugdroid1@chromium.org, Mar 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aa5d2f477221a57cc51cdef53279c3b1453f63d3

commit aa5d2f477221a57cc51cdef53279c3b1453f63d3
Author: qinmin <qinmin@chromium.org>
Date: Fri Mar 24 23:55:06 2017

Add a helper method to truncate SourceStream's length

To support half open requests, we need to truncate all SourceStreams when they came.
This CL adds a helper method to truncate the SourceStream when they are active.
And it fixes an issue that the usage of 0 can be very unclear.
Also this CL makes the initial request half open so we can get the correct content-length.

BUG=644352

Review-Url: https://codereview.chromium.org/2770303002
Cr-Commit-Position: refs/heads/master@{#459603}

[modify] https://crrev.com/aa5d2f477221a57cc51cdef53279c3b1453f63d3/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/aa5d2f477221a57cc51cdef53279c3b1453f63d3/content/browser/download/download_file_impl.h
[modify] https://crrev.com/aa5d2f477221a57cc51cdef53279c3b1453f63d3/content/browser/download/download_item_impl.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Mar 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cca0315b59850a2d21409f77e6c9c970cb8a47e2

commit cca0315b59850a2d21409f77e6c9c970cb8a47e2
Author: xingliu <xingliu@chromium.org>
Date: Mon Mar 27 20:54:23 2017

Handle early pause, cancel for parallel requests.

Currently we have an issue that Pause, Cancel can be called before
building the parallel requests or before adding the byte stream reader
to the file sink.

In this case, we should still call request handle's pause or cancel to
block the pipe or destroy the pipe.

BUG= 702683 , 644352

Review-Url: https://codereview.chromium.org/2767593003
Cr-Commit-Position: refs/heads/master@{#459873}

[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/download_job.cc
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/download_job.h
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/download_worker.cc
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/download_worker.h
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/cca0315b59850a2d21409f77e6c9c970cb8a47e2/content/browser/download/parallel_download_job_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Mar 29 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/75184f444eb8f9460f6d4ee36a90706eab762564

commit 75184f444eb8f9460f6d4ee36a90706eab762564
Author: qinmin <qinmin@chromium.org>
Date: Wed Mar 29 17:23:44 2017

Add more UMA to record whether parallel download is completed/interrupted/cancelled

This CL adds more UMA to record parallel download status,
so that we can study how parallel requests will impact download.

BUG=644352

Review-Url: https://codereview.chromium.org/2769933003
Cr-Commit-Position: refs/heads/master@{#460436}

[modify] https://crrev.com/75184f444eb8f9460f6d4ee36a90706eab762564/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/75184f444eb8f9460f6d4ee36a90706eab762564/content/browser/download/download_stats.cc
[modify] https://crrev.com/75184f444eb8f9460f6d4ee36a90706eab762564/content/browser/download/download_stats.h
[modify] https://crrev.com/75184f444eb8f9460f6d4ee36a90706eab762564/tools/metrics/histograms/histograms.xml

Project Member

Comment 33 by bugdroid1@chromium.org, Mar 30 2017

Project Member

Comment 34 by bugdroid1@chromium.org, Mar 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6d05e2a6461b0886140633e9a98a01e7c780dd5

commit d6d05e2a6461b0886140633e9a98a01e7c780dd5
Author: xingliu <xingliu@chromium.org>
Date: Thu Mar 30 19:56:01 2017

Fix issues and feature polishing for parallel download.

1. Fix a bug in ParallelDownloadJob that the initial request may
generate a ReceivedSlice, and currently it will hit a DCHECK.

2. Cover the edge cases that there might be holes before the initial
request offset, now we also create requests for them, but don't chunk
more slices in this scenario. (Talked offline, we might not need this logic when some later CL checked in, but leave this here for now.)

3. When chunk into smaller slices, also consider minimum slice size
Finch configuration. So we won't have extremely small new slices.

BUG=644352

Review-Url: https://codereview.chromium.org/2783473002
Cr-Commit-Position: refs/heads/master@{#460858}

[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/download_create_info.h
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/download_request_core.cc
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/d6d05e2a6461b0886140633e9a98a01e7c780dd5/content/browser/download/parallel_download_utils_unittest.cc

Project Member

Comment 35 by bugdroid1@chromium.org, Mar 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b791e0b97910713700bf8e4f3396521cff290d3b

commit b791e0b97910713700bf8e4f3396521cff290d3b
Author: xingliu <xingliu@chromium.org>
Date: Thu Mar 30 23:38:58 2017

Add control to use If-Range header for range request.

Currently If-Range is used in all half opened request, including normal
download resumption, and parallel download subsequent requests.

However parallel download subsequent requests may not necessarily use
If-Range header.

This CL adds a parameter to control if If-Range is sent in the request.

Also refactored the code to attach partial request headers.

BUG= 706181 , 644352

Review-Url: https://codereview.chromium.org/2786523002
Cr-Commit-Position: refs/heads/master@{#460930}

[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/browser/download/download_request_core.cc
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/browser/download/download_request_core.h
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/browser/download/download_request_core_unittest.cc
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/browser/download/download_worker.cc
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/public/browser/download_url_parameters.cc
[modify] https://crrev.com/b791e0b97910713700bf8e4f3396521cff290d3b/content/public/browser/download_url_parameters.h

Project Member

Comment 36 by bugdroid1@chromium.org, Apr 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec174ac638779aa4db3573285a40b513e98c5e93

commit ec174ac638779aa4db3573285a40b513e98c5e93
Author: xingliu <xingliu@chromium.org>
Date: Mon Apr 03 22:19:58 2017

Add UMA metric to track parallel download requests stats.

Add requests count to track the actual requests sent and special
requests(offset smaller than initial request offset) sent in parallel
download.

Add a boolean metric to track if the file sink is gone when we try to
push a byte stream reader to the sink.

BUG=644352

Review-Url: https://codereview.chromium.org/2789623005
Cr-Commit-Position: refs/heads/master@{#461556}

[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/download_create_info.h
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/download_request_core.cc
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/download_stats.cc
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/download_stats.h
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/content/browser/download/parallel_download_job_unittest.cc
[modify] https://crrev.com/ec174ac638779aa4db3573285a40b513e98c5e93/tools/metrics/histograms/histograms.xml

Project Member

Comment 37 by bugdroid1@chromium.org, Apr 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ba57766474b9e525e858ae8382d8acbdec34039

commit 3ba57766474b9e525e858ae8382d8acbdec34039
Author: qinmin <qinmin@chromium.org>
Date: Tue Apr 04 01:15:38 2017

set content length limit if half open range request fails

If a range request fails, normally we should report an error.
However, if the request is half open, it means we are reaching the end of the file.
When resuming a download, we get the slice info from the history db.
However, we cannot tell if the last slice has reached the end of the stream.
As a result, a half open request is always sent.
And this request can be used to determine the content length.

BUG=644352

Review-Url: https://codereview.chromium.org/2794683002
Cr-Commit-Position: refs/heads/master@{#461595}

[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_file.h
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_file_impl.h
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_job.cc
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_job.h
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/download_worker.cc
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/mock_download_file.h
[modify] https://crrev.com/3ba57766474b9e525e858ae8382d8acbdec34039/content/browser/download/parallel_download_job.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Apr 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/583a61ad6d736c81b9f82c9e24676af19ce868e9

commit 583a61ad6d736c81b9f82c9e24676af19ce868e9
Author: xingliu <xingliu@chromium.org>
Date: Mon Apr 10 22:12:37 2017

Clear the received slices in DownloadItemImpl when etag changed.

For parallel download, the received slices are currently passed in the
constructor of DownloadFileImpl, which is earlier than the etag check
logic in DownloadItemImpl. This raises an issue that the
DownloadFileImpl will use the out-dated received slices data. It will
block the download to refresh the data with new strong validators, and
download won't complete.

This CL cleared the received slices in DownloadItemImpl, and pass the
received slices to DownloadFileImpl via Initialize call instead of the
ctor.

Alternatively we may do it in DownloadManagerImpl, but it's probably
better to do this kind of clean up in DownloadItemImpl.

When etag changes, currently for a normal download, we update the etag
in DownloadItemImpl::UpdateValidatorsOnResumption, and
DownloadSaveInfo::offset is set to 0 in
DownloadRequestCore::HandleSuccessfulServerResponse, where we pass this
0 offset to BaseFile::Initialize to truncate the file size to 0.

BUG= 706182 , 644352

Review-Url: https://codereview.chromium.org/2799333002
Cr-Commit-Position: refs/heads/master@{#463417}

[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_factory.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_factory.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_impl.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/mock_download_file.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/public/test/test_file_error_injector.cc

Project Member

Comment 39 by bugdroid1@chromium.org, Apr 11 2017

Project Member

Comment 40 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/10603e265ae3928a1d9ea44470d82976d001bd3b

commit 10603e265ae3928a1d9ea44470d82976d001bd3b
Author: qinmin <qinmin@chromium.org>
Date: Thu Apr 13 03:33:44 2017

Record the parallel download UMA for fresh new downloads

Start() can be called on both resumption and fresh new download
This CL records only the parallel downloads that are new

BUG=644352

Review-Url: https://codereview.chromium.org/2810133003
Cr-Commit-Position: refs/heads/master@{#464278}

[modify] https://crrev.com/10603e265ae3928a1d9ea44470d82976d001bd3b/content/browser/download/download_item_impl.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Apr 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4f3c18cf2a88b9a88f42fc18461522f718685e59

commit 4f3c18cf2a88b9a88f42fc18461522f718685e59
Author: qinmin <qinmin@chromium.org>
Date: Thu Apr 13 20:50:27 2017

Properly handle the case when a parallel download becomes non-parallel

When finch is used, it is possible that user can resume a previous parallel
download without enabling parallel downloading.
This change handles that case by truncating the file
and discard all the received slices

BUG=644352

Review-Url: https://codereview.chromium.org/2812263004
Cr-Commit-Position: refs/heads/master@{#464538}

[modify] https://crrev.com/4f3c18cf2a88b9a88f42fc18461522f718685e59/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/4f3c18cf2a88b9a88f42fc18461522f718685e59/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/4f3c18cf2a88b9a88f42fc18461522f718685e59/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/4f3c18cf2a88b9a88f42fc18461522f718685e59/content/browser/download/parallel_download_utils_unittest.cc

Project Member

Comment 42 by bugdroid1@chromium.org, Apr 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b444a98187571cf9ec627ac52fee7e81ffcecb93

commit b444a98187571cf9ec627ac52fee7e81ffcecb93
Author: xingliu <xingliu@chromium.org>
Date: Tue Apr 18 18:09:49 2017

Fix an issue that we didn't clean url request properly.

The ByteStreamReader we used in DownloadFile does not support Close()
api, so we need to close the url request on UI thread if
"ByteStreamReader::Close" should be called.

Before we implement parallel download, we null out the callback on
ByteStreamReader during download interruption, and on UI thread, we
cancel the url request. But for parallel download, at some point, we
won't interrupt the download but need to cancel a particular request.

There is another CL that implements a browser test for parallel download and it will test the code here.
https://codereview.chromium.org/2819483002/

BUG= 710576 , 644352

Review-Url: https://codereview.chromium.org/2811293004
Cr-Commit-Position: refs/heads/master@{#465289}

[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_file.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_file_impl.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_item_impl.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_job.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/download_job.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/mock_download_file.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/b444a98187571cf9ec627ac52fee7e81ffcecb93/content/public/test/test_file_error_injector.cc

Project Member

Comment 43 by bugdroid1@chromium.org, Apr 19 2017

Labels: merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b

commit 8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b
Author: Xing Liu <xingliu@chromium.org>
Date: Wed Apr 19 18:38:18 2017

Fix an issue that we didn't clean url request properly.

The ByteStreamReader we used in DownloadFile does not support Close()
api, so we need to close the url request on UI thread if
"ByteStreamReader::Close" should be called.

Before we implement parallel download, we null out the callback on
ByteStreamReader during download interruption, and on UI thread, we
cancel the url request. But for parallel download, at some point, we
won't interrupt the download but need to cancel a particular request.

There is another CL that implements a browser test for parallel download and it will test the code here.
https://codereview.chromium.org/2819483002/

BUG= 710576 , 644352

Review-Url: https://codereview.chromium.org/2811293004
Cr-Commit-Position: refs/heads/master@{#465289}
(cherry picked from commit b444a98187571cf9ec627ac52fee7e81ffcecb93)

Review-Url: https://codereview.chromium.org/2828073002 .
Cr-Commit-Position: refs/branch-heads/3071@{#60}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_file.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_file_impl.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_item_impl.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_job.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/download_job.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/mock_download_file.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/8cb70eaa5ea6568193561a95fc04bdcd4cd8fe4b/content/public/test/test_file_error_injector.cc

Project Member

Comment 44 by bugdroid1@chromium.org, Apr 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a022d09fac4eae6cadf496a3f743488bbb60dafd

commit a022d09fac4eae6cadf496a3f743488bbb60dafd
Author: xingliu <xingliu@chromium.org>
Date: Wed Apr 19 22:55:50 2017

Add fieldtrial_testing_config for parallel downloading.

This is needed for beta finch config.

BUG=644352

Review-Url: https://codereview.chromium.org/2826253002
Cr-Commit-Position: refs/heads/master@{#465796}

[modify] https://crrev.com/a022d09fac4eae6cadf496a3f743488bbb60dafd/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 45 by bugdroid1@chromium.org, Apr 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2269e30d4ea0874704bfbd4021c087ba960e7ea5

commit 2269e30d4ea0874704bfbd4021c087ba960e7ea5
Author: qinmin <qinmin@chromium.org>
Date: Thu Apr 20 21:04:52 2017

Add new UMA stats for parallelizable download

To study the effectiveness of parallel download,
we want to compare the bandwidth if parallel downloads are not parallelized.
Because parallel download has requirements on file size, so comparing with all
other downloads is be fair.
This CL records the UMA metrics for downloads that satisfy the requirement
of parallel download, while don't use parallel requests.

BUG=644352, 712892 

Review-Url: https://codereview.chromium.org/2823273004
Cr-Commit-Position: refs/heads/master@{#466128}

[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_file.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_file_impl.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_job.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_job.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_job_factory.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_job_impl.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_job_impl.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_stats.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/download_stats.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/mock_download_file.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/content/public/test/test_file_error_injector.cc
[modify] https://crrev.com/2269e30d4ea0874704bfbd4021c087ba960e7ea5/tools/metrics/histograms/histograms.xml

Project Member

Comment 47 by bugdroid1@chromium.org, Apr 24 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f7823c3140937e3fbfdc8eee48dfd780707b78e3

commit f7823c3140937e3fbfdc8eee48dfd780707b78e3
Author: Min Qin <qinmin@chromium.org>
Date: Mon Apr 24 17:44:06 2017

Add new UMA stats for parallelizable download

To study the effectiveness of parallel download,
we want to compare the bandwidth if parallel downloads are not parallelized.
Because parallel download has requirements on file size, so comparing with all
other downloads is be fair.
This CL records the UMA metrics for downloads that satisfy the requirement
of parallel download, while don't use parallel requests.

BUG=644352, 712892 
TBR=dtrainor@chromium.org,alexmos@chromium.org,isherman@chromium.org

Review-Url: https://codereview.chromium.org/2823273004
Cr-Commit-Position: refs/heads/master@{#466128}
(cherry picked from commit 2269e30d4ea0874704bfbd4021c087ba960e7ea5)

Review-Url: https://codereview.chromium.org/2839693002 .
Cr-Commit-Position: refs/branch-heads/3071@{#168}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_file.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_file_impl.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_job.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_job.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_job_factory.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_job_impl.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_job_impl.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_stats.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/download_stats.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/mock_download_file.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/parallel_download_job.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/parallel_download_job.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/parallel_download_utils.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/browser/download/parallel_download_utils.h
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/content/public/test/test_file_error_injector.cc
[modify] https://crrev.com/f7823c3140937e3fbfdc8eee48dfd780707b78e3/tools/metrics/histograms/histograms.xml

Project Member

Comment 48 by bugdroid1@chromium.org, Apr 26 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aca52687045830c9b42e288df02ddcf4a01618f7

commit aca52687045830c9b42e288df02ddcf4a01618f7
Author: xingliu <xingliu@chromium.org>
Date: Wed Apr 26 18:59:15 2017

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}

[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/download_file_impl.h
[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/download_job.cc
[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/download_job.h
[modify] https://crrev.com/aca52687045830c9b42e288df02ddcf4a01618f7/content/browser/download/parallel_download_job.cc

Sign in to add a comment