net: Make UploadDataStream::Init() asynchronous.

net/base/upload_data_stream.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 "net/base/upload_data_stream.h"
 
 #include "base/file_util.h"
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/worker_pool.h"
 #include "net/base/file_stream.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
 namespace net {
 
+namespace {
+
+// This function is used to implement UploadDataStream::Init().
+void InitInternal(scoped_refptr<UploadData> upload_data,

[eroman, 2012/08/09 05:29:33]

UploadData is not threadsafe, however now it is being passed off to a worker
thread and used there. I do not believe this is safe.

(Even just calling AddRef() implicitly by the scoped_refptr<> can be risky
business when done from multiple threads and the base class was RefCounted
rather than RefCountedThreadSafe).

[hashimoto, 2012/08/09 13:35:02]

Oh I thought touching UploadData on a worker thread is safe because
UploadData::GetContentLength is doing it.
It seems we have to fix GetContentLength first.
How about making UploadData RecCountedThreadSafe and adding a class comment "Do
not touch this class while asynchronous operation is in progress" for
UploadData?
It is safe as long as all public methods of UploadData are called only on one
thread (i.e. IO thread) and only private methods touch UploadData on worker
threads.

If we are able to touch UploadData on worker threads, it becomes very easy to
avoid code duplication between synchronous and asynchronous implementations.
(Synchronous implementation should be maintained unless we fix Chrome Frame
radically)

+                  uint64* total_size,
+                  int* result) {
+  *total_size = upload_data->GetContentLengthSync();
+
+  // 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.
+  const std::vector<UploadData::Element>& elements = *upload_data->elements();
+  for (size_t i = 0; i < elements.size(); ++i) {
+    const UploadData::Element& element = elements[i];
+    if (element.type() == UploadData::TYPE_FILE &&
+        !element.expected_file_modification_time().is_null()) {
+      base::PlatformFileInfo info;
+      if (file_util::GetFileInfo(element.file_path(), &info) &&
+          element.expected_file_modification_time().ToTimeT() !=
+          info.last_modified.ToTimeT()) {
+        *result = ERR_UPLOAD_FILE_CHANGED;
+        return;
+      }
+    }
+  }
+
+  // Reset the offset, as upload_data_ may already be read (i.e. UploadData
+  // can be reused for a new UploadDataStream).
+  upload_data->ResetOffset();
+
+  *result = OK;
+  return;

[eroman, 2012/08/09 05:29:33]

delete this

+}
+
+}  // namespace
+
 bool UploadDataStream::merge_chunks_ = true;
 
 // static
 void UploadDataStream::ResetMergeChunks() {
   // WARNING: merge_chunks_ must match the above initializer.
   merge_chunks_ = true;
 }
 
 UploadDataStream::UploadDataStream(UploadData* upload_data)
     : upload_data_(upload_data),
       element_index_(0),
       total_size_(0),
       current_position_(0),
-      initialized_successfully_(false) {
+      initialized_successfully_(false),
+      weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
 UploadDataStream::~UploadDataStream() {
 }
 
-int UploadDataStream::Init() {
+int UploadDataStream::Init(const CompletionCallback& callback) {
   DCHECK(!initialized_successfully_);
+  // Fast path. Initialize synchronously if the data is in memory.
+  if (upload_data_->IsInMemory())
+    return InitSync();
 
+  uint64* total_size = new uint64(0);
+  int* result = new int(OK);
+  const bool posted = base::WorkerPool::PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&InitInternal, upload_data_, total_size, result),
+      base::Bind(&UploadDataStream::OnInitComplete,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 callback,
+                 base::Owned(total_size),
+                 base::Owned(result)),
+      true /* task_is_slow */);
+  DCHECK(posted);
+  return ERR_IO_PENDING;
+}
+
+int UploadDataStream::InitSync() {
+  DCHECK(!initialized_successfully_);
+  int result = OK;
   {
     base::ThreadRestrictions::ScopedAllowIO allow_io;
-    total_size_ = upload_data_->GetContentLengthSync();
+    InitInternal(upload_data_, &total_size_, &result);
   }
-
-  // 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.
-  const std::vector<UploadData::Element>& elements = *upload_data_->elements();
-  for (size_t i = 0; i < elements.size(); ++i) {
-    const UploadData::Element& element = elements[i];
-    if (element.type() == UploadData::TYPE_FILE &&
-        !element.expected_file_modification_time().is_null()) {
-      // Temporarily allow until fix: http://crbug.com/72001.
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
-      base::PlatformFileInfo info;
-      if (file_util::GetFileInfo(element.file_path(), &info) &&
-          element.expected_file_modification_time().ToTimeT() !=
-          info.last_modified.ToTimeT()) {
-        return ERR_UPLOAD_FILE_CHANGED;
-      }
-    }
-  }
-
-  // Reset the offset, as upload_data_ may already be read (i.e. UploadData
-  // can be reused for a new UploadDataStream).
-  upload_data_->ResetOffset();
-
-  initialized_successfully_ = true;
-  return OK;
+  // Passing a null callback to OnInitComplete() since we return the result
+  // synchronously here and there is no need to run callback.
+  CompletionCallback null_callback;
+  OnInitComplete(null_callback, &total_size_, &result);
+  return result;
 }
 
 int UploadDataStream::Read(IOBuffer* buf, int buf_len) {
   std::vector<UploadData::Element>& elements =
       *upload_data_->elements_mutable();
 
   int bytes_copied = 0;
   while (bytes_copied < buf_len && element_index_ < elements.size()) {
     UploadData::Element& element = elements[element_index_];
 
     bytes_copied += element.ReadSync(buf->data() + bytes_copied,
                                      buf_len - bytes_copied);
 
     if (element.BytesRemaining() == 0)
         ++element_index_;
 
     if (is_chunked() && !merge_chunks_)
       break;
   }
 
   current_position_ += bytes_copied;
   if (is_chunked() && !IsEOF() && bytes_copied == 0)
     return ERR_IO_PENDING;
 
   return bytes_copied;
 }
 
+void UploadDataStream::OnInitComplete(const CompletionCallback& callback,
+                                      uint64* total_size,
+                                      int* result) {
+  total_size_ = *total_size;
+  if (*result == OK)
+    initialized_successfully_ = true;
+
+  if (!callback.is_null())
+    callback.Run(*result);
+}
+
 bool UploadDataStream::IsEOF() const {
   const std::vector<UploadData::Element>& elements = *upload_data_->elements();
 
   // Check if all elements are consumed.
   if (element_index_ == elements.size()) {
     // If the upload data is chunked, check if the last element is the
     // last chunk.
     if (!upload_data_->is_chunked() ||
         (!elements.empty() && elements.back().is_last_chunk())) {
       return true;
     }
   }
   return false;
 }
 
 bool UploadDataStream::IsInMemory() const {
   DCHECK(initialized_successfully_);
 
   return upload_data_->IsInMemory();
 }
 
 }  // namespace net