Move predictor dns unit tests to browser tests

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>
 
 #include "base/base64.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/command_line.h"
 #include "base/json/json_string_value_serializer.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/ref_counted.h"
 #include "base/synchronization/lock.h"
@@ skipping 56 matching lines @@
   if (request.GetURL().path() != path)
     return nullptr;
   std::unique_ptr<net::test_server::BasicHttpResponse> response(
       new net::test_server::BasicHttpResponse);
   response->set_code(net::HTTP_MOVED_PERMANENTLY);
   response->AddCustomHeader("Location", redirect_url.spec());
   return std::move(response);
 }
 
 const char kBlinkPreconnectFeature[] = "LinkPreconnect";
-const char kChromiumHostname[] = "chromium.org";
-const char kInvalidLongHostname[] = "illegally-long-hostname-over-255-"
-    "characters-should-not-send-an-ipc-message-to-the-browser-"
-    "0000000000000000000000000000000000000000000000000000000000000000000000000"
-    "0000000000000000000000000000000000000000000000000000000000000000000000000"
-    "000000000000000000000000000000000000000000000000000000.org";
+const char kChromiumHost[] = "http://chromium.org";

[eroman, 2016/05/27 00:56:38]

nit: Can you call this a "url" or "origin" now?

[Charlie Harrison, 2016/05/27 11:55:53]

Done.

+const char kInvalidLongHost[] =

[eroman, 2016/05/27 00:56:38]

same

[Charlie Harrison, 2016/05/27 11:55:53]

Done.

+    "http://"
+    "illegally-long-hostname-over-255-characters-should-not-send-an-ipc-"
+    "message-to-the-browser-"
+    "00000000000000000000000000000000000000000000000000000000000000000000000000"
+    "00000000000000000000000000000000000000000000000000000000000000000000000000"
+    "0000000000000000000000000000000000000000000000000000.org";
 
 // Gets notified by the EmbeddedTestServer on incoming connections being
 // accepted or read from, keeps track of them and exposes that info to
 // the tests.
 // A port being reused is currently considered an error.  If a test
 // needs to verify multiple connections are opened in sequence, that will need
 // to be changed.
 class ConnectionListener
     : public net::test_server::EmbeddedTestServerConnectionListener {
  public:
@@ skipping 136 matching lines @@
 
   // If |num_accepted_connections_needed_| is non zero, then the object is
   // waiting for |num_accepted_connections_needed_| sockets to be accepted
   // before quitting the |num_accepted_connections_loop_|.
   size_t num_accepted_connections_needed_;
   base::RunLoop* num_accepted_connections_loop_;
 
   DISALLOW_COPY_AND_ASSIGN(ConnectionListener);
 };
 
