Refuse to pipeline frames, prefetches, and downloads.

BUG=119287
TEST=Follow steps in bug


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

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

net/http/http_pipelined_network_transaction_unittest.cc

@@ -113,11 +113,20 @@ class HttpPipelinedNetworkTransactionTest : public testing::Test {
     data_vector_.push_back(data);
   }
 
-  HttpRequestInfo* GetRequestInfo(const char* filename) {
+  enum RequestInfoOptions {
+    REQUEST_DEFAULT,
+    REQUEST_MAIN_RESOURCE,
+  };
+
+  HttpRequestInfo* GetRequestInfo(
+      const char* filename, RequestInfoOptions options = REQUEST_DEFAULT) {
     std::string url = StringPrintf("http://localhost/%s", filename);
     HttpRequestInfo* request_info = new HttpRequestInfo;
     request_info->url = GURL(url);
     request_info->method = "GET";
+    if (options == REQUEST_MAIN_RESOURCE) {
+      request_info->load_flags = LOAD_MAIN_FRAME;
+    }
     request_info_vector_.push_back(request_info);
     return request_info;
   }
@@ -176,32 +185,32 @@ class HttpPipelinedNetworkTransactionTest : public testing::Test {
     ExpectResponse("two.html", two_transaction, SYNCHRONOUS);
   }
 
-  void CompleteFourRequests() {
+  void CompleteFourRequests(RequestInfoOptions options) {
     scoped_ptr<HttpNetworkTransaction> one_transaction(
         new HttpNetworkTransaction(session_.get()));
     TestCompletionCallback one_callback;
     EXPECT_EQ(ERR_IO_PENDING,
-              one_transaction->Start(GetRequestInfo("one.html"),
+              one_transaction->Start(GetRequestInfo("one.html", options),
                                      one_callback.callback(), BoundNetLog()));
     EXPECT_EQ(OK, one_callback.WaitForResult());
 
     HttpNetworkTransaction two_transaction(session_.get());
     TestCompletionCallback two_callback;
     EXPECT_EQ(ERR_IO_PENDING,
-              two_transaction.Start(GetRequestInfo("two.html"),
+              two_transaction.Start(GetRequestInfo("two.html", options),
                                     two_callback.callback(), BoundNetLog()));
 
     HttpNetworkTransaction three_transaction(session_.get());
     TestCompletionCallback three_callback;
     EXPECT_EQ(ERR_IO_PENDING,
-              three_transaction.Start(GetRequestInfo("three.html"),
+              three_transaction.Start(GetRequestInfo("three.html", options),
                                       three_callback.callback(),
                                       BoundNetLog()));
 
     HttpNetworkTransaction four_transaction(session_.get());
     TestCompletionCallback four_callback;
     EXPECT_EQ(ERR_IO_PENDING,
-              four_transaction.Start(GetRequestInfo("four.html"),
+              four_transaction.Start(GetRequestInfo("four.html", options),
                                      four_callback.callback(), BoundNetLog()));
 
     ExpectResponse("one.html", *one_transaction.get(), SYNCHRONOUS);
@@ -316,7 +325,50 @@ TEST_F(HttpPipelinedNetworkTransactionTest, ReusesOnSpaceAvailable) {
   };
   AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
 
-  CompleteFourRequests();
+  CompleteFourRequests(REQUEST_DEFAULT);
+
+  ClientSocketPoolManager::set_max_sockets_per_group(
+      HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets);
+}
+
+TEST_F(HttpPipelinedNetworkTransactionTest, WontPipelineMainResource) {
+  int old_max_sockets = ClientSocketPoolManager::max_sockets_per_group(
+      HttpNetworkSession::NORMAL_SOCKET_POOL);
+  ClientSocketPoolManager::set_max_sockets_per_group(
+      HttpNetworkSession::NORMAL_SOCKET_POOL, 1);
+  Initialize(false);
+
+  MockWrite writes[] = {
+    MockWrite(SYNCHRONOUS, 0, "GET /one.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(SYNCHRONOUS, 4, "GET /two.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(SYNCHRONOUS, 8, "GET /three.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+    MockWrite(SYNCHRONOUS, 12, "GET /four.html HTTP/1.1\r\n"
+              "Host: localhost\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+  MockRead reads[] = {
+    MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 OK\r\n"),
+    MockRead(SYNCHRONOUS, 2, "Content-Length: 8\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 3, "one.html"),
+    MockRead(SYNCHRONOUS, 5, "HTTP/1.1 200 OK\r\n"),
+    MockRead(SYNCHRONOUS, 6, "Content-Length: 8\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 7, "two.html"),
+    MockRead(SYNCHRONOUS, 9, "HTTP/1.1 200 OK\r\n"),
+    MockRead(SYNCHRONOUS, 10, "Content-Length: 10\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 11, "three.html"),
+    MockRead(SYNCHRONOUS, 13, "HTTP/1.1 200 OK\r\n"),
+    MockRead(SYNCHRONOUS, 14, "Content-Length: 9\r\n\r\n"),
+    MockRead(SYNCHRONOUS, 15, "four.html"),
+  };
+  AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
+
+  CompleteFourRequests(REQUEST_MAIN_RESOURCE);
 
   ClientSocketPoolManager::set_max_sockets_per_group(
       HttpNetworkSession::NORMAL_SOCKET_POOL, old_max_sockets);
@@ -669,7 +721,7 @@ TEST_F(HttpPipelinedNetworkTransactionTest, PipelinesImmediatelyIfKnownGood) {
   };
   AddExpectedConnection(reads, arraysize(reads), writes, arraysize(writes));
 
-  CompleteFourRequests();
+  CompleteFourRequests(REQUEST_DEFAULT);
 
   HttpNetworkTransaction second_one_transaction(session_.get());
   TestCompletionCallback second_one_callback;

net/http/http_stream_factory_impl_job.cc

@@ -910,7 +910,6 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
   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_.
@@ -918,6 +917,7 @@ int HttpStreamFactoryImpl::Job::DoCreateStream() {
                         *http_pipelining_key_.get()));
       CHECK(stream_.get());
     } else if (!using_proxy && IsRequestEligibleForPipelining()) {
+      // TODO(simonjam): Support proxies.
       stream_.reset(
           stream_factory_->http_pipelined_host_pool_.CreateStreamOnNewPipeline(
               *http_pipelining_key_.get(),
@@ -1247,6 +1247,12 @@ bool HttpStreamFactoryImpl::Job::IsRequestEligibleForPipelining() {
   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)) {
+    // Avoid pipelining resources that may be streamed for a long time.
+    return false;
+  }
   return stream_factory_->http_pipelined_host_pool_.IsKeyEligibleForPipelining(
       *http_pipelining_key_.get());
 }