QUIC_VERSION_21: headers and crypto streams are now flow controlled at

net/quic/reliable_quic_stream.cc

 // Copyright (c) 2012 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/quic/reliable_quic_stream.h"
 
 #include "base/logging.h"
 #include "net/quic/iovector.h"
 #include "net/quic/quic_flow_controller.h"
 #include "net/quic/quic_session.h"
@@ skipping 141 matching lines @@
       fin_received_(false),
       rst_sent_(false),
       rst_received_(false),
       fec_policy_(FEC_PROTECT_OPTIONAL),
       is_server_(session_->is_server()),
       flow_controller_(
           session_->connection(), id_, is_server_,
           GetReceivedFlowControlWindow(session),
           GetInitialStreamFlowControlWindowToSend(session),
           GetInitialStreamFlowControlWindowToSend(session)),
-      connection_flow_controller_(session_->flow_controller()) {
+      connection_flow_controller_(session_->flow_controller()),
+      stream_contributes_to_connection_flow_control_(true) {
 }
 
 ReliableQuicStream::~ReliableQuicStream() {
 }
 
 bool ReliableQuicStream::OnStreamFrame(const QuicStreamFrame& frame) {
   if (read_side_closed_) {
     DVLOG(1) << ENDPOINT << "Ignoring frame " << frame.stream_id;
     // We don't want to be reading: blackhole the data.
     return true;
@@ skipping 152 matching lines @@
           delegate->WroteData(false);
         }
       }
       break;
     }
   }
 }
 
 void ReliableQuicStream::MaybeSendBlocked() {
   flow_controller_.MaybeSendBlocked();
-  connection_flow_controller_->MaybeSendBlocked();
+  if (stream_contributes_to_connection_flow_control_) {

[wtc, 2014/07/02 22:38:31]

Nit: we can merge these two if statements as follows:

  if (stream_contributes_to_connection_flow_control_) {
    connection_flow_controller_->MaybeSendBlocked();
    // If we are connection level flow control blocked, then add the stream
    // to the write blocked list. It will be given a chance to write when a
    // connection level WINDOW_UPDATE arrives.
    if (connection_flow_controller_->IsBlocked() &&
        !flow_controller_.IsBlocked()) {
      session_->MarkWriteBlocked(id(), EffectivePriority());
    }
  }

Alternatively, we can do an early return:

  if (!stream_contributes_to_connection_flow_control_) {
    return;
  }
  connection_flow_controller_->MaybeSendBlocked();
  ...

[jar (doing other things), 2014/07/14 21:49:23]

+1 for early return suggestion.

[ramant (doing other things), 2014/07/16 19:43:08]

Done.

[ramant (doing other things), 2014/07/16 19:43:08]

Done.

+    connection_flow_controller_->MaybeSendBlocked();
+  }
   // If we are connection level flow control blocked, then add the stream
   // to the write blocked list. It will be given a chance to write when a
   // connection level WINDOW_UPDATE arrives.
-  if (connection_flow_controller_->IsBlocked() &&
+  if (stream_contributes_to_connection_flow_control_ &&
+      connection_flow_controller_->IsBlocked() &&
       !flow_controller_.IsBlocked()) {
     session_->MarkWriteBlocked(id(), EffectivePriority());
   }
 }
 
 QuicConsumedData ReliableQuicStream::WritevData(
     const struct iovec* iov,
     int iov_count,
     bool fin,
     QuicAckNotifier::DelegateInterface* ack_notifier_delegate) {
   if (write_side_closed_) {
     DLOG(ERROR) << ENDPOINT << "Attempt to write when the write side is closed";
     return QuicConsumedData(0, false);
   }
 
   // How much data we want to write.
   size_t write_length = TotalIovecLength(iov, iov_count);
 
   // A FIN with zero data payload should not be flow control blocked.
   bool fin_with_zero_data = (fin && write_length == 0);
 
   if (flow_controller_.IsEnabled()) {
     // How much data we are allowed to write from flow control.
     uint64 send_window = flow_controller_.SendWindowSize();
-    if (connection_flow_controller_->IsEnabled()) {
+    // TODO(rjshade): Remove connection_flow_controller_->IsEnabled() check when
+    // removing QUIC_VERSION_19.
+    if (stream_contributes_to_connection_flow_control_ &&
+        connection_flow_controller_->IsEnabled()) {
       send_window =
           min(send_window, connection_flow_controller_->SendWindowSize());
     }
 
     if (send_window == 0 && !fin_with_zero_data) {
       // Quick return if we can't send anything.
       MaybeSendBlocked();
       return QuicConsumedData(0, false);
     }
 
@@ skipping 108 matching lines @@
 }
 
 bool ReliableQuicStream::MaybeIncreaseHighestReceivedOffset(uint64 new_offset) {
   if (flow_controller_.IsEnabled()) {
     uint64 increment =
         new_offset - flow_controller_.highest_received_byte_offset();
     if (flow_controller_.UpdateHighestReceivedOffset(new_offset)) {
       // If |new_offset| increased the stream flow controller's highest received
       // offset, then we need to increase the connection flow controller's value
       // by the incremental difference.
-      connection_flow_controller_->UpdateHighestReceivedOffset(
-          connection_flow_controller_->highest_received_byte_offset() +
-          increment);
+      if (stream_contributes_to_connection_flow_control_) {

[wtc, 2014/07/02 22:38:30]

Nit: merge this inner if with the if on line 496.

[ramant (doing other things), 2014/07/16 19:43:08]

Reworked the if statements.

+        connection_flow_controller_->UpdateHighestReceivedOffset(
+            connection_flow_controller_->highest_received_byte_offset() +
+            increment);
+      }
       return true;
     }
   }
   return false;
 }
 
 void ReliableQuicStream::AddBytesSent(uint64 bytes) {
   if (flow_controller_.IsEnabled()) {
     flow_controller_.AddBytesSent(bytes);
-    connection_flow_controller_->AddBytesSent(bytes);
+    if (stream_contributes_to_connection_flow_control_) {
+      connection_flow_controller_->AddBytesSent(bytes);
+    }
   }
 }
 
 void ReliableQuicStream::AddBytesConsumed(uint64 bytes) {
   if (flow_controller_.IsEnabled()) {
     // Only adjust stream level flow controller if we are still reading.
     if (!read_side_closed_) {
       flow_controller_.AddBytesConsumed(bytes);
     }
 
-    connection_flow_controller_->AddBytesConsumed(bytes);
+    if (stream_contributes_to_connection_flow_control_) {
+      connection_flow_controller_->AddBytesConsumed(bytes);
+    }
   }
 }
 
 bool ReliableQuicStream::IsFlowControlBlocked() {
-  return flow_controller_.IsBlocked() ||
-         connection_flow_controller_->IsBlocked();
+  bool stream_flow_control_blocked = flow_controller_.IsBlocked();
+  bool connecton_flow_control_blocked =
+      stream_contributes_to_connection_flow_control_ &&
+      connection_flow_controller_->IsBlocked();
+  return stream_flow_control_blocked || connecton_flow_control_blocked;

[wtc, 2014/07/02 22:38:31]

Nit: the new code loses some of the short circuit. We can also do:

  if (flow_controller_.IsBlocked()) {
    return true;
  }
  return stream_contributes_to_connection_flow_control_ &&
         connection_flow_controller_->IsBlocked();

[ramant (doing other things), 2014/07/16 19:43:08]

Done.

 }
 
 }  // namespace net