net: Make UploadData::GetContentLength() asynchronous.

net/base/upload_data.cc

Index: net/base/upload_data.cc
diff --git a/net/base/upload_data.cc b/net/base/upload_data.cc
index 424939e4238a20e10ccb4382579c26242ecfe5af..91970fe76904d3b0eefc339d53d6d9b2b4dc0ad4 100644
--- a/net/base/upload_data.cc
+++ b/net/base/upload_data.cc
@@ -6,15 +6,29 @@
 
 #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),
@@ -68,15 +82,8 @@ uint64 UploadData::Element::GetContentLength() {
     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
@@ -96,11 +103,6 @@ FileStream* UploadData::Element::NewFileStreamForReading() {
     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);
@@ -166,15 +168,29 @@ 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),
+      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::GetContentLengthSyncForTesting() {
+  uint64 content_length = 0;
+  DoGetContentLength(&content_length);
+  return content_length;
 }
 
 bool UploadData::IsInMemory() const {
@@ -197,6 +213,16 @@ 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_.at(i).GetContentLength();

[robertshield, 2012/02/04 05:39:04]

prefer using iterator syntax:
std::vector<Element>::iterator iter(elements_.begin());
for (; iter != elements_.end(); ++iter) { ...

[satorux1, 2012/02/04 07:18:32]

It's more verbose, and I don't see advantage here, but let's not build a bike
shed here. :)

[grt (UTC plus 2), 2012/02/04 19:33:56]

If you prefer to use an index, use operator[] rather than at().  The latter does
bounds checking and throws exceptions.  You clearly don't need the former since
your loop won't read out of bounds, and you don't want the latter since we don't
do exceptions in Chromium.  Also, if you prefer an index, cache the result of
size() rather than fetching it each time through the loop.

FWIW: I like iterators, but I won't try to convince you to like them too.

Oh, and mauve is an excellent shed color.

[robertshield, 2012/02/04 21:10:56]

Sure, no worries :-) Do note that grt@'s advice on such matters is always worth
heeding.

Except for the color; burgundy is a much better shed color!

+}
+
 UploadData::~UploadData() {
 }