Do not return alternative services equivalent to the origin in GetAlternativeServices().

If there is an alternative service equivalent to the origin (same host, same port, both TCP), this should not be returned by GetAlternativeServices(), because there is always an HttpStreamFactoryImpl::Job launched for the origin.

BUG=392576

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

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

4 files changed, 43 insertions, 4 deletions

diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index f05bcdc1..8fbc13d 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -9127,7 +9127,7 @@ TEST_P(HttpNetworkTransactionTest,
 
   base::WeakPtr<HttpServerProperties> http_server_properties =
       session->http_server_properties();
-  const int kUnrestrictedAlternatePort = 1024;
+  const int kUnrestrictedAlternatePort = 1025;
   AlternativeService alternative_service(
       AlternateProtocolFromNextProto(GetParam()), "www.example.org",
       kUnrestrictedAlternatePort);
diff --git a/net/http/http_server_properties_impl.cc b/net/http/http_server_properties_impl.cc
index d1c259e..a7030f8 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -287,6 +287,14 @@ AlternativeServiceVector HttpServerPropertiesImpl::GetAlternativeServices(
       if (alternative_service.host.empty()) {
         alternative_service.host = origin.host();
       }
+      // If the alternative service is equivalent to the origin (same host, same
+      // port, and both TCP), then there is already a Job for it, so do not
+      // return it here.
+      if (origin.Equals(alternative_service.host_port_pair()) &&
+          NPN_SPDY_MINIMUM_VERSION <= alternative_service.protocol &&
+          alternative_service.protocol <= NPN_SPDY_MAXIMUM_VERSION) {
+        continue;
+      }
       alternative_services_above_threshold.push_back(alternative_service);
     }
     return alternative_services_above_threshold;
diff --git a/net/http/http_server_properties_impl_unittest.cc b/net/http/http_server_properties_impl_unittest.cc
index 23cb395..a176845 100644
--- a/net/http/http_server_properties_impl_unittest.cc
+++ b/net/http/http_server_properties_impl_unittest.cc
@@ -269,6 +269,37 @@ TEST_F(AlternateProtocolServerPropertiesTest, Basic) {
   EXPECT_FALSE(HasAlternativeService(test_host_port_pair));
 }
 
+TEST_F(AlternateProtocolServerPropertiesTest, ExcludeOrigin) {
+  AlternativeServiceInfoVector alternative_service_info_vector;
+  // Same hostname, same port, TCP: should be ignored.
+  AlternativeService alternative_service1(NPN_HTTP_2, "foo", 443);
+  alternative_service_info_vector.push_back(
+      AlternativeServiceInfo(alternative_service1, 1.0));
+  // Different hostname: GetAlternativeServices should return this one.
+  AlternativeService alternative_service2(NPN_HTTP_2, "bar", 443);
+  alternative_service_info_vector.push_back(
+      AlternativeServiceInfo(alternative_service2, 1.0));
+  // Different port: GetAlternativeServices should return this one too.
+  AlternativeService alternative_service3(NPN_HTTP_2, "foo", 80);
+  alternative_service_info_vector.push_back(
+      AlternativeServiceInfo(alternative_service3, 1.0));
+  // QUIC: GetAlternativeServices should return this one too.
+  AlternativeService alternative_service4(QUIC, "foo", 443);
+  alternative_service_info_vector.push_back(
+      AlternativeServiceInfo(alternative_service4, 1.0));
+
+  HostPortPair test_host_port_pair("foo", 443);
+  impl_.SetAlternativeServices(test_host_port_pair,
+                               alternative_service_info_vector);
+
+  const AlternativeServiceVector alternative_service_vector =
+      impl_.GetAlternativeServices(test_host_port_pair);
+  ASSERT_EQ(3u, alternative_service_vector.size());
+  EXPECT_EQ(alternative_service2, alternative_service_vector[0]);
+  EXPECT_EQ(alternative_service3, alternative_service_vector[1]);
+  EXPECT_EQ(alternative_service4, alternative_service_vector[2]);
+}
+
 TEST_F(AlternateProtocolServerPropertiesTest, DefaultProbabilityExcluded) {
   HostPortPair test_host_port_pair("foo", 80);
   const AlternativeService alternative_service(NPN_HTTP_2, "foo", 443);
diff --git a/net/http/http_server_properties_manager_unittest.cc b/net/http/http_server_properties_manager_unittest.cc
index dbaedac..c1eb8d0 100644
--- a/net/http/http_server_properties_manager_unittest.cc
+++ b/net/http/http_server_properties_manager_unittest.cc
@@ -704,7 +704,7 @@ TEST_F(HttpServerPropertiesManagerTest, Clear) {
 
   HostPortPair spdy_server_mail("mail.google.com", 443);
   http_server_props_manager_->SetSupportsSpdy(spdy_server_mail, true);
-  AlternativeService alternative_service(NPN_HTTP_2, "mail.google.com", 443);
+  AlternativeService alternative_service(NPN_HTTP_2, "mail.google.com", 1234);
   http_server_props_manager_->SetAlternativeService(spdy_server_mail,
                                                     alternative_service, 1.0);
   IPAddressNumber actual_address;
@@ -776,7 +776,7 @@ TEST_F(HttpServerPropertiesManagerTest, BadSupportsQuic) {
   for (int i = 0; i < 200; ++i) {
     // Set up alternative_service for www.google.com:i.
     base::DictionaryValue* alternative_service_dict = new base::DictionaryValue;
-    alternative_service_dict->SetString("protocol_str", "npn-h2");
+    alternative_service_dict->SetString("protocol_str", "quic");
     alternative_service_dict->SetInteger("port", i);
     base::ListValue* alternative_service_list = new base::ListValue;
     alternative_service_list->Append(alternative_service_dict);
@@ -820,7 +820,7 @@ TEST_F(HttpServerPropertiesManagerTest, BadSupportsQuic) {
         http_server_props_manager_->GetAlternativeServices(
             HostPortPair::FromString(server));
     ASSERT_EQ(1u, alternative_service_vector.size());
-    EXPECT_EQ(NPN_HTTP_2, alternative_service_vector[0].protocol);
+    EXPECT_EQ(QUIC, alternative_service_vector[0].protocol);
     EXPECT_EQ(i, alternative_service_vector[0].port);
   }