This change enables FieldTrial for SPDY. When --use-spdy=npn is used, field test won't be enabled.

However, when that flag is missing, A/B test is added to browser_main.cc. Trial A: use npn and spdy. B: use npn but no spdy. C: do nothing. A and B are set to zero for now
The histograms we collect are: 
1. Page begin to finish time when spdy is enabled/disabled across all sites;
2. Page begin to finish time when spdy is used on sites that support spdy and when spdy is intentionally ignored. 


BUG=43997
TEST=Don't use --use-spdy=npn, manually change _npn_nospdy and _npn_withspdy percentile and go to spdy supported sites.

Review URL: http://codereview.chromium.org/2036012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@47896 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 129 insertions, 18 deletions

diff --git a/net/http/http_network_layer.cc b/net/http/http_network_layer.cc
index 09ee5cd..87422d4 100644
--- a/net/http/http_network_layer.cc
+++ b/net/http/http_network_layer.cc
@@ -4,6 +4,7 @@
 
 #include "net/http/http_network_layer.h"
 
+#include "base/field_trial.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "net/http/http_network_session.h"
@@ -122,7 +123,27 @@ HttpNetworkSession* HttpNetworkLayer::GetSession() {
 void HttpNetworkLayer::EnableSpdy(const std::string& mode) {
   static const char kDisableSSL[] = "no-ssl";
   static const char kDisableCompression[] = "no-compress";
+
+  // We want an A/B experiment between SPDY enabled and SPDY disabled,
+  // but only for pages where SPDY *could have been* negotiated.  To do
+  // this, we use NPN, but prevent it from negotiating SPDY.  If the
+  // server negotiates HTTP, rather than SPDY, today that will only happen
+  // on servers that installed NPN (and could have done SPDY).  But this is
+  // a bit of a hack, as this correlation between NPN and SPDY is not
+  // really guaranteed.
   static const char kEnableNPN[] = "npn";
+  static const char kEnableNpnHttpOnly[] = "npn-http";
+
+  // Except for the first element, the order is irrelevant.  First element
+  // specifies the fallback in case nothing matches
+  // (SSLClientSocket::kNextProtoNoOverlap).  Otherwise, the SSL library
+  // will choose the first overlapping protocol in the server's list, since
+  // it presumedly has a better understanding of which protocol we should
+  // use, therefore the rest of the ordering here is not important.
+  static const char kNpnProtosFull[] =
+      "\x08http/1.1\x07http1.1\x06spdy/1\x04spdy";
+  // No spdy specified.
+  static const char kNpnProtosHttpOnly[] = "\x08http/1.1\x07http1.1";
 
   std::vector<std::string> spdy_options;
   SplitString(mode, ',', &spdy_options);
@@ -133,22 +154,17 @@ void HttpNetworkLayer::EnableSpdy(const std::string& mode) {
   for (std::vector<std::string>::iterator it = spdy_options.begin();
        it != spdy_options.end(); ++it) {
     const std::string& option = *it;
-
-    // Disable SSL
     if (option == kDisableSSL) {
-      SpdySession::SetSSLMode(false);
+      SpdySession::SetSSLMode(false);  // Disable SSL
     } else if (option == kDisableCompression) {
       spdy::SpdyFramer::set_enable_compression_default(false);
     } else if (option == kEnableNPN) {
-      net::HttpNetworkTransaction::SetUseAlternateProtocols(true);
-      // Except for the first element, the order is irrelevant.  First element
-      // specifies the fallback in case nothing matches
-      // (SSLClientSocket::kNextProtoNoOverlap).  Otherwise, the SSL library
-      // will choose the first overlapping protocol in the server's list, since
-      // it presumedly has a better understanding of which protocol we should
-      // use, therefore the rest of the ordering here is not important.
-      HttpNetworkTransaction::SetNextProtos(
-          "\x08http/1.1\x07http1.1\x06spdy/1\x04spdy");
+      HttpNetworkTransaction::SetUseAlternateProtocols(true);
+      HttpNetworkTransaction::SetNextProtos(kNpnProtosFull);
+      force_spdy_ = false;
+    } else if (option == kEnableNpnHttpOnly) {
+      HttpNetworkTransaction::SetUseAlternateProtocols(true);
+      HttpNetworkTransaction::SetNextProtos(kNpnProtosHttpOnly);
       force_spdy_ = false;
     } else if (option.empty() && it == spdy_options.begin()) {
       continue;
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index 0d32ccf..36717c6f 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -911,6 +911,11 @@ int HttpNetworkTransaction::DoInitConnectionComplete(int result) {
   // trying to reuse a keep-alive connection.
   reused_socket_ = connection_->is_reused();
   if (reused_socket_) {
+    if (using_ssl_) {
+      SSLClientSocket* ssl_socket =
+          reinterpret_cast<SSLClientSocket*>(connection_->socket());
+      response_.was_npn_negotiated = ssl_socket->wasNpnNegotiated();
+    }
     next_state_ = STATE_SEND_REQUEST;
   } else {
     // Now we have a TCP connected socket.  Perform other connection setup as
@@ -962,9 +967,15 @@ int HttpNetworkTransaction::DoSSLConnectComplete(int result) {
   // here, then we know that we called SSL_ImportFD.
   if (result == OK || IsCertificateError(result))
     status = ssl_socket->GetNextProto(&proto);
-  using_spdy_ = (status == SSLClientSocket::kNextProtoNegotiated &&
-                 SSLClientSocket::NextProtoFromString(proto) ==
-                 SSLClientSocket::kProtoSPDY1);
+
+  if (status == SSLClientSocket::kNextProtoNegotiated) {
+    ssl_socket->setWasNpnNegotiated(true);
+    response_.was_npn_negotiated = true;
+    if (SSLClientSocket::NextProtoFromString(proto) ==
+        SSLClientSocket::kProtoSPDY1) {
+          using_spdy_ = true;
+    }
+  }
 
   if (alternate_protocol_mode_ == kUsingAlternateProtocol &&
       alternate_protocol_ == HttpAlternateProtocols::NPN_SPDY_1 &&
@@ -1130,7 +1141,7 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
       if (result == OK)
         return result;
     } else if ((result == ERR_SSL_DECOMPRESSION_FAILURE_ALERT ||
-                result == ERR_SSL_BAD_RECORD_MAC_ALERT ) &&
+                result == ERR_SSL_BAD_RECORD_MAC_ALERT) &&
                ssl_config_.tls1_enabled) {
       // Some buggy servers select DEFLATE compression when offered and then
       // fail to ever decompress anything. They will send a fatal alert telling
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 079f3ae..b427814 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -4868,6 +4868,8 @@ TEST_F(HttpNetworkTransactionTest, HonorAlternateProtocolHeader) {
   ASSERT_TRUE(response != NULL);
   ASSERT_TRUE(response->headers != NULL);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+  EXPECT_FALSE(response->was_fetched_via_spdy);
+  EXPECT_FALSE(response->was_npn_negotiated);
 
   std::string response_data;
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
@@ -5084,6 +5086,7 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForNpnSpdy) {
   SSLSocketDataProvider ssl(true, OK);
   ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated;
   ssl.next_proto = "spdy/1";
+  ssl.was_npn_negotiated = true;
   session_deps.socket_factory.AddSSLSocketDataProvider(&ssl);
 
   MockWrite spdy_writes[] = {
@@ -5134,6 +5137,8 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForNpnSpdy) {
   ASSERT_TRUE(response != NULL);
   ASSERT_TRUE(response->headers != NULL);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+  EXPECT_TRUE(response->was_fetched_via_spdy);
+  EXPECT_TRUE(response->was_npn_negotiated);
 
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
@@ -5213,6 +5218,7 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   SSLSocketDataProvider ssl(true, OK);
   ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated;
   ssl.next_proto = "spdy/1";
+  ssl.was_npn_negotiated = true;
   session_deps.socket_factory.AddSSLSocketDataProvider(&ssl);
 
   MockWrite spdy_writes[] = {
@@ -5253,6 +5259,8 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   ASSERT_TRUE(response != NULL);
   ASSERT_TRUE(response->headers != NULL);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+  EXPECT_FALSE(response->was_fetched_via_spdy);
+  EXPECT_FALSE(response->was_npn_negotiated);
 
   std::string response_data;
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
@@ -5268,6 +5276,8 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   ASSERT_TRUE(response != NULL);
   ASSERT_TRUE(response->headers != NULL);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+  EXPECT_TRUE(response->was_fetched_via_spdy);
+  EXPECT_TRUE(response->was_npn_negotiated);
 
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
@@ -5307,7 +5317,10 @@ TEST_F(HttpNetworkTransactionTest,
   SSLSocketDataProvider ssl(true, OK);
   ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated;
   ssl.next_proto = "spdy/1";
+  ssl.was_npn_negotiated = true;
   session_deps.socket_factory.AddSSLSocketDataProvider(&ssl);
+  // Make sure we use ssl for spdy here.
+  SpdySession::SetSSLMode(true);
 
   MockWrite spdy_writes[] = {
     MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
@@ -5354,7 +5367,6 @@ TEST_F(HttpNetworkTransactionTest,
                                         session, BoundNetLog());
   TCPSocketParams tcp_params("www.google.com", 443, MEDIUM, GURL(), false);
   spdy_session->Connect("www.google.com:443", tcp_params, MEDIUM);
-
   trans.reset(new HttpNetworkTransaction(session));
 
   rv = trans->Start(&request, &callback, BoundNetLog());
@@ -5365,6 +5377,8 @@ TEST_F(HttpNetworkTransactionTest,
   ASSERT_TRUE(response != NULL);
   ASSERT_TRUE(response->headers != NULL);
   EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+  EXPECT_TRUE(response->was_fetched_via_spdy);
+  EXPECT_TRUE(response->was_npn_negotiated);
 
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
@@ -5681,4 +5695,63 @@ TEST_F(HttpNetworkTransactionTest,
   EXPECT_EQ("ok.", response_data);
 }
 
+// This tests the case that a request is issued via http instead of spdy after
+// npn is negotiated.
+TEST_F(HttpNetworkTransactionTest, NpnWithHttpOverSSL) {
+  HttpNetworkTransaction::SetUseAlternateProtocols(true);
+  HttpNetworkTransaction::SetNextProtos("\x08http/1.1\x07http1.1");
+  SessionDependencies session_deps;
+  HttpRequestInfo request;
+  request.method = "GET";
+  request.url = GURL("https://www.google.com/");
+  request.load_flags = 0;
+
+  MockWrite data_writes[] = {
+    MockWrite("GET / HTTP/1.1\r\n"
+              "Host: www.google.com\r\n"
+              "Connection: keep-alive\r\n\r\n"),
+  };
+
+  MockRead data_reads[] = {
+    MockRead("HTTP/1.1 200 OK\r\n"),
+    MockRead("Alternate-Protocol: 443:npn-spdy/1\r\n\r\n"),
+    MockRead("hello world"),
+    MockRead(false, OK),
+  };
+
+  SSLSocketDataProvider ssl(true, OK);
+  ssl.next_proto_status = SSLClientSocket::kNextProtoNegotiated;
+  ssl.next_proto = "http/1.1";
+
+  session_deps.socket_factory.AddSSLSocketDataProvider(&ssl);
+
+  StaticSocketDataProvider data(data_reads, arraysize(data_reads),
+                                data_writes, arraysize(data_writes));
+  session_deps.socket_factory.AddSocketDataProvider(&data);
+
+  TestCompletionCallback callback;
+
+  scoped_refptr<HttpNetworkSession> session(CreateSession(&session_deps));
+  scoped_ptr<HttpTransaction> trans(new HttpNetworkTransaction(session));
+
+  int rv = trans->Start(&request, &callback, BoundNetLog());
+
+  EXPECT_EQ(ERR_IO_PENDING, rv);
+  EXPECT_EQ(OK, callback.WaitForResult());
+
+  const HttpResponseInfo* response = trans->GetResponseInfo();
+  ASSERT_TRUE(response != NULL);
+  ASSERT_TRUE(response->headers != NULL);
+  EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+  std::string response_data;
+  ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
+  EXPECT_EQ("hello world", response_data);
+
+  EXPECT_FALSE(response->was_fetched_via_spdy);
+  EXPECT_TRUE(response->was_npn_negotiated);
+
+  HttpNetworkTransaction::SetNextProtos("");
+  HttpNetworkTransaction::SetUseAlternateProtocols(false);
+}
 }  // namespace net
diff --git a/net/http/http_response_info.cc b/net/http/http_response_info.cc
index 3ce8cdd..d1e7911 100644
--- a/net/http/http_response_info.cc
+++ b/net/http/http_response_info.cc
@@ -41,7 +41,10 @@ enum {
   // This bit is set if the response was received via SPDY.
   RESPONSE_INFO_WAS_SPDY = 1 << 13,
 
-  // This bit is set if the response was received via SPDY.
+  // This bit is set if the request has NPN negotiated.
+  RESPONSE_INFO_WAS_NPN = 1 << 14,
+
+  // This bit is set if the request was fetched via an explicit proxy.
   RESPONSE_INFO_WAS_PROXY = 1 << 15,
 
   // TODO(darin): Add other bits to indicate alternate request methods.
@@ -51,6 +54,7 @@ enum {
 HttpResponseInfo::HttpResponseInfo()
     : was_cached(false),
       was_fetched_via_spdy(false),
+      was_npn_negotiated(false),
       was_fetched_via_proxy(false) {
 }
 
@@ -113,6 +117,8 @@ bool HttpResponseInfo::InitFromPickle(const Pickle& pickle,
 
   was_fetched_via_spdy = (flags & RESPONSE_INFO_WAS_SPDY) != 0;
 
+  was_npn_negotiated = (flags & RESPONSE_INFO_WAS_NPN) != 0;
+
   was_fetched_via_proxy = (flags & RESPONSE_INFO_WAS_PROXY) != 0;
 
   *response_truncated = (flags & RESPONSE_INFO_TRUNCATED) ? true : false;
@@ -136,6 +142,8 @@ void HttpResponseInfo::Persist(Pickle* pickle,
     flags |= RESPONSE_INFO_TRUNCATED;
   if (was_fetched_via_spdy)
     flags |= RESPONSE_INFO_WAS_SPDY;
+  if (was_npn_negotiated)
+    flags |= RESPONSE_INFO_WAS_NPN;
   if (was_fetched_via_proxy)
     flags |= RESPONSE_INFO_WAS_PROXY;
 
diff --git a/net/http/http_response_info.h b/net/http/http_response_info.h
index a61c299..518d8ac3 100644
--- a/net/http/http_response_info.h
+++ b/net/http/http_response_info.h
@@ -35,6 +35,9 @@ class HttpResponseInfo {
   // True if the request was fetched over a SPDY channel.
   bool was_fetched_via_spdy;
 
+  // True if the npn was negotiated for this request.
+  bool was_npn_negotiated;
+
   // True if the request was fetched via an explicit proxy.  The proxy could
   // be any type of proxy, HTTP or SOCKS.  Note, we do not know if a
   // transparent proxy may have been involved.