Add cross origin to Blink-driven preconnect

chrome/browser/net/predictor_browsertest.cc

Index: chrome/browser/net/predictor_browsertest.cc
diff --git a/chrome/browser/net/predictor_browsertest.cc b/chrome/browser/net/predictor_browsertest.cc
index c4ebc559fd76ea783fa357a164c99dbea33be3f7..6abe98fad846c13feb795f0ffc7386451e6ab466 100644
--- a/chrome/browser/net/predictor_browsertest.cc
+++ b/chrome/browser/net/predictor_browsertest.cc
@@ -61,6 +61,7 @@ class ConnectionListener
       const net::test_server::StreamListenSocket& connection) override {
     base::AutoLock lock(lock_);
     uint16_t socket = GetPort(connection);
+    printf("Socket %d\n", socket);

[Ryan Sleevi, 2015/07/10 09:46:45]

*cough* :P

[Yoav Weiss, 2015/07/10 10:03:40]

Doh! I got accustomed to Blink's cl upload hooks yelling at me about printf
leftovers...

     EXPECT_TRUE(sockets_.find(socket) == sockets_.end());
 
     sockets_[socket] = SOCKET_ACCEPTED;
@@ -104,7 +105,7 @@ class ConnectionListener
   static uint16_t GetPort(
       const net::test_server::StreamListenSocket& connection) {
     net::IPEndPoint address;
-    EXPECT_EQ(net::OK, connection.GetLocalAddress(&address));
+    EXPECT_EQ(net::OK, connection.GetPeerAddress(&address));

[Ryan Sleevi, 2015/07/10 09:46:45]

Wait, what? Why this change?

Connections are a tuple of
(listener ip, listener port, peer ip, peer port)

Can you explain why this change is correct compared to GetLocal, especially when
we may have situations where peer ip is different, but peer port is the same?

[Yoav Weiss, 2015/07/10 10:03:40]

GetLocal gives me the port number of the local listening port, which doesn't
change between connections. What I wanted to keep track of is port numbers of
the peer, that do change between connections.
Since this is only used for testing, where the peer is localhost anyway, I
didn't think I need to add the IP into the mix.

[Ryan Sleevi, 2015/07/10 10:09:35]

Right, that's basically what I was assuming, but wanted to make sure without
accidentally making things more confusing ;)

I think what we want to do here is just document the assumptions this function
is making and (to an extent) the known limitations. We do want to be careful
assuming it will always be localhost connections - we've had tests break those
assumptions in the past.

private;
  static uint16_t GetPort(...) {
    // 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;
  }


Basically, just spelling out what we've discussed as a known (and intentional)
limitation, and not just an "Oops, I never thought about that", so that if it
ever breaks, it's clear _why_ it breaks to future readers ("We didn't need a
pony, so we didn't buy a pony") ;)

     return address.port();
   }
 
@@ -350,7 +351,7 @@ IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, MAYBE_DnsPrefetch) {
 // Tests that preconnect warms up a socket connection to a test server.
 // Note: This test uses a data URI to serve the preconnect hint, to make sure
 // that the network stack doesn't just re-use its connection to the test server.
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, Preconnect) {
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectNonCORS) {
   GURL preconnect_url = embedded_test_server()->base_url();
   std::string preconnect_content =
       "<link rel=\"preconnect\" href=\"" + preconnect_url.spec() + "\">";
@@ -364,7 +365,7 @@ IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, Preconnect) {
 // and that that socket is later used when fetching a resource.
 // Note: This test uses a data URI to serve the preconnect hint, to make sure
 // that the network stack doesn't just re-use its connection to the test server.
-IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectAndUse) {
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectAndFetchNonCORS) {
   GURL preconnect_url = embedded_test_server()->base_url();
   // First navigation to content with a preconnect hint.
   std::string preconnect_content =
@@ -383,5 +384,76 @@ IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectAndUse) {
   EXPECT_EQ(1u, connection_listener_->GetReadSocketCount());
 }
 
+// Tests that preconnect warms up a CORS connection to a test
+// server, and that socket is later used when fetching a CORS resource.
+// Note: This test uses a data URI to serve the preconnect hint, to make sure
+// that the network stack doesn't just re-use its connection to the test server.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectAndFetchCORS) {
+  GURL preconnect_url = embedded_test_server()->base_url();
+  // First navigation to content with a preconnect hint.
+  std::string preconnect_content = "<link rel=\"preconnect\" href=\"" +
+                                   preconnect_url.spec() + "\" crossorigin>";
+  NavigateToDataURLWithContent(preconnect_content);
+  connection_listener_->WaitUntilFirstConnectionAccepted();
+  EXPECT_EQ(1u, connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(0u, connection_listener_->GetReadSocketCount());
+
+  // Second navigation to content with a font.
+  std::string font_content = "<script>var font = new FontFace('FontA', 'url(" +
+                             preconnect_url.spec() +
+                             "test.woff2)');font.load();</script>";
+  NavigateToDataURLWithContent(font_content);
+  connection_listener_->WaitUntilFirstConnectionRead();
+  EXPECT_EQ(1u, connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(1u, connection_listener_->GetReadSocketCount());
+}
+
+// Tests that preconnect warms up a non-CORS connection to a test
+// server, but that socket is not used when fetching a CORS resource.
+// Note: This test uses a data URI to serve the preconnect hint, to make sure
+// that the network stack doesn't just re-use its connection to the test server.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectNonCORSAndFetchCORS) {
+  GURL preconnect_url = embedded_test_server()->base_url();
+  // First navigation to content with a preconnect hint.
+  std::string preconnect_content =
+      "<link rel=\"preconnect\" href=\"" + preconnect_url.spec() + "\">";
+  NavigateToDataURLWithContent(preconnect_content);
+  connection_listener_->WaitUntilFirstConnectionAccepted();
+  EXPECT_EQ(1u, connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(0u, connection_listener_->GetReadSocketCount());
+
+  // Second navigation to content with a font.
+  std::string font_content = "<script>var font = new FontFace('FontA', 'url(" +
+                             preconnect_url.spec() +
+                             "test.woff2)');font.load();</script>";
+  NavigateToDataURLWithContent(font_content);
+  connection_listener_->WaitUntilFirstConnectionRead();
+  EXPECT_EQ(2u, connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(1u, connection_listener_->GetReadSocketCount());
+}
+
+// Tests that preconnect warms up a CORS connection to a test server,
+// but that socket is not used when fetching a non-CORS resource.
+// Note: This test uses a data URI to serve the preconnect hint, to make sure
+// that the network stack doesn't just re-use its connection to the test server.
+IN_PROC_BROWSER_TEST_F(PredictorBrowserTest, PreconnectCORSAndFetchNonCORS) {
+  GURL preconnect_url = embedded_test_server()->base_url();
+  // First navigation to content with a preconnect hint.
+  std::string preconnect_content = "<link rel=\"preconnect\" href=\"" +
+                                   preconnect_url.spec() + "\" crossorigin>";
+  NavigateToDataURLWithContent(preconnect_content);
+  connection_listener_->WaitUntilFirstConnectionAccepted();
+  EXPECT_EQ(1u, connection_listener_->GetAcceptedSocketCount());
+  EXPECT_EQ(0u, connection_listener_->GetReadSocketCount());
+
+  // 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