-// Records a history of all hostnames for which resolving has been requested,
-// and immediately fails the resolution requests themselves.
-class HostResolutionRequestRecorder : public net::HostResolverProc {
- public:
-  HostResolutionRequestRecorder()
-      : HostResolverProc(NULL),
-        is_waiting_for_hostname_(false) {
-  }
-
-  int Resolve(const std::string& host,
-              net::AddressFamily address_family,
-              net::HostResolverFlags host_resolver_flags,
-              net::AddressList* addrlist,
-              int* os_error) override {
-    BrowserThread::PostTask(
-        BrowserThread::UI,
-        FROM_HERE,
-        base::Bind(&HostResolutionRequestRecorder::AddToHistory,
-                   base::Unretained(this),
-                   host));
-    return net::ERR_NAME_NOT_RESOLVED;
-  }
-
-  int RequestedHostnameCount() const {
-    return requested_hostnames_.size();
-  }
-
-  bool HasHostBeenRequested(const std::string& hostname) const {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    return std::find(requested_hostnames_.begin(),
-                     requested_hostnames_.end(),
-                     hostname) != requested_hostnames_.end();
-  }
-
-  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();
-  }
-
- private:
-  ~HostResolutionRequestRecorder() override {}
-
-  void AddToHistory(const std::string& hostname) {
-    DCHECK_CURRENTLY_ON(BrowserThread::UI);
-    requested_hostnames_.push_back(hostname);
-    if (is_waiting_for_hostname_ && waiting_for_hostname_ == hostname) {
-      is_waiting_for_hostname_ = false;
-      waiting_for_hostname_.clear();
-      base::MessageLoop::current()->QuitWhenIdle();
-    }
-  }
-
-  // The hostname which WaitUntilHostHasBeenRequested is currently waiting for
-  // 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_;
-
-  DISALLOW_COPY_AND_ASSIGN(HostResolutionRequestRecorder);
-};
-
 // This class intercepts URLRequests and responds with the URLRequestJob*
 // callback provided by the constructor. Note that the port of the URL must
 // match the port given in the constructor.
 class MatchingPortRequestInterceptor : public net::URLRequestInterceptor {
  public:
   typedef base::Callback<net::URLRequestJob*(net::URLRequest*,
                                              net::NetworkDelegate*)>
       CreateJobCallback;
 
   MatchingPortRequestInterceptor(
@@ skipping 25 matching lines @@
 // two.
 class CrossSitePredictorObserver
     : public chrome_browser_net::PredictorObserver {
  public:
   CrossSitePredictorObserver(const GURL& source_host,
                              const GURL& cross_site_host)
       : source_host_(source_host),
         cross_site_host_(cross_site_host),
         cross_site_learned_(0),
         cross_site_preconnected_(0),
-        same_site_preconnected_(0) {}
+        same_site_preconnected_(0),
+        dns_run_loop_(nullptr) {}
 
   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_);
     if (original_url == cross_site_host_) {
       cross_site_preconnected_ = std::max(cross_site_preconnected_, count);
     } else if (original_url == source_host_) {
@@ skipping 19 matching lines @@
       // Same site learned. Branch retained for clarity.
     } else if (!(referring_url == cross_site_host_ &&
                  target_url == cross_site_host_)) {
       ADD_FAILURE() << "Learned " << referring_url << " => " << target_url
                     << " when should only be learning the source host: "
                     << source_host_
                     << " or the cross site host: " << cross_site_host_;
     }
   }
 
+  void OnDnsLookupFinished(const GURL& url, bool found) override {
+    base::AutoLock lock(lock_);
+    if (found) {
+      successful_dns_lookups_.insert(url);
+    } else {
+      unsuccessful_dns_lookups_.insert(url);
+    }
+    CheckForWaitingLoop();
+  }
+
   void ResetCounts() {
     base::AutoLock lock(lock_);
     cross_site_learned_ = 0;
     cross_site_preconnected_ = 0;
     same_site_preconnected_ = 0;
   }
 
   int CrossSiteLearned() {
     base::AutoLock lock(lock_);
     return cross_site_learned_;
   }
 
   int CrossSitePreconnected() {
     base::AutoLock lock(lock_);
     return cross_site_preconnected_;
   }
 
   int SameSitePreconnected() {
     base::AutoLock lock(lock_);
     return same_site_preconnected_;
   }
 
+  // Spins a run loop until |url| is added to one of the lookup maps.
+  void WaitUntilHostLookedUp(const GURL& url) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    base::RunLoop run_loop;
+    {
+      base::AutoLock lock(lock_);
+      DCHECK(waiting_on_dns_.is_empty());
+      DCHECK(!dns_run_loop_);
+      waiting_on_dns_ = url;
+      dns_run_loop_ = &run_loop;
+      CheckForWaitingLoop();
+    }
+    run_loop.Run();
+  }
+
+  bool HasHostBeenLookedUpLocked(const GURL& url) {
+    lock_.AssertAcquired();
+    return successful_dns_lookups_.find(url) != successful_dns_lookups_.end() ||

[eroman, 2016/05/27 00:56:38]

optional: can use ContainsKey() to simplify the various   x.find(y) != x.end()
patterns.

[Charlie Harrison, 2016/05/27 11:55:52]

Done.

+           unsuccessful_dns_lookups_.find(url) !=
+               unsuccessful_dns_lookups_.end();
+  }
+
+  bool HasHostBeenLookedUp(const GURL& url) {
+    base::AutoLock lock(lock_);
+    return HasHostBeenLookedUpLocked(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());
+    dns_run_loop_ = nullptr;
+  }
+
+  size_t TotalHostsLookedUp() {
+    base::AutoLock lock(lock_);
+    return successful_dns_lookups_.size() + unsuccessful_dns_lookups_.size();
+  }
+
+  // Note: this method expects the URL to have been looked up.
+  bool HostFound(const GURL& url) {
+    base::AutoLock lock(lock_);
+    EXPECT_TRUE(HasHostBeenLookedUpLocked(url)) << "Expected to have looked up"

[eroman, 2016/05/27 00:56:38]

nit: add a space at the end of the string.

[Charlie Harrison, 2016/05/27 11:55:53]

Done.

+                                                << url.spec();
+    return successful_dns_lookups_.find(url) != successful_dns_lookups_.end();
+  }
+
+  void set_task_runner(
+      scoped_refptr<base::SingleThreadTaskRunner> task_runner) {
+    task_runner_.swap(task_runner);
+  }
+
  private:
   const GURL source_host_;
   const GURL cross_site_host_;
 
