SPDY - default to SPDY/2.1 protocol in unittests

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
===================================================================
--- net/spdy/spdy_session.cc	(revision 119096)
+++ net/spdy/spdy_session.cc	(working copy)
@@ -227,7 +227,8 @@
 bool SpdySession::use_ssl_ = true;
 
 // static
-bool SpdySession::use_flow_control_ = false;
+SpdySession::FlowControl SpdySession::use_flow_control_ =
+    SpdySession::kFlowControlBasedOnNPN;
 
 // static
 size_t SpdySession::init_max_concurrent_streams_ = 10;
@@ -282,7 +283,7 @@
       trailing_ping_pending_(false),
       check_ping_status_pending_(false),
       need_to_send_ping_(false),
-      flow_control_(use_flow_control_),
+      flow_control_(false),
       initial_send_window_size_(spdy::kSpdyStreamInitialWindowSize),
       initial_recv_window_size_(spdy::kSpdyStreamInitialWindowSize),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SPDY_SESSION)),
@@ -293,6 +294,10 @@
       make_scoped_refptr(
           new NetLogSpdySessionParameter(host_port_proxy_pair_)));
 
+  // In unittests, check if use_flow_control_ is enabled or disabled.

[wtc, 2012/01/26 00:13:26]

Nit: change all occurrences of "unittests" to "unit tests"

The code here means flow_control_ is false for
SpdySession::kFlowControlBasedOnNPN.  Is that intended?

[ramant (doing other things), 2012/01/26 18:58:48]

Done.

Until the server is ready to support flow control, we will initialize to false
(afterwards we will initialize to true). We determine the flow_control based on
the protocol version that is negotiated.

+  if (use_flow_control_ == SpdySession::kEnableFlowControl)
+    flow_control_ = true;
+
   // TODO(mbelshe): consider randomization of the stream_hi_water_mark.
 
   buffered_spdy_framer_.set_visitor(this);
@@ -344,8 +349,20 @@
     SSLClientSocket* ssl_socket =
         reinterpret_cast<SSLClientSocket*>(connection_->socket());
     DCHECK(ssl_socket);
-    if (ssl_socket->protocol_negotiated() == SSLClientSocket::kProtoSPDY21)
+    if (ssl_socket->protocol_negotiated() == SSLClientSocket::kProtoSPDY21) {
       flow_control_ = true;
+
+      // In unittests, check if use_flow_control_ is enabled or disabled.
+      if (use_flow_control_ == SpdySession::kDisableFlowControl)
+        flow_control_ = false;
+

[wtc, 2012/01/26 00:13:26]

Delete this blank line.

[ramant (doing other things), 2012/01/26 18:58:48]

Done.

+    } else {
+      flow_control_ = false;
+
+      // In unittests, check if use_flow_control_ is enabled or disabled.
+      if (use_flow_control_ == SpdySession::kEnableFlowControl)
+        flow_control_ = true;
+    }

[wtc, 2012/01/26 00:13:26]

This logic is hard to understand.  It would be nice to explain
it.  I think this logic says in SPDY 2.1 flow control is
enabled by default, whereas in SPDY 2 flow control is disabled
by default.  Correct?

To support SPDY 3 in the future, I think the test on
line 352 should be
  ssl_socket->protocol_negotiated() >= SSLClientSocket::kProtoSPDY21

[ramant (doing other things), 2012/01/26 18:58:48]

Done.

   }
 
   // Write out any data that we might have to send, such as the settings frame.