Add and persist a new field in AlternativeServiceInfo to list QUIC verisons advertised

net/http/http_server_properties_manager.cc

Index: net/http/http_server_properties_manager.cc
diff --git a/net/http/http_server_properties_manager.cc b/net/http/http_server_properties_manager.cc
index 95ef2f444496beced2a3b87578b9781792a0ca76..2244b4e3edb91eab4e30ae1f2cc7bbcb2c7a418e 100644
--- a/net/http/http_server_properties_manager.cc
+++ b/net/http/http_server_properties_manager.cc
@@ -62,6 +62,7 @@ const char kProtocolKey[] = "protocol_str";
 const char kHostKey[] = "host";
 const char kPortKey[] = "port";
 const char kExpirationKey[] = "expiration";
+const char kAdvertisedVersionsKey[] = "advertised_versions";
 const char kNetworkStatsKey[] = "network_stats";
 const char kSrttKey[] = "srtt";
 
@@ -202,10 +203,11 @@ HttpServerPropertiesManager::GetAlternativeServiceInfos(
 bool HttpServerPropertiesManager::SetAlternativeService(
     const url::SchemeHostPort& origin,
     const AlternativeService& alternative_service,
-    base::Time expiration) {
+    base::Time expiration,
+    const QuicVersionVector& advertised_versions) {
   DCHECK(network_task_runner_->RunsTasksInCurrentSequence());
   const bool changed = http_server_properties_impl_->SetAlternativeService(
-      origin, alternative_service, expiration);
+      origin, alternative_service, expiration, advertised_versions);
   if (changed) {
     ScheduleUpdatePrefsOnNetworkSequence(SET_ALTERNATIVE_SERVICES);
   }
@@ -618,22 +620,48 @@ bool HttpServerPropertiesManager::ParseAlternativeServiceDict(
   }
 
   std::string expiration_string;
-  if (alternative_service_dict.GetStringWithoutPathExpansion(
+  if (!alternative_service_dict.GetStringWithoutPathExpansion(
           kExpirationKey, &expiration_string)) {
-    int64_t expiration_int64 = 0;
-    if (!base::StringToInt64(expiration_string, &expiration_int64)) {
-      DVLOG(1) << "Malformed alternative service expiration for server: "
+    DVLOG(1) << "Malformed alternative service expiration for server: "
+             << server_str;
+    return false;

[Ryan Hamilton, 2017/06/07 20:52:24]

Yay, early return!

+  }
+
+  int64_t expiration_int64 = 0;
+  if (!base::StringToInt64(expiration_string, &expiration_int64)) {
+    DVLOG(1) << "Malformed alternative service expiration for server: "
+             << server_str;
+    return false;
+  }
+  alternative_service_info->expiration =
+      base::Time::FromInternalValue(expiration_int64);
+
+  // Advertised versions list is optional.
+  if (!alternative_service_dict.HasKey(kAdvertisedVersionsKey))
+    return true;
+
+  const base::ListValue* versions_list = nullptr;
+  if (!alternative_service_dict.GetListWithoutPathExpansion(
+          kAdvertisedVersionsKey, &versions_list)) {
+    DVLOG(1)
+        << "Malformed alternative service advertised versions list for server: "
+        << server_str;
+    return false;
+  }
+
+  QuicVersionVector advertised_versions;
+  for (const auto& value : *versions_list) {
+    int version_int;

[Ryan Hamilton, 2017/06/07 20:52:23]

nit: since the loop variable is |value|, I think you can just call this
|version|.

[Zhongyi Shi, 2017/06/08 23:11:16]

Done.

+    if (!value.GetAsInteger(&version_int)) {
+      DVLOG(1) << "Malformed alternative service version for server: "
                << server_str;
       return false;
     }
-    alternative_service_info->expiration =
-        base::Time::FromInternalValue(expiration_int64);
-    return true;
+    advertised_versions.push_back(QuicVersion(version_int));
   }
+  alternative_service_info->set_advertised_versions(advertised_versions);
 
-  DVLOG(1) << "Malformed alternative service expiration for server: "
-           << server_str;
-  return false;
+  return true;
 }
 
 bool HttpServerPropertiesManager::AddToAlternativeServiceMap(
@@ -1087,6 +1115,13 @@ void HttpServerPropertiesManager::SaveAlternativeServiceToServerPrefs(
         kExpirationKey,
         base::Int64ToString(
             alternative_service_info.expiration.ToInternalValue()));
+    std::unique_ptr<base::ListValue> advertised_versions_list(

[Bence, 2017/06/08 17:44:51]

Optional: According to
https://groups.google.com/a/chromium.org/forum/#!msg/chromium-dev/evQtLFgfmyM...
also
https://cs.chromium.org/chromium/src/base/memory/ptr_util.h?l=43
the preferred format is:
auto advertised_versions_list = base::MakeUnique<base::ListValue>();

[Zhongyi Shi, 2017/06/08 23:11:16]

Done.

+        new base::ListValue);
+    for (const auto& version : alternative_service_info.advertised_versions()) {
+      advertised_versions_list->AppendInteger(version);
+    }
+    alternative_service_dict->SetList(kAdvertisedVersionsKey,
+                                      std::move(advertised_versions_list));
     alternative_service_list->Append(std::move(alternative_service_dict));
   }
   if (alternative_service_list->GetSize() == 0)