commit aa36e6c02780478d8fb79d19af0b3428285a59cb
author fayang <fayang@google.com> Fri Apr 17 13:12:43 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Apr 17 13:13:10 2020 -0700
tree df990b3dc5322243892786c4663e3053af4a48fe
parent 3a932e1cf766147b5aa27f8bd0a1e85dc74b1fb4

gfe-relnote: In QUIC, rewrite part of QuicSession::OnStreamClosed to make it simpler. Refactoring only. No functional change expected. Protected by existing gfe2_reloadable_flag_quic_break_session_stream_close_loop.

PiperOrigin-RevId: 307101589
Change-Id: Ifc90bfebf0006814788897729a8b621cd3c7d3e9

quic/core/quic_session.cc

diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 3c6d323..e79ec24 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -1008,14 +1008,22 @@
     }
   }
 
-  // If we haven't received a FIN or RST for this stream, we need to keep track
-  // of the how many bytes the stream's flow controller believes it has
-  // received, for accurate connection level flow control accounting.
-  const bool had_fin_or_rst = stream->HasReceivedFinalOffset();
-  if (!had_fin_or_rst) {
+  if (!stream->HasReceivedFinalOffset()) {
+    // If we haven't received a FIN or RST for this stream, we need to keep
+    // track of the how many bytes the stream's flow controller believes it has
+    // received, for accurate connection level flow control accounting.
+    // If this is an outgoing stream, it is technically open from peer's
+    // perspective. Do not inform stream Id manager yet.
+    DCHECK(!stream->was_draining());
     InsertLocallyClosedStreamsHighestOffset(
         stream_id, stream->flow_controller()->highest_received_byte_offset());
+    stream_map_.erase(it);
+    if (IsIncomingStream(stream_id)) {
+      --num_dynamic_incoming_streams_;
+    }
+    return;
   }
+
   bool stream_was_draining = false;
   if (deprecate_draining_streams_) {
     stream_was_draining = stream->was_draining();
@@ -1039,19 +1047,22 @@
       --num_draining_outgoing_streams_;
     }
     draining_streams_.erase(stream_id);
-  } else if (VersionHasIetfQuicFrames(transport_version())) {
-    // Stream was not draining, but we did have a fin or rst, so we can now
-    // free the stream ID if version 99.
-    if (had_fin_or_rst && connection_->connected()) {
-      // Do not bother informing stream ID manager if connection is closed.
+    // Stream Id manager has been informed with draining streams.
+    return;
+  }
+  if (!connection_->connected()) {
+    // Do not bother informing stream ID manager if connection has been
+    // disconnected.
+    return;
+  }
+  if (IsIncomingStream(stream_id)) {
+    // Stream Id manager is only interested in peer initiated stream IDs.
+    if (VersionHasIetfQuicFrames(transport_version())) {
       v99_streamid_manager_.OnStreamClosed(stream_id);
     }
+    return;
   }
-
-  if (!stream_was_draining && !IsIncomingStream(stream_id) && had_fin_or_rst &&
-      !VersionHasIetfQuicFrames(transport_version())) {
-    // Streams that first became draining already called OnCanCreate...
-    // This covers the case where the stream went directly to being closed.
+  if (!VersionHasIetfQuicFrames(transport_version())) {
     OnCanCreateNewOutgoingStream(type != BIDIRECTIONAL);
   }
 }