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) {