Add chunked uploads support to SPDY

As part of this, I had to move the chunked encoding part from UploadData::Element::SetChunk
to HttpStreamParser::DoSendBody as SPDY doesn't have this encoded format and UploadData
needs to serve both.

BUG=none
TEST=net_unittests (2 new tests added)

Committed and rolled back: http://src.chromium.org/viewvc/chrome?view=rev&revision=76892

Review URL: http://codereview.chromium.org/6292013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@76930 0039d316-1c4b-4281-b951-d872f2087c98

22 files changed, 355 insertions, 66 deletions

diff --git a/net/base/upload_data.cc b/net/base/upload_data.cc
index 490029c..4af308e 100644
--- a/net/base/upload_data.cc
+++ b/net/base/upload_data.cc
@@ -28,16 +28,13 @@ UploadData::Element::~Element() {
   delete file_stream_;
 }
 
-void UploadData::Element::SetToChunk(const char* bytes, int bytes_len) {
-  std::string chunk_length = StringPrintf("%X\r\n", bytes_len);
+void UploadData::Element::SetToChunk(const char* bytes,
+                                     int bytes_len,
+                                     bool is_last_chunk) {
   bytes_.clear();
-  bytes_.insert(bytes_.end(), chunk_length.data(),
-                chunk_length.data() + chunk_length.length());
   bytes_.insert(bytes_.end(), bytes, bytes + bytes_len);
-  const char* crlf = "\r\n";
-  bytes_.insert(bytes_.end(), crlf, crlf + 2);
   type_ = TYPE_CHUNK;
-  is_last_chunk_ = (bytes_len == 0);
+  is_last_chunk_ = is_last_chunk;
 }
 
 uint64 UploadData::Element::GetContentLength() {
@@ -146,10 +143,12 @@ void UploadData::AppendBlob(const GURL& blob_url) {
   elements_.back().SetToBlobUrl(blob_url);
 }
 
-void UploadData::AppendChunk(const char* bytes, int bytes_len) {
+void UploadData::AppendChunk(const char* bytes,
+                             int bytes_len,
+                             bool is_last_chunk) {
   DCHECK(is_chunked_);
   elements_.push_back(Element());
-  elements_.back().SetToChunk(bytes, bytes_len);
+  elements_.back().SetToChunk(bytes, bytes_len, is_last_chunk);
   if (chunk_callback_)
     chunk_callback_->OnChunkAvailable();
 }
diff --git a/net/base/upload_data.h b/net/base/upload_data.h
index e746f65..b9cc864 100644
--- a/net/base/upload_data.h
+++ b/net/base/upload_data.h
@@ -96,7 +96,7 @@ class UploadData : public base::RefCounted<UploadData> {
     // Though similar to bytes, a chunk indicates that the element is sent via
     // chunked transfer encoding and not buffered until the full upload data
     // is available.
-    void SetToChunk(const char* bytes, int bytes_len);
+    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.
@@ -153,8 +153,8 @@ class UploadData : public base::RefCounted<UploadData> {
   void AppendBlob(const GURL& blob_url);
 
   // Adds the given chunk of bytes to be sent immediately with chunked transfer
-  // encoding. Set bytes_len to zero for the last chunk.
-  void AppendChunk(const char* bytes, int bytes_len);
+  // encoding.
+  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);
diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc
index 9f7bdbb..4a056f4 100644
--- a/net/base/upload_data_stream.cc
+++ b/net/base/upload_data_stream.cc
@@ -12,6 +12,8 @@
 
 namespace net {
 
+bool UploadDataStream::merge_chunks_ = true;
+
 UploadDataStream::~UploadDataStream() {
 }
 
@@ -69,8 +71,13 @@ int UploadDataStream::FillBuf() {
 
       size_t bytes_copied = std::min(count, size_remaining);
 
-      memcpy(buf_->data() + buf_len_, &d[next_element_offset_], bytes_copied);
-      buf_len_ += bytes_copied;
+      // 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;
+      }
 
       if (bytes_copied == count) {
         advance_to_next_element = true;
@@ -126,6 +133,9 @@ int UploadDataStream::FillBuf() {
       next_element_remaining_ = 0;
       next_element_stream_.reset();
     }
+
+    if (is_chunked() && !merge_chunks_)
+      break;
   }
 
   if (next_element_ == elements.size() && !buf_len_) {
@@ -138,4 +148,13 @@ int UploadDataStream::FillBuf() {
   return OK;
 }
 
+bool UploadDataStream::IsOnLastChunk() const {
+  const std::vector<UploadData::Element>& elements = *data_->elements();
+  DCHECK(data_->is_chunked());
+  return (eof_ ||
+          (!elements.empty() &&
+           next_element_ == elements.size() &&
+           elements.back().is_last_chunk()));
+}
+
 }  // namespace net
diff --git a/net/base/upload_data_stream.h b/net/base/upload_data_stream.h
index f291140..decc3f6 100644
--- a/net/base/upload_data_stream.h
+++ b/net/base/upload_data_stream.h
@@ -27,6 +27,12 @@ class UploadDataStream {
   IOBuffer* buf() const { return buf_; }
   size_t buf_len() const { return buf_len_; }
 
+  // TODO(satish): We should ideally have UploadDataStream expose a Read()
+  // method which returns data in a caller provided IOBuffer. That would do away
+  // with this method and make the interface cleaner as well with less memmove
+  // calls.
+  size_t GetMaxBufferSize() const { return kBufSize; }
+
   // Call to indicate that a portion of the stream's buffer was consumed.  This
   // call modifies the stream's buffer so that it contains the next segment of
   // the upload data to be consumed.
@@ -50,6 +56,16 @@ class UploadDataStream {
   // position < size.
   bool eof() const { return eof_; }
 
+  // Returns whether the data available in buf() includes the last chunk in a
+  // chunked data stream. This method returns true once the final chunk has been
+  // placed in the IOBuffer returned by buf(), in contrast to eof() which
+  // returns true only after the data in buf() has been consumed.
+  bool IsOnLastChunk() const;
+
+#if defined(UNIT_TEST)
+  static void set_merge_chunks(bool merge) { merge_chunks_ = merge; }
+#endif
+
  private:
   enum { kBufSize = 16384 };
 
@@ -93,6 +109,10 @@ class UploadDataStream {
   // Whether there is no data left to read.
   bool eof_;
 
+  // TODO(satish): Remove this once we have a better way to unit test POST
+  // requests with chunked uploads.
+  static bool merge_chunks_;
+
   DISALLOW_COPY_AND_ASSIGN(UploadDataStream);
 };
 
diff --git a/net/http/http_stream_parser.cc b/net/http/http_stream_parser.cc
index 6621f0b..eb1ed35 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/address_list.h"
 #include "net/base/auth.h"
 #include "net/base/io_buffer.h"
@@ -40,7 +41,10 @@ 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_length_(0),
+      chunk_length_without_encoding_(0),
+      sent_last_chunk_(false) {
   DCHECK_EQ(0, read_buffer->offset());
 }
 
@@ -79,8 +83,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;
   result = DoLoop(OK);
@@ -276,17 +284,56 @@ int HttpStreamParser::DoSendHeaders(int result) {
 }
 
 int HttpStreamParser::DoSendBody(int result) {
+  if (request_body_->is_chunked()) {
+    chunk_length_ -= result;
+    if (chunk_length_) {
+      memmove(chunk_buf_->data(), chunk_buf_->data() + result, chunk_length_);
+      return connection_->socket()->Write(chunk_buf_, chunk_length_,
+                                          &io_callback_);
+    }
+
+    if (sent_last_chunk_) {
+      io_state_ = STATE_REQUEST_SENT;
+      return OK;
+    }
+
+    request_body_->MarkConsumedAndFillBuffer(chunk_length_without_encoding_);
+    chunk_length_without_encoding_ = 0;
+    chunk_length_ = 0;
+
+    int buf_len = static_cast<int>(request_body_->buf_len());
+    if (request_body_->eof()) {
+      static const char kLastChunk[] = "0\r\n\r\n";
+      chunk_length_ = strlen(kLastChunk);
+      memcpy(chunk_buf_->data(), kLastChunk, chunk_length_);
+      sent_last_chunk_ = true;
+    } else 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());
+      chunk_ptr += chunk_header.length();
+      memcpy(chunk_ptr, request_body_->buf()->data(), buf_len);
+      chunk_ptr += buf_len;
+      memcpy(chunk_ptr, "\r\n", 2);
+      chunk_length_without_encoding_ = buf_len;
+      chunk_length_ = chunk_header.length() + buf_len + 2;
+    }
+
+    if (!chunk_length_)  // More POST data is yet to come?
+      return ERR_IO_PENDING;
+
+    return connection_->socket()->Write(chunk_buf_, chunk_length_,
+                                        &io_callback_);
+  }
+
+  // Non-chunked request body.
   request_body_->MarkConsumedAndFillBuffer(result);
 
   if (!request_body_->eof()) {
     int buf_len = static_cast<int>(request_body_->buf_len());
-    if (buf_len) {
-      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;
-    }
+    result = connection_->socket()->Write(request_body_->buf(), buf_len,
+                                          &io_callback_);
   } else {
     io_state_ = STATE_REQUEST_SENT;
   }
diff --git a/net/http/http_stream_parser.h b/net/http/http_stream_parser.h
index 5f7e943..d9241a6 100644
--- a/net/http/http_stream_parser.h
+++ b/net/http/http_stream_parser.h
@@ -189,6 +189,13 @@ class HttpStreamParser  : public ChunkCallback {
   // Callback to be used when doing IO.
   CompletionCallbackImpl<HttpStreamParser> io_callback_;
 
+  // Stores an encoded chunk for chunked uploads.
+  // Note: This should perhaps be improved to not create copies of the data.
+  scoped_refptr<IOBuffer> chunk_buf_;
+  size_t chunk_length_;
+  size_t chunk_length_without_encoding_;
+  bool sent_last_chunk_;
+
   DISALLOW_COPY_AND_ASSIGN(HttpStreamParser);
 };
 
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index e07578e..51a2efd 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -177,6 +177,11 @@ void SpdyHttpStream::SetConnectionReused() {
   // SPDY doesn't need an indicator here.
 }
 
+void SpdyHttpStream::set_chunk_callback(ChunkCallback* callback) {
+  if (request_body_stream_ != NULL)
+    request_body_stream_->set_chunk_callback(callback);
+}
+
 int SpdyHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
                                 UploadDataStream* request_body,
                                 HttpResponseInfo* response,
@@ -200,7 +205,7 @@ int SpdyHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
 
   CHECK(!request_body_stream_.get());
   if (request_body) {
-    if (request_body->size())
+    if (request_body->size() || request_body->is_chunked())
       request_body_stream_.reset(request_body);
     else
       delete request_body;
@@ -264,17 +269,33 @@ bool SpdyHttpStream::OnSendHeadersComplete(int status) {
 
 int SpdyHttpStream::OnSendBody() {
   CHECK(request_body_stream_.get());
+
   int buf_len = static_cast<int>(request_body_stream_->buf_len());
   if (!buf_len)
     return OK;
-  return stream_->WriteStreamData(request_body_stream_->buf(), buf_len,
-                                  spdy::DATA_FLAG_FIN);
+  bool is_chunked = request_body_stream_->is_chunked();
+  // TODO(satish): For non-chunked POST data, we set DATA_FLAG_FIN for all
+  // blocks of data written out. This is wrong if the POST data was larger than
+  // UploadDataStream::kBufSize as that is the largest buffer that
+  // UploadDataStream returns at a time and we'll be setting the FIN flag for
+  // each block of data written out.
+  bool eof = !is_chunked || request_body_stream_->IsOnLastChunk();
+  return stream_->WriteStreamData(
+      request_body_stream_->buf(), buf_len,
+      eof ? spdy::DATA_FLAG_FIN : spdy::DATA_FLAG_NONE);
 }
 
-bool SpdyHttpStream::OnSendBodyComplete(int status) {
+int SpdyHttpStream::OnSendBodyComplete(int status, bool* eof) {
   CHECK(request_body_stream_.get());
+
   request_body_stream_->MarkConsumedAndFillBuffer(status);
-  return request_body_stream_->eof();
+  *eof = request_body_stream_->eof();
+  if (!*eof &&
+      request_body_stream_->is_chunked() &&
+      !request_body_stream_->buf_len())
+    return ERR_IO_PENDING;
+
+  return OK;
 }
 
 int SpdyHttpStream::OnResponseReceived(const spdy::SpdyHeaderBlock& response,
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index 6e78379..bdae342 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -69,13 +69,14 @@ class SpdyHttpStream : public SpdyStream::Delegate, public HttpStream {
   // SpdyStream::Delegate methods:
   virtual bool OnSendHeadersComplete(int status);
   virtual int OnSendBody();
-  virtual bool OnSendBodyComplete(int status);
+  virtual int OnSendBodyComplete(int status, bool* eof);
   virtual int OnResponseReceived(const spdy::SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status);
   virtual void OnDataReceived(const char* buffer, int bytes);
   virtual void OnDataSent(int length);
   virtual void OnClose(int status);
+  virtual void set_chunk_callback(ChunkCallback* callback);
 
  private:
   FRIEND_TEST_ALL_PREFIXES(SpdyNetworkTransactionTest, FlowControlStallResume);
@@ -127,6 +128,8 @@ class SpdyHttpStream : public SpdyStream::Delegate, public HttpStream {
   // Is this spdy stream direct to the origin server (or to a proxy).
   bool direct_;
 
+  bool send_last_chunk_;
+
   DISALLOW_COPY_AND_ASSIGN(SpdyHttpStream);
 };
 
diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc
index 1688871..bee345a 100644
--- a/net/spdy/spdy_http_stream_unittest.cc
+++ b/net/spdy/spdy_http_stream_unittest.cc
@@ -93,7 +93,69 @@ TEST_F(SpdyHttpStreamTest, SendRequest) {
 
   // Because we abandoned the stream, we don't expect to find a session in the
   // pool anymore.
-  EXPECT_TRUE(!http_session_->spdy_session_pool()->HasSession(pair));
+  EXPECT_FALSE(http_session_->spdy_session_pool()->HasSession(pair));
+  EXPECT_TRUE(data()->at_read_eof());
+  EXPECT_TRUE(data()->at_write_eof());
+}
+
+TEST_F(SpdyHttpStreamTest, SendChunkedPost) {
+  EnableCompression(false);
+  SpdySession::SetSSLMode(false);
+  UploadDataStream::set_merge_chunks(false);
+
+  scoped_ptr<spdy::SpdyFrame> req(ConstructChunkedSpdyPost(NULL, 0));
+  scoped_ptr<spdy::SpdyFrame> chunk1(ConstructSpdyBodyFrame(1, false));
+  scoped_ptr<spdy::SpdyFrame> chunk2(ConstructSpdyBodyFrame(1, true));
+  MockWrite writes[] = {
+    CreateMockWrite(*req.get(), 1),
+    CreateMockWrite(*chunk1, 2),  // POST upload frames
+    CreateMockWrite(*chunk2, 3),
+  };
+  scoped_ptr<spdy::SpdyFrame> resp(ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+    CreateMockRead(*resp, 4),
+    CreateMockRead(*chunk1, 5),
+    CreateMockRead(*chunk2, 5),
+    MockRead(false, 0, 6)  // EOF
+  };
+
+  HostPortPair host_port_pair("www.google.com", 80);
+  HostPortProxyPair pair(host_port_pair, ProxyServer::Direct());
+  EXPECT_EQ(OK, InitSession(reads, arraysize(reads), writes, arraysize(writes),
+                            host_port_pair));
+
+  HttpRequestInfo request;
+  request.method = "POST";
+  request.url = GURL("http://www.google.com/");
+  request.upload_data = new UploadData();
+  request.upload_data->set_is_chunked(true);
+  request.upload_data->AppendChunk(kUploadData, kUploadDataSize, false);
+  request.upload_data->AppendChunk(kUploadData, kUploadDataSize, true);
+  TestCompletionCallback callback;
+  HttpResponseInfo response;
+  HttpRequestHeaders headers;
+  BoundNetLog net_log;
+  SpdyHttpStream http_stream(session_.get(), true);
+  ASSERT_EQ(
+      OK,
+      http_stream.InitializeStream(&request, net_log, NULL));
+
+  UploadDataStream* upload_stream =
+      UploadDataStream::Create(request.upload_data, NULL);
+  EXPECT_EQ(ERR_IO_PENDING, http_stream.SendRequest(
+      headers, upload_stream, &response, &callback));
+  EXPECT_TRUE(http_session_->spdy_session_pool()->HasSession(pair));
+
+  // This triggers the MockWrite and read 2
+  callback.WaitForResult();
+
+  // This triggers read 3. The empty read causes the session to shut down.
+  data()->CompleteRead();
+  MessageLoop::current()->RunAllPending();
+
+  // Because we abandoned the stream, we don't expect to find a session in the
+  // pool anymore.
+  EXPECT_FALSE(http_session_->spdy_session_pool()->HasSession(pair));
   EXPECT_TRUE(data()->at_read_eof());
   EXPECT_TRUE(data()->at_write_eof());
 }
@@ -151,7 +213,7 @@ TEST_F(SpdyHttpStreamTest, SpdyURLTest) {
 
   // Because we abandoned the stream, we don't expect to find a session in the
   // pool anymore.
-  EXPECT_TRUE(!http_session_->spdy_session_pool()->HasSession(pair));
+  EXPECT_FALSE(http_session_->spdy_session_pool()->HasSession(pair));
   EXPECT_TRUE(data()->at_read_eof());
   EXPECT_TRUE(data()->at_write_eof());
 }
diff --git a/net/spdy/spdy_http_utils.cc b/net/spdy/spdy_http_utils.cc
index bf7376a..a3b7e54 100644
--- a/net/spdy/spdy_http_utils.cc
+++ b/net/spdy/spdy_http_utils.cc
@@ -83,8 +83,10 @@ void CreateSpdyHeadersFromHttpRequest(const HttpRequestInfo& info,
   HttpRequestHeaders::Iterator it(request_headers);
   while (it.GetNext()) {
     std::string name = StringToLowerASCII(it.name());
-    if (name == "connection" || name == "proxy-connection")
+    if (name == "connection" || name == "proxy-connection" ||
+        name == "transfer-encoding") {
       continue;
+    }
     if (headers->find(name) == headers->end()) {
       (*headers)[name] = it.value();
     } else {
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 84e19d6..94ef61d 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -40,6 +40,7 @@ class SpdyNetworkTransactionTest
     EnableCompression(false);
     google_get_request_initialized_ = false;
     google_post_request_initialized_ = false;
+    google_chunked_post_request_initialized_ = false;
   }
 
   virtual void TearDown() {
@@ -350,6 +351,21 @@ class SpdyNetworkTransactionTest
     return google_post_request_;
   }
 
+  const HttpRequestInfo& CreateChunkedPostRequest() {
+    if (!google_chunked_post_request_initialized_) {
+      google_chunked_post_request_.method = "POST";
+      google_chunked_post_request_.url = GURL(kDefaultURL);
+      google_chunked_post_request_.upload_data = new UploadData();
+      google_chunked_post_request_.upload_data->set_is_chunked(true);
+      google_chunked_post_request_.upload_data->AppendChunk(
+          kUploadData, kUploadDataSize, false);
+      google_chunked_post_request_.upload_data->AppendChunk(
+          kUploadData, kUploadDataSize, true);
+      google_chunked_post_request_initialized_ = true;
+    }
+    return google_chunked_post_request_;
+  }
+
   // Read the result of a particular transaction, knowing that we've got
   // multiple transactions in the read pipeline; so as we read, we may have
   // to skip over data destined for other transactions while we consume
@@ -454,8 +470,10 @@ class SpdyNetworkTransactionTest
  private:
   bool google_get_request_initialized_;
   bool google_post_request_initialized_;
+  bool google_chunked_post_request_initialized_;
   HttpRequestInfo google_get_request_;
   HttpRequestInfo google_post_request_;
+  HttpRequestInfo google_chunked_post_request_;
   HttpRequestInfo google_get_push_request_;
 };
 
@@ -1525,6 +1543,38 @@ TEST_P(SpdyNetworkTransactionTest, Post) {
   EXPECT_EQ("hello!", out.response_data);
 }
 
+// Test that a chunked POST works.
+TEST_P(SpdyNetworkTransactionTest, ChunkedPost) {
+  UploadDataStream::set_merge_chunks(false);
+  scoped_ptr<spdy::SpdyFrame> req(ConstructChunkedSpdyPost(NULL, 0));
+  scoped_ptr<spdy::SpdyFrame> chunk1(ConstructSpdyBodyFrame(1, false));
+  scoped_ptr<spdy::SpdyFrame> chunk2(ConstructSpdyBodyFrame(1, true));
+  MockWrite writes[] = {
+    CreateMockWrite(*req),
+    CreateMockWrite(*chunk1),
+    CreateMockWrite(*chunk2),
+  };
+
+  scoped_ptr<spdy::SpdyFrame> resp(ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+    CreateMockRead(*resp),
+    CreateMockRead(*chunk1),
+    CreateMockRead(*chunk2),
+    MockRead(true, 0, 0)  // EOF
+  };
+
+  scoped_refptr<DelayedSocketData> data(
+      new DelayedSocketData(2, reads, arraysize(reads),
+                            writes, arraysize(writes)));
+  NormalSpdyTransactionHelper helper(CreateChunkedPostRequest(),
+                                     BoundNetLog(), GetParam());
+  helper.RunToCompletion(data.get());
+  TransactionHelperResult out = helper.output();
+  EXPECT_EQ(OK, out.rv);
+  EXPECT_EQ("HTTP/1.1 200 OK", out.status_line);
+  EXPECT_EQ("hello!hello!", out.response_data);
+}
+
 // Test that a POST without any post data works.
 TEST_P(SpdyNetworkTransactionTest, NullPost) {
   // Setup the request
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index 452ee3b..f451868 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -404,11 +404,11 @@ int SpdyProxyClientSocket::OnSendBody() {
   return ERR_UNEXPECTED;
 }
 
-bool SpdyProxyClientSocket::OnSendBodyComplete(int status) {
+int SpdyProxyClientSocket::OnSendBodyComplete(int /*status*/, bool* /*eof*/) {
   // Because we use |spdy_stream_| via STATE_OPEN (ala WebSockets)
   // OnSendBodyComplete() should never be called.
   NOTREACHED();
-  return false;
+  return ERR_UNEXPECTED;
 }
 
 int SpdyProxyClientSocket::OnResponseReceived(
@@ -498,4 +498,7 @@ void SpdyProxyClientSocket::OnClose(int status)  {
     write_callback->Run(ERR_CONNECTION_CLOSED);
 }
 
+void SpdyProxyClientSocket::set_chunk_callback(ChunkCallback* /*callback*/) {
+}
+
 }  // namespace net
diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
index f66283e1..69261d3 100644
--- a/net/spdy/spdy_proxy_client_socket.h
+++ b/net/spdy/spdy_proxy_client_socket.h
@@ -88,13 +88,14 @@ class SpdyProxyClientSocket : public ProxyClientSocket,
   // SpdyStream::Delegate methods:
   virtual bool OnSendHeadersComplete(int status);
   virtual int OnSendBody();
-  virtual bool OnSendBodyComplete(int status);
+  virtual int OnSendBodyComplete(int status, bool* eof);
   virtual int OnResponseReceived(const spdy::SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status);
   virtual void OnDataReceived(const char* data, int length);
   virtual void OnDataSent(int length);
   virtual void OnClose(int status);
+  virtual void set_chunk_callback(ChunkCallback* /*callback*/);
 
  private:
   enum State {
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index c268ee4..9776aff 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -636,7 +636,7 @@ void SpdySession::OnWriteComplete(int result) {
         // size.
         if (result > 0) {
           result = in_flight_write_.buffer()->size();
-          DCHECK_GT(result, static_cast<int>(spdy::SpdyFrame::size()));
+          DCHECK_GE(result, static_cast<int>(spdy::SpdyFrame::size()));
           result -= static_cast<int>(spdy::SpdyFrame::size());
         }
 
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 3900993..e948b33 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -112,6 +112,8 @@ void SpdyStream::PushedStreamReplayData() {
 }
 
 void SpdyStream::DetachDelegate() {
+  if (delegate_)
+    delegate_->set_chunk_callback(NULL);
   delegate_ = NULL;
   if (!closed())
     Cancel();
@@ -348,13 +350,22 @@ void SpdyStream::OnWriteComplete(int bytes) {
   DoLoop(bytes);
 }
 
+void SpdyStream::OnChunkAvailable() {
+  DCHECK(io_state_ == STATE_SEND_HEADERS || io_state_ == STATE_SEND_BODY ||
+         io_state_ == STATE_SEND_BODY_COMPLETE);
+  if (io_state_ == STATE_SEND_BODY)
+    OnWriteComplete(0);
+}
+
 void SpdyStream::OnClose(int status) {
   io_state_ = STATE_DONE;
   response_status_ = status;
   Delegate* delegate = delegate_;
   delegate_ = NULL;
-  if (delegate)
+  if (delegate) {
+    delegate->set_chunk_callback(NULL);
     delegate->OnClose(status);
+  }
 }
 
 void SpdyStream::Cancel() {
@@ -367,6 +378,9 @@ void SpdyStream::Cancel() {
 }
 
 int SpdyStream::SendRequest(bool has_upload_data) {
+  if (delegate_)
+    delegate_->set_chunk_callback(this);
+
   // Pushed streams do not send any data, and should always be in STATE_OPEN or
   // STATE_DONE. However, we still want to return IO_PENDING to mimic non-push
   // behavior.
@@ -545,17 +559,17 @@ int SpdyStream::DoSendBodyComplete(int result) {
   if (result < 0)
     return result;
 
-  CHECK_NE(result, 0);
-
   if (!delegate_)
     return ERR_UNEXPECTED;
 
-  if (!delegate_->OnSendBodyComplete(result))
+  bool eof = false;
+  result = delegate_->OnSendBodyComplete(result, &eof);
+  if (!eof)
     io_state_ = STATE_SEND_BODY;
   else
     io_state_ = STATE_WAITING_FOR_RESPONSE;
 
-  return OK;
+  return result;
 }
 
 int SpdyStream::DoOpen(int result) {
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index 9a055c2..6d6b09c 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -16,6 +16,7 @@
 #include "googleurl/src/gurl.h"
 #include "net/base/bandwidth_metrics.h"
 #include "net/base/io_buffer.h"
+#include "net/base/upload_data.h"
 #include "net/base/net_log.h"
 #include "net/spdy/spdy_framer.h"
 #include "net/spdy/spdy_protocol.h"
@@ -34,7 +35,9 @@ class SSLInfo;
 // a SpdyNetworkTransaction) will maintain a reference to the stream.  When
 // initiated by the server, only the SpdySession will maintain any reference,
 // until such a time as a client object requests a stream for the path.
-class SpdyStream : public base::RefCounted<SpdyStream> {
+class SpdyStream
+    : public base::RefCounted<SpdyStream>,
+      public ChunkCallback {
  public:
   // Delegate handles protocol specific behavior of spdy stream.
   class Delegate {
@@ -50,9 +53,10 @@ class SpdyStream : public base::RefCounted<SpdyStream> {
     virtual int OnSendBody() = 0;
 
     // Called when data has been sent. |status| indicates network error
-    // or number of bytes has been sent.
-    // Returns true if no more data to be sent.
-    virtual bool OnSendBodyComplete(int status) = 0;
+    // or number of bytes that has been sent. On return, |eof| is set to true
+    // if no more data is available to send in the request body.
+    // Returns network error code. OK when it successfully sent data.
+    virtual int OnSendBodyComplete(int status, bool* eof) = 0;
 
     // Called when the SYN_STREAM, SYN_REPLY, or HEADERS frames are received.
     // Normal streams will receive a SYN_REPLY and optional HEADERS frames.
@@ -73,6 +77,9 @@ class SpdyStream : public base::RefCounted<SpdyStream> {
     // Called when SpdyStream is closed.
     virtual void OnClose(int status) = 0;
 
+    // Sets the callback to be invoked when a new chunk is available to upload.
+    virtual void set_chunk_callback(ChunkCallback* callback) = 0;
+
    protected:
     friend class base::RefCounted<Delegate>;
     virtual ~Delegate() {}
@@ -222,6 +229,9 @@ class SpdyStream : public base::RefCounted<SpdyStream> {
   // true.
   GURL GetUrl() const;
 
+  // ChunkCallback methods.
+  virtual void OnChunkAvailable();
+
  private:
   enum State {
     STATE_NONE,
diff --git a/net/spdy/spdy_stream_unittest.cc b/net/spdy/spdy_stream_unittest.cc
index 532c79f..a7b4fac 100644
--- a/net/spdy/spdy_stream_unittest.cc
+++ b/net/spdy/spdy_stream_unittest.cc
@@ -51,9 +51,9 @@ class TestSpdyStreamDelegate : public SpdyStream::Delegate {
     ADD_FAILURE() << "OnSendBody should not be called";
     return ERR_UNEXPECTED;
   }
-  virtual bool OnSendBodyComplete(int status) {
+  virtual int OnSendBodyComplete(int /*status*/, bool* /*eof*/) {
     ADD_FAILURE() << "OnSendBodyComplete should not be called";
-    return true;
+    return ERR_UNEXPECTED;
   }
 
   virtual int OnResponseReceived(const spdy::SpdyHeaderBlock& response,
@@ -80,6 +80,7 @@ class TestSpdyStreamDelegate : public SpdyStream::Delegate {
     callback_ = NULL;
     callback->Run(OK);
   }
+  virtual void set_chunk_callback(net::ChunkCallback *) {}
 
   bool send_headers_completed() const { return send_headers_completed_; }
   const linked_ptr<spdy::SpdyHeaderBlock>& response() const {
diff --git a/net/spdy/spdy_test_util.cc b/net/spdy/spdy_test_util.cc
index 6342b16..7e1711a 100644
--- a/net/spdy/spdy_test_util.cc
+++ b/net/spdy/spdy_test_util.cc
@@ -672,6 +672,35 @@ spdy::SpdyFrame* ConstructSpdyPost(int64 content_length,
                                    arraysize(post_headers));
 }
 
+// Constructs a chunked transfer SPDY POST SYN packet.
+// |extra_headers| are the extra header-value pairs, which typically
+// will vary the most between calls.
+// Returns a SpdyFrame.
+spdy::SpdyFrame* ConstructChunkedSpdyPost(const char* const extra_headers[],
+                                          int extra_header_count) {
+  const char* post_headers[] = {
+    "method",
+    "POST",
+    "url",
+    "/",
+    "host",
+    "www.google.com",
+    "scheme",
+    "http",
+    "version",
+    "HTTP/1.1"
+  };
+  return ConstructSpdyControlFrame(extra_headers,
+                                   extra_header_count,
+                                   false,
+                                   1,
+                                   LOWEST,
+                                   spdy::SYN_STREAM,
+                                   spdy::CONTROL_FLAG_NONE,
+                                   post_headers,
+                                   arraysize(post_headers));
+}
+
 // Constructs a standard SPDY SYN_REPLY packet to match the SPDY POST.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.
diff --git a/net/spdy/spdy_test_util.h b/net/spdy/spdy_test_util.h
index 7bd78b6..4cfed13 100644
--- a/net/spdy/spdy_test_util.h
+++ b/net/spdy/spdy_test_util.h
@@ -280,6 +280,13 @@ spdy::SpdyFrame* ConstructSpdyPost(int64 content_length,
                                    const char* const extra_headers[],
                                    int extra_header_count);
 
+// Constructs a chunked transfer SPDY POST SYN packet.
+// |extra_headers| are the extra header-value pairs, which typically
+// will vary the most between calls.
+// Returns a SpdyFrame.
+spdy::SpdyFrame* ConstructChunkedSpdyPost(const char* const extra_headers[],
+                                          int extra_header_count);
+
 // Constructs a standard SPDY SYN_REPLY packet to match the SPDY POST.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.
diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc
index e25bac6..5af99ea 100644
--- a/net/url_request/url_request.cc
+++ b/net/url_request/url_request.cc
@@ -184,17 +184,13 @@ void URLRequest::EnableChunkedUpload() {
   }
 }
 
-void URLRequest::AppendChunkToUpload(const char* bytes, int bytes_len) {
+void URLRequest::AppendChunkToUpload(const char* bytes,
+                                     int bytes_len,
+                                     bool is_last_chunk) {
   DCHECK(upload_);
   DCHECK(upload_->is_chunked());
   DCHECK_GT(bytes_len, 0);
-  upload_->AppendChunk(bytes, bytes_len);
-}
-
-void URLRequest::MarkEndOfChunks() {
-  DCHECK(upload_);
-  DCHECK(upload_->is_chunked());
-  upload_->AppendChunk(NULL, 0);
+  upload_->AppendChunk(bytes, bytes_len, is_last_chunk);
 }
 
 void URLRequest::set_upload(net::UploadData* upload) {
diff --git a/net/url_request/url_request.h b/net/url_request/url_request.h
index a2ad24b..849a03b 100644
--- a/net/url_request/url_request.h
+++ b/net/url_request/url_request.h
@@ -330,10 +330,9 @@ class URLRequest : public base::NonThreadSafe {
   // call MarkEndOfChunks() to indicate the end of upload data.
   //
   // This method may be called only after calling EnableChunkedUpload().
-  void AppendChunkToUpload(const char* bytes, int bytes_len);
-
-  // Indicates the end of a chunked transfer encoded request body.
-  void MarkEndOfChunks();
+  void AppendChunkToUpload(const char* bytes,
+                           int bytes_len,
+                           bool is_last_chunk);
 
   // Set the upload data directly.
   void set_upload(net::UploadData* upload);
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index 5e03f9b..d0b8f58 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -178,13 +178,12 @@ class URLRequestTestHTTP : public URLRequestTest {
   }
 
   void AddChunksToUpload(TestURLRequest* r) {
-    r->AppendChunkToUpload("a", 1);
-    r->AppendChunkToUpload("bcd", 3);
-    r->AppendChunkToUpload("this is a longer chunk than before.", 35);
-    r->AppendChunkToUpload("\r\n\r\n", 4);
-    r->AppendChunkToUpload("0", 1);
-    r->AppendChunkToUpload("2323", 4);
-    r->MarkEndOfChunks();
+    r->AppendChunkToUpload("a", 1, false);
+    r->AppendChunkToUpload("bcd", 3, false);
+    r->AppendChunkToUpload("this is a longer chunk than before.", 35, false);
+    r->AppendChunkToUpload("\r\n\r\n", 4, false);
+    r->AppendChunkToUpload("0", 1, false);
+    r->AppendChunkToUpload("2323", 4, true);
   }
 
   void VerifyReceivedDataMatchesChunks(TestURLRequest* r, TestDelegate* d) {