Commit 5b17c03b authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

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

This reverts commit 30c71bac.

Reason for revert: This CL needs to be cherry picked to m68 branch, but some testing function is changed in other CLs and breaks the build. So I'll revert this for now, check in a CL with production code only, and later land another for test.

Original change's description:
> 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/1112754
> Reviewed-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}

TBR=dtrainor@chromium.org,qinmin@chromium.org,xingliu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 837128
Change-Id: Id514b0f01b950d9940933fd17cb10d962ed69d85
Reviewed-on: https://chromium-review.googlesource.com/1129660
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573460}
parent 1fc231e8
...@@ -1443,14 +1443,17 @@ void DownloadItemImpl::Start( ...@@ -1443,14 +1443,17 @@ void DownloadItemImpl::Start(
// DownloadCreateInfo with an intact DownloadSaveInfo. // DownloadCreateInfo with an intact DownloadSaveInfo.
DCHECK(new_create_info.save_info); DCHECK(new_create_info.save_info);
int64_t offset = new_create_info.save_info->offset;
std::unique_ptr<crypto::SecureHash> hash_state = std::unique_ptr<crypto::SecureHash> hash_state =
new_create_info.save_info->hash_state new_create_info.save_info->hash_state
? new_create_info.save_info->hash_state->Clone() ? new_create_info.save_info->hash_state->Clone()
: nullptr; : nullptr;
destination_info_.received_bytes = offset;
hash_state_ = std::move(hash_state); hash_state_ = std::move(hash_state);
destination_info_.hash.clear(); destination_info_.hash.clear();
deferred_interrupt_reason_ = new_create_info.result; deferred_interrupt_reason_ = new_create_info.result;
received_slices_.clear();
TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL); TransitionTo(INTERRUPTED_TARGET_PENDING_INTERNAL);
DetermineDownloadTarget(); DetermineDownloadTarget();
return; return;
......
...@@ -1056,45 +1056,6 @@ TEST_F(DownloadItemTest, ClearReceivedSliceIfEtagChanged) { ...@@ -1056,45 +1056,6 @@ TEST_F(DownloadItemTest, ClearReceivedSliceIfEtagChanged) {
CleanupItem(item, download_file, DownloadItem::IN_PROGRESS); 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 that resumption uses the final URL in a URL chain when resuming.
TEST_F(DownloadItemTest, ResumeUsesFinalURL) { TEST_F(DownloadItemTest, ResumeUsesFinalURL) {
create_info()->save_info->prompt_for_save_location = false; create_info()->save_info->prompt_for_save_location = false;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment