Sent QUIC "PING" frames when a stream is open and the connection

has been idle for the half idle network timeout.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264950

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

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

9 files changed, 155 insertions, 13 deletions

diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index e29bea9..74cc16b 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -75,6 +75,8 @@ class AckAlarm : public QuicAlarm::Delegate {
 
  private:
   QuicConnection* connection_;
+
+  DISALLOW_COPY_AND_ASSIGN(AckAlarm);
 };
 
 // This alarm will be scheduled any time a data-bearing packet is sent out.
@@ -93,6 +95,8 @@ class RetransmissionAlarm : public QuicAlarm::Delegate {
 
  private:
   QuicConnection* connection_;
+
+  DISALLOW_COPY_AND_ASSIGN(RetransmissionAlarm);
 };
 
 // An alarm that is scheduled when the sent scheduler requires a
@@ -111,6 +115,8 @@ class SendAlarm : public QuicAlarm::Delegate {
 
  private:
   QuicConnection* connection_;
+
+  DISALLOW_COPY_AND_ASSIGN(SendAlarm);
 };
 
 class TimeoutAlarm : public QuicAlarm::Delegate {
@@ -127,6 +133,23 @@ class TimeoutAlarm : public QuicAlarm::Delegate {
 
  private:
   QuicConnection* connection_;
+
+  DISALLOW_COPY_AND_ASSIGN(TimeoutAlarm);
+};
+
+class PingAlarm : public QuicAlarm::Delegate {
+ public:
+  explicit PingAlarm(QuicConnection* connection)
+      : connection_(connection) {
+  }
+
+  virtual QuicTime OnAlarm() OVERRIDE {
+    connection_->SendPing();
+    return QuicTime::Zero();
+  }
+
+ private:
+  QuicConnection* connection_;
 };
 
 QuicConnection::PacketType GetPacketType(
@@ -191,6 +214,7 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id,
       send_alarm_(helper->CreateAlarm(new SendAlarm(this))),
       resume_writes_alarm_(helper->CreateAlarm(new SendAlarm(this))),
       timeout_alarm_(helper->CreateAlarm(new TimeoutAlarm(this))),
+      ping_alarm_(helper->CreateAlarm(new PingAlarm(this))),
       debug_visitor_(NULL),
       packet_creator_(connection_id_, &framer_, random_generator_, is_server),
       packet_generator_(this, NULL, &packet_creator_),
@@ -1060,6 +1084,7 @@ void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address,
   MaybeProcessUndecryptablePackets();
   MaybeProcessRevivedPacket();
   MaybeSendInResponseToPacket();
+  SetPingAlarm();
 }
 
 void QuicConnection::OnCanWrite() {
@@ -1409,6 +1434,7 @@ bool QuicConnection::OnPacketSent(WriteResult result) {
   if (transmission_type == NOT_RETRANSMISSION) {
     time_of_last_sent_new_packet_ = now;
   }
+  SetPingAlarm();
   DVLOG(1) << ENDPOINT << "time of last sent packet: "
            << now.ToDebuggingValue();
 
@@ -1484,6 +1510,27 @@ void QuicConnection::UpdateStopWaiting(QuicStopWaitingFrame* stop_waiting) {
       stop_waiting->least_unacked - 1);
 }
 
+void QuicConnection::SendPing() {
+  if (retransmission_alarm_->IsSet()) {
+    return;
+  }
+  if (version() <= QUIC_VERSION_17) {
+    // TODO(rch): remove this when we remove version 17.
+    // This is a horrible hideous hack which we should not support.
+    IOVector data;
+    char c_data[] = "C";
+    data.Append(c_data, 1);
+    QuicConsumedData consumed_data =
+        packet_generator_.ConsumeData(kCryptoStreamId, data, 0, false, NULL);
+    if (consumed_data.bytes_consumed == 0) {
+      DLOG(ERROR) << "Unable to send ping!?";
+    }
+  } else {
+    // TODO(rch): enable this when we merge version 18.
+    // packet_generator_.AddControlFrame(QuicFrame(new QuicPingFrame));
+  }
+}
+
 void QuicConnection::SendAck() {
   ack_alarm_->Cancel();
   stop_waiting_count_ = 0;
@@ -1808,6 +1855,20 @@ bool QuicConnection::CheckForTimeout() {
   return false;
 }
 
+void QuicConnection::SetPingAlarm() {
+  if (is_server_) {
+    // Only clients send pings.
+    return;
+  }
+  ping_alarm_->Cancel();
+  if (!visitor_->HasOpenDataStreams()) {
+    // Don't send a ping unless there are open streams.
+    return;
+  }
+  QuicTime::Delta ping_timeout = QuicTime::Delta::FromSeconds(kPingTimeoutSecs);
+  ping_alarm_->Set(clock_->ApproximateNow().Add(ping_timeout));
+}
+
 QuicConnection::ScopedPacketBundler::ScopedPacketBundler(
     QuicConnection* connection,
     AckBundling send_ack)
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index 6c8a2e5..3a51a49 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -103,6 +103,10 @@ class NET_EXPORT_PRIVATE QuicConnectionVisitorInterface {
 
   // Called to ask if any handshake messages are pending in this visitor.
   virtual bool HasPendingHandshake() const = 0;
+
+  // Called to ask if any streams are open in this visitor, excluding the
+  // reserved crypto and headers stream.
+  virtual bool HasOpenDataStreams() const = 0;
 };
 
 // Interface which gets callbacks from the QuicConnection at interesting
@@ -391,6 +395,9 @@ class NET_EXPORT_PRIVATE QuicConnection
   // true.  Otherwise, it will return false and will reset the timeout alarm.
   bool CheckForTimeout();
 
+  // Sends a ping, and resets the ping alarm.
+  void SendPing();
+
   // Sets up a packet with an QuicAckFrame and sends it out.
   void SendAck();
 
@@ -590,6 +597,9 @@ class NET_EXPORT_PRIVATE QuicConnection
   // Closes any FEC groups protecting packets before |sequence_number|.
   void CloseFecGroupsBefore(QuicPacketSequenceNumber sequence_number);
 
+  // Sets the ping alarm to the appropriate value, if any.
+  void SetPingAlarm();
+
   QuicFramer framer_;
   QuicConnectionHelperInterface* helper_;  // Not owned.
   QuicPacketWriter* writer_;  // Not owned.
@@ -669,6 +679,8 @@ class NET_EXPORT_PRIVATE QuicConnection
   scoped_ptr<QuicAlarm> resume_writes_alarm_;
   // An alarm that fires when the connection may have timed out.
   scoped_ptr<QuicAlarm> timeout_alarm_;
+  // An alarm that fires when a ping should be sent.
+  scoped_ptr<QuicAlarm> ping_alarm_;
 
   QuicConnectionVisitorInterface* visitor_;
   QuicConnectionDebugVisitorInterface* debug_visitor_;
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index 3918939..b37d367 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -520,6 +520,16 @@ class TestConnection : public QuicConnection {
         QuicConnectionPeer::GetAckAlarm(this));
   }
 
+  TestConnectionHelper::TestAlarm* GetPingAlarm() {
+    return reinterpret_cast<TestConnectionHelper::TestAlarm*>(
+        QuicConnectionPeer::GetPingAlarm(this));
+  }
+
+  TestConnectionHelper::TestAlarm* GetResumeWritesAlarm() {
+    return reinterpret_cast<TestConnectionHelper::TestAlarm*>(
+        QuicConnectionPeer::GetResumeWritesAlarm(this));
+  }
+
   TestConnectionHelper::TestAlarm* GetRetransmissionAlarm() {
     return reinterpret_cast<TestConnectionHelper::TestAlarm*>(
         QuicConnectionPeer::GetRetransmissionAlarm(this));
@@ -530,11 +540,6 @@ class TestConnection : public QuicConnection {
         QuicConnectionPeer::GetSendAlarm(this));
   }
 
-  TestConnectionHelper::TestAlarm* GetResumeWritesAlarm() {
-    return reinterpret_cast<TestConnectionHelper::TestAlarm*>(
-        QuicConnectionPeer::GetResumeWritesAlarm(this));
-  }
-
   TestConnectionHelper::TestAlarm* GetTimeoutAlarm() {
     return reinterpret_cast<TestConnectionHelper::TestAlarm*>(
         QuicConnectionPeer::GetTimeoutAlarm(this));
@@ -606,6 +611,7 @@ class QuicConnectionTest : public ::testing::TestWithParam<QuicVersion> {
     EXPECT_CALL(visitor_, HasPendingWrites()).Times(AnyNumber());
     EXPECT_CALL(visitor_, HasPendingHandshake()).Times(AnyNumber());
     EXPECT_CALL(visitor_, OnCanWrite()).Times(AnyNumber());
+    EXPECT_CALL(visitor_, HasOpenDataStreams()).WillRepeatedly(Return(false));
 
     EXPECT_CALL(*loss_algorithm_, GetLossTimeout())
         .WillRepeatedly(Return(QuicTime::Zero()));
@@ -2643,12 +2649,55 @@ TEST_P(QuicConnectionTest, InitialTimeout) {
   EXPECT_FALSE(connection_.connected());
 
   EXPECT_FALSE(connection_.GetAckAlarm()->IsSet());
+  EXPECT_FALSE(connection_.GetPingAlarm()->IsSet());
   EXPECT_FALSE(connection_.GetResumeWritesAlarm()->IsSet());
   EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
   EXPECT_FALSE(connection_.GetSendAlarm()->IsSet());
   EXPECT_FALSE(connection_.GetTimeoutAlarm()->IsSet());
 }
 
+TEST_P(QuicConnectionTest, PingAfterSend) {
+  EXPECT_TRUE(connection_.connected());
+  EXPECT_CALL(visitor_, HasOpenDataStreams()).WillRepeatedly(Return(true));
+  EXPECT_FALSE(connection_.GetPingAlarm()->IsSet());
+
+  // Advance to 5ms, and send a packet to the peer, which will set
+  // the ping alarm.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet());
+  SendStreamDataToPeer(1, "GET /", 0, kFin, NULL);
+  EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+  EXPECT_EQ(clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(15)),
+            connection_.GetPingAlarm()->deadline());
+
+  // Now recevie and ACK of the previous packet, which will move the
+  // ping alarm forward.
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  QuicAckFrame frame = InitAckFrame(1, 0);
+  EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_));
+  EXPECT_CALL(*send_algorithm_, UpdateRtt(_));
+  EXPECT_CALL(*send_algorithm_, OnPacketAcked(1, _));
+  ProcessAckPacket(&frame);
+  EXPECT_TRUE(connection_.GetPingAlarm()->IsSet());
+  EXPECT_EQ(clock_.ApproximateNow().Add(QuicTime::Delta::FromSeconds(15)),
+            connection_.GetPingAlarm()->deadline());
+
+  writer_->Reset();
+  clock_.AdvanceTime(QuicTime::Delta::FromSeconds(15));
+  connection_.GetPingAlarm()->Fire();
+  EXPECT_EQ(1u, writer_->frame_count());
+  ASSERT_EQ(1u, writer_->stream_frames().size());
+  EXPECT_EQ(kCryptoStreamId, writer_->stream_frames()[0].stream_id);
+  EXPECT_EQ(0u, writer_->stream_frames()[0].offset);
+  writer_->Reset();
+
+  EXPECT_CALL(visitor_, HasOpenDataStreams()).WillRepeatedly(Return(false));
+  clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(5));
+  SendAckPacketToPeer();
+
+  EXPECT_FALSE(connection_.GetPingAlarm()->IsSet());
+}
+
 TEST_P(QuicConnectionTest, TimeoutAfterSend) {
   EXPECT_TRUE(connection_.connected());
 
diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h
index c586c1d..dd65209 100644
--- a/net/quic/quic_protocol.h
+++ b/net/quic/quic_protocol.h
@@ -110,6 +110,9 @@ const int64 kDefaultInitialTimeoutSecs = 120;  // 2 mins.
 const int64 kDefaultTimeoutSecs = 60 * 10;  // 10 minutes.
 const int64 kDefaultMaxTimeForCryptoHandshakeSecs = 5;  // 5 secs.
 
+// Default ping timeout.
+const int64 kPingTimeoutSecs = 15;  // 15 secs.
+
 // We define an unsigned 16-bit floating point value, inspired by IEEE floats
 // (http://en.wikipedia.org/wiki/Half_precision_floating-point_format),
 // with 5-bit exponent (bias 1), 11-bit mantissa (effective 12 with hidden
diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc
index d6fae9b..7cc353a 100644
--- a/net/quic/quic_session.cc
+++ b/net/quic/quic_session.cc
@@ -86,6 +86,10 @@ class VisitorShim : public QuicConnectionVisitorInterface {
     return session_->HasPendingHandshake();
   }
 
+  virtual bool HasOpenDataStreams() const OVERRIDE {
+    return session_->HasOpenDataStreams();
+  }
+
  private:
   QuicSession* session_;
 };
