When QUIC is disabled because of timeouts with open streams, re-enable

net/quic/quic_stream_factory.cc

Index: net/quic/quic_stream_factory.cc
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index 039915bf735bc6a3a15bd77e0d0be64dae7e7d47..73e818417a2d81313faa48cc42daea92b8c8ab2f 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -646,6 +646,7 @@ QuicStreamFactory::QuicStreamFactory(
       check_persisted_supports_quic_(true),
       has_initialized_data_(false),
       num_push_streams_created_(0),
+      status_(OPEN),
       task_runner_(nullptr),
       weak_factory_(this) {
   if (disable_quic_on_timeout_with_open_streams)
@@ -998,9 +999,8 @@ const char* QuicStreamFactory::QuicDisabledReasonString() const {
   }
 }
 
-bool QuicStreamFactory::IsQuicDisabled(uint16_t port) {
-  return QuicDisabledReason(port) !=
-         QuicChromiumClientSession::QUIC_DISABLED_NOT;
+bool QuicStreamFactory::IsQuicDisabled(uint16_t port) const {
+  return status_ != OPEN;
 }
 
 bool QuicStreamFactory::OnHandshakeConfirmed(QuicChromiumClientSession* session,
@@ -1029,6 +1029,8 @@ bool QuicStreamFactory::OnHandshakeConfirmed(QuicChromiumClientSession* session,
                  max_number_of_lossy_connections_));
   }
 
+  MaybeDisableQuic(port);
+
   bool is_quic_disabled = IsQuicDisabled(port);
   if (is_quic_disabled) {
     // Close QUIC connection if Quic is disabled for this port.
@@ -1042,6 +1044,22 @@ bool QuicStreamFactory::OnHandshakeConfirmed(QuicChromiumClientSession* session,
   return is_quic_disabled;
 }
 
+void QuicStreamFactory::OnTcpJobCompleted(bool succeeded) {
+  if (status_ != CLOSED)
+    return;
+
+  // If QUIC connections are failing while TCP connections are working,
+  // then stop using QUIC. On the other hand if both QUIC and TCP are
+  // failing, then attempt to use QUIC again.
+  if (succeeded) {
+    status_ = DISABLED;
+    return;
+  }
+
+  status_ = OPEN;
+  num_timeouts_with_open_streams_ = 0;
+}
+
 void QuicStreamFactory::OnIdleSession(QuicChromiumClientSession* session) {}
 
 void QuicStreamFactory::OnSessionGoingAway(QuicChromiumClientSession* session) {
@@ -1112,6 +1130,7 @@ void QuicStreamFactory::MaybeDisableQuic(QuicChromiumClientSession* session) {
         num_public_resets_post_handshake_, 0, 20, 10);
   }
 
+  MaybeDisableQuic(port);
   if (IsQuicDisabled(port)) {
     if (disabled_reason ==
         QuicChromiumClientSession::QUIC_DISABLED_PUBLIC_RESET_POST_HANDSHAKE) {
@@ -1128,6 +1147,33 @@ void QuicStreamFactory::MaybeDisableQuic(QuicChromiumClientSession* session) {
   }
 }
 
+void QuicStreamFactory::MaybeDisableQuic(uint16_t port) {
+  if (status_ == DISABLED)
+    return;
+
+  QuicChromiumClientSession::QuicDisabledReason disabled_reason =
+      QuicDisabledReason(port);
+  if (disabled_reason == QuicChromiumClientSession::QUIC_DISABLED_NOT)

[ramant (doing other things), 2016/03/05 00:55:18]

nit: a small question when "disabled_reason ==
QuicChromiumClientSession::QUIC_DISABLED_NOT", status_ should be OPEN and 
IsQuicDisabled(port) should be false. Wondering if status_ being a  CLOSED is a
valid status?. I am guessing that status_ is CLOSED should never happen. thanks.

IMO, in the worst case, if this were ever to happen, 
Net.QuicStreamFactory.DisabledReasons histogram will have count for 
QUIC_DISABLED_NOT.

[Ryan Hamilton, 2016/03/05 00:59:50]

Yes, I believe status_ must be OPEN there. I've added a DCHECK. Does that sound
workable?

+    return;
+
+  if (disabled_reason ==
+      QuicChromiumClientSession::QUIC_DISABLED_TIMEOUT_WITH_OPEN_STREAMS) {
+    // When QUIC there are too many timeouts with open stream, the factory
+    // should be closed. When TCP jobs complete, they will move the factory
+    // to either fully disabled or back to open

[ramant (doing other things), 2016/03/05 00:55:18]

overly nit. "." at the end of line#1163.

[Ryan Hamilton, 2016/03/05 00:59:50]

Done.

+    status_ = CLOSED;
+    DCHECK(IsQuicDisabled(port));
+    DCHECK_NE(QuicChromiumClientSession::QuicDisabledReason(port),
+              QuicChromiumClientSession::QUIC_DISABLED_NOT);
+    return;
+  }
+
+  status_ = DISABLED;
+  DCHECK(IsQuicDisabled(port));
+  DCHECK_NE(QuicChromiumClientSession::QuicDisabledReason(port),
+            QuicChromiumClientSession::QUIC_DISABLED_NOT);
+}
+
 void QuicStreamFactory::OnSessionClosed(QuicChromiumClientSession* session) {
   DCHECK_EQ(0u, session->GetNumActiveStreams());
   MaybeDisableQuic(session);
@@ -1214,12 +1260,14 @@ void QuicStreamFactory::ClearCachedStatesInCryptoConfig() {
 
 void QuicStreamFactory::OnIPAddressChanged() {
   num_timeouts_with_open_streams_ = 0;
+  status_ = OPEN;
   CloseAllSessions(ERR_NETWORK_CHANGED, QUIC_IP_ADDRESS_CHANGED);
   set_require_confirmation(true);
 }
 
 void QuicStreamFactory::OnNetworkConnected(NetworkHandle network) {
   num_timeouts_with_open_streams_ = 0;
+  status_ = OPEN;
 }
 
 void QuicStreamFactory::OnNetworkMadeDefault(NetworkHandle network) {}