Simplify the the logic for setting the final response status for a

net/quic/chromium/quic_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/quic/chromium/quic_http_stream.h"
 
 #include <utility>
 
 #include "base/auto_reset.h"
 #include "base/callback_helpers.h"
@@ skipping 32 matching lines @@
 }
 
 }  // namespace
 
 QuicHttpStream::QuicHttpStream(
     const base::WeakPtr<QuicChromiumClientSession>& session)
     : MultiplexedHttpStream(MultiplexedSessionHandle(session)),
       next_state_(STATE_NONE),
       session_(session),
       quic_version_(session->GetQuicVersion()),
-      session_error_(OK),
+      session_error_(ERR_UNEXPECTED),
       was_handshake_confirmed_(session->IsCryptoHandshakeConfirmed()),
       stream_(nullptr),
       request_info_(nullptr),
       request_body_stream_(nullptr),
       priority_(MINIMUM_PRIORITY),
       response_info_(nullptr),
-      response_status_(OK),
+      has_response_status_(false),
+      response_status_(ERR_UNEXPECTED),
       response_headers_received_(false),
       headers_bytes_received_(0),
       headers_bytes_sent_(0),
       closed_stream_received_bytes_(0),
       closed_stream_sent_bytes_(0),
       closed_is_first_stream_(false),
       user_buffer_len_(0),
       quic_connection_error_(QUIC_NO_ERROR),
+      quic_stream_error_(QUIC_STREAM_NO_ERROR),
       port_migration_detected_(false),
       found_promise_(false),
       push_handle_(nullptr),
       in_loop_(false),
       weak_factory_(this) {
   DCHECK(session_);
   session_->AddObserver(this);
 }
 
 QuicHttpStream::~QuicHttpStream() {
@@ skipping 72 matching lines @@
   return HttpResponseInfo::CONNECTION_INFO_QUIC_UNKNOWN_VERSION;
 }
 
 int QuicHttpStream::InitializeStream(const HttpRequestInfo* request_info,
                                      RequestPriority priority,
                                      const NetLogWithSource& stream_net_log,
                                      const CompletionCallback& callback) {
   CHECK(callback_.is_null());
   DCHECK(!stream_);
   if (!session_)
-    return was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
-                                    : ERR_QUIC_HANDSHAKE_FAILED;
+    return GetResponseStatus();
 
   stream_net_log.AddEvent(
       NetLogEventType::HTTP_STREAM_REQUEST_BOUND_TO_QUIC_SESSION,
       session_->net_log().source().ToEventParametersCallback());
 
   stream_net_log_ = stream_net_log;
   request_info_ = request_info;
   request_time_ = base::Time::Now();
   priority_ = priority;
 
@@ skipping 70 matching lines @@
   // TODO(rch): remove this once we figure out why channel ID is not being
   // sent when it should be.
   HostPortPair origin = HostPortPair::FromURL(request_info_->url);
   if (origin.Equals(HostPortPair("accounts.google.com", 443)) &&
       request_headers.HasHeader(HttpRequestHeaders::kCookie)) {
     SSLInfo ssl_info;
     GetSSLInfo(&ssl_info);
     UMA_HISTOGRAM_BOOLEAN("Net.QuicSession.CookieSentToAccountsOverChannelId",
                           ssl_info.channel_id_sent);
   }
-  if ((!found_promise_ && !stream_) || !session_) {
-    return was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
-                                    : ERR_QUIC_HANDSHAKE_FAILED;
-  }
+
+  if ((!found_promise_ && !stream_) || !session_)
+    return GetResponseStatus();
 
   // Store the serialized request headers.
   CreateSpdyHeadersFromHttpRequest(*request_info_, request_headers,
                                    /*direct=*/true, &request_headers_);
 
   // Store the request body.
   request_body_stream_ = request_info_->upload_data_stream;
   if (request_body_stream_) {
     // A request with a body is ineligible for push, so reset the
     // promised stream and request a new stream.
@@ skipping 36 matching lines @@
     callback_ = callback;
 
   return rv > 0 ? OK : rv;
 }
 
 int QuicHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
   CHECK(callback_.is_null());
   CHECK(!callback.is_null());
 
   if (stream_ == nullptr)
-    return response_status_;
+    return GetResponseStatus();
 
   // Check if we already have the response headers. If so, return synchronously.
   if (response_headers_received_)
     return OK;
 
   // Still waiting for the response, return IO_PENDING.
   CHECK(callback_.is_null());
   callback_ = callback;
   return ERR_IO_PENDING;
 }
 
 int QuicHttpStream::ReadResponseBody(IOBuffer* buf,
                                      int buf_len,
                                      const CompletionCallback& callback) {
   CHECK(callback_.is_null());
   CHECK(!callback.is_null());
   CHECK(!user_buffer_.get());
   CHECK_EQ(0, user_buffer_len_);
 
   // Invalidate HttpRequestInfo pointer. This is to allow the stream to be
   // shared across multiple transactions which might require this
   // stream to outlive the request_info_'s owner.
   // Only allowed when Read state machine starts. It is safe to reset it at
   // this point since request_info_->upload_data_stream is also not needed
   // anymore.
   request_info_ = nullptr;
 
-  if (!stream_) {
-    // If the stream is already closed, there is no body to read.
-    return response_status_;
-  }
+  // If the stream is already closed, there is no body to read.
+  if (!stream_)
+    return GetResponseStatus();
 
   int rv = ReadAvailableData(buf, buf_len);
   if (rv != ERR_IO_PENDING)
     return rv;
 
   callback_ = callback;
   user_buffer_ = buf;
   user_buffer_len_ = buf_len;
   return ERR_IO_PENDING;
 }
 
