Change SPDY to call request_callback after data is sent.

net/spdy/spdy_http_stream.cc

Index: net/spdy/spdy_http_stream.cc
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 259581a5229173cc2e7fcbc84963396f357c0d80..97c4ad6e1fec4899313131ab8c4b1bdf20abb998 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -302,13 +302,12 @@ void SpdyHttpStream::Cancel() {
 }
 
 void SpdyHttpStream::OnRequestHeadersSent() {
-  if (!request_callback_.is_null())
-    DoRequestCallback(OK);
-
-  // TODO(akalin): Do this immediately after sending the request
-  // headers.
-  if (HasUploadData())
+  if (HasUploadData()) {
     ReadAndSendRequestBodyData();
+  } else {
+    if (!request_callback_.is_null())
+      DoRequestCallback(OK);
+  }
 }
 
 SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated(
@@ -437,8 +436,14 @@ void SpdyHttpStream::ReadAndSendRequestBodyData() {
   CHECK(HasUploadData());
   CHECK_EQ(request_body_buf_size_, 0);
 
-  if (request_info_->upload_data_stream->IsEOF())
+  if (request_info_->upload_data_stream->IsEOF()) {
+    // The cb should be null here, but it's better to be
+    // paranoid.

[mmenke, 2016/06/14 15:25:44]

I don't believe this is true - when uploading chunked files, we may not learn
it's a 0-byte file until the call here to upload_data_stream->IsEOF() returns
true.

That's the reason the IsEOF check is needed here in the first place.

[maksims (do not use this acc), 2016/06/15 07:51:45]

Check the SendChunkedPostLastEmpty that I added. As you have said there is a
0-byte chunk, but the eof is checked first in OnRequestBodyReadCompleted() and
then we check the eof here due to the DoLoop in SpdySession and then we exit.

+    if (!request_callback_.is_null()) {
+      DoRequestCallback(OK);
+    }

[mmenke, 2016/06/14 15:25:44]

nit:  Remove braces

     return;
+  }
 
   // Read the data from the request body stream.
   const int rv = request_info_->upload_data_stream
@@ -461,6 +466,10 @@ void SpdyHttpStream::OnRequestBodyReadCompleted(int status) {
   // Only the final fame may have a length of 0.
   if (eof) {
     CHECK_GE(request_body_buf_size_, 0);
+    // Call the cb only after the whole data is sent.

[mmenke, 2016/06/14 15:25:44]

nit:  "after the whole file is sent." or "after all data is sent."

The reason for that is that "whole" means all of a single thing, but "data" is
plural.

[maksims (do not use this acc), 2016/06/15 07:51:45]

Done.

+    if (!request_callback_.is_null()) {
+      DoRequestCallback(OK);
+    }

[mmenke, 2016/06/14 15:25:44]

nit:  Remove braces

[maksims (do not use this acc), 2016/06/15 07:51:45]

Done.

   } else {
     CHECK_GT(request_body_buf_size_, 0);
   }
@@ -533,7 +542,6 @@ void SpdyHttpStream::DoBufferedReadCallback() {
 void SpdyHttpStream::DoRequestCallback(int rv) {
   CHECK_NE(rv, ERR_IO_PENDING);
   CHECK(!request_callback_.is_null());
-
   // Since Run may result in being called back, reset request_callback_ in
   // advance.
   base::ResetAndReturn(&request_callback_).Run(rv);