Use lossy prefs in the net predictor, and update them more frequently

chrome/browser/net/predictor.cc

 // 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.
 
 #include "chrome/browser/net/predictor.h"
 
 #include <algorithm>
 #include <cmath>
 #include <iterator>
 #include <set>
 #include <sstream>
 #include <utility>
 
 #include "base/bind.h"
+#include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/containers/mru_cache.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
@@ skipping 99 matching lines @@
 Predictor* Predictor::CreatePredictor(bool preconnect_enabled,
                                       bool predictor_enabled,
                                       bool simple_shutdown) {
   if (simple_shutdown)
     return new SimplePredictor(preconnect_enabled, predictor_enabled);
   return new Predictor(preconnect_enabled, predictor_enabled);
 }
 
 void Predictor::RegisterProfilePrefs(
     user_prefs::PrefRegistrySyncable* registry) {
-  registry->RegisterListPref(prefs::kDnsPrefetchingStartupList);
-  registry->RegisterListPref(prefs::kDnsPrefetchingHostReferralList);
+  registry->RegisterListPref(prefs::kDnsPrefetchingStartupList,
+                             PrefRegistry::LOSSY_PREF);
+  registry->RegisterListPref(prefs::kDnsPrefetchingHostReferralList,
+                             PrefRegistry::LOSSY_PREF);
 }
 
 // --------------------- Start UI methods. ------------------------------------
 
 void Predictor::InitNetworkPredictor(PrefService* user_prefs,
                                      IOThread* io_thread,
                                      net::URLRequestContextGetter* getter,
                                      ProfileIOData* profile_io_data) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
@@ skipping 165 matching lines @@
 void Predictor::DiscardAllResultsAndClearPrefsOnUIThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::Bind(&Predictor::DiscardAllResults, weak_factory_->GetWeakPtr()));
   ClearPrefsOnUIThread();
 }
 
 void Predictor::ClearPrefsOnUIThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK(user_prefs_);
   user_prefs_->ClearPref(prefs::kDnsPrefetchingStartupList);
   user_prefs_->ClearPref(prefs::kDnsPrefetchingHostReferralList);
 }
 
 void Predictor::set_max_queueing_delay(int max_queueing_delay_ms) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   g_max_queueing_delay_ms = max_queueing_delay_ms;
 }
 
 void Predictor::set_max_parallel_resolves(size_t max_parallel_resolves) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   g_max_parallel_resolves = max_parallel_resolves;
 }
 
 void Predictor::ShutdownOnUIThread() {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  user_prefs_ = nullptr;
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&Predictor::Shutdown, base::Unretained(this)));
 }
 
 // ---------------------- End UI methods. -------------------------------------
 
 // --------------------- Start IO methods. ------------------------------------
 
@@ skipping 322 matching lines @@
         FROM_HERE,
         base::Bind(&Predictor::ResolveList, base::Unretained(this),
                    urls, motivation));
   }
 }
 
 //-----------------------------------------------------------------------------
 // Functions to handle saving of hostnames from one session to the next, to
 // expedite startup times.
 
-static void SaveDnsPrefetchStateForNextStartupOnIOThread(
-    base::ListValue* startup_list,
-    base::ListValue* referral_list,
-    base::WaitableEvent* completion,
-    Predictor* predictor) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+void Predictor::SaveStateForNextStartup() {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!predictor_enabled_ || !CanPreresolveAndPreconnect())
+    return;
 
-  if (nullptr == predictor) {
-    completion->Signal();
-    return;
-  }
-  predictor->SaveDnsPrefetchStateForNextStartup(startup_list, referral_list,
-                                                completion);
+  base::ListValue* referral_list =

[Bernhard Bauer, 2016/07/29 14:08:14]

Could you store this in a unique_ptr<>? DeepCopy() returns a raw pointer because
it's an old method, but we can do better than that now :)

[Charlie Harrison, 2016/07/29 16:50:12]

Done.

