To mitigate the effects of hanging 0-RTT QUIC connections,

set up a timer to cancel any requests, if the handshake takes too long.
The requests will be retried and will use either QUIC or TCP, whichever
connects first.

Review URL: https://codereview.chromium.org/318993004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275552 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 158 insertions, 7 deletions

diff --git a/net/quic/quic_client_session.cc b/net/quic/quic_client_session.cc
index 00b1411..a218dcd 100644
--- a/net/quic/quic_client_session.cc
+++ b/net/quic/quic_client_session.cc
@@ -28,6 +28,10 @@ namespace net {
 
 namespace {
 
+// The length of time to wait for a 0-RTT handshake to complete
+// before allowing the requests to possibly proceed over TCP.
+const int k0RttHandshakeTimeoutMs = 300;
+
 // Histograms for tracking down the crashes from http://crbug.com/354669
 // Note: these values must be kept in sync with the corresponding values in:
 // tools/metrics/histograms/histograms.xml
@@ -138,6 +142,7 @@ QuicClientSession::QuicClientSession(
     const QuicConfig& config,
     uint32 max_flow_control_receive_window_bytes,
     QuicCryptoClientConfig* crypto_config,
+    base::TaskRunner* task_runner,
     NetLog* net_log)
     : QuicClientSessionBase(connection,
                             max_flow_control_receive_window_bytes,
@@ -150,6 +155,7 @@ QuicClientSession::QuicClientSession(
       server_info_(server_info.Pass()),
       read_pending_(false),
       num_total_streams_(0),
+      task_runner_(task_runner),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_QUIC_SESSION)),
       logger_(net_log_),
       num_packets_read_(0),
@@ -436,16 +442,39 @@ int QuicClientSession::CryptoConnect(bool require_confirmation,
     return ERR_CONNECTION_FAILED;
   }
 
-  bool can_notify = require_confirmation_ ?
-      IsCryptoHandshakeConfirmed() : IsEncryptionEstablished();
-  if (can_notify) {
+  if (IsCryptoHandshakeConfirmed())
+    return OK;
+
+  // Unless we require handshake confirmation, activate the session if
+  // we have established initial encryption.
+  if (!require_confirmation_ && IsEncryptionEstablished()) {
+    // To mitigate the effects of hanging 0-RTT connections, set up a timer to
+    // cancel any requests, if the handshake takes too long.
+    task_runner_->PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&QuicClientSession::OnConnectTimeout,
+                   weak_factory_.GetWeakPtr()),
+        base::TimeDelta::FromMilliseconds(k0RttHandshakeTimeoutMs));
     return OK;
+
   }
 
   callback_ = callback;
   return ERR_IO_PENDING;
 }
 
+int QuicClientSession::ResumeCryptoConnect(const CompletionCallback& callback) {
+
+  if (IsCryptoHandshakeConfirmed())
+    return OK;
+
+  if (!connection()->connected())
+    return ERR_QUIC_HANDSHAKE_FAILED;
+
+  callback_ = callback;
+  return ERR_IO_PENDING;
+}
+
 int QuicClientSession::GetNumSentClientHellos() const {
   return crypto_stream_->num_sent_client_hellos();
 }
@@ -800,4 +829,16 @@ void QuicClientSession::NotifyFactoryOfSessionClosed() {
     stream_factory_->OnSessionClosed(this);
 }
 
+void QuicClientSession::OnConnectTimeout() {
+  DCHECK(callback_.is_null());
+  DCHECK(IsEncryptionEstablished());
+
+  if (IsCryptoHandshakeConfirmed())
+    return;
+
+  if (stream_factory_)
+    stream_factory_->OnSessionConnectTimeout(this);
+  CloseAllStreams(ERR_QUIC_HANDSHAKE_FAILED);
+}
+
 }  // namespace net
diff --git a/net/quic/quic_client_session.h b/net/quic/quic_client_session.h
index ca4d7b810..f12003e 100644
--- a/net/quic/quic_client_session.h
+++ b/net/quic/quic_client_session.h
@@ -99,6 +99,7 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase {
                     const QuicConfig& config,
                     uint32 max_flow_control_receive_window_bytes,
                     QuicCryptoClientConfig* crypto_config,
+                    base::TaskRunner* task_runner,
                     NetLog* net_log);
 
   virtual ~QuicClientSession();
