Basic HTTP pipelining support

net/http/http_stream_factory_impl_job.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_stream_factory_impl_job.h"
 
 #include "base/logging.h"
 #include "base/stl_util-inl.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "build/build_config.h"
 #include "net/base/connection_type_histograms.h"
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/base/ssl_cert_request_info.h"
 #include "net/http/http_basic_stream.h"
 #include "net/http/http_network_session.h"
+#include "net/http/http_pipelined_connection.h"
+#include "net/http/http_pipelined_host.h"
+#include "net/http/http_pipelined_host_pool.h"
+#include "net/http/http_pipelined_stream.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_proxy_client_socket_pool.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_stream_factory_impl_request.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/client_socket_pool.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/spdy/spdy_http_stream.h"
@@ skipping 49 matching lines @@
       dependent_job_(NULL),
       using_ssl_(false),
       using_spdy_(false),
       force_spdy_always_(HttpStreamFactory::force_spdy_always()),
       force_spdy_over_ssl_(HttpStreamFactory::force_spdy_over_ssl()),
       spdy_certificate_error_(OK),
       establishing_tunnel_(false),
       was_npn_negotiated_(false),
       num_streams_(0),
       spdy_session_direct_(false),
+      existing_available_pipeline_(NULL),
       ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)) {
   DCHECK(stream_factory);
   DCHECK(session);
 }
 
 HttpStreamFactoryImpl::Job::~Job() {
   net_log_.EndEvent(NetLog::TYPE_HTTP_STREAM_JOB, NULL);
 
   // When we're in a partially constructed state, waiting for the user to
   // provide certificate handling information or authentication, we can't reuse
@@ skipping 475 matching lines @@
     // actually need to preconnect any sockets, so we're done.
     if (IsPreconnecting())
       return OK;
     using_spdy_ = true;
     next_state_ = STATE_CREATE_STREAM;
     existing_spdy_session_ = spdy_session;
     return OK;
   } else if (request_ && (using_ssl_ || ShouldForceSpdyWithoutSSL())) {
     // Update the spdy session key for the request that launched this job.
     request_->SetSpdySessionKey(spdy_session_key);
+  } else if (IsRequestEligibleForPipelining()) {
+    HttpPipelinedHost* host = session_->http_pipelined_host_pool()->
+        GetPipelinedHostIfExists(origin_);
+    if (host) {
+      HttpPipelinedConnection* connection = host->FindAvailablePipeline();
+      if (connection) {
+        next_state_ = STATE_CREATE_STREAM;
+        existing_available_pipeline_ = connection;
+        return OK;
+      }
+    }
+    request_->SetHttpPipeliningKey(origin_);
   }
 
   // OK, there's no available SPDY session. Let |dependent_job_| resume if it's
   // paused.
 
   if (dependent_job_) {
     dependent_job_->Resume(this);
     dependent_job_ = NULL;
   }
 
