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..776c437ab23a131e14791c85bb2349c352c45b25 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -302,13 +302,14 @@ void SpdyHttpStream::Cancel() {
 }
 
 void SpdyHttpStream::OnRequestHeadersSent() {
-  if (!request_callback_.is_null())
-    DoRequestCallback(OK);
-
   // TODO(akalin): Do this immediately after sending the request

[Bence, 2016/06/13 13:53:36]

I do not exactly understand this TODO, but my impression is that it was either
already moot or you are resolving it with this change.  Therefore I suggest you
remove this TODO.

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

Done.

   // headers.
-  if (HasUploadData())
+  if (HasUploadData()) {
     ReadAndSendRequestBodyData();
+  } else {
+    if (!request_callback_.is_null())
+      DoRequestCallback(OK);
+  }
 }
 
 SpdyResponseHeadersStatus SpdyHttpStream::OnResponseHeadersUpdated(
@@ -467,6 +468,9 @@ void SpdyHttpStream::OnRequestBodyReadCompleted(int status) {
   stream_->SendData(request_body_buf_.get(),
                     request_body_buf_size_,
                     eof ? NO_MORE_DATA_TO_SEND : MORE_DATA_TO_SEND);
+
+  if (!request_callback_.is_null())
+    DoRequestCallback(OK);

[mmenke, 2016/06/13 14:31:06]

BUG:  Shouldn't we only do this if eof is true?

[mmenke, 2016/06/13 14:48:27]

Actually, that's not enough.  ReadAndSendRequestBodyData should also do it, if
IsEOF returns true....  May be safest to just call this method directly with
net::OK, in that case.

We should also have a tests where we have an UploadDataStream, and we first
discover upload_data_stream->IsEOF returns true here and in the first call to
ReadAndSendRequestBodyData.  (I assume we already have the first test, but not
the second)

[maksims (do not use this acc), 2016/06/14 08:35:03]

Oh,oh, my mistake. I haven't noticed that. I checked the implementation of QUIC,
and it seems like it returns OK only after the whole operation is done.
What method????? Actually, when eof is met here, then we immideately face the
eof in ReadAndSendRequestBodyData().. Maybe because of the posttasks or drains.
But, anyway, It is always so and the req_cb is already null if I put it in the
ReadAndSendRequestBodyData() method.
The second? Which one? Didn't get it.

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

This one....  If request_info_->upload_data_stream->IsEOF() returns true in
ReadAndSendRequestBodyData(), we could just call this one with a value of 0.
Test 1) Check that things work when we invoke DoRequestCallback(OK) here.  We
probably already have a such a test, but best to make sure (Can just put a CHECK
when we invoke the callback here and run tests, and see which one triggers it).
Test 2) Check that things work when we invoke DoRequestCallback(OK) in
SpdyHttpStream::ReadAndSendRequestBodyData.

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

As I said if I call OnRequestBodyReadCompleted from the place you mentioned,
then the write fails. Thus, I just call the callback. I might have understand
you wrong.
Yes, more than a half of the tests call this Callback.

 }
 
 void SpdyHttpStream::ScheduleBufferedReadCallback() {