Add a browsertest suite for net predictor

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 "base/base64.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/synchronization/lock.h"
 #include "base/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/net/predictor.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/dns/host_resolver_proc.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/socket/stream_socket.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
 #include "net/test/embedded_test_server/embedded_test_server_connection_listener.h"
+#include "net/test/embedded_test_server/http_request.h"
+#include "net/test/embedded_test_server/http_response.h"
+#include "net/test/url_request/url_request_failed_job.h"
+#include "net/url_request/url_request_filter.h"
+#include "net/url_request/url_request_interceptor.h"
+#include "net/url_request/url_request_test_job.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "url/gurl.h"
 
 using content::BrowserThread;
 using testing::HasSubstr;
 
 namespace {
 
+net::URLRequestJob* EmptyRequestJob(net::URLRequest* request,

[mmenke, 2016/05/06 19:54:01]

CreateEmptyBodyRequestJob?  The method itself isn't the job, and
ERR_EMPTY_RESPONSE means no headers, either.

[Charlie Harrison, 2016/05/09 19:47:05]

Done.

+                                    net::NetworkDelegate* delegate) {
+  const char kPlainTextHeaders[] =
+      "HTTP/1.1 200 OK\n"
+      "Content-Type: text/plain\n"
+      "Access-Control-Allow-Origin: *\n"
+      "Expires: Thu, 1 Jan 2100 20:00:00 GMT\n"

[mmenke, 2016/05/06 19:54:01]

I don't think we need the expires header?

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+      "\n";
+  return new net::URLRequestTestJob(
+      request, delegate,
+      std::string(kPlainTextHeaders, arraysize(kPlainTextHeaders)), "", true);

[mmenke, 2016/05/06 19:54:00]

std::string(kPlainTextHeaders, arraysize(kPlainTextHeaders)) can be replaced
with "kPlainTextHeaders", since the string doesn't have any nulls in it, and it
null terminated.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+}
+
+net::URLRequestJob* NotFoundRequestJob(net::URLRequest* request,

[mmenke, 2016/05/06 19:54:01]

CreateFailingRequestJob?  (It's not a 404 response, or an ERR_NOT_FOUND
response, as this name impies)

[Charlie Harrison, 2016/05/09 19:47:03]

Done.

+                                       net::NetworkDelegate* delegate) {
+  return new net::URLRequestFailedJob(request, delegate, net::ERR_ABORTED);

[mmenke, 2016/05/06 19:54:01]

ERR_ABORTED is a magical error code, that net shouldn't really be returning
(Though it does, in some obscure cases).  Can we just use ERR_FAILED?

If this has to be ERR_ABORTED due to predictor logic, that should be found.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+}
+
 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";
 
 // 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:
-  ConnectionListener() : task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
+  ConnectionListener()
+      : accept_n_(0),
+        accept_n_loop_(nullptr),
+        task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
 
   ~ConnectionListener() override {}
 
   // Get called from the EmbeddedTestServer thread to be notified that
   // a connection was accepted.
   void AcceptedSocket(const net::StreamSocket& connection) override {
     base::AutoLock lock(lock_);
     uint16_t socket = GetPort(connection);
     EXPECT_TRUE(sockets_.find(socket) == sockets_.end());
 
     sockets_[socket] = SOCKET_ACCEPTED;
     task_runner_->PostTask(FROM_HERE, accept_loop_.QuitClosure());
+    CheckAccepted();
   }
 
   // Get called from the EmbeddedTestServer thread to be notified that
   // a connection was read from.
-  void ReadFromSocket(const net::StreamSocket& connection) override {
+  void ReadFromSocket(const net::StreamSocket& connection, int rv) override {
+    // Don't log a read if no data was transferred. This case often happens if
+    // the sockets of the test server are being flushed and disconnected.
+    if (rv <= 0)
+      return;
     base::AutoLock lock(lock_);
     uint16_t socket = GetPort(connection);
     EXPECT_FALSE(sockets_.find(socket) == sockets_.end());
 
     sockets_[socket] = SOCKET_READ_FROM;
     task_runner_->PostTask(FROM_HERE, read_loop_.QuitClosure());
   }
 
   // Returns the number of sockets that were accepted by the server.
   size_t GetAcceptedSocketCount() const {
     base::AutoLock lock(lock_);
     return sockets_.size();
   }
 
   // Returns the number of sockets that were read from by the server.
   size_t GetReadSocketCount() const {
     base::AutoLock lock(lock_);
     size_t read_sockets = 0;
     for (const auto& socket : sockets_) {
       if (socket.second == SOCKET_READ_FROM)
         ++read_sockets;
     }
     return read_sockets;
   }
 
   void WaitUntilFirstConnectionAccepted() { accept_loop_.Run(); }
+  void WaitUntilFirstConnectionRead() { read_loop_.Run(); }
 
-  void WaitUntilFirstConnectionRead() { read_loop_.Run(); }
+  // The UI thread will wait for exactly |n| items in soeckts_.

[mmenke, 2016/05/06 19:54:01]

nit:  |sockets_|

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+  void WaitForAcceptedConnectionsOnUI(int n) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    base::RunLoop run_loop;
+    {
+      base::AutoLock lock(lock_);
+      accept_n_ = n;
+      accept_n_loop_ = &run_loop;
+      BrowserThread::PostTask(
+          BrowserThread::IO, FROM_HERE,
+          base::Bind(&ConnectionListener::CheckAcceptedLocked,
+                     base::Unretained(this)));
+    }
+    run_loop.Run();
+  }
+
+  void CheckAcceptedLocked() {
+    base::AutoLock lock(lock_);
+    CheckAccepted();
+  }
+
+  void CheckAccepted() {
+    if (accept_n_loop_ && accept_n_) {
+      if (accept_n_ == sockets_.size()) {
+        task_runner_->PostTask(FROM_HERE, accept_n_loop_->QuitClosure());
+        accept_n_loop_ = nullptr;
+        accept_n_ = 0;
+      }
+    }
+  }
 
  private:
   static uint16_t GetPort(const net::StreamSocket& connection) {
     // Get the remote port of the peer, since the local port will always be the
     // port the test server is listening on. This isn't strictly correct - it's
     // possible for multiple peers to connect with the same remote port but
     // different remote IPs - but the tests here assume that connections to the
     // test server (running on localhost) will always come from localhost, and
     // thus the peer port is all thats needed to distinguish two connections.
     // This also would be problematic if the OS reused ports, but that's not
     // something to worry about for these tests.
     net::IPEndPoint address;
     EXPECT_EQ(net::OK, connection.GetPeerAddress(&address));
     return address.port();
   }
 
   enum SocketStatus { SOCKET_ACCEPTED, SOCKET_READ_FROM };
 
   typedef base::hash_map<uint16_t, SocketStatus> SocketContainer;
   SocketContainer sockets_;
 
   base::RunLoop accept_loop_;
   base::RunLoop read_loop_;
+
+  // If accept_n_ > 0, then the object is waiting for |accept_n_| sockets to be

[mmenke, 2016/05/06 19:54:01]

nit:  |accept_n_|

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+  // accepted before quiting the loop.
+  size_t accept_n_;
+  base::RunLoop* accept_n_loop_;
+
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
 
   mutable base::Lock lock_;
 
   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 {
@@ skipping 59 matching lines @@
   // 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 empty 200s. Note that the
+// port of the URL must match the port given in the constructor.

[mmenke, 2016/05/06 19:54:01]

Update comment.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+class MatchingPortRequestInterceptor : public net::URLRequestInterceptor {
+ public:
+  MatchingPortRequestInterceptor(
+      int port,
+      base::Callback<net::URLRequestJob*(net::URLRequest*,
+                                         net::NetworkDelegate*)> job_callback)
+      : callback_(job_callback), port_(port) {}

[mmenke, 2016/05/06 19:54:01]

suggest a clearer name for both the argument and the member variable.  maybe
create_job_callback_(create_job_callback)

[Charlie Harrison, 2016/05/09 19:47:05]

Done.

+  ~MatchingPortRequestInterceptor() override {}
+
+ private:
+  net::URLRequestJob* MaybeInterceptRequest(
+      net::URLRequest* request,
+      net::NetworkDelegate* network_delegate) const override {
+    if (request->url().EffectiveIntPort() != port_)
+      return nullptr;
+    return callback_.Run(request, network_delegate);
+  }

[mmenke, 2016/05/06 19:54:01]

nit:  Blank ine between methods and member variables.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+  base::Callback<net::URLRequestJob*(net::URLRequest*, net::NetworkDelegate*)>
+      callback_;
+  int port_;

[mmenke, 2016/05/06 19:54:00]

nit: both these can be const

[Charlie Harrison, 2016/05/09 19:47:05]

Done.

+  DISALLOW_COPY_AND_ASSIGN(MatchingPortRequestInterceptor);

[mmenke, 2016/05/06 19:54:01]

nit:  Blank line before DISALLOW_COPY_AND_ASSIGN

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+};
+
+class CrossSitePredictorObserver

[mmenke, 2016/05/06 19:54:01]

Add description

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+    : 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) {}
+
+  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_) {
+      same_site_preconnected_ = std::max(same_site_preconnected_, count);
+    } else {
+      ADD_FAILURE() << "Preconnected" << original_url
+                    << " When should only be preconnecting the source host: "
+                    << source_host_
+                    << " or the cross site host: " << cross_site_host_;
+    }
+  }
+
+  void OnLearnFromNavigation(const GURL& referring_url,
+                             const GURL& target_url) override {
+    base::AutoLock lock(lock_);
+    // There are three possibilities:
+    //     source => target
+    //     source => source
+    //     target => target
+    if (referring_url == source_host_ && target_url == cross_site_host_) {
+      cross_site_learned_++;
+    } else if (referring_url == source_host_ && target_url == source_host_) {
+      // 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 ResetCounts() {
+    base::AutoLock lock(lock_);
+    cross_site_learned_ = 0;
+    cross_site_preconnected_ = 0;
+    same_site_preconnected_ = 0;
+  }
+
+  int cross_site_learned() const {

[mmenke, 2016/05/06 19:54:00]

These should use the BigSlowFunctionNamingScheme because of the lock

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+    base::AutoLock lock(lock_);
+    return cross_site_learned_;
+  }
+
+  int cross_site_preconnected() const {
+    base::AutoLock lock(lock_);
+    return cross_site_preconnected_;
+  }
+
+  int same_site_preconnected() const {
+    base::AutoLock lock(lock_);
+    return same_site_preconnected_;
+  }
+
+ private:
+  const GURL source_host_;
+  const GURL cross_site_host_;
+
+  int cross_site_learned_;
+
+  int cross_site_preconnected_;
+  int same_site_preconnected_;
+  mutable base::Lock lock_;

[mmenke, 2016/05/06 19:54:00]

suggest not making this mutable, and getting rid of the consts - think the
mutable costs us more than the consts get us, in terms of cleanliness.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+  DISALLOW_COPY_AND_ASSIGN(CrossSitePredictorObserver);

[mmenke, 2016/05/06 19:54:01]

nit:  Blank line before DISALLOW_COPY_AND_ASSIGN

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+};
+
 }  // 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) {
