Change the AlternativeServiceMap with SchemeOriginPair key.

net/http/http_server_properties_impl.cc

Index: net/http/http_server_properties_impl.cc
diff --git a/net/http/http_server_properties_impl.cc b/net/http/http_server_properties_impl.cc
index ab2088f0f65d6d321dfeaa812444260031c70444..d946f05e07d2131befaa4e9e11807c0f673068e8 100644
--- a/net/http/http_server_properties_impl.cc
+++ b/net/http/http_server_properties_impl.cc
@@ -113,8 +113,8 @@ void HttpServerPropertiesImpl::InitializeAlternativeServiceServers(
       HostPortPair canonical_host(canonical_suffix, canonical_ports[j]);
       // If we already have a valid canonical server, we're done.
       if (ContainsKey(canonical_host_to_origin_map_, canonical_host) &&
-          (alternative_service_map_.Peek(
-               canonical_host_to_origin_map_[canonical_host]) !=
+          (alternative_service_map_.Peek(SchemeOriginPair(
+               "https", canonical_host_to_origin_map_[canonical_host])) !=

[Ryan Hamilton, 2016/03/24 22:23:14]

If we want canoncical suffixes to only apply to HTTPS (which seems reasonable)
it'd be good to add a comment to that effect. Should also remove port 80 on line
109 (and avoid the loop).

[Zhongyi Shi, 2016/04/04 18:58:36]

Done.

            alternative_service_map_.end())) {
         continue;
       }
@@ -125,7 +125,8 @@ void HttpServerPropertiesImpl::InitializeAlternativeServiceServers(
            it != alternative_service_map_.end(); ++it) {
         if (base::EndsWith(it->first.host(), canonical_suffixes_[i],
                            base::CompareCase::INSENSITIVE_ASCII)) {
-          canonical_host_to_origin_map_[canonical_host] = it->first;
+          canonical_host_to_origin_map_[canonical_host] =
+              it->first.host_port_pair();
           break;
         }
       }
@@ -235,16 +236,16 @@ void HttpServerPropertiesImpl::Clear() {
 }
 
 bool HttpServerPropertiesImpl::SupportsRequestPriority(
-    const HostPortPair& host_port_pair) {
+    const SchemeOriginPair& server) {
   DCHECK(CalledOnValidThread());
-  if (host_port_pair.host().empty())
+  if (server.host().empty())
     return false;
 
-  if (GetSupportsSpdy(host_port_pair))
+  if (GetSupportsSpdy(server.host_port_pair()))
     return true;
 
   const AlternativeServiceVector alternative_service_vector =
-      GetAlternativeServices(host_port_pair);
+      GetAlternativeServices(server);
   for (const AlternativeService& alternative_service :
        alternative_service_vector) {
     if (alternative_service.protocol == QUIC) {
@@ -322,7 +323,7 @@ std::string HttpServerPropertiesImpl::GetCanonicalSuffix(
 }
 
 AlternativeServiceVector HttpServerPropertiesImpl::GetAlternativeServices(
-    const HostPortPair& origin) {
+    const SchemeOriginPair& origin) {
   // Copy alternative services with probability greater than or equal to the
   // threshold into |alternative_services_above_threshold|.
   AlternativeServiceVector alternative_services_above_threshold;
@@ -347,7 +348,9 @@ AlternativeServiceVector HttpServerPropertiesImpl::GetAlternativeServices(
       // 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()) &&
+      if (origin.scheme() == "https" &&

[Ryan Hamilton, 2016/03/24 22:23:14]

I don't understand this check. Can you say more?

[Zhongyi Shi, 2016/04/04 18:58:35]

I thought we want to check whether the alt-svc is equivalent to origin. But, I
think you're right, we are actually checking whether there's a job already
running. So no need to check the origin's scheme. 

+          origin.host_port_pair().Equals(
+              alternative_service.host_port_pair()) &&
           NPN_SPDY_MINIMUM_VERSION <= alternative_service.protocol &&
           alternative_service.protocol <= NPN_SPDY_MAXIMUM_VERSION) {
         ++it;
@@ -362,11 +365,16 @@ AlternativeServiceVector HttpServerPropertiesImpl::GetAlternativeServices(
     return alternative_services_above_threshold;
   }
 
-  CanonicalHostMap::const_iterator canonical = GetCanonicalHost(origin);
+  if (origin.scheme() != "https")

[Ryan Hamilton, 2016/03/24 22:23:14]

Comments.

[Zhongyi Shi, 2016/04/04 18:58:35]

Changing the CanonicalHostMap to be <SHP, SHP>, no need to check origin's scheme
anymore :D

+    return AlternativeServiceVector();
+
+  CanonicalHostMap::const_iterator canonical =
+      GetCanonicalHost(origin.host_port_pair());
   if (canonical == canonical_host_to_origin_map_.end()) {
     return AlternativeServiceVector();
   }
-  map_it = alternative_service_map_.Get(canonical->second);
+  map_it = alternative_service_map_.Get(
+      SchemeOriginPair("https", canonical->second));

[Ryan Hamilton, 2016/03/24 22:23:14]

can you use origin.scheme() instead?

[Zhongyi Shi, 2016/04/04 18:58:35]

Acknowledged.

   if (map_it == alternative_service_map_.end()) {
     return AlternativeServiceVector();
   }
@@ -402,7 +410,7 @@ AlternativeServiceVector HttpServerPropertiesImpl::GetAlternativeServices(
 }
 
 bool HttpServerPropertiesImpl::SetAlternativeService(
-    const HostPortPair& origin,
+    const SchemeOriginPair& origin,
     const AlternativeService& alternative_service,
     double alternative_probability,
     base::Time expiration) {
@@ -414,7 +422,7 @@ bool HttpServerPropertiesImpl::SetAlternativeService(
 }
 
 bool HttpServerPropertiesImpl::SetAlternativeServices(
-    const HostPortPair& origin,
+    const SchemeOriginPair& origin,
     const AlternativeServiceInfoVector& alternative_service_info_vector) {
   AlternativeServiceMap::iterator it = alternative_service_map_.Peek(origin);
 
@@ -452,10 +460,11 @@ bool HttpServerPropertiesImpl::SetAlternativeServices(
   // canonical host.
   for (size_t i = 0; i < canonical_suffixes_.size(); ++i) {
     std::string canonical_suffix = canonical_suffixes_[i];
-    if (base::EndsWith(origin.host(), canonical_suffixes_[i],
+    if (origin.scheme() == "https" &&

[Ryan Hamilton, 2016/03/24 22:23:14]

comments.

[Zhongyi Shi, 2016/04/04 18:58:35]

Done.

+        base::EndsWith(origin.host(), canonical_suffixes_[i],
                        base::CompareCase::INSENSITIVE_ASCII)) {
       HostPortPair canonical_host(canonical_suffix, origin.port());
-      canonical_host_to_origin_map_[canonical_host] = origin;
+      canonical_host_to_origin_map_[canonical_host] = origin.host_port_pair();
       break;
     }
   }
@@ -519,8 +528,9 @@ void HttpServerPropertiesImpl::ConfirmAlternativeService(
 }
 
 void HttpServerPropertiesImpl::ClearAlternativeServices(
-    const HostPortPair& origin) {
-  RemoveCanonicalHost(origin);
+    const SchemeOriginPair& origin) {
+  if (origin.scheme() == "https")

[Ryan Hamilton, 2016/03/24 22:23:14]

comments.

[Zhongyi Shi, 2016/04/04 18:58:35]

Acknowledged.

+    RemoveCanonicalHost(origin.host_port_pair());
 
   AlternativeServiceMap::iterator it = alternative_service_map_.Peek(origin);
   if (it == alternative_service_map_.end()) {
@@ -540,7 +550,8 @@ HttpServerPropertiesImpl::GetAlternativeServiceInfoAsValue()
   scoped_ptr<base::ListValue> dict_list(new base::ListValue);
   for (const auto& alternative_service_map_item : alternative_service_map_) {
     scoped_ptr<base::ListValue> alternative_service_list(new base::ListValue);
-    const HostPortPair& host_port_pair = alternative_service_map_item.first;
+    const HostPortPair& host_port_pair =
+        alternative_service_map_item.first.host_port_pair();
     for (const AlternativeServiceInfo& alternative_service_info :
          alternative_service_map_item.second) {
       std::string alternative_service_string(
@@ -704,19 +715,24 @@ void HttpServerPropertiesImpl::SetAlternativeServiceProbabilityThreshold(
 
 AlternativeServiceMap::const_iterator
 HttpServerPropertiesImpl::GetAlternateProtocolIterator(
-    const HostPortPair& server) {
+    const SchemeOriginPair& server) {
   AlternativeServiceMap::const_iterator it =
       alternative_service_map_.Get(server);
   if (it != alternative_service_map_.end())
     return it;
 
-  CanonicalHostMap::const_iterator canonical = GetCanonicalHost(server);
+  if (server.scheme() != "https")
+    return alternative_service_map_.end();
+
+  CanonicalHostMap::const_iterator canonical =
+      GetCanonicalHost(server.host_port_pair());
   if (canonical == canonical_host_to_origin_map_.end()) {
     return alternative_service_map_.end();
   }
 
   const HostPortPair canonical_host_port = canonical->second;
-  it = alternative_service_map_.Get(canonical_host_port);
+  it = alternative_service_map_.Get(
+      SchemeOriginPair("https", canonical_host_port));
   if (it == alternative_service_map_.end()) {
     return alternative_service_map_.end();
   }
@@ -797,7 +813,7 @@ void HttpServerPropertiesImpl::ExpireBrokenAlternateProtocolMappings() {
       // from both |canonical_host_to_origin_map_| and
       // |alternative_service_map_|.
       if (map_it->second.empty()) {
-        RemoveCanonicalHost(map_it->first);
+        RemoveCanonicalHost(map_it->first.host_port_pair());
         map_it = alternative_service_map_.Erase(map_it);
         continue;
       }