Send PING to check the status of the SPDY connection.

net/spdy/spdy_session.h

Index: net/spdy/spdy_session.h
===================================================================
--- net/spdy/spdy_session.h	(revision 105385)
+++ net/spdy/spdy_session.h	(working copy)
@@ -104,7 +104,11 @@
   // NOTE:  This function can have false negatives on some platforms.
   bool VerifyDomainAuthentication(const std::string& domain);
 
-  // Send the SYN frame for |stream_id|.
+  // Send the PING (pre-PING and post-PING) frames.
+  void SendPing();

[jar (doing other things), 2011/10/14 19:59:07]

In cases where you ONLY use methods for tests, please make these public methods,
which should not be used in chrome, private.

It is important that we don't lure users (in chrome) into calling these methods.
 Friends are bad when used in regular code, but not for test code, that needs to
indecently twiddle interior state etc.

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+
+  // Send the SYN frame for |stream_id|. This also sends PING message to check
+  // the status of the connection.
   int WriteSynStream(
       spdy::SpdyStreamId stream_id,
       RequestPriority priority,
@@ -156,6 +160,14 @@
     return max_concurrent_stream_limit_;
   }
 
+  // Enable sending of PING frame with each request.
+  static void set_send_ping_for_every_request(bool enable) {
+    send_ping_for_every_request_ = enable;
+  }
+  static bool send_ping_for_every_request() {
+    return send_ping_for_every_request_;
+  }
+
   // The initial max concurrent streams per session, can be overridden by the
   // server via SETTINGS.
   static void set_init_max_concurrent_streams(size_t value) {
@@ -215,6 +227,40 @@
   int GetPeerAddress(AddressList* address) const;
   int GetLocalAddress(IPEndPoint* address) const;
 
+  // --------------------------
+  // Helper methods for testing
+  // --------------------------
+  static void set_post_ping_delay_time_ms(int duration) {
+    post_ping_delay_time_ms_ = duration;
+  }
+  static int post_ping_delay_time_ms() {
+    return post_ping_delay_time_ms_;
+  }
+
+  static void set_check_status_delay_time_ms(int duration) {
+    check_status_delay_time_ms_ = duration;
+  }
+  static int check_status_delay_time_ms() {
+    return check_status_delay_time_ms_;
+  }
+
+  static void set_hung_interval_ms(int duration) {
+    hung_interval_ms_ = duration;
+  }
+  static int hung_interval_ms() {
+    return hung_interval_ms_;
+  }
+
+  int64 pings_in_flight() const { return pings_in_flight_; }
+
+  uint32 unique_id_counter() const { return unique_id_counter_; }
+
+  base::TimeTicks received_data_time() const { return received_data_time_; }
+
+  bool post_ping_pending() const { return post_ping_pending_; }
+
+  bool check_status_pending() const { return check_status_pending_; }
+
  private:
   friend class base::RefCounted<SpdySession>;
   FRIEND_TEST_ALL_PREFIXES(SpdySessionTest, GetActivePushStream);
@@ -277,6 +323,7 @@
                  const linked_ptr<spdy::SpdyHeaderBlock>& headers);
   void OnRst(const spdy::SpdyRstStreamControlFrame& frame);
   void OnGoAway(const spdy::SpdyGoAwayControlFrame& frame);
+  void OnPing(const spdy::SpdyPingControlFrame& frame);
   void OnSettings(const spdy::SpdySettingsControlFrame& frame);
   void OnWindowUpdate(const spdy::SpdyWindowUpdateControlFrame& frame);
 
@@ -291,6 +338,28 @@
   // SETTINGS ontrol frame, update our SpdySession accordingly.
   void HandleSettings(const spdy::SpdySettings& settings);
 
+  // Send PING if there are no PINGs in flight and we haven't heard from server.
+  void SendPrePing();
+
+  // Send a delayed PING if there is no |post_ping_pending_|. This PING
+  // verfies that the requests are being received by the server.
+  void SendPostPing();

[jar (doing other things), 2011/10/14 19:59:07]

You talked about replacing the word "Post" This could mean this would be
perchance:
SendTrailingPing



If you like that picture, we could change SendPrePing to:
SendPrefacePing

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+
+  // Send a PING after delay. Don't post a PING if there is already
+  // a PING post pending.
+  void PlanToSendPostPing();
+
+  // Post a CheckStatus call after delay. Don't post if there is already
+  // CheckStatus running.
+  void PlanToCheckStatus();

[jar (doing other things), 2011/10/14 19:59:07]

When talking, you suggested adding the word "Ping" to this name, such as:
PlanToCheckPingStatus

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+
+  // Send the PING frame.
+  void WritePingFrame();
+
+  // Check the status of the connection. It calls |CloseSessionOnError| if we
+  // haven't received any data in |kHungInterval| time period.
+  void CheckStatus(base::TimeTicks last_check_time);
+
   // Start reading from the socket.
   // Returns OK on success, or an error on failure.
   net::Error ReadSocket();
@@ -437,6 +506,26 @@
                             // frame.
   int stalled_streams_;     // Count of streams that were ever stalled.
 
+  // This list keeps track of all pings that are in flight.

[jar (doing other things), 2011/10/14 19:59:07]

nit: Suggest changing comment to:

Count of all pings on the wire, for which we have not gotten a response.

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+  int64 pings_in_flight_;
+
+  // This is the next unique_id to be sent in PING frame. unique_ids are odd
+  // numbers.
+  uint32  unique_id_counter_;

[jar (doing other things), 2011/10/14 19:59:07]

suggest: next_ping_id_;

(and adjust comment)

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+
+  // This is the last time we have received data.
+  base::TimeTicks received_data_time_;
+
+  // Flag if we have a pending post-PING.

[jar (doing other things), 2011/10/14 19:59:07]

Indicate if we have already scheduled a delayed task to send a trailing ping
(and we never have more than one scheduled at a time).

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+  bool post_ping_pending_;
+
+  // Flag if we have a check status pending.

[jar (doing other things), 2011/10/14 19:59:07]

Indicate if we have already scheduled a delayed task to check the ping status.

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+  bool check_status_pending_;
+
+  // Flag to indicate what was the last data sent (if it was PING or if we had
+  // sent any other data).

[jar (doing other things), 2011/10/14 19:59:07]

Indicate if the last data we sent was a ping (generally, a follower ping).  This
helps us decided if we need yet another follower ping, or if it would be a waste
of effort (and MUST not be done).

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+  bool last_sent_was_ping_;
+
   // Initial send window size for the session; can be changed by an
   // arriving SETTINGS frame; newly created streams use this value for the
   // initial send window size.
@@ -457,6 +546,11 @@
   static bool use_flow_control_;
   static size_t init_max_concurrent_streams_;
   static size_t max_concurrent_stream_limit_;
+  static bool send_ping_for_every_request_;

[jar (doing other things), 2011/10/14 19:59:07]

That name is no longer valid (since we don't send one per request).  You use it
to enable and disable this whole connection health checking system.  Perhaps
better would be:

enable_ping_based_connection_checking_

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+  static int post_ping_delay_time_ms_;
+  static int check_status_delay_time_ms_;
+  static int hung_interval_ms_;

[jar (doing other things), 2011/10/14 19:59:07]

Use only one var for line 551 and 552.

Suggest using the "hung_interval_ms_".

A comment could be:
The amount of time (in milliseconds) that we are willing to tolerate with no
data received (of any form), while there is a ping in flight, before we declare
the connection to be hung.

[ramant (doing other things), 2011/10/14 23:42:45]

Done.

+
 };
 
 class NetLogSpdySynParameter : public NetLog::EventParameters {