QUIC - disable QUIC under recent pathological connection errors.

net/quic/quic_stream_factory_test.cc

Index: net/quic/quic_stream_factory_test.cc
diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc
index 195e410ff72517f6ee587ffc27ad0eae6460a58c..8cb2a84aaa4eba00dd22a92ef3bf00ba9f048c2f 100644
--- a/net/quic/quic_stream_factory_test.cc
+++ b/net/quic/quic_stream_factory_test.cc
@@ -152,6 +152,27 @@ class QuicStreamFactoryPeer {
                                       const QuicServerId& server_id) {
     return (factory->active_jobs_[server_id]).size();
   }
+
+  static void SetThresholdTimeoutsStreamsOpen(
+      QuicStreamFactory* factory,
+      int threshold_timeouts_streams_open) {
+    factory->threshold_timeouts_streams_open_ = threshold_timeouts_streams_open;
+  }
+
+  static int GetTimeoutsStreamsOpen(QuicStreamFactory* factory) {
+    return factory->timeouts_streams_open_;

[Ryan Hamilton, 2015/06/30 18:55:32]

here and elsewhere, I think "timeouts with open streams" is easier to read than
"timeouts streams open" (which is admittedly shorter :>)

[Buck, 2015/07/01 19:06:20]

Done.

+  }
+
+  static void SetThresholdPublicResetsPostHandshake(
+      QuicStreamFactory* factory,
+      int threshold_public_resets_post_handshake) {
+    factory->threshold_public_resets_post_handshake_ =
+        threshold_public_resets_post_handshake;
+  }
+
+  static int GetPublicResetsPostHandshake(QuicStreamFactory* factory) {
+    return factory->public_resets_post_handshake_;
+  }
 };
 
 class MockQuicServerInfo : public QuicServerInfo {
@@ -222,6 +243,9 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<TestParams> {
                  /*prefer_aes=*/false,
                  /*max_number_of_lossy_connections=*/0,
                  /*packet_loss_threshold=*/1.0f,
+                 /*max_epitaphs=*/3,
+                 /*threshold_timeouts_streams_open=*/2,
+                 /*threshold_pulic_resets_post_handshake=*/2,
                  /*receive_buffer_size=*/0,
                  QuicTagVector()),
         host_port_pair_(kDefaultServerHostName, kDefaultServerPort),
@@ -1739,5 +1763,170 @@ TEST_P(QuicStreamFactoryTest, BadPacketLoss) {
   EXPECT_TRUE(socket_data4.AllWriteDataConsumed());
 }
 
+TEST_P(QuicStreamFactoryTest, PublicResetPostHandshake) {
+  factory_.set_quic_server_info_factory(&quic_server_info_factory_);
+  QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
+  QuicStreamFactoryPeer::SetDisableDiskCache(&factory_, true);
+  QuicStreamFactoryPeer::SetThresholdPublicResetsPostHandshake(&factory_, 2);
+  EXPECT_FALSE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));
+  EXPECT_EQ(0, QuicStreamFactoryPeer::GetNumberOfLossyConnections(
+                   &factory_, host_port_pair_.port()));
+
+  MockRead reads[] = {
+      MockRead(ASYNC, OK, 0)  // EOF
+  };
+  DeterministicSocketData socket_data(reads, arraysize(reads), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data);
+  socket_data.StopAfter(1);
+
+  //  DeterministicSocketData socket_data2(nullptr, 0, nullptr, 0);
+  DeterministicSocketData socket_data2(reads, arraysize(reads), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data2);
+  socket_data2.StopAfter(1);
+
+  HostPortPair server2("mail.example.org", kDefaultServerPort);
+
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::CONFIRM_HANDSHAKE);
+  host_resolver_.set_synchronous_mode(true);
+  host_resolver_.rules()->AddIPLiteralRule(host_port_pair_.host(),
+                                           "192.168.0.1", "");
+  host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
+
+  QuicStreamRequest request(&factory_);
+  EXPECT_EQ(OK, request.Request(host_port_pair_, is_https_, privacy_mode_,
+                                /*cert_verify_flags=*/0, host_port_pair_.host(),
+                                "GET", net_log_, callback_.callback()));
+
+  QuicClientSession* session = QuicStreamFactoryPeer::GetActiveSession(
+      &factory_, host_port_pair_, is_https_);
+
+  DVLOG(1) << "Create 1st session and trigger pulic reset post handshake";

