Persist broken and recently-broken alt-svcs to prefs in HttpServerPropertiesManager

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..dfc7935c25f5c08532fd4a9ad53ce2661974cf30 100644
--- a/net/http/http_server_properties_impl.h
+++ b/net/http/http_server_properties_impl.h
@@ -17,7 +17,6 @@
 #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"
@@ -28,6 +27,7 @@
 
 namespace base {
 class ListValue;
+class TickClock;
 }
 
 namespace net {
@@ -37,9 +37,13 @@ class NET_EXPORT HttpServerPropertiesImpl
     : public HttpServerProperties,
       public BrokenAlternativeServices::Delegate {
  public:
+  // |clock| is used for setting expiration times and scheduling the
+  // expiration of broken alternative services. It must not be null.
+  HttpServerPropertiesImpl(base::TickClock* clock);

[Zhongyi Shi, 2017/06/16 04:17:09]

nit: explicit

[wangyix1, 2017/06/16 18:41:15]

Done.

+
+  // Default clock will be used.
   HttpServerPropertiesImpl();
-  explicit HttpServerPropertiesImpl(
-      base::TickClock* broken_alternative_services_clock);
+
   ~HttpServerPropertiesImpl() override;
 
   // Sets |spdy_servers_map_| with the servers (host/port) from
@@ -61,6 +65,17 @@ class NET_EXPORT HttpServerPropertiesImpl
   void GetSpdyServerList(base::ListValue* spdy_server_list,
                          size_t max_size) const;
 
+  void SetBrokenAndRecentlyBrokenAlternativeServices(
+      std::unique_ptr<BrokenAlternativeServiceList>
+          broken_alternative_service_list,
+      std::unique_ptr<RecentlyBrokenAlternativeServices>
+          recently_broken_alternative_services);

[Zhongyi Shi, 2017/06/16 04:17:09]

Does the raw pointers work? Usually pass by unique_ptr means ownership transfer.
I didn't quite get why the ownership of the broken_alternative_service_list need
to be transferred. 

[wangyix1, 2017/06/16 18:41:15]

unique_ptrs are used since these params are simply passed through to
BrokenAlternativeServices::SetBrokenAndRecentlyBrokenAlternativeServices().
I chose to use unique_ptr params for
BrokenAlternativeServices::SetBrokenAndRecentlyBrokenAlternativeServices()
because its implementation modifies |broken_alternative_service_list| and
|recently_broken_alternative_services|. I suppose I could use raw pointers, but
I think using unique_ptrs makes it clear that those objects should not be used
after calling this function. Thoughts?

+
+  const BrokenAlternativeServiceList& broken_alternative_service_list() const;
+
+  const RecentlyBrokenAlternativeServices&
+  recently_broken_alternative_services() const;
+
   // Returns flattened string representation of the |host_port_pair|. Used by
   // unittests.
   static std::string GetFlattenedSpdyServer(const HostPortPair& host_port_pair);
@@ -147,14 +162,15 @@ class NET_EXPORT HttpServerPropertiesImpl
   // Remove the cononical host for |server|.
   void RemoveCanonicalHost(const url::SchemeHostPort& server);
 
-  base::DefaultTickClock broken_alternative_services_clock_;
-  BrokenAlternativeServices broken_alternative_services_;
+  base::DefaultTickClock default_clock_;
 
   SpdyServersMap spdy_servers_map_;
   Http11ServerHostPortSet http11_servers_;
 
   AlternativeServiceMap alternative_service_map_;
 
+  BrokenAlternativeServices broken_alternative_services_;
+
   IPAddress last_quic_address_;
   ServerNetworkStatsMap server_network_stats_map_;
   // Contains a map of servers which could share the same alternate protocol.