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

net/http/http_server_properties_impl.h

Index: net/http/http_server_properties_impl.h
diff --git a/net/http/http_server_properties_impl.h b/net/http/http_server_properties_impl.h
index f5eb4bad191afe9b7dcd1e2f94afab8a640ce0a0..432c97e1225c6e0b0baebeb34519fe8ee326b8dd 100644
--- a/net/http/http_server_properties_impl.h
+++ b/net/http/http_server_properties_impl.h
@@ -26,10 +26,6 @@
 #include "net/http/broken_alternative_services.h"
 #include "net/http/http_server_properties.h"
 
-namespace base {
-class ListValue;
-}
-
 namespace net {
 
 // The implementation for setting/retrieving the HTTP server properties.
@@ -44,22 +40,24 @@ class NET_EXPORT HttpServerPropertiesImpl
 
   // Sets |spdy_servers_map_| with the servers (host/port) from
   // |spdy_servers| that either support SPDY or not.
-  void SetSpdyServers(std::vector<std::string>* spdy_servers,
+  void SetSpdyServers(const std::vector<std::string>* spdy_servers,

[Ryan Hamilton, 2017/06/20 18:25:56]

Can this be a const&?

[wangyix1, 2017/06/21 18:45:33]

The unit-test SpdyServerPropertiesTest.Set tests this function with
spdy_servers=nullptr. Other than that, there's nothing stopping it from being
const&. I think that part of the unit-test can be removed as it doesn't seem too
important.

https://cs.chromium.org/chromium/src/net/http/http_server_properties_impl_uni...

Done.
 

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

This is now a unique_ptr not a const&. Is that to avoid a copy? If so, SGTM.

I notice that |supports_spdy| is also removed. Is that because it was unused
before?

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

Yes, it's now a unique_ptr<SpdyServersMap> instead of a vector<string> and a
bool. This avoids a copy and makes it more consistent with the other
HttpServerPropertiesImpl setters that use MRUCache params. However, this change
required some other changes to be made in HttpServerPropertiesManager,
specifically, the line you commented on where a ListValue is now iterated
backwards instead of forwards. I'll reply to that comment to explain more.

|supports_spdy| is removed since that information is included SpdyServersMap
(which is MRUCache<string, bool>). This does change the API a little: instead of
having to set every server in the vector to support or not support spdy, it can
now set some servers in the map to support spdy and others to not support spdy.

                       bool support_spdy);
 
   void SetAlternativeServiceServers(
-      AlternativeServiceMap* alternate_protocol_servers);
+      std::unique_ptr<AlternativeServiceMap> alternate_protocol_servers);
 
-  void SetSupportsQuic(IPAddress* last_address);
+  void SetSupportsQuic(const IPAddress* last_address);

[Ryan Hamilton, 2017/06/20 18:25:56]

ditto

[wangyix1, 2017/06/21 18:45:33]

Done.

 
-  void SetServerNetworkStats(ServerNetworkStatsMap* server_network_stats_map);
+  void SetServerNetworkStats(
+      std::unique_ptr<ServerNetworkStatsMap> server_network_stats_map);
 
-  void SetQuicServerInfoMap(QuicServerInfoMap* quic_server_info_map);
+  void SetQuicServerInfoMap(
+      std::unique_ptr<QuicServerInfoMap> quic_server_info_map);
 
   // 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;
+  void GetSpdyServers(std::vector<std::string>* spdy_servers,
+                      size_t max_size) const;
 
   // Returns flattened string representation of the |host_port_pair|. Used by
   // unittests.