Remove LogContainsSSLConnectEndEvent.

net/socket/ssl_client_socket_unittest.cc

 // Copyright (c) 2012 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 "base/callback_helpers.h"
 #include "base/memory/ref_counted.h"
 #include "base/run_loop.h"
 #include "base/thread_task_runner_handle.h"
@@ skipping 954 matching lines @@
         new AsyncFailingChannelIDStore(), base::ThreadTaskRunnerHandle::Get()));
     context_.channel_id_service = channel_id_service_.get();
   }
 
  private:
   scoped_ptr<ChannelIDService> channel_id_service_;
 };
 
 //-----------------------------------------------------------------------------
 
-// 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
-// write.
-static bool LogContainsSSLConnectEndEvent(const TestNetLogEntry::List& log,
-                                          int i) {
-  return LogContainsEndEvent(log, i, NetLog::TYPE_SSL_CONNECT) ||
-         LogContainsEvent(
-             log, i, NetLog::TYPE_SOCKET_BYTES_SENT, NetLog::PHASE_NONE);
-}
-
 bool SupportsAESGCM() {
 #if defined(USE_OPENSSL)
   return true;
 #else
   crypto::EnsureNSSInit();
   return PK11_TokenExists(CKM_AES_GCM) &&
          PK11_TokenExists(CKM_NSS_TLS_MASTER_KEY_DERIVE_DH_SHA256);
 #endif
 }
 
@@ skipping 25 matching lines @@
   rv = sock->Connect(callback.callback());
 
   TestNetLogEntry::List entries;
   log.GetEntries(&entries);
   EXPECT_TRUE(LogContainsBeginEvent(entries, 5, NetLog::TYPE_SSL_CONNECT));
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(sock->IsConnected());
   log.GetEntries(&entries);
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT));
 
   sock->Disconnect();
   EXPECT_FALSE(sock->IsConnected());
 }
 
 TEST_F(SSLClientSocketTest, ConnectExpired) {
   SpawnedTestServer::SSLOptions ssl_options(
       SpawnedTestServer::SSLOptions::CERT_EXPIRED);
   SpawnedTestServer test_server(
       SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath());
@@ skipping 26 matching lines @@
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
 
   EXPECT_EQ(ERR_CERT_DATE_INVALID, rv);
 
   // Rather than testing whether or not the underlying socket is connected,
   // test that the handshake has finished. This is because it may be
   // desirable to disconnect the socket before showing a user prompt, since
   // the user may take indefinitely long to respond.
   log.GetEntries(&entries);
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT));
 }
 
 TEST_F(SSLClientSocketTest, ConnectMismatched) {
   SpawnedTestServer::SSLOptions ssl_options(
       SpawnedTestServer::SSLOptions::CERT_MISMATCHED_NAME);
   SpawnedTestServer test_server(
       SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath());
   ASSERT_TRUE(test_server.Start());
 
   cert_verifier_->set_default_result(ERR_CERT_COMMON_NAME_INVALID);
@@ skipping 23 matching lines @@
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
 
   EXPECT_EQ(ERR_CERT_COMMON_NAME_INVALID, rv);
 
   // Rather than testing whether or not the underlying socket is connected,
   // test that the handshake has finished. This is because it may be
   // desirable to disconnect the socket before showing a user prompt, since
   // the user may take indefinitely long to respond.
   log.GetEntries(&entries);
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT));
 }
 
 // Attempt to connect to a page which requests a client certificate. It should
 // return an error code on connect.
 TEST_F(SSLClientSocketTest, ConnectClientAuthCertRequested) {
   SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.request_client_certificate = true;
   SpawnedTestServer test_server(
       SpawnedTestServer::TYPE_HTTPS, ssl_options, base::FilePath());
   ASSERT_TRUE(test_server.Start());
@@ skipping 17 matching lines @@
 
   rv = sock->Connect(callback.callback());
 
   TestNetLogEntry::List entries;
   log.GetEntries(&entries);
   EXPECT_TRUE(LogContainsBeginEvent(entries, 5, NetLog::TYPE_SSL_CONNECT));
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
 
   log.GetEntries(&entries);
-  // Because we prematurely kill the handshake at CertificateRequest,
-  // the server may still send data (notably the ServerHelloDone)
-  // after the error is returned. As a result, the SSL_CONNECT may not
-  // be the last entry. See http://crbug.com/54445. We use
-  // ExpectLogContainsSomewhere instead of
-  // LogContainsSSLConnectEndEvent to avoid assuming, e.g., only one
-  // extra read instead of two. This occurs before the handshake ends,
-  // so the corking logic of LogContainsSSLConnectEndEvent isn't
-  // necessary.
+  // Because the handshake is aborted during the CertificateRequest, the server
+  // may still send the ServerHelloDone afterwards. As a result, the SSL_CONNECT
+  // may not be the last entry. See http://crbug.com/54445. Because of this, use
+  // ExpectLogContainsSomewhere, rather than LogContainsEndEvent, to avoid
+  // assuming particular details about the read (e.g. assuming there will only
+  // be one extra read, when there might be more).
   //
-  // TODO(davidben): When SSL_RestartHandshakeAfterCertReq in NSS is
-  // fixed and we can respond to the first CertificateRequest
-  // without closing the socket, add a unit test for sending the
-  // certificate. This test may still be useful as we'll want to close
-  // the socket on a timeout if the user takes a long time to pick a
-  // cert. Related bug: https://bugzilla.mozilla.org/show_bug.cgi?id=542832
+  // TODO(davidben): Is this still possible with memio_GetReadRequest?
   ExpectLogContainsSomewhere(
       entries, 0, NetLog::TYPE_SSL_CONNECT, NetLog::PHASE_END);
   EXPECT_EQ(ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv);
   EXPECT_FALSE(sock->IsConnected());
 }
 
 // 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.
