Prototype of chunked transfer encoded POST.

net/base/upload_data_stream.cc

Index: net/base/upload_data_stream.cc
diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc
index 329058c6d771bcf8b4c20e1439cea8c487f97f80..bc1e4446876369192ebc6e6c14203d7858b033c0 100644
--- a/net/base/upload_data_stream.cc
+++ b/net/base/upload_data_stream.cc
@@ -27,12 +27,17 @@ UploadDataStream::UploadDataStream(UploadData* data)
     : data_(data),
       buf_(new IOBuffer(kBufSize)),
       buf_len_(0),
-      next_element_(data->elements()->begin()),
+      next_element_(0),
       next_element_offset_(0),
       next_element_remaining_(0),
       total_size_(data->GetContentLength()),
       current_position_(0),
+      waiting_for_data_(false),

[wtc, 2011/01/14 03:09:31]

IMPORTANT: after studying the code, I think
waiting_for_data_ should be maintained by UploadData
rather than UploadDataStream.

UploadData should initialize waiting_for_data_ to false,
and set waiting_for_data_ to true after it has added the
end-of-chunks marker.

But I'm not convinced that waiting_for_data_ is even
necessary.  It is only used in UploadDataStream::OnChunkAvailable().
UploadDataStream::OnChunkAvailable() is called when a new
chunk is added to the UploadData.  So by definition we must
be waiting for data.

[Satish, 2011/01/14 18:09:29]

Good call, yes this flag is unnecessary now. It was added earlier when I was
using one-shot callbacks to verify the calling order, but with the current
always-on callback model no need to verify. I have changed the code accordingly.

       eof_(false) {
+  if (data_->is_chunked()) {
+    waiting_for_data_ = true;
+    data_->set_chunk_callback(this);
+  }
 }
 
 UploadDataStream::~UploadDataStream() {
@@ -51,23 +56,43 @@ void UploadDataStream::DidConsume(size_t num_bytes) {
   current_position_ += num_bytes;
 }
 
+void UploadDataStream::set_chunk_callback(ChunkCallback* callback) {
+  chunk_callback_ = callback;
+}
+
+void UploadDataStream::OnChunkAvailable() {
+  if (!waiting_for_data_)
+    return;
+
+  waiting_for_data_ = false;
+  FillBuf();
+
+  if (chunk_callback_)
+    chunk_callback_->OnChunkAvailable();
+}
+
 int UploadDataStream::FillBuf() {
-  std::vector<UploadData::Element>::iterator end =
-      data_->elements()->end();
+  std::vector<UploadData::Element>& elements = *data_->elements();
+  if (elements.size() == 0 && data_->is_chunked()) {
+    // We are waiting for data from UploadData, so nothing to do.
+    return OK;
+  }
 
-  while (buf_len_ < kBufSize && next_element_ != end) {
+  while (buf_len_ < kBufSize && next_element_ < elements.size()) {
     bool advance_to_next_element = false;
 
-    UploadData::Element& element = *next_element_;
+    UploadData::Element& element = elements[next_element_];
 
     size_t size_remaining = kBufSize - buf_len_;
-    if (element.type() == UploadData::TYPE_BYTES) {
+    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);
 
-      memcpy(buf_->data() + buf_len_, &d[next_element_offset_], bytes_copied);
+      if (bytes_copied > 0)
+        memcpy(buf_->data() + buf_len_, &d[next_element_offset_], bytes_copied);
       buf_len_ += bytes_copied;
 
       if (bytes_copied == count) {
@@ -126,8 +151,17 @@ int UploadDataStream::FillBuf() {
     }
   }
 
-  if (next_element_ == end && !buf_len_)
-    eof_ = true;
+  if (next_element_ == elements.size() && !buf_len_) {

[vandebo (ex-Chrome), 2011/01/14 05:53:44]

Could this conditional be true and the next false (leading to eof_ = true) if we
get to this point before the first chunk of data is provided?  next_element_
will be 0, element.size() will be 0, but next_element_ won't be > 0.

[Satish, 2011/01/14 18:09:29]

There is an if statement at the very beginning of this function to catch this
case.

+    // If the last element in the list is a CHUNK buffer of non-zero length, we
+    // have to wait for more data to come in.
+    if (next_element_ > 0 &&
+        elements[next_element_ - 1].type() == UploadData::TYPE_CHUNK &&
+        elements[next_element_ - 1].bytes().size() > 0) {

[vandebo (ex-Chrome), 2011/01/14 05:53:44]

The last chunk won't have zero size at this point though, it should be
"0\r\n\r\n", size 5.  Probably cleanest to add a bool and accessor to
UploadData::Element to indicate the last chunk.  Either that, or you could
introduce a TYPE_LAST_CHUNK, but that may not be as clean.

[Satish, 2011/01/14 18:09:29]

In http the last chunk is not zero bytes in size. But in our elements array the
"0\r\n\r\n" chunk is the last-but-one elment. The very last item in our array is
an end-of-stream marker chunk with a zero length array. Wan-Teh pointed out as
well that this is confusing and suggested a change in name and comments, I have
made those changes now.

+      waiting_for_data_ = true;
+    } else {
+      eof_ = true;
+    }
+  }
 
   return OK;
 }