Download: Fix an issue that download percentage can move backwards.

On certain Android devices, when disconnecting from WIFI, the network
code will report NETWORK_TIMEOUT before NETWORK_DISCONNECT and trigger
auto download resumption. And we erase the data downloaded by parallel
requests, so the download percentage will go backwards.

Instead, we should keep the slice info when resume with an error.

Bug: 837128
Change-Id: I002c69ac4d5e1c33133e4dbddd44c3c5173a494e
Reviewed-on: https://chromium-review.googlesource.com/1112754Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Min Qin <qinmin@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571286}

components/download/internal/common/download_item_impl.cc

@@ -1443,17 +1443,14 @@ void DownloadItemImpl::Start(
     // DownloadCreateInfo with an intact DownloadSaveInfo.
     DCHECK(new_create_info.save_info);
 
-    int64_t offset = new_create_info.save_info->offset;
     std::unique_ptr<crypto::SecureHash> hash_state =
         new_create_info.save_info->hash_state
             ? new_create_info.save_info->hash_state->Clone()
             : nullptr;
 
-    destination_info_.received_bytes = offset;
     hash_state_ = std::move(hash_state);
     destination_info_.hash.clear();
     deferred_interrupt_reason_ = new_create_info.result;
-    received_slices_.clear();
     TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
     DetermineDownloadTarget();
     return;

components/download/internal/common/download_item_impl_unittest.cc

@@ -1056,6 +1056,45 @@ TEST_F(DownloadItemTest, ClearReceivedSliceIfEtagChanged) {
   CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
 }
 
+// Ensure when a network socket error happens on resumption, the received slices
+// info should be kept if the download is not restarted from beginning, so the
+// download progress will not move backward.
+TEST_F(DownloadItemTest, KeepReceivedSliceIfNetworkError) {
+  const char kFirstETag[] = "ABC";
+  const DownloadItem::ReceivedSlices kReceivedSlice = {
+      DownloadItem::ReceivedSlice(0, 10), DownloadItem::ReceivedSlice(20, 30)};
+  create_info()->etag = kFirstETag;
+
+  DownloadItemImpl* item = CreateDownloadItem();
+  MockDownloadFile* download_file =
+      DoIntermediateRename(item, DOWNLOAD_DANGER_TYPE_NOT_DANGEROUS);
+
+  EXPECT_CALL(*mock_delegate(), MockResumeInterruptedDownload(_));
+  EXPECT_CALL(*download_file, Detach());
+
+  item->DestinationObserverAsWeakPtr()->DestinationUpdate(20, 100,
+                                                          kReceivedSlice);
+  EXPECT_EQ(kReceivedSlice, item->GetReceivedSlices());
+  EXPECT_EQ(20, item->GetReceivedBytes());
+
+  item->DestinationObserverAsWeakPtr()->DestinationError(
+      DOWNLOAD_INTERRUPT_REASON_FILE_TRANSIENT_ERROR, 20 /* bytes_so_far */,
+      std::unique_ptr<crypto::SecureHash>());
+  task_environment_.RunUntilIdle();
+
+  // Simulate a socket error, and start the download.
+  create_info()->result = DOWNLOAD_INTERRUPT_REASON_NETWORK_TIMEOUT;
+  DownloadItemImplDelegate::DownloadTargetCallback target_callback;
+  download_file = CallDownloadItemStart(item, &target_callback);
+
+  // After starting the download, the slice info and received bytes should not
+  // change.
+  EXPECT_EQ(kReceivedSlice, item->GetReceivedSlices());
+  EXPECT_EQ(20, item->GetReceivedBytes());
+
+  CleanupItem(item, download_file, DownloadItem::IN_PROGRESS);
+}
+
 // Test that resumption uses the final URL in a URL chain when resuming.
 TEST_F(DownloadItemTest, ResumeUsesFinalURL) {
   create_info()->save_info->prompt_for_save_location = false;