@@ skipping 157 matching lines @@
 int HttpStreamFactoryImpl::Job::DoWaitingUserAction(int result) {
   // This state indicates that the stream request is in a partially
   // completed state, and we've called back to the delegate for more
   // information.
 
   // We're always waiting here for the delegate to call us back.
   return ERR_IO_PENDING;
 }
 
 int HttpStreamFactoryImpl::Job::DoCreateStream() {
-  if (!connection_->socket() && !existing_spdy_session_)
+  if (!connection_->socket() && !existing_spdy_session_ &&
+      !existing_available_pipeline_)

[willchan no longer on Chromium, 2011/07/19 13:29:38]

Heh, this is used to debug bug 80095. You probably shouldn't use it. You
probably want a DCHECK(connection_->socket() || existing_spdy_session_ ||
existing_available_pipeline_); Put that on the next line, then later on when I
remove the HACK function, your assertion will be the valid one.

[James Simonsen, 2011/07/19 22:54:23]

Done, but I still need to avoid falling into the HACK when I use an existing
pipeline, so I still need to modify the if().

     HACKCrashHereToDebug80095();
 
   next_state_ = STATE_CREATE_STREAM_COMPLETE;
 
   // We only set the socket motivation if we're the first to use
   // this socket.  Is there a race for two SPDY requests?  We really
   // need to plumb this through to the connect level.
   if (connection_->socket() && !connection_->is_reused())
     SetSocketMotivation();
 
   const ProxyServer& proxy_server = proxy_info_.proxy_server();
 
   if (!using_spdy_) {
     bool using_proxy = (proxy_info_.is_http() || proxy_info_.is_https()) &&
         request_info_.url.SchemeIs("http");
-    stream_.reset(new HttpBasicStream(connection_.release(), NULL,
-                                      using_proxy));
+    // TODO(simonjam): Support proxies.
+    if (existing_available_pipeline_) {
+      stream_.reset(new HttpPipelinedStream(existing_available_pipeline_));
+    } else if (!using_proxy && IsRequestEligibleForPipelining()) {
+      HttpPipelinedConnection* pipeline = session_->http_pipelined_host_pool()->
+          GetOrCreatePipelinedHost(origin_)->CreateNewPipeline(

[willchan no longer on Chromium, 2011/07/19 13:29:38]

I need to look at this a bit more. This is pretty key code. Currently this code
seems to make the assumption that for all hosts, we can pipeline requests to
them. Obviously, we'll be tying in heuristics here, and also need to check to
see if we already tried to pipeline to this host, and if it failed, we shouldn't
retry. Lots of logic should be going in around this location.

[James Simonsen, 2011/07/19 22:54:23]

Yeah, I'm still not sure exactly what we all need. For instance, will Google
keep a list of good/bad sites or will each client build their own? Anyway, I
don't intend to figure this out until we have some actual data.

+              connection_.release(),
+              ssl_config_,
+              proxy_info_,
+              net_log_,
+              was_npn_negotiated_);
+      stream_.reset(new HttpPipelinedStream(pipeline));
+    } else {
+      stream_.reset(new HttpBasicStream(connection_.release(), NULL,
+                                        using_proxy));
+    }
     return OK;
   }
 
   CHECK(!stream_.get());
 
   if (!connection_->socket() && !existing_spdy_session_)
     HACKCrashHereToDebug80095();
 
   bool direct = true;
   HostPortProxyPair pair(origin_, proxy_server);
@@ skipping 278 matching lines @@
 
 bool HttpStreamFactoryImpl::Job::IsPreconnecting() const {
   DCHECK_GE(num_streams_, 0);
   return num_streams_ > 0;
 }
 
 bool HttpStreamFactoryImpl::Job::IsOrphaned() const {
   return !IsPreconnecting() && !request_;
 }
 
+bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() const {

[willchan no longer on Chromium, 2011/07/19 13:29:38]

So, we probably want to expose pipelining via about:flags. That will let us set
a command line flag for this feature. You should correlate that command line
flag to a global variable here. When the command line flag is set to true, then
a global variable in this code should be set to true too. Note that command line
flags only exist in chrome/. For an example of how to do this, please refer to
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/browse....
That doesn't show how to tie it into about:flags, but you can probably read
http://codesearch.google.com/codesearch#OAMlx_jo-ck/src/chrome/browser/about_...
and figure it out pretty easily (I've never done it myself, but it looks easy).

[James Simonsen, 2011/07/19 22:54:23]

Done. Yep, easy.

+  if (IsPreconnecting() || !request_) {
+    return false;
+  }
+  return request_info_.method == "GET";

[willchan no longer on Chromium, 2011/07/19 13:29:38]

Allow other idempotent methods like HEAD?

[James Simonsen, 2011/07/19 22:54:23]

Done. HEAD seems the only other safe and useful one right now.

+}
+
 #if defined(OS_WIN)
 #pragma warning (disable: 4748)
 #pragma optimize( "", off )
 #endif
 
 void HttpStreamFactoryImpl::Job::HACKCrashHereToDebug80095() {
   // If we enter this code path, then we'll cause a crash later in
   // DoCreateStream(). Crash now and figure out what happened:
   // http://crbug.com/80095.
   GURL url = original_url_.get() ? *original_url_ : request_info_.url;
@@ skipping 24 matching lines @@
                << (using_spdy ? "true\n" : "false\n");
   }
 }
 
 #if defined(OS_WIN)
 #pragma optimize( "", on )
 #pragma warning (default: 4748)
 #endif
 
 }  // namespace net