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/threading/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/http/http_transaction_factory.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_context.h"
+#include "net/url_request/url_request_context_getter.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* CreateEmptyBodyRequestJob(net::URLRequest* request,
+                                              net::NetworkDelegate* delegate) {
+  const char kPlainTextHeaders[] =
+      "HTTP/1.1 200 OK\n"
+      "Content-Type: text/plain\n"
+      "Access-Control-Allow-Origin: *\n"
+      "\n";
+  return new net::URLRequestTestJob(request, delegate, kPlainTextHeaders, "",
+                                    true);
+}
+
+// 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(
+    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);
+}
+
 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 sockets_.

[mmenke, 2016/05/12 16:30:23]

nit:  sockets_

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+  void WaitForAcceptedConnectionsOnUI(int n) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);

[mmenke, 2016/05/12 16:30:24]

DCHECK(!run_loop_);

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+    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)));

[mmenke, 2016/05/12 16:30:23]

Why do we need to do this on the IO thread?  Can't we call CheckAccepted()
directly?  Could even check if "sockets_.size() == n" right after getting the
lock, and if so, exit without spinning the loop.

[Charlie Harrison, 2016/05/12 20:17:04]

I basically wanted to unconditionally loop, instead of having two paths with one
that waits. I've made it a bit more reasonable without adding a thread hop.

+    }
+    run_loop.Run();
+  }
+
+  void CheckAcceptedLocked() {
+    base::AutoLock lock(lock_);
+    CheckAccepted();
+  }
+
+  void CheckAccepted() {

[mmenke, 2016/05/12 16:30:23]

ASSERT_TRUE(lock_.AssertAcquired());

[mmenke, 2016/05/12 16:30:23]

Should probably document some of these.

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+    if (accept_n_loop_ && accept_n_) {
+      if (accept_n_ == sockets_.size()) {

[mmenke, 2016/05/12 16:30:23]

nit:  Early return is generally preferred, unless it's likely to lead to bugs
(i.e. if new code is added after the if, it's likely you'll want it to run in
both cases)

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+        task_runner_->PostTask(FROM_HERE, accept_n_loop_->QuitClosure());
+        accept_n_loop_ = nullptr;
+        accept_n_ = 0;
+      }
+    }
+  }
+
+  void ResetCounts() {
+    base::AutoLock lock(lock_);
+    sockets_.clear();
+  }
 
  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 accepted before quiting the loop.
+  size_t accept_n_;
+  base::RunLoop* accept_n_loop_;

[mmenke, 2016/05/12 16:30:24]

Should make it clearer what's protected by the lock, and what isn't.  Maybe put
everything protected by it in a row, with a comment.
...

[mmenke, 2016/05/12 16:30:24]

Optional:  Suggest making this a scoped_ptr and creating it + resetting it in
the new wait function.  It seems to be a more common patter (And saves one whole
line of code!)

[Charlie Harrison, 2016/05/12 20:17:05]

Done.

[Charlie Harrison, 2016/05/12 20:17:07]

I don't like this because we'd need to protect the "reset()" by reacquiring the
lock after the runloop is done.

Unless I'm misunderstanding you.

[Charlie Harrison, 2016/05/12 20:17:07]

Done.

+
   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 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:
+  MatchingPortRequestInterceptor(
+      int port,
+      base::Callback<net::URLRequestJob*(net::URLRequest*,
+                                         net::NetworkDelegate*)>

[mmenke, 2016/05/12 16:30:25]

nit:  include base/callback.h and base/bind.h

[Charlie Harrison, 2016/05/12 20:17:05]

Done.

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

[mmenke, 2016/05/12 16:30:24]

nit:  Suggest a typedef at the start of the class.  Doesn't really save us any
lines of code, but it's a bit prettier.

[Charlie Harrison, 2016/05/12 20:17:04]

Arguable... but I'll do it :)

+      create_job_callback_;
+  const int port_;
+
+  DISALLOW_COPY_AND_ASSIGN(MatchingPortRequestInterceptor);
+};
+
+// This class is owned by the test harness, and listens to Predictor events. It
+// takes as input the source host and cross site host used by the test harness,
+// and asserts that only valid preconnects and learning can occur between the
+// 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) {}
+
+  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);

[mmenke, 2016/05/12 16:30:23]

