Remove length from DownloadSaveInfo

All the parallel requests are now half open, no need for this field.
And it is very strange to pass a request's length to download file, as
the download file should know the whole file length, rather than
an individual stream's length.

BUG=1011427

Change-Id: Id29b5c2859441e7bfffee165bc760c183b5a6882
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1841183
Commit-Queue: Min Qin <qinmin@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#703539}

components/download/internal/common/download_file_impl.cc

@@ -59,11 +59,10 @@ const int kUnknownContentLength = -1;
 
 DownloadFileImpl::SourceStream::SourceStream(
     int64_t offset,
-    int64_t length,
     int64_t starting_file_write_offset,
     std::unique_ptr<InputStream> stream)
     : offset_(offset),
-      length_(length),
+      length_(DownloadSaveInfo::kLengthFullContent),
       starting_file_write_offset_(starting_file_write_offset),
       bytes_read_(0),
       bytes_written_(0),
@@ -72,11 +71,6 @@ DownloadFileImpl::SourceStream::SourceStream(
       input_stream_(std::move(stream)) {
   CHECK_LE(offset_, starting_file_write_offset_);
   CHECK_GE(offset_, 0);
-  DCHECK(length <= 0 || length >= starting_file_write_offset - offset)
-      << "Not enough for content validation. offset = " << offset
-      << ", length = " << length
-      << " , starting_file_write_offset = " << starting_file_write_offset
-      << ".";
 }
 
 DownloadFileImpl::SourceStream::~SourceStream() = default;
@@ -176,8 +170,8 @@ DownloadFileImpl::DownloadFileImpl(
                                     download_id);
 
   source_streams_[save_info_->offset] = std::make_unique<SourceStream>(
-      save_info_->offset, save_info_->length,
-      save_info_->GetStartingFileWriteOffset(), std::move(stream));
+      save_info_->offset, save_info_->GetStartingFileWriteOffset(),
+      std::move(stream));
 
   DETACH_FROM_SEQUENCE(sequence_checker_);
 }
@@ -243,8 +237,7 @@ void DownloadFileImpl::Initialize(
 }
 
 void DownloadFileImpl::AddInputStream(std::unique_ptr<InputStream> stream,
-                                      int64_t offset,
-                                      int64_t length) {
+                                      int64_t offset) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
   // UI thread may not be notified about completion and detach download file,
@@ -255,7 +248,7 @@ void DownloadFileImpl::AddInputStream(std::unique_ptr<InputStream> stream,
   }
   DCHECK(source_streams_.find(offset) == source_streams_.end());
   source_streams_[offset] =
-      std::make_unique<SourceStream>(offset, length, offset, std::move(stream));
+      std::make_unique<SourceStream>(offset, offset, std::move(stream));
   OnSourceStreamAdded(source_streams_[offset].get());
 }
 

components/download/internal/common/download_file_unittest.cc

