Add a force pipelining option to load flags.

net/http/http_stream_factory_impl_job.cc

 // Copyright (c) 2012 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/bind.h"
 #include "base/bind_helpers.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
@@ skipping 544 matching lines @@
   int rv = RunLoop(OK);
   DCHECK_EQ(ERR_IO_PENDING, rv);
   return rv;
 }
 
 int HttpStreamFactoryImpl::Job::DoStart() {
   int port = request_info_.url.EffectiveIntPort();
   origin_ = HostPortPair(request_info_.url.HostNoBrackets(), port);
   origin_url_ = HttpStreamFactory::ApplyHostMappingRules(
       request_info_.url, &origin_);
+  http_pipelining_key_.reset(new HttpPipelinedHost::Key(origin_));
 
   net_log_.BeginEvent(NetLog::TYPE_HTTP_STREAM_JOB,
                       HttpStreamJobParameters::Create(request_info_.url,
                                                       origin_url_));
 
   // Don't connect to restricted ports.
   if (!IsPortAllowedByDefault(port) && !IsPortAllowedByOverride(port)) {
     if (waiting_job_) {
       waiting_job_->Resume(this);
       waiting_job_ = NULL;
@@ skipping 104 matching lines @@
     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.
+    //
+    // Separate note: A forced pipeline is always available if one exists for
+    // this key. This is different than normal pipelines, which may be
+    // unavailable or unusable. So, there is no need to worry about a race
+    // between when a pipeline becomes available and when this job blocks.
     existing_available_pipeline_ = stream_factory_->http_pipelined_host_pool_.
-        IsExistingPipelineAvailableForOrigin(origin_);
+        IsExistingPipelineAvailableForKey(*http_pipelining_key_.get());
     if (existing_available_pipeline_) {
       return OK;
     } else {
-      request_->SetHttpPipeliningKey(origin_);
+      bool was_new_key = request_->SetHttpPipeliningKey(
+          *http_pipelining_key_.get());
+      if (!was_new_key && session_->force_http_pipelining()) {
+        return ERR_IO_PENDING;

[mmenke, 2012/03/01 16:50:30]

I'm not sure this address's Will's concern about HttpStreamFactoryImpl's
complexity.

[James Simonsen, 2012/03/01 18:49:29]

Any ideas?

The "right" thing to do would be to write a whole new factory, job, and request.
90% of the code would be the same though and it seems silly to copy & paste it.

Trying to share that code with common base classes would just confuse the normal
code path, which is a bad trade-off. I'd rather the special case be the
confusing one (which is what we have now).

Unless there's a better idea, I think the current code is the best of the
options.

[mmenke, 2012/03/01 19:10:09]

I was thinking maybe a subclass...Not sure how workable that would be, though.

+      }
     }
   }
 
   // OK, there's no available SPDY session. Let |waiting_job_| resume if it's
   // paused.
 
   if (waiting_job_) {
     waiting_job_->Resume(this);
     waiting_job_ = NULL;
   }
@@ skipping 43 matching lines @@
     return OK;
   }
 
   // TODO(willchan): Make this a bit more exact. Maybe there are recoverable
   // errors, such as ignoring certificate errors for Alternate-Protocol.
   if (result < 0 && waiting_job_) {
     waiting_job_->Resume(this);
     waiting_job_ = NULL;
   }
 
+  if (result < 0 && session_->force_http_pipelining()) {
+    stream_factory_->AbortPipelinedRequestsWithKey(
+        this, *http_pipelining_key_.get(), result, server_ssl_config_);
+  }
+
   // |result| may be the result of any of the stacked pools. The following
   // logic is used when determining how to interpret an error.
   // If |result| < 0:
   //   and connection_->socket() != NULL, then the SSL handshake ran and it
   //     is a potentially recoverable error.
   //   and connection_->socket == NULL and connection_->is_ssl_error() is true,
   //     then the SSL handshake ran with an unrecoverable error.
   //   otherwise, the error came from one of the other pools.
   bool ssl_started = using_ssl_ && (result == OK || connection_->socket() ||
                                     connection_->is_ssl_error());
@@ skipping 112 matching lines @@
   // 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");
     // TODO(simonjam): Support proxies.
-    if (existing_available_pipeline_) {
+    if (stream_factory_->http_pipelined_host_pool_.
+            IsExistingPipelineAvailableForKey(*http_pipelining_key_.get())) {
       stream_.reset(stream_factory_->http_pipelined_host_pool_.
-                    CreateStreamOnExistingPipeline(origin_));
+                    CreateStreamOnExistingPipeline(
+                        *http_pipelining_key_.get()));
       CHECK(stream_.get());
     } else if (!using_proxy && IsRequestEligibleForPipelining()) {
       stream_.reset(
           stream_factory_->http_pipelined_host_pool_.CreateStreamOnNewPipeline(
-              origin_,
+              *http_pipelining_key_.get(),
               connection_.release(),
               server_ssl_config_,
               proxy_info_,
               net_log_,
               was_npn_negotiated_,
               protocol_negotiated_));
       CHECK(stream_.get());
     } else {
       stream_.reset(new HttpBasicStream(connection_.release(), NULL,
                                         using_proxy));
@@ skipping 294 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() {
+  if (IsPreconnecting() || !request_) {
+    return false;
+  }
+  if (session_->force_http_pipelining()) {
+    return true;
+  }
   if (!HttpStreamFactory::http_pipelining_enabled()) {
     return false;
   }
-  if (IsPreconnecting() || !request_) {
-    return false;
-  }
   if (using_ssl_) {
     return false;
   }
   if (request_info_.method != "GET" && request_info_.method != "HEAD") {
     return false;
   }
-  return stream_factory_->http_pipelined_host_pool_.IsHostEligibleForPipelining(
-      origin_);
+  return stream_factory_->http_pipelined_host_pool_.IsKeyEligibleForPipelining(
+      *http_pipelining_key_.get());
 }
 
 }  // namespace net