nit:  #include <algorithm>

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+    } else if (original_url == source_host_) {
+      same_site_preconnected_ = std::max(same_site_preconnected_, count);
+    } else {
+      ADD_FAILURE() << "Preconnected" << original_url

[mmenke, 2016/05/12 16:30:24]

nit:  "Preconnected" -> "Preconnected "

[Charlie Harrison, 2016/05/12 20:17:05]

Done.

+                    << " When should only be preconnecting the source host: "

[mmenke, 2016/05/12 16:30:24]

nit:  When -> when

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+                    << 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: "

[mmenke, 2016/05/12 16:30:25]

nit:  when

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+                    << 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 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_;
+  }
+
+ private:
+  GURL source_host_;
+  GURL cross_site_host_;

[mmenke, 2016/05/12 16:30:24]

nit:  const (x2)

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+
+  int cross_site_learned_;
+
+  int cross_site_preconnected_;
+  int same_site_preconnected_;
+  base::Lock lock_;

[mmenke, 2016/05/12 16:30:24]

Should document what this lock protects.

[mmenke, 2016/05/12 16:30:25]

nit:  Weird to have a blank line between cross_site_learned_ and
cross_site_preconnected_, but not one before the lock.

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+
+  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) {
-  }
+        host_resolution_request_recorder_(new HostResolutionRequestRecorder),
+        cross_site_test_server_(new net::EmbeddedTestServer()) {}
 
  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);
   }
 
   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()->set_preconnect_enabled_for_test(true);
+    BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
+                            base::Bind(&PredictorBrowserTest::SetUpOnIOThread,
+                                       base::Unretained(this)));
+  }
+
+  void SetUpOnIOThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    // 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.
+    const GURL& favicon_url = cross_site_test_server()->GetURL("/favicon.ico");
+    net::URLRequestFilter::GetInstance()->AddUrlInterceptor(
+        favicon_url, base::WrapUnique(new MatchingPortRequestInterceptor(
+                         favicon_url.EffectiveIntPort(),
+                         base::Bind(&CreateEmptyBodyRequestJob))));

[mmenke, 2016/05/12 16:30:23]

Do we actually need to do this?  As you said, they're all for the main site,
anyways.

[Charlie Harrison, 2016/05/12 20:17:03]

Done.

+  }
+
+  // Note, the interceptor also looks at the port, to differentiate between the
+  // two test servers.

[mmenke, 2016/05/12 16:30:23]

Should also document what this method actually does (Something along the lines
of "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."

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+  static void StartInterceptingHost(const GURL& url) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    net::URLRequestFilter::GetInstance()->AddHostnameInterceptor(
+        "http", url.host(),

[mmenke, 2016/05/12 16:30:24]

"http" -> url.scheme()?  Doesn't really matter, but since we're taking a URL,
seems like we should be using it.

[Charlie Harrison, 2016/05/12 20:17:06]

Done.

+        base::WrapUnique(new MatchingPortRequestInterceptor(
+            url.EffectiveIntPort(), base::Bind(&CreateEmptyBodyRequestJob))));
+  }
+
+  static void StopInterceptingHost(const GURL& url) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    net::URLRequestFilter::GetInstance()->RemoveHostnameHandler("http",

[mmenke, 2016/05/12 16:30:24]

"http" -> url.scheme()

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+                                                                url.host());
+  }
+
+  void StartInterceptingCrossSiteOnUI() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::StartInterceptingHost,
+                   cross_site_test_server()->GetURL("/")));

[mmenke, 2016/05/12 16:30:24]

nit:  GetURL("/") -> base_url()

[Charlie Harrison, 2016/05/12 20:17:06]

Done.

+  }
+
+  void StopInterceptingCrossSiteOnUI() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    content::BrowserThread::PostTask(
+        content::BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::StopInterceptingHost,
+                   cross_site_test_server()->GetURL("/")));

[mmenke, 2016/05/12 16:30:23]

