Slow start pipelining.

We need to wait for an HTTP/1.1 keep-alive response before we try to pipeline. Notably, this fixes wordpress.com and techcrunch.com.

Remember which hosts clearly support, or don't support pipelining. If pipelining is supported, skip the slow start. If it's not, fall back to HttpBasicStreams.

A site is judged not to support pipelining if we see an old HTTP version or encounter a socket error. A site does support pipelining if it successfully handles 3 requests. There's obviously room for improvement here, but this is a
start.

Related changes:
- In the spirit of CHECK() failing. Use CHECK(false) instead of NOTREACHED().
- HttpPipelinedHost is now an interface with a corresponding Impl. This is to help unit test HttpPipelinedHostPool.

BUG=None
TEST=net_unittests


Review URL: http://codereview.chromium.org/8586015

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

1 files changed, 67 insertions, 11 deletions

diff --git a/net/http/http_pipelined_host_pool.cc b/net/http/http_pipelined_host_pool.cc
index bd238d8..fa705a0 100644
--- a/net/http/http_pipelined_host_pool.cc
+++ b/net/http/http_pipelined_host_pool.cc
@@ -6,19 +6,46 @@
 
 #include "base/logging.h"
 #include "base/stl_util.h"
-#include "net/http/http_pipelined_host.h"
-#include "net/http/http_stream_factory_impl.h"
+#include "net/http/http_pipelined_host_impl.h"
 
 namespace net {
 
-HttpPipelinedHostPool::HttpPipelinedHostPool(HttpStreamFactoryImpl* factory)
-    : factory_(factory) {
+// TODO(simonjam): Run experiments with different values of this to see what
+// value is good at avoiding evictions without eating too much memory. Until
+// then, this is just a bad guess.
+static const int kNumHostsToRemember = 200;
+
+class HttpPipelinedHostImplFactory : public HttpPipelinedHost::Factory {
+ public:
+  virtual HttpPipelinedHost* CreateNewHost(
+      HttpPipelinedHost::Delegate* delegate, const HostPortPair& origin,
+      HttpPipelinedConnection::Factory* factory,
+      HttpPipelinedHost::Capability capability) OVERRIDE {
+    return new HttpPipelinedHostImpl(delegate, origin, factory, capability);
+  }
+};
+
+HttpPipelinedHostPool::HttpPipelinedHostPool(
+    Delegate* delegate,
+    HttpPipelinedHost::Factory* factory)
+    : delegate_(delegate),
+      factory_(factory),
+      known_capability_map_(kNumHostsToRemember) {
+  if (!factory) {
+    factory_.reset(new HttpPipelinedHostImplFactory);
+  }
 }
 
 HttpPipelinedHostPool::~HttpPipelinedHostPool() {
   CHECK(host_map_.empty());
 }
 
+bool HttpPipelinedHostPool::IsHostEligibleForPipelining(
+    const HostPortPair& origin) {
+  HttpPipelinedHost::Capability capability = GetHostCapability(origin);
+  return capability != HttpPipelinedHost::INCAPABLE;
+}
+
 HttpPipelinedStream* HttpPipelinedHostPool::CreateStreamOnNewPipeline(
     const HostPortPair& origin,
     ClientSocketHandle* connection,
@@ -27,6 +54,9 @@ HttpPipelinedStream* HttpPipelinedHostPool::CreateStreamOnNewPipeline(
     const BoundNetLog& net_log,
     bool was_npn_negotiated) {
   HttpPipelinedHost* host = GetPipelinedHost(origin, true);
+  if (!host) {
+    return NULL;
+  }
   return host->CreateStreamOnNewPipeline(connection, used_ssl_config,
                                          used_proxy_info, net_log,
                                          was_npn_negotiated);
@@ -52,14 +82,21 @@ bool HttpPipelinedHostPool::IsExistingPipelineAvailableForOrigin(
 
 HttpPipelinedHost* HttpPipelinedHostPool::GetPipelinedHost(
     const HostPortPair& origin, bool create_if_not_found) {
-  HostMap::iterator it = host_map_.find(origin);
-  if (it != host_map_.end()) {
-    CHECK(it->second);
-    return it->second;
+  HostMap::iterator host_it = host_map_.find(origin);
+  if (host_it != host_map_.end()) {
+    CHECK(host_it->second);
+    return host_it->second;
   } else if (!create_if_not_found) {
     return NULL;
   }
-  HttpPipelinedHost* host = new HttpPipelinedHost(this, origin, NULL);
+
+  HttpPipelinedHost::Capability capability = GetHostCapability(origin);
+  if (capability == HttpPipelinedHost::INCAPABLE) {
+    return NULL;
+  }
+
+  HttpPipelinedHost* host = factory_->CreateNewHost(
+      this, origin, NULL, capability);
   host_map_[origin] = host;
   return host;
 }
@@ -67,14 +104,33 @@ HttpPipelinedHost* HttpPipelinedHostPool::GetPipelinedHost(
 void HttpPipelinedHostPool::OnHostIdle(HttpPipelinedHost* host) {
   const HostPortPair& origin = host->origin();
   CHECK(ContainsKey(host_map_, origin));
-  // TODO(simonjam): We should remember the pipeline state for each host.
   host_map_.erase(origin);
   delete host;
 }
 
 void HttpPipelinedHostPool::OnHostHasAdditionalCapacity(
     HttpPipelinedHost* host) {
-  factory_->OnHttpPipelinedHostHasAdditionalCapacity(host->origin());
+  delegate_->OnHttpPipelinedHostHasAdditionalCapacity(host->origin());
+}
+
+void HttpPipelinedHostPool::OnHostDeterminedCapability(
+    HttpPipelinedHost* host,
+    HttpPipelinedHost::Capability capability) {
+  CapabilityMap::iterator known_it = known_capability_map_.Get(host->origin());
+  if (known_it == known_capability_map_.end() ||
+      known_it->second != HttpPipelinedHost::INCAPABLE) {
+    known_capability_map_.Put(host->origin(), capability);
+  }
+}
+
+HttpPipelinedHost::Capability HttpPipelinedHostPool::GetHostCapability(
+    const HostPortPair& origin) {
+  HttpPipelinedHost::Capability capability = HttpPipelinedHost::UNKNOWN;
+  CapabilityMap::const_iterator it = known_capability_map_.Get(origin);
+  if (it != known_capability_map_.end()) {
+    capability = it->second;
+  }
+  return capability;
 }
 
 }  // namespace net