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

net/http/http_server_properties_impl.h

 // Copyright (c) 2012 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.
 
 #ifndef NET_HTTP_HTTP_SERVER_PROPERTIES_IMPL_H_
 #define NET_HTTP_HTTP_SERVER_PROPERTIES_IMPL_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <deque>
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/default_tick_clock.h"
 #include "base/values.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_address.h"
 #include "net/base/linked_hash_map.h"
 #include "net/base/net_export.h"
 #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.
 class NET_EXPORT HttpServerPropertiesImpl
     : public HttpServerProperties,
       public BrokenAlternativeServices::Delegate {
  public:
   HttpServerPropertiesImpl();
   explicit HttpServerPropertiesImpl(
       base::TickClock* broken_alternative_services_clock);
   ~HttpServerPropertiesImpl() override;
 
   // 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.
   static std::string GetFlattenedSpdyServer(const HostPortPair& host_port_pair);
 
   // Returns the canonical host suffix for |host|, or nullptr if none
   // exists.
   const std::string* GetCanonicalSuffix(const std::string& host) const;
 
   // -----------------------------
@@ skipping 96 matching lines @@
   size_t max_server_configs_stored_in_properties_;
 
   THREAD_CHECKER(thread_checker_);
 
   DISALLOW_COPY_AND_ASSIGN(HttpServerPropertiesImpl);
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_SERVER_PROPERTIES_IMPL_H_