+        host_resolution_request_recorder_(new HostResolutionRequestRecorder),
+        cross_site_test_server_(new net::EmbeddedTestServer()) {}
+  void StartInterceptingCrossSite() {

[mmenke, 2016/05/06 19:54:00]

Should have thread-safety DCHECKs on all of these
(DCHECK_CURRENTLY_ON(BrowserThread::IO))

[Charlie Harrison, 2016/05/09 19:47:04]

Done. Put it on the most relevant methods.

+    net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
+        "http", cross_site_test_server()->GetURL("/").host(),
+        base::WrapUnique(new MatchingPortRequestInterceptor(
+            cross_site_test_server()->GetURL("/").EffectiveIntPort(),

[mmenke, 2016/05/06 19:54:01]

The test server's GetURL method isn't guaranteed to be threadsafe.  Suggest just
making this method static, and taking the URL as an argument.

[Charlie Harrison, 2016/05/09 19:47:05]

Done.

+            base::Bind(&EmptyRequestJob))));
+  }
+  void StopInterceptingCrossSite() {

[mmenke, 2016/05/06 19:54:00]

Suggest you make this static, per above comment.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+    net::URLRequestFilter::GetInstance()->RemoveHostnameHandler(
+        "http", cross_site_test_server()->GetURL("/").host());
+  }
+  void StartInterceptingCrossSiteOnUI() {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::StartInterceptingCrossSite,
+                   base::Unretained(this)));
+  }
+  void StopInterceptingCrossSiteOnUI() {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::StopInterceptingCrossSite,
+                   base::Unretained(this)));
   }

