Don't send preface-PING. Send trailing ping 200ms

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
===================================================================
--- net/spdy/spdy_session.cc	(revision 105879)
+++ net/spdy/spdy_session.cc	(working copy)
@@ -237,7 +237,7 @@
 int SpdySession::connection_at_risk_of_loss_ms_ = 0;
 
 // static
-int SpdySession::trailing_ping_delay_time_ms_ = 1000;
+int SpdySession::trailing_ping_delay_time_ms_ = 200;

[jar (doing other things), 2011/10/17 22:09:28]

Will argued for keeping this at 1000, and was persuasive.  His argument is that
the common case will be a connect to a site... get top level html... connect to
get subresources... and *then* we'll send the trailing ping.  Hence he wanted to
be well over one or two round trip times... and 1 second works great.

[ramant (doing other things), 2011/10/17 23:40:57]

Done.

 
 // static
 int SpdySession::hung_interval_ms_ = 10000;
@@ -280,7 +280,7 @@
       received_data_time_(base::TimeTicks::Now()),
       trailing_ping_pending_(false),
       check_ping_status_pending_(false),
-      last_sent_was_ping_(false),
+      need_to_send_ping_(false),
       initial_send_window_size_(spdy::kSpdyStreamInitialWindowSize),
       initial_recv_window_size_(spdy::kSpdyStreamInitialWindowSize),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SPDY_SESSION)),
@@ -528,7 +528,7 @@
         make_scoped_refptr(
             new NetLogSpdySynParameter(headers, flags, stream_id, 0)));
   }
-  last_sent_was_ping_ = false;
+  need_to_send_ping_ = true;

[jar (doing other things), 2011/10/17 22:09:28]

Please add comment:
Some servers don't like too many pings, so we limit our current sending to no
more than one ping for any syn sent.  To do this, we avoid ever setting this to
true unless we send a syn (which we have just done).  This approach may change
over time as servers change their responses to
pings.

[ramant (doing other things), 2011/10/17 23:40:57]

Done.

 
   return ERR_IO_PENDING;
 }
@@ -584,7 +584,6 @@
   scoped_ptr<spdy::SpdyDataFrame> frame(
       spdy_framer_.CreateDataFrame(stream_id, data->data(), len, flags));
   QueueFrame(frame.get(), stream->priority(), stream);
-  last_sent_was_ping_ = false;
   return ERR_IO_PENDING;
 }
 
@@ -612,7 +611,6 @@
     priority = stream->priority();
   }
   QueueFrame(rst_frame.get(), priority, NULL);
-  last_sent_was_ping_ = false;
   DeleteStream(stream_id, ERR_SPDY_PROTOCOL_ERROR);
 }
 
@@ -1383,7 +1381,7 @@
   if (pings_in_flight_ > 0)
     return;
 
-  if (last_sent_was_ping_)
+  if (!need_to_send_ping_)
     return;
 
   PlanToSendTrailingPing();
@@ -1446,7 +1444,6 @@
   scoped_ptr<spdy::SpdyWindowUpdateControlFrame> window_update_frame(
       spdy_framer_.CreateWindowUpdate(stream_id, delta_window_size));
   QueueFrame(window_update_frame.get(), stream->priority(), stream);
-  last_sent_was_ping_ = false;
 }
 
 // Given a cwnd that we would have sent to the server, modify it based on the
@@ -1511,7 +1508,6 @@
       spdy_framer_.CreateSettings(settings));
   sent_settings_ = true;
   QueueFrame(settings_frame.get(), 0, NULL);
-  last_sent_was_ping_ = false;
 }
 
 void SpdySession::HandleSettings(const spdy::SpdySettings& settings) {
@@ -1546,7 +1542,8 @@
 }
 
 void SpdySession::SendPrefacePing() {
-  WritePingFrame(next_ping_id_);
+  // TODO(rtenneti): Enable sending Preface-PING after server fix.

[jar (doing other things), 2011/10/17 22:40:05]

Suggest:
TODO(rtenneti): Send preface pings when more servers support additional pings.

[ramant (doing other things), 2011/10/17 23:40:57]

Done.

+  // WritePingFrame(next_ping_id_);
 }
 
 void SpdySession::PlanToSendTrailingPing() {
@@ -1579,7 +1576,7 @@
   if (unique_id % 2 != 0) {
     next_ping_id_ += 2;
     ++pings_in_flight_;
-    last_sent_was_ping_ = true;
+    need_to_send_ping_ = false;
     PlanToCheckPingStatus();
   }
 }