base_url()

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

   }
 
   void TearDownOnMainThread() override {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
     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) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
     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() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    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) {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    predictor->SetObserver(observer_.get());
+  }
+
+  // Note: For many of the tests to get to a "clean slate" mid run, they must
+  // flush sockets on both the client and server.
+  void FlushClientSockets() {
+    DCHECK_CURRENTLY_ON(BrowserThread::IO);
+    predictor()
+        ->url_request_context_getter_for_test()
+        ->GetURLRequestContext()
+        ->http_transaction_factory()
+        ->GetSession()
+        ->CloseAllConnections();
+  }
+
+  void FlushClientSocketsOnUIThread() {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    base::RunLoop wait_loop;

[mmenke, 2016/05/12 16:30:24]

nit:  run_loop?  Just for consistency

[Charlie Harrison, 2016/05/12 20:17:06]

Done.

+    BrowserThread::PostTaskAndReply(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&PredictorBrowserTest::FlushClientSockets,
+                   base::Unretained(this)),
+        wait_loop.QuitClosure());
+    wait_loop.Run();
+  }
+
+  void FlushServerSocketsOnUIThread(net::EmbeddedTestServer* test_server) {
+    DCHECK_CURRENTLY_ON(BrowserThread::UI);
+    EXPECT_TRUE(test_server->FlushAllSocketsAndConnectionsOnUIThread());
+  }
+
+  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(
+        "/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);
+  }
+
   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_;

[mmenke, 2016/05/12 16:30:23]

include <memory>

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

 
  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_;
+  scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

[mmenke, 2016/05/12 16:30:24]

include base/memory/ref_counted.h

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

 };
 
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PRE_ShutdownStartupCycle) {
+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);

[mmenke, 2016/05/12 16:30:24]

Hrm...Should we make WaitForAcceptedConnectionsOnUI have an EXPECT failure if we
see too many sockets?  As long as we make sure real cross-site requests don't
make it to the socket pools, think there are no concerns about real requests
racing with preconnects.

[Charlie Harrison, 2016/05/12 20:17:07]

Yeah, good idea. That will force tests to be careful about killing sockets, etc.

+}
+
+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);
+}
+
+// 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()->CrossSiteLearned());
+  EXPECT_EQ(2, observer()->SameSitePreconnected());
+}
+
+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()->CrossSiteLearned());
+  EXPECT_EQ(2, observer()->SameSitePreconnected());
+}
+
+// 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

[mmenke, 2016/05/12 16:30:24]

We don't disable preconnects any more.

[Charlie Harrison, 2016/05/12 20:17:04]

Done.

+// 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.
+//
+// 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) {
+  StartInterceptingCrossSiteOnUI();
+
+  InstallPredictorObserver();

[mmenke, 2016/05/12 16:30:24]

Should we call these two in the test fixture itself?  Don't think they hurt for
tests that don't use them, and we call them on a number of tests.

[Charlie Harrison, 2016/05/12 20:17:08]

The observer EXPECTS certain URLs, so the one test we use localhost fails. We
can special case that test though.

Per your comment below, I'm fine moving these to the setup.

+  NavigateToCrossSiteHtmlUrl(2 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(2, observer()->CrossSiteLearned());
+  StopInterceptingCrossSiteOnUI();

[mmenke, 2016/05/12 16:30:24]

Do we need to stop intercepting?  We're just interested in preconnects, and
these could confuse things (WaitForAcceptedConnectionsOnUI(4u) could succeed,
even if we don't preconnect - admittedly, we also check with the observer, but
I'm paranoid).

[Charlie Harrison, 2016/05/12 20:17:05]

Actually, you're right. That simplifies things :)

+  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()->CrossSitePreconnected());
+  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()->CrossSiteLearned());
+  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

[mmenke, 2016/05/12 16:30:23]

host_piece -> host name?

[Charlie Harrison, 2016/05/12 20:17:07]

Done.

+  // 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()->CrossSitePreconnected());
+}
+
+// Expect that the predictor correctly predicts subframe navigations.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, SubframeCrossSitePrediction) {
+  InstallPredictorObserver();
+  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()->CrossSiteLearned());
+
+  // The redirect causes two preconnects for the initial navigation.

[mmenke, 2016/05/12 16:30:24]

This isn't a redirect - it's a (subframe) navigation.

[Charlie Harrison, 2016/05/12 20:17:08]

Done.