-void QuicHttpStream::Close(bool not_reusable) {
-  // Note: the not_reusable flag has no meaning for SPDY streams.
+void QuicHttpStream::Close(bool /*not_reusable*/) {
+  GetResponseStatus();  // Sets response_status_ if not already set.
+  // Note: the not_reusable flag has no meaning for QUIC streams.
   if (stream_) {
     stream_->SetDelegate(nullptr);
     stream_->Reset(QUIC_STREAM_CANCELLED);
-    response_status_ = was_handshake_confirmed_ ? ERR_CONNECTION_CLOSED
-                                                : ERR_QUIC_HANDSHAKE_FAILED;
   }
   ResetStream();
 }
 
 bool QuicHttpStream::IsResponseBodyComplete() const {
   return next_state_ == STATE_OPEN && !stream_;
 }
 
 bool QuicHttpStream::IsConnectionReused() const {
   // TODO(rch): do something smarter here.
@@ skipping 58 matching lines @@
 void QuicHttpStream::OnHeadersAvailable(const SpdyHeaderBlock& headers,
                                         size_t frame_len) {
   headers_bytes_received_ += frame_len;
 
   // QuicHttpStream ignores trailers.
   if (response_headers_received_) {
     if (stream_->IsDoneReading()) {
       // Close the read side. If the write side has been closed, this will
       // invoke QuicHttpStream::OnClose to reset the stream.
       stream_->OnFinRead();
+      SetResponseStatus(OK);
     }
     return;
   }
 
   int rv = ProcessResponseHeaders(headers);
   if (rv != ERR_IO_PENDING && !callback_.is_null()) {
     DoCallback(rv);
   }
 }
 
@@ skipping 11 matching lines @@
     return;
   }
 
   CHECK(!callback_.is_null());
   user_buffer_ = nullptr;
   user_buffer_len_ = 0;
   DoCallback(rv);
 }
 
 void QuicHttpStream::OnClose() {
-  if (stream_->connection_error() != QUIC_NO_ERROR ||
-      stream_->stream_error() != QUIC_STREAM_NO_ERROR) {
-    response_status_ = was_handshake_confirmed_ ? ERR_QUIC_PROTOCOL_ERROR
-                                                : ERR_QUIC_HANDSHAKE_FAILED;
-  } else if (!response_headers_received_) {
-    response_status_ = ERR_ABORTED;
-  }
+  quic_connection_error_ = stream_->connection_error();
+  quic_stream_error_ = stream_->stream_error();
+  GetResponseStatus();  // Sets response_status_ if not already set.
 
-  quic_connection_error_ = stream_->connection_error();
   ResetStream();
-  if (in_loop_) {
-    // If already in DoLoop(), |callback_| will be handled when DoLoop() exits.
+  // If already in DoLoop(), |callback_| will be handled when DoLoop() exits.
+  if (in_loop_)
     return;
-  }
+
   if (!callback_.is_null()) {
-    DoCallback(response_status_);
+    DoCallback(GetResponseStatus());

[Buck, 2017/03/28 21:19:57]

Why not use a cached result from the call above?  If there is a reason, perhaps
explain in a comment.

[Ryan Hamilton, 2017/03/28 21:54:03]

Done. Good idea!

(Well, and then it changed as a result of a different suggestion of yours.  but
still done :>)

   }
 }
 
 void QuicHttpStream::OnError(int error) {
   ResetStream();
-  response_status_ =
-      was_handshake_confirmed_ ? error : ERR_QUIC_HANDSHAKE_FAILED;
+  session_error_ = error;
+  GetResponseStatus();  // Sets response_status_ if not already set.
   if (in_loop_) {
     // If already in DoLoop(), |callback_| will be handled when DoLoop() exits.
     return;
   }
   if (!callback_.is_null())
-    DoCallback(response_status_);
+    DoCallback(GetResponseStatus());
 }
 
 bool QuicHttpStream::HasSendHeadersComplete() {
   return next_state_ > STATE_SEND_HEADERS_COMPLETE;
 }
 
 void QuicHttpStream::OnCryptoHandshakeConfirmed() {
   was_handshake_confirmed_ = true;
   if (next_state_ == STATE_WAIT_FOR_CONFIRMATION_COMPLETE) {
     // Post a task to avoid reentrant calls into the session.
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE, base::Bind(&QuicHttpStream::OnIOComplete,
                               weak_factory_.GetWeakPtr(), OK));
   }
 }
 
 void QuicHttpStream::OnSuccessfulVersionNegotiation(
     const QuicVersion& version) {
   quic_version_ = version;
 }
 
 void QuicHttpStream::OnSessionClosed(int error, bool port_migration_detected) {
-  Close(false);
   session_error_ = error;
   port_migration_detected_ = port_migration_detected;
+  GetResponseStatus();  // Sets response_status_ if not already set.
+
+  Close(false);
   session_.reset();
 }
 
 void QuicHttpStream::OnIOComplete(int rv) {
   rv = DoLoop(rv);
 
   if (rv != ERR_IO_PENDING && !callback_.is_null()) {
     DoCallback(rv);
   }
 }
