net: Make UploadData::GetContentLength() asynchronous.

net/base/upload_data.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.h"
 
 #include <algorithm>
 
+#include "base/bind.h"
 #include "base/file_util.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/worker_pool.h"
+#include "base/tracked_objects.h"
 #include "net/base/file_stream.h"
 #include "net/base/net_errors.h"
 
 namespace net {
 
+namespace {
+
+// Helper function for GetContentLength().
+void OnGetContentLengthComplete(
+    const UploadData::ContentLengthCallback& callback,
+    uint64* content_length) {
+  callback.Run(*content_length);
+}
+
+}  // namespace
+
 UploadData::Element::Element()
     : type_(TYPE_BYTES),
       file_range_offset_(0),
       file_range_length_(kuint64max),
       is_last_chunk_(false),
       override_content_length_(false),
       content_length_computed_(false),
       content_length_(-1),
       file_stream_(NULL) {
 }
@@ skipping 33 matching lines @@
   content_length_ = 0;
 
   // We need to open the file here to decide if we should report the file's
   // size or zero.  We cache the open file, so that we can still read it when
   // it comes time to.
   file_stream_ = NewFileStreamForReading();
   if (!file_stream_)
     return 0;
 
   int64 length = 0;
-
-  {
-    // TODO(tzik):
-    // file_util::GetFileSize may cause blocking IO.
-    // Temporary allow until fix: http://crbug.com/72001.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    if (!file_util::GetFileSize(file_path_, &length))
+  if (!file_util::GetFileSize(file_path_, &length))
       return 0;
-  }
 
   if (file_range_offset_ >= static_cast<uint64>(length))
     return 0;  // range is beyond eof
 
   // compensate for the offset and clip file_range_length_ to eof
   content_length_ =  std::min(length - file_range_offset_, file_range_length_);
   return content_length_;
 }
 
 FileStream* UploadData::Element::NewFileStreamForReading() {
   // In common usage GetContentLength() will call this first and store the
   // result into |file_| and a subsequent call (from UploadDataStream) will
   // get the cached open FileStream.
   if (file_stream_) {
     FileStream* file = file_stream_;
     file_stream_ = NULL;
     return file;
   }
 
-  // TODO(tzik):
-  // FileStream::Open and FileStream::Seek may cause blocking IO.
-  // Temporary allow until fix: http://crbug.com/72001.
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
-
   scoped_ptr<FileStream> file(new FileStream(NULL));
   int64 rv = file->Open(file_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 \"" << file_path_.value()
                   << "\" for reading: " << rv;
     return NULL;
   }
   if (file_range_offset_) {
@@ skipping 45 matching lines @@
   elements_.push_back(Element());
   elements_.back().SetToChunk(bytes, bytes_len, is_last_chunk);
   if (chunk_callback_)
     chunk_callback_->OnChunkAvailable();
 }
 
 void UploadData::set_chunk_callback(ChunkCallback* callback) {
   chunk_callback_ = callback;
 }
 
-uint64 UploadData::GetContentLength() {
-  if (is_chunked_)
-    return 0;
+void UploadData::GetContentLength(const ContentLengthCallback& callback) {
+  uint64* result = new uint64(0);
+  const bool task_is_slow = true;
+  const bool posted = base::WorkerPool::PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&UploadData::DoGetContentLength, this, result),

[willchan no longer on Chromium, 2012/02/08 20:44:03]

I have to think about this more, but I thought I'd point it out here so you can
think about it too while I am in meetings. This is extending the lifetime of
UploadData. It's possible that all consumers of UploadData are gone and have
released all their references and expect UploadData to go away (and delete the
UploadData::Elements and their FileStreams and what not). I think it's safe to
extend the lifetime a bit (if the WorkerPool task is still outstanding), but
thought I should point out that there's a change in lifetime behavior here that
has some implication.

I pondered suggesting using a base::WeakPtrFactory to avoid extending the
lifetime, but I think it's problematic for us to delete UploadData's Elements
while the DoGetContentLength() operation is underway.

Anyway, some food for thought. I'll think more about it later.

[satorux1, 2012/02/08 21:02:19]

Thank you for sharing the thought.

I think one way to do this without extending the life time is to  compute length
of non-file payloads now, and then compute length of file payloads in the
background. For the latter, we just need file names which can easily be passed
to a callback without referencing 'this'.

However, UploadData::Element::GetContentLength() does some caching not to reopen
the same file twice, which makes it difficult for us to go with the
"non-life-extending" idea. I'd argue that caching here is a premature
optimization and we can remove this behavior. Caching makes code a bit hard to
follow and is one of reasons why a memory leak was introduced
(crrev.com/120952).

+      base::Bind(&OnGetContentLengthComplete, callback, base::Owned(result)),
+      task_is_slow);
+  DCHECK(posted);
+}
 
-  uint64 len = 0;
-  std::vector<Element>::iterator it = elements_.begin();
-  for (; it != elements_.end(); ++it)
-    len += (*it).GetContentLength();
-  return len;
+uint64 UploadData::GetContentLengthSyncHack() {
+  // Temporarily allow until fix: http://crbug.com/72001.
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
+  uint64 content_length = 0;
+  DoGetContentLength(&content_length);
+  return content_length;
+}
+
+uint64 UploadData::GetContentLengthSync() {
+  uint64 content_length = 0;
+  DoGetContentLength(&content_length);
+  return content_length;
 }
 
 bool UploadData::IsInMemory() const {
   // Chunks are in memory, but UploadData does not have all the chunks at
   // once. Chunks are provided progressively with AppendChunk() as chunks
   // are ready. Check is_chunked_ here, rather than relying on the loop
   // below, as there is a case that is_chunked_ is set to true, but the
   // first chunk is not yet delivered.
   if (is_chunked_)
     return false;
 
   for (size_t i = 0; i < elements_.size(); ++i) {
     if (elements_[i].type() != TYPE_BYTES)
       return false;
   }
   return true;
 }
 
 void UploadData::SetElements(const std::vector<Element>& elements) {
   elements_ = elements;
 }
 
+void UploadData::DoGetContentLength(uint64* content_length) {
+  *content_length = 0;
+
+  if (is_chunked_)
+    return;
+
+  for (size_t i = 0; i < elements_.size(); ++i)
+    *content_length += elements_[i].GetContentLength();
+}
+
 UploadData::~UploadData() {
 }
 
 }  // namespace net