Enable TCP preconnect and DNS preresolve on cellular.

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 fea0171d5f284050519fded47c939a0b70537b36..1900d9ea1d6cde693069b3083450ef9a8c09d5f9 100644
--- a/chrome/browser/net/predictor.cc
+++ b/chrome/browser/net/predictor.cc
@@ -229,12 +229,10 @@ void Predictor::InitNetworkPredictor(PrefService* user_prefs,
 
 void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (!predictor_enabled_)
+  if (!preconnect_enabled_ || !CanPreresolveAndPreconnectUI())

[mmenke, 2014/08/12 17:38:10]

I think !preconnect_enabled_ should go down to where preconnectable is?  At
least I don't believe it should disable presresolve as well, if we want to keep
the old behavior.

[Bence, 2014/08/12 20:21:56]

Done.

     return;
   if (!url.is_valid() || !url.has_host())
     return;
-  if (!CanPredictNetworkActionsUI())
-    return;
 
   std::string host = url.HostNoBrackets();
   bool is_new_host_request = (host != last_omnibox_host_);
@@ -243,44 +241,42 @@ void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
   UrlInfo::ResolutionMotivation motivation(UrlInfo::OMNIBOX_MOTIVATED);
   base::TimeTicks now = base::TimeTicks::Now();
 
-  if (preconnect_enabled_) {
-    if (preconnectable && !is_new_host_request) {
-      ++consecutive_omnibox_preconnect_count_;
-      // The omnibox suggests a search URL (for which we can preconnect) after
-      // one or two characters are typed, even though such typing often (1 in
-      // 3?) becomes a real URL.  This code waits till is has more evidence of a
-      // preconnectable URL (search URL) before forming a preconnection, so as
-      // to reduce the useless preconnect rate.
-      // Perchance this logic should be pushed back into the omnibox, where the
-      // actual characters typed, such as a space, can better forcast whether
-      // we need to search/preconnect or not.  By waiting for at least 4
-      // characters in a row that have lead to a search proposal, we avoid
-      // preconnections for a prefix like "www." and we also wait until we have
-      // at least a 4 letter word to search for.
-      // Each character typed appears to induce 2 calls to
-      // AnticipateOmniboxUrl(), so we double 4 characters and limit at 8
-      // requests.
-      // TODO(jar): Use an A/B test to optimize this.
-      const int kMinConsecutiveRequests = 8;
-      if (consecutive_omnibox_preconnect_count_ >= kMinConsecutiveRequests) {
-        // TODO(jar): Perhaps we should do a GET to leave the socket open in the
-        // pool.  Currently, we just do a connect, which MAY be reset if we
-        // don't use it in 10 secondes!!!  As a result, we may do more
-        // connections, and actually cost the server more than if we did a real
-        // get with a fake request (/gen_204 might be the good path on Google).
-        const int kMaxSearchKeepaliveSeconds(10);
-        if ((now - last_omnibox_preconnect_).InSeconds() <
-             kMaxSearchKeepaliveSeconds)
-          return;  // We've done a preconnect recently.
-        last_omnibox_preconnect_ = now;
-        const int kConnectionsNeeded = 1;
-        PreconnectUrl(CanonicalizeUrl(url), GURL(), motivation,
-                      kConnectionsNeeded);
-        return;  // Skip pre-resolution, since we'll open a connection.
-      }
-    } else {
-      consecutive_omnibox_preconnect_count_ = 0;
+  if (preconnectable && !is_new_host_request) {
+    ++consecutive_omnibox_preconnect_count_;
+    // The omnibox suggests a search URL (for which we can preconnect) after
+    // one or two characters are typed, even though such typing often (1 in
+    // 3?) becomes a real URL.  This code waits till is has more evidence of a
+    // preconnectable URL (search URL) before forming a preconnection, so as
+    // to reduce the useless preconnect rate.
+    // Perchance this logic should be pushed back into the omnibox, where the
+    // actual characters typed, such as a space, can better forcast whether
+    // we need to search/preconnect or not.  By waiting for at least 4
+    // characters in a row that have lead to a search proposal, we avoid
+    // preconnections for a prefix like "www." and we also wait until we have
+    // at least a 4 letter word to search for.
+    // Each character typed appears to induce 2 calls to
+    // AnticipateOmniboxUrl(), so we double 4 characters and limit at 8
+    // requests.
+    // TODO(jar): Use an A/B test to optimize this.
+    const int kMinConsecutiveRequests = 8;
+    if (consecutive_omnibox_preconnect_count_ >= kMinConsecutiveRequests) {
+      // TODO(jar): Perhaps we should do a GET to leave the socket open in the
+      // pool.  Currently, we just do a connect, which MAY be reset if we
+      // don't use it in 10 secondes!!!  As a result, we may do more
+      // connections, and actually cost the server more than if we did a real
+      // get with a fake request (/gen_204 might be the good path on Google).
+      const int kMaxSearchKeepaliveSeconds(10);
+      if ((now - last_omnibox_preconnect_).InSeconds() <
+          kMaxSearchKeepaliveSeconds)
+        return;  // We've done a preconnect recently.
+      last_omnibox_preconnect_ = now;
+      const int kConnectionsNeeded = 1;
+      PreconnectUrl(
+          CanonicalizeUrl(url), GURL(), motivation, kConnectionsNeeded);
+      return;  // Skip pre-resolution, since we'll open a connection.
     }
+  } else {
+    consecutive_omnibox_preconnect_count_ = 0;
   }
 
   // Fall through and consider pre-resolution.
@@ -311,10 +307,10 @@ void Predictor::PreconnectUrlAndSubresources(const GURL& url,
     return;
 
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    if (!CanPredictNetworkActionsUI())
+    if (!CanPreresolveAndPreconnectUI())
       return;
   } else {
-    if (!CanPredictNetworkActionsIO())
+    if (!CanPreresolveAndPreconnectIO())
       return;
   }
 
@@ -473,7 +469,7 @@ void Predictor::Resolve(const GURL& url,
 void Predictor::LearnFromNavigation(const GURL& referring_url,
                                     const GURL& target_url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!predictor_enabled_ || !CanPredictNetworkActionsIO())
+  if (!predictor_enabled_ || !CanPrefetchAndPrerenderIO())
     return;
   DCHECK_EQ(referring_url, Predictor::CanonicalizeUrl(referring_url));
   DCHECK_NE(referring_url, GURL::EmptyGURL());
@@ -497,7 +493,7 @@ void Predictor::PredictorGetHtmlInfo(Predictor* predictor,
                  // "<META HTTP-EQUIV=\"Pragma\" CONTENT=\"no-cache\">"
                  "</head><body>");
   if (predictor && predictor->predictor_enabled() &&
-      predictor->CanPredictNetworkActionsIO()) {
+      predictor->CanPrefetchAndPrerenderIO()) {
     predictor->GetHtmlInfo(output);
   } else {
     output->append("DNS pre-resolution and TCP pre-connection is disabled.");
@@ -728,7 +724,7 @@ void Predictor::FinalizeInitializationOnIOThread(
 void Predictor::LearnAboutInitialNavigation(const GURL& url) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!predictor_enabled_ || NULL == initial_observer_.get() ||
-      !CanPredictNetworkActionsIO())
+      !CanPrefetchAndPrerenderIO())
     return;

[mmenke, 2014/08/12 17:38:10]

nit:  While you're here, mind adding braces?

[Bence, 2014/08/12 20:21:56]

Done.

   initial_observer_->Append(url, this);
 }
@@ -760,10 +756,10 @@ void Predictor::DnsPrefetchMotivatedList(
     return;
 
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    if (!CanPredictNetworkActionsUI())
+    if (!CanPrefetchAndPrerenderUI())
       return;
   } else {
-    if (!CanPredictNetworkActionsIO())
+    if (!CanPrefetchAndPrerenderIO())
       return;
   }
 
@@ -802,10 +798,10 @@ void Predictor::SaveStateForNextStartupAndTrim() {
     return;
 
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    if (!CanPredictNetworkActionsUI())
+    if (!CanPrefetchAndPrerenderUI())
       return;
   } else {
-    if (!CanPredictNetworkActionsIO())
+    if (!CanPrefetchAndPrerenderIO())
       return;
   }
 
@@ -908,10 +904,10 @@ void Predictor::PredictFrameSubresources(const GURL& url,
     return;
 
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    if (!CanPredictNetworkActionsUI())
+    if (!CanPrefetchAndPrerenderUI())
       return;
   } else {
-    if (!CanPredictNetworkActionsIO())
+    if (!CanPrefetchAndPrerenderIO())
       return;
   }
   DCHECK_EQ(url.GetWithEmptyPath(), url);
@@ -948,12 +944,20 @@ void Predictor::AdviseProxy(const GURL& url,
   }
 }
 
-bool Predictor::CanPredictNetworkActionsUI() {
-  return chrome_browser_net::CanPredictNetworkActionsUI(user_prefs_);
+bool Predictor::CanPrefetchAndPrerenderUI() const {

[mmenke, 2014/08/12 17:38:10]

Suggest adding DCHECKs for which thread we're on in these functions.

[mmenke, 2014/08/12 17:38:10]

optional:  It's marginally slower, but you might want to merge these into a
single function that both figures out what thread it's on, and then calls it,
and completely get rid of the thread-dependent versions.  we're doing the checks
half the time, anyways, and I don't think we're making calls often enough for
the performance to matter.

[Bence, 2014/08/12 20:21:55]

Done.

[Bence, 2014/08/12 20:21:56]

Done.

+  return chrome_browser_net::CanPrefetchAndPrerenderUI(user_prefs_);
+}
+
+bool Predictor::CanPrefetchAndPrerenderIO() const {
+  return chrome_browser_net::CanPrefetchAndPrerenderIO(profile_io_data_);
+}
+
+bool Predictor::CanPreresolveAndPreconnectUI() const {
+  return chrome_browser_net::CanPreresolveAndPreconnectUI(user_prefs_);
 }
 
-bool Predictor::CanPredictNetworkActionsIO() {
-  return chrome_browser_net::CanPredictNetworkActionsIO(profile_io_data_);
+bool Predictor::CanPreresolveAndPreconnectIO() const {
+  return chrome_browser_net::CanPreresolveAndPreconnectIO(profile_io_data_);
 }
 
 enum SubresourceValue {
@@ -1345,7 +1349,9 @@ void SimplePredictor::ShutdownOnUIThread() {
   SetShutdown(true);
 }
 
-bool SimplePredictor::CanPredictNetworkActionsUI() { return true; }
-bool SimplePredictor::CanPredictNetworkActionsIO() { return true; }
+bool SimplePredictor::CanPrefetchAndPrerenderUI() const { return true; }
+bool SimplePredictor::CanPrefetchAndPrerenderIO() const { return true; }
+bool SimplePredictor::CanPreresolveAndPreconnectUI() const { return true; }
+bool SimplePredictor::CanPreresolveAndPreconnectIO() const { return true; }
 
 }  // namespace chrome_browser_net