remove is_sparse_file_ from DownloadFileImpl

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 74 matching lines @@
     length_ = offset - offset_;
   }
 }
 
 DownloadFileImpl::DownloadFileImpl(
     std::unique_ptr<DownloadSaveInfo> save_info,
     const base::FilePath& default_download_directory,
     std::unique_ptr<ByteStreamReader> stream_reader,
     const std::vector<DownloadItem::ReceivedSlice>& received_slices,
     const net::NetLogWithSource& download_item_net_log,
-    bool is_sparse_file,
     base::WeakPtr<DownloadDestinationObserver> observer)
     : net_log_(
           net::NetLogWithSource::Make(download_item_net_log.net_log(),
                                       net::NetLogSourceType::DOWNLOAD_FILE)),
       file_(net_log_),
       save_info_(std::move(save_info)),
       default_download_directory_(default_download_directory),
-      is_sparse_file_(is_sparse_file),
       bytes_seen_(0),
       num_active_streams_(0),
       record_stream_bandwidth_(true),
       bytes_seen_with_parallel_streams_(0),
       bytes_seen_without_parallel_streams_(0),
       received_slices_(received_slices),
       observer_(observer),
       weak_factory_(this) {
   source_streams_[save_info_->offset] = base::MakeUnique<SourceStream>(
       save_info_->offset, save_info_->length, std::move(stream_reader));
 
   download_item_net_log.AddEvent(
       net::NetLogEventType::DOWNLOAD_FILE_CREATED,
       net_log_.source().ToEventParametersCallback());
   net_log_.BeginEvent(
       net::NetLogEventType::DOWNLOAD_FILE_ACTIVE,
       download_item_net_log.source().ToEventParametersCallback());
 }
 
 DownloadFileImpl::~DownloadFileImpl() {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   net_log_.EndEvent(net::NetLogEventType::DOWNLOAD_FILE_ACTIVE);
 }
 
 void DownloadFileImpl::Initialize(const InitializeCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   update_timer_.reset(new base::RepeatingTimer());
   int64_t bytes_so_far = 0;
-  if (is_sparse_file_) {
+  if (IsSparseFile()) {
     for (const auto& received_slice : received_slices_) {
       bytes_so_far += received_slice.received_bytes;
     }
   } else {
     bytes_so_far = save_info_->offset;
   }
-  DownloadInterruptReason result =
-      file_.Initialize(save_info_->file_path, default_download_directory_,
-                       std::move(save_info_->file), bytes_so_far,
-                       save_info_->hash_of_partial_file,
-                       std::move(save_info_->hash_state), is_sparse_file_);
+  DownloadInterruptReason result = file_.Initialize(
+      save_info_->file_path, default_download_directory_,
+      std::move(save_info_->file), bytes_so_far,
+      save_info_->hash_of_partial_file, std::move(save_info_->hash_state),
+      IsSparseFile());
   if (result != DOWNLOAD_INTERRUPT_REASON_NONE) {
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE, base::Bind(callback, result));
     return;
   }
 
   download_start_ = base::TimeTicks::Now();
   last_update_time_ = download_start_;
 
   // Primarily to make reset to zero in restart visible to owner.
@@ skipping 10 matching lines @@
 
 void DownloadFileImpl::AddByteStream(
     std::unique_ptr<ByteStreamReader> stream_reader,
     int64_t offset,
     int64_t length) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
 
   source_streams_[offset] =
       base::MakeUnique<SourceStream>(offset, length, std::move(stream_reader));
 
+  // There are writers at different offsets now, create the received slices
+  // vector if necessary.
+  if (received_slices_.empty() && TotalBytesReceived() > 0) {
+    size_t index = AddOrMergeReceivedSliceIntoSortedArray(
+        DownloadItem::ReceivedSlice(0, TotalBytesReceived()), received_slices_);
+    DCHECK_EQ(index, 0u);
+  }
   // If the file is initialized, start to write data, or wait until file opened.
   if (file_.in_progress())
     RegisterAndActivateStream(source_streams_[offset].get());
 }
 
 DownloadInterruptReason DownloadFileImpl::WriteDataToFile(int64_t offset,
                                                           const char* data,
                                                           size_t data_len) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   WillWriteToDisk(data_len);
