Disable QUIC for 5 miuntes, subject to exponential backoff, when

Disable QUIC for 5 miuntes, subject to exponential backoff, when
sessions timeout with open streams.

Committed: https://crrev.com/f10ad270e18d0b8c11110ba9a314a58f7ea544f0
Cr-Commit-Position: refs/heads/master@{#418682}

[Ryan Hamilton, 2016-09-12 19:18:04]

rch@chromium.org changed reviewers:
+ ianswett@chromium.org

[Ryan Hamilton, 2016-09-12 19:18:05]

I hope (believe! :>) this is what we discussed...

[ianswett, 2016-09-14 20:10:13]

The CQ bit was checked by ianswett@chromium.org

[ianswett, 2016-09-14 20:10:14]

lgtm

A few nits, but this looks great.  It's surprisingly simple as well. Nicely
done!

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.cc (right):

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.cc:1218: task_runner_->PostDelayedTask(
Can you add a one line comment on what this does?  It looks like it re-opens the
QuicStreamFactory after a delay once it's been disabled?

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.cc:1612: task_runner_->PostDelayedTask(
Small comment please.

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory.h (right):

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory.h:560: bool
need_to_evaluate_consecutive_disabled_count_;
Also, maybe just evaluate_consecutive_disabled_count_, or
use_consecutive_disabled_count_ would be easier to read?

After reading the code, I think this could be named better, but I don't have an
easy suggestion.  Conceptually, I'd consider the 5 minute timer time a
'probation' period of sorts where we're trying to figure out if QUIC works. 
arm_probation_timer_ is a bit confusing as well.

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
File net/quic/chromium/quic_stream_factory_test.cc (right):

https://codereview.chromium.org/2322423002/diff/40001/net/quic/chromium/quic_...
net/quic/chromium/quic_stream_factory_test.cc:3345:
crypto_client_stream_factory_.AddProofVerifyDetails(&verify_details);
Any reason you're adding the ProofVerifyDetails twice, both here and below?

[commit-bot: I haz the power, 2016-09-14 20:10:54]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 21:34:26]

Description was changed from

==========
Disable QUIC for 5 miuntes, subject to exponential backoff, when
sessions timeout with open streams.
==========

to

==========
Disable QUIC for 5 miuntes, subject to exponential backoff, when
sessions timeout with open streams.

Committed: https://crrev.com/f10ad270e18d0b8c11110ba9a314a58f7ea544f0
Cr-Commit-Position: refs/heads/master@{#418682}
==========

[commit-bot: I haz the power, 2016-09-14 21:34:27]

Patchset 3 (id:??) landed as
https://crrev.com/f10ad270e18d0b8c11110ba9a314a58f7ea544f0
Cr-Commit-Position: refs/heads/master@{#418682}

[commit-bot: I haz the power, 2016-09-14 21:35:05]

Failed to apply patch for net/quic/chromium/quic_stream_factory.cc:
While running git apply --index -3 -p1;
  error: patch failed: net/quic/chromium/quic_stream_factory.cc:90
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: net/quic/chromium/quic_stream_factory.cc: patch does not apply

Patch:       net/quic/chromium/quic_stream_factory.cc
Index: net/quic/chromium/quic_stream_factory.cc
diff --git a/net/quic/chromium/quic_stream_factory.cc
b/net/quic/chromium/quic_stream_factory.cc
index
8212a9c0366cecc3cf5d4a411710c79a23eee8df..b645b65c0be2d963ae446f03cda332f1260c00fb
100644
--- a/net/quic/chromium/quic_stream_factory.cc
+++ b/net/quic/chromium/quic_stream_factory.cc
@@ -90,6 +90,9 @@ const int32_t kQuicStreamMaxRecvWindowSize = 6 * 1024 * 1024; 
  // 6 MB
 // Set the maximum number of undecryptable packets the connection will store.
 const int32_t kMaxUndecryptablePackets = 100;
 
+// How long QUIC will be disabled for because of timeouts with open streams.
+const int kDisableQuicTimeoutSecs = 5 * 60;
+
 std::unique_ptr<base::Value> NetLogQuicConnectionMigrationTriggerCallback(
     std::string trigger,
     NetLogCaptureMode capture_mode) {
@@ -774,6 +777,8 @@ QuicStreamFactory::QuicStreamFactory(
       prefer_aes_(prefer_aes),
       disable_quic_on_timeout_with_open_streams_(
           disable_quic_on_timeout_with_open_streams),
+      consecutive_disabled_count_(0),
+      need_to_evaluate_consecutive_disabled_count_(false),
       socket_receive_buffer_size_(socket_receive_buffer_size),
       delay_tcp_race_(delay_tcp_race),
       ping_timeout_(QuicTime::Delta::FromSeconds(kPingTimeoutSecs)),
@@ -1208,8 +1213,18 @@ void QuicStreamFactory::OnTimeoutWithOpenStreams() {
   if (ping_timeout_ > reduced_ping_timeout_) {
     ping_timeout_ = reduced_ping_timeout_;
   }
-  if (disable_quic_on_timeout_with_open_streams_)
+  if (disable_quic_on_timeout_with_open_streams_) {
+    if (status_ == OPEN) {
+      task_runner_->PostDelayedTask(
+          FROM_HERE, base::Bind(&QuicStreamFactory::OpenFactory,
+                                weak_factory_.GetWeakPtr()),
+          base::TimeDelta::FromSeconds(kDisableQuicTimeoutSecs *
+                                       (1 << consecutive_disabled_count_)));
+      consecutive_disabled_count_++;
+      need_to_evaluate_consecutive_disabled_count_ = true;
+    }
     status_ = CLOSED;
+  }
 }
 
 void QuicStreamFactory::CancelRequest(QuicStreamRequest* request) {
@@ -1593,6 +1608,15 @@ int QuicStreamFactory::CreateSession(
     base::TimeTicks dns_resolution_end_time,
     const BoundNetLog& net_log,
     QuicChromiumClientSession** session) {
+  if (need_to_evaluate_consecutive_disabled_count_) {
+    task_runner_->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&QuicStreamFactory::MaybeClearConsecutiveDisabledCount,
+                   weak_factory_.GetWeakPtr()),
+        base::TimeDelta::FromSeconds(kDisableQuicTimeoutSecs));
+
+    need_to_evaluate_consecutive_disabled_count_ = false;
+  }
   TRACE_EVENT0("net", "QuicStreamFactory::CreateSession");
   IPEndPoint addr = *address_list.begin();
   bool enable_port_selection = enable_port_selection_;
@@ -1885,4 +1909,13 @@ void QuicStreamFactory::ProcessGoingAwaySession(
       alternative_service);
 }
 
+void QuicStreamFactory::OpenFactory() {
+  status_ = OPEN;
+}
+
+void QuicStreamFactory::MaybeClearConsecutiveDisabledCount() {
+  if (status_ == OPEN)
+    consecutive_disabled_count_ = 0;
+}
+
 }  // namespace net