Change FindNextSliceToDownload() into FindSlicesToDownload()

content/browser/download/parallel_download_utils_unittest.cc

Index: content/browser/download/parallel_download_utils_unittest.cc
diff --git a/content/browser/download/parallel_download_utils_unittest.cc b/content/browser/download/parallel_download_utils_unittest.cc
index abab9428a6c8786ea138d6e008086c47408fcb04..f6c52a9e38aa4fcf5b5af50000ad280e2efa3472 100644
--- a/content/browser/download/parallel_download_utils_unittest.cc
+++ b/content/browser/download/parallel_download_utils_unittest.cc
@@ -9,34 +9,50 @@
 
 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);
-
-  slices.emplace_back(0, 500);
-  slice = FindNextSliceToDownload(slices);
-  EXPECT_EQ(500, 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);
+
+  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