Do not persist HttpServerProperties to disk that often.

net/http/http_server_properties_manager.cc

 // Copyright 2014 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.
 
 #include "net/http/http_server_properties_manager.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/metrics/histogram_macros.h"
@@ skipping 14 matching lines @@
 // Time to wait before starting an update the http_server_properties_impl_ cache
 // from preferences. Scheduling another update during this period will reset the
 // timer.
 const int64_t kUpdateCacheDelayMs = 1000;
 
 // Time to wait before starting an update the preferences from the
 // http_server_properties_impl_ cache. Scheduling another update during this
 // period will reset the timer.
 const int64_t kUpdatePrefsDelayMs = 60000;
 
+// Maximum time to wait between ScheduleUpdatePrefsOnNetworkThread() calls from
+// SetAlternativeService() and SetAlternativeServices(), in seconds.
+const int64_t kAlternativeServicesUpdatePrefsS = 3600;

[Ryan Hamilton, 2016/07/21 22:27:05]

This is one hour, right? That seems like a LONG time. On mobile, that's forever.

[Bence, 2016/07/22 16:28:54]

Well, then how about five minutes?  Then I can change it to milliseconds to
match the other two.

+
 // "version" 0 indicates, http_server_properties doesn't have "version"
 // property.
 const int kMissingVersion = 0;
 
 // The version number of persisted http_server_properties.
 const int kVersionNumber = 5;
 
 // Persist 200 MRU AlternateProtocolHostPortPairs.
 const int kMaxAlternateProtocolHostsToPersist = 200;
 
@@ skipping 29 matching lines @@
 //  HttpServerPropertiesManager
 
 HttpServerPropertiesManager::PrefDelegate::~PrefDelegate() {}
 
 HttpServerPropertiesManager::HttpServerPropertiesManager(
     PrefDelegate* pref_delegate,
     scoped_refptr<base::SequencedTaskRunner> network_task_runner)
     : pref_task_runner_(base::ThreadTaskRunnerHandle::Get()),
       pref_delegate_(pref_delegate),
       setting_prefs_(false),
-      network_task_runner_(network_task_runner) {
+      network_task_runner_(network_task_runner),
+      last_update_prefs_from_cache_(base::Time::Now()) {

[Bence, 2016/07/22 16:28:54]

Oops.

   DCHECK(pref_delegate_);
   pref_weak_ptr_factory_.reset(
       new base::WeakPtrFactory<HttpServerPropertiesManager>(this));
   pref_weak_ptr_ = pref_weak_ptr_factory_->GetWeakPtr();
   pref_cache_update_timer_.reset(new base::OneShotTimer);
   pref_delegate_->StartListeningForUpdates(
       base::Bind(&HttpServerPropertiesManager::OnHttpServerPropertiesChanged,
                  base::Unretained(this)));
 }
 
@@ skipping 93 matching lines @@
     const url::SchemeHostPort& origin) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   return http_server_properties_impl_->GetAlternativeServices(origin);
 }
 
 bool HttpServerPropertiesManager::SetAlternativeService(
     const url::SchemeHostPort& origin,
     const AlternativeService& alternative_service,
     base::Time expiration) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
-  const bool changed = http_server_properties_impl_->SetAlternativeService(
-      origin, alternative_service, expiration);
+  const bool changed =
+      http_server_properties_impl_->SetAlternativeService(
+          origin, alternative_service, expiration) ||
+      (last_update_prefs_from_cache_ +
+           base::TimeDelta::FromSeconds(kAlternativeServicesUpdatePrefsS) <
+       base::Time::Now());
   if (changed) {
     ScheduleUpdatePrefsOnNetworkThread(SET_ALTERNATIVE_SERVICES);
   }
   return changed;
 }
 
 bool HttpServerPropertiesManager::SetAlternativeServices(
     const url::SchemeHostPort& origin,
     const AlternativeServiceInfoVector& alternative_service_info_vector) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
-  const bool changed = http_server_properties_impl_->SetAlternativeServices(
-      origin, alternative_service_info_vector);
+  const bool changed =
+      http_server_properties_impl_->SetAlternativeServices(
+          origin, alternative_service_info_vector) ||
+      (last_update_prefs_from_cache_ +
+           base::TimeDelta::FromSeconds(kAlternativeServicesUpdatePrefsS) <
+       base::Time::Now());

[Ryan Hamilton, 2016/07/21 22:27:05]

Hm. If nothing has changed, why bother persisting again?

[Bence, 2016/07/22 16:28:54]

Because old_time in HttpServerPropertiesImpl is the expiration time of the entry
in memory, not of the one on disk.  Imagine a server advertising max-age of one
week.  If there is a request every three days, old_time is four days in the
future, new_time is seven, so |changed| is false, and we will never ever ever
persist to disk.  On desktop, Chrome could be running for months.  Then there is
a crash or someone trips in the power chord, and we do not have a valid Alt-Svc
entry on disk for the next startup.

[Ryan Hamilton, 2016/07/22 23:31:35]

I'm not sure this bothers me. If the user only hits this site every 3 days, and
hits *no other* sites which might cause HttpServerProperties to be persisted,
then it's really no big deal if this happens. That being said, in this example,
won't we persist on the first request (day 0), and then we crash on day 3, and
then write again on day 6?

[Bence, 2016/07/27 16:30:55]

All right then.  I think you convinced me that the added complexity is not worth
it.  I'm reverting this timing logic.

   if (changed) {
     ScheduleUpdatePrefsOnNetworkThread(SET_ALTERNATIVE_SERVICES);
   }
   return changed;
 }
 
 void HttpServerPropertiesManager::MarkAlternativeServiceBroken(
     const AlternativeService& alternative_service) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
   http_server_properties_impl_->MarkAlternativeServiceBroken(
@@ skipping 667 matching lines @@
 }
 
 // This is required so we can set this as the callback for a timer.
 void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkThread() {
   UpdatePrefsFromCacheOnNetworkThread(base::Closure());
 }
 
 void HttpServerPropertiesManager::UpdatePrefsFromCacheOnNetworkThread(
     const base::Closure& completion) {
   DCHECK(network_task_runner_->RunsTasksOnCurrentThread());
+  last_update_prefs_from_cache_ = base::Time::Now();
 
   // It is in MRU order.
   base::ListValue* spdy_server_list = new base::ListValue;
   http_server_properties_impl_->GetSpdyServerList(
       spdy_server_list, kMaxSupportsSpdyServerHostsToPersist);
 
   SpdySettingsMap* spdy_settings_map =
       new SpdySettingsMap(kMaxSpdySettingsHostsToPersist);
   const SpdySettingsMap& main_map =
       http_server_properties_impl_->spdy_settings_map();
@@ skipping 340 matching lines @@
                                                        quic_servers_dict);
 }
 
 void HttpServerPropertiesManager::OnHttpServerPropertiesChanged() {
   DCHECK(pref_task_runner_->RunsTasksOnCurrentThread());
   if (!setting_prefs_)
     ScheduleUpdateCacheOnPrefThread();
 }
 
 }  // namespace net