@@ -301,6 +305,10 @@ bool QuicSession::HasPendingHandshake() const {
   return has_pending_handshake_;
 }
 
+bool QuicSession::HasOpenDataStreams() const {
+  return GetNumOpenStreams() > 0;
+}
+
 QuicConsumedData QuicSession::WritevData(
     QuicStreamId id,
     const IOVector& data,
diff --git a/net/quic/quic_session.h b/net/quic/quic_session.h
index 76fb274..ca2189a3 100644
--- a/net/quic/quic_session.h
+++ b/net/quic/quic_session.h
@@ -73,6 +73,7 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface {
   virtual void OnCanWrite() OVERRIDE;
   virtual bool HasPendingWrites() const OVERRIDE;
   virtual bool HasPendingHandshake() const OVERRIDE;
+  virtual bool HasOpenDataStreams() const OVERRIDE;
 
   // Called by the headers stream when headers have been received for a stream.
   virtual void OnStreamHeaders(QuicStreamId stream_id,
@@ -175,7 +176,8 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface {
   QuicPacketCreator::Options* options() { return connection()->options(); }
 
   // Returns the number of currently open streams, including those which have
-  // been implicitly created.
+  // been implicitly created, but excluding the reserved headers and crypto
+  // streams.
   virtual size_t GetNumOpenStreams() const;
 
   void MarkWriteBlocked(QuicStreamId id, QuicPriority priority);
diff --git a/net/quic/test_tools/quic_connection_peer.cc b/net/quic/test_tools/quic_connection_peer.cc
index 83b3884..a3ac8da 100644
--- a/net/quic/test_tools/quic_connection_peer.cc
+++ b/net/quic/test_tools/quic_connection_peer.cc
@@ -167,6 +167,17 @@ QuicAlarm* QuicConnectionPeer::GetAckAlarm(QuicConnection* connection) {
 }
 
 // static
+QuicAlarm* QuicConnectionPeer::GetPingAlarm(QuicConnection* connection) {
+  return connection->ping_alarm_.get();
+}
+
+// static
+QuicAlarm* QuicConnectionPeer::GetResumeWritesAlarm(
+    QuicConnection* connection) {
+  return connection->resume_writes_alarm_.get();
+}
+
+// static
 QuicAlarm* QuicConnectionPeer::GetRetransmissionAlarm(
     QuicConnection* connection) {
   return connection->retransmission_alarm_.get();
@@ -178,12 +189,6 @@ QuicAlarm* QuicConnectionPeer::GetSendAlarm(QuicConnection* connection) {
 }
 
 // static
-QuicAlarm* QuicConnectionPeer::GetResumeWritesAlarm(
-    QuicConnection* connection) {
-  return connection->resume_writes_alarm_.get();
-}
-
-// static
 QuicAlarm* QuicConnectionPeer::GetTimeoutAlarm(QuicConnection* connection) {
   return connection->timeout_alarm_.get();
 }
diff --git a/net/quic/test_tools/quic_connection_peer.h b/net/quic/test_tools/quic_connection_peer.h
index dc675a2..8c74c1a 100644
--- a/net/quic/test_tools/quic_connection_peer.h
+++ b/net/quic/test_tools/quic_connection_peer.h
@@ -96,9 +96,10 @@ class QuicConnectionPeer {
   static QuicFecGroup* GetFecGroup(QuicConnection* connection, int fec_group);
 
   static QuicAlarm* GetAckAlarm(QuicConnection* connection);
+  static QuicAlarm* GetPingAlarm(QuicConnection* connection);
+  static QuicAlarm* GetResumeWritesAlarm(QuicConnection* connection);
   static QuicAlarm* GetRetransmissionAlarm(QuicConnection* connection);
   static QuicAlarm* GetSendAlarm(QuicConnection* connection);
-  static QuicAlarm* GetResumeWritesAlarm(QuicConnection* connection);
   static QuicAlarm* GetTimeoutAlarm(QuicConnection* connection);
 
   static QuicPacketWriter* GetWriter(QuicConnection* connection);
diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h
index 7c5be78..19e7d80 100644
--- a/net/quic/test_tools/quic_test_utils.h
+++ b/net/quic/test_tools/quic_test_utils.h
@@ -193,6 +193,7 @@ class MockConnectionVisitor : public QuicConnectionVisitorInterface {
   MOCK_METHOD0(OnCanWrite, void());
   MOCK_CONST_METHOD0(HasPendingWrites, bool());
   MOCK_CONST_METHOD0(HasPendingHandshake, bool());
+  MOCK_CONST_METHOD0(HasOpenDataStreams, bool());
   MOCK_METHOD1(OnSuccessfulVersionNegotiation,
                void(const QuicVersion& version));
   MOCK_METHOD0(OnConfigNegotiated, void());