Change FindNextSliceToDownload() into FindSlicesToDownload()

content/browser/download/parallel_download_job.cc

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 
 namespace content {
 
 namespace {
 
 // TODO(xingliu): Use finch parameters to configure constants.
 // Default number of requests in a parallel download, including the original
 // request.
 const int kParallelRequestCount = 2;
 
 }  // namespace
 
 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_)
     worker->Cancel();
 }
 
 void ParallelDownloadJob::Pause() {
   DownloadJobImpl::Pause();
   for (auto& worker : workers_)
     worker->Pause();
@@ skipping 54 matching lines @@
   // Subsequent range requests have the same referrer URL as the original
   // download request.
   download_params->set_referrer(Referrer(download_item_->GetReferrerUrl(),
                                          blink::WebReferrerPolicyAlways));
   // Send the request.
   worker->SendRequest(std::move(download_params));
   workers_.push_back(std::move(worker));
 }
 
 }  // namespace content