Change FindNextSliceToDownload() into FindSlicesToDownload()

content/browser/download/parallel_download_job.cc

Index: content/browser/download/parallel_download_job.cc
diff --git a/content/browser/download/parallel_download_job.cc b/content/browser/download/parallel_download_job.cc
index 41ba75beceffe372b04a1a4a06f4c0cc739e4bd8..bc06750fa0847b383f20d10a47c006aefa1aa4a9 100644
--- a/content/browser/download/parallel_download_job.cc
+++ b/content/browser/download/parallel_download_job.cc
@@ -5,6 +5,8 @@
 #include "content/browser/download/parallel_download_job.h"
 
 #include "base/memory/ptr_util.h"
+#include "content/browser/download/download_create_info.h"
+#include "content/browser/download/parallel_download_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/storage_partition.h"
 
@@ -21,12 +23,33 @@ const int kParallelRequestCount = 2;
 
 ParallelDownloadJob::ParallelDownloadJob(
     DownloadItemImpl* download_item,
-    std::unique_ptr<DownloadRequestHandleInterface> request_handle)
+    std::unique_ptr<DownloadRequestHandleInterface> request_handle,
+    const DownloadCreateInfo& create_info)
     : DownloadJobImpl(download_item, std::move(request_handle)),
-      request_num_(kParallelRequestCount) {}
+      request_num_(kParallelRequestCount),
+      initial_request_offset_(create_info.save_info->offset),
+      initial_request_length_(create_info.save_info->length) {}
 
 ParallelDownloadJob::~ParallelDownloadJob() = default;
 
+void ParallelDownloadJob::Start() {
+  DownloadJobImpl::Start();
+
+  std::vector<DownloadItem::ReceivedSlice> slices_to_download =

[xingliu, 2017/03/03 18:44:15]

There is already a function to build requests for new parallel download. But
unified logic for both new download and resumption like this one is probably
better.

[qinmin, 2017/03/03 19:32:35]

Start() is called on both resumption and new download, so this is the single
entry point. If this is a new download,  |slice_to_download| will be the whole
file, and we need to split it before we can create additional workers. If this
is a resumption, then |slice_to_download| will contain all the slices to fill
the gaps.

We should remove ForkRequestsForNewDownload() method and implement the if/else
statement below. But i will leave that to a separate CL.

[qinmin, 2017/03/03 19:44:33]

To clarify, Start() is called when Resuming an interrupted download, while
Resume(true) is called when resuming a paused download. Either one or the other
will get called, but not both.

[xingliu, 2017/03/03 21:37:44]

sgtm.

+      FindSlicesToDownload(download_item_->GetReceivedSlices());
+  // The initial request should cover the first slice.
+  DCHECK_GE(slices_to_download[0].offset, initial_request_offset_);
+  DCHECK(initial_request_length_ == DownloadSaveInfo::kLengthFullContent ||
+         initial_request_offset_ + initial_request_length_ >=
+             slices_to_download[0].offset +
+             slices_to_download[0].received_bytes);
+  if (slices_to_download.size() >= kParallelRequestCount) {

[xingliu, 2017/03/03 18:44:15]

Maybe we can keep the mechanism simple for the first iteration: 

1. For new download, fork N requests, and save N slice info in the database.

2. Download resumption, check the N slice, see if any of them need to be patched
up. Each slice at most uses one request.

Continue build sub requests or build them too dynamically is hard to manage and
may bring more and more db IO and network overhead, especially for unstable
network.


Also since we don't put logic here, maybe just put a TODO.

[qinmin, 2017/03/03 19:32:35]

For resumption, there could be less than N slices. This is because we forked the
parallel request, but they never yield any data before browser gets killed. So
we should also fork new request if the last slice is really large, and there are
less than N slices available.

added a TODO here.

[xingliu, 2017/03/03 21:37:44]

In general sgtm if the slices are written to db after the file received data.  

Just for discussion here, that does it make sense to write slice info even
before that, like when we first figure out the draft to chunk the file?

[qinmin, 2017/03/03 22:09:43]

what do you mean by "write slice info"? 
You mean write the info into history db? Then the answer is no. The slice info
should never reach the history db unless actual data are written. The history db
will also ignore slices that has 0 bytes of data. 
History db commit is lagging the actual file thread, so it is perfectly fine
that 1000 bytes is written to disk while history db only stores 500.

+      // Get the next |kParallelRequestCount - 1| slices and fork new requests.
+  } else {
+    // Split the last slice and assign it to different workers.
+  }
+}
+
 void ParallelDownloadJob::Cancel(bool user_cancel) {
   DownloadJobImpl::Cancel(user_cancel);
   for (auto& worker : workers_)