+  GURL waiting_on_dns_;
+
+  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> successful_dns_lookups_;
+  std::set<GURL> unsuccessful_dns_lookups_;
+  base::RunLoop* dns_run_loop_;
+
   DISALLOW_COPY_AND_ASSIGN(CrossSitePredictorObserver);
 };
 
 }  // namespace
 
 namespace chrome_browser_net {
 
 class PredictorBrowserTest : public InProcessBrowserTest {
  public:
   PredictorBrowserTest()
-      : 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()) {}
+      : startup_url_("http://host1/"),
+        referring_url_("http://host2/"),
+        target_url_("http://host3/"),
+        rule_based_resolver_proc_(new net::RuleBasedHostResolverProc(nullptr)),
+        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 {
     scoped_host_resolver_proc_.reset(new net::ScopedDefaultHostResolverProc(
-        host_resolution_request_recorder_.get()));
+        rule_based_resolver_proc_.get()));
     InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
     command_line->AppendSwitchASCII(
         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();
     cross_site_test_server_->ServeFilesFromSourceDirectory("chrome/test/data/");
 
     connection_listener_.reset(new ConnectionListener());
     cross_site_connection_listener_.reset(new ConnectionListener());
     embedded_test_server()->SetConnectionListener(connection_listener_.get());
     cross_site_test_server_->SetConnectionListener(
         cross_site_connection_listener_.get());
     ASSERT_TRUE(embedded_test_server()->Start());
     ASSERT_TRUE(cross_site_test_server_->Start());
 
     embedded_test_server()->RegisterRequestHandler(
         base::Bind(&RedirectForPathHandler, "/",
                    cross_site_test_server()->GetURL("/title1.html")));
 
     predictor()->SetPreconnectEnabledForTest(true);
     InstallPredictorObserver(embedded_test_server()->base_url(),
                              cross_site_test_server()->base_url());
+    observer()->set_task_runner(task_runner_);
     StartInterceptingCrossSiteOnUI();
   }
 
   // Intercepts all requests to the specified host and returns a response with
   // an empty body. Needed to prevent requests from actually going to the test
   // server, to avoid any races related to socket accounting. Note, the
   // interceptor also looks at the port, to differentiate between the
   // two test servers.
   static void StartInterceptingHost(const GURL& url) {
     DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ skipping 65 matching lines @@
   }
 
   void GetListFromPrefsAsString(const char* list_path,
                                 std::string* value_as_string) const {
     PrefService* prefs = browser()->profile()->GetPrefs();
     const base::ListValue* list_value = prefs->GetList(list_path);
     JSONStringValueSerializer serializer(value_as_string);
     serializer.Serialize(*list_value);
   }
 
-  bool HasHostBeenRequested(const std::string& hostname) const {
-    return host_resolution_request_recorder_->HasHostBeenRequested(hostname);
+  void WaitUntilHostsLookedUp(const network_hints::UrlList& names) {
+    for (const GURL& url : names)
+      observer()->WaitUntilHostLookedUp(url);
   }
 
-  void WaitUntilHostHasBeenRequested(const std::string& hostname) {
-    host_resolution_request_recorder_->WaitUntilHostHasBeenRequested(hostname);
+  void FloodResolveRequestsOnUIThread(const network_hints::UrlList& names) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::FloodResolveRequests, this, names));
   }
 
-  int RequestedHostnameCount() const {
-    return host_resolution_request_recorder_->RequestedHostnameCount();
+  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();
   }
 
   Predictor* predictor() { return browser()->profile()->GetNetworkPredictor(); }
 
   void InstallPredictorObserver(const GURL& source_host,
                                 const GURL& cross_site_host) {
@@ skipping 36 matching lines @@
                    base::Unretained(this)),
         run_loop.QuitClosure());
     run_loop.Run();
   }
 
   void FlushServerSocketsOnUIThread(net::EmbeddedTestServer* test_server) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     EXPECT_TRUE(test_server->FlushAllSocketsAndConnectionsOnUIThread());
   }
 
