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

net/quic/quic_stream_factory.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/quic/quic_stream_factory.h"
 
 #include <algorithm>
 #include <set>
 #include <utility>
 
@@ skipping 628 matching lines @@
       close_sessions_on_ip_change_(close_sessions_on_ip_change),
       migrate_sessions_on_network_change_(
           migrate_sessions_on_network_change &&
           NetworkChangeNotifier::AreNetworkHandlesSupported()),
       migrate_sessions_early_(migrate_sessions_early &&
                               migrate_sessions_on_network_change_),
       port_seed_(random_generator_->RandUint64()),
       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)
     threshold_timeouts_with_open_streams_ = 1;
   DCHECK(transport_security_state_);
   DCHECK(http_server_properties_);
   crypto_config_.set_user_agent_id(user_agent_id);
   crypto_config_.AddCanonicalSuffix(".c.youtube.com");
   crypto_config_.AddCanonicalSuffix(".googlevideo.com");
   crypto_config_.AddCanonicalSuffix(".googleusercontent.com");
@@ skipping 332 matching lines @@
       return "Bad packet loss rate.";
     case QuicChromiumClientSession::QUIC_DISABLED_PUBLIC_RESET_POST_HANDSHAKE:
       return "Public resets after successful handshakes.";
     case QuicChromiumClientSession::QUIC_DISABLED_TIMEOUT_WITH_OPEN_STREAMS:
       return "Connection timeouts with streams open.";
     default:
       return "";
   }
 }
 
-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,
                                              float packet_loss_rate) {
   DCHECK(session);
   uint16_t port = session->server_id().port();
   if (packet_loss_rate < packet_loss_threshold_) {
     number_of_lossy_connections_[port] = 0;
     return false;
   }
 
   // We mark it as recently broken, which means that 0-RTT will be disabled
   // but we'll still race.
   http_server_properties_->MarkAlternativeServiceRecentlyBroken(
       AlternativeService(QUIC, session->server_id().host(), port));
 
   bool was_quic_disabled = IsQuicDisabled(port);
   ++number_of_lossy_connections_[port];
 
   // Collect data for port 443 for packet loss events.
   if (port == 443 && max_number_of_lossy_connections_ > 0) {
     UMA_HISTOGRAM_SPARSE_SLOWLY(
         base::StringPrintf("Net.QuicStreamFactory.BadPacketLossEvents%d",
                            max_number_of_lossy_connections_),
         std::min(number_of_lossy_connections_[port],
                  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.
     session->CloseSessionOnErrorAndNotifyFactoryLater(
         ERR_ABORTED, QUIC_BAD_PACKET_LOSS_RATE);
 
     // If this bad packet loss rate disabled the QUIC, then record it.
     if (!was_quic_disabled)
       UMA_HISTOGRAM_SPARSE_SLOWLY("Net.QuicStreamFactory.QuicIsDisabled", port);
   }
   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) {
   const AliasSet& aliases = session_aliases_[session];
   for (AliasSet::const_iterator it = aliases.begin(); it != aliases.end();
        ++it) {
     DCHECK(active_sessions_.count(*it));
     DCHECK_EQ(session, active_sessions_[*it]);
     // Track sessions which have recently gone away so that we can disable
     // port suggestions.
@@ skipping 50 matching lines @@
                                 num_timeouts_with_open_streams_, 0, 20, 10);
   }
 
   if (num_public_resets_post_handshake_ > max_public_resets_post_handshake_) {
     max_public_resets_post_handshake_ = num_public_resets_post_handshake_;
     UMA_HISTOGRAM_CUSTOM_COUNTS(
         "Net.QuicStreamFactory.PublicResetsPostHandshake",
         num_public_resets_post_handshake_, 0, 20, 10);
   }
 
+  MaybeDisableQuic(port);
   if (IsQuicDisabled(port)) {
     if (disabled_reason ==
         QuicChromiumClientSession::QUIC_DISABLED_PUBLIC_RESET_POST_HANDSHAKE) {
       session->CloseSessionOnErrorAndNotifyFactoryLater(
           ERR_ABORTED, QUIC_PUBLIC_RESETS_POST_HANDSHAKE);
     } else if (disabled_reason == QuicChromiumClientSession::
                                       QUIC_DISABLED_TIMEOUT_WITH_OPEN_STREAMS) {
       session->CloseSessionOnErrorAndNotifyFactoryLater(
           ERR_ABORTED, QUIC_TIMEOUTS_WITH_OPEN_STREAMS);
     }
     UMA_HISTOGRAM_ENUMERATION("Net.QuicStreamFactory.DisabledReasons",
                               disabled_reason,
                               QuicChromiumClientSession::QUIC_DISABLED_MAX);
   }
 }
 
+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);
   OnSessionGoingAway(session);
   delete session;
   all_sessions_.erase(session);
 }
 
 void QuicStreamFactory::OnSessionConnectTimeout(
     QuicChromiumClientSession* session) {
@@ skipping 66 matching lines @@
   }
   return std::move(list);
 }
 
 void QuicStreamFactory::ClearCachedStatesInCryptoConfig() {
   crypto_config_.ClearCachedStates();
 }
 
 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) {}
 
 void QuicStreamFactory::OnNetworkDisconnected(NetworkHandle network) {
   MaybeMigrateOrCloseSessions(network, /*force_close=*/true);
   set_require_confirmation(true);
 }
 
 // This method is expected to only be called when migrating from Cellular to
@@ skipping 439 matching lines @@
   // Since the session was active, there's no longer an
   // HttpStreamFactoryImpl::Job running which can mark it broken, unless the TCP
   // job also fails. So to avoid not using QUIC when we otherwise could, we mark
   // it as recently broken, which means that 0-RTT will be disabled but we'll
   // still race.
   http_server_properties_->MarkAlternativeServiceRecentlyBroken(
       alternative_service);
 }
 
 }  // namespace net