+      user_prefs_->GetList(prefs::kDnsPrefetchingHostReferralList)->DeepCopy();
+  base::ListValue* startup_list =
+      user_prefs_->GetList(prefs::kDnsPrefetchingStartupList)->DeepCopy();
+  BrowserThread::PostTaskAndReply(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&Predictor::WriteDnsPrefetchState, weak_factory_->GetWeakPtr(),
+                 startup_list, referral_list),
+      // TODO(csharrison): This should not be unretained!
+      base::Bind(&Predictor::UpdatePrefsOnUIThread, base::Unretained(this),
+                 base::Owned(startup_list), base::Owned(referral_list)));
 }
 
-void Predictor::SaveStateForNextStartup() {
-  if (!predictor_enabled_)
+// The ListValues should be Owned by the closure created in
+// SaveStateForNextStartup.
+void Predictor::UpdatePrefsOnUIThread(base::ListValue* startup_list,
+                                      base::ListValue* referral_list) {
+  // Don't perform an update if the Predictor has been shut down on the UI
+  // thread.
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  if (!user_prefs_)
     return;
-  if (!CanPreresolveAndPreconnect())
-    return;
+  ListPrefUpdate startup_update(user_prefs_, prefs::kDnsPrefetchingStartupList);
+  ListPrefUpdate referral_update(user_prefs_,
+                                 prefs::kDnsPrefetchingHostReferralList);
 
-  base::WaitableEvent completion(
-      base::WaitableEvent::ResetPolicy::MANUAL,
-      base::WaitableEvent::InitialState::NOT_SIGNALED);
-
-  ListPrefUpdate update_startup_list(user_prefs_,
-                                     prefs::kDnsPrefetchingStartupList);
-  ListPrefUpdate update_referral_list(user_prefs_,
-                                      prefs::kDnsPrefetchingHostReferralList);
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    SaveDnsPrefetchStateForNextStartupOnIOThread(update_startup_list.Get(),
-                                                 update_referral_list.Get(),
-                                                 &completion, this);
-  } else {
-    bool posted = BrowserThread::PostTask(
-        BrowserThread::IO, FROM_HERE,
-        base::Bind(&SaveDnsPrefetchStateForNextStartupOnIOThread,
-                   update_startup_list.Get(), update_referral_list.Get(),
-                   &completion, this));
-
-    // TODO(jar): Synchronous waiting for the IO thread is a potential source
-    // to deadlocks and should be investigated. See http://crbug.com/78451.
-    DCHECK(posted);
-    if (posted) {
-      // http://crbug.com/124954
-      base::ThreadRestrictions::ScopedAllowWait allow_wait;
-      completion.Wait();
-    }
-  }
+  startup_update->Swap(startup_list);

[Bernhard Bauer, 2016/07/29 14:08:14]

You can just call user_prefs_->Set(key, value). ListPrefUpdate() is for making
modifications in-place.

[Charlie Harrison, 2016/07/29 16:50:12]

Much better :) done.

+  referral_update->Swap(referral_list);
 }
 
-void Predictor::SaveDnsPrefetchStateForNextStartup(
-    base::ListValue* startup_list,
-    base::ListValue* referral_list,
-    base::WaitableEvent* completion) {
+void Predictor::WriteDnsPrefetchState(base::ListValue* startup_list,
+                                      base::ListValue* referral_list) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (initial_observer_.get())
     initial_observer_->GetInitialDnsResolutionList(startup_list);
 
   SerializeReferrers(referral_list);
-
-  completion->Signal();
 }
 
 void Predictor::PreconnectUrl(const GURL& url,
                               const GURL& first_party_for_cookies,
                               UrlInfo::ResolutionMotivation motivation,
                               bool allow_credentials,
                               int count) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
 
@@ skipping 471 matching lines @@
 }
 
 void SimplePredictor::ShutdownOnUIThread() {
   SetShutdown(true);
 }
 
 bool SimplePredictor::CanPrefetchAndPrerender() const { return true; }
 bool SimplePredictor::CanPreresolveAndPreconnect() const { return true; }
 
 }  // namespace chrome_browser_net