New issue
Advanced search Search tips

Issue 706182 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Parallel Download, when precondition fails, received slices and temporary file should be reset.

Project Member Reported by xingliu@chromium.org, Mar 28 2017

Issue description

For normal download, when precondition fails, since we use the If-Range header, the server will return http 200 and the full content where offset is 0.

content::BaseFile will truncate the file size to the offset, so precondition fails will result in the file being truncate to 0.

For parallel downloading, we don't truncate, so there is a problem that the file and the received_slices can be out-dated when precondition fails.

We should reset the file and flush away the received_slices when this happens.
 
Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/583a61ad6d736c81b9f82c9e24676af19ce868e9

commit 583a61ad6d736c81b9f82c9e24676af19ce868e9
Author: xingliu <xingliu@chromium.org>
Date: Mon Apr 10 22:12:37 2017

Clear the received slices in DownloadItemImpl when etag changed.

For parallel download, the received slices are currently passed in the
constructor of DownloadFileImpl, which is earlier than the etag check
logic in DownloadItemImpl. This raises an issue that the
DownloadFileImpl will use the out-dated received slices data. It will
block the download to refresh the data with new strong validators, and
download won't complete.

This CL cleared the received slices in DownloadItemImpl, and pass the
received slices to DownloadFileImpl via Initialize call instead of the
ctor.

Alternatively we may do it in DownloadManagerImpl, but it's probably
better to do this kind of clean up in DownloadItemImpl.

When etag changes, currently for a normal download, we update the etag
in DownloadItemImpl::UpdateValidatorsOnResumption, and
DownloadSaveInfo::offset is set to 0 in
DownloadRequestCore::HandleSuccessfulServerResponse, where we pass this
0 offset to BaseFile::Initialize to truncate the file size to 0.

BUG= 706182 , 644352

Review-Url: https://codereview.chromium.org/2799333002
Cr-Commit-Position: refs/heads/master@{#463417}

[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_browsertest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_factory.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_factory.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_impl.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_file_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_item_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_item_impl_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_manager_impl.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/download_manager_impl_unittest.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/mock_download_file.cc
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/browser/download/mock_download_file.h
[modify] https://crrev.com/583a61ad6d736c81b9f82c9e24676af19ce868e9/content/public/test/test_file_error_injector.cc

Status: Fixed (was: Started)

Sign in to add a comment