net: rework the NPN patch.

net/http/http_network_transaction_unittest.cc

Index: net/http/http_network_transaction_unittest.cc
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 9449a1241973ef514de590e076358e47ba70a801..fe81614c7b8019d9a90a0410b0bdc80e2def08de 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -5,6 +5,8 @@
 #include "net/http/http_network_transaction.h"
 
 #include <math.h>  // ceil
+#include <stdarg.h>
+#include <string>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -71,6 +73,32 @@ const string16 kWrongPassword(ASCIIToUTF16("wrongpassword"));
 
 namespace net {
 
+// MakeNextProtos is a utility function that returns a vector of std::strings
+// from its arguments. Don't forget to terminate the argument list with a NULL.
+static std::vector<std::string> MakeNextProtos(const char* a, ...) {
+  std::vector<std::string> ret;
+  ret.push_back(a);
+
+  va_list args;
+  va_start(args, a);
+
+  for (;;) {
+    const char* value = va_arg(args, const char*);
+    if (value == NULL)
+      break;
+    ret.push_back(value);
+  }
+  va_end(args);
+
+  return ret;
+}
+
+// SpdyNextProtos returns a vector of NPN protocol strings for negotiating
+// SPDY.
+static std::vector<std::string> SpdyNextProtos() {
+  return MakeNextProtos("http/1.1", "spdy/2", NULL);
+}

[wtc, 2011/10/11 23:43:04]

I suggest moving these two functions into the anonymous
namespace above and remove 'static'.

[agl, 2011/10/17 17:37:24]

Done.

+
 // Helper to manage the lifetimes of the dependencies for a
 // HttpNetworkTransaction.
 struct SessionDependencies {
@@ -6469,7 +6497,7 @@ TEST_F(HttpNetworkTransactionTest, ChangeAuthRealms) {
 }
 
 TEST_F(HttpNetworkTransactionTest, HonorAlternateProtocolHeader) {
-  HttpStreamFactory::set_next_protos("needs_to_be_set_for_this_test");
+  HttpStreamFactory::set_next_protos(MakeNextProtos("foo", "bar", NULL));
   HttpStreamFactory::set_use_alternate_protocols(true);
 
   SessionDependencies session_deps;
@@ -6526,7 +6554,7 @@ TEST_F(HttpNetworkTransactionTest, HonorAlternateProtocolHeader) {
   EXPECT_TRUE(expected_alternate.Equals(alternate));
 
   HttpStreamFactory::set_use_alternate_protocols(false);
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
 }
 
 TEST_F(HttpNetworkTransactionTest, MarkBrokenAlternateProtocolAndFallback) {
@@ -6782,7 +6810,7 @@ TEST_F(HttpNetworkTransactionTest, AlternateProtocolPortUnrestrictedAllowed2) {
 
 TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForNpnSpdy) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
   SessionDependencies session_deps;
 
   HttpRequestInfo request;
@@ -6867,13 +6895,13 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForNpnSpdy) {
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
 TEST_F(HttpNetworkTransactionTest, AlternateProtocolWithSpdyLateBinding) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
   SessionDependencies session_deps;
 
   HttpRequestInfo request;
@@ -6985,13 +7013,13 @@ TEST_F(HttpNetworkTransactionTest, AlternateProtocolWithSpdyLateBinding) {
   ASSERT_EQ(OK, ReadTransaction(&trans3, &response_data));
   EXPECT_EQ("hello!", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
 TEST_F(HttpNetworkTransactionTest, StallAlternateProtocolForNpnSpdy) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
   SessionDependencies session_deps;
 
   HttpRequestInfo request;
@@ -7061,7 +7089,7 @@ TEST_F(HttpNetworkTransactionTest, StallAlternateProtocolForNpnSpdy) {
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello world", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -7105,7 +7133,7 @@ class CapturingProxyResolver : public ProxyResolver {
 
 TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
 
   ProxyConfig proxy_config;
   proxy_config.set_auto_detect(true);
@@ -7213,14 +7241,14 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   EXPECT_EQ("https://www.google.com/",
             capturing_proxy_resolver->resolved()[1].spec());
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
 TEST_F(HttpNetworkTransactionTest,
        UseAlternateProtocolForNpnSpdyWithExistingSpdySession) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
   SessionDependencies session_deps;
 
   HttpRequestInfo request;
@@ -7332,7 +7360,7 @@ TEST_F(HttpNetworkTransactionTest,
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -8048,7 +8076,8 @@ TEST_F(HttpNetworkTransactionTest,
 // npn is negotiated.
 TEST_F(HttpNetworkTransactionTest, NpnWithHttpOverSSL) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos("\x08http/1.1\x07http1.1");
+  HttpStreamFactory::set_next_protos(
+      MakeNextProtos("http/1.1", "http1.1", NULL));
   SessionDependencies session_deps;
   HttpRequestInfo request;
   request.method = "GET";
@@ -8100,7 +8129,7 @@ TEST_F(HttpNetworkTransactionTest, NpnWithHttpOverSSL) {
   EXPECT_FALSE(response->was_fetched_via_spdy);
   EXPECT_TRUE(response->was_npn_negotiated);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -8109,7 +8138,7 @@ TEST_F(HttpNetworkTransactionTest, SpdyPostNPNServerHangup) {
   // followed by an immediate server closing of the socket.
   // Fix crash:  http://crbug.com/46369
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
   SessionDependencies session_deps;
 
   HttpRequestInfo request;
@@ -8146,7 +8175,7 @@ TEST_F(HttpNetworkTransactionTest, SpdyPostNPNServerHangup) {
   EXPECT_EQ(ERR_IO_PENDING, rv);
   EXPECT_EQ(ERR_CONNECTION_CLOSED, callback.WaitForResult());
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -8155,7 +8184,7 @@ TEST_F(HttpNetworkTransactionTest, SpdyAlternateProtocolThroughProxy) {
   // to https when doing an Alternate Protocol upgrade.
   HttpStreamFactory::set_use_alternate_protocols(true);
   HttpStreamFactory::set_next_protos(
-      "\x08http/1.1\x07http1.1\x06spdy/2\x04spdy");
+      MakeNextProtos("http/1.1", "http1.1", "spdy/2", "spdy", NULL));
 
   SessionDependencies session_deps(ProxyService::CreateFixed("myproxy:70"));
   HttpAuthHandlerMock::Factory* auth_factory =
@@ -8296,7 +8325,7 @@ TEST_F(HttpNetworkTransactionTest, SpdyAlternateProtocolThroughProxy) {
   EXPECT_EQ("https", request_url.scheme());
   EXPECT_EQ("www.google.com", request_url.host());
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -8947,7 +8976,7 @@ void IPPoolingAddAlias(MockCachingHostResolver* host_resolver,
 
 TEST_F(HttpNetworkTransactionTest, UseIPConnectionPooling) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
 
   // Set up a special HttpNetworkSession with a MockCachingHostResolver.
   SessionDependencies session_deps;
@@ -9052,7 +9081,7 @@ TEST_F(HttpNetworkTransactionTest, UseIPConnectionPooling) {
   ASSERT_EQ(OK, ReadTransaction(&trans2, &response_data));
   EXPECT_EQ("hello!", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }
 
@@ -9107,7 +9136,7 @@ class OneTimeCachingHostResolver : public net::HostResolver {
 TEST_F(HttpNetworkTransactionTest,
        UseIPConnectionPoolingWithHostCacheExpiration) {
   HttpStreamFactory::set_use_alternate_protocols(true);
-  HttpStreamFactory::set_next_protos(kExpectedNPNString);
+  HttpStreamFactory::set_next_protos(SpdyNextProtos());
 
   // Set up a special HttpNetworkSession with a OneTimeCachingHostResolver.
   SessionDependencies session_deps;
@@ -9211,7 +9240,7 @@ TEST_F(HttpNetworkTransactionTest,
   ASSERT_EQ(OK, ReadTransaction(&trans2, &response_data));
   EXPECT_EQ("hello!", response_data);
 
-  HttpStreamFactory::set_next_protos("");
+  HttpStreamFactory::set_next_protos(std::vector<std::string>());
   HttpStreamFactory::set_use_alternate_protocols(false);
 }