QUIC: mark QUIC handshake failed if connection is closed after CryptoConnect

net/quic/chromium/quic_stream_factory_test.cc

Index: net/quic/chromium/quic_stream_factory_test.cc
diff --git a/net/quic/chromium/quic_stream_factory_test.cc b/net/quic/chromium/quic_stream_factory_test.cc
index d6fd5aeb42ec0c74be6e856b963bf382beebf52f..eb03ece8a0a0a358d902c384d995eb2763874463 100644
--- a/net/quic/chromium/quic_stream_factory_test.cc
+++ b/net/quic/chromium/quic_stream_factory_test.cc
@@ -316,6 +316,12 @@ class QuicStreamFactoryTestBase {
     return QuicStreamFactoryPeer::HasActiveSession(factory_.get(), server_id);
   }
 
+  bool HasActiveJob(const HostPortPair& host_port_pair,
+                    const PrivacyMode privacy_mode) {
+    QuicServerId server_id(host_port_pair, privacy_mode);
+    return QuicStreamFactoryPeer::HasActiveJob(factory_.get(), server_id);
+  }
+
   bool HasActiveCertVerifierJob(const QuicServerId& server_id) {
     return QuicStreamFactoryPeer::HasActiveCertVerifierJob(factory_.get(),
                                                            server_id);
@@ -1803,6 +1809,92 @@ TEST_P(QuicStreamFactoryTest, CloseAllSessions) {
   EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
 }
 
+// Regression test for crbug.com/700617. Test a write error during the
+// crypto handshake will not hang QuicStreamFactory::Job and should
+// report QUIC_HANDSHAKE_FAILED to upper layers.
+TEST_P(QuicStreamFactoryTest, WriteErrorInCryptoConnect) {
+  Initialize();
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::USE_DEFAULT_CRYPTO_STREAM);
+
+  MockQuicData socket_data;
+  socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+  socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+  socket_data.AddSocketDataToFactory(&socket_factory_);
+
+  // Create request and QuicHttpStream.
+  QuicStreamRequest request(factory_.get());
+  EXPECT_EQ(ERR_IO_PENDING,
+            request.Request(host_port_pair_, privacy_mode_,
+                            /*cert_verify_flags=*/0, url_, "GET", net_log_,
+                            callback_.callback()));
+  EXPECT_EQ(ERR_QUIC_HANDSHAKE_FAILED, callback_.WaitForResult());
+  EXPECT_FALSE(HasActiveSession(host_port_pair_));
+  EXPECT_FALSE(HasActiveJob(host_port_pair_, privacy_mode_));
+}
+
+TEST_P(QuicStreamFactoryTest, RetryAfterWriteErrorInCryptoConnect) {
+  Initialize();
+  // Use unmocked crypto stream to do crypto connect.
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::USE_DEFAULT_CRYPTO_STREAM);
+  host_resolver_.set_synchronous_mode(true);
+  host_resolver_.rules()->AddIPLiteralRule(host_port_pair_.host(),
+                                           "192.168.0.1", "");
+
+  MockQuicData socket_data;
+  socket_data.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+  // Trigger PACKET_WRITE_ERROR when sending packets in crypto connect.
+  socket_data.AddWrite(SYNCHRONOUS, ERR_ADDRESS_UNREACHABLE);
+  socket_data.AddSocketDataToFactory(&socket_factory_);
+
+  // Create request, should fail immediately.

[Ryan Hamilton, 2017/03/22 14:13:01]

It would be nice if the comments in this test and the previous test matched.

[Zhongyi Shi, 2017/03/22 18:53:10]

Done.

+  QuicStreamRequest request(factory_.get());
+  EXPECT_EQ(ERR_QUIC_HANDSHAKE_FAILED,
+            request.Request(host_port_pair_, privacy_mode_,
+                            /*cert_verify_flags=*/0, url_, "GET", net_log_,
+                            callback_.callback()));
+  // Check no active session, or active jobs left for this server.
+  EXPECT_FALSE(HasActiveSession(host_port_pair_));
+  EXPECT_FALSE(HasActiveJob(host_port_pair_, privacy_mode_));
+
+  // Verify new requests can be sent normally without hanging.
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::COLD_START);
+  ProofVerifyDetailsChromium verify_details = DefaultProofVerifyDetails();
+  crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
+  MockQuicData socket_data2;
+  socket_data2.AddRead(SYNCHRONOUS, ERR_IO_PENDING);
+  socket_data2.AddWrite(
+      ConstructSettingsPacket(1, SETTINGS_MAX_HEADER_LIST_SIZE,
+                              kDefaultMaxUncompressedHeaderSize, nullptr));
+  socket_data2.AddSocketDataToFactory(&socket_factory_);
+
+  QuicStreamRequest request2(factory_.get());
+  EXPECT_EQ(ERR_IO_PENDING,
+            request2.Request(host_port_pair_, privacy_mode_,
+                             /*cert_verify_flags=*/0, url_, "GET", net_log_,
+                             callback_.callback()));
+  EXPECT_FALSE(HasActiveSession(host_port_pair_));
+  EXPECT_TRUE(HasActiveJob(host_port_pair_, privacy_mode_));
+  // Complete handshake.
+  crypto_client_stream_factory_.last_stream()->SendOnCryptoHandshakeEvent(
+      QuicSession::HANDSHAKE_CONFIRMED);
+  EXPECT_THAT(callback_.WaitForResult(), IsOk());
+
+  EXPECT_TRUE(HasActiveSession(host_port_pair_));
+  EXPECT_FALSE(HasActiveJob(host_port_pair_, privacy_mode_));
+
+  // Create QuicHttpStream.
+  std::unique_ptr<QuicHttpStream> stream = request2.CreateStream();
+  EXPECT_TRUE(stream.get());
+  stream.reset();
+  EXPECT_TRUE(socket_data.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data.AllWriteDataConsumed());
+  EXPECT_TRUE(socket_data2.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
+}
+
 TEST_P(QuicStreamFactoryTest, OnIPAddressChanged) {
   close_sessions_on_ip_change_ = true;
   Initialize();