Poll CanPredictNetworkActions in Predictor class.

chrome/browser/net/predictor.cc

Index: chrome/browser/net/predictor.cc
diff --git a/chrome/browser/net/predictor.cc b/chrome/browser/net/predictor.cc
index 3c4381e5a51dcd197c78a4d73bcba314ac23eb2e..4e9c45309c70f99073eaac8e6d6d04adb6399e57 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -26,8 +26,10 @@
 #include "base/values.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/browser/net/preconnect.h"
+#include "chrome/browser/net/prediction_options.h"
 #include "chrome/browser/net/spdyproxy/proxy_advisor.h"
 #include "chrome/browser/prefs/session_startup_pref.h"
+#include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/data_reduction_proxy/browser/data_reduction_proxy_settings.h"
@@ -133,9 +135,11 @@ class Predictor::LookupRequest {
   DISALLOW_COPY_AND_ASSIGN(LookupRequest);
 };
 
-Predictor::Predictor(bool preconnect_enabled)
+Predictor::Predictor(bool preconnect_enabled, bool predictor_enabled)
     : url_request_context_getter_(NULL),
-      predictor_enabled_(true),
+      predictor_enabled_(predictor_enabled),
+      user_prefs_(NULL),
+      profile_io_data_(NULL),
       peak_pending_lookups_(0),
       shutdown_(false),
       max_concurrent_dns_lookups_(g_max_parallel_resolves),
@@ -160,10 +164,11 @@ Predictor::~Predictor() {
 
 // static
 Predictor* Predictor::CreatePredictor(bool preconnect_enabled,
+                                      bool predictor_enabled,
                                       bool simple_shutdown) {
   if (simple_shutdown)
-    return new SimplePredictor(preconnect_enabled);
-  return new Predictor(preconnect_enabled);
+    return new SimplePredictor(preconnect_enabled, predictor_enabled);
+  return new Predictor(preconnect_enabled, predictor_enabled);
 }
 
 void Predictor::RegisterProfilePrefs(
@@ -179,12 +184,11 @@ void Predictor::RegisterProfilePrefs(
 void Predictor::InitNetworkPredictor(PrefService* user_prefs,
                                      PrefService* local_state,
                                      IOThread* io_thread,
-                                     net::URLRequestContextGetter* getter) {
+                                     net::URLRequestContextGetter* getter,
+                                     ProfileIOData* profile_io_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  bool predictor_enabled =
-      user_prefs->GetBoolean(prefs::kNetworkPredictionEnabled);
-
+  user_prefs_ = user_prefs;
   url_request_context_getter_ = getter;
 
   // Gather the list of hostnames to prefetch on startup.
@@ -220,7 +224,7 @@ void Predictor::InitNetworkPredictor(PrefService* user_prefs,
           &Predictor::FinalizeInitializationOnIOThread,
           base::Unretained(this),
           urls, referral_list,
-          io_thread, predictor_enabled));
+          io_thread, profile_io_data));
 }
 
 void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
@@ -236,7 +240,8 @@ void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
   UrlInfo::ResolutionMotivation motivation(UrlInfo::OMNIBOX_MOTIVATED);
   base::TimeTicks now = base::TimeTicks::Now();
 
