net: remove test flakyness caused by uncorking.

net/socket/ssl_client_socket_unittest.cc

 // Copyright (c) 2010 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 "net/socket/ssl_client_socket.h"
 
 #include "net/base/address_list.h"
 #include "net/base/host_resolver.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_log.h"
@@ skipping 17 matching lines @@
   SSLClientSocketTest()
       : socket_factory_(net::ClientSocketFactory::GetDefaultFactory()) {
   }
 
  protected:
   net::ClientSocketFactory* socket_factory_;
 };
 
 //-----------------------------------------------------------------------------
 
+// LogContainsSSLConnectEndEvent returns true if the given index in the given
+// log is an SSL connect end event. The NSS sockets will cork in an attempt to
+// merge the first application data record with the Finished message when false
+// starting. However, in order to avoid the server timing out the handshake,
+// they'll give up waiting for application data and send the Finished after a
+// timeout. This means that an SSL connect end event may appear as a socket

[wtc, 2010/10/07 21:29:54]

This sentence is confusing.  Can you explain exactly what
happens?  I think when the test fails, we have an
SSL connect end event, followed by a SOCKET_BYTES_SENT
event, right?  So the SSL connect end event is not missing;
it's just shadowed by the SOCKET_BYTES_SENT event caused by
the uncorking timer.

+// write.
+static bool LogContainsSSLConnectEndEvent(
+    const net::CapturingNetLog::EntryList& log, int i) {

[wtc, 2010/10/07 21:25:26]

BUG: the parameter 'i' is not used.  It seems that you
should pass 'i' instead of -1 to net::LogContainsEndEvent.

+  return net::LogContainsEndEvent(log, -1, net::NetLog::TYPE_SSL_CONNECT) ||
+         net::LogContainsEndEvent(log, -1, net::NetLog::TYPE_SOCKET_BYTES_SENT);
+};
+
 TEST_F(SSLClientSocketTest, Connect) {
   net::TestServer test_server(net::TestServer::TYPE_HTTPS, FilePath());
   ASSERT_TRUE(test_server.Start());
 
   net::AddressList addr;
   ASSERT_TRUE(test_server.GetAddressList(&addr));
 
   TestCompletionCallback callback;
   net::CapturingNetLog log(net::CapturingNetLog::kUnbounded);
   net::ClientSocket* transport = new net::TCPClientSocket(
@@ skipping 16 matching lines @@
     ASSERT_EQ(net::ERR_IO_PENDING, rv);
     EXPECT_FALSE(sock->IsConnected());
     EXPECT_FALSE(net::LogContainsEndEvent(
         log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
 
     rv = callback.WaitForResult();
     EXPECT_EQ(net::OK, rv);
   }
 
   EXPECT_TRUE(sock->IsConnected());
-  EXPECT_TRUE(net::LogContainsEndEvent(
-      log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
+  EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1));
 
   sock->Disconnect();
   EXPECT_FALSE(sock->IsConnected());
 }
 
 TEST_F(SSLClientSocketTest, ConnectExpired) {
   net::TestServer test_server(net::TestServer::TYPE_HTTPS_EXPIRED_CERTIFICATE,
                               FilePath());
   ASSERT_TRUE(test_server.Start());
 
@@ skipping 24 matching lines @@
     EXPECT_FALSE(net::LogContainsEndEvent(
         log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
 
     rv = callback.WaitForResult();
     EXPECT_EQ(net::ERR_CERT_DATE_INVALID, rv);
   }
 
   // We cannot test sock->IsConnected(), as the NSS implementation disconnects
   // the socket when it encounters an error, whereas other implementations
   // leave it connected.
-
-  EXPECT_TRUE(net::LogContainsEndEvent(
-      log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
+  EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1));
 }
 
 TEST_F(SSLClientSocketTest, ConnectMismatched) {
   net::TestServer test_server(net::TestServer::TYPE_HTTPS_MISMATCHED_HOSTNAME,
                               FilePath());
   ASSERT_TRUE(test_server.Start());
 
   net::AddressList addr;
   ASSERT_TRUE(test_server.GetAddressList(&addr));
 
@@ skipping 22 matching lines @@
     EXPECT_FALSE(net::LogContainsEndEvent(
         log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
 
     rv = callback.WaitForResult();
     EXPECT_EQ(net::ERR_CERT_COMMON_NAME_INVALID, rv);
   }
 
   // We cannot test sock->IsConnected(), as the NSS implementation disconnects
   // the socket when it encounters an error, whereas other implementations
   // leave it connected.
-
-  EXPECT_TRUE(net::LogContainsEndEvent(
-      log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
+  EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1));
 }
 
 // Attempt to connect to a page which requests a client certificate. It should
 // return an error code on connect.
 TEST_F(SSLClientSocketTest, FLAKY_ConnectClientAuthCertRequested) {
   net::TestServer test_server(net::TestServer::TYPE_HTTPS_CLIENT_AUTH,
                               FilePath());
   ASSERT_TRUE(test_server.Start());
 
   net::AddressList addr;
@@ skipping 23 matching lines @@
     EXPECT_FALSE(net::LogContainsEndEvent(
         log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
 
     rv = callback.WaitForResult();
     EXPECT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
   }
 
   // We cannot test sock->IsConnected(), as the NSS implementation disconnects
   // the socket when it encounters an error, whereas other implementations
   // leave it connected.
-
-  EXPECT_TRUE(net::LogContainsEndEvent(
-      log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
+  EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1));
 }
 
 // Connect to a server requesting optional client authentication. Send it a
 // null certificate. It should allow the connection.
 //
 // TODO(davidben): Also test providing an actual certificate.
 TEST_F(SSLClientSocketTest, ConnectClientAuthSendNullCert) {
   net::TestServer test_server(net::TestServer::TYPE_HTTPS_CLIENT_AUTH,
                               FilePath());
   ASSERT_TRUE(test_server.Start());
@@ skipping 29 matching lines @@
     ASSERT_EQ(net::ERR_IO_PENDING, rv);
     EXPECT_FALSE(sock->IsConnected());
     EXPECT_FALSE(net::LogContainsEndEvent(
         log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
 
     rv = callback.WaitForResult();
     EXPECT_EQ(net::OK, rv);
   }
 
   EXPECT_TRUE(sock->IsConnected());
-  EXPECT_TRUE(net::LogContainsEndEvent(
-      log.entries(), -1, net::NetLog::TYPE_SSL_CONNECT));
+  EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1));
 
   sock->Disconnect();
   EXPECT_FALSE(sock->IsConnected());
 }
 
 // TODO(wtc): Add unit tests for IsConnectedAndIdle:
 //   - Server closes an SSL connection (with a close_notify alert message).
 //   - Server closes the underlying TCP connection directly.
 //   - Server sends data unexpectedly.
 
@@ skipping 257 matching lines @@
     rv = callback.WaitForResult();
   EXPECT_EQ(net::OK, rv);
 
   scoped_ptr<net::SSLClientSocket> sock(
       socket_factory_->CreateSSLClientSocket(
           transport, test_server.host_port_pair().host(), kDefaultSSLConfig));
 
   rv = sock->Connect(&callback);
   EXPECT_EQ(net::ERR_SSL_PROTOCOL_ERROR, rv);
 }