Add chunked uploads support to SPDY

net/http/http_stream_parser.cc

Index: net/http/http_stream_parser.cc
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 2d00a26648c60bbb6a4fd13703c17ec58a497a69..12011c55a23ff228d6214c123817df47146c4ce1 100644
--- a/net/http/http_stream_parser.cc
+++ b/net/http/http_stream_parser.cc
@@ -6,6 +6,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/metrics/histogram.h"
+#include "base/string_util.h"
 #include "net/base/auth.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ssl_cert_request_info.h"
@@ -39,7 +40,8 @@ HttpStreamParser::HttpStreamParser(ClientSocketHandle* connection,
       connection_(connection),
       net_log_(net_log),
       ALLOW_THIS_IN_INITIALIZER_LIST(
-          io_callback_(this, &HttpStreamParser::OnIOComplete)) {
+          io_callback_(this, &HttpStreamParser::OnIOComplete)),
+      chunk_data_length_(0) {
   DCHECK_EQ(0, read_buffer->offset());
 }
 
@@ -70,8 +72,12 @@ int HttpStreamParser::SendRequest(const std::string& request_line,
   request_headers_ = new DrainableIOBuffer(headers_io_buf,
                                            headers_io_buf->size());
   request_body_.reset(request_body);
-  if (request_body_ != NULL && request_body_->is_chunked())
+  if (request_body_ != NULL && request_body_->is_chunked()) {
     request_body_->set_chunk_callback(this);
+    const int kChunkHeaderFooterSize = 12;  // 2 CRLFs + max of 8 hex chars.
+    chunk_buf_ = new IOBuffer(request_body_->GetMaxBufferSize() +
+                              kChunkHeaderFooterSize);
+  }
 
   io_state_ = STATE_SENDING_HEADERS;
   int result = DoLoop(OK);
@@ -266,18 +272,37 @@ int HttpStreamParser::DoSendHeaders(int result) {
 }
 
 int HttpStreamParser::DoSendBody(int result) {
-  request_body_->MarkConsumedAndFillBuffer(result);
+  request_body_->MarkConsumedAndFillBuffer(request_body_->is_chunked() ?
+      chunk_data_length_ : result);

[Satish, 2011/01/26 22:36:33]

We use "chunk_data_length_" here because UploadDataStream only has the data and
not the chunk header+footer. For e.g. If the chunk data was 100 bytes long we'll
be sending 106 bytes (2 byte len, 2 crlfs, data) over the wire and result will
be 106 here.. but we should be consuming only 100 bytes from UploadDataStream

[willchan no longer on Chromium, 2011/01/27 18:47:49]

This is buggy. ClientSocket::Write() may not write all bytes. It might only
partially write.

I think the correct way to do this is to only call MarkConsumedAndFillBuffer()
when you've successfully written the full chunk.

Going further, it seems to be that we should stop calling this
MarkConsumedAndFillBuffer() and redo the functionality. Perhaps it'd be better
to have something like int int GetNextBuffer(scoped_refptr<IOBufferWithSize>*
buf, CompletionCallback* callback);

Then it'd return ERR_IO_PENDING if the buffer is not yet available. Otherwise,
it'd return the number of bytes. 0 should mean EOF.

[Satish, 2011/01/27 20:42:09]

Ok. I can stash the number of bytes given to Write() in a member var, decrement
it by the value received by OnIOComplete() and call DoLoop() only when it
reaches zero. Is that ok or do you have other suggestions?
Sounds good. This seems like a general change in UploadDataStream and not
related to SPDY. Can I do it in a subsequent CL? This also requires that
UploadDataStream will now implement ChunkCallback and receive the call from
UploadData, and in turn invoke the CompletionCallback you mention above. That
would be a self contained change on its own.

[willchan no longer on Chromium, 2011/01/28 00:56:17]

I'm not sure I follow. Write() with 10 bytes may have a return value of 3. In
order to complete it, you need to call Write() again, with the buffer index
incremented by 3, and the number of bytes to write changed to 7. When
OnIOComplete(result) is called, there's no way to know which state you're in. It
could be a Read() that is completing. So you have to call DoLoop() to enter the
state machine so you know that it's the return value for Write().

I think you need to expand the number of states in the state machine. And the
state machine will be easier to understand if you implement my suggestion below
wrt UploadDataStream::MarkConsumedAndFillBuffer() vs
UploadDataStream::GetNextBuffer().

I think you need states like.
STATE_GET_BODY,
STATE_GET_BODY_COMPLETE,
STATE_SEND_BODY,
STATE_SEND_BODY_COMPLETE

STATE_GET_BODY calls request_body_->GetNextBuffer(). Pass the return value to
STATE_GET_BODY_COMPLETE
In STATE_GET_BODY_COMPLETE, if the result is positive, go straight to
STATE_SEND_BODY. If the result is negative, it must be ERR_IO_PENDING, mark the
next state as STATE_SEND_BODY, and then bail out of the state machine. We'll
continue when it comes back.
In STATE_SEND_BODY_COMPLETE, if we're sent the whole chunk, go to
STATE_GET_BODY. Otherwise, go to back to STATE_SEND_BODY.

 
   if (!request_body_->eof()) {
     int buf_len = static_cast<int>(request_body_->buf_len());
-    if (buf_len) {
+    if (request_body_->is_chunked()) {
+      if (buf_len) {
+        // Encode and send the buffer as 1 chunk.
+        std::string chunk_header = StringPrintf("%X\r\n", buf_len);
+        char* chunk_ptr = chunk_buf_->data();
+        memcpy(chunk_ptr, chunk_header.data(), chunk_header.length());
+        memcpy(chunk_ptr + chunk_header.length(), request_body_->buf()->data(),
+               buf_len);
+        memcpy(chunk_ptr + chunk_header.length() + buf_len, "\r\n", 2);
+        chunk_data_length_ = buf_len;
+        result = connection_->socket()->Write(chunk_buf_,
+            chunk_header.length() + buf_len + 2, &io_callback_);
+      } else {
+        // More POST data is to come hence wait for the callback.
+        result = ERR_IO_PENDING;
+      }
+    } else {
       result = connection_->socket()->Write(request_body_->buf(), buf_len,
                                             &io_callback_);
-    } else {
-      // More POST data is to come hence wait for the callback.
-      result = ERR_IO_PENDING;
     }
   } else {
+    if (request_body_->is_chunked()) {
+      chunk_data_length_ = 0;
+      memcpy(chunk_buf_->data(), "0\r\n\r\n", 5);
+      result = connection_->socket()->Write(chunk_buf_, 5, &io_callback_);
+    }
     io_state_ = STATE_REQUEST_SENT;
   }
   return result;