Move predictor dns unit tests to browser tests

chrome/browser/net/predictor_browsertest.cc

Index: chrome/browser/net/predictor_browsertest.cc
diff --git a/chrome/browser/net/predictor_browsertest.cc b/chrome/browser/net/predictor_browsertest.cc
index be8568ae32814a97956936b9c6ac16cc3cb4b7b7..712d5b418faa18f446b1eb6ef78ba7cc3a1e0674 100644
--- a/chrome/browser/net/predictor_browsertest.cc
+++ b/chrome/browser/net/predictor_browsertest.cc
@@ -249,56 +249,79 @@ class ConnectionListener
 // and immediately fails the resolution requests themselves.
 class HostResolutionRequestRecorder : public net::HostResolverProc {
  public:
-  HostResolutionRequestRecorder()
-      : HostResolverProc(NULL),
-        is_waiting_for_hostname_(false) {
-  }
+  explicit HostResolutionRequestRecorder(net::HostResolverProc* previous)
+      : HostResolverProc(previous), run_loop_(nullptr) {}
 
   int Resolve(const std::string& host,
               net::AddressFamily address_family,
               net::HostResolverFlags host_resolver_flags,
               net::AddressList* addrlist,
               int* os_error) override {
+    // Note the rule based proc sleep synchronously.

[Randy Smith (Not in Mondays), 2016/05/23 19:55:18]

Sorry, I'm getting past the lexing phase, but failing to parse.  What does this
sentence mean?

[Charlie Harrison, 2016/05/24 16:57:06]

Hahaha yeah that's extremely confusing. I meant the rule based host resolver
proc *sleeps* synchronously, so AddToHistory call will be after the resolution
is performed.

+    int result = ResolveUsingPrevious(host, address_family, host_resolver_flags,
+                                      addrlist, os_error);
     BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
+        BrowserThread::IO, FROM_HERE,
         base::Bind(&HostResolutionRequestRecorder::AddToHistory,
-                   base::Unretained(this),
-                   host));
-    return net::ERR_NAME_NOT_RESOLVED;
+                   base::Unretained(this), host));
+    return result;
   }
 
   int RequestedHostnameCount() const {
+    base::AutoLock lock(lock_);
     return requested_hostnames_.size();
   }
 
   bool HasHostBeenRequested(const std::string& hostname) const {
+    base::AutoLock lock(lock_);
+    return HasHostBeenRequestedLocked(hostname);
+  }
+
+  bool HasHostBeenRequestedLocked(const std::string& hostname) const {
+    lock_.AssertAcquired();
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     return std::find(requested_hostnames_.begin(),
                      requested_hostnames_.end(),
                      hostname) != requested_hostnames_.end();
   }
 
+  void WaitUntilHostNamesHaveBeenRequested(
+      const network_hints::UrlList& names) {
+    for (const auto& name : names) {
+      WaitUntilHostHasBeenRequested(name.host());
+    }
+  }
+
   void WaitUntilHostHasBeenRequested(const std::string& hostname) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    DCHECK(!is_waiting_for_hostname_);
-    if (HasHostBeenRequested(hostname))
-      return;
-    waiting_for_hostname_ = hostname;
-    is_waiting_for_hostname_ = true;
-    content::RunMessageLoop();
+    base::RunLoop run_loop;
+    {
+      base::AutoLock lock(lock_);
+      if (HasHostBeenRequestedLocked(hostname))
+        return;
+      waiting_for_hostname_ = hostname;
+      DCHECK(!run_loop_);
+      run_loop_ = &run_loop;
+    }
+    run_loop_->Run();
+  }
+
+  void set_task_runner(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    task_runner_.swap(task_runner);
   }
 
  private:
   ~HostResolutionRequestRecorder() override {}
 
   void AddToHistory(const std::string& hostname) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    base::AutoLock lock(lock_);
     requested_hostnames_.push_back(hostname);
-    if (is_waiting_for_hostname_ && waiting_for_hostname_ == hostname) {
-      is_waiting_for_hostname_ = false;
+    if (run_loop_ && waiting_for_hostname_ == hostname) {
       waiting_for_hostname_.clear();
-      base::MessageLoop::current()->QuitWhenIdle();
+      task_runner_->PostTask(FROM_HERE, run_loop_->QuitClosure());
+      run_loop_ = nullptr;
     }
   }
 
@@ -306,14 +329,17 @@ class HostResolutionRequestRecorder : public net::HostResolverProc {
   // to be requested.
   std::string waiting_for_hostname_;
 
-  // Whether WaitUntilHostHasBeenRequested is waiting for a hostname to be
-  // requested and thus is running a nested message loop.
-  bool is_waiting_for_hostname_;
-
   // A list of hostnames for which resolution has already been requested. Only
   // to be accessed from the UI thread.
   std::vector<std::string> requested_hostnames_;
 
+  base::RunLoop* run_loop_;
+
+  // Protects |waiting_for_hostname_| and |requested_hostnames_|.
+  mutable base::Lock lock_;
+
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
+
   DISALLOW_COPY_AND_ASSIGN(HostResolutionRequestRecorder);
 };
 
