Change SPDY to call request_callback after data is sent.

net/spdy/spdy_http_stream.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_http_stream.h"
 
 #include <algorithm>
 #include <list>
 #include <utility>
 
@@ skipping 284 matching lines @@
 void SpdyHttpStream::Cancel() {
   request_callback_.Reset();
   response_callback_.Reset();
   if (stream_.get()) {
     stream_->Cancel();
     DCHECK(!stream_.get());
   }
 }
 
 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(
     const SpdyHeaderBlock& response_headers) {
   CHECK_EQ(response_headers_status_, RESPONSE_HEADERS_ARE_INCOMPLETE);
 
   if (!response_info_) {
     DCHECK_EQ(stream_->type(), SPDY_PUSH_STREAM);
     push_response_info_.reset(new HttpResponseInfo);
     response_info_ = push_response_info_.get();
@@ skipping 108 matching lines @@
     stream_ = stream_request_.ReleaseStream();
     stream_->SetDelegate(this);
   }
   callback.Run(rv);
 }
 
 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
       ->Read(request_body_buf_.get(),
              request_body_buf_->size(),
              base::Bind(&SpdyHttpStream::OnRequestBodyReadCompleted,
                         weak_factory_.GetWeakPtr()));
 
   if (rv != ERR_IO_PENDING) {
     // ERR_IO_PENDING is the only possible error.
     CHECK_GE(rv, 0);
     OnRequestBodyReadCompleted(rv);
   }
 }
 
 void SpdyHttpStream::OnRequestBodyReadCompleted(int status) {
   CHECK_GE(status, 0);
   request_body_buf_size_ = status;
   const bool eof = request_info_->upload_data_stream->IsEOF();
   // 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);
   }
   stream_->SendData(request_body_buf_.get(),
                     request_body_buf_size_,
                     eof ? NO_MORE_DATA_TO_SEND : MORE_DATA_TO_SEND);
 }
 
 void SpdyHttpStream::ScheduleBufferedReadCallback() {
   // If there is already a scheduled DoBufferedReadCallback, don't issue
@@ skipping 52 matching lines @@
     CHECK_NE(rv, ERR_IO_PENDING);
     user_buffer_ = NULL;
     user_buffer_len_ = 0;
     DoResponseCallback(rv);
   }
 }
 
 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);
 }
 
 void SpdyHttpStream::DoResponseCallback(int rv) {
   CHECK_NE(rv, ERR_IO_PENDING);
   CHECK(!response_callback_.is_null());
 
   // Since Run may result in being called back, reset response_callback_ in
@@ skipping 37 matching lines @@
   details->connection_info = HttpResponseInfo::CONNECTION_INFO_HTTP2;
   return;
 }
 
 void SpdyHttpStream::SetPriority(RequestPriority priority) {
   // TODO(akalin): Plumb this through to |stream_request_| and
   // |stream_|.
 }
 
 }  // namespace net