2 files changed, 114 insertions, 80 deletions

diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc
index 50e604d..2d2a3a5 100644
--- a/net/base/upload_data_stream.cc
+++ b/net/base/upload_data_stream.cc
@@ -21,7 +21,7 @@ UploadDataStream::~UploadDataStream() {
 
 UploadDataStream* UploadDataStream::Create(UploadData* data, int* error_code) {
   scoped_ptr<UploadDataStream> stream(new UploadDataStream(data));
-  int rv = stream->FillBuf();
+  int rv = stream->FillBuffer();
   if (error_code)
     *error_code = rv;
   if (rv != OK)
@@ -41,60 +41,67 @@ void UploadDataStream::MarkConsumedAndFillBuffer(size_t num_bytes) {
 
   if (num_bytes) {
     buf_len_ -= num_bytes;
-    if (buf_len_)
+    // Move the remaining data to the beginning.
+    if (buf_len_ > 0)
       memmove(buf_->data(), buf_->data() + num_bytes, buf_len_);
   }
 
-  FillBuf();
+  FillBuffer();
 
   current_position_ += num_bytes;
 }
 
-UploadDataStream::UploadDataStream(UploadData* data)
-    : data_(data),
+UploadDataStream::UploadDataStream(UploadData* upload_data)
+    : upload_data_(upload_data),
       buf_(new IOBuffer(kBufferSize)),
       buf_len_(0),
-      next_element_(0),
-      next_element_offset_(0),
-      next_element_remaining_(0),
-      total_size_(data->GetContentLength()),
+      element_index_(0),
+      element_offset_(0),
+      element_file_bytes_remaining_(0),
+      total_size_(upload_data->GetContentLength()),
       current_position_(0),
       eof_(false) {
 }
 
-int UploadDataStream::FillBuf() {
-  std::vector<UploadData::Element>& elements = *data_->elements();
+int UploadDataStream::FillBuffer() {
+  std::vector<UploadData::Element>& elements = *upload_data_->elements();
 
-  while (buf_len_ < kBufferSize && next_element_ < elements.size()) {
+  while (buf_len_ < kBufferSize && element_index_ < elements.size()) {
     bool advance_to_next_element = false;
 
-    UploadData::Element& element = elements[next_element_];
+    // This is not const as GetContentLength() is not const.
+    UploadData::Element& element = elements[element_index_];
 
-    size_t size_remaining = kBufferSize - buf_len_;
+    const size_t free_buffer_space = kBufferSize - buf_len_;
     if (element.type() == UploadData::TYPE_BYTES ||
         element.type() == UploadData::TYPE_CHUNK) {
-      const std::vector<char>& d = element.bytes();
-      size_t count = d.size() - next_element_offset_;
-
-      size_t bytes_copied = std::min(count, size_remaining);
-
-      // Check if we have anything to copy first, because we are getting the
-      // address of an element in |d| and that will throw an exception if |d|
-      // is an empty vector.
-      if (bytes_copied) {
-        memcpy(buf_->data() + buf_len_, &d[next_element_offset_], bytes_copied);
-        buf_len_ += bytes_copied;
+      const std::vector<char>& element_data = element.bytes();
+      const size_t num_bytes_left_in_element =
+          element_data.size() - element_offset_;
+
+      const size_t num_bytes_to_copy = std::min(num_bytes_left_in_element,
+                                                free_buffer_space);
+
+      // Check if we have anything to copy first, because we are getting
+      // the address of an element in |element_data| and that will throw an
+      // exception if |element_data| is an empty vector.
+      if (num_bytes_to_copy > 0) {
+        memcpy(buf_->data() + buf_len_,
+               &element_data[element_offset_],
+               num_bytes_to_copy);
+        buf_len_ += num_bytes_to_copy;
+        element_offset_ += num_bytes_to_copy;
       }
 
-      if (bytes_copied == count) {
+      // Advance to the next element if we have consumed all data in the
+      // current element.
+      if (element_offset_ == element_data.size())
         advance_to_next_element = true;
-      } else {
-        next_element_offset_ += bytes_copied;
-      }
     } else {
       DCHECK(element.type() == UploadData::TYPE_FILE);
 
-      if (!next_element_remaining_) {
+      // Open the file of the current element if not yet opened.
+      if (!element_file_stream_.get()) {
         // If the underlying file has been changed, 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.
@@ -106,69 +113,87 @@ int UploadDataStream::FillBuf() {
             return ERR_UPLOAD_FILE_CHANGED;
           }
         }
-        next_element_remaining_ = element.GetContentLength();
-        next_element_stream_.reset(element.NewFileStreamForReading());
+        element_file_bytes_remaining_ = element.GetContentLength();
+        element_file_stream_.reset(element.NewFileStreamForReading());
       }
 
-      int rv = 0;
-      int count =
-          static_cast<int>(std::min(next_element_remaining_,
-                                    static_cast<uint64>(size_remaining)));
-      if (count > 0) {
+      const int num_bytes_to_read =
+          static_cast<int>(std::min(element_file_bytes_remaining_,
+                                    static_cast<uint64>(free_buffer_space)));
+      if (num_bytes_to_read > 0) {
+        int num_bytes_consumed = 0;
         // Temporarily allow until fix: http://crbug.com/72001.
         base::ThreadRestrictions::ScopedAllowIO allow_io;
-        if (next_element_stream_.get())
-          rv = next_element_stream_->Read(buf_->data() + buf_len_, count,
-                                          CompletionCallback());
-        if (rv <= 0) {
+        // element_file_stream_.get() is NULL if the target file is
+        // missing or not readable.
+        if (element_file_stream_.get()) {
+          num_bytes_consumed =
+              element_file_stream_->Read(buf_->data() + buf_len_,
+                                         num_bytes_to_read,
+                                         CompletionCallback());
+        }
+        if (num_bytes_consumed <= 0) {
           // If there's less data to read than we initially observed, then
           // pad with zero.  Otherwise the server will hang waiting for the
           // rest of the data.
-          memset(buf_->data() + buf_len_, 0, count);
-          rv = count;
+          memset(buf_->data() + buf_len_, 0, num_bytes_to_read);
+          num_bytes_consumed = num_bytes_to_read;
         }
-        buf_len_ += rv;
+        buf_len_ += num_bytes_consumed;
+        element_file_bytes_remaining_ -= num_bytes_consumed;
       }
 
-      if (static_cast<int>(next_element_remaining_) == rv) {
+      // Advance to the next element if we have consumed all data in the
+      // current element.
+      if (element_file_bytes_remaining_ == 0)
         advance_to_next_element = true;
-      } else {
-        next_element_remaining_ -= rv;
-      }
     }
 
-    if (advance_to_next_element) {
-      ++next_element_;
-      next_element_offset_ = 0;
-      next_element_remaining_ = 0;
-      next_element_stream_.reset();
-    }
+    if (advance_to_next_element)
+      AdvanceToNextElement();
 
     if (is_chunked() && !merge_chunks_)
       break;
   }
 
-  if (next_element_ == elements.size() && !buf_len_) {
-    if (!data_->is_chunked() ||
+  eof_ = IsEOF();
+
+  return OK;
+}
+
+void UploadDataStream::AdvanceToNextElement() {
+  ++element_index_;
+  element_offset_ = 0;
+  element_file_bytes_remaining_ = 0;
+  element_file_stream_.reset();
+}
+
+bool UploadDataStream::IsEOF() const {
+  const std::vector<UploadData::Element>& elements = *upload_data_->elements();
+
+  // Check if all elements are consumed and the buffer is empty.
+  if (element_index_ == elements.size() && !buf_len_) {
+    // 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())) {
-      eof_ = true;
+      return true;
     }
   }
-
-  return OK;
+  return false;
 }
 
 bool UploadDataStream::IsOnLastChunk() const {
-  const std::vector<UploadData::Element>& elements = *data_->elements();
-  DCHECK(data_->is_chunked());
+  const std::vector<UploadData::Element>& elements = *upload_data_->elements();
+  DCHECK(upload_data_->is_chunked());
   return (eof_ ||
           (!elements.empty() &&
-           next_element_ == elements.size() &&
+           element_index_ == elements.size() &&
            elements.back().is_last_chunk()));
 }
 
 bool UploadDataStream::IsInMemory() const {
-  return data_->IsInMemory();
+  return upload_data_->IsInMemory();
 }
 
 }  // namespace net
diff --git a/net/base/upload_data_stream.h b/net/base/upload_data_stream.h
index e71cc68..f0a59fa 100644
--- a/net/base/upload_data_stream.h
+++ b/net/base/upload_data_stream.h
@@ -22,7 +22,7 @@ class NET_EXPORT UploadDataStream {
   // Returns a new instance of UploadDataStream if it can be created and
   // initialized successfully. If not, NULL will be returned and the error
   // code will be set if the output parameter error_code is not empty.
-  static UploadDataStream* Create(UploadData* data, int* error_code);
+  static UploadDataStream* Create(UploadData* upload_data, int* error_code);
 
   // Returns the stream's buffer.
   IOBuffer* buf() const { return buf_; }
@@ -44,7 +44,7 @@ class NET_EXPORT UploadDataStream {
 
   // Sets the callback to be invoked when new chunks are available to upload.
   void set_chunk_callback(ChunkCallback* callback) {
-    data_->set_chunk_callback(callback);
+    upload_data_->set_chunk_callback(callback);
   }
 
   // Returns the total size of the data stream and the current position.
@@ -54,7 +54,7 @@ class NET_EXPORT UploadDataStream {
   uint64 size() const { return total_size_; }
   uint64 position() const { return current_position_; }
 
-  bool is_chunked() const { return data_->is_chunked(); }
+  bool is_chunked() const { return upload_data_->is_chunked(); }
 
   // Returns whether there is no more data to read, regardless of whether
   // position < size.
@@ -75,14 +75,21 @@ class NET_EXPORT UploadDataStream {
  private:
   // Protects from public access since now we have a static creator function
   // which will do both creation and initialization and might return an error.
-  explicit UploadDataStream(UploadData* data);
+  explicit UploadDataStream(UploadData* upload_data);
 
   // Fills the buffer with any remaining data and sets eof_ if there was nothing
   // left to fill the buffer with.
   // Returns OK if the operation succeeds. Otherwise error code is returned.
-  int FillBuf();
+  int FillBuffer();
 
-  scoped_refptr<UploadData> data_;
+  // Advances to the next element. Updates the internal states.
+  void AdvanceToNextElement();
+
+  // Returns true if all data has been consumed from this upload data
+  // stream.
+  bool IsEOF() const;
+
+  scoped_refptr<UploadData> upload_data_;
 
   // This buffer is filled with data to be uploaded.  The data to be sent is
   // always at the front of the buffer.  If we cannot send all of the buffer at
@@ -91,22 +98,24 @@ class NET_EXPORT UploadDataStream {
   scoped_refptr<IOBuffer> buf_;
   size_t buf_len_;
 
-  // Index of the upload element to be written to the send buffer next.
-  size_t next_element_;
+  // Index of the current upload element (i.e. the element currently being
+  // read). The index is used as a cursor to iterate over elements in
+  // |upload_data_|.
+  size_t element_index_;
 
-  // The byte offset into next_element_'s data buffer if the next element is
-  // a TYPE_BYTES element.
-  size_t next_element_offset_;
+  // The byte offset into the current element's data buffer if the current
+  // element is a TYPE_BYTES or TYPE_DATA element.
+  size_t element_offset_;
 
-  // A stream to the currently open file, for next_element_ if the next element
-  // is a TYPE_FILE element.
-  scoped_ptr<FileStream> next_element_stream_;
+  // A stream to the currently open file, for the current element if the
+  // current element is a TYPE_FILE element.
+  scoped_ptr<FileStream> element_file_stream_;
 
   // The number of bytes remaining to be read from the currently open file
-  // if the next element is of TYPE_FILE.
-  uint64 next_element_remaining_;
+  // if the current element is of TYPE_FILE.
+  uint64 element_file_bytes_remaining_;
 
-  // Size and current read position within the stream.
+  // Size and current read position within the upload data stream.
   uint64 total_size_;
   uint64 current_position_;