@@ -449,8 +475,21 @@ class PredictorBrowserTest : public InProcessBrowserTest {
       : startup_url_("http://host1:1"),
         referring_url_("http://host2:1"),
         target_url_("http://host3:1"),
-        host_resolution_request_recorder_(new HostResolutionRequestRecorder),
-        cross_site_test_server_(new net::EmbeddedTestServer()) {}
+        rule_based_resolver_proc_(new net::RuleBasedHostResolverProc(nullptr)),
+        host_resolution_request_recorder_(
+            new HostResolutionRequestRecorder(rule_based_resolver_proc_.get())),
+        cross_site_test_server_(new net::EmbeddedTestServer()) {
+    rule_based_resolver_proc_->AddRuleWithLatency("www.example.test",
+                                                  "127.0.0.1", 50);
+    rule_based_resolver_proc_->AddRuleWithLatency("gmail.google.com",
+                                                  "127.0.0.1", 70);
+    rule_based_resolver_proc_->AddRuleWithLatency("mail.google.com",
+                                                  "127.0.0.1", 44);
+    rule_based_resolver_proc_->AddRuleWithLatency("gmail.com", "127.0.0.1", 63);
+    rule_based_resolver_proc_->AddSimulatedFailure("*.notfound");
+    rule_based_resolver_proc_->AddRuleWithLatency("delay.google.com",
+                                                  "127.0.0.1", 1000 * 60);
+  }
 
  protected:
   void SetUpInProcessBrowserTestFixture() override {
@@ -466,11 +505,14 @@ class PredictorBrowserTest : public InProcessBrowserTest {
         switches::kEnableBlinkFeatures, kBlinkPreconnectFeature);
     command_line->AppendSwitchASCII(switches::kEnableFeatures,
                                     "PreconnectMore");
+    command_line->AppendSwitchASCII(switches::kEnableFeatures,
+                                    "UsePredictorDNSQueue");
   }
 
   void SetUpOnMainThread() override {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     task_runner_ = base::ThreadTaskRunnerHandle::Get();
+    host_resolution_request_recorder_->set_task_runner(task_runner_);
     cross_site_test_server_->ServeFilesFromSourceDirectory("chrome/test/data/");
 
     connection_listener_.reset(new ConnectionListener());
@@ -581,10 +623,31 @@ class PredictorBrowserTest : public InProcessBrowserTest {
     host_resolution_request_recorder_->WaitUntilHostHasBeenRequested(hostname);
   }
 
+  void WaitUntilHostNamesHaveBeenRequested(
+      const network_hints::UrlList& names) {
+    host_resolution_request_recorder_->WaitUntilHostNamesHaveBeenRequested(
+        names);
+  }
+
   int RequestedHostnameCount() const {
     return host_resolution_request_recorder_->RequestedHostnameCount();
   }
 
+  void FloodResolveRequestsOnUIThread(const network_hints::UrlList& names) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::FloodResolveRequests, this, names));
+  }
+
+  void FloodResolveRequests(const network_hints::UrlList& names) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    for (int i = 0; i < 10; i++) {
+      predictor()->DnsPrefetchMotivatedList(names,
+                                            UrlInfo::PAGE_SCAN_MOTIVATED);
+    }
+  }
+
   net::EmbeddedTestServer* cross_site_test_server() {
     return cross_site_test_server_.get();
   }
@@ -639,6 +702,31 @@ class PredictorBrowserTest : public InProcessBrowserTest {
     EXPECT_TRUE(test_server->FlushAllSocketsAndConnectionsOnUIThread());
   }
 
+  void ExpectFoundUrlsOnUIThread(
+      const network_hints::UrlList& found_names,
+      const network_hints::UrlList& not_found_names) {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&PredictorBrowserTest::ExpectFoundUrls,
+                                       this, found_names, not_found_names));
+  }
+  void ExpectFoundUrls(const network_hints::UrlList& found_names,
+                       const network_hints::UrlList& not_found_names) {
+    for (const auto& name : found_names) {
+      EXPECT_TRUE(predictor()->WasFound(name)) << "Expected to have found "
+                                               << name.spec();
+    }
+    for (const auto& name : not_found_names) {
+      EXPECT_FALSE(predictor()->WasFound(name)) << "Did not expect to find "
+                                                << name.spec();
+    }
+  }
+
+  void DiscardAllResultsOnUIThread() {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&Predictor::DiscardAllResults,
+                                       base::Unretained(predictor())));
+  }
+
   CrossSitePredictorObserver* observer() { return observer_.get(); }
 
   // Navigate to an html file on embedded_test_server and tell it to request
