Move the logic to determine how much data can be written to another function

content/browser/download/download_file_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/download/download_file_impl.h"
 
 #include <string>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 138 matching lines @@
 }
 
 DownloadInterruptReason DownloadFileImpl::WriteDataToFile(int64_t offset,
                                                           const char* data,
                                                           size_t data_len) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   WillWriteToDisk(data_len);
   return file_.WriteDataToFile(offset, data, data_len);
 }
 
+bool DownloadFileImpl::CalculateBytesToWrite(
+    SourceStream* source_stream,
+    size_t bytes_read,

[David Trainor- moved to gerrit, 2017/03/15 22:35:42]

bytes_available_to_write more clear?

[qinmin, 2017/03/16 00:15:49]

Done.

+    size_t *bytes_to_write) {
+  // If a new slice find that its target position has already been written,

[David Trainor- moved to gerrit, 2017/03/15 22:35:42]

find -> finds

[qinmin, 2017/03/16 00:15:49]

Done.

+  // terminate the stream.
+  if (source_stream->bytes_written() == 0) {
+    for (const auto& received_slice : received_slices_) {
+      if (received_slice.offset <= source_stream->offset() &&
+          received_slice.offset + received_slice.received_bytes >
+              source_stream->offset()) {
+        *bytes_to_write = 0;
+        return true;
+      }
+    }
+  }
+
+  if (source_stream->length() != DownloadSaveInfo::kLengthFullContent &&

[David Trainor- moved to gerrit, 2017/03/15 22:35:42]

Is source_stream->length() DownloadSaveInfo::kLengthFullContent for both the
first and last slice?  If it's the first slice and we get a chunk of data that
spans the boundary of the start of the second slice, we won't stop and will
bypass this block and return false to keep reading right?

It'll kind of be okay because it'll break during the next read, but still seems
incorrect.

[qinmin, 2017/03/16 00:15:49]

No, that won't happen. If the second slice starts to write,
DownloadFileImpl::AddNewSlice() will change the length of the first slice. So it
will never span across the boundary.

[David Trainor- moved to gerrit, 2017/03/17 07:26:51]

Thanks for the explanation.  Sounds good!

+      source_stream->bytes_written() +
+              static_cast<int64_t>(bytes_read) >= source_stream->length()) {
+    // Write a partial buffer as the incoming data exceeds the length limit.
+    *bytes_to_write = source_stream->length() - source_stream->bytes_written();
+    return true;
+  }
+
+  *bytes_to_write = bytes_read;
+  return false;
+}
+
 void DownloadFileImpl::RenameAndUniquify(
     const base::FilePath& full_path,
     const RenameCompletionCallback& callback) {
   std::unique_ptr<RenameParameters> parameters(
       new RenameParameters(UNIQUIFY, full_path, callback));
   RenameWithRetryInternal(std::move(parameters));
 }
 
 void DownloadFileImpl::RenameAndAnnotate(
     const base::FilePath& full_path,
@@ skipping 116 matching lines @@
 }
 
 void DownloadFileImpl::StreamActive(SourceStream* source_stream) {
   DCHECK(source_stream->stream_reader());
   base::TimeTicks start(base::TimeTicks::Now());
   base::TimeTicks now;
   scoped_refptr<net::IOBuffer> incoming_data;
   size_t incoming_data_size = 0;
   size_t total_incoming_data_size = 0;
   size_t num_buffers = 0;
+  size_t bytes_to_write = 0;
   bool should_terminate = false;
   ByteStreamReader::StreamState state(ByteStreamReader::STREAM_EMPTY);
   DownloadInterruptReason reason = DOWNLOAD_INTERRUPT_REASON_NONE;
   base::TimeDelta delta(
       base::TimeDelta::FromMilliseconds(kMaxTimeBlockingFileThreadMs));
 
   // Take care of any file local activity required.
   do {
     state = source_stream->stream_reader()->Read(&incoming_data,
                                                  &incoming_data_size);
 
     switch (state) {
       case ByteStreamReader::STREAM_EMPTY:
         break;
       case ByteStreamReader::STREAM_HAS_DATA:
         {
           ++num_buffers;
           base::TimeTicks write_start(base::TimeTicks::Now());
-          // Stop the stream if it writes more bytes than expected.
-          if (source_stream->length() != DownloadSaveInfo::kLengthFullContent &&
-              source_stream->bytes_written() +
-                      static_cast<int64_t>(incoming_data_size) >=
-                  source_stream->length()) {
-            should_terminate = true;
-            incoming_data_size =
-                source_stream->length() - source_stream->bytes_written();
-          }
+          should_terminate = CalculateBytesToWrite(
+              source_stream, incoming_data_size, &bytes_to_write);

[David Trainor- moved to gerrit, 2017/03/15 22:35:42]

Just for sanity sake, DCHECK_GE(incoming_data_size, bytes_to_write)?

[qinmin, 2017/03/16 00:15:49]

Done.

+
           reason = WriteDataToFile(
               source_stream->offset() + source_stream->bytes_written(),
-              incoming_data.get()->data(), incoming_data_size);
+              incoming_data.get()->data(), bytes_to_write);
           disk_writes_time_ += (base::TimeTicks::Now() - write_start);
-          bytes_seen_ += incoming_data_size;
-          total_incoming_data_size += incoming_data_size;
+          bytes_seen_ += bytes_to_write;
+          total_incoming_data_size += bytes_to_write;
           if (reason == DOWNLOAD_INTERRUPT_REASON_NONE) {
             int64_t prev_bytes_written = source_stream->bytes_written();
-            source_stream->OnWriteBytesToDisk(incoming_data_size);
+            source_stream->OnWriteBytesToDisk(bytes_to_write);
             if (!is_sparse_file_)
               break;
             // If the write operation creates a new slice, add it to the
             // |received_slices_| and update all the entries in
             // |source_streams_|.
-            if (incoming_data_size > 0 && prev_bytes_written == 0) {
-              AddNewSlice(source_stream->offset(), incoming_data_size);
+            if (bytes_to_write > 0 && prev_bytes_written == 0) {
+              AddNewSlice(source_stream->offset(), bytes_to_write);
             } else {
               received_slices_[source_stream->index()].received_bytes +=
-                  incoming_data_size;
+                  bytes_to_write;
             }
           }
         }
         break;
       case ByteStreamReader::STREAM_COMPLETE:
         {
           reason = static_cast<DownloadInterruptReason>(
             source_stream->stream_reader()->GetStatus());
           SendUpdate();
         }
@@ skipping 137 matching lines @@
     const base::FilePath& new_path,
     const RenameCompletionCallback& completion_callback)
     : option(option),
       new_path(new_path),
       retries_left(kMaxRenameRetries),
       completion_callback(completion_callback) {}
 
 DownloadFileImpl::RenameParameters::~RenameParameters() {}
 
 }  // namespace content