Move logic to figure out if a socket can be reused into HttpStream.

net/http/http_network_transaction.cc

Index: net/http/http_network_transaction.cc
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 0733fdf4187a309b11cfb5eed5eb54228b856c00..29967723d4b1a1b7a6eec19cedb8044247672746 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -159,28 +159,18 @@ HttpNetworkTransaction::HttpNetworkTransaction(RequestPriority priority,
 
 HttpNetworkTransaction::~HttpNetworkTransaction() {
   if (stream_.get()) {
-    HttpResponseHeaders* headers = GetResponseHeaders();
     // TODO(mbelshe): The stream_ should be able to compute whether or not the
     //                stream should be kept alive.  No reason to compute here
     //                and pass it in.
-    bool try_to_keep_alive =
-        next_state_ == STATE_NONE &&
-        stream_->CanFindEndOfResponse() &&
-        (!headers || headers->IsKeepAlive());
-    if (!try_to_keep_alive) {
+    if (!stream_->CanReuseConnection() || next_state_ != STATE_NONE) {
       stream_->Close(true /* not reusable */);
+    } else if (stream_->IsResponseBodyComplete()) {
+      // If the response body is complete, we can just reuse the socket.
+      stream_->Close(false /* reusable */);
     } else {
-      if (stream_->IsResponseBodyComplete()) {
-        // If the response body is complete, we can just reuse the socket.
-        stream_->Close(false /* reusable */);
-      } else if (stream_->IsSpdyHttpStream()) {

[mmenke, 2015/09/01 18:59:54]

Checking for SPDY and not QUIC here is what inspired this CL - turns out the
stream->Drain does the right thing, anyways, but it's weird to have only one
check here.

-        // Doesn't really matter for SpdyHttpStream. Just close it.
-        stream_->Close(true /* not reusable */);
-      } else {
-        // Otherwise, we try to drain the response body.
-        HttpStream* stream = stream_.release();
-        stream->Drain(session_);
-      }
+      // Otherwise, we try to drain the response body.
+      HttpStream* stream = stream_.release();
+      stream->Drain(session_);
     }
   }
 
@@ -293,8 +283,7 @@ void HttpNetworkTransaction::PrepareForAuthRestart(HttpAuth::Target target) {
   bool keep_alive = false;
   // Even if the server says the connection is keep-alive, we have to be
   // able to find the end of each response in order to reuse the connection.
-  if (GetResponseHeaders()->IsKeepAlive() &&
-      stream_->CanFindEndOfResponse()) {
+  if (stream_->CanReuseConnection()) {
     // If the response body hasn't been completely read, we need to drain
     // it first.
     if (!stream_->IsResponseBodyComplete()) {
@@ -317,7 +306,7 @@ void HttpNetworkTransaction::DidDrainBodyForAuthRestart(bool keep_alive) {
   if (stream_.get()) {
     total_received_bytes_ += stream_->GetTotalReceivedBytes();
     HttpStream* new_stream = NULL;
-    if (keep_alive && stream_->IsConnectionReusable()) {
+    if (keep_alive && stream_->CanReuseConnection()) {

[mmenke, 2015/09/01 18:59:54]

This checks a bit more than necessary, but the redundant checks don't hurt.

I'd really like to turn this into something like:

int result = stream_->CreateNewStreamForAuthRestart(callback);

And make the BasicStream do the draining and such (SPDY/QUIC just return
ERR_NEED_NEW_STREAM synchronously or something), but I'll leave that for a
future cleanup - maybe next fixit.

[Ryan Hamilton, 2015/09/02 04:42:40]

Hear! Hear! That would be awesome.

       // We should call connection_->set_idle_time(), but this doesn't occur
       // often enough to be worth the trouble.
       stream_->SetConnectionReused();
@@ -1115,26 +1104,18 @@ int HttpNetworkTransaction::DoReadBodyComplete(int result) {
     done = true;
   }
 
-  bool keep_alive = false;
-  if (stream_->IsResponseBodyComplete()) {
+  // Clean up connection if we are done.
+  if (done) {
     // Note: Just because IsResponseBodyComplete is true, we're not
     // necessarily "done".  We're only "done" when it is the last
     // read on this HttpNetworkTransaction, which will be signified
     // by a zero-length read.
-    // TODO(mbelshe): The keepalive property is really a property of
+    // TODO(mbelshe): The keep-alive property is really a property of
     //    the stream.  No need to compute it here just to pass back
     //    to the stream's Close function.
-    // TODO(rtenneti): CanFindEndOfResponse should return false if there are no
-    // ResponseHeaders.
-    if (stream_->CanFindEndOfResponse()) {
-      HttpResponseHeaders* headers = GetResponseHeaders();
-      if (headers)
-        keep_alive = headers->IsKeepAlive();
-    }
-  }
+    bool keep_alive =
+        stream_->IsResponseBodyComplete() && stream_->CanReuseConnection();
 
-  // Clean up connection if we are done.
-  if (done) {
     stream_->Close(!keep_alive);
     // Note: we don't reset the stream here.  We've closed it, but we still
     // need it around so that callers can call methods such as