-  if (preconnect_enabled()) {
+  if (preconnect_enabled_ &&
+      chrome_browser_net::CanPredictNetworkActionsUI(user_prefs_)) {
     if (preconnectable && !is_new_host_request) {
       ++consecutive_omnibox_preconnect_count_;
       // The omnibox suggests a search URL (for which we can preconnect) after
@@ -299,10 +304,18 @@ void Predictor::PreconnectUrlAndSubresources(const GURL& url,
     const GURL& first_party_for_cookies) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!predictor_enabled_ || !preconnect_enabled() ||
+  if (!predictor_enabled_ || !preconnect_enabled_ ||
       !url.is_valid() || !url.has_host())
     return;
 
+  if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    if (!chrome_browser_net::CanPredictNetworkActionsUI(user_prefs_))
+      return;
+  } else {
+    if (!chrome_browser_net::CanPredictNetworkActionsIO(profile_io_data_))
+      return;
+  }
+
   UrlInfo::ResolutionMotivation motivation(UrlInfo::EARLY_LOAD_MOTIVATED);
   const int kConnectionsNeeded = 1;
   PreconnectUrl(CanonicalizeUrl(url), first_party_for_cookies,
@@ -481,7 +494,9 @@ void Predictor::PredictorGetHtmlInfo(Predictor* predictor,
                  // We'd like the following no-cache... but it doesn't work.
                  // "<META HTTP-EQUIV=\"Pragma\" CONTENT=\"no-cache\">"
                  "</head><body>");
-  if (predictor && predictor->predictor_enabled()) {
+  if (predictor && predictor->predictor_enabled() &&
+      chrome_browser_net::CanPredictNetworkActionsIO(
+          predictor->profile_io_data())) {
     predictor->GetHtmlInfo(output);
   } else {
     output->append("DNS pre-resolution and TCP pre-connection is disabled.");
@@ -678,10 +693,10 @@ void Predictor::FinalizeInitializationOnIOThread(
     const UrlList& startup_urls,
     base::ListValue* referral_list,
     IOThread* io_thread,
-    bool predictor_enabled) {
+    ProfileIOData* profile_io_data) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
-  predictor_enabled_ = predictor_enabled;
+  profile_io_data_ = profile_io_data;
   initial_observer_.reset(new InitialObserver());
   host_resolver_ = io_thread->globals()->host_resolver.get();
 
@@ -772,14 +787,15 @@ static void SaveDnsPrefetchStateForNextStartupAndTrimOnIOThread(
       startup_list, referral_list, completion);
 }
 
-void Predictor::SaveStateForNextStartupAndTrim(PrefService* prefs) {
+void Predictor::SaveStateForNextStartupAndTrim() {
   if (!predictor_enabled_)
     return;
 
   base::WaitableEvent completion(true, false);
 
-  ListPrefUpdate update_startup_list(prefs, prefs::kDnsPrefetchingStartupList);
-  ListPrefUpdate update_referral_list(prefs,
+  ListPrefUpdate update_startup_list(user_prefs_,
+                                     prefs::kDnsPrefetchingStartupList);
+  ListPrefUpdate update_referral_list(user_prefs_,
                                       prefs::kDnsPrefetchingHostReferralList);

[battre, 2014/07/10 09:01:59]

This is actually a pretty serious bug that existed before and should be fixed.
You may not instantiate a ListPrefUpdate and/or use a ListPrefUpdate if you are
on the IO thread. All modifications are directly written into a the memory of
the PrefService. Therefore, this is pretty racy.

CCing original author (rlp) as FYI.

[Bernhard Bauer, 2014/07/10 09:59:02]

I think in this particular case, we're okay. The only place where this method
seems to be called is from the ProfileImplIOData::Handle destructor, which
DCHECKs that it's on the UI thread. I suspect the reason why we have the check
below is because this might run in a unit test where everything is mapped to a
single thread.

I'm still in favor of adding those DCHECKs to ScopedUserPrefUpdate though.

   if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     SaveDnsPrefetchStateForNextStartupAndTrimOnIOThread(
@@ -825,26 +841,6 @@ void Predictor::SaveDnsPrefetchStateForNextStartupAndTrim(
   completion->Signal();
 }
 
-void Predictor::EnablePredictor(bool enable) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
-         BrowserThread::CurrentlyOn(BrowserThread::IO));
-
-  if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
-    EnablePredictorOnIOThread(enable);
-  } else {
-    BrowserThread::PostTask(
-        BrowserThread::IO,
-        FROM_HERE,
-        base::Bind(&Predictor::EnablePredictorOnIOThread,
-                   base::Unretained(this), enable));
-  }
-}
-
-void Predictor::EnablePredictorOnIOThread(bool enable) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  predictor_enabled_ = enable;
-}
-
 void Predictor::PreconnectUrl(const GURL& url,
                               const GURL& first_party_for_cookies,
                               UrlInfo::ResolutionMotivation motivation,
@@ -949,7 +945,8 @@ void Predictor::PrepareFrameSubresources(const GURL& original_url,
     // size of the list with all the "Leaf" nodes in the tree (nodes that don't
     // load any subresources).  If we learn about this resource, we will instead
     // provide a more carefully estimated preconnection count.
-    if (preconnect_enabled_) {
+    if (preconnect_enabled_ &&
+        chrome_browser_net::CanPredictNetworkActionsIO(profile_io_data_)) {
       PreconnectUrlOnIOThread(url, first_party_for_cookies,
                               UrlInfo::SELF_REFERAL_MOTIVATED, 2);
     }
@@ -969,6 +966,7 @@ void Predictor::PrepareFrameSubresources(const GURL& original_url,
                                 10, 5000, 50);
     future_url->second.ReferrerWasObserved();
     if (preconnect_enabled_ &&
+        chrome_browser_net::CanPredictNetworkActionsIO(profile_io_data_) &&
         connection_expectation > kPreconnectWorthyExpectedValue) {
       evalution = PRECONNECTION;
       future_url->second.IncrementPreconnectionCount();
@@ -1306,7 +1304,8 @@ void SimplePredictor::InitNetworkPredictor(
     PrefService* user_prefs,
     PrefService* local_state,
     IOThread* io_thread,
-    net::URLRequestContextGetter* getter) {
+    net::URLRequestContextGetter* getter,
+    ProfileIOData* profile_io_data) {
   // Empty function for unittests.
 }