Clean up QuicStreamIdManager comments and remove unnecessary methods.
gfe-relnote: no behavior change. not protected.
PiperOrigin-RevId: 305359897
Change-Id: I4253fba5f6de6fb408310e57b5c73c85fa06a93f
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc
index 5cb8077..7aafb9d 100644
--- a/quic/core/quic_stream_id_manager.cc
+++ b/quic/core/quic_stream_id_manager.cc
@@ -35,8 +35,6 @@
next_outgoing_stream_id_(GetFirstOutgoingStreamId()),
outgoing_stream_count_(0),
incoming_actual_max_streams_(max_allowed_incoming_streams),
- // Advertised max starts at actual because it's communicated in the
- // handshake.
incoming_advertised_max_streams_(max_allowed_incoming_streams),
incoming_initial_max_open_streams_(max_allowed_incoming_streams),
incoming_stream_count_(0),
@@ -45,16 +43,12 @@
QuicStreamIdManager::~QuicStreamIdManager() {}
-// The peer sends a streams blocked frame when it can not open any more
-// streams because it has runs into the limit.
bool QuicStreamIdManager::OnStreamsBlockedFrame(
const QuicStreamsBlockedFrame& frame,
std::string* error_details) {
- // Ensure that the frame has the correct directionality.
DCHECK_EQ(frame.unidirectional, unidirectional_);
if (frame.stream_count > incoming_advertised_max_streams_) {
// Peer thinks it can send more streams that we've told it.
- // This is a protocol error.
*error_details = quiche::QuicheStrCat(
"StreamsBlockedFrame's stream count ", frame.stream_count,
" exceeds incoming max stream ", incoming_advertised_max_streams_);
@@ -62,8 +56,7 @@
}
if (frame.stream_count < incoming_actual_max_streams_) {
// Peer thinks it's blocked on a stream count that is less than our current
- // max. Inform the peer of the correct stream count. Sending a MAX_STREAMS
- // frame in this case is not controlled by the window.
+ // max. Inform the peer of the correct stream count.
SendMaxStreamsFrame();
}
return true;
@@ -121,22 +114,18 @@
return;
}
// If the stream is inbound, we can increase the actual stream limit and maybe
- // advertise the new limit to the peer. Have to check to make sure that we do
- // not exceed the maximum.
+ // advertise the new limit to the peer.
if (incoming_actual_max_streams_ == QuicUtils::GetMaxStreamCount()) {
// Reached the maximum stream id value that the implementation
// supports. Nothing can be done here.
return;
}
- // One stream closed ... another can be opened.
+ // One stream closed, and another one can be opened.
incoming_actual_max_streams_++;
MaybeSendMaxStreamsFrame();
}
QuicStreamId QuicStreamIdManager::GetNextOutgoingStreamId() {
- // Applications should always consult CanOpenNextOutgoingStream() first.
- // If they ask for stream ids that violate the limit, it's an implementation
- // bug.
QUIC_BUG_IF(outgoing_stream_count_ >= outgoing_max_streams_)
<< "Attempt to allocate a new outgoing stream that would exceed the "
"limit ("
@@ -158,7 +147,7 @@
// |stream_id| must be an incoming stream of the right directionality.
DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id), unidirectional_);
DCHECK_NE(QuicUtils::IsServerInitiatedStreamId(transport_version_, stream_id),
- perspective() == Perspective::IS_SERVER);
+ perspective_ == Perspective::IS_SERVER);
if (available_streams_.erase(stream_id) == 1) {
// stream_id is available.
return true;
@@ -225,24 +214,18 @@
QuicStreamId QuicStreamIdManager::GetFirstOutgoingStreamId() const {
return (unidirectional_) ? QuicUtils::GetFirstUnidirectionalStreamId(
- transport_version_, perspective())
+ transport_version_, perspective_)
: QuicUtils::GetFirstBidirectionalStreamId(
- transport_version_, perspective());
+ transport_version_, perspective_);
}
QuicStreamId QuicStreamIdManager::GetFirstIncomingStreamId() const {
return (unidirectional_) ? QuicUtils::GetFirstUnidirectionalStreamId(
- transport_version_, peer_perspective())
+ transport_version_,
+ QuicUtils::InvertPerspective(perspective_))
: QuicUtils::GetFirstBidirectionalStreamId(
- transport_version_, peer_perspective());
-}
-
-Perspective QuicStreamIdManager::perspective() const {
- return perspective_;
-}
-
-Perspective QuicStreamIdManager::peer_perspective() const {
- return QuicUtils::InvertPerspective(perspective());
+ transport_version_,
+ QuicUtils::InvertPerspective(perspective_));
}
QuicStreamCount QuicStreamIdManager::available_incoming_streams() {
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h
index 4edc363..cf1e81e 100644
--- a/quic/core/quic_stream_id_manager.h
+++ b/quic/core/quic_stream_id_manager.h
@@ -43,7 +43,7 @@
std::string DebugString() const {
return quiche::QuicheStrCat(
" { unidirectional_: ", unidirectional_,
- ", perspective: ", perspective(),
+ ", perspective: ", perspective_,
", outgoing_max_streams_: ", outgoing_max_streams_,
", next_outgoing_stream_id_: ", next_outgoing_stream_id_,
", outgoing_stream_count_: ", outgoing_stream_count_,
@@ -61,7 +61,7 @@
bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame,
std::string* error_details);
- // Indicates whether the next outgoing stream ID can be allocated or not.
+ // Returns whether the next outgoing stream ID can be allocated or not.
bool CanOpenNextOutgoingStream() const;
// Generate and send a MAX_STREAMS frame.
@@ -74,8 +74,7 @@
void OnStreamClosed(QuicStreamId stream_id);
// Returns the next outgoing stream id. Applications must call
- // CanOpenNextOutgoingStream() first. A QUIC_BUG is logged if this method
- // allocates a stream ID past the peer specified limit.
+ // CanOpenNextOutgoingStream() first.
QuicStreamId GetNextOutgoingStreamId();
void SetMaxOpenIncomingStreams(QuicStreamCount max_open_streams);
@@ -86,11 +85,7 @@
bool MaybeAllowNewOutgoingStreams(QuicStreamCount max_open_streams);
// Checks if the incoming stream ID exceeds the MAX_STREAMS limit. If the
- // limit is exceeded, populates |error_detials| and returns false. Uses the
- // actual maximium, not the most recently advertised value, in order to
- // enforce the Google-QUIC number of open streams behavior.
- // This method should be called exactly once for each incoming stream
- // creation.
+ // limit is exceeded, populates |error_detials| and returns false.
bool MaybeIncreaseLargestPeerStreamId(const QuicStreamId stream_id,
std::string* error_details);
@@ -115,11 +110,6 @@
return largest_peer_created_stream_id_;
}
- // These are the limits for outgoing and incoming streams,
- // respectively. For incoming there are two limits, what has
- // been advertised to the peer and what is actually available.
- // The advertised incoming amount should never be more than the actual
- // incoming one.
QuicStreamCount outgoing_max_streams() const { return outgoing_max_streams_; }
QuicStreamCount incoming_actual_max_streams() const {
return incoming_actual_max_streams_;
@@ -127,14 +117,8 @@
QuicStreamCount incoming_advertised_max_streams() const {
return incoming_advertised_max_streams_;
}
- // Number of streams that have been opened (including those that have been
- // opened and then closed. Must never exceed outgoing_max_streams
QuicStreamCount outgoing_stream_count() { return outgoing_stream_count_; }
- // Perspective (CLIENT/SERVER) of this node and the peer, respectively.
- Perspective perspective() const;
- Perspective peer_perspective() const;
-
private:
friend class test::QuicSessionPeer;
friend class test::QuicStreamIdManagerPeer;
@@ -150,7 +134,6 @@
QuicStreamId GetFirstIncomingStreamId() const;
// Back reference to the session containing this Stream ID Manager.
- // needed to access various session methods, such as perspective()
DelegateInterface* delegate_;
// Whether this stream id manager is for unidrectional (true) or bidirectional
@@ -163,11 +146,9 @@
// Transport version used for this manager.
const QuicTransportVersion transport_version_;
- // This is the number of streams that this node can initiate.
- // This limit is:
- // - Initiated to a value specified in the constructor
- // - May be updated when the config is received.
- // - Is updated whenever a MAX STREAMS frame is received.
+ // The number of streams that this node can initiate.
+ // This limit is first set when config is negotiated, but may be updated upon
+ // receiving MAX_STREAMS frame.
QuicStreamCount outgoing_max_streams_;
// The ID to use for the next outgoing stream.
@@ -180,16 +161,18 @@
// FOR INCOMING STREAMS
- // The maximum number of streams that can be opened by the peer.
+ // The actual maximum number of streams that can be opened by the peer.
QuicStreamCount incoming_actual_max_streams_;
+ // Max incoming stream number that has been advertised to the peer and is <=
+ // incoming_actual_max_streams_. It is set to incoming_actual_max_streams_
+ // when a MAX_STREAMS is sent.
QuicStreamCount incoming_advertised_max_streams_;
// Initial maximum on the number of open streams allowed.
QuicStreamCount incoming_initial_max_open_streams_;
- // This is the number of streams that have been created -- some are still
- // open, the others have been closed. It is the number that is compared
- // against MAX_STREAMS when deciding whether to accept a new stream or not.
+ // The number of streams that have been created, including open ones and
+ // closed ones.
QuicStreamCount incoming_stream_count_;
// Set of stream ids that are less than the largest stream id that has been
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc
index aedec33..a5895ff 100644
--- a/quic/core/quic_stream_id_manager_test.cc
+++ b/quic/core/quic_stream_id_manager_test.cc
@@ -273,12 +273,11 @@
EXPECT_TRUE(
stream_id_manager_.MaybeAllowNewOutgoingStreams(number_of_streams));
- QuicStreamId stream_id =
- IsUnidirectional()
- ? QuicUtils::GetFirstUnidirectionalStreamId(
- transport_version(), stream_id_manager_.perspective())
- : QuicUtils::GetFirstBidirectionalStreamId(
- transport_version(), stream_id_manager_.perspective());
+ QuicStreamId stream_id = IsUnidirectional()
+ ? QuicUtils::GetFirstUnidirectionalStreamId(
+ transport_version(), perspective())
+ : QuicUtils::GetFirstBidirectionalStreamId(
+ transport_version(), perspective());
EXPECT_EQ(number_of_streams, stream_id_manager_.outgoing_max_streams());
while (number_of_streams) {