@@ -669,6 +757,7 @@ class PredictorBrowserTest : public InProcessBrowserTest {
   std::unique_ptr<ConnectionListener> cross_site_connection_listener_;
 
  private:
+  scoped_refptr<net::RuleBasedHostResolverProc> rule_based_resolver_proc_;
   scoped_refptr<HostResolutionRequestRecorder>
       host_resolution_request_recorder_;
   std::unique_ptr<net::ScopedDefaultHostResolverProc>
@@ -678,6 +767,84 @@ class PredictorBrowserTest : public InProcessBrowserTest {
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 };
 
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SingleLookupTest) {
+  DiscardAllResultsOnUIThread();
+
+  GURL goog("http://www.example.test:80");
+
+  network_hints::UrlList names{goog};
+
+  // Try to flood the predictor with many concurrent requests.
+  FloodResolveRequestsOnUIThread(names);
+  WaitUntilHostNamesHaveBeenRequested(names);
+  base::MessageLoop::current()->RunUntilIdle();
+  ExpectFoundUrlsOnUIThread(names, network_hints::UrlList());
+
+  EXPECT_GT(predictor()->peak_pending_lookups(), names.size() / 2);
+  EXPECT_LE(predictor()->peak_pending_lookups(), names.size());
+  EXPECT_LE(predictor()->peak_pending_lookups(),
+            predictor()->max_concurrent_dns_lookups());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ConcurrentLookupTest) {
+  DiscardAllResultsOnUIThread();
+
+  GURL goog("http://www.example.test:80"), goog2("http://gmail.google.com:80"),
+      goog3("http://mail.google.com:80"), goog4("http://gmail.com:80");
+  GURL bad1("http://bad1.notfound:80"), bad2("http://bad2.notfound:80");
+
+  UrlList found_names{goog, goog3, goog2, goog4, goog};
+  UrlList not_found_names{bad1, bad2};
+
+  FloodResolveRequestsOnUIThread(found_names);
+  FloodResolveRequestsOnUIThread(not_found_names);
+  WaitUntilHostNamesHaveBeenRequested(found_names);
+  WaitUntilHostNamesHaveBeenRequested(not_found_names);
+  base::MessageLoop::current()->RunUntilIdle();
+
+  ExpectFoundUrlsOnUIThread(found_names, not_found_names);
+
+  EXPECT_LE(predictor()->peak_pending_lookups(),
+            found_names.size() + not_found_names.size());
+  EXPECT_LE(predictor()->peak_pending_lookups(),
+            predictor()->max_concurrent_dns_lookups());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, MassiveConcurrentLookupTest) {
+  DiscardAllResultsOnUIThread();
+  UrlList not_found_names;
+  for (int i = 0; i < 100; i++) {
+    not_found_names.push_back(
+        GURL(base::StringPrintf("http://host%d.notfound:80", i)));
+  }
+  FloodResolveRequestsOnUIThread(not_found_names);
+  WaitUntilHostNamesHaveBeenRequested(not_found_names);
+  base::MessageLoop::current()->RunUntilIdle();
+
+  ExpectFoundUrlsOnUIThread(network_hints::UrlList(), not_found_names);
+
+  EXPECT_LE(predictor()->peak_pending_lookups(), not_found_names.size());
+
+  EXPECT_LE(predictor()->peak_pending_lookups(),
+            predictor()->max_concurrent_dns_lookups());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       ShutdownWhenResolutionIsPendingTest) {
+  GURL localhost("http://delay.google.com:80");
+  UrlList names;
+  names.push_back(localhost);
+
+  // Flood with delayed requests.
+  FloodResolveRequestsOnUIThread(names);
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
+      base::TimeDelta::FromMilliseconds(500));
+  base::MessageLoop::current()->Run();
+
+  EXPECT_FALSE(predictor()->WasFound(localhost));
+}
+
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SimplePreconnectOne) {
   predictor()->PreconnectUrl(
       embedded_test_server()->base_url(), GURL(),
@@ -1132,17 +1299,14 @@ IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ShutdownStartupCyclePreresolve) {
   WaitUntilHostHasBeenRequested(target_url_.host());
 }
 
-// Flaky on Windows: http://crbug.com/469120
-#if defined(OS_WIN)
-#define MAYBE_DnsPrefetch DISABLED_DnsPrefetch
-#else
-#define MAYBE_DnsPrefetch DnsPrefetch
-#endif
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, MAYBE_DnsPrefetch) {
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, DnsPrefetch) {
+  // Navigate once to make sure all initial hostnames are requested.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+
   int hostnames_requested_before_load = RequestedHostnameCount();
-  ui_test_utils::NavigateToURL(
-      browser(),
-      GURL(embedded_test_server()->GetURL("/predictor/dns_prefetch.html")));
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL(
+                                              "/predictor/dns_prefetch.html"));
   WaitUntilHostHasBeenRequested(kChromiumHostname);
   ASSERT_FALSE(HasHostBeenRequested(kInvalidLongHostname));
   ASSERT_EQ(hostnames_requested_before_load + 1, RequestedHostnameCount());