Update param types of HttpServerPropertiesImpl setters and getters. Fix MRU order when loading

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 <utility>
 
 #include "base/bind.h"
 #include "base/memory/ptr_util.h"
@@ skipping 457 matching lines @@
     //          ...
     //      ], ...
     // },
     if (!http_server_properties_dict.GetListWithoutPathExpansion(
             kServersKey, &servers_list)) {
       DVLOG(1) << "Malformed http_server_properties for servers list.";
       return;
     }
   }
 
-  IPAddress* addr = new IPAddress;
-  ReadSupportsQuic(http_server_properties_dict, addr);
+  std::unique_ptr<IPAddress> addr = base::MakeUnique<IPAddress>();
+  ReadSupportsQuic(http_server_properties_dict, addr.get());
 
   // String is "scheme://host:port" tuple of spdy server.
-  std::unique_ptr<ServerList> spdy_servers(new ServerList);
+  std::unique_ptr<SpdyServersMap> spdy_servers_map(
+      new SpdyServersMap(SpdyServersMap::NO_AUTO_EVICT));
   std::unique_ptr<AlternativeServiceMap> alternative_service_map(
       new AlternativeServiceMap(kMaxAlternateProtocolHostsToPersist));
   std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map(
       new ServerNetworkStatsMap(kMaxServerNetworkStatsHostsToPersist));
   std::unique_ptr<QuicServerInfoMap> quic_server_info_map(
       new QuicServerInfoMap(QuicServerInfoMap::NO_AUTO_EVICT));
 
   if (version < 4) {
-    if (!AddServersData(*servers_dict, spdy_servers.get(),
+    if (!AddServersData(*servers_dict, spdy_servers_map.get(),
                         alternative_service_map.get(),
                         server_network_stats_map.get(), version)) {
       detected_corrupted_prefs = true;
     }
   } else {
-    for (base::ListValue::const_iterator it = servers_list->begin();
-         it != servers_list->end(); ++it) {
+    /*for (base::ListValue::const_iterator it = servers_list->begin();
+         it != servers_list->end(); ++it) {*/

[Ryan Hamilton, 2017/06/22 02:45:52]

Remove?

[wangyix1, 2017/06/22 17:45:09]

Done.

+    for (base::ListValue::const_iterator it = servers_list->end();
+         it != servers_list->begin();) {

[Ryan Hamilton, 2017/06/22 02:45:52]

This appears to iterate in a different order than before. Does that represent a
behavior change?

[wangyix1, 2017/06/22 17:45:09]

No behavior change. This change is a result of Impl::SetSpdyServers() taking a
SpdyServersMap directly instead of a vector of servers. Before, the code did
this:

* In UpdateCacheFromPrefsOnPrefSequence(), |servers_list| iterated forwards,
AddServersData() called on each entry.

* AddServersData() appends entry to |spdy_servers|, which is a vector<string>

* |spdy_servers| is eventually passed to Impl::SetSpdyServers().

* Impl::SetSpdyServers() internally iterates |spdy_servers| backwards and adds
each entry to a SpdyServersMap


Now, the code does this:

* In UpdateCacheFromPrefsOnPrefSequence(), |servers_list| iterated backwards,
AddServersData() called on each entry.

* AddServersData() adds entry to |spdy_servers_map|, which is an MRUCache.

* |spdy_servers_map| is eventually passed to Impl::SetSpdyServers()



In summary, the step where a list of servers is iterated backwards to convert it
into an MRUCache of servers has moved from Impl::SetSpdyServers() to
Manager::UpdateCacheFromPrefsOnPrefSequence()

 

[Ryan Hamilton, 2017/06/22 22:10:30]

Ah! Thanks for the great summary.

[wangyix1, 2017/06/22 22:11:06]

I take that back, this does have a behavior change. AddServersData() adds stuff
to other maps too. Let me look into this first.

[wangyix1, 2017/06/23 19:06:06]

Discussed with rch. This behavior change actually fixes a bug where the
alternative service map when loaded from prefs is loaded with reverse MRU order.

+      --it;
       if (!it->GetAsDictionary(&servers_dict)) {
         DVLOG(1) << "Malformed http_server_properties for servers dictionary.";
         detected_corrupted_prefs = true;
         continue;
       }
-      if (!AddServersData(*servers_dict, spdy_servers.get(),
+      if (!AddServersData(*servers_dict, spdy_servers_map.get(),
                           alternative_service_map.get(),
                           server_network_stats_map.get(), version)) {
         detected_corrupted_prefs = true;
       }
     }
   }
 
   if (!AddToQuicServerInfoMap(http_server_properties_dict,
                               quic_server_info_map.get())) {
     detected_corrupted_prefs = true;
   }
 
   network_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(
           &HttpServerPropertiesManager::UpdateCacheFromPrefsOnNetworkSequence,
-          base::Unretained(this), base::Owned(spdy_servers.release()),
-          base::Owned(alternative_service_map.release()), base::Owned(addr),
-          base::Owned(server_network_stats_map.release()),
-          base::Owned(quic_server_info_map.release()),
-          detected_corrupted_prefs));
+          base::Unretained(this), base::Passed(&spdy_servers_map),
+          base::Passed(&alternative_service_map), base::Passed(&addr),
+          base::Passed(&server_network_stats_map),
+          base::Passed(&quic_server_info_map), detected_corrupted_prefs));
 }
 
 bool HttpServerPropertiesManager::AddServersData(
     const base::DictionaryValue& servers_dict,
-    ServerList* spdy_servers,
+    SpdyServersMap* spdy_servers_map,
     AlternativeServiceMap* alternative_service_map,
     ServerNetworkStatsMap* network_stats_map,
     int version) {
   for (base::DictionaryValue::Iterator it(servers_dict); !it.IsAtEnd();
        it.Advance()) {
     // Get server's scheme/host/pair.
     const std::string& server_str = it.key();
     std::string spdy_server_url = server_str;
     if (version < 5) {
       // For old version disk data, always use HTTPS as the scheme.
       spdy_server_url.insert(0, "https://");
     }
     url::SchemeHostPort spdy_server((GURL(spdy_server_url)));
     if (spdy_server.host().empty()) {
       DVLOG(1) << "Malformed http_server_properties for server: " << server_str;
       return false;
     }
 
     const base::DictionaryValue* server_pref_dict = nullptr;
     if (!it.value().GetAsDictionary(&server_pref_dict)) {
       DVLOG(1) << "Malformed http_server_properties server: " << server_str;
       return false;
     }
 
     // Get if server supports Spdy.
     bool supports_spdy = false;
-    if ((server_pref_dict->GetBoolean(kSupportsSpdyKey, &supports_spdy)) &&
-        supports_spdy) {
-      spdy_servers->push_back(spdy_server.Serialize());
+    if (server_pref_dict->GetBoolean(kSupportsSpdyKey, &supports_spdy)) {
+      spdy_servers_map->Put(spdy_server.Serialize(), supports_spdy);
     }
 
     if (!AddToAlternativeServiceMap(spdy_server, *server_pref_dict,
                                     alternative_service_map) ||
         !AddToNetworkStatsMap(spdy_server, *server_pref_dict,
                               network_stats_map)) {
       return false;
     }
   }
   return true;
@@ skipping 198 matching lines @@
                << quic_server_id_str;
       detected_corrupted_prefs = true;
       continue;
     }
     quic_server_info_map->Put(quic_server_id, quic_server_info);
   }
   return !detected_corrupted_prefs;
 }
 
 void HttpServerPropertiesManager::UpdateCacheFromPrefsOnNetworkSequence(
-    ServerList* spdy_servers,
-    AlternativeServiceMap* alternative_service_map,
-    IPAddress* last_quic_address,
-    ServerNetworkStatsMap* server_network_stats_map,
-    QuicServerInfoMap* quic_server_info_map,
+    std::unique_ptr<SpdyServersMap> spdy_servers_map,
+    std::unique_ptr<AlternativeServiceMap> alternative_service_map,
+    std::unique_ptr<IPAddress> last_quic_address,
+    std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map,
+    std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
     bool detected_corrupted_prefs) {
   // Preferences have the master data because admins might have pushed new
   // preferences. Update the cached data with new data from preferences.
   DCHECK(network_task_runner_->RunsTasksInCurrentSequence());
 
-  UMA_HISTOGRAM_COUNTS("Net.CountOfSpdyServers", spdy_servers->size());
-  http_server_properties_impl_->SetSpdyServers(spdy_servers, true);
+  UMA_HISTOGRAM_COUNTS("Net.CountOfSpdyServers", spdy_servers_map->size());
+  http_server_properties_impl_->SetSpdyServers(std::move(spdy_servers_map));
 
   // 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_->SetAlternativeServiceServers(
-      alternative_service_map);
+      std::move(alternative_service_map));
 