[mmenke, 2016/05/06 19:54:01]

Suggest putting these 4 methods below SetUpOnIOThread - that way, all the set up
during test fixture creation stuff is together, at the top.

[mmenke, 2016/05/06 19:54:01]

nit:  Suggest a blank line between methods.

[Charlie Harrison, 2016/05/09 19:47:03]

Done.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

 
  protected:
   void SetUpInProcessBrowserTestFixture() override {
     scoped_host_resolver_proc_.reset(new net::ScopedDefaultHostResolverProc(
         host_resolution_request_recorder_.get()));
     InProcessBrowserTest::SetUpInProcessBrowserTestFixture();
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(
         switches::kEnableExperimentalWebPlatformFeatures);
     command_line->AppendSwitchASCII(
         switches::kEnableBlinkFeatures, kBlinkPreconnectFeature);
   }
 
+  // Override the test server to redirect requests matching some path. This is
+  // used because the predictor only learns simple redirects with a path of "/"
+  std::unique_ptr<net::test_server::HttpResponse> RedirectForPathHandler(

[mmenke, 2016/05/06 19:54:02]

This should be static.  Actually, could even just be moved out of the test
fixture.

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+      const std::string& path,
+      const GURL& redirect_url,
+      const net::test_server::HttpRequest& request) {
+    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);

