Fix bug where requests to QUIC servers were throttled by the resource scheduler.

Review URL: https://codereview.chromium.org/893693002

Cr-Commit-Position: refs/heads/master@{#314032}

content/browser/loader/resource_scheduler.cc

@@ -473,8 +473,8 @@ class ResourceScheduler::Client {
     request->set_request_priority_params(new_priority_params);
     if (!pending_requests_.IsQueued(request)) {
       DCHECK(ContainsKey(in_flight_requests_, request));
-      // The priority and SPDY support may have changed, so update the
-      // delayable count.
+      // The priority of the request and priority support of the server may
+      // have changed, so update the delayable count.
       SetRequestClassification(request, ClassifyRequest(request));
       // Request has already started.
       return;
@@ -617,7 +617,7 @@ class ResourceScheduler::Client {
           net::HostPortPair::FromURL(request->url_request()->url());
       net::HttpServerProperties& http_server_properties =
           *request->url_request()->context()->http_server_properties();
-      if (!http_server_properties.SupportsSpdy(host_port_pair) &&
+      if (!http_server_properties.SupportsRequestPriority(host_port_pair) &&
           ContainsKey(in_flight_requests_, request)) {
         return IN_FLIGHT_DELAYABLE_REQUEST;
       }
@@ -654,7 +654,7 @@ class ResourceScheduler::Client {
   //   * Synchronous requests.
   //   * Non-HTTP[S] requests.
   //
-  // 2. Requests to SPDY-capable origin servers.
+  // 2. Requests to request-priority-capable origin servers.
   //
   // 3. High-priority requests:
   //   * Higher priority requests (>= net::LOW).
@@ -668,8 +668,8 @@ class ResourceScheduler::Client {
   //  The following rules are followed:
   //
   //  ACTIVE_AND_LOADING and UNTHROTTLED Clients follow these rules:
-  //   * Non-delayable, High-priority and SPDY capable requests are issued
-  //     immediately.
+  //   * Non-delayable, High-priority and request-priority capable requests are
+  //     issued immediately.
   //   * Low priority requests are delayable.
   //   * Allow one delayable request to load at a time while layout-blocking
   //     requests are loading or the body tag has not yet been parsed.
@@ -679,8 +679,10 @@ class ResourceScheduler::Client {
   //   * Never exceed 6 delayable requests for a given host.
   //
   //  THROTTLED Clients follow these rules:
-  //   * Non-delayable and SPDY-capable requests are issued immediately.
-  //   * At most one non-SPDY request will be issued per THROTTLED Client
+  //   * Non-delayable and request-priority-capable requests are issued
+  //     immediately.
+  //   * At most one non-request-priority-capable request will be issued per
+  //     THROTTLED Client
   //   * If no high priority requests are in flight, start loading low priority
   //     requests.
   //
@@ -721,15 +723,16 @@ class ResourceScheduler::Client {
         *url_request.context()->http_server_properties();
 
     // TODO(willchan): We should really improve this algorithm as described in
-    // crbug.com/164101. Also, theoretically we should not count a SPDY request
-    // against the delayable requests limit.
-    if (http_server_properties.SupportsSpdy(host_port_pair)) {
+    // crbug.com/164101. Also, theoretically we should not count a
+    // request-priority capable request against the delayable requests limit.
+    if (http_server_properties.SupportsRequestPriority(host_port_pair)) {
       return START_REQUEST;
     }
 
     if (throttle_state_ == THROTTLED &&
         in_flight_requests_.size() >= kMaxNumThrottledRequestsPerClient) {
-      // There may still be SPDY-capable requests that should be issued.
+      // There may still be request-priority-capable requests that should be
+      // issued.
       return DO_NOT_START_REQUEST_AND_KEEP_SEARCHING;
     }
 

net/http/http_server_properties.h

@@ -162,8 +162,9 @@ class NET_EXPORT HttpServerProperties {
   // Deletes all data.
   virtual void Clear() = 0;
 
-  // Returns true if |server| supports SPDY.
-  virtual bool SupportsSpdy(const HostPortPair& server) = 0;
+  // Returns true if |server| supports a network protocol which honors
+  // request prioritization.
+  virtual bool SupportsRequestPriority(const HostPortPair& server) = 0;
 
   // Add |server| into the persistent store. Should only be called from IO
   // thread.

net/http/http_server_properties_impl.cc

@@ -168,7 +168,7 @@ void HttpServerPropertiesImpl::Clear() {
   server_network_stats_map_.Clear();
 }
 
-bool HttpServerPropertiesImpl::SupportsSpdy(
+bool HttpServerPropertiesImpl::SupportsRequestPriority(
     const HostPortPair& host_port_pair) {
   DCHECK(CalledOnValidThread());
   if (host_port_pair.host().empty())
@@ -176,9 +176,14 @@ bool HttpServerPropertiesImpl::SupportsSpdy(
 
   SpdyServerHostPortMap::iterator spdy_host_port =
       spdy_servers_map_.Get(host_port_pair.ToString());
-  if (spdy_host_port != spdy_servers_map_.end())
-    return spdy_host_port->second;
-  return false;
+  if (spdy_host_port != spdy_servers_map_.end() && spdy_host_port->second)
+    return true;
+
+  if (!HasAlternateProtocol(host_port_pair))
+    return false;
+
+  AlternateProtocolInfo info = GetAlternateProtocol(host_port_pair);
+  return info.protocol == QUIC;
 }
 
 void HttpServerPropertiesImpl::SetSupportsSpdy(

net/http/http_server_properties_impl.h

@@ -73,7 +73,7 @@ class NET_EXPORT HttpServerPropertiesImpl
 
   base::WeakPtr<HttpServerProperties> GetWeakPtr() override;
   void Clear() override;
-  bool SupportsSpdy(const HostPortPair& server) override;
+  bool SupportsRequestPriority(const HostPortPair& server) override;
   void SetSupportsSpdy(const HostPortPair& server, bool support_spdy) override;
   bool RequiresHTTP11(const HostPortPair& server) override;
   void SetHTTP11Required(const HostPortPair& server) override;

net/http/http_server_properties_impl_unittest.cc

@@ -41,18 +41,18 @@ TEST_F(SpdyServerPropertiesTest, Initialize) {
 
   // Check by initializing NULL spdy servers.
   impl_.InitializeSpdyServers(NULL, true);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_google));
 
   // Check by initializing empty spdy servers.
   std::vector<std::string> spdy_servers;
   impl_.InitializeSpdyServers(&spdy_servers, true);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_google));
 
   // Check by initializing with www.google.com:443 spdy server.
   std::vector<std::string> spdy_servers1;
   spdy_servers1.push_back(spdy_server_g);
   impl_.InitializeSpdyServers(&spdy_servers1, true);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
 
   // Check by initializing with www.google.com:443 and docs.google.com:443 spdy
   // servers.
@@ -71,55 +71,38 @@ TEST_F(SpdyServerPropertiesTest, Initialize) {
   std::string string_value_d;
   ASSERT_TRUE(spdy_server_list.GetString(1, &string_value_d));
   ASSERT_EQ(spdy_server_d, string_value_d);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_docs));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_docs));
 }
 
-TEST_F(SpdyServerPropertiesTest, SupportsSpdyTest) {
+TEST_F(SpdyServerPropertiesTest, SupportsRequestPriorityTest) {
   HostPortPair spdy_server_empty(std::string(), 443);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_empty));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_empty));
 
   // Add www.google.com:443 as supporting SPDY.
   HostPortPair spdy_server_google("www.google.com", 443);
   impl_.SetSupportsSpdy(spdy_server_google, true);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
 
   // Add mail.google.com:443 as not supporting SPDY.
   HostPortPair spdy_server_mail("mail.google.com", 443);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_mail));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_mail));
 
   // Add docs.google.com:443 as supporting SPDY.
   HostPortPair spdy_server_docs("docs.google.com", 443);
   impl_.SetSupportsSpdy(spdy_server_docs, true);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_docs));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_docs));
 
