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.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.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"
 #include "net/spdy/spdy_session.h"
 #include "net/spdy/spdy_session_pool.h"
 
@@ skipping 48 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 480 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()) {
+    // TODO(simonjam): With pipelining, we might be better off using fewer
+    // connections and thus should make fewer preconnections. Explore
+    // preconnecting fewer than the requested num_connections.
+    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 156 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_)
     HACKCrashHereToDebug80095();

[willchan no longer on Chromium, 2011/09/26 23:49:20]

Can you just kill this code? It's been debugged, I haven't cleaned it up yet.
Thanks.

[James Simonsen, 2011/09/29 02:39:31]

Done.

 
+  DCHECK(connection_->socket() || existing_spdy_session_ ||
+         existing_available_pipeline_);
+
   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(existing_available_pipeline_->CreateNewStream());
+    } else if (!using_proxy && IsRequestEligibleForPipelining()) {
+      HttpPipelinedConnection* pipeline = session_->http_pipelined_host_pool()->

[willchan no longer on Chromium, 2011/09/26 23:49:20]

It's not clear why the HttpPipelinedConnection* is exposed to
HttpStreamFactoryImpl::Job. It sort of seems like, from the Job*'s perspective,
we should just have an API on the HttpPipelinedHostPool that creates a stream.
The HttpPipelinedConnection and HttpPipelinedHost are implementation details
behind HttpPipelinedHostPool.

[James Simonsen, 2011/09/29 02:39:31]

Done.

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

I see. I feel like this is revealing a problem with the design, since I would
think that HttpStreamFactory should be able to hide the pipelining details. It
seems to me that the reason the tight coupling is getting introduced is we have
an issue with how we locate some of our HTTP state. It seems to me that the
HttpPipelinedHostPool should live in HttpStreamFactoryImpl instead of in
HttpNetworkSession. I think the same is true of SpdySessionPool. I should file a
SPDY bug to clean this up. Can you move HttpPipelinedHostPool to be a member of
HttpStreamFactoryImpl?

+          GetOrCreatePipelinedHost(origin_)->CreateNewPipeline(
+              connection_.release(),
+              server_ssl_config_,
+              proxy_info_,
+              net_log_,
+              was_npn_negotiated_);
+      stream_.reset(pipeline->CreateNewStream());
+    } 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 285 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 {
+  if (!HttpStreamFactory::http_pipelining_enabled()) {
+    return false;
+  }
+  if (IsPreconnecting() || !request_) {
+    return false;
+  }
+  if (using_ssl_) {
+    return false;
+  }
+  return request_info_.method == "GET" || request_info_.method == "HEAD";
+}
+
 #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