If we read nothing (EOF) after sending a request on a

persistent connection, it also indicates that the server
closed the connection before receiving the request, and we
should resend the request on a new connection.

I refactored the main code of HandleIOError into a new
ShouldResendRequest method so that I can reuse it to handle
the read-nothing case.

R=darin
BUG=2489
Review URL: http://codereview.chromium.org/4264

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@2613 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 34 insertions, 11 deletions

diff --git a/net/base/ssl_client_socket.cc b/net/base/ssl_client_socket.cc
index 0449eb9..3229c86 100644
--- a/net/base/ssl_client_socket.cc
+++ b/net/base/ssl_client_socket.cc
@@ -159,6 +159,12 @@ void SSLClientSocket::Disconnect() {
 }
 
 bool SSLClientSocket::IsConnected() const {
+  // Ideally, we should also check if we have received the close_notify alert
+  // message from the server, and return false in that case.  We're not doing
+  // that, so this function may return a false positive.  Since the upper
+  // layer (HttpNetworkTransaction) needs to handle a persistent connection
+  // closed by the server when we send a request anyway, a false positive in
+  // exchange for simpler code is a good trade-off.
   return completed_handshake_ && transport_->IsConnected();
 }
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 3e809c5..ea0f3d7 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -576,6 +576,9 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
   if (result < 0)
     return HandleIOError(result);
 
+  if (result == 0 && ShouldResendRequest())
+    return result;
+
   // Record our best estimate of the 'response time' as the time when we read
   // the first bytes of the response headers.
   if (header_buf_len_ == 0)
@@ -848,21 +851,30 @@ int HttpNetworkTransaction::HandleIOError(int error) {
     case ERR_CONNECTION_RESET:
     case ERR_CONNECTION_CLOSED:
     case ERR_CONNECTION_ABORTED:
-      if (!establishing_tunnel_ &&
-          reused_socket_ &&    // We reused a keep-alive connection.
-          !header_buf_len_) {  // We haven't received any response header yet.
-        connection_.set_socket(NULL);
-        connection_.Reset();
-        request_headers_bytes_sent_ = 0;
-        if (request_body_stream_.get())
-          request_body_stream_->Reset();
-        next_state_ = STATE_INIT_CONNECTION;  // Resend the request.
+      if (ShouldResendRequest())
         error = OK;
-      }
       break;
   }
   return error;
 }
 
+bool HttpNetworkTransaction::ShouldResendRequest() {
+  // NOTE: we resend a request only if we reused a keep-alive connection.
+  // This automatically prevents an infinite resend loop because we'll run
+  // out of the cached keep-alive connections eventually.
+  if (establishing_tunnel_ ||
+      !reused_socket_ ||  // We didn't reuse a keep-alive connection.
+      header_buf_len_) {  // We have received some response headers.
+    return false;
+  }
+  connection_.set_socket(NULL);
+  connection_.Reset();
+  request_headers_bytes_sent_ = 0;
+  if (request_body_stream_.get())
+    request_body_stream_->Reset();
+  next_state_ = STATE_INIT_CONNECTION;  // Resend the request.
+  return true;
+}
+
 }  // namespace net
 
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index 3fb2285..5334e43 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -87,9 +87,14 @@ class HttpNetworkTransaction : public HttpTransaction {
   // is returned.
   int HandleIOError(int error);
 
+  // Called when we reached EOF or got an error.  If we should resend the
+  // request, sets next_state_ and returns true.  Otherwise, does nothing and
+  // returns false.
+  bool ShouldResendRequest();
+
   // Return true if based on the bytes read so far, the start of the
   // status line is known. This is used to distingish between HTTP/0.9
-  // responses (which have no status line).
+  // responses (which have no status line) and HTTP/1.x responses.
   bool has_found_status_line_start() const {
     return header_buf_http_offset_ != -1;
   }