[mmenke, 2016/05/06 19:54:01]

Is the std::move needed here?  May be because it's a
unique_ptr<net::test_server::BasicHttpResponse> and you're returning it as a
unique_ptr<net::test_server::HttpResponse>, but I'm not sure.

[Charlie Harrison, 2016/05/09 19:47:03]

Yeah it's needed.

+  }
+
   void SetUpOnMainThread() override {
+    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(
+        &PredictorBrowserTest::RedirectForPathHandler, base::Unretained(this),
+        "/", cross_site_test_server()->GetURL("/title1.html")));
+
+    predictor()->set_preconnect_enabled_for_test(true);
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&PredictorBrowserTest::SetUpOnIOThread,
+                                       base::Unretained(this)));
+  }
+
+  void SetUpOnIOThread() {
+    // Ignore all favicon requests. These are tricky to deal with because they
+    // are scheduled on the UI thread and are not accounted for in the load
+    // event. The interceptor will respond with 404s, and won't connect any
+    // sockets to the test server.
+    AddUrlInterceptor(cross_site_test_server()->GetURL("/favicon.ico"));
   }
 
   void TearDownOnMainThread() override {
     ASSERT_TRUE(embedded_test_server()->ShutdownAndWaitUntilComplete());
   }
 
   // Navigates to a data URL containing the given content, with a MIME type of
   // text/html.
   void NavigateToDataURLWithContent(const std::string& content) {
     std::string encoded_content;
     base::Base64Encode(content, &encoded_content);
     std::string data_uri_content = "data:text/html;base64," + encoded_content;
     ui_test_utils::NavigateToURL(browser(), GURL(data_uri_content));
   }
 
   void TearDownInProcessBrowserTestFixture() override {
     InProcessBrowserTest::TearDownInProcessBrowserTestFixture();
     scoped_host_resolver_proc_.reset();
   }
 
   void LearnAboutInitialNavigation(const GURL& url) {
-    Predictor* predictor = browser()->profile()->GetNetworkPredictor();
-    BrowserThread::PostTask(BrowserThread::IO,
-                            FROM_HERE,
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
                             base::Bind(&Predictor::LearnAboutInitialNavigation,
-                                       base::Unretained(predictor),
-                                       url));
+                                       base::Unretained(predictor()), url));
     content::RunAllPendingInMessageLoop(BrowserThread::IO);
   }
 
   void LearnFromNavigation(const GURL& referring_url, const GURL& target_url) {
-    Predictor* predictor = browser()->profile()->GetNetworkPredictor();
-    BrowserThread::PostTask(BrowserThread::IO,
-                            FROM_HERE,
-                            base::Bind(&Predictor::LearnFromNavigation,
-                                       base::Unretained(predictor),
-                                       referring_url,
-                                       target_url));
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&Predictor::LearnFromNavigation,
+                   base::Unretained(predictor()), referring_url, target_url));
     content::RunAllPendingInMessageLoop(BrowserThread::IO);
   }
 
   void PrepareFrameSubresources(const GURL& url) {
-    Predictor* predictor = browser()->profile()->GetNetworkPredictor();
-    predictor->PredictFrameSubresources(url, GURL());
+    predictor()->PredictFrameSubresources(url, GURL());
   }
 
   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 WaitUntilHostHasBeenRequested(const std::string& hostname) {
     host_resolution_request_recorder_->WaitUntilHostHasBeenRequested(hostname);
   }
 
   int RequestedHostnameCount() const {
     return host_resolution_request_recorder_->RequestedHostnameCount();
   }
 
