Basic HTTP pipelining support

net/http/http_network_transaction.cc

 // Copyright (c) 2011 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_network_transaction.h"
 
 #include <set>
 #include <vector>
 
 #include "base/compiler_specific.h"
@@ skipping 21 matching lines @@
 #include "net/http/http_auth_handler.h"
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_chunked_decoder.h"
 #include "net/http/http_net_log_params.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
-#include "net/http/http_response_body_drainer.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_stream_factory.h"
 #include "net/http/http_util.h"
 #include "net/http/url_security_manager.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/socks_client_socket_pool.h"
 #include "net/socket/ssl_client_socket.h"
 #include "net/socket/ssl_client_socket_pool.h"
 #include "net/socket/transport_client_socket_pool.h"
@@ skipping 75 matching lines @@
       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_->IsSpdyHttpStream()) {
         // Doesn't really matter for SpdyHttpStream. Just close it.
         stream_->Close(true /* not reusable */);
       } else {
         // Otherwise, we try to drain the response body.
-        // TODO(willchan): Consider moving this response body draining to the
-        // stream implementation.  For SPDY, there's clearly no point.  For
-        // HTTP, it can vary depending on whether or not we're pipelining.  It's
-        // stream dependent, so the different subtypes should be implementing
-        // their solutions.
-        HttpResponseBodyDrainer* drainer =
-          new HttpResponseBodyDrainer(stream_.release());
-        drainer->Start(session_);
-        // |drainer| will delete itself.
+        HttpStream* stream = stream_.release();
+        stream->Drain(session_);
       }
     }
   }
 }
 
 int HttpNetworkTransaction::Start(const HttpRequestInfo* request_info,
                                   CompletionCallback* callback,
                                   const BoundNetLog& net_log) {
   SIMPLE_STATS_COUNTER("HttpNetworkTransaction.Count");
 
@@ skipping 383 matching lines @@
         net_log_.EndEventWithNetErrorCode(
             NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST, rv);
         break;
       case STATE_READ_HEADERS:
         DCHECK_EQ(OK, rv);
         net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_READ_HEADERS, NULL);
         rv = DoReadHeaders();
         break;
       case STATE_READ_HEADERS_COMPLETE:
         rv = DoReadHeadersComplete(rv);
-        net_log_.EndEventWithNetErrorCode(
-            NetLog::TYPE_HTTP_TRANSACTION_READ_HEADERS, rv);

[willchan no longer on Chromium, 2011/09/29 19:32:48]

You should keep this back where it was.

[James Simonsen, 2011/10/12 01:36:58]

Didn't mean to delete it. Forgot to save before uploading.

         break;
       case STATE_READ_BODY:
         DCHECK_EQ(OK, rv);
         net_log_.BeginEvent(NetLog::TYPE_HTTP_TRANSACTION_READ_BODY, NULL);
         rv = DoReadBody();
         break;
       case STATE_READ_BODY_COMPLETE:
         rv = DoReadBodyComplete(rv);
         net_log_.EndEventWithNetErrorCode(
             NetLog::TYPE_HTTP_TRANSACTION_READ_BODY, rv);
@@ skipping 303 matching lines @@
                            *response_.headers,
                            endpoint);
 
   int rv = HandleAuthChallenge();
   if (rv != OK)
     return rv;
 
   if (is_https_request())
     stream_->GetSSLInfo(&response_.ssl_info);
 
+  if (read_buf_len_) {
+    next_state_ = STATE_READ_BODY;

[willchan no longer on Chromium, 2011/09/29 19:32:48]

I believe this is a bug. We should only transition to this state when Read() is
called.

[James Simonsen, 2011/10/12 01:36:58]

It's a tricky situation. Maybe I should just forget about it.

If we were to be evicted after Read() was called, then after restarting, we
shouldn't wait for Read() to be called again. We know Read() has been called if
we have a read_buf_len_.

Anyway, this shouldn't happen in practice, so I just removed it. I'd only seen
it while playing with the unit test.

[willchan no longer on Chromium, 2011/10/13 02:29:16]

A new test? Because this shouldn't happen, since no one is allowed to call
Read() until the response headers have been returned. If someone does this, they
are broken.

+  }
+
   headers_valid_ = true;
   return OK;
 }
 
 int HttpNetworkTransaction::DoReadBody() {
   DCHECK(read_buf_);
   DCHECK_GT(read_buf_len_, 0);
   DCHECK(stream_ != NULL);
 
   next_state_ = STATE_READ_BODY_COMPLETE;
@@ skipping 294 matching lines @@
   // SSL errors may happen at any time during the stream and indicate issues
   // with the underlying connection. Because the peer may request
   // renegotiation at any time, check and handle any possible SSL handshake
   // related errors. In addition to renegotiation, TLS False/Snap Start may
   // cause SSL handshake errors to be delayed until the first or second Write
   // (Snap Start) or the first Read (False & Snap Start) on the underlying
   // connection.
   error = HandleSSLHandshakeError(error);
 
   switch (error) {
+    case ERR_PIPELINE_EVICTION:

[willchan no longer on Chromium, 2011/09/29 19:32:48]

I think here you should log to net_log_. You should probably create a new event
for this. I think this event should also be logged below, above the call to 
ResetConnectionAndRequestForResend();

[James Simonsen, 2011/10/12 01:36:58]

Done.

+      error = RestartAfterPipelineEviction();
+      break;
+
     // If we try to reuse a connection that the server is in the process of
     // closing, we may end up successfully writing out our request (or a
     // portion of our request) only to find a connection error when we try to
     // read from (or finish writing to) the socket.
     case ERR_CONNECTION_RESET:
     case ERR_CONNECTION_CLOSED:
     case ERR_CONNECTION_ABORTED:
       if (ShouldResendRequest(error)) {
         ResetConnectionAndRequestForResend();
         error = OK;
       }
       break;
   }
   return error;
 }
 
+int HttpNetworkTransaction::RestartAfterPipelineEviction() {
+  stream_->Close(true);
+  stream_.reset();
+  stream_request_.reset();
+  next_state_ = STATE_CREATE_STREAM;
+  return OK;
+}
+
 void HttpNetworkTransaction::ResetStateForRestart() {
   ResetStateForAuthRestart();
   stream_.reset();
 }
 
 void HttpNetworkTransaction::ResetStateForAuthRestart() {
   pending_auth_target_ = HttpAuth::AUTH_NONE;
   read_buf_ = NULL;
   read_buf_len_ = 0;
   headers_valid_ = false;
@@ skipping 119 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net