+  // Note this method also expects that all the urls (found or not) were looked
+  // up.
+  void ExpectFoundUrls(
+      const network_hints::UrlList& found_names,
+      const network_hints::UrlList& not_found_names) {
+    for (const auto& name : found_names) {
+      EXPECT_TRUE(observer()->HostFound(name)) << "Expected to have found "
+                                               << name.spec();
+    }
+    for (const auto& name : not_found_names) {
+      EXPECT_FALSE(observer()->HostFound(name)) << "Did not expect to find "
+                                                << name.spec();
+    }
+  }
+
+  // This method verifies that |url| is in the predictors |results_| map. This

[eroman, 2016/05/27 00:56:38]

nit: predictors --> predictor's

[Charlie Harrison, 2016/05/27 11:55:52]

Done.

+  // is used for pending lookups, and lookups performed before the observer is
+  // attached.
+  void ExpectUrlRequestedFromPredictorOnUIThread(const GURL& url) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::ExpectUrlRequestedFromPredictor,
+                   base::Unretained(this), url));
+  }
+
+  void ExpectUrlRequestedFromPredictor(const GURL& url) {
+    EXPECT_NE(predictor()->results_.find(url), predictor()->results_.end());
+  }
+
+  void DiscardAllResultsOnUIThread() {
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&Predictor::DiscardAllResults,
+                                       base::Unretained(predictor())));
+  }
+
+  void ExpectValidPeakPendingLookupsOnUI(size_t num_names_requested) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::ExpectValidPeakPendingLookups,
+                   base::Unretained(this), num_names_requested));
+  }
+
+  void ExpectValidPeakPendingLookups(size_t num_names_requested) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    EXPECT_LE(predictor()->peak_pending_lookups_, num_names_requested);
+    EXPECT_LE(predictor()->peak_pending_lookups_,
+              predictor()->max_concurrent_dns_lookups());
+  }
+
   CrossSitePredictorObserver* observer() { return observer_.get(); }
 
   // Navigate to an html file on embedded_test_server and tell it to request
   // |num_cors| resources from the cross_site_test_server. It then waits for
   // those requests to complete. Note that "cors" here means using cors-mode in
   // correspondence with the fetch spec.
   void NavigateToCrossSiteHtmlUrl(int num_cors, const char* file_suffix) {
     DCHECK_CURRENTLY_ON(BrowserThread::UI);
     const GURL& base_url = cross_site_test_server()->base_url();
     std::string path = base::StringPrintf(
@@ skipping 10 matching lines @@
     EXPECT_TRUE(result);
   }
 
   const GURL startup_url_;
   const GURL referring_url_;
   const GURL target_url_;
   std::unique_ptr<ConnectionListener> connection_listener_;
   std::unique_ptr<ConnectionListener> cross_site_connection_listener_;
 
  private:
-  scoped_refptr<HostResolutionRequestRecorder>
-      host_resolution_request_recorder_;
+  scoped_refptr<net::RuleBasedHostResolverProc> rule_based_resolver_proc_;
   std::unique_ptr<net::ScopedDefaultHostResolverProc>
       scoped_host_resolver_proc_;
   std::unique_ptr<net::EmbeddedTestServer> cross_site_test_server_;
   std::unique_ptr<CrossSitePredictorObserver> observer_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 };
 
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SingleLookupTest) {
+  DiscardAllResultsOnUIThread();
+  GURL goog("http://www.example.test/");

[eroman, 2016/05/27 00:56:38]

probably better to call this "url" -- not actually google.

[Charlie Harrison, 2016/05/27 11:55:53]

Done.

+
+  // Try to flood the predictor with many concurrent requests.
+  network_hints::UrlList names{goog};
+  FloodResolveRequestsOnUIThread(names);
+  observer()->WaitUntilHostLookedUp(goog);
+  EXPECT_TRUE(observer()->HostFound(goog));
+  ExpectValidPeakPendingLookupsOnUI(1u);
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ConcurrentLookupTest) {
+  DiscardAllResultsOnUIThread();
+  GURL goog("http://www.example.test"), goog2("http://gmail.google.com"),

[eroman, 2016/05/27 00:56:38]

same here -- at least for one of them.

[Charlie Harrison, 2016/05/27 11:55:53]

Done.

+      goog3("http://mail.google.com"), goog4("http://gmail.com");
+  GURL bad1("http://bad1.notfound"), bad2("http://bad2.notfound");
+
+  UrlList found_names{goog, goog3, goog2, goog4, goog};
+  UrlList not_found_names{bad1, bad2};
+  FloodResolveRequestsOnUIThread(found_names);
+  FloodResolveRequestsOnUIThread(not_found_names);
+
+  WaitUntilHostsLookedUp(found_names);
+  WaitUntilHostsLookedUp(not_found_names);
+  ExpectFoundUrls(found_names, not_found_names);
+  ExpectValidPeakPendingLookupsOnUI(found_names.size() +
+                                    not_found_names.size());
+}
+
+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);
+
+  WaitUntilHostsLookedUp(not_found_names);
+  ExpectFoundUrls(network_hints::UrlList(), not_found_names);
+  ExpectValidPeakPendingLookupsOnUI(not_found_names.size());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       ShutdownWhenResolutionIsPendingTest) {
+  GURL delayed_url("http://delay.google.com:80");
+  UrlList names{delayed_url};
+
+  // Flood with delayed requests, then wait.
+  FloodResolveRequestsOnUIThread(names);
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
+      base::TimeDelta::FromMilliseconds(500));
+  base::MessageLoop::current()->Run();
+
+  ExpectUrlRequestedFromPredictor(delayed_url);
+  EXPECT_FALSE(observer()->HasHostBeenLookedUp(delayed_url));
+}
+
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SimplePreconnectOne) {
   predictor()->PreconnectUrl(
       embedded_test_server()->base_url(), GURL(),
       UrlInfo::ResolutionMotivation::EARLY_LOAD_MOTIVATED,
       false /* allow credentials */, 1);
   connection_listener_->WaitForAcceptedConnectionsOnUI(1u);
 }
 
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SimplePreconnectTwo) {
   predictor()->PreconnectUrl(
@@ skipping 430 matching lines @@
   GetListFromPrefsAsString(prefs::kDnsPrefetchingHostReferralList,
                            &cleared_referral_list);
 
   EXPECT_THAT(cleared_startup_list, Not(HasSubstr(startup_url_.host())));
   EXPECT_THAT(cleared_referral_list, Not(HasSubstr(referring_url_.host())));
   EXPECT_THAT(cleared_referral_list, Not(HasSubstr(target_url_.host())));
 
   // But also make sure this data has been first loaded into the Predictor, by
   // inspecting that the Predictor starts making the expected hostname requests.
   PrepareFrameSubresources(referring_url_);
-  WaitUntilHostHasBeenRequested(startup_url_.host());
-  WaitUntilHostHasBeenRequested(target_url_.host());
+  observer()->WaitUntilHostLookedUp(target_url_);
+
+  // Verify that both urls were requested by the predictor. Note that the
+  // startup URL may be requested before the observer attaches itself.
+  ExpectUrlRequestedFromPredictor(startup_url_);
+  EXPECT_FALSE(observer()->HostFound(target_url_));
 }
 
-// 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) {
-  int hostnames_requested_before_load = RequestedHostnameCount();
-  ui_test_utils::NavigateToURL(
-      browser(),
-      GURL(embedded_test_server()->GetURL("/predictor/dns_prefetch.html")));
-  WaitUntilHostHasBeenRequested(kChromiumHostname);
-  ASSERT_FALSE(HasHostBeenRequested(kInvalidLongHostname));
-  ASSERT_EQ(hostnames_requested_before_load + 1, RequestedHostnameCount());
+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"));
+
+  size_t hosts_looked_up_before_load = observer()->TotalHostsLookedUp();
+
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL(
+                                              "/predictor/dns_prefetch.html"));
+  observer()->WaitUntilHostLookedUp(GURL(kChromiumHost));
+  ASSERT_FALSE(observer()->HasHostBeenLookedUp(GURL(kInvalidLongHost)));
+
+  EXPECT_FALSE(observer()->HostFound(GURL(kChromiumHost)));
+  ASSERT_EQ(hosts_looked_up_before_load + 1, observer()->TotalHostsLookedUp());
 }
 
 // Tests that preconnect warms up a socket connection to a test server.
 // Note: This test uses a data URI to serve the preconnect hint, to make sure
 // that the network stack doesn't just re-use its connection to the test server.
 IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectNonCORS) {
   GURL preconnect_url = embedded_test_server()->base_url();
   std::string preconnect_content =
       "<link rel=\"preconnect\" href=\"" + preconnect_url.spec() + "\">";
   NavigateToDataURLWithContent(preconnect_content);
@@ skipping 90 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