net: Make UploadData::GetContentLength() asynchronous.

net/base/upload_data.h

 // 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.
 
 #ifndef NET_BASE_UPLOAD_DATA_H_
 #define NET_BASE_UPLOAD_DATA_H_
 #pragma once
 
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/callback_forward.h"
 #include "base/file_path.h"
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/time.h"
 #include "googleurl/src/gurl.h"
 #include "net/base/net_export.h"
 
 namespace net {
 
 class FileStream;
@@ skipping 78 matching lines @@
     void SetToChunk(const char* bytes, int bytes_len, bool is_last_chunk);
 
     bool is_last_chunk() const { return is_last_chunk_; }
     // Sets whether this is the last chunk. Used during IPC marshalling.
     void set_is_last_chunk(bool is_last_chunk) {
       is_last_chunk_ = is_last_chunk;
     }
 
     // Returns the byte-length of the element.  For files that do not exist, 0
     // is returned.  This is done for consistency with Mozilla.
     // Once called, this function will always return the same value.

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

it looks like this last sentence is only true if type_ is TYPE_FILE?

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

You are right. The caching works only for TYPE_FILE. Removed the last sentence
as it's just an implementation detail that clients should not care.

     uint64 GetContentLength();
 
     // Returns a FileStream opened for reading for this element, positioned at
     // |file_range_offset_|.  The caller gets ownership and is responsible
     // for cleaning up the FileStream. Returns NULL if this element is not of
     // type TYPE_FILE or if the file is not openable.
     FileStream* NewFileStreamForReading();
 
    private:
     // Allows tests to override the result of GetContentLength.
@@ skipping 35 matching lines @@
   void AppendChunk(const char* bytes, int bytes_len, bool is_last_chunk);
 
   // Sets the callback to be invoked when a new chunk is available to upload.
   void set_chunk_callback(ChunkCallback* callback);
 
   // Initializes the object to send chunks of upload data over time rather
   // than all at once.
   void set_is_chunked(bool set) { is_chunked_ = set; }
   bool is_chunked() const { return is_chunked_; }
 
+  // Gets the total size in bytes of the data to upload. Computing the
+  // content length can result in performing file IO hence the operation is
+  // done asynchronously. Runs the callback with the content length once the
+  // computation is done.
+  typedef base::Callback<void(uint64 content_length)> ContentLengthCallback;
+  void GetContentLength(const ContentLengthCallback& callback);

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

This only hits the IO thread when there are elements of TYPE_FILE afaict. It
seems odd to force all clients to use a (slightly more complicated) async
interface to GetContentLength when only some types of UploadData really require
it. 

Would you consider keeping a GetContentLengthSync() method that works on
UploadData instances that don't refer to file data?

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

Sounds reasonable. I originally thought it'd be simpler to provide only the
async version but since Chrome Frame will need the sync version, and the sync
version is useful in tests (FWIW, ResourceDispatcherHost and UploadDataStream
are the only non-test clients in Chrome), I'd keep GetContentLengthSync() 
without the thread IO restrictions allowed.

+
   // Returns the total size in bytes of the data to upload.
-  uint64 GetContentLength();
+  // This version may perform file IO on the current thread.
+  // TODO(satorux): Remove this once clients are gone.

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

Are you planning on attempting to convert the Urlmon client of this in Chrome
Frame to using the asynchronous interface? That seems like it would be hard to
get right. My understanding was that the synchronous interface would remain.

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

No. I don't plan to make a major change to chrome_frame code. Besides, since
Chrome Frame clients are not affected by the thread io restrictions, there is no
need to make them async.

So I changed the code to have two sync versions:


GetContentLengthSync() - without AllowIO. used by chrome frame and tests
GetContentLengthSyncHack() - with AllowIO. will be removed.

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

Thanks! That looks good to me. 

+  uint64 GetContentLengthSyncHack();
+
+  // Returns the total size in bytes of the data to upload, for testing.
+  // This version may perform file IO on the current thread.
+  // Should only be used in tests where file IO is allowed.
+  uint64 GetContentLengthSyncForTesting();
 
   // Returns true if the upload data is entirely in memory (i.e. the
   // upload data is not chunked, and all elemnts are of TYPE_BYTES).
   bool IsInMemory() const;
 
   std::vector<Element>* elements() {
     return &elements_;
   }
 
   void SetElements(const std::vector<Element>& elements);
 
   void swap_elements(std::vector<Element>* elements) {
     elements_.swap(*elements);
   }
 
   // Identifies a particular upload instance, which is used by the cache to
   // formulate a cache key.  This value should be unique across browser
   // sessions.  A value of 0 is used to indicate an unspecified identifier.
   void set_identifier(int64 id) { identifier_ = id; }
   int64 identifier() const { return identifier_; }
 
  private:
+  // Helper function for GetContentLength().
+  void DoGetContentLength(uint64* content_length);
+
   friend class base::RefCounted<UploadData>;
 
   ~UploadData();
 
   std::vector<Element> elements_;
   int64 identifier_;
   ChunkCallback* chunk_callback_;
   bool is_chunked_;
 
   DISALLOW_COPY_AND_ASSIGN(UploadData);
@@ skipping 20 matching lines @@
 
 inline bool operator!=(const UploadData::Element& a,
                        const UploadData::Element& b) {
   return !(a == b);
 }
 #endif  // defined(UNIT_TEST)
 
 }  // namespace net
 
 #endif  // NET_BASE_UPLOAD_DATA_H_