@@ skipping 77 matching lines @@
 
 int QuicHttpStream::DoRequestStream() {
   next_state_ = STATE_REQUEST_STREAM_COMPLETE;
   return stream_request_.StartRequest(
       session_, &stream_,
       base::Bind(&QuicHttpStream::OnIOComplete, weak_factory_.GetWeakPtr()));
 }
 
 int QuicHttpStream::DoRequestStreamComplete(int rv) {
   DCHECK(rv == OK || !stream_);
-  if (rv != OK)
-    return was_handshake_confirmed_ ? rv : ERR_QUIC_HANDSHAKE_FAILED;
+  if (rv != OK) {
+    session_error_ = rv;
+    return GetResponseStatus();
+  }
 
   stream_->SetDelegate(this);
   if (request_info_->load_flags & LOAD_DISABLE_CONNECTION_MIGRATION) {
     stream_->DisableConnectionMigration();
   }
 
   if (response_info_) {
     // This happens in the case of a asynchronous push rendezvous
     // that ultimately fails (e.g. vary failure).  |response_info_|
     // non-null implies that |DoRequestStream()| was called via
@@ skipping 26 matching lines @@
 int QuicHttpStream::DoWaitForConfirmationComplete(int rv) {
   if (rv < 0)
     return rv;
 
   next_state_ = STATE_SEND_HEADERS;
   return OK;
 }
 
 int QuicHttpStream::DoSendHeaders() {
   if (!stream_)
-    return ERR_UNEXPECTED;
+    return GetResponseStatus();
 
   // Log the actual request with the URL Request's net log.
   stream_net_log_.AddEvent(
       NetLogEventType::HTTP_TRANSACTION_QUIC_SEND_REQUEST_HEADERS,
       base::Bind(&QuicRequestNetLogCallback, stream_->id(), &request_headers_,
                  priority_));
   bool has_upload_data = request_body_stream_ != nullptr;
 
   next_state_ = STATE_SEND_HEADERS_COMPLETE;
   size_t frame_len = stream_->WriteHeaders(std::move(request_headers_),
                                            !has_upload_data, nullptr);
   headers_bytes_sent_ += frame_len;
 
   request_headers_ = SpdyHeaderBlock();
   return static_cast<int>(frame_len);
 }
 
 int QuicHttpStream::DoSendHeadersComplete(int rv) {
   if (rv < 0)
     return rv;
 
   // If the stream is already closed, don't read the request body.
   if (!stream_)
-    return response_status_;
+    return GetResponseStatus();
 
   next_state_ = request_body_stream_ ? STATE_READ_REQUEST_BODY : STATE_OPEN;
 
   return OK;
 }
 
 int QuicHttpStream::DoReadRequestBody() {
   next_state_ = STATE_READ_REQUEST_BODY_COMPLETE;
   return request_body_stream_->Read(
       raw_request_body_buf_.get(), raw_request_body_buf_->size(),
       base::Bind(&QuicHttpStream::OnIOComplete, weak_factory_.GetWeakPtr()));
 }
 
 int QuicHttpStream::DoReadRequestBodyComplete(int rv) {
   // If the stream is already closed, don't continue.
   if (!stream_)
-    return response_status_;
+    return GetResponseStatus();
 
   // |rv| is the result of read from the request body from the last call to
   // DoSendBody().
   if (rv < 0) {
     stream_->SetDelegate(nullptr);
     stream_->Reset(QUIC_ERROR_PROCESSING_STREAM);
     ResetStream();
     return rv;
   }
 
   request_body_buf_ = new DrainableIOBuffer(raw_request_body_buf_.get(), rv);
   if (rv == 0) {  // Reached the end.
     DCHECK(request_body_stream_->IsEOF());
   }
 
   next_state_ = STATE_SEND_BODY;
   return OK;
 }
 
 int QuicHttpStream::DoSendBody() {
   if (!stream_)
-    return ERR_UNEXPECTED;
+    return GetResponseStatus();
 
   CHECK(request_body_stream_);
   CHECK(request_body_buf_.get());
   const bool eof = request_body_stream_->IsEOF();
   int len = request_body_buf_->BytesRemaining();
   if (len > 0 || eof) {
     next_state_ = STATE_SEND_BODY_COMPLETE;
     QuicStringPiece data(request_body_buf_->data(), len);
     return stream_->WriteStreamData(
         data, eof,
         base::Bind(&QuicHttpStream::OnIOComplete, weak_factory_.GetWeakPtr()));
   }
 
   next_state_ = STATE_OPEN;
   return OK;
 }
 
 int QuicHttpStream::DoSendBodyComplete(int rv) {
   if (rv < 0)
     return rv;
 
   // If the stream is already closed, don't continue.
   if (!stream_)
-    return response_status_;
+    return GetResponseStatus();
 
   request_body_buf_->DidConsume(request_body_buf_->BytesRemaining());
 
   if (!request_body_stream_->IsEOF()) {
     next_state_ = STATE_READ_REQUEST_BODY;
     return OK;
   }
 
   next_state_ = STATE_OPEN;
   return OK;
@@ skipping 29 matching lines @@
   // TODO(rtenneti): Temporary fix for crbug.com/585591. Added a check for null
   // |stream_| to fix crash bug. Delete |stream_| check and histogram after fix
   // is merged.
   bool null_stream = stream_ == nullptr;
   UMA_HISTOGRAM_BOOLEAN("Net.QuicReadAvailableData.NullStream", null_stream);
   if (null_stream)
     return rv;
   if (stream_->IsDoneReading()) {
     stream_->SetDelegate(nullptr);
     stream_->OnFinRead();
+    SetResponseStatus(OK);
     ResetStream();
   }
   return rv;
 }
 
 void QuicHttpStream::ResetStream() {
   if (push_handle_) {
     push_handle_->Cancel();
     push_handle_ = nullptr;
   }
   if (!stream_)
     return;
   DCHECK_LE(stream_->sequencer()->NumBytesConsumed(),
             stream_->stream_bytes_read());
   // Only count the uniquely received bytes.
   closed_stream_received_bytes_ = stream_->sequencer()->NumBytesConsumed();
   closed_stream_sent_bytes_ = stream_->stream_bytes_written();
   closed_is_first_stream_ = stream_->IsFirstStream();
   stream_ = nullptr;
 
   // If |request_body_stream_| is non-NULL, Reset it, to abort any in progress
   // read.
   if (request_body_stream_)
     request_body_stream_->Reset();
 }
 
+int QuicHttpStream::GetResponseStatus() {
+  if (!has_response_status_) {
+    SetResponseStatus(ComputeResponseStatus());
+  }
+
+  return response_status_;
+}
+
+void QuicHttpStream::SetResponseStatus(int response_status) {
+  has_response_status_ = true;
+  response_status_ = response_status;
+}
+
+int QuicHttpStream::ComputeResponseStatus() const {
+  DCHECK(!has_response_status_);
+
+  // If the handshake has failed this will be handled by the
+  // QuicStreamFactory and HttpStreamFactory to mark QUIC as broken.
+  if (!was_handshake_confirmed_)
+    return ERR_QUIC_HANDSHAKE_FAILED;
+
+  // If the session was aborted by a higher layer, simply use that error code.
+  if (session_error_ != ERR_UNEXPECTED)
+    return session_error_;
+
+  // For bening errors, return ERR_CONNECTION_CLOSED to permit

[Buck, 2017/03/28 21:19:57]

s/bening/benign/

[Ryan Hamilton, 2017/03/28 21:54:03]

Done.

+  // HttpNetworkTransaction to retry the request (assuming that
+  // headers have not been received, and this is not the firs stream).

[Buck, 2017/03/28 21:19:57]

s/firs/first/

[Ryan Hamilton, 2017/03/28 21:54:03]

Done.

+  if ((quic_connection_error_ == QUIC_CONNECTION_IP_POOLED ||
+       quic_connection_error_ == QUIC_NETWORK_IDLE_TIMEOUT ||
+       quic_connection_error_ == QUIC_NO_ERROR) &&
+      quic_stream_error_ == QUIC_STREAM_NO_ERROR) {
+    return ERR_CONNECTION_CLOSED;
+  }
+
+  return ERR_QUIC_PROTOCOL_ERROR;
+}
+
 }  // namespace net