+  net::EmbeddedTestServer* cross_site_test_server() {
+    return cross_site_test_server_.get();
+  }
+
+  Predictor* predictor() { return browser()->profile()->GetNetworkPredictor(); }
+
+  void InstallPredictorObserver() {
+    observer_.reset(
+        new CrossSitePredictorObserver(embedded_test_server()->base_url(),
+                                       cross_site_test_server()->base_url()));
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::InstallPredictorObserverOnIOThread,
+                   base::Unretained(this), base::Unretained(predictor())));
+  }
+
+  void InstallPredictorObserverOnIOThread(
+      chrome_browser_net::Predictor* predictor) {
+    predictor->SetObserver(observer_.get());
+  }
+
+  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) {

[mmenke, 2016/05/06 19:54:00]

nit:  HtmlUrl (A lot of class and methods capitalize both words, but camel case
is now the preferred style).

[Charlie Harrison, 2016/05/09 19:47:05]

Done.

+    const GURL& base_url = cross_site_test_server()->base_url();
+    std::string path = base::StringPrintf(
+        "/predictor/"
+        "predictor_cross_site%s.html?subresourceHost=%s&"
+        "numCORSResources=%d",
+        file_suffix, base_url.spec().c_str(), num_cors);
+    ui_test_utils::NavigateToURL(browser(),
+                                 embedded_test_server()->GetURL(path));
+    bool result = false;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+        browser()->tab_strip_model()->GetActiveWebContents(),
+        "startFetchesAndWaitForReply()", &result));
+    EXPECT_TRUE(result);
+  }
+
+  static void AddUrlInterceptor(const GURL& url) {

[mmenke, 2016/05/06 19:54:00]

This method name doesn't make the behavior clear.  AddFailRequestInterceptor?

[mmenke, 2016/05/06 19:54:01]

I guess this is needed because failed requests aren't learned by the predictor? 
Where's that logic?  ChromeNetworkDelegate::OnBeforeURLRequest calls
WitnessURLRequest, which happens before requests make it to the interceptors.

[Charlie Harrison, 2016/05/09 19:47:04]

Actually, I think this could use the regular interceptor. This is an artifact of
previous patches. favicon is self-learned, so we don't interfere with cross site
learning. We only need to avoid connecting a socket.

+    net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
+        url, base::WrapUnique(new MatchingPortRequestInterceptor(
+                 url.EffectiveIntPort(), base::Bind(&NotFoundRequestJob))));
+  }
+
   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_;
   std::unique_ptr<net::ScopedDefaultHostResolverProc>
       scoped_host_resolver_proc_;
