Get tests working for Alternate-Protocol and proxies.

Fixed some bugs along the way.

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

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

4 files changed, 43 insertions, 31 deletions

diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index d0edafa..063460d 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -1105,7 +1105,7 @@ int HttpNetworkTransaction::DoReadHeadersComplete(int result) {
         next_state_ = STATE_SSL_CONNECT;
         // Reset for the real request and response headers.
         request_headers_.clear();
-        http_stream_.reset(new HttpBasicStream(connection_.get(), net_log_));
+        http_stream_.reset(NULL);
         headers_valid_ = false;
         establishing_tunnel_ = false;
         // TODO(mbelshe): We should put in a test case to trip this code path.
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 5b6a9b8..b8eb6e4 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -4984,8 +4984,11 @@ class CapturingProxyResolver : public ProxyResolver {
                              CompletionCallback* callback,
                              RequestHandle* request,
                              const BoundNetLog& net_log) {
+    ProxyServer proxy_server(
+        ProxyServer::SCHEME_HTTP, "myproxy", 80);
+    results->UseProxyServer(proxy_server);
     resolved_.push_back(url);
-    return ERR_NOT_IMPLEMENTED;
+    return OK;
   }
 
   virtual void CancelRequest(RequestHandle request) {
@@ -4998,7 +5001,7 @@ class CapturingProxyResolver : public ProxyResolver {
   virtual int SetPacScript(const GURL& /*pac_url*/,
                            const std::string& /*pac_bytes*/,
                            CompletionCallback* /*callback*/) {
-    return ERR_NOT_IMPLEMENTED;
+    return OK;
   }
 
   std::vector<GURL> resolved_;
@@ -5006,18 +5009,14 @@ class CapturingProxyResolver : public ProxyResolver {
   DISALLOW_COPY_AND_ASSIGN(CapturingProxyResolver);
 };
 
-// TODO(willchan): Enable this test after I refactor the OrderedSocketData out
-// of SpdyNetworkTransaction.  Currently, I need to the CONNECT, read the
-// response, and then send the request, and then read the response.
-// DelayedSocketDataProvider doesn't permit this yet.
-#if 0
 TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   HttpNetworkTransaction::SetUseAlternateProtocols(true);
   HttpNetworkTransaction::SetNextProtos(
           "\x08http/1.1\x07http1.1\x06spdy/1\x04spdy");
 
   ProxyConfig proxy_config;
-  proxy_config.proxy_rules().ParseFromString("myproxy:80");
+  proxy_config.set_auto_detect(true);
+  proxy_config.set_pac_url(GURL("http://fooproxyurl"));
 
   CapturingProxyResolver* capturing_proxy_resolver =
       new CapturingProxyResolver();
@@ -5051,23 +5050,24 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
   MockWrite spdy_writes[] = {
     MockWrite("CONNECT www.google.com:443 HTTP/1.1\r\n"
               "Host: www.google.com\r\n"
-              "Proxy-Connection: keep-alive\r\n\r\n"),
+              "Proxy-Connection: keep-alive\r\n\r\n"),  // 0
     MockWrite(true, reinterpret_cast<const char*>(kGetSyn),
-              arraysize(kGetSyn)),
+              arraysize(kGetSyn)),  // 3
   };
 
+  const char kCONNECTResponse[] = "HTTP/1.1 200 Connected\r\n\r\n";
+
   MockRead spdy_reads[] = {
-    MockRead("HTTP/1.1 200 Connected\r\n\r\n"),
-    MockRead(true, reinterpret_cast<const char*>(kGetSynReply),
-             arraysize(kGetSynReply)),
-    MockRead(true, reinterpret_cast<const char*>(kGetBodyFrame),
-             arraysize(kGetBodyFrame)),
-    MockRead(true, 0, 0),
+    MockRead(true, kCONNECTResponse, arraysize(kCONNECTResponse) - 1, 1),  // 1
+    MockRead(true, reinterpret_cast<const char*>(kGetSynReply),  // 2, 4
+             arraysize(kGetSynReply), 4),
+    MockRead(true, reinterpret_cast<const char*>(kGetBodyFrame),  // 5
+             arraysize(kGetBodyFrame), 4),
+    MockRead(true, 0, 0, 4),  // 6
   };
 
-  scoped_refptr<DelayedSocketData> spdy_data(
-      new DelayedSocketData(
-          1,  // wait for one write to finish before reading.
+  scoped_refptr<OrderedSocketData> spdy_data(
+      new OrderedSocketData(
           spdy_reads, arraysize(spdy_reads),
           spdy_writes, arraysize(spdy_writes)));
   session_deps.socket_factory.AddSocketDataProvider(spdy_data);
@@ -5103,14 +5103,15 @@ TEST_F(HttpNetworkTransactionTest, UseAlternateProtocolForTunneledNpnSpdy) {
 
   ASSERT_EQ(OK, ReadTransaction(trans.get(), &response_data));
   EXPECT_EQ("hello!", response_data);
-  ASSERT_EQ(1u, capturing_proxy_resolver->resolved().size());
-  EXPECT_EQ("https://www.google.com:443/",
+  ASSERT_EQ(2u, capturing_proxy_resolver->resolved().size());
+  EXPECT_EQ("http://www.google.com/",
             capturing_proxy_resolver->resolved()[0].spec());
+  EXPECT_EQ("https://www.google.com/",
+            capturing_proxy_resolver->resolved()[1].spec());
 
   HttpNetworkTransaction::SetNextProtos("");
   HttpNetworkTransaction::SetUseAlternateProtocols(false);
 }
-#endif
 
 TEST_F(HttpNetworkTransactionTest,
        UseAlternateProtocolForNpnSpdyWithExistingSpdySession) {
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index 399a34c..99a200f 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -531,7 +531,7 @@ OrderedSocketData::OrderedSocketData(
     MockRead* reads, size_t reads_count, MockWrite* writes, size_t writes_count)
     : StaticSocketDataProvider(reads, reads_count, writes, writes_count),
       sequence_number_(0), loop_stop_stage_(0), callback_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
+      blocked_(false), ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
 }
 
 OrderedSocketData::OrderedSocketData(
@@ -540,11 +540,13 @@ OrderedSocketData::OrderedSocketData(
     MockWrite* writes, size_t writes_count)
     : StaticSocketDataProvider(reads, reads_count, writes, writes_count),
       sequence_number_(0), loop_stop_stage_(0), callback_(NULL),
-      ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
+      blocked_(false), ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) {
   set_connect_data(connect);
 }
 
 MockRead OrderedSocketData::GetNextRead() {
+  factory_.RevokeAll();
+  blocked_ = false;
   const MockRead& next_read = StaticSocketDataProvider::PeekRead();
   if (next_read.sequence_number & MockRead::STOPLOOP)
     EndLoop();
@@ -559,6 +561,7 @@ MockRead OrderedSocketData::GetNextRead() {
                             << ": I/O Pending";
   MockRead result = MockRead(true, ERR_IO_PENDING);
   DumpMockRead(result);
+  blocked_ = true;
   return result;
 }
 
@@ -567,9 +570,16 @@ MockWriteResult OrderedSocketData::OnWrite(const std::string& data) {
                             << ": Write " << write_index();
   DumpMockRead(PeekWrite());
   ++sequence_number_;
-  MessageLoop::current()->PostDelayedTask(
-      FROM_HERE,
-      factory_.NewRunnableMethod(&OrderedSocketData::CompleteRead), 100);
+  if (blocked_) {
+    // TODO(willchan): This 100ms delay seems to work around some weirdness.  We
+    // should probably fix the weirdness.  One example is in SpdyStream,
+    // DoSendRequest() will return ERR_IO_PENDING, and there's a race.  If the
+    // SYN_REPLY causes OnResponseReceived() to get called before
+    // SpdyStream::ReadResponseHeaders() is called, we hit a NOTREACHED().
+    MessageLoop::current()->PostDelayedTask(
+        FROM_HERE,
+        factory_.NewRunnableMethod(&OrderedSocketData::CompleteRead), 100);
+  }
   return StaticSocketDataProvider::OnWrite(data);
 }
 
diff --git a/net/socket/socket_test_util.h b/net/socket/socket_test_util.h
index 6a47539..f50cf7d 100644
--- a/net/socket/socket_test_util.h
+++ b/net/socket/socket_test_util.h
@@ -93,9 +93,9 @@ struct MockRead {
   const char* data;
   int data_len;
 
-  // For OrderedSocketData in spdy_network_transaction_unittest.cc, which only
-  // allows reads to occur in a particular sequence.  If a read occurs before
-  // the given |sequence_number| is reached, an ERR_IO_PENDING is returned.
+  // For OrderedSocketData, which only allows reads to occur in a particular
+  // sequence.  If a read occurs before the given |sequence_number| is reached,
+  // an ERR_IO_PENDING is returned.
   int sequence_number;      // The sequence number at which a read is allowed
                             // to occur.
   base::Time time_stamp;    // The time stamp at which the operation occurred.
@@ -324,6 +324,7 @@ class OrderedSocketData : public StaticSocketDataProvider,
   int sequence_number_;
   int loop_stop_stage_;
   CompletionCallback* callback_;
+  bool blocked_;
   ScopedRunnableMethodFactory<OrderedSocketData> factory_;
 };