Implement multiple alternative services per origin.

net/http/http_server_properties_manager.cc

 // Copyright 2014 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_server_properties_manager.h"
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/prefs/pref_service.h"
 #include "base/single_thread_task_runner.h"
@@ skipping 172 matching lines @@
   http_server_properties_impl_->SetHTTP11Required(server);
   ScheduleUpdatePrefsOnNetworkThread(HTTP_11_REQUIRED);
 }
 
 void HttpServerPropertiesManager::MaybeForceHTTP11(const HostPortPair& server,
                                                    SSLConfig* ssl_config) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   http_server_properties_impl_->MaybeForceHTTP11(server, ssl_config);
 }
 
-AlternativeService HttpServerPropertiesManager::GetAlternativeService(
+AlternativeServiceVector HttpServerPropertiesManager::GetAlternativeServices(
     const HostPortPair& origin) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
-  return http_server_properties_impl_->GetAlternativeService(origin);
+  return http_server_properties_impl_->GetAlternativeServices(origin);
 }
 
-void HttpServerPropertiesManager::SetAlternativeService(
+void HttpServerPropertiesManager::AddAlternativeService(
     const HostPortPair& origin,
     const AlternativeService& alternative_service,
     double alternative_probability) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
-  AlternativeService old_alternative_service = GetAlternativeService(origin);
-  http_server_properties_impl_->SetAlternativeService(
+  http_server_properties_impl_->AddAlternativeService(
       origin, alternative_service, alternative_probability);
-  AlternativeService new_alternative_service = GetAlternativeService(origin);
-  // If |alternative_probability| was above the threashold now it is below or
-  // vice versa, then a different alternative_service will be returned from the
-  // old and if so, then persist.
-  if (old_alternative_service != new_alternative_service)
-    ScheduleUpdatePrefsOnNetworkThread(SET_ALTERNATIVE_SERVICE);

[Ryan Hamilton, 2015/06/29 21:15:52]

You seem to have removed the conditional here of not scheduling an update if
nothing changed. Was that intentional? Raman added this recently because
otherwise we were doing a noticeable amount of extra work when nothing had
changed. I think we perhaps need to move from Clear + Set/Add to a single
SetAlternateServices() so that this method can avoid the update. What do you
think?

[Bence, 2015/06/30 19:16:14]

I think SetAlternativeServices is a good idea.  It might make tests more
complicated, but is aligned with production use.  Done.

Note, however, that if the server keeps advertising the exact same alternative
services, this only cuts updates in half.  So I modified the return types to
bool.  This way alternative services are only updated if they really change.

[Ryan Hamilton, 2015/07/06 17:48:09]

I'm not sure that I understand. If the output of GetAlternativeServices has not
change, we shouldn't need to persist, right? And if it has changed we should
persist, I would think. Can you expand on your "only cuts updates in half"
comment?

[Bence, 2015/07/07 09:55:55]

There might be situations when the output of GetAlternativeServices does not
change but we need to persist.  For example, the server advertises an
alternative service with a probability below threshold, so it is hidden by
GetAlternativeServices, but might be important for the next browser session.  Or
later I might change GetAlternativeServices not to return broken alternative
services.  What matters instead is |alternative_service_map_|.  Therefore I
modified HttpServerPropertiesImpl::SetAlternativeServices to return whether the
|alternative_service_map_| entry for |origin| has actually changed, so that
HttpServerProperiesManager::SetAlternativeServices only triggers an update if
necessary.
Without detecting change in HttpServerPropertiesImpl::SetAlternativeServices,
replacing ClearAlternativeServices and AddAlternativeServices by
SetAlternativeServices (two methods by one) would mean one update triggered
instead of two every time the origin sends alternative service information.  I
didn't find this improvement satisfactory, so I modified
HttpServerPropertiesImpl::SetAlternativeServices to detect if persisting
settings is actually needed.

+  ScheduleUpdatePrefsOnNetworkThread(SET_ALTERNATIVE_SERVICE);
 }
 
 void HttpServerPropertiesManager::MarkAlternativeServiceBroken(
     const AlternativeService& alternative_service) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   http_server_properties_impl_->MarkAlternativeServiceBroken(
       alternative_service);
   ScheduleUpdatePrefsOnNetworkThread(MARK_ALTERNATIVE_SERVICE_BROKEN);
 }
 
