Slow start pipelining.

net/http/http_pipelined_connection_impl.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_pipelined_connection_impl.h"
 
 #include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "net/base/io_buffer.h"
 #include "net/http/http_pipelined_stream.h"
 #include "net/http/http_request_info.h"
+#include "net/http/http_response_headers.h"
 #include "net/http/http_stream_parser.h"
+#include "net/http/http_version.h"
 #include "net/socket/client_socket_handle.h"
 
 namespace net {
 
 HttpPipelinedConnectionImpl::HttpPipelinedConnectionImpl(
     ClientSocketHandle* connection,
     HttpPipelinedConnection::Delegate* delegate,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     const BoundNetLog& net_log,
@@ skipping 135 matching lines @@
       case SEND_STATE_SEND_ACTIVE_REQUEST:
         rv = DoSendActiveRequest(rv);
         break;
       case SEND_STATE_COMPLETE:
         rv = DoSendComplete(rv);
         break;
       case SEND_STATE_EVICT_PENDING_REQUESTS:
         rv = DoEvictPendingSendRequests(rv);
         break;
       default:
-        NOTREACHED() << "bad send state: " << state;
+        CHECK(false) << "bad send state: " << state;
         rv = ERR_FAILED;
         break;
     }
   } while (rv != ERR_IO_PENDING && send_next_state_ != SEND_STATE_NONE);
   send_still_on_call_stack_ = false;
   return rv;
 }
 
 void HttpPipelinedConnectionImpl::OnSendIOCallback(int result) {
   CHECK(active_send_request_.get());
@@ skipping 149 matching lines @@
         return rv;
       case READ_STATE_STREAM_CLOSED:
         rv = DoReadStreamClosed();
         break;
       case READ_STATE_EVICT_PENDING_READS:
         rv = DoEvictPendingReadHeaders(rv);
         break;
       case READ_STATE_NONE:
         break;
       default:
-        NOTREACHED() << "bad read state";
+        CHECK(false) << "bad read state";
         rv = ERR_FAILED;
         break;
     }
   } while (rv != ERR_IO_PENDING && read_next_state_ != READ_STATE_NONE);
   read_still_on_call_stack_ = false;
   return rv;
 }
 
 void HttpPipelinedConnectionImpl::OnReadIOCallback(int result) {
   DoReadHeadersLoop(result);
@@ skipping 36 matching lines @@
       // Since nobody will read whatever data is on the pipeline associated with
       // this closed request, we must shut down the rest of the pipeline.
       read_next_state_ = READ_STATE_EVICT_PENDING_READS;
       break;
 
     case STREAM_SENT:
       read_next_state_ = READ_STATE_NONE;
       break;
 
     default:
-      NOTREACHED() << "Unexpected read state: "
+      CHECK(false) << "Unexpected read state: "
                    << stream_info_map_[next_id].state;
   }
 
   return OK;
 }
 
 int HttpPipelinedConnectionImpl::DoReadHeaders(int result) {
   CHECK(active_read_id_);
   CHECK(ContainsKey(stream_info_map_, active_read_id_));
   CHECK_EQ(STREAM_READ_PENDING, stream_info_map_[active_read_id_].state);
@@ skipping 10 matching lines @@
   CHECK_EQ(STREAM_ACTIVE, stream_info_map_[active_read_id_].state);
 
   read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
   if (result < OK) {
     if (result == ERR_SOCKET_NOT_CONNECTED && completed_one_request_) {
       result = ERR_PIPELINE_EVICTION;
     }
     usable_ = false;
   }
 
+  CheckHeadersForPipelineCompatibility(result, active_read_id_);
+
   if (!read_still_on_call_stack_) {
     QueueUserCallback(active_read_id_,
                       stream_info_map_[active_read_id_].read_headers_callback,
                       result, FROM_HERE);
   }
 
   return result;
 }
 
 int HttpPipelinedConnectionImpl::DoReadWaitForClose(int result) {
   read_next_state_ = READ_STATE_WAITING_FOR_CLOSE;
   return result;
 }
 
 int HttpPipelinedConnectionImpl::DoReadStreamClosed() {
   CHECK(active_read_id_);
   CHECK(ContainsKey(stream_info_map_, active_read_id_));
   CHECK_EQ(stream_info_map_[active_read_id_].state, STREAM_CLOSED);
   active_read_id_ = 0;
   if (!usable_) {
+    // TODO(simonjam): Don't wait this long to evict.
     read_next_state_ = READ_STATE_EVICT_PENDING_READS;
     return OK;
   }
   completed_one_request_ = true;
   MessageLoop::current()->PostTask(
       FROM_HERE,
       method_factory_.NewRunnableMethod(
           &HttpPipelinedConnectionImpl::StartNextDeferredRead));
   read_next_state_ = READ_STATE_NONE;
   return OK;
@@ skipping 62 matching lines @@
     case STREAM_READ_EVICTED:
       stream_info_map_[pipeline_id].state = STREAM_CLOSED;
       break;
 
     case STREAM_CLOSED:
     case STREAM_UNUSED:
       // TODO(simonjam): Why is Close() sometimes called twice?
       break;
 
     default:
-      NOTREACHED();
+      CHECK(false);
       break;
   }
 }
 
 int HttpPipelinedConnectionImpl::ReadResponseBody(
     int pipeline_id,
     IOBuffer* buf,
     int buf_len,
     OldCompletionCallback* callback) {
   CHECK(ContainsKey(stream_info_map_, pipeline_id));
@@ skipping 60 matching lines @@
 
 void HttpPipelinedConnectionImpl::GetSSLCertRequestInfo(
     int pipeline_id,
     SSLCertRequestInfo* cert_request_info) {
   CHECK(ContainsKey(stream_info_map_, pipeline_id));
   CHECK(stream_info_map_[pipeline_id].parser.get());
   return stream_info_map_[pipeline_id].parser->GetSSLCertRequestInfo(
       cert_request_info);
 }
 
+void HttpPipelinedConnectionImpl::CheckHeadersForPipelineCompatibility(
+    int result,
+    int pipeline_id) {
+  if (result < OK) {

[mmenke, 2011/11/21 14:51:39]

When things go wrong, we won't necessarily get the error, if any, when receiving
headers.  I think we're going to have to play with things in the wild to see if
excluding certain error codes like this works at all.

[James Simonsen, 2011/12/01 01:17:10]

Okay, I've added a TODO. Eventually, I want to circle back and start
instrumenting all of this code so we get some metrics and can start tuning.

+    switch (result) {
+      case ERR_ABORTED:
+      case ERR_INTERNET_DISCONNECTED:
+        // These errors are no fault of the server.
+        break;
+
+      default:
+        delegate_->OnPipelineFeedback(this, SOCKET_ERROR);
+        return;
+    }
+  }
+  HttpResponseInfo* info = GetResponseInfo(pipeline_id);
+  const HttpVersion required_version(1, 1);
+  if (info->headers->GetParsedHttpVersion() < required_version) {
+    delegate_->OnPipelineFeedback(this, OLD_HTTP_VERSION);
+    return;
+  }
+  if (!info->headers->IsKeepAlive() || !CanFindEndOfResponse(pipeline_id)) {
+    usable_ = false;
+    delegate_->OnPipelineFeedback(this, MUST_CLOSE_CONNECTION);
+    return;
+  }

[mmenke, 2011/11/21 14:51:39]

We should also be doing something about NTLM AUTH requests.  Currently, simplest
thing to do is to disallow pipelining and force the connection to close, in that
case.  Feel free to add it as a TODO in this CL.

[James Simonsen, 2011/12/01 01:17:10]

Yeah, that was next on my list. I'm going to do it in a separate CL to directly
address the bug report.

+  delegate_->OnPipelineFeedback(this, OK);
+}
+
 void HttpPipelinedConnectionImpl::QueueUserCallback(
     int pipeline_id,
     OldCompletionCallback* callback,
     int rv,
     const tracked_objects::Location& from_here) {
   CHECK(!stream_info_map_[pipeline_id].pending_user_callback);
   stream_info_map_[pipeline_id].pending_user_callback = callback;
   MessageLoop::current()->PostTask(
       from_here,
       method_factory_.NewRunnableMethod(
@@ skipping 50 matching lines @@
 HttpPipelinedConnectionImpl::StreamInfo::StreamInfo()
     : read_headers_callback(NULL),
       pending_user_callback(NULL),
       state(STREAM_CREATED) {
 }
 
 HttpPipelinedConnectionImpl::StreamInfo::~StreamInfo() {
 }
 
 }  // namespace net