@@ -438,7 +438,7 @@ class DownloadFileTest : public testing::Test {
     // 1. RegisterCallback: Must called twice. One to set the callback, the
     // other to release the stream.
     // 2. Read: If filled with N buffer, called (N+1) times, where the last Read
-    // call doesn't read any data but returns STRAM_COMPLETE.
+    // call doesn't read any data but returns STREAM_COMPLETE.
     // The stream may terminate in the middle and less Read calls are expected.
     // 3. GetStatus: Only called if the stream is completed and last Read call
     // returns STREAM_COMPLETE.
@@ -997,8 +997,8 @@ TEST_F(DownloadFileTest, MultipleStreamsWrite) {
 
   // Activate the streams.
   download_file_->AddInputStream(
-      std::unique_ptr<MockInputStream>(additional_streams_[0]), stream_0_length,
-      DownloadSaveInfo::kLengthFullContent);
+      std::unique_ptr<MockInputStream>(additional_streams_[0]),
+      stream_0_length);
   sink_callback_.Run(MOJO_RESULT_OK);
   base::RunLoop().RunUntilIdle();
 
@@ -1039,12 +1039,12 @@ TEST_F(DownloadFileTest, MultipleStreamsLimitedLength) {
   EXPECT_CALL(*(observer_.get()), MockDestinationCompleted(_, _));
 
   // Activate all the streams.
-  download_file_->AddInputStream(
-      std::unique_ptr<MockInputStream>(additional_streams_[0]), stream_0_length,
-      stream_1_length);
-  download_file_->AddInputStream(
+   download_file_->AddInputStream(
       std::unique_ptr<MockInputStream>(additional_streams_[1]),
-      stream_0_length + stream_1_length, DownloadSaveInfo::kLengthFullContent);
+      stream_0_length + stream_1_length);
+  download_file_->AddInputStream(
+      std::unique_ptr<MockInputStream>(additional_streams_[0]),
+      stream_0_length);
   sink_callback_.Run(MOJO_RESULT_OK);
   base::RunLoop().RunUntilIdle();
 
@@ -1085,7 +1085,7 @@ TEST_F(DownloadFileTest, MultipleStreamsFirstStreamWriteAllData) {
   additional_streams_[0] = new StrictMock<MockInputStream>();
   download_file_->AddInputStream(
       std::unique_ptr<MockInputStream>(additional_streams_[0]),
-      stream_0_length - 1, DownloadSaveInfo::kLengthFullContent);
+      stream_0_length - 1);
   base::RunLoop().RunUntilIdle();
 
   SourceStreamTestData stream_data_0(0, stream_0_length, true);
@@ -1127,7 +1127,7 @@ TEST_F(DownloadFileTest, SecondStreamStartingOffsetAlreadyWritten) {
 
   download_file_->AddInputStream(
       std::unique_ptr<MockInputStream>(additional_streams_[0]),
-      strlen(kTestData1), DownloadSaveInfo::kLengthFullContent);
+      strlen(kTestData1));
 
   // The stream should get terminated and reset the callback.
   EXPECT_TRUE(sink_callback_.is_null());
@@ -1169,8 +1169,7 @@ TEST_F(DownloadFileTest, SecondStreamReadsOffsetWrittenByFirst) {
       .RetiresOnSaturation();
   int64_t offset = strlen(kTestData1) + strlen(kTestData2);
   download_file_->AddInputStream(
-      std::unique_ptr<MockInputStream>(additional_streams_[0]), offset,
-      DownloadSaveInfo::kLengthFullContent);
+      std::unique_ptr<MockInputStream>(additional_streams_[0]), offset);
   base::RunLoop().RunUntilIdle();
 
   // First stream reads the 3rd chunk and writes it to disk.

components/download/internal/common/download_job.cc

@@ -76,8 +76,7 @@ void DownloadJob::OnDownloadFileInitialized(
 }
 
 bool DownloadJob::AddInputStream(std::unique_ptr<InputStream> stream,
-                                 int64_t offset,
-                                 int64_t length) {
+                                 int64_t offset) {
   DownloadFile* download_file = download_item_->GetDownloadFile();
   if (!download_file) {
     CancelRequestWithOffset(offset);
@@ -90,7 +89,7 @@ bool DownloadJob::AddInputStream(std::unique_ptr<InputStream> stream,
   GetDownloadTaskRunner()->PostTask(
       FROM_HERE, base::BindOnce(&DownloadFile::AddInputStream,
                                 base::Unretained(download_file),
-                                std::move(stream), offset, length));
+                                std::move(stream), offset));
   return true;
 }
 

components/download/internal/common/download_utils.cc

@@ -177,15 +177,8 @@ DownloadInterruptReason HandleSuccessfulServerResponse(
     return result;
 
   // The caller is expecting a partial response.
-  if (save_info && (save_info->offset > 0 || save_info->length > 0)) {
+  if (save_info && save_info->offset > 0) {
     if (http_headers.response_code() != net::HTTP_PARTIAL_CONTENT) {
-      // Server should send partial content when "If-Match" or
-      // "If-Unmodified-Since" check passes, and the range request header has
-      // last byte position. e.g. "Range:bytes=50-99".
-      if (save_info->length != DownloadSaveInfo::kLengthFullContent &&
-          !fetch_error_body)
-        return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
-
       // Requested a partial range, but received the entire response, when
       // the range request header is "Range:bytes={offset}-".
       // The response can be HTTP 200 or other error code when
@@ -204,9 +197,7 @@ DownloadInterruptReason HandleSuccessfulServerResponse(
       return DOWNLOAD_INTERRUPT_REASON_SERVER_BAD_CONTENT;
     DCHECK_GE(first_byte, 0);
 
-    if (first_byte != save_info->offset ||
-        (save_info->length > 0 &&
-         last_byte != save_info->offset + save_info->length - 1)) {
+    if (first_byte != save_info->offset) {
       // The server returned a different range than the one we requested. Assume
       // the server doesn't support range request.
       return DOWNLOAD_INTERRUPT_REASON_SERVER_NO_RANGE;
@@ -337,8 +328,7 @@ int GetLoadFlags(DownloadUrlParameters* params, bool has_upload_data) {
 std::unique_ptr<net::HttpRequestHeaders> GetAdditionalRequestHeaders(
     DownloadUrlParameters* params) {
   auto headers = std::make_unique<net::HttpRequestHeaders>();
-  if (params->offset() == 0 &&
-      params->length() == DownloadSaveInfo::kLengthFullContent) {
+  if (params->offset() == 0) {
     AppendExtraHeaders(headers.get(), params);
     return headers;
   }
@@ -361,10 +351,7 @@ std::unique_ptr<net::HttpRequestHeaders> GetAdditionalRequestHeaders(
 
   // Add "Range" header.
   std::string range_header =
-      (params->length() == DownloadSaveInfo::kLengthFullContent)
-          ? base::StringPrintf("bytes=%" PRId64 "-", params->offset())
-          : base::StringPrintf("bytes=%" PRId64 "-%" PRId64, params->offset(),
-                               params->offset() + params->length() - 1);
+      base::StringPrintf("bytes=%" PRId64 "-", params->offset());
   headers->SetHeader(net::HttpRequestHeaders::kRange, range_header);
 
   // Add "If-Range" headers.

components/download/internal/common/download_worker.cc

@@ -68,11 +68,9 @@ void CreateUrlDownloadHandler(
 }  // namespace
 
 DownloadWorker::DownloadWorker(DownloadWorker::Delegate* delegate,
-                               int64_t offset,
-                               int64_t length)
+                               int64_t offset)
     : delegate_(delegate),
       offset_(offset),
-      length_(length),
       is_paused_(false),
       is_canceled_(false),
       url_download_handler_(nullptr, base::OnTaskRunnerDeleter(nullptr)) {

components/download/internal/common/download_worker.h

@@ -38,13 +38,10 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker
         std::unique_ptr<DownloadCreateInfo> download_create_info) = 0;
   };
 
-  DownloadWorker(DownloadWorker::Delegate* delegate,
-                 int64_t offset,
-                 int64_t length);
+  DownloadWorker(DownloadWorker::Delegate* delegate, int64_t offset);
   virtual ~DownloadWorker();
 
   int64_t offset() const { return offset_; }
-  int64_t length() const { return length_; }
 
   // Send network request to ask for a download.
   void SendRequest(std::unique_ptr<DownloadUrlParameters> params,
@@ -74,9 +71,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadWorker
   // The starting position of the content for this worker to download.
   int64_t offset_;
 
-  // The length of the request. May be 0 to fetch to the end of the file.
-  int64_t length_;
-
   // States of the worker.
   bool is_paused_;
   bool is_canceled_;

components/download/internal/common/parallel_download_job.cc

@@ -128,8 +128,8 @@ void ParallelDownloadJob::OnInputStreamReady(
   // If server returns a wrong range, abort the parallel request.
   bool success = download_create_info->offset == worker->offset();
   if (success) {
-    success = DownloadJob::AddInputStream(std::move(input_stream),
-                                          worker->offset(), worker->length());
+    success =
+        DownloadJob::AddInputStream(std::move(input_stream), worker->offset());
   }
 
   RecordParallelDownloadAddStreamSuccess(
@@ -236,15 +236,14 @@ void ParallelDownloadJob::ForkSubRequests(
     // All parallel requests are half open, which sends request headers like
     // "Range:50-".
     // If server rejects a certain request, others should take over.
-    CreateRequest(it->offset, DownloadSaveInfo::kLengthFullContent);
+    CreateRequest(it->offset);
   }
 }
 
-void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) {
+void ParallelDownloadJob::CreateRequest(int64_t offset) {
   DCHECK(download_item_);
-  DCHECK_EQ(DownloadSaveInfo::kLengthFullContent, length);
 
-  auto worker = std::make_unique<DownloadWorker>(this, offset, length);
+  auto worker = std::make_unique<DownloadWorker>(this, offset);
 
   net::NetworkTrafficAnnotationTag traffic_annotation =
       net::DefineNetworkTrafficAnnotation("parallel_download_job", R"(
@@ -277,10 +276,6 @@ void ParallelDownloadJob::CreateRequest(int64_t offset, int64_t length) {
   download_params->set_etag(download_item_->GetETag());
   download_params->set_offset(offset);
 
-  // Setting the length will result in range request to fetch a slice of the
-  // file.
-  download_params->set_length(length);
-
   // Subsequent range requests don't need the "If-Range" header.
   download_params->set_use_if_range(false);
 

components/download/internal/common/parallel_download_job.h

@@ -88,9 +88,8 @@ class COMPONENTS_DOWNLOAD_EXPORT ParallelDownloadJob
   void ForkSubRequests(const DownloadItem::ReceivedSlices& slices_to_download);
 
   // Create one range request, virtual for testing. Range request will start
-  // from |offset| to |length|. Range request will be half open, e.g.
-  // "Range:50-" if |length| is 0.
-  virtual void CreateRequest(int64_t offset, int64_t length);
+  // from |offset| and will be half open.
+  virtual void CreateRequest(int64_t offset);
 
   // Information about the initial request when download is started.
   int64_t initial_request_offset_;

components/download/internal/common/parallel_download_job_unittest.cc

@@ -67,8 +67,8 @@ class ParallelDownloadJobForTest : public ParallelDownloadJob {
         min_slice_size_(min_slice_size),
         min_remaining_time_(min_remaining_time) {}
 
-  void CreateRequest(int64_t offset, int64_t length) override {
-    auto worker = std::make_unique<DownloadWorker>(this, offset, length);
+  void CreateRequest(int64_t offset) override {
+    auto worker = std::make_unique<DownloadWorker>(this, offset);
 
     DCHECK(workers_.find(offset) == workers_.end());
     workers_[offset] = std::move(worker);
@@ -155,7 +155,6 @@ class ParallelDownloadJobTest : public testing::Test {
   void VerifyWorker(int64_t offset, int64_t length) const {
     EXPECT_TRUE(job_->workers_.find(offset) != job_->workers_.end());
     EXPECT_EQ(offset, job_->workers_[offset]->offset());
-    EXPECT_EQ(length, job_->workers_[offset]->length());
   }
 
   void OnFileInitialized(DownloadInterruptReason result, int64_t bytes_wasted) {

components/download/internal/common/parallel_download_utils_unittest.cc

@@ -37,14 +37,12 @@ class ParallelDownloadUtilsRecoverErrorTest
 
   // Creates a source stream to test.
   std::unique_ptr<DownloadFileImpl::SourceStream> CreateSourceStream(
-      int64_t offset,
-      int64_t length) {
+      int64_t offset) {
     input_stream_ = new StrictMock<MockInputStream>();
     EXPECT_CALL(*input_stream_, GetCompletionStatus())
         .WillRepeatedly(Return(DOWNLOAD_INTERRUPT_REASON_NONE));
     return std::make_unique<DownloadFileImpl::SourceStream>(
-        offset, length, offset,
-        std::unique_ptr<MockInputStream>(input_stream_));
+        offset, offset, std::unique_ptr<MockInputStream>(input_stream_));
   }
 
  protected:
@@ -140,15 +138,14 @@ TEST_P(ParallelDownloadUtilsRecoverErrorTest,
   // Create a stream that will work on byte range "100-".
   const int kErrorStreamOffset = 100;
 
-  auto error_stream = CreateSourceStream(kErrorStreamOffset,
-                                         DownloadSaveInfo::kLengthFullContent);
+  auto error_stream = CreateSourceStream(kErrorStreamOffset);
   error_stream->set_finished(true);
 
   // Get starting offset of preceding stream.
   int64_t preceding_offset = GetParam();
   EXPECT_LT(preceding_offset, kErrorStreamOffset);
-  auto preceding_stream = CreateSourceStream(
-      preceding_offset, DownloadSaveInfo::kLengthFullContent);
+  auto preceding_stream = CreateSourceStream(preceding_offset);
+  // Half open preceding stream can always recover the error for later streams.
   EXPECT_FALSE(preceding_stream->is_finished());
   EXPECT_EQ(0u, preceding_stream->bytes_written());
   EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
@@ -170,88 +167,74 @@ TEST_P(ParallelDownloadUtilsRecoverErrorTest,
   preceding_stream->OnBytesConsumed(1000u, 1000u);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 
-  // Now capped the length of preceding stream with different values.
-  preceding_stream = CreateSourceStream(preceding_offset,
-                                        kErrorStreamOffset - preceding_offset);
-  // Since preceding stream can't reach the first byte of the error stream, it
-  // will fail.
+  int64_t bytes_consumed = kErrorStreamOffset - preceding_offset - 1;
+  // Half open successfully finished preceding stream should always be
+  // able to recover error, even if it is not reaching the error offset as the
+  // error stream might be requesting something our of range.
+  preceding_stream = CreateSourceStream(preceding_offset);
   preceding_stream->set_finished(false);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->set_finished(true);
-  int64_t bytes_consumed = kErrorStreamOffset - preceding_offset;
   preceding_stream->OnBytesConsumed(bytes_consumed, bytes_consumed);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
-
-  // Inject an error results in failure, even if data written exceeds the first
-  // byte of error stream.
-  EXPECT_CALL(*input_stream_, GetCompletionStatus())
-      .WillRepeatedly(Return(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE));
-  preceding_stream->OnBytesConsumed(1000u, 1000u);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  preceding_stream->OnBytesConsumed(1, 1);
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 
-  // Make preceding stream can reach the first byte of error stream.
-  preceding_stream = CreateSourceStream(
-      preceding_offset, kErrorStreamOffset - preceding_offset + 1);
-  // Since the error stream is half opened, no matter what it should fail.
-  preceding_stream->set_finished(false);
+  // If the preceding stream is truncated, it should never be able to recover
+  // a half open stream.
+  preceding_stream = CreateSourceStream(preceding_offset);
+  preceding_stream->TruncateLengthWithWrittenDataBlock(kErrorStreamOffset, 1);
+  EXPECT_EQ(preceding_stream->length(), kErrorStreamOffset - preceding_offset);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->set_finished(true);
+  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(bytes_consumed, bytes_consumed);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(1, 1);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
-
-  // Preceding stream that never download data won't recover the error stream.
-  preceding_stream = CreateSourceStream(preceding_offset, -1);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 }
 
 // Verify recovery for length capped error stream.
-// Since the error stream length is capped, assume the previous stream length
-// is also capped or the previous stream is finished due to error like http
-// 404.
 TEST_P(ParallelDownloadUtilsRecoverErrorTest,
        RecoverErrorForLengthCappedErrorStream) {
   // Create a stream that will work on byte range "100-150".
   const int kErrorStreamLength = 50;
-  auto error_stream =
-      CreateSourceStream(kErrorStreamOffset, kErrorStreamLength);
+  auto error_stream = CreateSourceStream(kErrorStreamOffset);
+  error_stream->TruncateLengthWithWrittenDataBlock(
+      kErrorStreamOffset + kErrorStreamLength, 1);
+  EXPECT_EQ(error_stream->length(), 50);
   error_stream->set_finished(true);
 
   // Get starting offset of preceding stream.
   const int64_t preceding_offset = GetParam();
   EXPECT_LT(preceding_offset, kErrorStreamOffset);
 
-  // Create preceding stream capped before starting offset of error stream.
-  auto preceding_stream = CreateSourceStream(
-      preceding_offset, kErrorStreamOffset - preceding_offset);
+  // Create an half open preceding stream.
+  auto preceding_stream = CreateSourceStream(preceding_offset);
   EXPECT_FALSE(preceding_stream->is_finished());
   EXPECT_EQ(0u, preceding_stream->bytes_written());
 
-  // Since the preceding stream can never reach the starting offset, for an
-  // unfinished stream, we rely on length instead of bytes written.
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  // Since the preceding stream can reach the starting offset, it should be able
+  // to recover the error stream..
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   int64_t bytes_consumed = kErrorStreamOffset - preceding_offset;
   preceding_stream->OnBytesConsumed(bytes_consumed, bytes_consumed);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(kErrorStreamLength - 1,
                                     kErrorStreamLength - 1);
-  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(1, 1);
-
-  // Create preceding stream that can reach the upper bound of error stream.
-  // Since it's unfinished, it potentially can take over error stream's work
-  // even if no data is written.
-  preceding_stream = CreateSourceStream(
-      preceding_offset,
-      kErrorStreamOffset - preceding_offset + kErrorStreamLength);
-  EXPECT_FALSE(preceding_stream->is_finished());
-  EXPECT_EQ(0u, preceding_stream->bytes_written());
   EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 
-  // Finished preceding stream only checks data written.
-  preceding_stream = CreateSourceStream(preceding_offset, 1);
+  // If preceding stream is truncated after error stream, checks data written.
+  preceding_stream = CreateSourceStream(preceding_offset);
+  preceding_stream->TruncateLengthWithWrittenDataBlock(
+      kErrorStreamOffset + kErrorStreamLength, 1);
+  EXPECT_EQ(preceding_stream->length(),
+            kErrorStreamOffset + kErrorStreamLength - preceding_offset);
+  EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->set_finished(true);
+  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(bytes_consumed, bytes_consumed);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
   preceding_stream->OnBytesConsumed(kErrorStreamLength - 1,
@@ -266,8 +249,17 @@ TEST_P(ParallelDownloadUtilsRecoverErrorTest,
       .WillRepeatedly(Return(DOWNLOAD_INTERRUPT_REASON_FILE_NO_SPACE));
   EXPECT_TRUE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 
-  // Preceding stream that never download data won't recover the error stream.
-  preceding_stream = CreateSourceStream(preceding_offset, -1);
+  // If preceding stream is truncated before or in the middle of error stream,
+  // it should not recover the error stream when it reaches its length.
+  preceding_stream = CreateSourceStream(preceding_offset);
+  preceding_stream->TruncateLengthWithWrittenDataBlock(kErrorStreamOffset + 1,
+                                                       1);
+  EXPECT_EQ(preceding_stream->length(),
+            kErrorStreamOffset + 1 - preceding_offset);
+  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  preceding_stream->set_finished(true);
+  EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
+  preceding_stream->OnBytesConsumed(bytes_consumed + 1, bytes_consumed + 1);
   EXPECT_FALSE(CanRecoverFromError(error_stream.get(), preceding_stream.get()));
 }
 

components/download/public/common/download_file.h

@@ -72,8 +72,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFile {
   // Add an input stream to write into a slice of the file, used for
   // parallel download.
   virtual void AddInputStream(std::unique_ptr<InputStream> stream,
-                              int64_t offset,
-                              int64_t length) = 0;
+                              int64_t offset) = 0;
 
   // Rename the download file to |full_path|.  If that file exists
   // |full_path| will be uniquified by suffixing " (<number>)" to the

components/download/public/common/download_file_impl.h

@@ -60,8 +60,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFileImpl : public DownloadFile {
                   const DownloadItem::ReceivedSlices& received_slices,
                   bool is_parallelizable) override;
   void AddInputStream(std::unique_ptr<InputStream> stream,
-                      int64_t offset,
-                      int64_t length) override;
+                      int64_t offset) override;
   void RenameAndUniquify(const base::FilePath& full_path,
                          const RenameCompletionCallback& callback) override;
   void RenameAndAnnotate(
@@ -102,7 +101,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadFileImpl : public DownloadFile {
   class COMPONENTS_DOWNLOAD_EXPORT SourceStream {
    public:
     SourceStream(int64_t offset,
-                 int64_t length,
                  int64_t starting_file_write_offset,
                  std::unique_ptr<InputStream> stream);
     ~SourceStream();

components/download/public/common/download_job.h

@@ -64,9 +64,7 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadJob {
 
   // Add an input stream to the download sink. Return false if we start to
   // destroy download file.
-  bool AddInputStream(std::unique_ptr<InputStream> stream,
-                      int64_t offset,
-                      int64_t length);
+  bool AddInputStream(std::unique_ptr<InputStream> stream, int64_t offset);
 
   DownloadItem* download_item_;
 

components/download/public/common/download_save_info.h

@@ -56,12 +56,6 @@ struct COMPONENTS_DOWNLOAD_EXPORT DownloadSaveInfo {
   // used for validation purpose.
   int64_t file_offset = -1;
 
-  // The number of the bytes to download from |offset|.
-  // Ask to retrieve segment of the download file when length is greater than 0.
-  // Request the rest of the file starting from |offset|, when length is
-  // |kLengthFullContent|.
-  int64_t length = kLengthFullContent;
-
   // The state of the hash. If specified, this hash state must indicate the
   // state of the partial file for the first |offset| bytes.
   std::unique_ptr<crypto::SecureHash> hash_state;

components/download/public/common/download_url_parameters.h

@@ -172,15 +172,8 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
 
   // If |offset| is non-zero, then a byte range request will be issued to fetch
   // the range of bytes starting at |offset|.
-  // Use |set_length| to specify the last byte position, or the range
-  // request will be "Range:bytes={offset}-" to retrieve the rest of the file.
   void set_offset(int64_t offset) { save_info_.offset = offset; }
 
-  // When |length| > 0, the range of bytes will be from
-  // |save_info_.offset| to |save_info_.offset| + |length| - 1.
-  // See |DownloadSaveInfo.length|.
-  void set_length(int64_t length) { save_info_.length = length; }
-
   // Sets the offset to start writing to the file. If set, The data received
   // before |file_offset| are discarded or are used for validation purpose.
   void set_file_offset(int64_t file_offset) {
@@ -302,7 +295,6 @@ class COMPONENTS_DOWNLOAD_EXPORT DownloadUrlParameters {
     return save_info_.suggested_name;
   }
   int64_t offset() const { return save_info_.offset; }
-  int64_t length() const { return save_info_.length; }
   const std::string& hash_of_partial_file() const {
     return save_info_.hash_of_partial_file;
   }

components/download/public/common/mock_download_file.cc

@@ -32,11 +32,10 @@ MockDownloadFile::MockDownloadFile() {
 MockDownloadFile::~MockDownloadFile() {}
 
 void MockDownloadFile::AddInputStream(std::unique_ptr<InputStream> input_stream,
-                                      int64_t offset,
-                                      int64_t length) {
+                                      int64_t offset) {
   // Gmock currently can't mock method that takes move-only parameters,
   // delegate the EXPECT_CALL count to |DoAddByteStream|.
-  DoAddInputStream(input_stream.get(), offset, length);
+  DoAddInputStream(input_stream.get(), offset);
 }
 
 }  // namespace download

components/download/public/common/mock_download_file.h

@@ -35,10 +35,9 @@ class MockDownloadFile : public DownloadFile {
                     const DownloadItem::ReceivedSlices& received_slices,
                     bool is_parallelizable));
   void AddInputStream(std::unique_ptr<InputStream> input_stream,
-                      int64_t offset,
-                      int64_t length) override;
-  MOCK_METHOD3(DoAddInputStream,
-               void(InputStream* input_stream, int64_t offset, int64_t length));
+                      int64_t offset) override;
+  MOCK_METHOD2(DoAddInputStream,
+               void(InputStream* input_stream, int64_t offset));
   MOCK_METHOD2(OnResponseCompleted,
                void(int64_t offset, DownloadInterruptReason status));
   MOCK_METHOD2(AppendDataToFile,