SPDY - persist 300 most recently used servers that support SPDY to

preferences file.

This change reduces the size of Preferences file for around 3% of users
who have lot of entries (some users have more than 10,000 entries).

HttpServerProperties - Implement MRU for supports SPDY server list.
Persist only the most recently used 300 entries. In memory keep all the
entries.

This change is similar to MRU change for AlternateProtocol.

R=rch@chromium.org, mmenke@chromium.org
BUG=258189

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@267022 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/net/http_server_properties_manager.cc

@@ -49,6 +49,9 @@ const int kMaxAlternateProtocolHostsToPersist = 200;
 // Persist 200 MRU SpdySettingsHostPortPairs.
 const int kMaxSpdySettingsHostsToPersist = 200;
 
+// Persist 300 MRU SupportsSpdyServerHostPortPairs.
+const int kMaxSupportsSpdyServerHostsToPersist = 300;
+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -139,7 +142,7 @@ void HttpServerPropertiesManager::Clear(const base::Closure& completion) {
 }
 
 bool HttpServerPropertiesManager::SupportsSpdy(
-    const net::HostPortPair& server) const {
+    const net::HostPortPair& server) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   return http_server_properties_impl_->SupportsSpdy(server);
 }
@@ -334,13 +337,6 @@ void HttpServerPropertiesManager::UpdateCacheFromPrefsOnUI() {
     return;
   }
 
