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 107 matching lines @@
   if (rv == OK) {
     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()) {
+    if (!request_callback_.is_null()) {
+      // Temporary for the review.
+      // Never called!!!

[mmenke, 2016/06/15 16:15:18]

What if we have a chunked upload with no data? - i.e., we just call
upload_stream.AppendData(nullptr, 0, true);, before we start the request?

[maksims (do not use this acc), 2016/06/16 05:12:46]

Nope, check the SendChunkedPostLastEmpty unit test.

First, the eof is hit on the line 467 and then here, but the cb is empty by this
time.

[maksims (do not use this acc), 2016/06/16 05:28:08]

And we know about the eof only after upload_data_stream->Read() has been called.
Thus, we cannot know if there is eof before the actual read.

[maksims (do not use this acc), 2016/06/16 06:52:28]

And I guess the only way to envoke the first eof is by implementing own reader
that sets the UploadDataStream's  bool is_chunked_ to false;

Then is_eof_ will be true on OnInitCompleted.

And the problem with the .AppendData(nullptr, 0, true); is that the ReadChunk
will not be envoked, because there is no buffer. The method just returns without
setting the eof to true. And when the SendRequest is envoked, buffer is created
and passed to the UploadDataStream, which reads the data and then sets eof to
true. That is why the eof is true on line 467 and then here.

+      DCHECK(false);
+      DoRequestCallback(OK);
+    }
     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 all data is sent.
+    if (!request_callback_.is_null())
+      DoRequestCallback(OK);
   } 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