net: Make UploadDataStream::Init() asynchronous.

net/base/upload_file_element_reader.cc

Index: net/base/upload_file_element_reader.cc
diff --git a/net/base/upload_file_element_reader.cc b/net/base/upload_file_element_reader.cc
index b71c0225b8acb32b3c96a8cd81fc350866e74434..559a8f869b9742efb13c081d5cc9110f6f37c4f1 100644
--- a/net/base/upload_file_element_reader.cc
+++ b/net/base/upload_file_element_reader.cc
@@ -4,8 +4,11 @@
 
 #include "net/base/upload_file_element_reader.h"
 
+#include "base/bind.h"
 #include "base/file_util.h"
+#include "base/location.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/worker_pool.h"
 #include "net/base/file_stream.h"
 #include "net/base/net_errors.h"
 
@@ -17,78 +20,121 @@ namespace {
 // UploadFileElementReader::GetContentLength() when set to non-zero.
 uint64 overriding_content_length = 0;
 
-}  // namespace
-
-UploadFileElementReader::UploadFileElementReader(
-    const FilePath& path,
-    uint64 range_offset,
-    uint64 range_length,
-    const base::Time& expected_modification_time)
-    : path_(path),
-      range_offset_(range_offset),
-      range_length_(range_length),
-      expected_modification_time_(expected_modification_time),
-      content_length_(0),
-      bytes_remaining_(0) {
-}
-
-UploadFileElementReader::~UploadFileElementReader() {
-  // Temporarily allow until fix: http://crbug.com/72001.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-  if (file_stream_.get())
-    file_stream_->CloseSync();
-}
-
-int UploadFileElementReader::InitSync() {
-  // Temporarily allow until fix: http://crbug.com/72001.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-
+// This method is used to implement Init().
+void InitInternal(const FilePath& path,
+                  uint64 range_offset,
+                  uint64 range_length,
+                  const base::Time& expected_modification_time,
+                  scoped_ptr<FileStream>* out_file_stream,
+                  uint64* out_content_length,
+                  int* out_result) {

[willchan no longer on Chromium, 2012/09/11 19:32:40]

Should we add a ThreadRestrictions assertion here to make sure we're running on
a thread that can have sync IO?

[hashimoto, 2012/09/11 22:51:31]

According to the comment in base/threading/thread_restrictions.h, their practice
to decide where to put AssertIOAllowed is "It's best if you put them as close to
the disk access as possible, at the lowest level."

Since there are already AssertIOAllowed in base/platform_file_posix.cc, I think
we should not add AssertIOAllowed here.

   scoped_ptr<FileStream> file_stream(new FileStream(NULL));
   int64 rv = file_stream->OpenSync(
-      path_, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ);
+      path, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ);
   if (rv != OK) {
     // If the file can't be opened, we'll just upload an empty file.
-    DLOG(WARNING) << "Failed to open \"" << path_.value()
+    DLOG(WARNING) << "Failed to open \"" << path.value()
                   << "\" for reading: " << rv;
     file_stream.reset();
   }
-  if (file_stream.get() && range_offset_) {
-    rv = file_stream->SeekSync(FROM_BEGIN, range_offset_);
+  if (file_stream.get() && range_offset) {
+    rv = file_stream->SeekSync(FROM_BEGIN, range_offset);
     if (rv < 0) {
-      DLOG(WARNING) << "Failed to seek \"" << path_.value()
-                    << "\" to offset: " << range_offset_ << " (" << rv
+      DLOG(WARNING) << "Failed to seek \"" << path.value()
+                    << "\" to offset: " << range_offset << " (" << rv
                     << ")";
       file_stream->CloseSync();
       file_stream.reset();
     }
   }
-  file_stream_.reset(file_stream.release());
 
   int64 length = 0;
-  if (file_stream_.get() &&
-      file_util::GetFileSize(path_, &length) &&
-      range_offset_ < static_cast<uint64>(length)) {
+  if (file_stream.get() &&
+      file_util::GetFileSize(path, &length) &&
+      range_offset < static_cast<uint64>(length)) {
     // Compensate for the offset.
-    length = std::min(length - range_offset_, range_length_);
+    length = std::min(length - range_offset, range_length);
   }
-  content_length_ = length;
-  bytes_remaining_ = GetContentLength();
+  *out_content_length = length;
+  out_file_stream->reset(file_stream.release());
 
   // If the underlying file has been changed and the expected file
   // modification time is set, treat it as error. Note that the expected
   // modification time from WebKit is based on time_t precision. So we
   // have to convert both to time_t to compare. This check is used for
   // sliced files.
-  if (!expected_modification_time_.is_null()) {
+  if (!expected_modification_time.is_null()) {
     base::PlatformFileInfo info;
-    if (file_util::GetFileInfo(path_, &info) &&
-        expected_modification_time_.ToTimeT() !=
+    if (file_util::GetFileInfo(path, &info) &&
+        expected_modification_time.ToTimeT() !=
         info.last_modified.ToTimeT()) {
-      return ERR_UPLOAD_FILE_CHANGED;
+      *out_result = ERR_UPLOAD_FILE_CHANGED;
+      return;
     }
   }
 
-  return OK;
+  *out_result = OK;
+}
+
+}  // namespace
+
+UploadFileElementReader::UploadFileElementReader(
+    const FilePath& path,
+    uint64 range_offset,
+    uint64 range_length,
+    const base::Time& expected_modification_time)
+    : path_(path),
+      range_offset_(range_offset),
+      range_length_(range_length),
+      expected_modification_time_(expected_modification_time),
+      content_length_(0),
+      bytes_remaining_(0),
+      weak_ptr_factory_(this) {
+}
+
+UploadFileElementReader::~UploadFileElementReader() {
+  // Temporarily allow until fix: http://crbug.com/72001.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  if (file_stream_.get())
+    file_stream_->CloseSync();
+}
+
+int UploadFileElementReader::Init(const CompletionCallback& callback) {
+  scoped_ptr<FileStream>* file_stream = new scoped_ptr<FileStream>;
+  uint64* content_length = new uint64;
+  int* result = new int;
+  const bool posted = base::WorkerPool::PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&InitInternal,
+                 path_,
+                 range_offset_,
+                 range_length_,
+                 expected_modification_time_,
+                 file_stream,
+                 content_length,
+                 result),
+      base::Bind(&UploadFileElementReader::OnInitCompleted,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 base::Owned(file_stream),
+                 base::Owned(content_length),
+                 base::Owned(result),
+                 callback),
+      true /* task_is_slow */);
+  DCHECK(posted);
+  return ERR_IO_PENDING;
+}
+
+int UploadFileElementReader::InitSync() {
+  // Temporarily allow until fix: http://crbug.com/72001.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+
+  scoped_ptr<FileStream> file_stream;
+  uint64 content_length = 0;
+  int result = OK;
+  InitInternal(path_, range_offset_, range_length_, expected_modification_time_,
+               &file_stream, &content_length, &result);
+  OnInitCompleted(&file_stream, &content_length, &result, CompletionCallback());
+  return result;
 }
 
 uint64 UploadFileElementReader::GetContentLength() const {
@@ -129,6 +175,18 @@ int UploadFileElementReader::ReadSync(char* buf, int buf_length) {
   return num_bytes_to_read;
 }
 
+void UploadFileElementReader::OnInitCompleted(
+    scoped_ptr<FileStream>* file_stream,
+    uint64* content_length,
+    int* result,
+    const CompletionCallback& callback) {
+  file_stream_.reset(file_stream->release());
+  content_length_ = *content_length;
+  bytes_remaining_ = GetContentLength();
+  if (!callback.is_null())
+    callback.Run(*result);
+}
+
 UploadFileElementReader::ScopedOverridingContentLengthForTests::
 ScopedOverridingContentLengthForTests(uint64 value) {
   overriding_content_length = value;