+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(2u);
+  StopInterceptingCrossSiteOnUI();
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+
+  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()->CrossSitePreconnected());
+  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) {

[mmenke, 2016/05/12 16:30:24]

I'm not following what this test does.  If we go to
<same_site>/predictor/predictor_cross_site, and then to a data URL that contains
<same_site>/title1.html, why do we preconnect to cross site at all?

[Charlie Harrison, 2016/05/12 20:17:05]

This test is essentially the same as
CrossSiteSimplePredictionAfterOneNavigation, except the second navigation is the
same site in an iframe instead of a main frame.

Because the first navigation finds the relationship between same site and cross
site, when the same-site shows up again (even in a subframe), we navigate.

I've updated the comment to be a little clearer.

+  InstallPredictorObserver();
+  StartInterceptingCrossSiteOnUI();
+
+  // Navigate to the normal cross site URL to learn the relationship.
+  NavigateToCrossSiteHtmlUrl(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(1, observer()->CrossSiteLearned());
+
+  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()->CrossSitePreconnected());
+  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();
+  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()->CrossSiteLearned());
+
+  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>");
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+  EXPECT_EQ(4u, cross_site_connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+}
+
+// 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()->CrossSiteLearned());
+  EXPECT_EQ(2, observer()->SameSitePreconnected());
+}
+
+// This test navigates to an html file with a tag:
+// <meta name="referrer" content="never">
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteNoReferrerNoPredictionAfterOneNavigation) {
+  InstallPredictorObserver();
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHtmlUrl(2 /* num_cors */,
+                             "_no_referrer" /* file_suffix */);
+  EXPECT_EQ(0, observer()->CrossSiteLearned());
+
+  StopInterceptingCrossSiteOnUI();
+  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()->CrossSitePreconnected());
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteSimplePredictionAfterTwoNavigations) {
+  InstallPredictorObserver();
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHtmlUrl(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(0, observer()->CrossSitePreconnected());
+
+  NavigateToCrossSiteHtmlUrl(1 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  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) + .33)*.66) + 1 = 3 preconnects.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(3, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(3u);
+}
+
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest,
+                       CrossSiteSimplePredictionAfterTwoNavigations2) {
+  InstallPredictorObserver();
+  StartInterceptingCrossSiteOnUI();
+
+  NavigateToCrossSiteHtmlUrl(2 /* num_cors */, "" /* file_suffix */);
+  EXPECT_EQ(0, observer()->CrossSitePreconnected());
+
+  NavigateToCrossSiteHtmlUrl(2 /* num_cors */, "" /* file_suffix */);
+
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  StopInterceptingCrossSiteOnUI();
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  // ((2 + .66) + .66)*.66 + 1 ~= 3.2.
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+}
+
+// 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()->CrossSiteLearned());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // (2 + .33) + 1 = 3.33.
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+  observer()->ResetCounts();
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // ceil((2 + .33) * .66) + 1 = 3.
+  EXPECT_EQ(3, observer()->CrossSitePreconnected());
+  observer()->ResetCounts();
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // ceil((2 + .33) * .66 * .66) + 1 = 3.
+  EXPECT_EQ(3, observer()->CrossSitePreconnected());
+  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()->CrossSitePreconnected());
+}
+
+// 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();
+  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()->CrossSiteLearned());
+
+  EXPECT_EQ(2, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(2u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  StopInterceptingCrossSiteOnUI();
+  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()->CrossSitePreconnected());
+}
+
+// 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();
+  StartInterceptingCrossSiteOnUI();
+
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(1, observer()->CrossSiteLearned());
+
+  EXPECT_EQ(2, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(2u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  StopInterceptingCrossSiteOnUI();
+  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()->CrossSitePreconnected());
+  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) {
+  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();
+  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();
+  StartInterceptingCrossSiteOnUI();
+
+  // Navigate once and redirect.
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(1, observer()->CrossSiteLearned());
+
+  EXPECT_EQ(2, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(2u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  // Navigate again and redirect.
+  ui_test_utils::NavigateToURL(browser(), embedded_test_server()->GetURL("/"));
+  EXPECT_EQ(1, observer()->CrossSiteLearned());
+
+  // 2 + .33 + 1 = 3.33.
+  EXPECT_EQ(4, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(4u);
+
+  FlushClientSocketsOnUIThread();
+  FlushServerSocketsOnUIThread(cross_site_test_server());
+  cross_site_connection_listener_->ResetCounts();
+  observer()->ResetCounts();
+
+  StopInterceptingCrossSiteOnUI();
+  EXPECT_EQ(0u, cross_site_connection_listener_->GetAcceptedSocketCount());
+
+  ui_test_utils::NavigateToURL(browser(),
+                               embedded_test_server()->GetURL("/title1.html"));
+  // 3 preconnects expected because ((2 + .33)*.66 + .33)*.66 + 1 = 2.23.
+  EXPECT_EQ(3, observer()->CrossSitePreconnected());
+  cross_site_connection_listener_->WaitForAcceptedConnectionsOnUI(3u);
+}
+
+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 124 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