-  // Verify all the entries are the same after additions.
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_mail));
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_docs));
-}
-
-TEST_F(SpdyServerPropertiesTest, SetSupportsSpdy) {
-  HostPortPair spdy_server_empty(std::string(), 443);
-  impl_.SetSupportsSpdy(spdy_server_empty, true);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_empty));
+  // Add www.youtube.com:443 as supporting QUIC.
+  HostPortPair quic_server_youtube("www.youtube.com", 443);
+  impl_.SetAlternateProtocol(quic_server_youtube, 443, QUIC, 1);
+  EXPECT_TRUE(impl_.SupportsRequestPriority(quic_server_youtube));
 
-  // Add www.google.com:443 as supporting SPDY.
-  HostPortPair spdy_server_google("www.google.com", 443);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
-  impl_.SetSupportsSpdy(spdy_server_google, true);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
-
-  // Make www.google.com:443 as not supporting SPDY.
-  impl_.SetSupportsSpdy(spdy_server_google, false);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
-
-  // Add mail.google.com:443 as supporting SPDY.
-  HostPortPair spdy_server_mail("mail.google.com", 443);
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_mail));
-  impl_.SetSupportsSpdy(spdy_server_mail, true);
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_mail));
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
+  // Verify all the entries are the same after additions.
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_mail));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_docs));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(quic_server_youtube));
 }
 
 TEST_F(SpdyServerPropertiesTest, Clear) {
@@ -129,12 +112,12 @@ TEST_F(SpdyServerPropertiesTest, Clear) {
   HostPortPair spdy_server_mail("mail.google.com", 443);
   impl_.SetSupportsSpdy(spdy_server_mail, true);
 
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_mail));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_mail));
 
   impl_.Clear();
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_google));
-  EXPECT_FALSE(impl_.SupportsSpdy(spdy_server_mail));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_google));
+  EXPECT_FALSE(impl_.SupportsRequestPriority(spdy_server_mail));
 }
 
 TEST_F(SpdyServerPropertiesTest, GetSpdyServerList) {
@@ -223,7 +206,7 @@ TEST_F(SpdyServerPropertiesTest, MRUOfGetSpdyServerList) {
 
   // Get www.google.com:443 should reorder SpdyServerHostPortMap. Verify that it
   // is www.google.com:443 is the MRU server.
-  EXPECT_TRUE(impl_.SupportsSpdy(spdy_server_google));
+  EXPECT_TRUE(impl_.SupportsRequestPriority(spdy_server_google));
   impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   ASSERT_EQ(2U, spdy_server_list.GetSize());
   ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));