[Ryan Hamilton, 2015/06/30 18:55:32]

The session has already been created here, right? Perhaps "Created 1st session.
Now trigger ..."?

[Buck, 2015/07/01 19:06:20]

Done.

+  session->connection()->CloseConnection(QUIC_PUBLIC_RESET, true);
+  // Need to spin the loop now to ensure that
+  // QuicStreamFactory::OnSessionClosed() runs.
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();

[Ryan Hamilton, 2015/06/30 18:55:32]

Does it work if you do base::RunLoop().RunUntilIdle()?

[Buck, 2015/07/01 19:06:20]

Nope.

[Ryan Hamilton, 2015/07/06 18:25:41]

*head explodes*

+
+  EXPECT_EQ(1, QuicStreamFactoryPeer::GetPublicResetsPostHandshake(&factory_));
+  EXPECT_FALSE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));
+
+  // Test N-in-a-row public reset post handshakes..

[Ryan Hamilton, 2015/06/30 18:55:32]

Does "N-in-a-row" simply mean 2, or is there another test which does Good, Bad,
Good and verifies that QUIC is disabled there too?

[Buck, 2015/07/01 19:06:20]

It did mean two.  I've revised the comment.  I've also added more tests,
specifically cased for two of three (bad, good, bad), and two of four (bad,
good, good, bad).

+  DVLOG(1) << "Create 2nd session and trigger public reset post handshake";
+  TestCompletionCallback callback2;
+  QuicStreamRequest request2(&factory_);
+  EXPECT_EQ(OK, request2.Request(server2, is_https_, privacy_mode_,
+                                 /*cert_verify_flags=*/0, server2.host(), "GET",
+                                 net_log_, callback2.callback()));
+  QuicClientSession* session2 =
+      QuicStreamFactoryPeer::GetActiveSession(&factory_, server2, is_https_);
+
+  session2->connection()->CloseConnection(QUIC_PUBLIC_RESET, true);
+  // Need to spin the loop now to ensure that
+  // QuicStreamFactory::OnSessionClosed() runs.
+  base::RunLoop run_loop2;
+  run_loop2.RunUntilIdle();
+  EXPECT_EQ(2, QuicStreamFactoryPeer::GetPublicResetsPostHandshake(&factory_));
+  EXPECT_TRUE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));

[Ryan Hamilton, 2015/06/30 18:55:32]

Can you also test your disabled reason method here too?

[Buck, 2015/07/01 19:06:20]

Done.