+  std::unique_ptr<net::EmbeddedTestServer> cross_site_test_server_;
+  std::unique_ptr<CrossSitePredictorObserver> observer_;
 };
 
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PRE_ShutdownStartupCycle) {
+// 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) {
+  InstallPredictorObserver();
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(1u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(1, observer()->cross_site_learned());
+  EXPECT_EQ(2, observer()->same_site_preconnected());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, CrossSiteUseThreeSockets) {
+  InstallPredictorObserver();
+  NavigateToCrossSiteHTMLURL(3 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(3u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(3, observer()->cross_site_learned());
+  EXPECT_EQ(2, observer()->same_site_preconnected());
+}
+
+// The following tests confirm that Chrome accurately predicts preconnects after
+// learning from navigations. Note that every "learned" connection adds ~.33 to
+// the expected connection number, which starts at 2. Every preconnect Chrome
+// performs multiplies the expected connections by .66.
+//
+// In order to simplify things, many of the following tests intercept requests
+// and disable preconnects to the cross site test server. This allows the test
+// server to maintain a "clean slate" with no connected sockets, so that when
+// the tests need to check that predictor initiated preconnects occur, there are
+// no races and the connections are deterministic.

[mmenke, 2016/05/06 19:54:01]

Could we just wait until we see the expected number of sockets from a "clean
slate" state, and then flush them?  Turning the predictor off and then on just
seems a bit unusual from the usual case, from the predictor's standpoint. 
Coincidentally, it also gives us coverage of the number of preconnects we make
in the no-knowledge state.

+//
+// One additional complexity is that if a preconnect from A to B is learned, an
+// extra preconnect will be issued if the host part of A and B match (ignoring
+// port). TODO(csharrison): This logic could probably be removed from the
+// predictor.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteSimplePredictionAfterOneNavigation) {
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  InstallPredictorObserver();
+  NavigateToCrossSiteHTMLURL(2 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(2, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil(2.66) + 1 = 4 preconnects.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+// This test does not intercept initial requests / preconnects to the cross site
+// test server.
+IN_PROC_BROWSER_TEST_F(
+    PredictorBrowserTest,
+    CrossSiteSimplePredictionAfterOneNavigationNoInterceptor) {
+  InstallPredictorObserver();
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(1, observer()->cross_site_learned());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(1u);
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil(2.33) + 1 = 4 preconnects.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // Just check that predictor has initiated preconnects to the cross site test
+  // server. It's tricky to reset the connections to the test server, and
+  // sockets can be reused.
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+}
+
+// Expect that the predictor correctly predicts subframe navigations.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SubframeCrossSitePrediction) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  InstallPredictorObserver();
+  ui_test_utils::NavigateToURL(
+      browser(), embedded_test_server()->GetURL(
+                     "/predictor/predictor_cross_site_subframe_nav.html"));
+  bool result = false;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      base::StringPrintf(
+          "navigateSubframe('%s')",
+          cross_site_test_server()->GetURL("/title1.html").spec().c_str()),
+      &result));
+  EXPECT_TRUE(result);
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil(2 + .33) + 1 = 4 preconnects.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+// Expect that the predictor correctly preconnects an already learned resource
+// if the host shows up in a subframe.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SubframeInitiatesPreconnects) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  // Navigate to the normal cross site URL to learn the relationship.
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  predictor()->set_preconnect_enabled_for_test(true);
+  StopInterceptingCrossSiteOnUI();
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil(2 + .33) + 1 = 4 preconnects.
+  NavigateToDataURLWithContent(
+      "<iframe src=\"" + embedded_test_server()->GetURL("/title1.html").spec() +
+      "\"></iframe>");
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+// Expect that the predictor correctly learns the subresources a subframe needs
+// to preconnect to.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SubframeLearning) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  std::string path = base::StringPrintf(
+      "/predictor/"
+      "predictor_cross_site.html?subresourceHost=%s&"
+      "numCORSResources=1&sendImmediately=1",
+      cross_site_test_server()->base_url().spec().c_str());
+  NavigateToDataURLWithContent(
+      base::StringPrintf("<iframe src=\"%s\"></iframe>",
+                         embedded_test_server()->GetURL(path).spec().c_str()));
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil(2 + .33) + 1 = 4 preconnects.
+  NavigateToDataURLWithContent(
+      "<iframe src=\"" + embedded_test_server()->GetURL("/title1.html").spec() +
+      "\"></iframe>");
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+  EXPECT_EQ(4u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+}
+
+// This tests the implementation details of the predictor. The current predictor
+// only learns via the referrer header.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, CrossSiteNoReferrerNoLearning) {
+  InstallPredictorObserver();
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */,
+                             "_no_referrer" /* file_suffix */);
+  cross_site_connection_listener_->WaitUntilFirstConnectionAccepted();
+  EXPECT_EQ(1u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(0, observer()->cross_site_learned());
+  EXPECT_EQ(2, observer()->same_site_preconnected());
+}
+
+// This test navigates to an html file with a tag:
+// <meta name="referrer" content="never">
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteNoReferrerNoPredictionAfterOneNavigation) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHTMLURL(2 /* num_cors */,
+                             "_no_referrer" /* file_suffix */);
+  EXPECT_EQ(0, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(false);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm that no preconnects occurred.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(0, observer()->cross_site_preconnected());
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteSimplePredictionAfterTwoNavigations) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // Navigate again and confirm a preconnect. Note that because the two
+  // embedded test servers have the same host_piece, an extra preconnect is
+  // issued. This results in ceil((2 + .33) + .33) + 1 ~= 4 preconnects.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteSimplePredictionAfterTwoNavigations2) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHTMLURL(2 /* num_cors */, "" /* file_suffix */);
+  NavigateToCrossSiteHTMLURL(2 /* num_cors */, "" /* file_suffix */);
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // ((2 + .66) + .66) + 1 ~= 4.2.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(5, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(5u);
+}
+
+// The first navigation uses a subresource. Subsequent navigations don't use
+// that subresource. This tests how the predictor forgets about these bad
+// navigations.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ForgetBadPrediction) {
+  InstallPredictorObserver();
+
+  NavigateToCrossSiteHTMLURL(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // (2 + .33) + 1 = 3.33.
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  observer()->ResetCounts();
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // ceil((2 + .33) * .66) + 1 = 3.
+  EXPECT_EQ(3, observer()->cross_site_preconnected());
+  observer()->ResetCounts();
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // ceil((2 + .33) * .66 * .66) + 1 = 3.
+  EXPECT_EQ(3, observer()->cross_site_preconnected());
+  observer()->ResetCounts();
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // Finally, (2 + .33) * .66^3 ~= .67. Not enough for a preconnect.
+  EXPECT_EQ(0, observer()->cross_site_preconnected());
+}
+
+// The predictor does not follow redirects if the original url had a non-empty
+// path (a path that was more than just "/").
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteRedirectNoPredictionWithPath) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL(base::StringPrintf(
+          "/server-redirect?%s",
+          cross_site_test_server()->GetURL("/title1.html").spec().c_str())));
+  EXPECT_EQ(0, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(0, observer()->cross_site_preconnected());
+}
+
+// The predictor does follow redirects if the original url had an empty path
+// (a path that was more than just "/"). Use the registered "/" path to redirect
+// to the target test server.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteRedirectPredictionWithNoPath) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // Preconnect 4 sockets because ceil(2 + .33) + 1 = 4.
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+// This test uses "localhost" instead of "127.0.0.1" to avoid extra preconnects
+// to hosts with the same host piece (ignoring port). Note that the preconnect
+// observer is not used here due to its strict checks on hostname.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteRedirectPredictionWithNoPathDifferentHostName) {
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  GURL localhost_source = GURL(base::StringPrintf(
+      "http://localhost:%s/predictor/"
+      "predictor_cross_site.html?subresourceHost=%s&numCORSResources=1",
+      embedded_test_server()->base_url().port().c_str(),
+      cross_site_test_server()->base_url().spec().c_str()));
+  ui_test_utils::NavigateToURL(browser(), localhost_source);
+  bool result = false;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      "startFetchesAndWaitForReply()", &result));
+  EXPECT_TRUE(result);
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  ui_test_utils::NavigateToURL(
+      browser(), GURL(base::StringPrintf(
+                     "http://localhost:%s/title1.html",
+                     embedded_test_server()->base_url().port().c_str())));
+  // Preconnect 3 sockets because ceil(2 + .33) = 3. Note that this time there
+  // is no additional connection due to same host.
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(3u);
+}
+
+// Perform the "/" redirect twice and make sure the predictor updates twice.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteTwoRedirectsPredictionWithNoPath) {
+  InstallPredictorObserver();
+  predictor()->set_preconnect_enabled_for_test(false);
+  StartInterceptingCrossSiteOnUI();
+
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(1, observer()->cross_site_learned());
+
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(2, observer()->cross_site_learned());
+
+  StopInterceptingCrossSiteOnUI();
+  predictor()->set_preconnect_enabled_for_test(true);
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // 4 preconnects expected because (2 + .33) + .33 + 1 = 3.66.
+  EXPECT_EQ(4, observer()->cross_site_preconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       PRE_ShutdownStartupCyclePreresolve) {
   // Prepare state that will be serialized on this shut-down and read on next
-  // start-up.
+  // start-up. Ensure preresolution over preconnection.
   LearnAboutInitialNavigation(startup_url_);
+  // The target url will have a expected connection count of 2 after this call.
   LearnFromNavigation(referring_url_, target_url_);
-}
-
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ShutdownStartupCycle) {
+
+  // In order to reduce the expected connection count < .8, issue predictions 3
+  // times. 2 * .66^3 ~= .58.
+  PrepareFrameSubresources(referring_url_);
+  PrepareFrameSubresources(referring_url_);
+  PrepareFrameSubresources(referring_url_);
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, ShutdownStartupCyclePreresolve) {
   // Make sure that the Preferences file is actually wiped of all DNS prefetch
   // related data after start-up.
   std::string cleared_startup_list;
   std::string cleared_referral_list;
   GetListFromPrefsAsString(prefs::kDnsPrefetchingStartupList,
                            &cleared_startup_list);
   GetListFromPrefsAsString(prefs::kDnsPrefetchingHostReferralList,
                            &cleared_referral_list);
 
   EXPECT_THAT(cleared_startup_list, Not(HasSubstr(startup_url_.host())));
@@ skipping 123 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());
 }
 
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SimplePreconnectOne) {

[mmenke, 2016/05/06 19:54:00]

Suggest putting these first (Think that putting the more basic tests before the
ones that depend on it can help with picking the best test to start debugging
when you run into a regression).

[Charlie Harrison, 2016/05/09 19:47:04]

Done.

+  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(
+      embedded_test_server()->base_url(), GURL(),
+      UrlInfo::ResolutionMotivation::EARLY_LOAD_MOTIVATED,
+      false /* allow credentials */, 2);
+  connection_listener_->WaitForAcceptedConnectionsOnUI(2u);
+}
+
+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);
+}
+
 }  // namespace chrome_browser_net