@@ skipping 31 matching lines @@
 
   TestNetLogEntry::List entries;
   log.GetEntries(&entries);
   EXPECT_TRUE(LogContainsBeginEvent(entries, 5, NetLog::TYPE_SSL_CONNECT));
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
 
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(sock->IsConnected());
   log.GetEntries(&entries);
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT));
 
   // We responded to the server's certificate request with a Certificate
   // message with no client certificate in it.  ssl_info.client_cert_sent
   // should be false in this case.
   SSLInfo ssl_info;
   sock->GetSSLInfo(&ssl_info);
   EXPECT_FALSE(ssl_info.client_cert_sent);
 
   sock->Disconnect();
   EXPECT_FALSE(sock->IsConnected());
@@ skipping 1033 matching lines @@
   // the socket when it encounters an error, whereas other implementations
   // leave it connected.
   // Because this an error that the test server is mutually aware of, as opposed
   // to being an error such as a certificate name mismatch, which is
   // client-only, the exact index of the SSL connect end depends on how
   // quickly the test server closes the underlying socket. If the test server
   // closes before the IO message loop pumps messages, there may be a 0-byte
   // Read event in the NetLog due to TCPClientSocket picking up the EOF. As a
   // result, the SSL connect end event will be the second-to-last entry,
   // rather than the last entry.
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1) ||
-              LogContainsSSLConnectEndEvent(entries, -2));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT) ||
+              LogContainsEndEvent(entries, -2, NetLog::TYPE_SSL_CONNECT));
 }
 
 // When creating an SSLClientSocket, it is allowed to pass in a
 // ClientSocketHandle that is not obtained from a client socket pool.
 // Here we verify that such a simple ClientSocketHandle, not associated with any
 // client socket pool, can be destroyed safely.
 TEST_F(SSLClientSocketTest, ClientSocketHandleNotFromPool) {
   SpawnedTestServer test_server(SpawnedTestServer::TYPE_HTTPS,
                                 SpawnedTestServer::kLocalhost,
                                 base::FilePath());
@@ skipping 257 matching lines @@
 
   TestNetLogEntry::List entries;
   log.GetEntries(&entries);
   EXPECT_TRUE(LogContainsBeginEvent(entries, 5, NetLog::TYPE_SSL_CONNECT));
   if (rv == ERR_IO_PENDING)
     rv = callback.WaitForResult();
 
   EXPECT_EQ(OK, rv);
   EXPECT_TRUE(sock->IsConnected());
   log.GetEntries(&entries);
-  EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1));
+  EXPECT_TRUE(LogContainsEndEvent(entries, -1, NetLog::TYPE_SSL_CONNECT));
 
   SSLInfo ssl_info;
   sock->GetSSLInfo(&ssl_info);
 
   // Verify that SSLInfo contains the corrected re-constructed chain A -> B
   // -> C2.
   const X509Certificate::OSCertHandles& intermediates =
       ssl_info.cert->GetIntermediateCertificates();
   ASSERT_EQ(2U, intermediates.size());
   EXPECT_TRUE(X509Certificate::IsSameOSCert(ssl_info.cert->os_cert_handle(),
@@ skipping 868 matching lines @@
   ssl_config.channel_id_enabled = true;
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(ssl_config, &rv));
 
   EXPECT_EQ(ERR_UNEXPECTED, rv);
   EXPECT_FALSE(sock_->IsConnected());
 }
 
 }  // namespace net