Remove QuicStreamIdManager::IsIncomingStream() as it's hard coded. Also QuicSession doesn't need the indirectness. QuicUtils should be the single source of truth. gfe-relnote: no behavior change. not protected. PiperOrigin-RevId: 305518573 Change-Id: I926c93ccd33f12c694b823d764a88d13df3e6c94
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index d190b6c..b57ddd7 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -1840,7 +1840,7 @@ bool QuicSession::IsIncomingStream(QuicStreamId id) const { if (VersionHasIetfQuicFrames(transport_version())) { - return v99_streamid_manager_.IsIncomingStream(id); + return !QuicUtils::IsOutgoingStreamId(version(), id, perspective_); } return stream_id_manager_.IsIncomingStream(id); }
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 7aafb9d..11f0668 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -10,6 +10,7 @@ #include "net/third_party/quiche/src/quic/core/quic_constants.h" #include "net/third_party/quiche/src/quic/core/quic_session.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/core/quic_versions.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flag_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" @@ -24,13 +25,13 @@ DelegateInterface* delegate, bool unidirectional, Perspective perspective, - QuicTransportVersion transport_version, + ParsedQuicVersion version, QuicStreamCount max_allowed_outgoing_streams, QuicStreamCount max_allowed_incoming_streams) : delegate_(delegate), unidirectional_(unidirectional), perspective_(perspective), - transport_version_(transport_version), + version_(version), outgoing_max_streams_(max_allowed_outgoing_streams), next_outgoing_stream_id_(GetFirstOutgoingStreamId()), outgoing_stream_count_(0), @@ -39,7 +40,7 @@ incoming_initial_max_open_streams_(max_allowed_incoming_streams), incoming_stream_count_(0), largest_peer_created_stream_id_( - QuicUtils::GetInvalidStreamId(transport_version)) {} + QuicUtils::GetInvalidStreamId(version.transport_version)) {} QuicStreamIdManager::~QuicStreamIdManager() {} @@ -109,7 +110,7 @@ void QuicStreamIdManager::OnStreamClosed(QuicStreamId stream_id) { DCHECK_NE(QuicUtils::IsBidirectionalStreamId(stream_id), unidirectional_); - if (!IsIncomingStream(stream_id)) { + if (QuicUtils::IsOutgoingStreamId(version_, stream_id, perspective_)) { // Nothing to do for outgoing streams. return; } @@ -131,13 +132,14 @@ "limit (" << outgoing_max_streams_ << ")"; QuicStreamId id = next_outgoing_stream_id_; - next_outgoing_stream_id_ += QuicUtils::StreamIdDelta(transport_version_); + next_outgoing_stream_id_ += + QuicUtils::StreamIdDelta(version_.transport_version); outgoing_stream_count_++; return id; } bool QuicStreamIdManager::CanOpenNextOutgoingStream() const { - DCHECK(VersionHasIetfQuicFrames(transport_version_)); + DCHECK(VersionHasIetfQuicFrames(version_.transport_version)); return outgoing_stream_count_ < outgoing_max_streams_; } @@ -146,7 +148,8 @@ std::string* error_details) { // |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), + DCHECK_NE(QuicUtils::IsServerInitiatedStreamId(version_.transport_version, + stream_id), perspective_ == Perspective::IS_SERVER); if (available_streams_.erase(stream_id) == 1) { // stream_id is available. @@ -154,15 +157,16 @@ } if (largest_peer_created_stream_id_ != - QuicUtils::GetInvalidStreamId(transport_version_)) { + QuicUtils::GetInvalidStreamId(version_.transport_version)) { DCHECK_GT(stream_id, largest_peer_created_stream_id_); } // Calculate increment of incoming_stream_count_ by creating stream_id. - const QuicStreamCount delta = QuicUtils::StreamIdDelta(transport_version_); + const QuicStreamCount delta = + QuicUtils::StreamIdDelta(version_.transport_version); const QuicStreamId least_new_stream_id = largest_peer_created_stream_id_ == - QuicUtils::GetInvalidStreamId(transport_version_) + QuicUtils::GetInvalidStreamId(version_.transport_version) ? GetFirstIncomingStreamId() : largest_peer_created_stream_id_ + delta; const QuicStreamCount stream_count_increment = @@ -190,45 +194,35 @@ bool QuicStreamIdManager::IsAvailableStream(QuicStreamId id) const { DCHECK_NE(QuicUtils::IsBidirectionalStreamId(id), unidirectional_); - if (!IsIncomingStream(id)) { + if (QuicUtils::IsOutgoingStreamId(version_, id, perspective_)) { // Stream IDs under next_ougoing_stream_id_ are either open or previously // open but now closed. return id >= next_outgoing_stream_id_; } // For peer created streams, we also need to consider available streams. return largest_peer_created_stream_id_ == - QuicUtils::GetInvalidStreamId(transport_version_) || + QuicUtils::GetInvalidStreamId(version_.transport_version) || id > largest_peer_created_stream_id_ || QuicContainsKey(available_streams_, id); } -bool QuicStreamIdManager::IsIncomingStream(QuicStreamId id) const { - DCHECK_NE(QuicUtils::IsBidirectionalStreamId(id), unidirectional_); - // The 0x1 bit in the stream id indicates whether the stream id is - // server- or client- initiated. Next_OUTGOING_stream_id_ has that bit - // set based on whether this node is a server or client. Thus, if the stream - // id in question has the 0x1 bit set opposite of next_OUTGOING_stream_id_, - // then that stream id is incoming -- it is for streams initiated by the peer. - return (id & 0x1) != (next_outgoing_stream_id_ & 0x1); -} - QuicStreamId QuicStreamIdManager::GetFirstOutgoingStreamId() const { return (unidirectional_) ? QuicUtils::GetFirstUnidirectionalStreamId( - transport_version_, perspective_) + version_.transport_version, perspective_) : QuicUtils::GetFirstBidirectionalStreamId( - transport_version_, perspective_); + version_.transport_version, perspective_); } QuicStreamId QuicStreamIdManager::GetFirstIncomingStreamId() const { return (unidirectional_) ? QuicUtils::GetFirstUnidirectionalStreamId( - transport_version_, + version_.transport_version, QuicUtils::InvertPerspective(perspective_)) : QuicUtils::GetFirstBidirectionalStreamId( - transport_version_, + version_.transport_version, QuicUtils::InvertPerspective(perspective_)); } -QuicStreamCount QuicStreamIdManager::available_incoming_streams() { +QuicStreamCount QuicStreamIdManager::available_incoming_streams() const { return incoming_advertised_max_streams_ - incoming_stream_count_; }
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h index cf1e81e..bdcb7cb 100644 --- a/quic/core/quic_stream_id_manager.h +++ b/quic/core/quic_stream_id_manager.h
@@ -32,7 +32,7 @@ QuicStreamIdManager(DelegateInterface* delegate, bool unidirectional, Perspective perspective, - QuicTransportVersion transport_version, + ParsedQuicVersion version, QuicStreamCount max_allowed_outgoing_streams, QuicStreamCount max_allowed_incoming_streams); @@ -92,9 +92,6 @@ // Returns true if |id| is still available. bool IsAvailableStream(QuicStreamId id) const; - // Return true if given stream is peer initiated. - bool IsIncomingStream(QuicStreamId id) const; - QuicStreamCount incoming_initial_max_open_streams() const { return incoming_initial_max_open_streams_; } @@ -104,7 +101,7 @@ } // Number of streams that the peer believes that it can still create. - QuicStreamCount available_incoming_streams(); + QuicStreamCount available_incoming_streams() const; QuicStreamId largest_peer_created_stream_id() const { return largest_peer_created_stream_id_; @@ -117,7 +114,9 @@ QuicStreamCount incoming_advertised_max_streams() const { return incoming_advertised_max_streams_; } - QuicStreamCount outgoing_stream_count() { return outgoing_stream_count_; } + QuicStreamCount outgoing_stream_count() const { + return outgoing_stream_count_; + } private: friend class test::QuicSessionPeer; @@ -143,8 +142,8 @@ // Is this manager a client or a server. const Perspective perspective_; - // Transport version used for this manager. - const QuicTransportVersion transport_version_; + // QUIC version used for this manager. + const ParsedQuicVersion version_; // The number of streams that this node can initiate. // This limit is first set when config is negotiated, but may be updated upon
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc index a5895ff..cf7129c 100644 --- a/quic/core/quic_stream_id_manager_test.cc +++ b/quic/core/quic_stream_id_manager_test.cc
@@ -71,7 +71,7 @@ : stream_id_manager_(&delegate_, IsUnidirectional(), perspective(), - transport_version(), + GetParam().version, 0, kDefaultMaxStreamsPerConnection) { DCHECK(VersionHasIetfQuicFrames(transport_version()));
diff --git a/quic/core/uber_quic_stream_id_manager.cc b/quic/core/uber_quic_stream_id_manager.cc index daa5630..6951b2c 100644 --- a/quic/core/uber_quic_stream_id_manager.cc +++ b/quic/core/uber_quic_stream_id_manager.cc
@@ -20,14 +20,14 @@ : bidirectional_stream_id_manager_(delegate, /*unidirectional=*/false, perspective, - version.transport_version, + version, max_open_outgoing_bidirectional_streams, max_open_incoming_bidirectional_streams), unidirectional_stream_id_manager_( delegate, /*unidirectional=*/true, perspective, - version.transport_version, + version, max_open_outgoing_unidirectional_streams, max_open_incoming_unidirectional_streams) {} @@ -96,13 +96,6 @@ error_details); } -bool UberQuicStreamIdManager::IsIncomingStream(QuicStreamId id) const { - if (QuicUtils::IsBidirectionalStreamId(id)) { - return bidirectional_stream_id_manager_.IsIncomingStream(id); - } - return unidirectional_stream_id_manager_.IsIncomingStream(id); -} - bool UberQuicStreamIdManager::IsAvailableStream(QuicStreamId id) const { if (QuicUtils::IsBidirectionalStreamId(id)) { return bidirectional_stream_id_manager_.IsAvailableStream(id);
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h index d3f374a..b1fc126 100644 --- a/quic/core/uber_quic_stream_id_manager.h +++ b/quic/core/uber_quic_stream_id_manager.h
@@ -66,9 +66,6 @@ bool OnStreamsBlockedFrame(const QuicStreamsBlockedFrame& frame, std::string* error_details); - // Return true if |id| is peer initiated. - bool IsIncomingStream(QuicStreamId id) const; - // Returns true if |id| is still available. bool IsAvailableStream(QuicStreamId id) const;
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc index 1590f6e..6fe8363 100644 --- a/quic/core/uber_quic_stream_id_manager_test.cc +++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -300,17 +300,6 @@ EXPECT_TRUE(manager_.OnStreamsBlockedFrame(frame, nullptr)); } -TEST_P(UberQuicStreamIdManagerTest, IsIncomingStream) { - EXPECT_TRUE( - manager_.IsIncomingStream(GetNthPeerInitiatedBidirectionalStreamId(0))); - EXPECT_TRUE( - manager_.IsIncomingStream(GetNthPeerInitiatedUnidirectionalStreamId(0))); - EXPECT_FALSE( - manager_.IsIncomingStream(GetNthSelfInitiatedBidirectionalStreamId(0))); - EXPECT_FALSE( - manager_.IsIncomingStream(GetNthSelfInitiatedUnidirectionalStreamId(0))); -} - TEST_P(UberQuicStreamIdManagerTest, SetMaxOpenOutgoingStreamsPlusFrame) { const size_t kNumMaxOutgoingStream = 123; // Set the uni- and bi- directional limits to different values to ensure