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

net/spdy/spdy_session.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_session.h"
 
 #include "base/basictypes.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/message_loop.h"
@@ skipping 219 matching lines @@
 // static
 size_t SpdySession::max_concurrent_stream_limit_ = 256;
 
 // static
 bool SpdySession::enable_ping_based_connection_checking_ = true;
 
 // static
 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;
 
 SpdySession::SpdySession(const HostPortProxyPair& host_port_proxy_pair,
                          SpdySessionPool* spdy_session_pool,
                          SpdySettingsStorage* spdy_settings,
                          bool verify_domain_authentication,
                          NetLog* net_log)
     : ALLOW_THIS_IN_INITIALIZER_LIST(
@@ skipping 22 matching lines @@
       frames_received_(0),
       bytes_received_(0),
       sent_settings_(false),
       received_settings_(false),
       stalled_streams_(0),
       pings_in_flight_(0),
       next_ping_id_(1),
       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)),
       verify_domain_authentication_(verify_domain_authentication) {
   DCHECK(HttpStreamFactory::spdy_enabled());
   net_log_.BeginEvent(
       NetLog::TYPE_SPDY_SESSION,
       make_scoped_refptr(
           new NetLogSpdySessionParameter(host_port_proxy_pair_)));
 
@@ skipping 227 matching lines @@
   base::StatsCounter spdy_requests("spdy.requests");
   spdy_requests.Increment();
   streams_initiated_count_++;
 
   if (net_log().IsLoggingAllEvents()) {
     net_log().AddEvent(
         NetLog::TYPE_SPDY_SESSION_SYN_STREAM,
         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;
 }
 
 int SpdySession::WriteStreamData(spdy::SpdyStreamId stream_id,
                                  net::IOBuffer* data, int len,
                                  spdy::SpdyDataFlags flags) {
   // Find our stream
   DCHECK(IsStreamActive(stream_id));
   scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
@@ skipping 35 matching lines @@
   if (net_log().IsLoggingAllEvents()) {
     net_log().AddEvent(
         NetLog::TYPE_SPDY_SESSION_SEND_DATA,
         make_scoped_refptr(new NetLogSpdyDataParameter(stream_id, len, flags)));
   }
 
   // TODO(mbelshe): reduce memory copies here.
   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;
 }
 
 void SpdySession::CloseStream(spdy::SpdyStreamId stream_id, int status) {
   // TODO(mbelshe): We should send a RST_STREAM control frame here
   //                so that the server can cancel a large send.
 
   DeleteStream(stream_id, status);
 }
 
 void SpdySession::ResetStream(
     spdy::SpdyStreamId stream_id, spdy::SpdyStatusCodes status) {
 
   net_log().AddEvent(
       NetLog::TYPE_SPDY_SESSION_SEND_RST_STREAM,
       make_scoped_refptr(new NetLogSpdyRstParameter(stream_id, status)));
 
   scoped_ptr<spdy::SpdyRstStreamControlFrame> rst_frame(
       spdy_framer_.CreateRstStream(stream_id, status));
 
   // Default to lowest priority unless we know otherwise.
   int priority = 3;
   if(IsStreamActive(stream_id)) {
     scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
     priority = stream->priority();
   }
   QueueFrame(rst_frame.get(), priority, NULL);
-  last_sent_was_ping_ = false;
   DeleteStream(stream_id, ERR_SPDY_PROTOCOL_ERROR);
 }
 
 bool SpdySession::IsStreamActive(spdy::SpdyStreamId stream_id) const {
   return ContainsKey(active_streams_, stream_id);
 }
 
 LoadState SpdySession::GetLoadState() const {
   // NOTE: The application only queries the LoadState via the
   //       SpdyNetworkTransaction, and details are only needed when
@@ skipping 750 matching lines @@
 
   --pings_in_flight_;
   if (pings_in_flight_ < 0) {
     CloseSessionOnError(net::ERR_SPDY_PROTOCOL_ERROR, true);
     return;
   }
 
   if (pings_in_flight_ > 0)
     return;
 
-  if (last_sent_was_ping_)
+  if (!need_to_send_ping_)
     return;
 
   PlanToSendTrailingPing();
 }
 
 void SpdySession::OnSettings(const spdy::SpdySettingsControlFrame& frame) {
   spdy::SpdySettings settings;
   if (spdy_framer_.ParseSettings(&frame, &settings)) {
     HandleSettings(settings);
     spdy_settings_->Set(host_port_pair(), settings);
@@ skipping 42 matching lines @@
   CHECK_EQ(stream->stream_id(), stream_id);
 
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_RECV_WINDOW_UPDATE,
       make_scoped_refptr(new NetLogSpdyWindowUpdateParameter(
           stream_id, delta_window_size, stream->recv_window_size())));
 
   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
 // field trial policy.
 uint32 ApplyCwndFieldTrialPolicy(int cwnd) {
   base::FieldTrial* trial = base::FieldTrialList::Find("SpdyCwnd");
   if (!trial) {
       LOG(WARNING) << "Could not find \"SpdyCwnd\" in FieldTrialList";
       return cwnd;
   }
@@ skipping 44 matching lines @@
 
   net_log_.AddEvent(
       NetLog::TYPE_SPDY_SESSION_SEND_SETTINGS,
       make_scoped_refptr(new NetLogSpdySettingsParameter(settings)));
 
   // Create the SETTINGS frame and send it.
   scoped_ptr<spdy::SpdySettingsControlFrame> settings_frame(
       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) {
   for (spdy::SpdySettings::const_iterator i = settings.begin(),
            end = settings.end(); i != end; ++i) {
     const uint32 id = i->first.id();
     const uint32 val = i->second;
     switch (id) {
       case spdy::SETTINGS_MAX_CONCURRENT_STREAMS:
         max_concurrent_streams_ = std::min(static_cast<size_t>(val),
@@ skipping 14 matching lines @@
 
   base::TimeTicks now = base::TimeTicks::Now();
   // If we haven't heard from server, then send a preface-PING.
   if ((now - received_data_time_) > kConnectionAtRiskOfLoss)
     SendPrefacePing();
 
   PlanToSendTrailingPing();
 }
 
 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() {
   if (trailing_ping_pending_)
     return;
 
   trailing_ping_pending_ = true;
   MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
       method_factory_.NewRunnableMethod(&SpdySession::SendTrailingPing),
@@ skipping 12 matching lines @@
   QueueFrame(ping_frame.get(), SPDY_PRIORITY_HIGHEST, NULL);
 
   if (net_log().IsLoggingAllEvents()) {
     net_log().AddEvent(
         NetLog::TYPE_SPDY_SESSION_PING,
         make_scoped_refptr(new NetLogSpdyPingParameter(next_ping_id_)));
   }
   if (unique_id % 2 != 0) {
     next_ping_id_ += 2;
     ++pings_in_flight_;
-    last_sent_was_ping_ = true;
+    need_to_send_ping_ = false;
     PlanToCheckPingStatus();
   }
 }
 
 void SpdySession::PlanToCheckPingStatus() {
   if (check_ping_status_pending_)
     return;
 
   check_ping_status_pending_ = true;
   MessageLoop::current()->PostDelayedTask(
@@ skipping 111 matching lines @@
   if (it == pending_callback_map_.end())
     return;
 
   OldCompletionCallback* callback = it->second.callback;
   int result = it->second.result;
   pending_callback_map_.erase(it);
   callback->Run(result);
 }
 
 }  // namespace net