Do not persist expired alternative service entries to disk.

Expired alternative service entries stay around in
HttpServerPropertiesImpl::alternative_service_map_ until the next
GetAlternativeService() call lazily purges them.  However, they should not be
persisted to disk.

BUG=568386

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

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

2 files changed, 70 insertions, 2 deletions

diff --git a/net/http/http_server_properties_manager.cc b/net/http/http_server_properties_manager.cc
index 3bfba4a..66b4401 100644
--- a/net/http/http_server_properties_manager.cc
+++ b/net/http/http_server_properties_manager.cc
@@ -869,12 +869,15 @@ void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkThread(
     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)) {
+      // Do not persist expired entries.
+      if (alternative_service_info.expiration < base::Time::Now()) {
         continue;
       }
       AlternativeService alternative_service(
           alternative_service_info.alternative_service);
+      if (!IsAlternateProtocolValid(alternative_service.protocol)) {
+        continue;
+      }
       if (alternative_service.host.empty()) {
         alternative_service.host = server.host();
       }
diff --git a/net/http/http_server_properties_manager_unittest.cc b/net/http/http_server_properties_manager_unittest.cc
index daa3830..14470fc 100644
--- a/net/http/http_server_properties_manager_unittest.cc
+++ b/net/http/http_server_properties_manager_unittest.cc
@@ -1036,6 +1036,71 @@ TEST_F(HttpServerPropertiesManagerTest, AddToAlternativeServiceMap) {
   EXPECT_EQ(expected_expiration, alternative_service_info_vector[2].expiration);
 }
 
+// Do not persist expired or broken alternative service entries to disk.
+TEST_F(HttpServerPropertiesManagerTest,
+       DoNotPersistExpiredOrBrokenAlternativeService) {
+  ExpectScheduleUpdatePrefsOnNetworkThreadRepeatedly();
+
+  AlternativeServiceInfoVector alternative_service_info_vector;
+
+  const AlternativeService broken_alternative_service(
+      NPN_HTTP_2, "broken.example.com", 443);
+  const base::Time time_one_day_later =
+      base::Time::Now() + base::TimeDelta::FromDays(1);
+  alternative_service_info_vector.push_back(AlternativeServiceInfo(
+      broken_alternative_service, 1.0, time_one_day_later));
+  http_server_props_manager_->MarkAlternativeServiceBroken(
+      broken_alternative_service);
+
+  const AlternativeService expired_alternative_service(
+      NPN_HTTP_2, "expired.example.com", 443);
+  const base::Time time_one_day_ago =
+      base::Time::Now() - base::TimeDelta::FromDays(1);
+  alternative_service_info_vector.push_back(AlternativeServiceInfo(
+      expired_alternative_service, 1.0, time_one_day_ago));
+
+  const AlternativeService valid_alternative_service(NPN_HTTP_2,
+                                                     "valid.example.com", 443);
+  alternative_service_info_vector.push_back(AlternativeServiceInfo(
+      valid_alternative_service, 1.0, time_one_day_later));
+
+  const HostPortPair host_port_pair("www.example.com", 443);
+  http_server_props_manager_->SetAlternativeServices(
+      host_port_pair, alternative_service_info_vector);
+
+  // Update cache.
+  ExpectPrefsUpdate();
+  ExpectCacheUpdate();
+  http_server_props_manager_->ScheduleUpdateCacheOnPrefThread();
+  base::RunLoop().RunUntilIdle();
+
+  const base::Value* pref_value =
+      pref_service_.GetUserPref(kTestHttpServerProperties);
+  ASSERT_NE(nullptr, pref_value);
+
+  const base::DictionaryValue* pref_dict;
+  ASSERT_TRUE(pref_value->GetAsDictionary(&pref_dict));
+
+  const base::DictionaryValue* server_pref_dict;
+  ASSERT_TRUE(pref_dict->GetDictionary("servers", &server_pref_dict));
+
+  const base::DictionaryValue* example_pref_dict;
+  ASSERT_TRUE(server_pref_dict->GetDictionaryWithoutPathExpansion(
+      "www.example.com:443", &example_pref_dict));
+
+  const base::ListValue* altsvc_list;
+  ASSERT_TRUE(example_pref_dict->GetList("alternative_service", &altsvc_list));
+
+  ASSERT_EQ(1u, altsvc_list->GetSize());
+
+  const base::DictionaryValue* altsvc_entry;
+  ASSERT_TRUE(altsvc_list->GetDictionary(0, &altsvc_entry));
+
+  std::string hostname;
+  ASSERT_TRUE(altsvc_entry->GetString("host", &hostname));
+  EXPECT_EQ("valid.example.com", hostname);
+}
+
 TEST_F(HttpServerPropertiesManagerTest, ShutdownWithPendingUpdateCache0) {
   // Post an update task to the UI thread.
   http_server_props_manager_->ScheduleUpdateCacheOnPrefThread();