Add a utility function to calculate the next slice to download

content/browser/download/parallel_download_utils.cc

Index: content/browser/download/parallel_download_utils.cc
diff --git a/content/browser/download/parallel_download_utils.cc b/content/browser/download/parallel_download_utils.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1999e726c6c0f0ecdc80f8bae1d30c0a4442eea3
--- /dev/null
+++ b/content/browser/download/parallel_download_utils.cc
@@ -0,0 +1,33 @@
+// 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_utils.h"
+
+#include "content/public/browser/download_save_info.h"
+
+namespace content {
+
+DownloadItem::ReceivedSlice FindNextSliceToDownload(
+    const std::vector<DownloadItem::ReceivedSlice>& received_slices) {
+  std::vector<DownloadItem::ReceivedSlice>::const_iterator iter =
+      received_slices.begin();
+  while (iter != received_slices.end()) {
+    int64_t offset = iter->offset + iter->received_bytes;
+    std::vector<DownloadItem::ReceivedSlice>::const_iterator next =
+        std::next(iter);
+    if (next == received_slices.end()) {

[xingliu, 2017/03/01 23:03:59]

Is it possible that the vector only contains one element such as [10,50]? where
we need to download the slice [0,10].

If it's impossible then we're safe.

[qinmin, 2017/03/01 23:25:28]

hmm... good catch, this is actually possible. The main request can fail while
workers can download content successfully, thus causing a hole at the beginning.

+      return DownloadItem::ReceivedSlice(offset,
+                                         DownloadSaveInfo::kLengthFullContent);
+    }
+
+    int64_t remaining_bytes = next->offset - offset;
+    DCHECK_LT(remaining_bytes, 0);

[xingliu, 2017/03/01 23:03:59]

Maybe DCHECK_GE(remaining_bytes, 0) or DCHECK_GT()?

If the vector contains [0,50], [50,100] that 2 slices are fully downloaded, then
remaining_bytes will be 0. The return value is correct. Can we add unit test for
this.

Since each slice maps to one db record, we probably won't merge them.

[qinmin, 2017/03/01 23:25:28]

Yes, this should be GE. Somehow I messed up the DCHECK when I was thinking
whether there is a -1 in calculating the offset. The test should already cover
the case you mentioned below.

we can actually merge the slices. In DownloadFileImpl::SendUpdate(), for
example, we can actually merge the slices and report it to DownloadItem.

On 2017/03/01 23:03:59, xingliu wrote:

[qinmin, 2017/03/01 23:41:16]

well, merging the slices requires 2 updates to the db. One with an empty vector,
and the other with a new vector, so it will require DownloadItem to update its
observers 2 times, which may not be very desirable. When download finishes, the
slice vector is cleared, so the slice information will be removed from db

[xingliu, 2017/03/02 00:23:51]

Agree, probably no need to merge. Also easier to debug and understand.

+    if (remaining_bytes > 0)
+      return DownloadItem::ReceivedSlice(offset, remaining_bytes);
+    iter = next;
+  }
+  return DownloadItem::ReceivedSlice(0, DownloadSaveInfo::kLengthFullContent);
+}
+
+}  // namespace content