Make Network Predictor skip empty urls

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>
@@ skipping 153 matching lines @@
       base::BindOnce(&Predictor::FinalizeInitializationOnIOThread,
                      base::Unretained(this), urls,
                      base::Passed(std::move(referral_list)), io_thread,
                      profile_io_data));
 }
 
 void Predictor::AnticipateOmniboxUrl(const GURL& url, bool preconnectable) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (!PredictorEnabled())
     return;
-  if (!url.is_valid() || !url.has_host())
+  GURL canonicalized_url = CanonicalizeUrl(url);

[Charlie Harrison, 2017/05/15 00:01:07]

Here and below can you make sure we canoncalize the URL only if
preconnect/preresolve is enabled? The URL canonicalization we do is kinda
expensive.

[xunjieli, 2017/05/15 16:41:44]

Done.

+  if (!canonicalized_url.is_valid() || !canonicalized_url.has_host())
     return;
   if (!CanPreresolveAndPreconnect())
     return;
 
-  std::string host = url.HostNoBrackets();
+  std::string host = canonicalized_url.HostNoBrackets();
   bool is_new_host_request = (host != last_omnibox_host_);
   last_omnibox_host_ = host;
 
   UrlInfo::ResolutionMotivation motivation(UrlInfo::OMNIBOX_MOTIVATED);
   base::TimeTicks now = base::TimeTicks::Now();
 
   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
@@ skipping 15 matching lines @@
       // 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,
+      PreconnectUrl(canonicalized_url, GURL(), motivation,
                     kAllowCredentialsOnPreconnectByDefault, kConnectionsNeeded);
       return;  // Skip pre-resolution, since we'll open a connection.
     }
   } else {
     consecutive_omnibox_preconnect_count_ = 0;
   }
 
   // Fall through and consider pre-resolution.
 
   // Omnibox tends to call in pairs (just a few milliseconds apart), and we
   // really don't need to keep resolving a name that often.
   // TODO(jar): A/B tests could check for perf impact of the early returns.
   if (!is_new_host_request) {
     const int kMinPreresolveSeconds(10);
     if (kMinPreresolveSeconds > (now - last_omnibox_preresolve_).InSeconds())
       return;
   }
   last_omnibox_preresolve_ = now;
 
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       base::BindOnce(&Predictor::Resolve, base::Unretained(this),
-                     CanonicalizeUrl(url), motivation));
+                     canonicalized_url, motivation));
 }
 
 void Predictor::PreconnectUrlAndSubresources(const GURL& url,
     const GURL& first_party_for_cookies) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI) ||
          BrowserThread::CurrentlyOn(BrowserThread::IO));
-  if (!PredictorEnabled() || !url.is_valid() ||
-      !url.has_host())
+  GURL canonicalized_url = CanonicalizeUrl(url);
+  if (!PredictorEnabled() || !canonicalized_url.is_valid() ||
+      !canonicalized_url.has_host())
     return;
   if (!CanPreresolveAndPreconnect())
     return;
 
   UrlInfo::ResolutionMotivation motivation(UrlInfo::EARLY_LOAD_MOTIVATED);
   const int kConnectionsNeeded = 1;
-  PreconnectUrl(CanonicalizeUrl(url), first_party_for_cookies, motivation,
+  PreconnectUrl(canonicalized_url, first_party_for_cookies, motivation,
                 kConnectionsNeeded, kAllowCredentialsOnPreconnectByDefault);
-  PredictFrameSubresources(url.GetWithEmptyPath(), first_party_for_cookies);
+  PredictFrameSubresources(canonicalized_url.GetWithEmptyPath(),
+                           first_party_for_cookies);
 }
 
 std::vector<GURL> Predictor::GetPredictedUrlListAtStartup(
     PrefService* user_prefs) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   std::vector<GURL> urls;
   // Recall list of URLs we learned about during last session.
   // This may catch secondary hostnames, pulled in by the homepages.  It will
   // also catch more of the "primary" home pages, since that was (presumably)
   // rendered first (and will be rendered first this time too).
@@ skipping 452 matching lines @@
   SerializeReferrers(referral_list);
 }
 
 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));
+  DCHECK(url.is_valid());
 
   if (BrowserThread::CurrentlyOn(BrowserThread::IO)) {
     PreconnectUrlOnIOThread(url, first_party_for_cookies, motivation,
                             allow_credentials, count);
   } else {
     BrowserThread::PostTask(
         BrowserThread::IO, FROM_HERE,
         base::BindOnce(&Predictor::PreconnectUrlOnIOThread,
                        base::Unretained(this), url, first_party_for_cookies,
                        motivation, allow_credentials, count));
@@ skipping 459 matching lines @@
 }
 
 void SimplePredictor::ShutdownOnUIThread() {
   SetShutdown(true);
 }
 
 bool SimplePredictor::CanPrefetchAndPrerender() const { return true; }
 bool SimplePredictor::CanPreresolveAndPreconnect() const { return true; }
 
 }  // namespace chrome_browser_net