-  // TODO(rtenneti): Mark entries with an LRU sequence number (date of access?),
-  // and then truncate down deleting old stuff.
-  if (version != kVersionNumber && servers_dict->size() > 300) {
-    DVLOG(1) << "Size is too large. Clearing all properties.";
-    return;
-  }
-
   // String is host/port pair of spdy server.
   scoped_ptr<StringVector> spdy_servers(new StringVector);
   scoped_ptr<net::SpdySettingsMap> spdy_settings_map(
@@ -532,7 +528,8 @@ void HttpServerPropertiesManager::UpdatePrefsFromCacheOnIO(
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   base::ListValue* spdy_server_list = new base::ListValue;
-  http_server_properties_impl_->GetSpdyServerList(spdy_server_list);
+  http_server_properties_impl_->GetSpdyServerList(
+      spdy_server_list, kMaxSupportsSpdyServerHostsToPersist);
 
   net::SpdySettingsMap* spdy_settings_map =
       new net::SpdySettingsMap(kMaxSpdySettingsHostsToPersist);
@@ -605,8 +602,6 @@ void HttpServerPropertiesManager::UpdatePrefsOnUI(
     net::PipelineCapabilityMap* pipeline_capability_map,
     const base::Closure& completion) {
 
-  // TODO(rtenneti): Fix ServerPrefMap to preserve MRU order of
-  // spdy_settings_map, alternate_protocol_map and pipeline_capability_map.
   typedef std::map<net::HostPortPair, ServerPref> ServerPrefMap;
   ServerPrefMap server_pref_map;
 
@@ -692,7 +687,8 @@ void HttpServerPropertiesManager::UpdatePrefsOnUI(
     base::DictionaryValue* server_pref_dict = new base::DictionaryValue;
 
     // Save supports_spdy.
-    server_pref_dict->SetBoolean("supports_spdy", server_pref.supports_spdy);
+    if (server_pref.supports_spdy)
+      server_pref_dict->SetBoolean("supports_spdy", server_pref.supports_spdy);
 
     // Save SPDY settings.
     if (server_pref.settings_map) {

chrome/browser/net/http_server_properties_manager.h

@@ -89,7 +89,7 @@ class HttpServerPropertiesManager
 
   // Returns true if |server| supports SPDY. Should only be called from IO
   // thread.
-  virtual bool SupportsSpdy(const net::HostPortPair& server) const OVERRIDE;
+  virtual bool SupportsSpdy(const net::HostPortPair& server) OVERRIDE;
 
   // Add |server| as the SPDY server which supports SPDY protocol into the
   // persisitent store. Should only be called from IO thread.

content/browser/loader/resource_scheduler.cc

@@ -337,7 +337,7 @@ class ResourceScheduler::Client {
     if (request->url_request()->priority() < net::LOW) {
       net::HostPortPair host_port_pair =
           net::HostPortPair::FromURL(request->url_request()->url());
-      const net::HttpServerProperties& http_server_properties =
+      net::HttpServerProperties& http_server_properties =
           *request->url_request()->context()->http_server_properties();
       if (!http_server_properties.SupportsSpdy(host_port_pair)) {
         return true;
@@ -410,7 +410,7 @@ class ResourceScheduler::Client {
       return START_REQUEST;
     }
 
-    const net::HttpServerProperties& http_server_properties =
+    net::HttpServerProperties& http_server_properties =
         *url_request.context()->http_server_properties();
 
     if (url_request.priority() >= net::LOW ||

net/http/http_server_properties.h

@@ -121,7 +121,7 @@ class NET_EXPORT HttpServerProperties {
   virtual void Clear() = 0;
 
   // Returns true if |server| supports SPDY.
-  virtual bool SupportsSpdy(const HostPortPair& server) const = 0;
+  virtual bool SupportsSpdy(const HostPortPair& server) = 0;
 
   // Add |server| into the persistent store. Should only be called from IO
   // thread.

net/http/http_server_properties_impl.cc

@@ -27,10 +27,11 @@ const uint64 kBrokenAlternateProtocolDelaySecs = 300;
 }  // namespace
 
 HttpServerPropertiesImpl::HttpServerPropertiesImpl()
-    : alternate_protocol_map_(AlternateProtocolMap::NO_AUTO_EVICT),
+    : spdy_servers_map_(SpdyServerHostPortMap::NO_AUTO_EVICT),
+      alternate_protocol_map_(AlternateProtocolMap::NO_AUTO_EVICT),
       spdy_settings_map_(SpdySettingsMap::NO_AUTO_EVICT),
       pipeline_capability_map_(
-        new CachedPipelineCapabilityMap(kDefaultNumHostsToRemember)),
+          new CachedPipelineCapabilityMap(kDefaultNumHostsToRemember)),
       weak_ptr_factory_(this) {
   canoncial_suffixes_.push_back(".c.youtube.com");
   canoncial_suffixes_.push_back(".googlevideo.com");
@@ -43,12 +44,12 @@ void HttpServerPropertiesImpl::InitializeSpdyServers(
     std::vector<std::string>* spdy_servers,
     bool support_spdy) {
   DCHECK(CalledOnValidThread());
-  spdy_servers_table_.clear();
   if (!spdy_servers)
     return;
-  for (std::vector<std::string>::iterator it = spdy_servers->begin();
-       it != spdy_servers->end(); ++it) {
-    spdy_servers_table_[*it] = support_spdy;
+  // Add the entries from persisted data.
+  for (std::vector<std::string>::reverse_iterator it = spdy_servers->rbegin();
+       it != spdy_servers->rend(); ++it) {
+    spdy_servers_map_.Put(*it, support_spdy);
   }
 }
 
@@ -122,16 +123,20 @@ void HttpServerPropertiesImpl::SetNumPipelinedHostsToRemember(int max_size) {
 }
 
 void HttpServerPropertiesImpl::GetSpdyServerList(
-    base::ListValue* spdy_server_list) const {
+    base::ListValue* spdy_server_list,
+    size_t max_size) const {
   DCHECK(CalledOnValidThread());
   DCHECK(spdy_server_list);
   spdy_server_list->Clear();
+  size_t count = 0;
   // Get the list of servers (host/port) that support SPDY.
-  for (SpdyServerHostPortTable::const_iterator it = spdy_servers_table_.begin();
-       it != spdy_servers_table_.end(); ++it) {
+  for (SpdyServerHostPortMap::const_iterator it = spdy_servers_map_.begin();
+       it != spdy_servers_map_.end() && count < max_size; ++it) {
     const std::string spdy_server_host_port = it->first;
-    if (it->second)
+    if (it->second) {
       spdy_server_list->Append(new base::StringValue(spdy_server_host_port));
+      ++count;
+    }
   }
 }
 
@@ -168,22 +173,22 @@ base::WeakPtr<HttpServerProperties> HttpServerPropertiesImpl::GetWeakPtr() {
 
 void HttpServerPropertiesImpl::Clear() {
   DCHECK(CalledOnValidThread());
-  spdy_servers_table_.clear();
+  spdy_servers_map_.Clear();
   alternate_protocol_map_.Clear();
   spdy_settings_map_.Clear();
   pipeline_capability_map_->Clear();
 }
 
 bool HttpServerPropertiesImpl::SupportsSpdy(
-    const net::HostPortPair& host_port_pair) const {
+    const net::HostPortPair& host_port_pair) {
   DCHECK(CalledOnValidThread());
   if (host_port_pair.host().empty())
     return false;
   std::string spdy_server = GetFlattenedSpdyServer(host_port_pair);
 
-  SpdyServerHostPortTable::const_iterator spdy_host_port =
-      spdy_servers_table_.find(spdy_server);
-  if (spdy_host_port != spdy_servers_table_.end())
+  SpdyServerHostPortMap::iterator spdy_host_port =
+      spdy_servers_map_.Get(spdy_server);
+  if (spdy_host_port != spdy_servers_map_.end())
     return spdy_host_port->second;
   return false;
 }
@@ -196,14 +201,14 @@ void HttpServerPropertiesImpl::SetSupportsSpdy(
     return;
   std::string spdy_server = GetFlattenedSpdyServer(host_port_pair);
 
-  SpdyServerHostPortTable::iterator spdy_host_port =
-      spdy_servers_table_.find(spdy_server);
-  if ((spdy_host_port != spdy_servers_table_.end()) &&
+  SpdyServerHostPortMap::iterator spdy_host_port =
+      spdy_servers_map_.Get(spdy_server);
+  if ((spdy_host_port != spdy_servers_map_.end()) &&
       (spdy_host_port->second == support_spdy)) {
     return;
   }
   // Cache the data.
-  spdy_servers_table_[spdy_server] = support_spdy;
+  spdy_servers_map_.Put(spdy_server, support_spdy);
 }
 
 bool HttpServerPropertiesImpl::HasAlternateProtocol(

net/http/http_server_properties_impl.h

@@ -34,7 +34,7 @@ class NET_EXPORT HttpServerPropertiesImpl
   HttpServerPropertiesImpl();
   virtual ~HttpServerPropertiesImpl();
 
-  // Initializes |spdy_servers_table_| with the servers (host/port) from
+  // Initializes |spdy_servers_map_| with the servers (host/port) from
   // |spdy_servers| that either support SPDY or not.
   void InitializeSpdyServers(std::vector<std::string>* spdy_servers,
                              bool support_spdy);
@@ -49,8 +49,10 @@ class NET_EXPORT HttpServerPropertiesImpl
   void InitializePipelineCapabilities(
       const PipelineCapabilityMap* pipeline_capability_map);
 
-  // Get the list of servers (host/port) that support SPDY.
-  void GetSpdyServerList(base::ListValue* spdy_server_list) const;
+  // Get the list of servers (host/port) that support SPDY. The max_size is the
+  // number of MRU servers that support SPDY that are to be returned.
+  void GetSpdyServerList(base::ListValue* spdy_server_list,
+                         size_t max_size) const;
 
   // Returns flattened string representation of the |host_port_pair|. Used by
   // unittests.
@@ -80,7 +82,7 @@ class NET_EXPORT HttpServerPropertiesImpl
   virtual void Clear() OVERRIDE;
 
   // Returns true if |server| supports SPDY.
-  virtual bool SupportsSpdy(const HostPortPair& server) const OVERRIDE;
+  virtual bool SupportsSpdy(const HostPortPair& server) OVERRIDE;
 
   // Add |server| into the persistent store.
   virtual void SetSupportsSpdy(const HostPortPair& server,
@@ -157,9 +159,9 @@ class NET_EXPORT HttpServerPropertiesImpl
  private:
   typedef base::MRUCache<
       HostPortPair, HttpPipelinedHostCapability> CachedPipelineCapabilityMap;
-  // |spdy_servers_table_| has flattened representation of servers (host/port
-  // pair) that either support or not support SPDY protocol.
-  typedef base::hash_map<std::string, bool> SpdyServerHostPortTable;
+  // |spdy_servers_map_| has flattened representation of servers (host, port)
+  // that either support or not support SPDY protocol.
+  typedef base::MRUCache<std::string, bool> SpdyServerHostPortMap;
   typedef std::map<HostPortPair, NetworkStats> ServerNetworkStatsMap;
   typedef std::map<HostPortPair, HostPortPair> CanonicalHostMap;
   typedef std::vector<std::string> CanonicalSufficList;
@@ -180,7 +182,7 @@ class NET_EXPORT HttpServerPropertiesImpl
   void ExpireBrokenAlternateProtocolMappings();
   void ScheduleBrokenAlternateProtocolMappingsExpiration();
 
-  SpdyServerHostPortTable spdy_servers_table_;
+  SpdyServerHostPortMap spdy_servers_map_;
 
   AlternateProtocolMap alternate_protocol_map_;
   BrokenAlternateProtocolList broken_alternate_protocol_list_;

net/http/http_server_properties_impl_unittest.cc

@@ -21,6 +21,8 @@ class ListValue;
 
 namespace net {
 
+const int kMaxSupportsSpdyServerHosts = 500;
+
 namespace {
 
 class HttpServerPropertiesImplTest : public testing::Test {
@@ -60,6 +62,17 @@ TEST_F(SpdyServerPropertiesTest, Initialize) {
   spdy_servers2.push_back(spdy_server_g);
   spdy_servers2.push_back(spdy_server_d);
   impl_.InitializeSpdyServers(&spdy_servers2, true);
+
+  // Verify spdy_server_g and spdy_server_d are in the list in the same order.
+  base::ListValue spdy_server_list;
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
+  EXPECT_EQ(2U, spdy_server_list.GetSize());
+  std::string string_value_g;
+  ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));
+  ASSERT_EQ(spdy_server_g, string_value_g);
+  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));
 }
@@ -130,13 +143,13 @@ TEST_F(SpdyServerPropertiesTest, GetSpdyServerList) {
   base::ListValue spdy_server_list;
 
   // Check there are no spdy_servers.
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   EXPECT_EQ(0U, spdy_server_list.GetSize());
 
   // Check empty server is not added.
   HostPortPair spdy_server_empty(std::string(), 443);
   impl_.SetSupportsSpdy(spdy_server_empty, true);
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   EXPECT_EQ(0U, spdy_server_list.GetSize());
 
   std::string string_value_g;
@@ -150,38 +163,79 @@ TEST_F(SpdyServerPropertiesTest, GetSpdyServerList) {
 
   // Add www.google.com:443 as not supporting SPDY.
   impl_.SetSupportsSpdy(spdy_server_google, false);
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   EXPECT_EQ(0U, spdy_server_list.GetSize());
 
   // Add www.google.com:443 as supporting SPDY.
   impl_.SetSupportsSpdy(spdy_server_google, true);
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   ASSERT_EQ(1U, spdy_server_list.GetSize());
   ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));
   ASSERT_EQ(spdy_server_g, string_value_g);
 
   // Add mail.google.com:443 as not supporting SPDY.
   impl_.SetSupportsSpdy(spdy_server_mail, false);
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   ASSERT_EQ(1U, spdy_server_list.GetSize());
   ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));
   ASSERT_EQ(spdy_server_g, string_value_g);
 
   // Add mail.google.com:443 as supporting SPDY.
   impl_.SetSupportsSpdy(spdy_server_mail, true);
-  impl_.GetSpdyServerList(&spdy_server_list);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
   ASSERT_EQ(2U, spdy_server_list.GetSize());
 
   // Verify www.google.com:443 and mail.google.com:443 are in the list.
+  ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_m));
+  ASSERT_EQ(spdy_server_m, string_value_m);
+  ASSERT_TRUE(spdy_server_list.GetString(1, &string_value_g));
+  ASSERT_EQ(spdy_server_g, string_value_g);
+
+  // Request for only one server and verify that we get only one server.
+  impl_.GetSpdyServerList(&spdy_server_list, 1);
+  ASSERT_EQ(1U, spdy_server_list.GetSize());
+  ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_m));
+  ASSERT_EQ(spdy_server_m, string_value_m);
+}
+
+TEST_F(SpdyServerPropertiesTest, MRUOfGetSpdyServerList) {
+  base::ListValue spdy_server_list;
+
+  std::string string_value_g;
+  std::string string_value_m;
+  HostPortPair spdy_server_google("www.google.com", 443);
+  std::string spdy_server_g =
+      HttpServerPropertiesImpl::GetFlattenedSpdyServer(spdy_server_google);
+  HostPortPair spdy_server_mail("mail.google.com", 443);
+  std::string spdy_server_m =
+      HttpServerPropertiesImpl::GetFlattenedSpdyServer(spdy_server_mail);
+
+  // Add www.google.com:443 as supporting SPDY.
+  impl_.SetSupportsSpdy(spdy_server_google, true);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
+  ASSERT_EQ(1U, spdy_server_list.GetSize());
   ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));
+  ASSERT_EQ(spdy_server_g, string_value_g);
+
+  // Add mail.google.com:443 as supporting SPDY. Verify mail.google.com:443 and
+  // www.google.com:443 are in the list.
+  impl_.SetSupportsSpdy(spdy_server_mail, true);
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
+  ASSERT_EQ(2U, spdy_server_list.GetSize());
+  ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_m));
+  ASSERT_EQ(spdy_server_m, string_value_m);
+  ASSERT_TRUE(spdy_server_list.GetString(1, &string_value_g));
+  ASSERT_EQ(spdy_server_g, string_value_g);
+
+  // 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));
+  impl_.GetSpdyServerList(&spdy_server_list, kMaxSupportsSpdyServerHosts);
+  ASSERT_EQ(2U, spdy_server_list.GetSize());
+  ASSERT_TRUE(spdy_server_list.GetString(0, &string_value_g));
+  ASSERT_EQ(spdy_server_g, string_value_g);
   ASSERT_TRUE(spdy_server_list.GetString(1, &string_value_m));
-  if (string_value_g.compare(spdy_server_g) == 0) {
-    ASSERT_EQ(spdy_server_g, string_value_g);
-    ASSERT_EQ(spdy_server_m, string_value_m);
-  } else {
-    ASSERT_EQ(spdy_server_g, string_value_m);
-    ASSERT_EQ(spdy_server_m, string_value_g);
-  }
+  ASSERT_EQ(spdy_server_m, string_value_m);
 }
 
 typedef HttpServerPropertiesImplTest AlternateProtocolServerPropertiesTest;