[HttpStreamParser]: Flush buffers as soon as request is sent.

net/http/http_stream_parser.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/http/http_stream_parser.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
@@ skipping 456 matching lines @@
   io_state_ = STATE_SEND_HEADERS_COMPLETE;
   return connection_->socket()
       ->Write(request_headers_.get(), bytes_remaining, io_callback_);
 }
 
 int HttpStreamParser::DoSendHeadersComplete(int result) {
   if (result < 0) {
     // In the unlikely case that the headers and body were merged, all the
     // the headers were sent, but not all of the body way, and |result| is
     // an error that this should try reading after, stash the error for now and
     // act like the request was successfully sent.

[mmenke, 2016/07/11 15:15:35]

I'd suggest adding a new state, DO_SEND_REQUEST_COMPLETE, and have it be the
only method to call FlushBuffers().

Also, I'd add DCHECKs in DoLoop, after all the other DoSend cases, to make sure
io_state_ is not NONE, to make sure we always go through the new state.  I'd
also add DCHECKs in HttpStreamParser::Read, to make sure the relevant fields are
empty.  Sound good?

[mmenke, 2016/07/11 15:15:35]

Missed a spot.

[maksims (do not use this acc), 2016/07/12 10:49:25]

Do you like it now?

     if (request_headers_->BytesConsumed() >= request_headers_length_ &&
         ShouldTryReadingOnUploadError(result)) {
       upload_error_ = result;
       return OK;
     }
     return result;
   }
 
   sent_bytes_ += result;
   request_headers_->DidConsume(result);
@@ skipping 11 matching lines @@
         NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_BODY,
         base::Bind(&NetLogSendRequestBodyCallback,
                    request_->upload_data_stream->size(),
                    request_->upload_data_stream->is_chunked(),
                    false /* not merged */));
     io_state_ = STATE_SEND_BODY;
     return OK;
   }
 
   // Finished sending the request.
+  FlushBuffers();
   return OK;
 }
 
 int HttpStreamParser::DoSendBody() {
   if (request_body_send_buf_->BytesRemaining() > 0) {
     io_state_ = STATE_SEND_BODY_COMPLETE;
     return connection_->socket()
         ->Write(request_body_send_buf_.get(),
                 request_body_send_buf_->BytesRemaining(),
                 io_callback_);
   }
 
   if (request_->upload_data_stream->is_chunked() && sent_last_chunk_) {
     // Finished sending the request.
+    FlushBuffers();
     return OK;
   }
 
   request_body_read_buf_->Clear();
   io_state_ = STATE_SEND_REQUEST_READ_BODY_COMPLETE;
   return request_->upload_data_stream->Read(request_body_read_buf_.get(),
                                             request_body_read_buf_->capacity(),
                                             io_callback_);
 }
 
@@ skipping 11 matching lines @@
   sent_bytes_ += result;
   request_body_send_buf_->DidConsume(result);
 
   io_state_ = STATE_SEND_BODY;
   return OK;
 }
 
 int HttpStreamParser::DoSendRequestReadBodyComplete(int result) {
   // |result| is the result of read from the request body from the last call to
   // DoSendBody().
-  if (result < 0)
+  if (result < 0) {
+    FlushBuffers();
     return result;
+  }
 
   // Chunked data needs to be encoded.
   if (request_->upload_data_stream->is_chunked()) {
     if (result == 0) {  // Reached the end.
       DCHECK(request_->upload_data_stream->IsEOF());
       sent_last_chunk_ = true;
     }
     // Encode the buffer as 1 chunk.
     const base::StringPiece payload(request_body_read_buf_->data(), result);
     request_body_send_buf_->Clear();
     result = EncodeChunk(payload,
                          request_body_send_buf_->data(),
                          request_body_send_buf_->capacity());
   }
 
   if (result == 0) {  // Reached the end.
     // Reaching EOF means we can finish sending request body unless the data is
     // chunked. (i.e. No need to send the terminal chunk.)
     DCHECK(request_->upload_data_stream->IsEOF());
     DCHECK(!request_->upload_data_stream->is_chunked());
     // Finished sending the request.
+    FlushBuffers();
   } else if (result > 0) {
     request_body_send_buf_->DidAppend(result);
     result = 0;
     io_state_ = STATE_SEND_BODY;
   }
   return result;
 }
 
 int HttpStreamParser::DoReadHeaders() {
   io_state_ = STATE_READ_HEADERS_COMPLETE;
@@ skipping 594 matching lines @@
   return false;
 }
 
 void HttpStreamParser::ValidateStatusLine(const std::string& status_line) {
   HttpStatusLineValidator::StatusLineStatus status =
       HttpStatusLineValidator::ValidateStatusLine(status_line);
   UMA_HISTOGRAM_ENUMERATION("Net.HttpStatusLineStatus", status,
                             HttpStatusLineValidator::STATUS_LINE_MAX);
 }
 
+void HttpStreamParser::FlushBuffers() {
+  if (request_headers_)

[mmenke, 2016/07/11 15:15:35]

None of these ifs are necessary - refptrs have identical null checks when you
clear them, and it's better to avoid unnecessary code (Binary size matters most
for performance, 99 times out of 99.01).

+    request_headers_ = nullptr;
+  if (request_body_send_buf_)
+    request_body_send_buf_ = nullptr;
+  if (request_body_read_buf_)
+    request_body_read_buf_ = nullptr;
+}
+
 }  // namespace net