@@ skipping 198 matching lines @@
           DCHECK_GE(incoming_data_size, bytes_to_write);
           reason = WriteDataToFile(
               source_stream->offset() + source_stream->bytes_written(),
               incoming_data.get()->data(), bytes_to_write);
           disk_writes_time_ += (base::TimeTicks::Now() - write_start);
           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(bytes_to_write);
-            if (!is_sparse_file_)
+            if (!IsSparseFile())
               break;
             // If the write operation creates a new slice, add it to the
             // |received_slices_| and update all the entries in
             // |source_streams_|.
             if (bytes_to_write > 0 && prev_bytes_written == 0) {
               AddNewSlice(source_stream->offset(), bytes_to_write);

[xingliu, 2017/03/29 20:14:02]

nit%:  I might not fully understand the logic here, please ignore the comment if
I'm wrong.

The slice for initial request is also added in AddByteStream.

If the last function call in the task runner is AddByteStream for a second
stream, the ReceivedSlice for the initial request may be created twice if this
is the first disk IO for initial request.

[qinmin, 2017/03/29 20:44:08]

Shouldn't AddByteStream only called on parallel requests? The response for the
initial request is added in the ctor, and once another stream arrives, we will
create the received_slice vector

[xingliu, 2017/03/29 21:06:28]

Yeah, I mean (bytes_to_write > 0 && prev_bytes_written == 0) may pass for the
first source stream and try to add the slice twice. 

This is probably ok since new slice is merged to the vector of ReceivedSlice.

             } else {
               received_slices_[source_stream->index()].received_bytes +=
                   bytes_to_write;
             }
           }
         }
         break;
       case ByteStreamReader::STREAM_COMPLETE:
         {
           reason = static_cast<DownloadInterruptReason>(
@@ skipping 33 matching lines @@
     source_stream->set_finished(true);
     num_active_streams_--;
 
     // Inform observers.
     SendUpdate();
 
     // All the stream reader are completed, shut down file IO processing.
     if (IsDownloadCompleted()) {
       RecordFileBandwidth(bytes_seen_, disk_writes_time_,
                           base::TimeTicks::Now() - download_start_);
-      if (is_sparse_file_ && record_stream_bandwidth_) {
+      if (IsSparseFile() && record_stream_bandwidth_) {
         RecordParallelDownloadStats(bytes_seen_with_parallel_streams_,
                                     download_time_with_parallel_streams_,
                                     bytes_seen_without_parallel_streams_,
                                     download_time_without_parallel_streams_);
       }
       weak_factory_.InvalidateWeakPtrs();
       std::unique_ptr<crypto::SecureHash> hash_state = file_.Finish();
       update_timer_.reset();
       BrowserThread::PostTask(
           BrowserThread::UI, FROM_HERE,
@@ skipping 40 matching lines @@
                  received_slices_));
 }
 
 void DownloadFileImpl::WillWriteToDisk(size_t data_len) {
   if (!update_timer_->IsRunning()) {
     update_timer_->Start(FROM_HERE,
                          base::TimeDelta::FromMilliseconds(kUpdatePeriodMs),
                          this, &DownloadFileImpl::SendUpdate);
   }
   rate_estimator_.Increment(data_len);
-  if (is_sparse_file_) {
-    base::TimeTicks now = base::TimeTicks::Now();
-    base::TimeDelta time_elapsed = (now - last_update_time_);
-    last_update_time_ = now;
-    if (num_active_streams_ > 1) {
-      download_time_with_parallel_streams_ += time_elapsed;
-      bytes_seen_with_parallel_streams_ += data_len;
-    } else {
-      download_time_without_parallel_streams_ += time_elapsed;
-      bytes_seen_without_parallel_streams_ += data_len;
-    }
+  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeDelta time_elapsed = (now - last_update_time_);
+  last_update_time_ = now;
+  if (num_active_streams_ > 1) {
+    download_time_with_parallel_streams_ += time_elapsed;
+    bytes_seen_with_parallel_streams_ += data_len;
+  } else {
+    download_time_without_parallel_streams_ += time_elapsed;
+    bytes_seen_without_parallel_streams_ += data_len;
   }
 }
 
 void DownloadFileImpl::AddNewSlice(int64_t offset, int64_t length) {
-  if (!is_sparse_file_)
-    return;
   size_t index = AddOrMergeReceivedSliceIntoSortedArray(
       DownloadItem::ReceivedSlice(offset, length), received_slices_);
   // Check if the slice is added as a new slice, or merged with an existing one.
   bool slice_added = (offset == received_slices_[index].offset);
   // Update the index of exising SourceStreams.
   for (auto& stream : source_streams_) {
     SourceStream* source_stream = stream.second.get();
     if (source_stream->offset() > offset) {
       if (slice_added && source_stream->bytes_written() > 0)
         source_stream->set_index(source_stream->index() + 1);
@@ skipping 12 matching lines @@
     SourceStream* source_stream = stream.second.get();
     if (source_stream->offset() >= last_slice_offset &&
         source_stream->bytes_written() > 0) {
       stream_for_last_slice = source_stream;
       last_slice_offset = source_stream->offset();
     }
     if (!source_stream->is_finished())
       return false;
   }
 
-  if (!is_sparse_file_)
+  if (!IsSparseFile())
     return true;
 
   // Verify that all the file slices have been downloaded.
   std::vector<DownloadItem::ReceivedSlice> slices_to_download =
       FindSlicesToDownload(received_slices_);
   if (slices_to_download.size() > 1) {
     // If there are 1 or more holes in the file, download is not finished.
     // Some streams might not have been added to |source_streams_| yet.
     return false;
   }
@@ skipping 16 matching lines @@
 
 void DownloadFileImpl::HandleStreamError(SourceStream* source_stream,
                                          DownloadInterruptReason reason) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   source_stream->stream_reader()->RegisterCallback(base::Closure());
   source_stream->set_finished(true);
   num_active_streams_--;
 
   bool can_recover_from_error = false;
 
-  if (is_sparse_file_ && source_stream->length() != kNoBytesToWrite) {
+  if (IsSparseFile() && source_stream->length() != kNoBytesToWrite) {
     // If a neighboring stream request is available, check if it can help
     // download all the data left by |source stream| or has already done so. We
     // want to avoid the situation that a server always fail additional requests
     // from the client thus causing the initial request and the download going
     // nowhere.
     // TODO(qinmin): make all streams half open so that they can recover
     // failures from their neighbors.
     SourceStream* preceding_neighbor = FindPrecedingNeighbor(source_stream);
     while (preceding_neighbor) {
       int64_t upper_range = source_stream->offset() + source_stream->length();
@@ skipping 38 matching lines @@
     // Our observer will clean us up.
     weak_factory_.InvalidateWeakPtrs();
     std::unique_ptr<crypto::SecureHash> hash_state = file_.Finish();
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&DownloadDestinationObserver::DestinationError, observer_,
                    reason, TotalBytesReceived(), base::Passed(&hash_state)));
   }
 }
 
+bool DownloadFileImpl::IsSparseFile() const {
+  return source_streams_.size() > 1 || !received_slices_.empty();
+}
+
 DownloadFileImpl::SourceStream* DownloadFileImpl::FindPrecedingNeighbor(
     SourceStream* source_stream) {
   int64_t max_preceding_offset = 0;
   SourceStream* ret = nullptr;
   for (auto& stream : source_streams_) {
     int64_t offset = stream.second->offset();
     if (offset < source_stream->offset() && offset >= max_preceding_offset) {
       ret = stream.second.get();
       max_preceding_offset = offset;
     }
@@ skipping 19 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