Make Network Predictor skip empty urls

chrome/browser/net/predictor_browsertest.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 <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
 #include <memory>
 #include <set>
@@ skipping 302 matching lines @@
         same_site_preconnected_(0),
         dns_run_loop_(nullptr),
         strict_(true) {}
 
   void OnPreconnectUrl(
       const GURL& original_url,
       const GURL& first_party_for_cookies,
       chrome_browser_net::UrlInfo::ResolutionMotivation motivation,
       int count) override {
     base::AutoLock lock(lock_);
+    preconnect_url_attempts_.insert(original_url);

[Charlie Harrison, 2017/05/15 17:01:25]

Does OnDnsLookupFinished trigger for these bad URLs? If so it would be preferred
to use unsuccessful_dns_lookups / CheckForWaitingLoop so you don't need
RunUntilIdle().

[xunjieli, 2017/05/15 17:06:51]

Unfortunately no. OnDnsLookupFinished() is only used for presolve.

[xunjieli, 2017/05/15 17:08:56]

s/presolve/preresolve :)

     if (original_url == cross_site_host_) {
       cross_site_preconnected_ = std::max(cross_site_preconnected_, count);
     } else if (original_url == source_host_) {
       same_site_preconnected_ = std::max(same_site_preconnected_, count);
     } else if (strict_) {
       ADD_FAILURE() << "Preconnected " << original_url
                     << " when should only be preconnecting the source host: "
                     << source_host_
                     << " or the cross site host: " << cross_site_host_;
     }
@@ skipping 71 matching lines @@
     lock_.AssertAcquired();
     return base::ContainsKey(successful_dns_lookups_, url) ||
            base::ContainsKey(unsuccessful_dns_lookups_, url);
   }
 
   bool HasHostBeenLookedUp(const GURL& url) {
     base::AutoLock lock(lock_);
     return HasHostBeenLookedUpLocked(url);
   }
 
+  bool HasHostAttemptedToPreconnect(const GURL& url) {
+    base::AutoLock lock(lock_);
+    return base::ContainsKey(preconnect_url_attempts_, url);
+  }
+
   void CheckForWaitingLoop() {
     lock_.AssertAcquired();
     if (waiting_on_dns_.is_empty())
       return;
     if (!HasHostBeenLookedUpLocked(waiting_on_dns_))
       return;
     DCHECK(dns_run_loop_);
     DCHECK(task_runner_);
     waiting_on_dns_ = GURL();
     task_runner_->PostTask(FROM_HERE, dns_run_loop_->QuitClosure());
@@ skipping 34 matching lines @@
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   // Protects all following members. They are read and updated from different
   // threads.
   base::Lock lock_;
 
   int cross_site_learned_;
   int cross_site_preconnected_;
   int same_site_preconnected_;
 
+  std::set<GURL> preconnect_url_attempts_;
   std::set<GURL> successful_dns_lookups_;
   std::set<GURL> unsuccessful_dns_lookups_;
   base::RunLoop* dns_run_loop_;
 
   // This member can be set to optionally allow url learning other than from
   // source => source, source => target, or target => target. It will also allow
   // preconnects to other hosts.
   bool strict_;
 
   DISALLOW_COPY_AND_ASSIGN(CrossSitePredictorObserver);
@@ skipping 390 matching lines @@
 }
 
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SimplePreconnectFour) {
   predictor()->PreconnectUrl(
       embedded_test_server()->base_url(), GURL(),
       UrlInfo::ResolutionMotivation::EARLY_LOAD_MOTIVATED,
       false /* allow credentials */, 4);
   connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
 }
 
+// Regression test for crbug.com/721981.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectNonHttpScheme) {
+  GURL url("chrome-native://dummyurl");
+  predictor()->PreconnectUrlAndSubresources(url, GURL());
+  base::RunLoop().RunUntilIdle();
+  // Since |url| is non-HTTP(s) scheme, Predictor will canonicalize it to an
+  // empty url. Make sure that there is no attempt to preconnect |url| or an
+  // empty url.
+  EXPECT_FALSE(observer()->HasHostAttemptedToPreconnect(url));
+  EXPECT_FALSE(observer()->HasHostAttemptedToPreconnect(GURL()));
+}
+
 // Test the html test harness used to initiate cross site fetches. These
 // initiate cross site subresource requests to the cross site test server.
 // Inspect the predictor's internal state to make sure that they are properly
 // logged.
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, CrossSiteUseOneSocket) {
   StopInterceptingCrossSiteOnUI();
   NavigateToCrossSiteHtmlUrl(1 /* num_cors */, "" /* file_suffix */);
   EXPECT_EQ(1u, cross_site_connection_listener_->GetAcceptedSocketCount());
   EXPECT_EQ(1, observer()->CrossSiteLearned());
   EXPECT_EQ(2, observer()->SameSitePreconnected());
@@ skipping 677 matching lines @@
   // Second navigation to content with an img.
   std::string img_content =
       "<img src=\"" + preconnect_url.spec() + "test.gif\">";
   NavigateToDataURLWithContent(img_content);
   connection_listener_->WaitUntilFirstConnectionRead();
   EXPECT_EQ(2u, connection_listener_->GetAcceptedSocketCount());
   EXPECT_EQ(1u, connection_listener_->GetReadSocketCount());
 }
 
 }  // namespace chrome_browser_net