Fix a QUIC bug where the headers stream becomes flow control blocked and

never writes data again.

Merge internal change: 77265365

R=rch@chromium.org
BUG=423022

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

Cr-Commit-Position: refs/heads/master@{#299361}

4 files changed, 93 insertions, 7 deletions

diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc
index 5acc287..f2fb5d9 100644
--- a/net/quic/quic_session.cc
+++ b/net/quic/quic_session.cc
@@ -528,14 +528,12 @@ void QuicSession::OnNewStreamFlowControlWindow(uint32 new_window) {
 
   // Inform all existing streams about the new window.
   if (connection_->version() >= QUIC_VERSION_21) {
-    GetCryptoStream()->flow_controller()->UpdateSendWindowOffset(new_window);
-    headers_stream_->flow_controller()->UpdateSendWindowOffset(new_window);
+    GetCryptoStream()->UpdateSendWindowOffset(new_window);
+    headers_stream_->UpdateSendWindowOffset(new_window);
   }
   for (DataStreamMap::iterator it = stream_map_.begin();
        it != stream_map_.end(); ++it) {
-    if (it->second->flow_controller()->UpdateSendWindowOffset(new_window)) {
-      it->second->OnCanWrite();
-    }
+    it->second->UpdateSendWindowOffset(new_window);
   }
 }
 
diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc
index a33355c..ca68891 100644
--- a/net/quic/quic_session_test.cc
+++ b/net/quic/quic_session_test.cc
@@ -9,6 +9,8 @@
 
 #include "base/basictypes.h"
 #include "base/containers/hash_tables.h"
+#include "base/rand_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "net/quic/crypto/crypto_protocol.h"
 #include "net/quic/quic_crypto_stream.h"
 #include "net/quic/quic_flags.h"
@@ -656,6 +658,81 @@ TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedStream) {
   EXPECT_FALSE(stream2->flow_controller()->IsBlocked());
 }
 
+TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedCryptoStream) {
+  if (version() <= QUIC_VERSION_19) {
+    return;
+  }
+  // Test that if the crypto stream is flow control blocked, then if the SHLO
+  // contains a larger send window offset, the stream becomes unblocked.
+  session_.set_writev_consumes_all_data(true);
+  TestCryptoStream* crypto_stream = session_.GetCryptoStream();
+  EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+  QuicHeadersStream* headers_stream =
+        QuicSessionPeer::GetHeadersStream(&session_);
+  EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+  // Write until the crypto stream is flow control blocked.
+  int i = 0;
+  while (!crypto_stream->flow_controller()->IsBlocked() && i < 1000) {
+    QuicConfig config;
+    CryptoHandshakeMessage crypto_message;
+    config.ToHandshakeMessage(&crypto_message);
+    crypto_stream->SendHandshakeMessage(crypto_message);
+    ++i;
+  }
+  EXPECT_TRUE(crypto_stream->flow_controller()->IsBlocked());
+  EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+  EXPECT_FALSE(session_.HasDataToWrite());
+  EXPECT_TRUE(crypto_stream->HasBufferedData());
+
+  // The handshake message will call OnCanWrite, so the stream can
+  // resume writing.
+  EXPECT_CALL(*crypto_stream, OnCanWrite());
+  // Now complete the crypto handshake, resulting in an increased flow control
+  // send window.
+  CryptoHandshakeMessage msg;
+  session_.GetCryptoStream()->OnHandshakeMessage(msg);
+
+  // Stream is now unblocked and will no longer have buffered data.
+  EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+}
+
+TEST_P(QuicSessionTest, HandshakeUnblocksFlowControlBlockedHeadersStream) {
+  if (version() <= QUIC_VERSION_19) {
+    return;
+  }
+  // Test that if the header stream is flow control blocked, then if the SHLO
+  // contains a larger send window offset, the stream becomes unblocked.
+  session_.set_writev_consumes_all_data(true);
+  TestCryptoStream* crypto_stream = session_.GetCryptoStream();
+  EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+  QuicHeadersStream* headers_stream =
+      QuicSessionPeer::GetHeadersStream(&session_);
+  EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+  QuicStreamId stream_id = 5;
+  // Write until the header stream is flow control blocked.
+  while (!headers_stream->flow_controller()->IsBlocked() && stream_id < 2000) {
+    SpdyHeaderBlock headers;
+    headers["header"] = base::Uint64ToString(base::RandUint64()) +
+        base::Uint64ToString(base::RandUint64()) +
+        base::Uint64ToString(base::RandUint64());
+    headers_stream->WriteHeaders(stream_id, headers, true, nullptr);
+    stream_id += 2;
+  }
+  EXPECT_TRUE(headers_stream->flow_controller()->IsBlocked());
+  EXPECT_FALSE(crypto_stream->flow_controller()->IsBlocked());
+  EXPECT_FALSE(session_.HasDataToWrite());
+  EXPECT_TRUE(headers_stream->HasBufferedData());
+
+  // Now complete the crypto handshake, resulting in an increased flow control
+  // send window.
+  CryptoHandshakeMessage msg;
+  session_.GetCryptoStream()->OnHandshakeMessage(msg);
+
+  // Stream is now unblocked and will no longer have buffered data.
+  EXPECT_FALSE(headers_stream->flow_controller()->IsBlocked());
+  EXPECT_FALSE(headers_stream->HasBufferedData());
+}
+
 TEST_P(QuicSessionTest, InvalidFlowControlWindowInHandshake) {
   // TODO(rjshade): Remove this test when removing QUIC_VERSION_19.
   // Test that receipt of an invalid (< default) flow control window from
diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc
index ff21230..a65203b 100644
--- a/net/quic/reliable_quic_stream.cc
+++ b/net/quic/reliable_quic_stream.cc
@@ -532,6 +532,12 @@ void ReliableQuicStream::AddBytesConsumed(uint64 bytes) {
   }
 }
 
+void ReliableQuicStream::UpdateSendWindowOffset(uint64 new_window) {
+  if (flow_controller_.UpdateSendWindowOffset(new_window)) {
+    OnCanWrite();
+  }
+}
+
 bool ReliableQuicStream::IsFlowControlBlocked() {
   if (flow_controller_.IsBlocked()) {
     return true;
diff --git a/net/quic/reliable_quic_stream.h b/net/quic/reliable_quic_stream.h
index 5b5ae0f..24bc349 100644
--- a/net/quic/reliable_quic_stream.h
+++ b/net/quic/reliable_quic_stream.h
@@ -111,6 +111,10 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
   // WINDOW_UPDATE frame.
   void AddBytesConsumed(uint64 bytes);
 
+  // Updates the flow controller's send window offset and calls OnCanWrite if
+  // it was blocked before.
+  void UpdateSendWindowOffset(uint64 new_offset);
+
   // Returns true if the stream is flow control blocked, by the stream flow
   // control window or the connection flow control window.
   bool IsFlowControlBlocked();
@@ -123,6 +127,9 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
     return fin_received_ || rst_received_;
   }
 
+  // Returns true if the stream has queued data waiting to write.
+  bool HasBufferedData() const;
+
  protected:
   // Sends as much of 'data' to the connection as the connection will consume,
   // and then buffers any remaining data in queued_data_.
@@ -151,8 +158,6 @@ class NET_EXPORT_PRIVATE ReliableQuicStream {
   // Close the write side of the socket.  Further writes will fail.
   void CloseWriteSide();
 
-  bool HasBufferedData() const;
-
   bool fin_buffered() const { return fin_buffered_; }
 
   const QuicSession* session() const { return session_; }