Remove QuicSession from LegacyQuicStreamIdManager and pass in transport_version and perspective instead.
gfe-relnote: n/a (Refactor)
PiperOrigin-RevId: 284747835
Change-Id: I5baf02f87cf72cdc02d6b8ddbd6befcccca199f3
diff --git a/quic/core/legacy_quic_stream_id_manager.cc b/quic/core/legacy_quic_stream_id_manager.cc
index 157e375..b8498b3 100644
--- a/quic/core/legacy_quic_stream_id_manager.cc
+++ b/quic/core/legacy_quic_stream_id_manager.cc
@@ -4,47 +4,33 @@
#include "net/third_party/quiche/src/quic/core/legacy_quic_stream_id_manager.h"
#include "net/third_party/quiche/src/quic/core/quic_session.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.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_map_util.h"
namespace quic {
-#define ENDPOINT \
- (session_->perspective() == Perspective::IS_SERVER ? "Server: " : "Client: ")
-
LegacyQuicStreamIdManager::LegacyQuicStreamIdManager(
- QuicSession* session,
+ Perspective perspective,
+ QuicTransportVersion transport_version,
size_t max_open_outgoing_streams,
size_t max_open_incoming_streams)
- : session_(session),
+ : perspective_(perspective),
+ transport_version_(transport_version),
max_open_outgoing_streams_(max_open_outgoing_streams),
max_open_incoming_streams_(max_open_incoming_streams),
next_outgoing_stream_id_(
- QuicUtils::GetFirstBidirectionalStreamId(session->transport_version(),
- session->perspective())),
+ QuicUtils::GetFirstBidirectionalStreamId(transport_version_,
+ perspective_)),
largest_peer_created_stream_id_(
- session->perspective() == Perspective::IS_SERVER
- ? (QuicVersionUsesCryptoFrames(session->transport_version())
- ? QuicUtils::GetInvalidStreamId(
- session->transport_version())
- : QuicUtils::GetCryptoStreamId(
- session->transport_version()))
- : QuicUtils::GetInvalidStreamId(session->transport_version())) {}
+ perspective_ == Perspective::IS_SERVER
+ ? (QuicVersionUsesCryptoFrames(transport_version_)
+ ? QuicUtils::GetInvalidStreamId(transport_version_)
+ : QuicUtils::GetCryptoStreamId(transport_version_))
+ : QuicUtils::GetInvalidStreamId(transport_version_)) {}
-LegacyQuicStreamIdManager::~LegacyQuicStreamIdManager() {
- QUIC_LOG_IF(WARNING,
- session_->num_locally_closed_incoming_streams_highest_offset() >
- max_open_incoming_streams_)
- << "Surprisingly high number of locally closed peer initiated streams"
- "still waiting for final byte offset: "
- << session_->num_locally_closed_incoming_streams_highest_offset();
- QUIC_LOG_IF(WARNING,
- session_->GetNumLocallyClosedOutgoingStreamsHighestOffset() >
- max_open_outgoing_streams_)
- << "Surprisingly high number of locally closed self initiated streams"
- "still waiting for final byte offset: "
- << session_->GetNumLocallyClosedOutgoingStreamsHighestOffset();
-}
+LegacyQuicStreamIdManager::~LegacyQuicStreamIdManager() {}
bool LegacyQuicStreamIdManager::CanOpenNextOutgoingStream(
size_t current_num_open_outgoing_streams) const {
@@ -69,7 +55,7 @@
available_streams_.erase(stream_id);
if (largest_peer_created_stream_id_ !=
- QuicUtils::GetInvalidStreamId(session_->transport_version()) &&
+ QuicUtils::GetInvalidStreamId(transport_version_) &&
stream_id <= largest_peer_created_stream_id_) {
return true;
}
@@ -80,31 +66,26 @@
size_t additional_available_streams =
(stream_id - largest_peer_created_stream_id_) / 2 - 1;
if (largest_peer_created_stream_id_ ==
- QuicUtils::GetInvalidStreamId(session_->transport_version())) {
+ QuicUtils::GetInvalidStreamId(transport_version_)) {
additional_available_streams = (stream_id + 1) / 2 - 1;
}
size_t new_num_available_streams =
GetNumAvailableStreams() + additional_available_streams;
if (new_num_available_streams > MaxAvailableStreams()) {
- QUIC_DLOG(INFO) << ENDPOINT
+ QUIC_DLOG(INFO) << perspective_
<< "Failed to create a new incoming stream with id:"
<< stream_id << ". There are already "
<< GetNumAvailableStreams()
<< " streams available, which would become "
<< new_num_available_streams << ", which exceeds the limit "
<< MaxAvailableStreams() << ".";
- session_->connection()->CloseConnection(
- QUIC_TOO_MANY_AVAILABLE_STREAMS,
- QuicStrCat(new_num_available_streams, " above ", MaxAvailableStreams()),
- ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
return false;
}
QuicStreamId first_available_stream = largest_peer_created_stream_id_ + 2;
if (largest_peer_created_stream_id_ ==
- QuicUtils::GetInvalidStreamId(session_->transport_version())) {
+ QuicUtils::GetInvalidStreamId(transport_version_)) {
first_available_stream = QuicUtils::GetFirstBidirectionalStreamId(
- session_->transport_version(),
- QuicUtils::InvertPerspective(session_->perspective()));
+ transport_version_, QuicUtils::InvertPerspective(perspective_));
}
for (QuicStreamId id = first_available_stream; id < stream_id; id += 2) {
available_streams_.insert(id);
@@ -128,7 +109,7 @@
}
// For peer created streams, we also need to consider available streams.
return largest_peer_created_stream_id_ ==
- QuicUtils::GetInvalidStreamId(session_->transport_version()) ||
+ QuicUtils::GetInvalidStreamId(transport_version_) ||
id > largest_peer_created_stream_id_ ||
QuicContainsKey(available_streams_, id);
}
diff --git a/quic/core/legacy_quic_stream_id_manager.h b/quic/core/legacy_quic_stream_id_manager.h
index 78d4545..1a94905 100644
--- a/quic/core/legacy_quic_stream_id_manager.h
+++ b/quic/core/legacy_quic_stream_id_manager.h
@@ -5,6 +5,8 @@
#define QUICHE_QUIC_CORE_LEGACY_QUIC_STREAM_ID_MANAGER_H_
#include "net/third_party/quiche/src/quic/core/quic_stream_id_manager.h"
+#include "net/third_party/quiche/src/quic/core/quic_types.h"
+#include "net/third_party/quiche/src/quic/core/quic_versions.h"
namespace quic {
@@ -19,7 +21,8 @@
// next outgoing stream ID) and 2) can a new incoming stream be opened.
class QUIC_EXPORT_PRIVATE LegacyQuicStreamIdManager {
public:
- LegacyQuicStreamIdManager(QuicSession* session,
+ LegacyQuicStreamIdManager(Perspective perspective,
+ QuicTransportVersion transport_version,
size_t max_open_outgoing_streams,
size_t max_open_incoming_streams);
@@ -32,6 +35,8 @@
// Returns true if a new incoming stream can be opened.
bool CanOpenIncomingStream(size_t current_num_open_incoming_streams) const;
+ // Returns false when increasing the largest created stream id to |id| would
+ // violate the limit, so the connection should be closed.
bool MaybeIncreaseLargestPeerStreamId(const QuicStreamId id);
// Returns true if |id| is still available.
@@ -75,13 +80,13 @@
return largest_peer_created_stream_id_;
}
+ size_t GetNumAvailableStreams() const;
+
private:
friend class test::QuicSessionPeer;
- size_t GetNumAvailableStreams() const;
-
- // Not owned.
- QuicSession* session_;
+ const Perspective perspective_;
+ const QuicTransportVersion transport_version_;
// The maximum number of outgoing streams this connection can open.
size_t max_open_outgoing_streams_;
diff --git a/quic/core/legacy_quic_stream_id_manager_test.cc b/quic/core/legacy_quic_stream_id_manager_test.cc
index 6a861a5..0270774 100644
--- a/quic/core/legacy_quic_stream_id_manager_test.cc
+++ b/quic/core/legacy_quic_stream_id_manager_test.cc
@@ -7,6 +7,7 @@
#include <utility>
#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_test.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_session_peer.h"
#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h"
@@ -19,64 +20,64 @@
using testing::StrictMock;
class LegacyQuicStreamIdManagerTest : public QuicTest {
- protected:
- void Initialize(Perspective perspective) {
+ public:
+ LegacyQuicStreamIdManagerTest(Perspective perspective,
+ QuicTransportVersion transport_version)
+ : transport_version_(transport_version),
+ manager_(perspective,
+ transport_version,
+ kDefaultMaxStreamsPerConnection,
+ kDefaultMaxStreamsPerConnection) {
// LegacyQuicStreamIdManager is only used for versions < 99.
// TODO(b/145768765) parameterize this test by version.
SetQuicReloadableFlag(quic_enable_version_q099, false);
SetQuicReloadableFlag(quic_enable_version_t099, false);
- connection_ = new MockQuicConnection(
- &helper_, &alarm_factory_, perspective,
- ParsedVersionOfIndex(CurrentSupportedVersions(), 0));
- session_ = std::make_unique<StrictMock<MockQuicSession>>(connection_);
- manager_ = QuicSessionPeer::GetStreamIdManager(session_.get());
- session_->Initialize();
}
+ protected:
QuicStreamId GetNthClientInitiatedId(int n) {
- return QuicUtils::GetFirstBidirectionalStreamId(
- connection_->transport_version(), Perspective::IS_CLIENT) +
+ return QuicUtils::GetFirstBidirectionalStreamId(transport_version_,
+ Perspective::IS_CLIENT) +
2 * n;
}
QuicStreamId GetNthServerInitiatedId(int n) { return 2 + 2 * n; }
- MockQuicConnectionHelper helper_;
- MockAlarmFactory alarm_factory_;
- MockQuicConnection* connection_;
- std::unique_ptr<StrictMock<MockQuicSession>> session_;
- LegacyQuicStreamIdManager* manager_;
+ QuicTransportVersion transport_version_;
+ LegacyQuicStreamIdManager manager_;
};
class LegacyQuicStreamIdManagerTestServer
: public LegacyQuicStreamIdManagerTest {
protected:
- LegacyQuicStreamIdManagerTestServer() { Initialize(Perspective::IS_SERVER); }
+ LegacyQuicStreamIdManagerTestServer()
+ : LegacyQuicStreamIdManagerTest(Perspective::IS_SERVER,
+ QuicTransportVersion::QUIC_VERSION_46) {}
};
TEST_F(LegacyQuicStreamIdManagerTestServer, CanOpenNextOutgoingStream) {
- EXPECT_TRUE(manager_->CanOpenNextOutgoingStream(
- manager_->max_open_outgoing_streams() - 1));
- EXPECT_FALSE(manager_->CanOpenNextOutgoingStream(
- manager_->max_open_outgoing_streams()));
+ EXPECT_TRUE(manager_.CanOpenNextOutgoingStream(
+ manager_.max_open_outgoing_streams() - 1));
+ EXPECT_FALSE(
+ manager_.CanOpenNextOutgoingStream(manager_.max_open_outgoing_streams()));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, CanOpenIncomingStream) {
- EXPECT_TRUE(manager_->CanOpenIncomingStream(
- manager_->max_open_incoming_streams() - 1));
+ EXPECT_TRUE(
+ manager_.CanOpenIncomingStream(manager_.max_open_incoming_streams() - 1));
EXPECT_FALSE(
- manager_->CanOpenIncomingStream(manager_->max_open_incoming_streams()));
+ manager_.CanOpenIncomingStream(manager_.max_open_incoming_streams()));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, AvailableStreams) {
ASSERT_TRUE(
- manager_->MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(3)));
- EXPECT_TRUE(manager_->IsAvailableStream(GetNthClientInitiatedId(1)));
- EXPECT_TRUE(manager_->IsAvailableStream(GetNthClientInitiatedId(2)));
+ manager_.MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(3)));
+ EXPECT_TRUE(manager_.IsAvailableStream(GetNthClientInitiatedId(1)));
+ EXPECT_TRUE(manager_.IsAvailableStream(GetNthClientInitiatedId(2)));
ASSERT_TRUE(
- manager_->MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(2)));
+ manager_.MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(2)));
ASSERT_TRUE(
- manager_->MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(1)));
+ manager_.MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(1)));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, MaxAvailableStreams) {
@@ -84,17 +85,16 @@
// streams available. The server accepts slightly more than the negotiated
// stream limit to deal with rare cases where a client FIN/RST is lost.
const size_t kMaxStreamsForTest = 10;
- session_->OnConfigNegotiated();
- const size_t kAvailableStreamLimit = manager_->MaxAvailableStreams();
+ const size_t kAvailableStreamLimit = manager_.MaxAvailableStreams();
EXPECT_EQ(
- manager_->max_open_incoming_streams() * kMaxAvailableStreamsMultiplier,
- manager_->MaxAvailableStreams());
+ manager_.max_open_incoming_streams() * kMaxAvailableStreamsMultiplier,
+ manager_.MaxAvailableStreams());
// The protocol specification requires that there can be at least 10 times
// as many available streams as the connection's maximum open streams.
EXPECT_LE(10 * kMaxStreamsForTest, kAvailableStreamLimit);
EXPECT_TRUE(
- manager_->MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(0)));
+ manager_.MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(0)));
// Establish available streams up to the server's limit.
const int kLimitingStreamId =
@@ -102,85 +102,66 @@
// This exceeds the stream limit. In versions other than 99
// this is allowed. Version 99 hews to the IETF spec and does
// not allow it.
- EXPECT_TRUE(manager_->MaybeIncreaseLargestPeerStreamId(kLimitingStreamId));
- // A further available stream will result in connection close.
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_TOO_MANY_AVAILABLE_STREAMS, _, _));
+ EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(kLimitingStreamId));
// This forces stream kLimitingStreamId + 2 to become available, which
// violates the quota.
EXPECT_FALSE(
- manager_->MaybeIncreaseLargestPeerStreamId(kLimitingStreamId + 2 * 2));
+ manager_.MaybeIncreaseLargestPeerStreamId(kLimitingStreamId + 2 * 2));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, MaximumAvailableOpenedStreams) {
QuicStreamId stream_id = GetNthClientInitiatedId(0);
- EXPECT_TRUE(manager_->MaybeIncreaseLargestPeerStreamId(stream_id));
+ EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(stream_id));
- EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0);
- EXPECT_TRUE(manager_->MaybeIncreaseLargestPeerStreamId(
- stream_id + 2 * (manager_->max_open_incoming_streams() - 1)));
+ EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(
+ stream_id + 2 * (manager_.max_open_incoming_streams() - 1)));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, TooManyAvailableStreams) {
QuicStreamId stream_id = GetNthClientInitiatedId(0);
- EXPECT_TRUE(manager_->MaybeIncreaseLargestPeerStreamId(stream_id));
+ EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(stream_id));
// A stream ID which is too large to create.
QuicStreamId stream_id2 =
- GetNthClientInitiatedId(2 * manager_->MaxAvailableStreams() + 4);
- EXPECT_CALL(*connection_,
- CloseConnection(QUIC_TOO_MANY_AVAILABLE_STREAMS, _, _));
- EXPECT_FALSE(manager_->MaybeIncreaseLargestPeerStreamId(stream_id2));
+ GetNthClientInitiatedId(2 * manager_.MaxAvailableStreams() + 4);
+ EXPECT_FALSE(manager_.MaybeIncreaseLargestPeerStreamId(stream_id2));
}
TEST_F(LegacyQuicStreamIdManagerTestServer, ManyAvailableStreams) {
// When max_open_streams_ is 200, should be able to create 200 streams
// out-of-order, that is, creating the one with the largest stream ID first.
- manager_->set_max_open_incoming_streams(200);
+ manager_.set_max_open_incoming_streams(200);
QuicStreamId stream_id = GetNthClientInitiatedId(0);
- EXPECT_TRUE(manager_->MaybeIncreaseLargestPeerStreamId(stream_id));
- EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0);
+ EXPECT_TRUE(manager_.MaybeIncreaseLargestPeerStreamId(stream_id));
// Create the largest stream ID of a threatened total of 200 streams.
// GetNth... starts at 0, so for 200 streams, get the 199th.
EXPECT_TRUE(
- manager_->MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(199)));
+ manager_.MaybeIncreaseLargestPeerStreamId(GetNthClientInitiatedId(199)));
}
TEST_F(LegacyQuicStreamIdManagerTestServer,
TestMaxIncomingAndOutgoingStreamsAllowed) {
- // Tests that on server side, the value of max_open_incoming/outgoing
- // streams are setup correctly during negotiation. The value for outgoing
- // stream is limited to negotiated value and for incoming stream it is set to
- // be larger than that.
- session_->OnConfigNegotiated();
- // The max number of open outgoing streams is less than that of incoming
- // streams, and it should be same as negotiated value.
- EXPECT_LT(manager_->max_open_outgoing_streams(),
- manager_->max_open_incoming_streams());
- EXPECT_EQ(manager_->max_open_outgoing_streams(),
+ EXPECT_EQ(manager_.max_open_incoming_streams(),
kDefaultMaxStreamsPerConnection);
- EXPECT_GT(manager_->max_open_incoming_streams(),
+ EXPECT_EQ(manager_.max_open_outgoing_streams(),
kDefaultMaxStreamsPerConnection);
}
class LegacyQuicStreamIdManagerTestClient
: public LegacyQuicStreamIdManagerTest {
protected:
- LegacyQuicStreamIdManagerTestClient() { Initialize(Perspective::IS_CLIENT); }
+ LegacyQuicStreamIdManagerTestClient()
+ : LegacyQuicStreamIdManagerTest(Perspective::IS_CLIENT,
+ QuicTransportVersion::QUIC_VERSION_46) {}
};
TEST_F(LegacyQuicStreamIdManagerTestClient,
TestMaxIncomingAndOutgoingStreamsAllowed) {
- // Tests that on client side, the value of max_open_incoming/outgoing
- // streams are setup correctly during negotiation. When flag is true, the
- // value for outgoing stream is limited to negotiated value and for incoming
- // stream it is set to be larger than that.
- session_->OnConfigNegotiated();
- EXPECT_LT(manager_->max_open_outgoing_streams(),
- manager_->max_open_incoming_streams());
- EXPECT_EQ(manager_->max_open_outgoing_streams(),
+ EXPECT_EQ(manager_.max_open_incoming_streams(),
+ kDefaultMaxStreamsPerConnection);
+ EXPECT_EQ(manager_.max_open_outgoing_streams(),
kDefaultMaxStreamsPerConnection);
}
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index 6381bf2..c0f5992 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -59,7 +59,8 @@
visitor_(owner),
write_blocked_streams_(connection->transport_version()),
config_(config),
- stream_id_manager_(this,
+ stream_id_manager_(perspective(),
+ connection->transport_version(),
kDefaultMaxStreamsPerConnection,
config_.GetMaxIncomingBidirectionalStreamsToSend()),
v99_streamid_manager_(
@@ -131,6 +132,16 @@
QuicSession::~QuicSession() {
QUIC_LOG_IF(WARNING, !zombie_streams_.empty()) << "Still have zombie streams";
+ QUIC_LOG_IF(WARNING, num_locally_closed_incoming_streams_highest_offset() >
+ stream_id_manager_.max_open_incoming_streams())
+ << "Surprisingly high number of locally closed peer initiated streams"
+ "still waiting for final byte offset: "
+ << num_locally_closed_incoming_streams_highest_offset();
+ QUIC_LOG_IF(WARNING, GetNumLocallyClosedOutgoingStreamsHighestOffset() >
+ stream_id_manager_.max_open_outgoing_streams())
+ << "Surprisingly high number of locally closed self initiated streams"
+ "still waiting for final byte offset: "
+ << GetNumLocallyClosedOutgoingStreamsHighestOffset();
}
void QuicSession::PendingStreamOnStreamFrame(const QuicStreamFrame& frame) {
@@ -1551,7 +1562,15 @@
if (VersionHasIetfQuicFrames(transport_version())) {
return v99_streamid_manager_.MaybeIncreaseLargestPeerStreamId(stream_id);
}
- return stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id);
+ if (!stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)) {
+ connection()->CloseConnection(
+ QUIC_TOO_MANY_AVAILABLE_STREAMS,
+ QuicStrCat(stream_id, " exceeds available streams ",
+ stream_id_manager_.MaxAvailableStreams()),
+ ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET);
+ return false;
+ }
+ return true;
}
bool QuicSession::ShouldYield(QuicStreamId stream_id) {