-  http_server_properties_impl_->SetSupportsQuic(last_quic_address);
+  http_server_properties_impl_->SetSupportsQuic(*last_quic_address);
 
-  http_server_properties_impl_->SetServerNetworkStats(server_network_stats_map);
+  http_server_properties_impl_->SetServerNetworkStats(
+      std::move(server_network_stats_map));
 
   UMA_HISTOGRAM_COUNTS_1000("Net.CountOfQuicServerInfos",
                             quic_server_info_map->size());
 
-  http_server_properties_impl_->SetQuicServerInfoMap(quic_server_info_map);
+  http_server_properties_impl_->SetQuicServerInfoMap(
+      std::move(quic_server_info_map));
 
   // Update the prefs with what we have read (delete all corrupted prefs).
   if (detected_corrupted_prefs)
     ScheduleUpdatePrefsOnNetworkSequence(DETECTED_CORRUPTED_PREFS);
 }
 
 //
 // Update Preferences with data from the cached data.
 //
 void HttpServerPropertiesManager::ScheduleUpdatePrefsOnNetworkSequence(
@@ skipping 15 matching lines @@
 // This is required so we can set this as the callback for a timer.
 void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkSequence() {
   UpdatePrefsFromCacheOnNetworkSequence(base::Closure());
 }
 
 void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkSequence(
     const base::Closure& completion) {
   DCHECK(network_task_runner_->RunsTasksInCurrentSequence());
 
   // It is in MRU order.
-  base::ListValue* spdy_server_list = new base::ListValue;
+  std::unique_ptr<std::vector<std::string>> spdy_servers =
+      base::MakeUnique<std::vector<std::string>>();
   http_server_properties_impl_->GetSpdyServerList(
-      spdy_server_list, kMaxSupportsSpdyServerHostsToPersist);
+      spdy_servers.get(), kMaxSupportsSpdyServerHostsToPersist);
 
-  AlternativeServiceMap* alternative_service_map =
-      new AlternativeServiceMap(kMaxAlternateProtocolHostsToPersist);
+  std::unique_ptr<AlternativeServiceMap> alternative_service_map =
+      base::MakeUnique<AlternativeServiceMap>(
+          kMaxAlternateProtocolHostsToPersist);
   const AlternativeServiceMap& map =
       http_server_properties_impl_->alternative_service_map();
   UMA_HISTOGRAM_COUNTS("Net.CountOfAlternateProtocolServers.Memory",
                        map.size());
   int count = 0;
   typedef std::map<std::string, bool> CanonicalHostPersistedMap;
   CanonicalHostPersistedMap persisted_map;
   // Maintain MRU order.
   for (AlternativeServiceMap::const_reverse_iterator it = map.rbegin();
        it != map.rend() && count < kMaxAlternateProtocolHostsToPersist; ++it) {
@@ skipping 26 matching lines @@
     if (canonical_suffix != nullptr) {
       if (persisted_map.find(*canonical_suffix) != persisted_map.end())
         continue;
       persisted_map[*canonical_suffix] = true;
     }
     alternative_service_map->Put(server,
                                  notbroken_alternative_service_info_vector);
     ++count;
   }
 
-  ServerNetworkStatsMap* server_network_stats_map =
-      new ServerNetworkStatsMap(kMaxServerNetworkStatsHostsToPersist);
+  std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map =
+      base::MakeUnique<ServerNetworkStatsMap>(
+          kMaxServerNetworkStatsHostsToPersist);
   const ServerNetworkStatsMap& network_stats_map =
       http_server_properties_impl_->server_network_stats_map();
   count = 0;
   for (ServerNetworkStatsMap::const_reverse_iterator
            it = network_stats_map.rbegin();
        it != network_stats_map.rend() &&
        count < kMaxServerNetworkStatsHostsToPersist;
        ++it, ++count) {
     server_network_stats_map->Put(it->first, it->second);
   }
 
-  QuicServerInfoMap* quic_server_info_map = nullptr;
+  std::unique_ptr<QuicServerInfoMap> quic_server_info_map;
   const QuicServerInfoMap& main_quic_server_info_map =
       http_server_properties_impl_->quic_server_info_map();
   if (main_quic_server_info_map.size() > 0) {
-    quic_server_info_map =
-        new QuicServerInfoMap(max_server_configs_stored_in_properties());
+    quic_server_info_map = base::MakeUnique<QuicServerInfoMap>(
+        max_server_configs_stored_in_properties());
     for (const std::pair<const QuicServerId, std::string>& entry :
          main_quic_server_info_map) {
       quic_server_info_map->Put(entry.first, entry.second);
     }
   }
 
-  IPAddress* last_quic_addr = new IPAddress;
-  http_server_properties_impl_->GetSupportsQuic(last_quic_addr);
+  std::unique_ptr<IPAddress> last_quic_addr = base::MakeUnique<IPAddress>();
+  http_server_properties_impl_->GetSupportsQuic(last_quic_addr.get());
   // 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(alternative_service_map),
-          base::Owned(last_quic_addr), base::Owned(server_network_stats_map),
-          base::Owned(quic_server_info_map), completion));
+      base::Bind(&HttpServerPropertiesManager::UpdatePrefsOnPrefThread,
+                 pref_weak_ptr_, base::Passed(&spdy_servers),
+                 base::Passed(&alternative_service_map),
+                 base::Passed(&last_quic_addr),
+                 base::Passed(&server_network_stats_map),
+                 base::Passed(&quic_server_info_map), completion));
 }
 
 // A local or temporary data structure to hold |supports_spdy|, SpdySettings,
 // AlternativeServiceInfoVector, and SupportsQuic preferences for a server. This
 // is used only in UpdatePrefsOnPrefThread.
 struct ServerPref {
   ServerPref()
       : supports_spdy(false),
         settings_map(nullptr),
         alternative_service_info_vector(nullptr),
@@ skipping 12 matching lines @@
         server_network_stats(server_network_stats) {}
   bool supports_spdy;
   const SettingsMap* settings_map;
   const AlternativeServiceInfoVector* alternative_service_info_vector;
   const SupportsQuic* supports_quic;
   const ServerNetworkStats* server_network_stats;
 };
 
 // All maps and lists are in MRU order.
 void HttpServerPropertiesManager::UpdatePrefsOnPrefThread(
-    base::ListValue* spdy_server_list,
-    AlternativeServiceMap* alternative_service_map,
-    IPAddress* last_quic_address,
-    ServerNetworkStatsMap* server_network_stats_map,
-    QuicServerInfoMap* quic_server_info_map,
+    std::unique_ptr<std::vector<std::string>> spdy_servers,
+    std::unique_ptr<AlternativeServiceMap> alternative_service_map,
+    std::unique_ptr<IPAddress> last_quic_address,
+    std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map,
+    std::unique_ptr<QuicServerInfoMap> quic_server_info_map,
     const base::Closure& completion) {
   typedef base::MRUCache<url::SchemeHostPort, ServerPref> ServerPrefMap;
   ServerPrefMap server_pref_map(ServerPrefMap::NO_AUTO_EVICT);
 
   DCHECK(pref_task_runner_->RunsTasksInCurrentSequence());
 
   // Add servers that support spdy to server_pref_map in the MRU order.
-  for (size_t index = spdy_server_list->GetSize(); index > 0; --index) {
-    std::string server_str;
-    if (spdy_server_list->GetString(index - 1, &server_str)) {
-      url::SchemeHostPort server((GURL(server_str)));
-      ServerPrefMap::iterator it = server_pref_map.Get(server);
-      if (it == server_pref_map.end()) {
-        ServerPref server_pref;
-        server_pref.supports_spdy = true;
-        server_pref_map.Put(server, server_pref);
-      } else {
-        it->second.supports_spdy = true;
-      }
+  DCHECK(spdy_servers);
+  for (size_t i = spdy_servers->size(); i > 0; --i) {
+    url::SchemeHostPort server((GURL(spdy_servers->at(i - 1))));
+    ServerPrefMap::iterator it = server_pref_map.Get(server);
+    if (it == server_pref_map.end()) {
+      ServerPref server_pref;
+      server_pref.supports_spdy = true;
+      server_pref_map.Put(server, server_pref);
+    } else {
+      it->second.supports_spdy = true;
     }
   }
 
   // Add alternative services to server_pref_map in the MRU order.
+  DCHECK(alternative_service_map);
   for (AlternativeServiceMap::const_reverse_iterator map_it =
            alternative_service_map->rbegin();
        map_it != alternative_service_map->rend(); ++map_it) {
     const url::SchemeHostPort server = map_it->first;
     ServerPrefMap::iterator it = server_pref_map.Get(server);
     if (it == server_pref_map.end()) {
       ServerPref server_pref;
       server_pref.alternative_service_info_vector = &map_it->second;
       server_pref_map.Put(server, server_pref);
     } else {
       it->second.alternative_service_info_vector = &map_it->second;
     }
   }
 
   // Add ServerNetworkStats servers to server_pref_map in the MRU order.
+  DCHECK(server_network_stats_map);
   for (ServerNetworkStatsMap::const_reverse_iterator map_it =
            server_network_stats_map->rbegin();
        map_it != server_network_stats_map->rend(); ++map_it) {
     const url::SchemeHostPort server = map_it->first;
     ServerPrefMap::iterator it = server_pref_map.Get(server);
     if (it == server_pref_map.end()) {
       ServerPref server_pref;
       server_pref.server_network_stats = &map_it->second;
       server_pref_map.Put(server, server_pref);
     } else {
       it->second.server_network_stats = &map_it->second;
     }
   }
 
   // Persist properties to the prefs in the MRU order.
   base::DictionaryValue http_server_properties_dict;
   auto servers_list = base::MakeUnique<base::ListValue>();
   for (ServerPrefMap::const_reverse_iterator map_it = server_pref_map.rbegin();
        map_it != server_pref_map.rend(); ++map_it) {
     const url::SchemeHostPort server = map_it->first;
     const ServerPref& server_pref = map_it->second;
 
     auto servers_dict = base::MakeUnique<base::DictionaryValue>();
     auto server_pref_dict = base::MakeUnique<base::DictionaryValue>();
 
     // Save supports_spdy.
     if (server_pref.supports_spdy)
       server_pref_dict->SetBoolean(kSupportsSpdyKey, server_pref.supports_spdy);
     SaveAlternativeServiceToServerPrefs(
-        server_pref.alternative_service_info_vector, server_pref_dict.get());
-    SaveNetworkStatsToServerPrefs(server_pref.server_network_stats,
+        *server_pref.alternative_service_info_vector, server_pref_dict.get());
+    SaveNetworkStatsToServerPrefs(*server_pref.server_network_stats,
                                   server_pref_dict.get());
 
     servers_dict->SetWithoutPathExpansion(server.Serialize(),
                                           std::move(server_pref_dict));
     bool value = servers_list->AppendIfNotPresent(std::move(servers_dict));
     DCHECK(value);  // Should never happen.
   }
 
   http_server_properties_dict.SetWithoutPathExpansion(kServersKey,
                                                       std::move(servers_list));
   SetVersion(&http_server_properties_dict, kVersionNumber);
 
-  SaveSupportsQuicToPrefs(last_quic_address, &http_server_properties_dict);
+  DCHECK(last_quic_address);
+  SaveSupportsQuicToPrefs(*last_quic_address, &http_server_properties_dict);
 
-  SaveQuicServerInfoMapToServerPrefs(quic_server_info_map,
-                                     &http_server_properties_dict);
+  if (quic_server_info_map) {
+    SaveQuicServerInfoMapToServerPrefs(*quic_server_info_map,
+                                       &http_server_properties_dict);
+  }
 
   setting_prefs_ = true;
   pref_delegate_->SetServerProperties(http_server_properties_dict);
   setting_prefs_ = false;
 
   // Note that |completion| will be fired after we have written everything to
   // the Preferences, but likely before these changes are serialized to disk.
   // This is not a problem though, as JSONPrefStore guarantees that this will
   // happen, pretty soon, and even in the case we shut down immediately.
   if (!completion.is_null())
     completion.Run();
 }
 
 void HttpServerPropertiesManager::SaveAlternativeServiceToServerPrefs(
-    const AlternativeServiceInfoVector* alternative_service_info_vector,
+    const AlternativeServiceInfoVector& alternative_service_info_vector,
     base::DictionaryValue* server_pref_dict) {
-  if (!alternative_service_info_vector ||
-      alternative_service_info_vector->empty()) {
+  if (alternative_service_info_vector.empty()) {
     return;
   }
   std::unique_ptr<base::ListValue> alternative_service_list(
       new base::ListValue);
   for (const AlternativeServiceInfo& alternative_service_info :
-       *alternative_service_info_vector) {
+       alternative_service_info_vector) {
     const AlternativeService alternative_service =
         alternative_service_info.alternative_service();
     DCHECK(IsAlternateProtocolValid(alternative_service.protocol));
     std::unique_ptr<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, NextProtoToString(alternative_service.protocol));
     // JSON cannot store int64_t, so expiration is converted to a string.
     alternative_service_dict->SetString(
         kExpirationKey,
         base::Int64ToString(
             alternative_service_info.expiration().ToInternalValue()));
     alternative_service_list->Append(std::move(alternative_service_dict));
   }
   if (alternative_service_list->GetSize() == 0)
     return;
   server_pref_dict->SetWithoutPathExpansion(
       kAlternativeServiceKey, std::move(alternative_service_list));
 }
 
 void HttpServerPropertiesManager::SaveSupportsQuicToPrefs(
-    const IPAddress* last_quic_address,
+    const IPAddress& last_quic_address,
     base::DictionaryValue* http_server_properties_dict) {
-  if (!last_quic_address || !last_quic_address->IsValid())
+  if (!last_quic_address.IsValid())
     return;
 
   auto supports_quic_dict = base::MakeUnique<base::DictionaryValue>();
   supports_quic_dict->SetBoolean(kUsedQuicKey, true);
-  supports_quic_dict->SetString(kAddressKey, last_quic_address->ToString());
+  supports_quic_dict->SetString(kAddressKey, last_quic_address.ToString());
   http_server_properties_dict->SetWithoutPathExpansion(
       kSupportsQuicKey, std::move(supports_quic_dict));
 }
 
 void HttpServerPropertiesManager::SaveNetworkStatsToServerPrefs(
-    const ServerNetworkStats* server_network_stats,
+    const ServerNetworkStats& server_network_stats,
     base::DictionaryValue* server_pref_dict) {
-  if (!server_network_stats)
-    return;
-
   auto server_network_stats_dict = base::MakeUnique<base::DictionaryValue>();
   // Becasue JSON doesn't support int64_t, persist int64_t as a string.
   server_network_stats_dict->SetInteger(
-      kSrttKey, static_cast<int>(server_network_stats->srtt.ToInternalValue()));
+      kSrttKey, static_cast<int>(server_network_stats.srtt.ToInternalValue()));
   // TODO(rtenneti): When QUIC starts using bandwidth_estimate, then persist
   // bandwidth_estimate.
   server_pref_dict->SetWithoutPathExpansion(
       kNetworkStatsKey, std::move(server_network_stats_dict));
 }
 
 void HttpServerPropertiesManager::SaveQuicServerInfoMapToServerPrefs(
-    QuicServerInfoMap* quic_server_info_map,
+    const QuicServerInfoMap& quic_server_info_map,
     base::DictionaryValue* http_server_properties_dict) {
-  if (!quic_server_info_map)
-    return;
-
   auto quic_servers_dict = base::MakeUnique<base::DictionaryValue>();
   for (const std::pair<QuicServerId, std::string>& entry :
-       *quic_server_info_map) {
+       quic_server_info_map) {
     const QuicServerId& server_id = entry.first;
     auto quic_server_pref_dict = base::MakeUnique<base::DictionaryValue>();
     quic_server_pref_dict->SetStringWithoutPathExpansion(kServerInfoKey,
                                                          entry.second);
     quic_servers_dict->SetWithoutPathExpansion(
         server_id.ToString(), std::move(quic_server_pref_dict));
   }
   http_server_properties_dict->SetWithoutPathExpansion(
       kQuicServers, std::move(quic_servers_dict));
 }
 
 void HttpServerPropertiesManager::OnHttpServerPropertiesChanged() {
   DCHECK(pref_task_runner_->RunsTasksInCurrentSequence());
   if (!setting_prefs_)
     ScheduleUpdateCacheOnPrefThread();
 }
 
 void HttpServerPropertiesManager::SetInitialized() {
   DCHECK(network_task_runner_->RunsTasksInCurrentSequence());
   is_initialized_ = true;
 }
 
 }  // namespace net