@@ skipping 26 matching lines @@
       alternative_service);
   http_server_properties_impl_->ConfirmAlternativeService(alternative_service);
   bool new_value = http_server_properties_impl_->IsAlternativeServiceBroken(
       alternative_service);
   // For persisting, we only care about the value returned by
   // IsAlternativeServiceBroken. If that value changes, then call persist.
   if (old_value != new_value)
     ScheduleUpdatePrefsOnNetworkThread(CONFIRM_ALTERNATIVE_SERVICE);
 }
 
-void HttpServerPropertiesManager::ClearAlternativeService(
+void HttpServerPropertiesManager::ClearAlternativeServices(
     const HostPortPair& origin) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   const AlternativeServiceMap& map =
       http_server_properties_impl_->alternative_service_map();
   size_t old_size = map.size();
-  http_server_properties_impl_->ClearAlternativeService(origin);
+  http_server_properties_impl_->ClearAlternativeServices(origin);
   size_t new_size = map.size();
   // Persist only if we have deleted an entry.
   if (old_size != new_size)
     ScheduleUpdatePrefsOnNetworkThread(CLEAR_ALTERNATIVE_SERVICE);
 }
 
 const AlternativeServiceMap&
 HttpServerPropertiesManager::alternative_service_map() const {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   return http_server_properties_impl_->alternative_service_map();
@@ skipping 290 matching lines @@
 
 bool HttpServerPropertiesManager::AddToAlternativeServiceMap(
     const HostPortPair& server,
     const base::DictionaryValue& server_pref_dict,
     AlternativeServiceMap* alternative_service_map) {
   DCHECK(alternative_service_map->Peek(server) ==
          alternative_service_map->end());
   // Get alternative_services...
   const base::ListValue* alternative_service_list;
   const base::DictionaryValue* alternative_service_dict;
-  AlternativeServiceInfo alternative_service_info;
+  AlternativeServiceInfoVector alternative_service_info_vector;
   if (server_pref_dict.GetListWithoutPathExpansion(kAlternativeServiceKey,
                                                    &alternative_service_list)) {
-    if (alternative_service_list->empty()) {
-      return false;
+    for (const base::Value* alternative_service_list_item :
+         *alternative_service_list) {
+      if (!alternative_service_list_item->GetAsDictionary(
+              &alternative_service_dict))
+        return false;
+      AlternativeServiceInfo alternative_service_info =
+          ParseAlternativeServiceDict(*alternative_service_dict,
+                                      server.ToString());
+      if (alternative_service_info.alternative_service.protocol ==
+          UNINITIALIZED_ALTERNATE_PROTOCOL) {
+        return false;
+      }
+      alternative_service_info_vector.push_back(alternative_service_info);
     }
-    // Get first element of the list.
-    // TODO(bnc): Once we store multiple AlternativeServiceInfo per server, read
-    // all of them.
-    if (!alternative_service_list->GetDictionary(0,
-                                                 &alternative_service_dict)) {
-      return false;
-    }
-    alternative_service_info = ParseAlternativeServiceDict(
-        *alternative_service_dict, server.ToString());
   } else {
     // ...or alternate_protocol.
     // TODO(bnc): Remove this in M46, we do not need preference migration for
     // long.
     if (!server_pref_dict.GetDictionaryWithoutPathExpansion(
             kAlternateProtocolKey, &alternative_service_dict)) {
       return true;
     }
-    alternative_service_info = ParseAlternativeServiceDict(
-        *alternative_service_dict, server.ToString());
+    AlternativeServiceInfo alternative_service_info =
+        ParseAlternativeServiceDict(*alternative_service_dict,
+                                    server.ToString());
+    if (alternative_service_info.alternative_service.protocol ==
+        UNINITIALIZED_ALTERNATE_PROTOCOL) {
+      return false;
+    }
+    alternative_service_info_vector.push_back(alternative_service_info);
   }
 
-  if (alternative_service_info.alternative_service.protocol ==
-      UNINITIALIZED_ALTERNATE_PROTOCOL) {
+  if (alternative_service_info_vector.empty()) {
     return false;
   }
-  alternative_service_map->Put(server, alternative_service_info);
+
+  alternative_service_map->Put(server, alternative_service_info_vector);
   return true;
 }
 
 bool HttpServerPropertiesManager::ReadSupportsQuic(
     const base::DictionaryValue& http_server_properties_dict,
     IPAddressNumber* last_quic_address) {
   const base::DictionaryValue* supports_quic_dict = NULL;
   if (!http_server_properties_dict.GetDictionaryWithoutPathExpansion(
           kSupportsQuicKey, &supports_quic_dict)) {
     return true;
@@ skipping 54 matching lines @@
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
 
   UMA_HISTOGRAM_COUNTS("Net.CountOfSpdyServers", spdy_servers->size());
   http_server_properties_impl_->InitializeSpdyServers(spdy_servers, true);
 
   // Update the cached data and use the new spdy_settings from preferences.
   UMA_HISTOGRAM_COUNTS("Net.CountOfSpdySettings", spdy_settings_map->size());
   http_server_properties_impl_->InitializeSpdySettingsServers(
       spdy_settings_map);
 
-  // Update the cached data and use the new Alternate-Protocol server list from
+  // Update the cached data and use the new alternative service list from
   // preferences.
   UMA_HISTOGRAM_COUNTS("Net.CountOfAlternateProtocolServers",
                        alternative_service_map->size());
   http_server_properties_impl_->InitializeAlternativeServiceServers(
       alternative_service_map);
 
   http_server_properties_impl_->InitializeSupportsQuic(last_quic_address);
 
   http_server_properties_impl_->InitializeServerNetworkStats(
       server_network_stats_map);
@@ skipping 55 matching lines @@
 
   AlternativeServiceMap* alternative_service_map =
       new AlternativeServiceMap(kMaxAlternateProtocolHostsToPersist);
   const AlternativeServiceMap& map =
       http_server_properties_impl_->alternative_service_map();
   count = 0;
   typedef std::map<std::string, bool> CanonicalHostPersistedMap;
   CanonicalHostPersistedMap persisted_map;
   for (AlternativeServiceMap::const_iterator it = map.begin();
        it != map.end() && count < kMaxAlternateProtocolHostsToPersist; ++it) {
-    const AlternativeServiceInfo& alternative_service_info = it->second;
-    if (!IsAlternateProtocolValid(
-            alternative_service_info.alternative_service.protocol)) {
+    const HostPortPair& server = it->first;
+    AlternativeServiceInfoVector notbroken_alternative_service_info_vector;
+    for (const AlternativeServiceInfo& alternative_service_info : it->second) {
+      if (!IsAlternateProtocolValid(
+              alternative_service_info.alternative_service.protocol)) {
+        continue;
+      }
+      AlternativeService alternative_service(
+          alternative_service_info.alternative_service);
+      if (alternative_service.host.empty()) {
+        alternative_service.host = server.host();
+      }
+      if (IsAlternativeServiceBroken(alternative_service)) {
+        continue;
+      }
+      notbroken_alternative_service_info_vector.push_back(
+          alternative_service_info);
+    }
+    if (notbroken_alternative_service_info_vector.empty()) {
       continue;
     }
-    const HostPortPair& server = it->first;
-    AlternativeService alternative_service(
-        alternative_service_info.alternative_service);
-    if (alternative_service.host.empty()) {
-      alternative_service.host = server.host();
-    }
-    if (IsAlternativeServiceBroken(alternative_service)) {
-      continue;
-    }
+    alternative_service_map->Put(server,
+                                 notbroken_alternative_service_info_vector);
     std::string canonical_suffix =
         http_server_properties_impl_->GetCanonicalSuffix(server.host());
     if (!canonical_suffix.empty()) {
       if (persisted_map.find(canonical_suffix) != persisted_map.end())
         continue;
       persisted_map[canonical_suffix] = true;
     }
-    alternative_service_map->Put(server, alternative_service_info);
     ++count;
   }
 
   ServerNetworkStatsMap* server_network_stats_map =
       new ServerNetworkStatsMap(kMaxServerNetworkStatsHostsToPersist);
   const ServerNetworkStatsMap& main_server_network_stats_map =
       http_server_properties_impl_->server_network_stats_map();
   for (ServerNetworkStatsMap::const_iterator it =
            main_server_network_stats_map.begin();
        it != main_server_network_stats_map.end(); ++it) {
     server_network_stats_map->Put(it->first, it->second);
   }
 
   IPAddressNumber* last_quic_addr = new IPAddressNumber;
   http_server_properties_impl_->GetSupportsQuic(last_quic_addr);
   // Update the preferences on the pref thread.
   pref_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(
           &HttpServerPropertiesManager::UpdatePrefsOnPrefThread, pref_weak_ptr_,
           base::Owned(spdy_server_list), base::Owned(spdy_settings_map),
           base::Owned(alternative_service_map), base::Owned(last_quic_addr),
           base::Owned(server_network_stats_map), completion));
 }
 
 // A local or temporary data structure to hold |supports_spdy|, SpdySettings,
-// AlternativeServiceInfo and SupportsQuic preferences for a server. This is
-// used only in UpdatePrefsOnPrefThread.
+// AlternativeServiceInfoVector, and SupportsQuic preferences for a server. This
+// is used only in UpdatePrefsOnPrefThread.
 struct ServerPref {
   ServerPref()
       : supports_spdy(false),
         settings_map(NULL),
-        alternative_service(NULL),
+        alternative_service_info_vector(NULL),
         supports_quic(NULL),
         server_network_stats(NULL) {}
-  ServerPref(bool supports_spdy,
-             const SettingsMap* settings_map,
-             const AlternativeServiceInfo* alternative_service,
-             const SupportsQuic* supports_quic,
-             const ServerNetworkStats* server_network_stats)
+  ServerPref(
+      bool supports_spdy,
+      const SettingsMap* settings_map,
+      const AlternativeServiceInfoVector* alternative_service_info_vector,
+      const SupportsQuic* supports_quic,
+      const ServerNetworkStats* server_network_stats)
       : supports_spdy(supports_spdy),
         settings_map(settings_map),
-        alternative_service(alternative_service),
+        alternative_service_info_vector(alternative_service_info_vector),
         supports_quic(supports_quic),
         server_network_stats(server_network_stats) {}
   bool supports_spdy;
   const SettingsMap* settings_map;
-  const AlternativeServiceInfo* alternative_service;
+  const AlternativeServiceInfoVector* alternative_service_info_vector;
   const SupportsQuic* supports_quic;
   const ServerNetworkStats* server_network_stats;
 };
 
 void HttpServerPropertiesManager::UpdatePrefsOnPrefThread(
     base::ListValue* spdy_server_list,
     SpdySettingsMap* spdy_settings_map,
     AlternativeServiceMap* alternative_service_map,
     IPAddressNumber* last_quic_address,
     ServerNetworkStatsMap* server_network_stats_map,
@@ skipping 14 matching lines @@
     }
   }
 
   // Add servers that have SpdySettings to server_pref_map.
   for (SpdySettingsMap::iterator map_it = spdy_settings_map->begin();
        map_it != spdy_settings_map->end(); ++map_it) {
     const HostPortPair& server = map_it->first;
     server_pref_map[server].settings_map = &map_it->second;
   }
 
-  // Add AlternateProtocol servers to server_pref_map.
+  // Add alternative services to server_pref_map.
   for (AlternativeServiceMap::const_iterator map_it =
            alternative_service_map->begin();
        map_it != alternative_service_map->end(); ++map_it) {
-    server_pref_map[map_it->first].alternative_service = &map_it->second;
+    server_pref_map[map_it->first].alternative_service_info_vector =
+        &map_it->second;
   }
 
   // Add ServerNetworkStats servers to server_pref_map.
   for (ServerNetworkStatsMap::const_iterator map_it =
            server_network_stats_map->begin();
        map_it != server_network_stats_map->end(); ++map_it) {
     const HostPortPair& server = map_it->first;
     server_pref_map[server].server_network_stats = &map_it->second;
   }
 
   // Persist properties to the |path_|.
   base::DictionaryValue http_server_properties_dict;
   base::DictionaryValue* servers_dict = new base::DictionaryValue;
   for (ServerPrefMap::const_iterator map_it = server_pref_map.begin();
        map_it != server_pref_map.end();
        ++map_it) {
     const HostPortPair& server = map_it->first;
     const ServerPref& server_pref = map_it->second;
 
     base::DictionaryValue* server_pref_dict = new base::DictionaryValue;
 
     // Save supports_spdy.
     if (server_pref.supports_spdy)
       server_pref_dict->SetBoolean(kSupportsSpdyKey, server_pref.supports_spdy);
     SaveSpdySettingsToServerPrefs(server_pref.settings_map, server_pref_dict);
-    SaveAlternativeServiceToServerPrefs(server_pref.alternative_service,
-                                        server_pref_dict);
+    SaveAlternativeServiceToServerPrefs(
+        server_pref.alternative_service_info_vector, server_pref_dict);
     SaveNetworkStatsToServerPrefs(server_pref.server_network_stats,
                                   server_pref_dict);
 
     servers_dict->SetWithoutPathExpansion(server.ToString(), server_pref_dict);
   }
 
   http_server_properties_dict.SetWithoutPathExpansion(kServersKey,
                                                       servers_dict);
   SetVersion(&http_server_properties_dict, kVersionNumber);
 
@@ skipping 22 matching lines @@
        it != settings_map->end(); ++it) {
     SpdySettingsIds id = it->first;
     uint32 value = it->second.second;
     std::string key = base::StringPrintf("%u", id);
     spdy_settings_dict->SetInteger(key, value);
   }
   server_pref_dict->SetWithoutPathExpansion(kSettingsKey, spdy_settings_dict);
 }
 
 void HttpServerPropertiesManager::SaveAlternativeServiceToServerPrefs(
-    const AlternativeServiceInfo* alternative_service_info,
+    const AlternativeServiceInfoVector* alternative_service_info_vector,
     base::DictionaryValue* server_pref_dict) {
-  if (!alternative_service_info)
+  if (!alternative_service_info_vector ||
+      alternative_service_info_vector->empty()) {
     return;
-
-  const AlternativeService& alternative_service =
-      alternative_service_info->alternative_service;
-  base::DictionaryValue* alternative_service_info_dict =
-      new base::DictionaryValue;
-  alternative_service_info_dict->SetString(
-      kProtocolKey, AlternateProtocolToString(alternative_service.protocol));
-  if (!alternative_service.host.empty()) {
-    alternative_service_info_dict->SetString(kHostKey,
-                                             alternative_service.host);
   }
-  alternative_service_info_dict->SetInteger(kPortKey, alternative_service.port);
-  alternative_service_info_dict->SetDouble(
-      kProbabilityKey, alternative_service_info->probability);
-
-  // Create a single element list here.
-  // TODO(bnc): Once we store multiple AlternativeServiceInfo per server, save
-  // all of them.
-  base::ListValue* alternative_service_list = new base::ListValue();
-  alternative_service_list->Append(alternative_service_info_dict);
+  scoped_ptr<base::ListValue> alternative_service_list(new base::ListValue);
+  for (const AlternativeServiceInfo& alternative_service_info :
+       *alternative_service_info_vector) {
+    const AlternativeService alternative_service =
+        alternative_service_info.alternative_service;
+    DCHECK(IsAlternateProtocolValid(alternative_service.protocol));
+    base::DictionaryValue* alternative_service_dict = new base::DictionaryValue;
+    alternative_service_dict->SetInteger(kPortKey, alternative_service.port);
+    if (!alternative_service.host.empty()) {
+      alternative_service_dict->SetString(kHostKey, alternative_service.host);
+    }
+    alternative_service_dict->SetString(
+        kProtocolKey, AlternateProtocolToString(alternative_service.protocol));
+    alternative_service_dict->SetDouble(kProbabilityKey,
+                                        alternative_service_info.probability);
+    alternative_service_list->Append(alternative_service_dict);
+  }
+  if (alternative_service_list->GetSize() == 0)
+    return;
   server_pref_dict->SetWithoutPathExpansion(kAlternativeServiceKey,
-                                            alternative_service_list);
+                                            alternative_service_list.release());
 }
 
 void HttpServerPropertiesManager::SaveSupportsQuicToPrefs(
     const IPAddressNumber* last_quic_address,
     base::DictionaryValue* http_server_properties_dict) {
   if (!last_quic_address || last_quic_address->empty())
     return;
 
   base::DictionaryValue* supports_quic_dict = new base::DictionaryValue;
   supports_quic_dict->SetBoolean(kUsedQuicKey, true);
@@ skipping 19 matching lines @@
                                             server_network_stats_dict);
 }
 
 void HttpServerPropertiesManager::OnHttpServerPropertiesChanged() {
   DCHECK(pref_task_runner_->RunsTasksOnCurrentThread());
   if (!setting_prefs_)
     ScheduleUpdateCacheOnPrefThread();
 }
 
 }  // namespace net