+
+  scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
+  EXPECT_TRUE(stream.get());
+  scoped_ptr<QuicHttpStream> stream2 = request2.ReleaseStream();
+  EXPECT_TRUE(stream2.get());
+  EXPECT_TRUE(socket_data.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data.AllWriteDataConsumed());
+  EXPECT_TRUE(socket_data2.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
+}
+
+TEST_P(QuicStreamFactoryTest, TimeoutsStreamsOpen) {
+  factory_.set_quic_server_info_factory(&quic_server_info_factory_);
+  QuicStreamFactoryPeer::SetTaskRunner(&factory_, runner_.get());
+  QuicStreamFactoryPeer::SetDisableDiskCache(&factory_, true);
+  QuicStreamFactoryPeer::SetThresholdTimeoutsStreamsOpen(&factory_, 2);
+  EXPECT_FALSE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));
+  EXPECT_EQ(0, QuicStreamFactoryPeer::GetNumberOfLossyConnections(
+                   &factory_, host_port_pair_.port()));
+
+  MockRead reads[] = {
+      MockRead(ASYNC, OK, 0)  // EOF
+  };
+  DeterministicSocketData socket_data(reads, arraysize(reads), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data);
+  socket_data.StopAfter(1);
+
+  //  DeterministicSocketData socket_data2(nullptr, 0, nullptr, 0);
+  DeterministicSocketData socket_data2(reads, arraysize(reads), nullptr, 0);
+  socket_factory_.AddSocketDataProvider(&socket_data2);
+  socket_data2.StopAfter(1);
+
+  HostPortPair server2("mail.example.org", kDefaultServerPort);
+
+  crypto_client_stream_factory_.set_handshake_mode(
+      MockCryptoClientStream::CONFIRM_HANDSHAKE);
+  host_resolver_.set_synchronous_mode(true);
+  host_resolver_.rules()->AddIPLiteralRule(host_port_pair_.host(),
+                                           "192.168.0.1", "");
+  host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");
+
+  QuicStreamRequest request(&factory_);
+  EXPECT_EQ(OK, request.Request(host_port_pair_, is_https_, privacy_mode_,
+                                /*cert_verify_flags=*/0, host_port_pair_.host(),
+                                "GET", net_log_, callback_.callback()));
+
+  QuicClientSession* session = QuicStreamFactoryPeer::GetActiveSession(
+      &factory_, host_port_pair_, is_https_);
+
+  scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
+  EXPECT_TRUE(stream.get());
+  HttpRequestInfo request_info;
+  EXPECT_EQ(OK, stream->InitializeStream(&request_info, DEFAULT_PRIORITY,
+                                         net_log_, CompletionCallback()));
+
+  DVLOG(1) << "Create 1st session and trigger pulic reset post handshake";
+  session->connection()->CloseConnection(QUIC_CONNECTION_TIMED_OUT, false);
+  // Need to spin the loop now to ensure that
+  // QuicStreamFactory::OnSessionClosed() runs.
+  base::RunLoop run_loop;
+  run_loop.RunUntilIdle();
+
+  EXPECT_EQ(1, QuicStreamFactoryPeer::GetTimeoutsStreamsOpen(&factory_));
+  EXPECT_FALSE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));
+
+  // Test N-in-a-row public reset post handshakes..

[Ryan Hamilton, 2015/06/30 18:55:32]

s/public reset post handshakes/timeouts with open streams/?

[Buck, 2015/07/01 19:06:20]

Done.

+  DVLOG(1) << "Create 2nd session and trigger public reset post handshake";
+  TestCompletionCallback callback2;
+  QuicStreamRequest request2(&factory_);
+  EXPECT_EQ(OK, request2.Request(server2, is_https_, privacy_mode_,
+                                 /*cert_verify_flags=*/0, server2.host(), "GET",
+                                 net_log_, callback2.callback()));
+  QuicClientSession* session2 =
+      QuicStreamFactoryPeer::GetActiveSession(&factory_, server2, is_https_);
+
+  scoped_ptr<QuicHttpStream> stream2 = request2.ReleaseStream();
+  EXPECT_TRUE(stream2.get());
+  EXPECT_EQ(OK, stream2->InitializeStream(&request_info, DEFAULT_PRIORITY,
+                                          net_log_, CompletionCallback()));
+
+  session2->connection()->CloseConnection(QUIC_CONNECTION_TIMED_OUT, false);
+  // Need to spin the loop now to ensure that
+  // QuicStreamFactory::OnSessionClosed() runs.
+  base::RunLoop run_loop2;
+  run_loop2.RunUntilIdle();
+  EXPECT_EQ(2, QuicStreamFactoryPeer::GetTimeoutsStreamsOpen(&factory_));
+  EXPECT_TRUE(
+      QuicStreamFactoryPeer::IsQuicDisabled(&factory_, host_port_pair_.port()));
+
+  EXPECT_TRUE(socket_data.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data.AllWriteDataConsumed());
+  EXPECT_TRUE(socket_data2.AllReadDataConsumed());
+  EXPECT_TRUE(socket_data2.AllWriteDataConsumed());
+}
+
 }  // namespace test
 }  // namespace net