Change FindNextSliceToDownload() into FindSlicesToDownload()

content/browser/download/parallel_download_utils_unittest.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_utils.h"
 
 #include "content/public/browser/download_save_info.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
-TEST(ParallelDownloadUtilsTest, FindNextSliceToDownload) {
-  std::vector<DownloadItem::ReceivedSlice> slices;
-  DownloadItem::ReceivedSlice slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(0, slice.offset);
-  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent, slice.received_bytes);
+TEST(ParallelDownloadUtilsTest, FindSlicesToDownload) {
+  std::vector<DownloadItem::ReceivedSlice> downloaded_slices;
+  std::vector<DownloadItem::ReceivedSlice> slices_to_download =
+      FindSlicesToDownload(downloaded_slices);
+  EXPECT_EQ(1u, slices_to_download.size());
+  EXPECT_EQ(0, slices_to_download[0].offset);
+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent,
+            slices_to_download[0].received_bytes);
 
-  slices.emplace_back(0, 500);
-  slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(500, slice.offset);
-  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent, slice.received_bytes);
+  downloaded_slices.emplace_back(0, 500);
+  slices_to_download = FindSlicesToDownload(downloaded_slices);
+  EXPECT_EQ(1u, slices_to_download.size());
+  EXPECT_EQ(500, slices_to_download[0].offset);
+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent,
+            slices_to_download[0].received_bytes);
 
   // Create a gap between slices.
-  slices.emplace_back(1000, 500);
-  slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(500, slice.offset);
-  EXPECT_EQ(500, slice.received_bytes);
+  downloaded_slices.emplace_back(1000, 500);
+  slices_to_download = FindSlicesToDownload(downloaded_slices);
+  EXPECT_EQ(2u, slices_to_download.size());
+  EXPECT_EQ(500, slices_to_download[0].offset);
+  EXPECT_EQ(500, slices_to_download[0].received_bytes);
+  EXPECT_EQ(1500, slices_to_download[1].offset);
+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent,
+            slices_to_download[1].received_bytes);
 
   // Fill the gap.
-  slices.emplace(slices.begin() + 1, slice);
-  slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(1500, slice.offset);
-  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent, slice.received_bytes);
+  downloaded_slices.emplace(
+      downloaded_slices.begin() + 1, slices_to_download[0]);
+  slices_to_download = FindSlicesToDownload(downloaded_slices);
+  EXPECT_EQ(1u, slices_to_download.size());
+  EXPECT_EQ(1500, slices_to_download[0].offset);
+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent,
+            slices_to_download[0].received_bytes);
 
   // Create a new gap at the beginning.
-  slices.erase(slices.begin());
-  slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(0, slice.offset);
-  EXPECT_EQ(500, slice.received_bytes);
+  downloaded_slices.erase(downloaded_slices.begin());
+  slices_to_download = FindSlicesToDownload(downloaded_slices);
+  EXPECT_EQ(2u, slices_to_download.size());
+  EXPECT_EQ(0, slices_to_download[0].offset);
+  EXPECT_EQ(500, slices_to_download[0].received_bytes);
+  EXPECT_EQ(1500, slices_to_download[1].offset);

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

The last slice returned, e.g. [1500, 0]. How does the job knows if it should
create a request or not? Does it mean the job must compare the content-length to
figure out if it the last slice is done?

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

Yes, it has to figure it out by itself.
This issue is not related to this Change, but it is related to parallel
downloading itself. For example, if I created 2 parallel requests, and the 2nd
requests finishes early. Then if browser crashes before the 1st request
finishes, we won't know if we should start the 2nd request on resumption. Unless
we store another boolean variable in history db to indicate that the slice is
finished, we won't have a answer.
We could:
1. check the content-length if it is there. Though this can still introduce some
issues if the content-length is wrong and the 2nd request happened to download
content-length bytes last time.
2. or, Always create a request at the end, i think this is a safer solution

+  EXPECT_EQ(DownloadSaveInfo::kLengthFullContent,
+            slices_to_download[1].received_bytes);
 }
 
 }  // namespace content