net/http/http_server_properties_manager.cc

@@ -133,9 +133,10 @@ void HttpServerPropertiesManager::Clear(const base::Closure& completion) {
   UpdatePrefsFromCacheOnNetworkThread(completion);
 }
 
-bool HttpServerPropertiesManager::SupportsSpdy(const HostPortPair& server) {
+bool HttpServerPropertiesManager::SupportsRequestPriority(
+    const HostPortPair& server) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
-  return http_server_properties_impl_->SupportsSpdy(server);
+  return http_server_properties_impl_->SupportsRequestPriority(server);
 }
 
 void HttpServerPropertiesManager::SetSupportsSpdy(const HostPortPair& server,

net/http/http_server_properties_manager.h

@@ -80,7 +80,7 @@ class NET_EXPORT HttpServerPropertiesManager : public HttpServerProperties {
 
   base::WeakPtr<HttpServerProperties> GetWeakPtr() override;
   void Clear() override;
-  bool SupportsSpdy(const HostPortPair& server) override;
+  bool SupportsRequestPriority(const HostPortPair& server) override;
   void SetSupportsSpdy(const HostPortPair& server, bool support_spdy) override;
   bool RequiresHTTP11(const HostPortPair& server) override;
   void SetHTTP11Required(const HostPortPair& server) override;

net/http/http_server_properties_manager_unittest.cc

@@ -223,9 +223,10 @@ TEST_F(HttpServerPropertiesManagerTest,
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
   // Verify SupportsSpdy.
-  EXPECT_TRUE(http_server_props_manager_->SupportsSpdy(google_server));
-  EXPECT_TRUE(http_server_props_manager_->SupportsSpdy(mail_server));
-  EXPECT_FALSE(http_server_props_manager_->SupportsSpdy(
+  EXPECT_TRUE(
+      http_server_props_manager_->SupportsRequestPriority(google_server));
+  EXPECT_TRUE(http_server_props_manager_->SupportsRequestPriority(mail_server));
+  EXPECT_FALSE(http_server_props_manager_->SupportsRequestPriority(
       HostPortPair::FromString("foo.google.com:1337")));
 
   // Verify AlternateProtocol.
@@ -304,7 +305,7 @@ TEST_F(HttpServerPropertiesManagerTest, BadCachedHostPortPair) {
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 
   // Verify that nothing is set.
-  EXPECT_FALSE(http_server_props_manager_->SupportsSpdy(
+  EXPECT_FALSE(http_server_props_manager_->SupportsRequestPriority(
       HostPortPair::FromString("www.google.com:65536")));
   EXPECT_FALSE(http_server_props_manager_->HasAlternateProtocol(
       HostPortPair::FromString("www.google.com:65536")));
@@ -363,13 +364,15 @@ TEST_F(HttpServerPropertiesManagerTest, SupportsSpdy) {
 
   // Add mail.google.com:443 as a supporting spdy server.
   HostPortPair spdy_server_mail("mail.google.com", 443);
-  EXPECT_FALSE(http_server_props_manager_->SupportsSpdy(spdy_server_mail));
+  EXPECT_FALSE(
+      http_server_props_manager_->SupportsRequestPriority(spdy_server_mail));
   http_server_props_manager_->SetSupportsSpdy(spdy_server_mail, true);
 
   // Run the task.
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(http_server_props_manager_->SupportsSpdy(spdy_server_mail));
+  EXPECT_TRUE(
+      http_server_props_manager_->SupportsRequestPriority(spdy_server_mail));
   Mock::VerifyAndClearExpectations(http_server_props_manager_.get());
 }
 
@@ -556,7 +559,8 @@ TEST_F(HttpServerPropertiesManagerTest, Clear) {
   // Run the task.
   base::RunLoop().RunUntilIdle();
 
-  EXPECT_TRUE(http_server_props_manager_->SupportsSpdy(spdy_server_mail));
+  EXPECT_TRUE(
+      http_server_props_manager_->SupportsRequestPriority(spdy_server_mail));
   EXPECT_TRUE(
       http_server_props_manager_->HasAlternateProtocol(spdy_server_mail));
   SupportsQuic supports_quic =
@@ -585,7 +589,8 @@ TEST_F(HttpServerPropertiesManagerTest, Clear) {
   http_server_props_manager_->Clear(base::MessageLoop::QuitClosure());
   base::RunLoop().Run();
 
-  EXPECT_FALSE(http_server_props_manager_->SupportsSpdy(spdy_server_mail));
+  EXPECT_FALSE(
+      http_server_props_manager_->SupportsRequestPriority(spdy_server_mail));
   EXPECT_FALSE(
       http_server_props_manager_->HasAlternateProtocol(spdy_server_mail));
   SupportsQuic supports_quic1 =

net/http/http_stream_factory_impl_job.cc

@@ -134,8 +134,9 @@ int HttpStreamFactoryImpl::Job::Preconnect(int num_streams) {
   DCHECK_GT(num_streams, 0);
   base::WeakPtr<HttpServerProperties> http_server_properties =
       session_->http_server_properties();
-  if (http_server_properties && http_server_properties->SupportsSpdy(
-      HostPortPair::FromURL(request_info_.url))) {
+  if (http_server_properties &&
+      http_server_properties->SupportsRequestPriority(
+          HostPortPair::FromURL(request_info_.url))) {
     num_streams_ = 1;
   } else {
     num_streams_ = num_streams;