Refuse to pipeline frames, prefetches, and downloads.

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 884 matching lines @@
   // 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");
-    // TODO(simonjam): Support proxies.
     if (stream_factory_->http_pipelined_host_pool_.
             IsExistingPipelineAvailableForKey(*http_pipelining_key_.get())) {
       stream_.reset(stream_factory_->http_pipelined_host_pool_.
                     CreateStreamOnExistingPipeline(
                         *http_pipelining_key_.get()));
       CHECK(stream_.get());
+    // TODO(simonjam): Support proxies.

[mmenke, 2012/04/19 14:40:20]

nit:  This should be indented two spaces.  Suggest you move it one line down.

[James Simonsen, 2012/04/26 19:17:02]

Done.

     } else if (!using_proxy && IsRequestEligibleForPipelining()) {
       stream_.reset(
           stream_factory_->http_pipelined_host_pool_.CreateStreamOnNewPipeline(
               *http_pipelining_key_.get(),
               connection_.release(),
               server_ssl_config_,
               proxy_info_,
               net_log_,
               was_npn_negotiated_,
               protocol_negotiated_));
@@ skipping 317 matching lines @@
   }
   if (!HttpStreamFactory::http_pipelining_enabled()) {
     return false;
   }
   if (using_ssl_) {
     return false;
   }
   if (request_info_.method != "GET" && request_info_.method != "HEAD") {
     return false;
   }
+  if (request_info_.load_flags &
+      (net::LOAD_MAIN_FRAME | net::LOAD_SUB_FRAME | net::LOAD_PREFETCH |
+       net::LOAD_IS_DOWNLOAD)) {

[mmenke, 2012/04/19 14:40:20]

nit:  Think a comment might be nice.

While most downloads should be main frames, subframes, or downloads, I wonder if
it includes all downloads.  Wonder how only pipelining resources of the same
type compares in terms of success rate to this.

[mmenke, 2012/04/19 14:40:20]

Might be nice to have a regression test for this as well.

[James Simonsen, 2012/04/26 19:17:02]

Done.

[James Simonsen, 2012/04/26 19:17:02]

Done.
Yeah, that was my original plan: to make it opt-in instead of opt-out. WebKit
tells the browser process what type of resource it's downloading (image, css,
js, etc.). That info doesn't make it to net/ though. Instead, that type is
translated into load flags.

This patch is simple and fixes the build.chromium.org/.../stdio case. If we find
other bugs, I can switch it around.

+    return false;
+  }
   return stream_factory_->http_pipelined_host_pool_.IsKeyEligibleForPipelining(
       *http_pipelining_key_.get());
 }
 
 }  // namespace net