@@ -150,6 +151,9 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase {
   int CryptoConnect(bool require_confirmation,
                     const CompletionCallback& callback);
 
+  // Resumes a crypto handshake with the server after a timeout.
+  int ResumeCryptoConnect(const CompletionCallback& callback);
+
   // Causes the QuicConnectionHelper to start reading from the socket
   // and passing the data along to the QuicConnection.
   void StartReading();
@@ -214,6 +218,8 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase {
   // delete |this|.
   void NotifyFactoryOfSessionClosed();
 
+  void OnConnectTimeout();
+
   bool require_confirmation_;
   scoped_ptr<QuicCryptoClientStream> crypto_stream_;
   QuicStreamFactory* stream_factory_;
@@ -227,6 +233,7 @@ class NET_EXPORT_PRIVATE QuicClientSession : public QuicClientSessionBase {
   bool read_pending_;
   CompletionCallback callback_;
   size_t num_total_streams_;
+  base::TaskRunner* task_runner_;
   BoundNetLog net_log_;
   QuicConnectionLogger logger_;
   // Number of packets read in the current read loop.
diff --git a/net/quic/quic_client_session_test.cc b/net/quic/quic_client_session_test.cc
index 2ca8bc9..8706d7c 100644
--- a/net/quic/quic_client_session_test.cc
+++ b/net/quic/quic_client_session_test.cc
@@ -73,7 +73,9 @@ class QuicClientSessionTest : public ::testing::TestWithParam<QuicVersion> {
                  QuicServerId(kServerHostname, kServerPort, false,
                               PRIVACY_MODE_DISABLED),
                  DefaultQuicConfig(), kInitialFlowControlWindowForTest,
-                 &crypto_config_, &net_log_) {
+                 &crypto_config_,
+                 base::MessageLoop::current()->message_loop_proxy().get(),
+                 &net_log_) {
     session_.config()->SetDefaults();
     crypto_config_.SetDefaults();
   }
diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc
index 9fc78d9..66261d4 100644
--- a/net/quic/quic_http_stream_test.cc
+++ b/net/quic/quic_http_stream_test.cc
@@ -214,6 +214,8 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> {
                                            false, PRIVACY_MODE_DISABLED),
                               DefaultQuicConfig(),
                               kInitialFlowControlWindowForTest, &crypto_config_,
+                              base::MessageLoop::current()->
+                                  message_loop_proxy().get(),
                               NULL));
     session_->GetCryptoStream()->CryptoConnect();
     EXPECT_TRUE(session_->IsCryptoHandshakeConfirmed());
diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc
index 14037ef..af6fb7b 100644
--- a/net/quic/quic_network_transaction_unittest.cc
+++ b/net/quic/quic_network_transaction_unittest.cc
@@ -860,6 +860,34 @@ TEST_P(QuicNetworkTransactionTest, FailedZeroRttBrokenAlternateProtocol) {
   EXPECT_TRUE(quic_data.at_write_eof());
 }
 
+TEST_P(QuicNetworkTransactionTest, HangingZeroRttFallback) {
+  // Alternate-protocol job
+  MockRead quic_reads[] = {
+    MockRead(ASYNC, ERR_IO_PENDING),
+  };
+  StaticSocketDataProvider quic_data(quic_reads, arraysize(quic_reads),
+                                     NULL, 0);
+  socket_factory_.AddSocketDataProvider(&quic_data);
+
+  // Main job that will proceed when the QUIC job fails.
+  MockRead http_reads[] = {
+    MockRead("HTTP/1.1 200 OK\r\n\r\n"),
+    MockRead("hello from http"),
+    MockRead(SYNCHRONOUS, ERR_TEST_PEER_CLOSE_AFTER_NEXT_MOCK_READ),
+    MockRead(ASYNC, OK)
+  };
+
+  StaticSocketDataProvider http_data(http_reads, arraysize(http_reads),
+                                     NULL, 0);
+  socket_factory_.AddSocketDataProvider(&http_data);
+
+  CreateSessionWithNextProtos();
+
+  AddQuicAlternateProtocolMapping(MockCryptoClientStream::ZERO_RTT);
+
+  SendRequestAndExpectHttpResponse("hello from http");
+}
+
 TEST_P(QuicNetworkTransactionTest, BrokenAlternateProtocolOnConnectFailure) {
   // Alternate-protocol job will fail before creating a QUIC session.
   StaticSocketDataProvider quic_data(NULL, 0, NULL, 0);
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index 3f1a7c8..52c7168 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -120,6 +120,13 @@ class QuicStreamFactory::Job {
       QuicServerInfo* server_info,
       const BoundNetLog& net_log);
 
+  // Creates a new job to handle the resumption of for connecting an
+  // existing session.
+  Job(QuicStreamFactory* factory,
+      HostResolver* host_resolver,
+      QuicClientSession* session,
+      QuicServerId server_id);
+
   ~Job();
 
   int Run(const CompletionCallback& callback);
@@ -130,6 +137,7 @@ class QuicStreamFactory::Job {
   int DoLoadServerInfo();
   int DoLoadServerInfoComplete(int rv);
   int DoConnect();
+  int DoResumeConnect();
   int DoConnectComplete(int rv);
 
   void OnIOComplete(int rv);
@@ -150,6 +158,7 @@ class QuicStreamFactory::Job {
     STATE_LOAD_SERVER_INFO,
     STATE_LOAD_SERVER_INFO_COMPLETE,
     STATE_CONNECT,
+    STATE_RESUME_CONNECT,
     STATE_CONNECT_COMPLETE,
   };
   IoState io_state_;
@@ -178,7 +187,8 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
                             base::StringPiece method,
                             QuicServerInfo* server_info,
                             const BoundNetLog& net_log)
-    : factory_(factory),
+    : io_state_(STATE_RESOLVE_HOST),
+      factory_(factory),
       host_resolver_(host_resolver),
       server_id_(host_port_pair, is_https, privacy_mode),
       is_post_(method == "POST"),
@@ -189,11 +199,24 @@ QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
       session_(NULL),
       weak_factory_(this) {}
 
+QuicStreamFactory::Job::Job(QuicStreamFactory* factory,
+                            HostResolver* host_resolver,
+                            QuicClientSession* session,
+                            QuicServerId server_id)
+    : io_state_(STATE_RESUME_CONNECT),
+      factory_(factory),
+      host_resolver_(host_resolver),  // unused
+      server_id_(server_id),
+      is_post_(false),  // unused
+      was_alternate_protocol_recently_broken_(false),  // unused
+      net_log_(session->net_log()),  // unused
+      session_(session),
+      weak_factory_(this) {}
+
 QuicStreamFactory::Job::~Job() {
 }
 
 int QuicStreamFactory::Job::Run(const CompletionCallback& callback) {
-  io_state_ = STATE_RESOLVE_HOST;
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     callback_ = callback;
@@ -224,6 +247,10 @@ int QuicStreamFactory::Job::DoLoop(int rv) {
         CHECK_EQ(OK, rv);
         rv = DoConnect();
         break;
+      case STATE_RESUME_CONNECT:
+        CHECK_EQ(OK, rv);
+        rv = DoResumeConnect();
+        break;
       case STATE_CONNECT_COMPLETE:
         rv = DoConnectComplete(rv);
         break;
@@ -326,6 +353,16 @@ int QuicStreamFactory::Job::DoConnect() {
   return rv;
 }
 
+int QuicStreamFactory::Job::DoResumeConnect() {
+  io_state_ = STATE_CONNECT_COMPLETE;
+
+  int rv = session_->ResumeCryptoConnect(
+      base::Bind(&QuicStreamFactory::Job::OnIOComplete,
+                 base::Unretained(this)));
+
+  return rv;
+}
+
 int QuicStreamFactory::Job::DoConnectComplete(int rv) {
   if (rv != OK)
     return rv;
@@ -605,6 +642,35 @@ void QuicStreamFactory::OnSessionClosed(QuicClientSession* session) {
   all_sessions_.erase(session);
 }
 
+void QuicStreamFactory::OnSessionConnectTimeout(
+    QuicClientSession* 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]);
+    active_sessions_.erase(*it);
+  }
+
+  if (aliases.empty()) {
+    return;
+  }
+
+  const IpAliasKey ip_alias_key(session->connection()->peer_address(),
+                                aliases.begin()->is_https());
+  ip_aliases_[ip_alias_key].erase(session);
+  if (ip_aliases_[ip_alias_key].empty()) {
+    ip_aliases_.erase(ip_alias_key);
+  }
+  QuicServerId server_id = *aliases.begin();
+  session_aliases_.erase(session);
+  Job* job = new Job(this, host_resolver_, session, server_id);
+  active_jobs_[server_id] = job;
+  int rv = job->Run(base::Bind(&QuicStreamFactory::OnJobComplete,
+                               base::Unretained(this), job));
+  DCHECK_EQ(ERR_IO_PENDING, rv);
+}
+
 void QuicStreamFactory::CancelRequest(QuicStreamRequest* request) {
   DCHECK(ContainsKey(active_requests_, request));
   Job* job = active_requests_[request];
@@ -771,7 +837,9 @@ int QuicStreamFactory::CreateSession(
   *session = new QuicClientSession(
       connection, socket.Pass(), writer.Pass(), this,
       quic_crypto_client_stream_factory_, server_info.Pass(), server_id,
-      config, kInitialReceiveWindowSize, &crypto_config_, net_log.net_log());
+      config, kInitialReceiveWindowSize, &crypto_config_,
+      base::MessageLoop::current()->message_loop_proxy().get(),
+      net_log.net_log());
   all_sessions_[*session] = server_id;  // owning pointer
   return OK;
 }
diff --git a/net/quic/quic_stream_factory.h b/net/quic/quic_stream_factory.h
index 03888fb..cc79285 100644
--- a/net/quic/quic_stream_factory.h
+++ b/net/quic/quic_stream_factory.h
@@ -126,6 +126,9 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
   // Called by a session after it shuts down.
   void OnSessionClosed(QuicClientSession* session);
 
+  // Called by a session whose connection has timed out.
+  void OnSessionConnectTimeout(QuicClientSession* session);
+
   // Cancels a pending request.
   void CancelRequest(QuicStreamRequest* request);