Refactor BaseFile class to support sparse files

content/browser/download/base_file.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/base_file.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/files/file.h"
@@ skipping 26 matching lines @@
   else
     Cancel();  // Will delete the file.
 }
 
 DownloadInterruptReason BaseFile::Initialize(
     const base::FilePath& full_path,
     const base::FilePath& default_directory,
     base::File file,
     int64_t bytes_so_far,
     const std::string& hash_so_far,
-    std::unique_ptr<crypto::SecureHash> hash_state) {
+    std::unique_ptr<crypto::SecureHash> hash_state,
+    AccessMode access_mode) {
   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   DCHECK(!detached_);
 
   if (full_path.empty()) {
     base::FilePath initial_directory(default_directory);
     base::FilePath temp_file;
     if (initial_directory.empty()) {
       initial_directory =
           GetContentClient()->browser()->GetDefaultDownloadDirectory();
     }
     // |initial_directory| can still be empty if ContentBrowserClient returned
     // an empty path for the downloads directory.
     if ((initial_directory.empty() ||
          !base::CreateTemporaryFileInDir(initial_directory, &temp_file)) &&
         !base::CreateTemporaryFile(&temp_file)) {
       return LogInterruptReason("Unable to create", 0,
                                 DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
     }
     full_path_ = temp_file;
   } else {
     full_path_ = full_path;
   }
 
   bytes_so_far_ = bytes_so_far;
   secure_hash_ = std::move(hash_state);
+  access_mode_ = access_mode;
+  DCHECK(access_mode_ == EXCLUSIVE || !secure_hash_);
   file_ = std::move(file);
 
   return Open(hash_so_far);
 }
 
 DownloadInterruptReason BaseFile::AppendDataToFile(const char* data,
                                                    size_t data_len) {
-  DCHECK_CURRENTLY_ON(BrowserThread::FILE);
-  DCHECK(!detached_);
+  DCHECK_EQ(access_mode_, EXCLUSIVE);
+  return WriteAtCurrentPos(data, data_len);
+}
 
+DownloadInterruptReason BaseFile::WriteDataToFile(int64_t offset,
+                                                  const char* data,
+                                                  size_t data_len) {
+  DCHECK_EQ(access_mode_, SHARED);
+
+  if (file_.Seek(base::File::FROM_BEGIN, offset) < 0) {

[asanka, 2017/02/16 18:11:24]

base::File::Write() can be used to write to a specific offset without an
additional Seek()

[asanka, 2017/02/16 18:46:25]

Disregard.

[qinmin, 2017/02/16 21:35:50]

I used this seek() to reuse some of code from AppendDataToFile(). For example,
if I use Write(offset, data, len), when will have to check the return value, and
do a for loop to write the remainder data, and update the bytes_so_far_. By
doing a seek, i can reuse the while loop in AppendDataToFile(). 

It is also very strange that we actually have this while loop in our code.
base::File::Write() already does a while loop for us. If it writes fewer bytes
than asked, this means an error. So I don't understand why this comment is here
"// The Write call below is not guaranteed to write all the data.". Maybe this
is for some systems that have a flawed base::File::Write() implementation?  I
can fix this in a separate CL if this comment is incorrect.

[asanka, 2017/02/16 21:58:02]

Probably because this code predates the logic in base::File ?  We should remove
the loop around the File::Write() calls. Then afterwards we can get rid of the
additional Seek(). I'm okay with doing so in a separate CL.

[qinmin, 2017/02/16 22:20:29]

Done. Removed the while loop around the Write() call and removed the seek()
call.

+    logging::SystemErrorCode error = logging::GetLastSystemErrorCode();
+    return LogSystemError("Unable to seek", error);
+  }
+  return WriteAtCurrentPos(data, data_len);
+}
+
+DownloadInterruptReason BaseFile::WriteAtCurrentPos(const char* data,
+                                                    size_t data_len) {
   // NOTE(benwells): The above DCHECK won't be present in release builds,
   // so we log any occurences to see how common this error is in the wild.
   if (detached_)
     RecordDownloadCount(APPEND_TO_DETACHED_FILE_COUNT);
 
   if (!file_.IsValid())
     return LogInterruptReason("No file stream on append", 0,
                               DOWNLOAD_INTERRUPT_REASON_FILE_FAILED);
 
   // TODO(phajdan.jr): get rid of this check.
@@ skipping 23 matching lines @@
   }
   net_log_.EndEvent(net::NetLogEventType::DOWNLOAD_FILE_WRITTEN,
                     net::NetLog::Int64Callback("bytes", data_len));
 
   if (secure_hash_)
     secure_hash_->Update(data, data_len);
 
   return DOWNLOAD_INTERRUPT_REASON_NONE;
 }
 
 DownloadInterruptReason BaseFile::Rename(const base::FilePath& new_path) {

[asanka, 2017/02/16 18:11:24]

Methods like Rename() and Annotate...() depend on this BaseFile being the sole
owner.

Overall, BaseFile still assumes sole ownership since it will delete the file if
BaseFile is destroyed without calling Detach().

[asanka, 2017/02/16 18:46:26]

Disregard. I misunderstood how BaseFile is intended to be used with parallel
downloads.

[qinmin, 2017/02/16 21:35:50]

Yes, the BaseFile will still be owned by a DownloadFileImpl object and will be
shared among the workers. So this CL is not going to create multiple BaseFiles
for one physical file

   DCHECK_CURRENTLY_ON(BrowserThread::FILE);
   DownloadInterruptReason rename_result = DOWNLOAD_INTERRUPT_REASON_NONE;
 
   // If the new path is same as the old one, there is no need to perform the
   // following renaming logic.
   if (new_path == full_path_)
     return DOWNLOAD_INTERRUPT_REASON_NONE;
 
   // Save the information whether the download is in progress because
   // it will be overwritten by closing the file.
@@ skipping 144 matching lines @@
     if (!file_.IsValid()) {
       return LogNetError("Open/Initialize File",
                          net::FileErrorToNetError(file_.error_details()));
     }
   }
 
   net_log_.BeginEvent(
       net::NetLogEventType::DOWNLOAD_FILE_OPENED,
       base::Bind(&FileOpenedNetLogCallback, &full_path_, bytes_so_far_));
 
+  // For SHARED file, skip hash validation.
+  if (access_mode_ == SHARED) {
+    if (file_.GetLength() < bytes_so_far_) {
+      ClearFile();
+      return LogInterruptReason("File has fewer written bytes than expected", 0,
+                                DOWNLOAD_INTERRUPT_REASON_FILE_TOO_SHORT);
+    }
+    return DOWNLOAD_INTERRUPT_REASON_NONE;
+  }
+
   if (!secure_hash_) {
     DownloadInterruptReason reason = CalculatePartialHash(hash_so_far);
     if (reason != DOWNLOAD_INTERRUPT_REASON_NONE) {
       ClearFile();
       return reason;
     }
   }
 
   int64_t file_size = file_.Seek(base::File::FROM_END, 0);
   if (file_size < 0) {
@@ skipping 155 matching lines @@
 #else  // !OS_WIN && !OS_MACOSX && !OS_LINUX
 DownloadInterruptReason BaseFile::AnnotateWithSourceInformation(
     const std::string& client_guid,
     const GURL& source_url,
     const GURL& referrer_url) {
   return DOWNLOAD_INTERRUPT_REASON_NONE;
 }
 #endif
 
 }  // namespace content