gfe-relnote: Decouple the QuicSession from the QuicStreamIdManager by introducing a QuicStreamIdManager::DelegateInterface which the session implements. Does not change behavior, merely adds a layer of abstraction which allows unit tests of QuicStreamIdManager to be simplified. PiperOrigin-RevId: 270302454 Change-Id: I4a66803d4a2f1f640ef68ee6373af9b67423bac4
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index abeed2b..3bd13bc 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -310,6 +310,8 @@ Http3DebugVisitor::~Http3DebugVisitor() {} +// Expected unidirectional static streams Requirement can be found at +// https://tools.ietf.org/html/draft-ietf-quic-http-22#section-6.2. QuicSpdySession::QuicSpdySession( QuicConnection* connection, QuicSession::Visitor* visitor,
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index 23873d0..9d0c7bf 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -62,6 +62,7 @@ config_.GetMaxIncomingBidirectionalStreamsToSend()), v99_streamid_manager_( this, + num_expected_unidirectional_static_streams, kDefaultMaxStreamsPerConnection, kDefaultMaxStreamsPerConnection, config_.GetMaxIncomingBidirectionalStreamsToSend(), @@ -771,6 +772,12 @@ control_frame_manager_.WriteOrBufferWindowUpdate(id, byte_offset); } +void QuicSession::OnError(QuicErrorCode error_code, std::string error_details) { + connection_->CloseConnection( + error_code, error_details, + ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); +} + void QuicSession::SendMaxStreams(QuicStreamCount stream_count, bool unidirectional) { control_frame_manager_.WriteOrBufferMaxStreams(stream_count, unidirectional); @@ -1049,6 +1056,7 @@ // STREAMS_BLOCKED or MAX_STREAMS frames against the config and either send // the frames or discard them. if (VersionHasIetfQuicFrames(connection_->transport_version())) { + QuicConnection::ScopedPacketFlusher flusher(connection()); v99_streamid_manager_.OnConfigNegotiated(); } }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 6fb4c8d..758106f 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -41,9 +41,11 @@ class QuicSessionPeer; } // namespace test -class QUIC_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface, - public SessionNotifierInterface, - public QuicStreamFrameDataProducer { +class QUIC_EXPORT_PRIVATE QuicSession + : public QuicConnectionVisitorInterface, + public SessionNotifierInterface, + public QuicStreamFrameDataProducer, + public QuicStreamIdManager::DelegateInterface { public: // An interface from the session to the entity owning the session. // This lets the session notify its owner (the Dispatcher) when the connection @@ -147,6 +149,16 @@ bool HasUnackedCryptoData() const override; bool HasUnackedStreamData() const override; + // QuicStreamIdManager::DelegateInterface methods: + void OnError(QuicErrorCode error_code, std::string error_details) override; + void SendMaxStreams(QuicStreamCount stream_count, + bool unidirectional) override; + void SendStreamsBlocked(QuicStreamCount stream_count, + bool unidirectional) override; + // The default implementation does nothing. Subclasses should override if + // for example they queue up stream requests. + void OnCanCreateNewOutgoingStream(bool unidirectional) override; + // Called on every incoming packet. Passes |packet| through to |connection_|. virtual void ProcessUdpPacket(const QuicSocketAddress& self_address, const QuicSocketAddress& peer_address, @@ -210,12 +222,6 @@ // Sends a WINDOW_UPDATE frame. virtual void SendWindowUpdate(QuicStreamId id, QuicStreamOffset byte_offset); - // Send a MAX_STREAMS frame. - void SendMaxStreams(QuicStreamCount stream_count, bool unidirectional); - - // Send a STREAMS_BLOCKED frame. - void SendStreamsBlocked(QuicStreamCount stream_count, bool unidirectional); - // Create and transmit a STOP_SENDING frame virtual void SendStopSending(uint16_t code, QuicStreamId stream_id); @@ -404,14 +410,6 @@ return supported_versions_; } - // Called when new outgoing streams are available to be opened. This occurs - // when an extant, open, stream is moved to draining or closed. The default - // implementation does nothing. |unidirectional| indicates whether - // unidirectional or bidirectional streams are now available. If both become - // available at the same time then there will be two calls to this method, one - // with unidirectional==true, the other with it ==false. - virtual void OnCanCreateNewOutgoingStream(bool unidirectional); - QuicStreamId next_outgoing_bidirectional_stream_id() const; QuicStreamId next_outgoing_unidirectional_stream_id() const;
diff --git a/quic/core/quic_stream_id_manager.cc b/quic/core/quic_stream_id_manager.cc index 9d94157..57fd0a4 100644 --- a/quic/core/quic_stream_id_manager.cc +++ b/quic/core/quic_stream_id_manager.cc
@@ -16,17 +16,23 @@ namespace quic { -#define ENDPOINT \ - (session_->perspective() == Perspective::IS_SERVER ? " Server: " \ - : " Client: ") +#define ENDPOINT \ + (perspective_ == Perspective::IS_SERVER ? " Server: " : " Client: ") QuicStreamIdManager::QuicStreamIdManager( - QuicSession* session, + DelegateInterface* delegate, bool unidirectional, + Perspective perspective, + QuicTransportVersion transport_version, + QuicStreamCount num_expected_static_streams, QuicStreamCount max_allowed_outgoing_streams, QuicStreamCount max_allowed_incoming_streams) - : session_(session), + : delegate_(delegate), unidirectional_(unidirectional), + perspective_(perspective), + transport_version_(transport_version), + num_expected_static_streams_(num_expected_static_streams), + is_config_negotiated_(false), outgoing_max_streams_(max_allowed_outgoing_streams), next_outgoing_stream_id_(GetFirstOutgoingStreamId()), outgoing_stream_count_(0), @@ -38,16 +44,15 @@ incoming_initial_max_open_streams_(max_allowed_incoming_streams), incoming_stream_count_(0), largest_peer_created_stream_id_( - QuicUtils::GetInvalidStreamId(transport_version())), + QuicUtils::GetInvalidStreamId(transport_version)), max_streams_window_(0), pending_max_streams_(false), pending_streams_blocked_( - QuicUtils::GetInvalidStreamId(transport_version())) { + QuicUtils::GetInvalidStreamId(transport_version)) { CalculateIncomingMaxStreamsWindow(); } -QuicStreamIdManager::~QuicStreamIdManager() { -} +QuicStreamIdManager::~QuicStreamIdManager() {} bool QuicStreamIdManager::OnMaxStreamsFrame(const QuicMaxStreamsFrame& frame) { // Ensure that the frame has the correct directionality. @@ -74,9 +79,8 @@ // TODO(fkastenholz): revise when proper IETF Connection Close support is // done. QUIC_CODE_COUNT(quic_streams_blocked_too_big); - session_->connection()->CloseConnection( - QUIC_STREAMS_BLOCKED_ERROR, "Invalid stream count specified", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + delegate_->OnError(QUIC_STREAMS_BLOCKED_ERROR, + "Invalid stream count specified"); return false; } if (frame.stream_count < incoming_actual_max_streams_) { @@ -92,17 +96,14 @@ // Used when configuration has been done and we have an initial // maximum stream count from the peer. bool QuicStreamIdManager::SetMaxOpenOutgoingStreams(size_t max_open_streams) { - if (unidirectional_ && - max_open_streams < - session_->num_expected_unidirectional_static_streams()) { - // Requirement can be found at - // https://tools.ietf.org/html/draft-ietf-quic-http-22#section-6.2. - QUIC_DLOG(ERROR) << "Received max unidirectional stream " - << max_open_streams << " < " - << session_->num_expected_unidirectional_static_streams(); - session_->connection()->CloseConnection( - QUIC_MAX_STREAMS_ERROR, "New unidirectional stream limit is too low.", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + if (max_open_streams < num_expected_static_streams_) { + QUIC_DLOG(ERROR) << "Received max streams " << max_open_streams << " < " + << num_expected_static_streams_; + delegate_->OnError(QUIC_MAX_STREAMS_ERROR, + unidirectional_ + ? "New unidirectional stream limit is too low." + : "New bidirectional stream limit is too low."); + return false; } if (using_default_max_streams_) { @@ -114,10 +115,8 @@ // be less than the number of existing outgoing streams. If that happens, // close the connection. if (max_open_streams < outgoing_stream_count_) { - session_->connection()->CloseConnection( - QUIC_MAX_STREAMS_ERROR, - "Stream limit less than existing stream count", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + delegate_->OnError(QUIC_MAX_STREAMS_ERROR, + "Stream limit less than existing stream count"); return false; } using_default_max_streams_ = false; @@ -133,11 +132,11 @@ // if it would exceed the max 32 bits can express. outgoing_max_streams_ = std::min<size_t>( max_open_streams, - QuicUtils::GetMaxStreamCount(unidirectional_, session_->perspective())); + QuicUtils::GetMaxStreamCount(unidirectional_, perspective_)); // Inform the higher layers that the stream limit has increased and that // new streams may be created. - session_->OnCanCreateNewOutgoingStream(unidirectional_); + delegate_->OnCanCreateNewOutgoingStream(unidirectional_); return true; } @@ -148,9 +147,8 @@ QuicStreamCount new_max = std::min( implementation_max, static_cast<QuicStreamCount>(max_open_streams)); if (new_max < incoming_stream_count_) { - session_->connection()->CloseConnection( - QUIC_MAX_STREAMS_ERROR, "Stream limit less than existing stream count", - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + delegate_->OnError(QUIC_MAX_STREAMS_ERROR, + "Stream limit less than existing stream count"); return; } incoming_actual_max_streams_ = new_max; @@ -170,15 +168,16 @@ } void QuicStreamIdManager::SendMaxStreamsFrame() { - if (!session_->is_configured()) { - // Session not configured, so we can not send the MAX STREAMS frame yet. - // Remember that we would have sent one and then return. A new frame will - // be generated once the configuration is received. + if (!is_config_negotiated_) { + // The config has not yet been negotiated, so we can not send the + // MAX STREAMS frame yet. Record that we would have sent one and then + // return. A new frame will be generated once the configuration is + // received. pending_max_streams_ = true; return; } incoming_advertised_max_streams_ = incoming_actual_max_streams_; - session_->SendMaxStreams(incoming_advertised_max_streams_, unidirectional_); + delegate_->SendMaxStreams(incoming_advertised_max_streams_, unidirectional_); } void QuicStreamIdManager::OnStreamClosed(QuicStreamId stream_id) { @@ -220,15 +219,14 @@ } // Next stream ID would exceed the limit, need to inform the peer. - if (!session_->is_configured()) { - // Session not configured, so we can not send the STREAMS_BLOCKED frame yet. - // Remember that we would have sent one, and what the limit was when we were - // blocked, and return. + if (!is_config_negotiated_) { + // The config is not negotiated, so we can not send the STREAMS_BLOCKED + // frame yet. Record that we would have sent one, and what the limit was + // when we were blocked, and return. pending_streams_blocked_ = outgoing_max_streams_; return false; } - - session_->SendStreamsBlocked(outgoing_max_streams_, unidirectional_); + delegate_->SendStreamsBlocked(outgoing_max_streams_, unidirectional_); QUIC_CODE_COUNT(quic_reached_outgoing_stream_id_limit); return false; } @@ -273,11 +271,10 @@ << "Failed to create a new incoming stream with id:" << stream_id << ", reaching MAX_STREAMS limit: " << incoming_advertised_max_streams_ << "."; - session_->connection()->CloseConnection( + delegate_->OnError( QUIC_INVALID_STREAM_ID, QuicStrCat("Stream id ", stream_id, " would exceed stream count limit ", - incoming_advertised_max_streams_), - ConnectionCloseBehavior::SEND_CONNECTION_CLOSE_PACKET); + incoming_advertised_max_streams_)); return false; } @@ -335,7 +332,7 @@ } Perspective QuicStreamIdManager::perspective() const { - return session_->perspective(); + return perspective_; } Perspective QuicStreamIdManager::peer_perspective() const { @@ -343,7 +340,7 @@ } QuicTransportVersion QuicStreamIdManager::transport_version() const { - return session_->transport_version(); + return transport_version_; } size_t QuicStreamIdManager::available_incoming_streams() { @@ -358,7 +355,7 @@ } void QuicStreamIdManager::OnConfigNegotiated() { - QuicConnection::ScopedPacketFlusher flusher(session_->connection()); + is_config_negotiated_ = true; // If a STREAMS_BLOCKED or MAX_STREAMS is pending, send it and clear // the pending state. if (pending_streams_blocked_ != @@ -366,7 +363,7 @@ if (pending_streams_blocked_ >= outgoing_max_streams_) { // There is a pending STREAMS_BLOCKED frame and the current limit does not // let new streams be formed. Regenerate and send the frame. - session_->SendStreamsBlocked(outgoing_max_streams_, unidirectional_); + delegate_->SendStreamsBlocked(outgoing_max_streams_, unidirectional_); } pending_streams_blocked_ = QuicUtils::GetInvalidStreamId(transport_version());
diff --git a/quic/core/quic_stream_id_manager.h b/quic/core/quic_stream_id_manager.h index 51524ba..1ac2947 100644 --- a/quic/core/quic_stream_id_manager.h +++ b/quic/core/quic_stream_id_manager.h
@@ -17,8 +17,6 @@ class QuicStreamIdManagerPeer; } // namespace test -class QuicSession; - // Amount to increment a stream ID value to get the next stream ID in // the stream ID space. const QuicStreamId kV99StreamIdIncrement = 4; @@ -31,8 +29,36 @@ // This class manages the stream ids for Version 99/IETF QUIC. class QUIC_EXPORT_PRIVATE QuicStreamIdManager { public: - QuicStreamIdManager(QuicSession* session, + class DelegateInterface { + public: + virtual ~DelegateInterface() = default; + + // Called when new outgoing streams are available to be opened. This occurs + // when an extant, open, stream is moved to draining or closed. + // |unidirectional| indicates whether unidirectional or bidirectional + // streams are now available. If both become available at the same time then + // there will be two calls to this method, one with unidirectional==true, + // the other with it ==false. + virtual void OnCanCreateNewOutgoingStream(bool unidirectional) = 0; + + // Closes the connection when an error is encountered. + virtual void OnError(QuicErrorCode error_code, + std::string error_details) = 0; + + // Send a MAX_STREAMS frame. + virtual void SendMaxStreams(QuicStreamCount stream_count, + bool unidirectional) = 0; + + // Send a STREAMS_BLOCKED frame. + virtual void SendStreamsBlocked(QuicStreamCount stream_count, + bool unidirectional) = 0; + }; + + QuicStreamIdManager(DelegateInterface* delegate, bool unidirectional, + Perspective perspective, + QuicTransportVersion transport_version, + QuicStreamCount num_expected_static_streams, QuicStreamCount max_allowed_outgoing_streams, QuicStreamCount max_allowed_incoming_streams); @@ -171,11 +197,23 @@ // Back reference to the session containing this Stream ID Manager. // needed to access various session methods, such as perspective() - QuicSession* session_; + DelegateInterface* delegate_; // Whether this stream id manager is for unidrectional (true) or bidirectional // (false) streams. - bool unidirectional_; + const bool unidirectional_; + + // Is this manager a client or a server. + const Perspective perspective_; + + // Transport version used for this manager. + const QuicTransportVersion transport_version_; + + // Number of expected static streams. + const QuicStreamCount num_expected_static_streams_; + + // True if the config has been negotiated_; + bool is_config_negotiated_; // This is the number of streams that this node can initiate. // This limit is:
diff --git a/quic/core/quic_stream_id_manager_test.cc b/quic/core/quic_stream_id_manager_test.cc index 7c1233b..748a10c 100644 --- a/quic/core/quic_stream_id_manager_test.cc +++ b/quic/core/quic_stream_id_manager_test.cc
@@ -4,150 +4,68 @@ #include "net/third_party/quiche/src/quic/core/quic_stream_id_manager.h" #include <cstdint> -#include <set> #include <string> #include <utility> -#include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h" -#include "net/third_party/quiche/src/quic/core/crypto/null_encrypter.h" -#include "net/third_party/quiche/src/quic/core/quic_crypto_stream.h" -#include "net/third_party/quiche/src/quic/core/quic_data_writer.h" -#include "net/third_party/quiche/src/quic/core/quic_packets.h" -#include "net/third_party/quiche/src/quic/core/quic_session.h" -#include "net/third_party/quiche/src/quic/core/quic_stream.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" #include "net/third_party/quiche/src/quic/platform/api/quic_expect_bug.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_map_util.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_str_cat.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" #include "net/third_party/quiche/src/quic/platform/api/quic_test.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_test_mem_slice_vector.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_config_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_connection_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_flow_controller_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_session_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_stream_id_manager_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_stream_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_stream_send_buffer_peer.h" -#include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" using testing::_; using testing::Invoke; -using testing::Return; using testing::StrictMock; namespace quic { namespace test { namespace { -class TestQuicStream : public QuicStream { - // TestQuicStream exists simply as a place to hang OnDataAvailable(). +class MockDelegate : public QuicStreamIdManager::DelegateInterface { public: - TestQuicStream(QuicStreamId id, QuicSession* session, StreamType type) - : QuicStream(id, session, /*is_static=*/false, type) {} - - void OnDataAvailable() override {} -}; - -class TestQuicSession : public MockQuicSession { - public: - TestQuicSession(QuicConnection* connection) - : MockQuicSession(connection, /*create_mock_crypto_stream=*/true) { - // Initialize to an invalid frame type to detect cases where the frame type - // is not set subsequently. - save_frame_.type = static_cast<QuicFrameType>(-1); - Initialize(); - } - - TestQuicStream* CreateIncomingStream(QuicStreamId id) override { - TestQuicStream* stream = new TestQuicStream( - id, this, - DetermineStreamType(id, transport_version(), perspective(), - /*is_incoming=*/true, BIDIRECTIONAL)); - ActivateStream(QuicWrapUnique(stream)); - return stream; - } - - bool SaveFrame(const QuicFrame& frame) { - save_frame_ = frame; - DeleteFrame(&const_cast<QuicFrame&>(frame)); - return true; - } - - const QuicFrame& save_frame() { return save_frame_; } - - TestQuicStream* CreateOutgoingBidirectionalStream() { - if (!CanOpenNextOutgoingBidirectionalStream()) { - return nullptr; - } - QuicStreamId id = GetNextOutgoingBidirectionalStreamId(); - TestQuicStream* stream = new TestQuicStream(id, this, BIDIRECTIONAL); - ActivateStream(QuicWrapUnique(stream)); - return stream; - } - - TestQuicStream* CreateOutgoingUnidirectionalStream() { - if (!CanOpenNextOutgoingUnidirectionalStream()) { - return nullptr; - } - QuicStreamId id = GetNextOutgoingUnidirectionalStreamId(); - TestQuicStream* stream = new TestQuicStream(id, this, WRITE_UNIDIRECTIONAL); - ActivateStream(QuicWrapUnique(stream)); - return stream; - } - - private: - QuicFrame save_frame_; + MOCK_METHOD1(OnCanCreateNewOutgoingStream, void(bool unidirectional)); + MOCK_METHOD2(OnError, + void(QuicErrorCode error_code, std::string error_details)); + MOCK_METHOD2(SendMaxStreams, + void(QuicStreamCount stream_count, bool unidirectional)); + MOCK_METHOD2(SendStreamsBlocked, + void(QuicStreamCount stream_count, bool unidirectional)); }; class QuicStreamIdManagerTestBase : public QuicTestWithParam<bool> { protected: explicit QuicStreamIdManagerTestBase(Perspective perspective) - : connection_(new StrictMock<MockQuicConnection>( - &helper_, - &alarm_factory_, - perspective, - ParsedQuicVersionVector( - {{PROTOCOL_QUIC_CRYPTO, QUIC_VERSION_99}}))) { - connection_->AdvanceTime(QuicTime::Delta::FromSeconds(1)); - session_ = std::make_unique<TestQuicSession>(connection_); - stream_id_manager_ = - IsBidi() ? QuicSessionPeer::v99_bidirectional_stream_id_manager( - session_.get()) - : QuicSessionPeer::v99_unidirectional_stream_id_manager( - session_.get()); - } + : stream_id_manager_(&delegate_, + IsUnidi(), + perspective, + QUIC_VERSION_99, + 0, + kDefaultMaxStreamsPerConnection, + kDefaultMaxStreamsPerConnection) {} - QuicTransportVersion transport_version() const { - return connection_->transport_version(); - } - - void CloseStream(QuicStreamId id) { session_->CloseStream(id); } + QuicTransportVersion transport_version() const { return QUIC_VERSION_99; } QuicStreamId GetNthClientInitiatedBidirectionalId(int n) { - return QuicUtils::GetFirstBidirectionalStreamId( - connection_->transport_version(), Perspective::IS_CLIENT) + + return QuicUtils::GetFirstBidirectionalStreamId(transport_version(), + Perspective::IS_CLIENT) + kV99StreamIdIncrement * n; } QuicStreamId GetNthClientInitiatedUnidirectionalId(int n) { - return QuicUtils::GetFirstUnidirectionalStreamId( - connection_->transport_version(), Perspective::IS_CLIENT) + + return QuicUtils::GetFirstUnidirectionalStreamId(transport_version(), + Perspective::IS_CLIENT) + kV99StreamIdIncrement * n; } QuicStreamId GetNthServerInitiatedBidirectionalId(int n) { - return QuicUtils::GetFirstBidirectionalStreamId( - connection_->transport_version(), Perspective::IS_SERVER) + + return QuicUtils::GetFirstBidirectionalStreamId(transport_version(), + Perspective::IS_SERVER) + kV99StreamIdIncrement * n; } QuicStreamId GetNthServerInitiatedUnidirectionalId(int n) { - return QuicUtils::GetFirstUnidirectionalStreamId( - connection_->transport_version(), Perspective::IS_SERVER) + + return QuicUtils::GetFirstUnidirectionalStreamId(transport_version(), + Perspective::IS_SERVER) + kV99StreamIdIncrement * n; } @@ -172,11 +90,8 @@ bool IsUnidi() { return GetParam() ? false : true; } bool IsBidi() { return GetParam(); } - MockQuicConnectionHelper helper_; - MockAlarmFactory alarm_factory_; - StrictMock<MockQuicConnection>* connection_; - std::unique_ptr<TestQuicSession> session_; - QuicStreamIdManager* stream_id_manager_; + StrictMock<MockDelegate> delegate_; + QuicStreamIdManager stream_id_manager_; }; // Following tests are either client-specific (they depend, in some way, on @@ -200,29 +115,29 @@ // These fields are inited via the QuicSession constructor to default // values defined as a constant. EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->outgoing_max_streams()); + stream_id_manager_.outgoing_max_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->incoming_actual_max_streams()); + stream_id_manager_.incoming_actual_max_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->incoming_advertised_max_streams()); + stream_id_manager_.incoming_advertised_max_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->incoming_initial_max_open_streams()); + stream_id_manager_.incoming_initial_max_open_streams()); // The window for advertising updates to the MAX STREAM ID is half the number // of streams allowed. EXPECT_EQ(kDefaultMaxStreamsPerConnection / kMaxStreamsWindowDivisor, - stream_id_manager_->max_streams_window()); + stream_id_manager_.max_streams_window()); } // This test checks that the stream advertisement window is set to 1 // if the number of stream ids is 1. This is a special case in the code. TEST_P(QuicStreamIdManagerTestClient, CheckMaxStreamsWindow1) { - stream_id_manager_->SetMaxOpenIncomingStreams(1); - EXPECT_EQ(1u, stream_id_manager_->incoming_initial_max_open_streams()); - EXPECT_EQ(1u, stream_id_manager_->incoming_actual_max_streams()); + stream_id_manager_.SetMaxOpenIncomingStreams(1); + EXPECT_EQ(1u, stream_id_manager_.incoming_initial_max_open_streams()); + EXPECT_EQ(1u, stream_id_manager_.incoming_actual_max_streams()); // If streamid_count/2==0 (integer math) force it to 1. - EXPECT_EQ(1u, stream_id_manager_->max_streams_window()); + EXPECT_EQ(1u, stream_id_manager_.max_streams_window()); } // Now check that setting to a value larger than the maximum fails. @@ -232,12 +147,13 @@ QuicUtils::GetMaxStreamCount(!GetParam(), /* GetParam==true for bidi */ Perspective::IS_CLIENT); // Ensure that the limit is less than the implementation maximum. - EXPECT_LT(stream_id_manager_->outgoing_max_streams(), implementation_max); + EXPECT_LT(stream_id_manager_.outgoing_max_streams(), implementation_max); // Try to go over. - stream_id_manager_->SetMaxOpenOutgoingStreams(implementation_max + 1); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(implementation_max + 1); // Should be pegged at the max. - EXPECT_EQ(implementation_max, stream_id_manager_->outgoing_max_streams()); + EXPECT_EQ(implementation_max, stream_id_manager_.outgoing_max_streams()); } // Now do the same for the incoming streams @@ -245,79 +161,70 @@ QuicStreamCount implementation_max = QuicUtils::GetMaxStreamCount(!GetParam(), /* GetParam==true for bidi */ Perspective::IS_CLIENT); - stream_id_manager_->SetMaxOpenIncomingStreams(implementation_max - 1u); + stream_id_manager_.SetMaxOpenIncomingStreams(implementation_max - 1u); EXPECT_EQ(implementation_max - 1u, - stream_id_manager_->incoming_initial_max_open_streams()); + stream_id_manager_.incoming_initial_max_open_streams()); EXPECT_EQ(implementation_max - 1u, - stream_id_manager_->incoming_actual_max_streams()); + stream_id_manager_.incoming_actual_max_streams()); EXPECT_EQ((implementation_max - 1u) / 2u, - stream_id_manager_->max_streams_window()); + stream_id_manager_.max_streams_window()); - stream_id_manager_->SetMaxOpenIncomingStreams(implementation_max); + stream_id_manager_.SetMaxOpenIncomingStreams(implementation_max); EXPECT_EQ(implementation_max, - stream_id_manager_->incoming_initial_max_open_streams()); + stream_id_manager_.incoming_initial_max_open_streams()); EXPECT_EQ(implementation_max, - stream_id_manager_->incoming_actual_max_streams()); - EXPECT_EQ(implementation_max / 2, stream_id_manager_->max_streams_window()); + stream_id_manager_.incoming_actual_max_streams()); + EXPECT_EQ(implementation_max / 2, stream_id_manager_.max_streams_window()); // Reset to 1 so that we can detect the change. - stream_id_manager_->SetMaxOpenIncomingStreams(1u); - EXPECT_EQ(1u, stream_id_manager_->incoming_initial_max_open_streams()); - EXPECT_EQ(1u, stream_id_manager_->incoming_actual_max_streams()); - EXPECT_EQ(1u, stream_id_manager_->max_streams_window()); + stream_id_manager_.SetMaxOpenIncomingStreams(1u); + EXPECT_EQ(1u, stream_id_manager_.incoming_initial_max_open_streams()); + EXPECT_EQ(1u, stream_id_manager_.incoming_actual_max_streams()); + EXPECT_EQ(1u, stream_id_manager_.max_streams_window()); // Now try to exceed the max, without wrapping. - stream_id_manager_->SetMaxOpenIncomingStreams(implementation_max + 1); + stream_id_manager_.SetMaxOpenIncomingStreams(implementation_max + 1); EXPECT_EQ(implementation_max, - stream_id_manager_->incoming_initial_max_open_streams()); + stream_id_manager_.incoming_initial_max_open_streams()); EXPECT_EQ(implementation_max, - stream_id_manager_->incoming_actual_max_streams()); - EXPECT_EQ(implementation_max / 2u, stream_id_manager_->max_streams_window()); + stream_id_manager_.incoming_actual_max_streams()); + EXPECT_EQ(implementation_max / 2u, stream_id_manager_.max_streams_window()); } // Check the case of the stream count in a STREAMS_BLOCKED frame is less than // the count most recently advertised in a MAX_STREAMS frame. This should cause // a MAX_STREAMS frame with the most recently advertised count to be sent. TEST_P(QuicStreamIdManagerTestClient, ProcessStreamsBlockedOk) { - EXPECT_CALL(*connection_, SendControlFrame(_)) - .WillRepeatedly(Invoke(session_.get(), &TestQuicSession::SaveFrame)); + // Set the config negotiated so that the MAX_STREAMS is transmitted. + stream_id_manager_.OnConfigNegotiated(); + QuicStreamCount stream_count = - stream_id_manager_->incoming_initial_max_open_streams() - 1; - QuicStreamsBlockedFrame frame(0, stream_count, /*unidirectional=*/false); - - // Simulate being configured so that the MAX_STREAMS is transmitted. - session_->OnConfigNegotiated(); - - session_->OnStreamsBlockedFrame(frame); - - // We should see a MAX_STREAMS frame. - EXPECT_EQ(MAX_STREAMS_FRAME, session_->save_frame().type); - - // and it should advertise the current max-allowed value. - EXPECT_EQ(stream_id_manager_->incoming_initial_max_open_streams(), - session_->save_frame().max_streams_frame.stream_count); + stream_id_manager_.incoming_initial_max_open_streams(); + QuicStreamsBlockedFrame frame(0, stream_count - 1, IsUnidi()); + EXPECT_CALL(delegate_, SendMaxStreams(stream_count, IsUnidi())); + stream_id_manager_.OnStreamsBlockedFrame(frame); } // Check the case of the stream count in a STREAMS_BLOCKED frame is equal to the // count most recently advertised in a MAX_STREAMS frame. No MAX_STREAMS // should be generated. TEST_P(QuicStreamIdManagerTestClient, ProcessStreamsBlockedNoOp) { - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); QuicStreamCount stream_count = - stream_id_manager_->incoming_initial_max_open_streams(); - QuicStreamsBlockedFrame frame(0, stream_count, /*unidirectional=*/false); - session_->OnStreamsBlockedFrame(frame); + stream_id_manager_.incoming_initial_max_open_streams(); + QuicStreamsBlockedFrame frame(0, stream_count, IsUnidi()); + EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); } // Check the case of the stream count in a STREAMS_BLOCKED frame is greater than // the count most recently advertised in a MAX_STREAMS frame. Expect a // connection close with an error. TEST_P(QuicStreamIdManagerTestClient, ProcessStreamsBlockedTooBig) { - EXPECT_CALL(*connection_, CloseConnection(QUIC_STREAMS_BLOCKED_ERROR, _, _)); - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + EXPECT_CALL(delegate_, OnError(QUIC_STREAMS_BLOCKED_ERROR, _)); + EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); QuicStreamCount stream_count = - stream_id_manager_->incoming_initial_max_open_streams() + 1; - QuicStreamsBlockedFrame frame(0, stream_count, /*unidirectional=*/false); - session_->OnStreamsBlockedFrame(frame); + stream_id_manager_.incoming_initial_max_open_streams() + 1; + QuicStreamsBlockedFrame frame(0, stream_count, IsUnidi()); + stream_id_manager_.OnStreamsBlockedFrame(frame); } // Same basic tests as above, but calls @@ -328,33 +235,31 @@ // First test make sure that streams with ids below the limit are accepted. TEST_P(QuicStreamIdManagerTestClient, IsIncomingStreamIdValidBelowLimit) { QuicStreamId stream_id = - StreamCountToId(stream_id_manager_->incoming_actual_max_streams() - 1, + StreamCountToId(stream_id_manager_.incoming_actual_max_streams() - 1, Perspective::IS_CLIENT); - EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); - EXPECT_TRUE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_CALL(delegate_, OnError(_, _)).Times(0); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); } // Accept a stream with an ID that equals the limit. TEST_P(QuicStreamIdManagerTestClient, IsIncomingStreamIdValidAtLimit) { - QuicStreamId stream_id = - StreamCountToId(stream_id_manager_->incoming_actual_max_streams(), - Perspective::IS_CLIENT); - EXPECT_CALL(*connection_, CloseConnection(_, _, _)).Times(0); - EXPECT_TRUE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId(stream_id)); + QuicStreamId stream_id = StreamCountToId( + stream_id_manager_.incoming_actual_max_streams(), Perspective::IS_CLIENT); + EXPECT_CALL(delegate_, OnError(_, _)).Times(0); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); } // Close the connection if the id exceeds the limit. TEST_P(QuicStreamIdManagerTestClient, IsIncomingStreamIdInValidAboveLimit) { QuicStreamId stream_id = StreamCountToId( - stream_id_manager_->incoming_actual_max_streams() + 1, + stream_id_manager_.incoming_actual_max_streams() + 1, Perspective::IS_SERVER); // This node is a client, incoming // stream ids must be server-originated. std::string error_details = GetParam() ? "Stream id 401 would exceed stream count limit 100" : "Stream id 403 would exceed stream count limit 100"; - EXPECT_CALL(*connection_, - CloseConnection(QUIC_INVALID_STREAM_ID, error_details, _)); - EXPECT_FALSE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_CALL(delegate_, OnError(QUIC_INVALID_STREAM_ID, error_details)); + EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); } // Test functionality for reception of a MAX_STREAMS frame. This code is @@ -365,7 +270,7 @@ // need to know the number of request/response streams. // This is the total number of outgoing streams (which includes both // req/resp and statics). - stream_id_manager_->outgoing_max_streams(); + stream_id_manager_.outgoing_max_streams(); QuicMaxStreamsFrame frame; @@ -376,27 +281,29 @@ frame.stream_count = initial_stream_count - 1; frame.unidirectional = IsUnidi(); - EXPECT_TRUE(stream_id_manager_->OnMaxStreamsFrame(frame)); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); EXPECT_EQ(initial_stream_count - 1u, - stream_id_manager_->outgoing_max_streams()); + stream_id_manager_.outgoing_max_streams()); QuicStreamCount save_outgoing_max_streams = - stream_id_manager_->outgoing_max_streams(); + stream_id_manager_.outgoing_max_streams(); // Now that there has been one MAX STREAMS frame, we should not // accept a MAX_STREAMS that reduces the limit... frame.stream_count = initial_stream_count - 2; frame.unidirectional = IsUnidi(); - EXPECT_TRUE(stream_id_manager_->OnMaxStreamsFrame(frame)); + EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); // should not change from previous setting. EXPECT_EQ(save_outgoing_max_streams, - stream_id_manager_->outgoing_max_streams()); + stream_id_manager_.outgoing_max_streams()); // A stream count greater than the current limit should increase the limit. frame.stream_count = initial_stream_count + 1; - EXPECT_TRUE(stream_id_manager_->OnMaxStreamsFrame(frame)); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + EXPECT_TRUE(stream_id_manager_.OnMaxStreamsFrame(frame)); EXPECT_EQ(initial_stream_count + 1u, - stream_id_manager_->outgoing_max_streams()); + stream_id_manager_.outgoing_max_streams()); } // Test functionality for reception of a STREAMS_BLOCKED frame. @@ -404,9 +311,10 @@ TEST_P(QuicStreamIdManagerTestClient, StreamIdManagerOnStreamsBlockedFrame) { // Get the current maximum allowed incoming stream count. QuicStreamCount advertised_stream_count = - stream_id_manager_->incoming_advertised_max_streams(); - // Simulate receiving a config to allow frame transmission - session_->OnConfigNegotiated(); + stream_id_manager_.incoming_advertised_max_streams(); + + // Set the config negotiated to allow frame transmission. + stream_id_manager_.OnConfigNegotiated(); QuicStreamsBlockedFrame frame; @@ -415,14 +323,14 @@ // If the peer is saying it's blocked on the stream count that // we've advertised, it's a noop since the peer has the correct information. frame.stream_count = advertised_stream_count; - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); - EXPECT_TRUE(stream_id_manager_->OnStreamsBlockedFrame(frame)); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame)); // If the peer is saying it's blocked on a stream count that is larger // than what we've advertised, the connection should get closed. frame.stream_count = advertised_stream_count + 1; - EXPECT_CALL(*connection_, CloseConnection(QUIC_STREAMS_BLOCKED_ERROR, _, _)); - EXPECT_FALSE(stream_id_manager_->OnStreamsBlockedFrame(frame)); + EXPECT_CALL(delegate_, OnError(QUIC_STREAMS_BLOCKED_ERROR, _)); + EXPECT_FALSE(stream_id_manager_.OnStreamsBlockedFrame(frame)); // If the peer is saying it's blocked on a count that is less than // our actual count, we send a MAX_STREAMS frame and update @@ -430,16 +338,16 @@ // First, need to bump up the actual max so there is room for the MAX // STREAMS frame to send a larger ID. QuicStreamCount actual_stream_count = - stream_id_manager_->incoming_actual_max_streams(); + stream_id_manager_.incoming_actual_max_streams(); // Closing a stream will result in the ability to initiate one more // stream - stream_id_manager_->OnStreamClosed( - QuicStreamIdManagerPeer::GetFirstIncomingStreamId(stream_id_manager_)); + stream_id_manager_.OnStreamClosed( + QuicStreamIdManagerPeer::GetFirstIncomingStreamId(&stream_id_manager_)); EXPECT_EQ(actual_stream_count + 1u, - stream_id_manager_->incoming_actual_max_streams()); - EXPECT_EQ(stream_id_manager_->incoming_actual_max_streams(), - stream_id_manager_->incoming_advertised_max_streams() + 1u); + stream_id_manager_.incoming_actual_max_streams()); + EXPECT_EQ(stream_id_manager_.incoming_actual_max_streams(), + stream_id_manager_.incoming_advertised_max_streams() + 1u); // Now simulate receiving a STREAMS_BLOCKED frame... // Changing the actual maximum, above, forces a MAX_STREAMS frame to be @@ -450,19 +358,14 @@ // MAX_STREAMS sent earler. frame.stream_count = advertised_stream_count; - EXPECT_CALL(*connection_, SendControlFrame(_)) - .Times(1) - .WillRepeatedly(Invoke(session_.get(), &TestQuicSession::SaveFrame)); + EXPECT_CALL(delegate_, + SendMaxStreams(stream_id_manager_.incoming_actual_max_streams(), + IsUnidi())); - EXPECT_TRUE(stream_id_manager_->OnStreamsBlockedFrame(frame)); + EXPECT_TRUE(stream_id_manager_.OnStreamsBlockedFrame(frame)); // Check that the saved frame is correct. - EXPECT_EQ(stream_id_manager_->incoming_actual_max_streams(), - stream_id_manager_->incoming_advertised_max_streams()); - EXPECT_EQ(MAX_STREAMS_FRAME, session_->save_frame().type); - EXPECT_EQ(stream_id_manager_->incoming_advertised_max_streams(), - session_->save_frame().max_streams_frame.stream_count); - // Make sure that this is the only MAX_STREAMS - EXPECT_EQ(1u, GetControlFrameId(session_->save_frame())); + EXPECT_EQ(stream_id_manager_.incoming_actual_max_streams(), + stream_id_manager_.incoming_advertised_max_streams()); } // Test GetNextOutgoingStream. This is client/server agnostic. @@ -471,41 +374,34 @@ // opening... size_t number_of_streams = kDefaultMaxStreamsPerConnection; - // Set up config to allow the default stream limit and then - // simulate receiving a config to allow frame transmission - QuicConfigPeer::SetReceivedMaxIncomingUnidirectionalStreams( - session_->config(), kDefaultMaxStreamsPerConnection); - QuicConfigPeer::SetReceivedMaxIncomingBidirectionalStreams( - session_->config(), kDefaultMaxStreamsPerConnection); - session_->OnConfigNegotiated(); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(100); + + stream_id_manager_.OnConfigNegotiated(); QuicStreamId stream_id = - IsUnidi() ? session_->next_outgoing_unidirectional_stream_id() - : session_->next_outgoing_bidirectional_stream_id(); + IsUnidi() ? QuicUtils::GetFirstUnidirectionalStreamId( + QUIC_VERSION_99, stream_id_manager_.perspective()) + : QuicUtils::GetFirstBidirectionalStreamId( + QUIC_VERSION_99, stream_id_manager_.perspective()); - EXPECT_EQ(number_of_streams, stream_id_manager_->outgoing_max_streams()); + EXPECT_EQ(number_of_streams, stream_id_manager_.outgoing_max_streams()); while (number_of_streams) { - EXPECT_TRUE(stream_id_manager_->CanOpenNextOutgoingStream()); - EXPECT_EQ(stream_id, stream_id_manager_->GetNextOutgoingStreamId()); + EXPECT_TRUE(stream_id_manager_.CanOpenNextOutgoingStream()); + EXPECT_EQ(stream_id, stream_id_manager_.GetNextOutgoingStreamId()); stream_id += kV99StreamIdIncrement; number_of_streams--; } // If we try to check that the next outgoing stream id is available it should // A) fail and B) generate a STREAMS_BLOCKED frame. - EXPECT_CALL(*connection_, SendControlFrame(_)) - .Times(1) - .WillRepeatedly(Invoke(session_.get(), &TestQuicSession::SaveFrame)); - EXPECT_FALSE(stream_id_manager_->CanOpenNextOutgoingStream()); - EXPECT_EQ(STREAMS_BLOCKED_FRAME, session_->save_frame().type); - // If bidi, Crypto stream default created at start up, it is one - // more stream to account for since initialization is "number of - // request/responses" & crypto is added in to that, not streams. - EXPECT_EQ(kDefaultMaxStreamsPerConnection, - session_->save_frame().max_streams_frame.stream_count); + EXPECT_CALL(delegate_, + SendStreamsBlocked(kDefaultMaxStreamsPerConnection, IsUnidi())); + EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream()); + // If we try to get the next id (above the limit), it should cause a quic-bug. EXPECT_QUIC_BUG( - stream_id_manager_->GetNextOutgoingStreamId(), + stream_id_manager_.GetNextOutgoingStreamId(), "Attempt to allocate a new outgoing stream that would exceed the limit"); } @@ -513,27 +409,26 @@ // server/client agnostic. TEST_P(QuicStreamIdManagerTestClient, StreamIdManagerServerMaybeIncreaseLargestPeerStreamId) { - QuicStreamId max_stream_id = - StreamCountToId(stream_id_manager_->incoming_actual_max_streams(), - Perspective::IS_SERVER); + QuicStreamId max_stream_id = StreamCountToId( + stream_id_manager_.incoming_actual_max_streams(), Perspective::IS_SERVER); EXPECT_TRUE( - stream_id_manager_->MaybeIncreaseLargestPeerStreamId(max_stream_id)); + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(max_stream_id)); QuicStreamId server_initiated_stream_id = StreamCountToId(1u, // get 1st id Perspective::IS_SERVER); - EXPECT_TRUE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId( + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( server_initiated_stream_id)); // A bad stream ID results in a closed connection. - EXPECT_CALL(*connection_, CloseConnection(QUIC_INVALID_STREAM_ID, _, _)); - EXPECT_FALSE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId( + EXPECT_CALL(delegate_, OnError(QUIC_INVALID_STREAM_ID, _)); + EXPECT_FALSE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId( max_stream_id + kV99StreamIdIncrement)); } // Test the MAX STREAMS Window functionality. TEST_P(QuicStreamIdManagerTestClient, StreamIdManagerServerMaxStreams) { - // Simulate completed config to allow frame transmission - session_->OnConfigNegotiated(); + // Set the config negotiated to allow frame transmission. + stream_id_manager_.OnConfigNegotiated(); // Test that a MAX_STREAMS frame is generated when the peer has less than // |max_streams_window_| streams left that it can initiate. @@ -543,96 +438,89 @@ // should be sent. The -1 is because the check in // QuicStreamIdManager::MaybeSendMaxStreamsFrame sends a MAX_STREAMS if the // number of available streams at the peer is <= |max_streams_window_| - int stream_count = stream_id_manager_->max_streams_window() - 1; + int stream_count = stream_id_manager_.max_streams_window() - 1; // Should not get a control-frame transmission since the peer should have // "plenty" of stream IDs to use. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); + EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); // Get the first incoming stream ID to try and allocate. QuicStreamId stream_id = IsBidi() ? GetNthServerInitiatedBidirectionalId(0) : GetNthServerInitiatedUnidirectionalId(0); size_t old_available_incoming_streams = - stream_id_manager_->available_incoming_streams(); + stream_id_manager_.available_incoming_streams(); while (stream_count) { - EXPECT_TRUE( - stream_id_manager_->MaybeIncreaseLargestPeerStreamId(stream_id)); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); // This node should think that the peer believes it has one fewer // stream it can create. old_available_incoming_streams--; EXPECT_EQ(old_available_incoming_streams, - stream_id_manager_->available_incoming_streams()); + stream_id_manager_.available_incoming_streams()); stream_count--; stream_id += kV99StreamIdIncrement; } // Now close them, still should get no MAX_STREAMS - stream_count = stream_id_manager_->max_streams_window(); + stream_count = stream_id_manager_.max_streams_window(); stream_id = IsBidi() ? GetNthServerInitiatedBidirectionalId(0) : GetNthServerInitiatedUnidirectionalId(0); QuicStreamCount expected_actual_max = - stream_id_manager_->incoming_actual_max_streams(); + stream_id_manager_.incoming_actual_max_streams(); QuicStreamCount expected_advertised_max_streams = - stream_id_manager_->incoming_advertised_max_streams(); + stream_id_manager_.incoming_advertised_max_streams(); while (stream_count) { - stream_id_manager_->OnStreamClosed(stream_id); + stream_id_manager_.OnStreamClosed(stream_id); stream_count--; stream_id += kV99StreamIdIncrement; expected_actual_max++; EXPECT_EQ(expected_actual_max, - stream_id_manager_->incoming_actual_max_streams()); + stream_id_manager_.incoming_actual_max_streams()); // Advertised maximum should remain the same. EXPECT_EQ(expected_advertised_max_streams, - stream_id_manager_->incoming_advertised_max_streams()); + stream_id_manager_.incoming_advertised_max_streams()); } // This should not change. EXPECT_EQ(old_available_incoming_streams, - stream_id_manager_->available_incoming_streams()); + stream_id_manager_.available_incoming_streams()); // Now whenever we close a stream we should get a MAX_STREAMS frame. // Above code closed all the open streams, so we have to open/close - EXPECT_CALL(*connection_, SendControlFrame(_)) - .Times(1) - .WillRepeatedly(Invoke(session_.get(), &TestQuicSession::SaveFrame)); - EXPECT_TRUE(stream_id_manager_->MaybeIncreaseLargestPeerStreamId(stream_id)); - stream_id_manager_->OnStreamClosed(stream_id); - stream_id += kV99StreamIdIncrement; - - // Check that the MAX STREAMS was sent and has the correct values. - EXPECT_EQ(MAX_STREAMS_FRAME, session_->save_frame().type); - EXPECT_EQ(stream_id_manager_->incoming_advertised_max_streams(), - session_->save_frame().max_streams_frame.stream_count); + // EXPECT_CALL(delegate_, + // SendMaxStreams(stream_id_manager_.incoming_actual_max_streams(), + // IsUnidi())); + EXPECT_CALL(delegate_, SendMaxStreams(_, IsUnidi())); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(stream_id)); + stream_id_manager_.OnStreamClosed(stream_id); } // Check that edge conditions of the stream count in a STREAMS_BLOCKED frame // are. properly handled. TEST_P(QuicStreamIdManagerTestClient, StreamsBlockedEdgeConditions) { - // Simulate completed config to allow frame transmission - session_->OnConfigNegotiated(); + // Set the config negotiated to allow frame transmission. + stream_id_manager_.OnConfigNegotiated(); QuicStreamsBlockedFrame frame; frame.unidirectional = IsUnidi(); // Check that receipt of a STREAMS BLOCKED with stream-count = 0 does nothing // when max_allowed_incoming_streams is 0. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); - stream_id_manager_->SetMaxOpenIncomingStreams(0); + EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); + stream_id_manager_.SetMaxOpenIncomingStreams(0); frame.stream_count = 0; - stream_id_manager_->OnStreamsBlockedFrame(frame); + stream_id_manager_.OnStreamsBlockedFrame(frame); // Check that receipt of a STREAMS BLOCKED with stream-count = 0 invokes a // MAX STREAMS, count = 123, when the MaxOpen... is set to 123. - EXPECT_CALL(*connection_, SendControlFrame(_)) - .Times(1) - .WillOnce(Invoke(session_.get(), &TestQuicSession::SaveFrame)); - stream_id_manager_->SetMaxOpenIncomingStreams(123); + EXPECT_CALL(delegate_, SendMaxStreams(123u, IsUnidi())); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); + stream_id_manager_.SetMaxOpenIncomingStreams(123); frame.stream_count = 0; - stream_id_manager_->OnStreamsBlockedFrame(frame); - EXPECT_EQ(MAX_STREAMS_FRAME, session_->save_frame().type); - EXPECT_EQ(123u, session_->save_frame().max_streams_frame.stream_count); + stream_id_manager_.OnStreamsBlockedFrame(frame); } TEST_P(QuicStreamIdManagerTestClient, HoldMaxStreamsFrame) { @@ -640,21 +528,18 @@ return; } - // The session has not been configured so frame transmission will not be - // allowed. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + // The config has not been negotiated so the MAX_STREAMS frame will not be + // sent. + EXPECT_CALL(delegate_, SendMaxStreams(_, _)).Times(0); - QuicStreamsBlockedFrame frame( - 1u, 0u, QuicStreamIdManagerPeer::get_unidirectional(stream_id_manager_)); + QuicStreamsBlockedFrame frame(1u, 0u, IsUnidi()); // Should cause change in pending_max_streams. - stream_id_manager_->OnStreamsBlockedFrame(frame); - // Will do OnConfig. We should see a control frame pop out now. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(1); + stream_id_manager_.OnStreamsBlockedFrame(frame); - // Now allow frame transmission -- which happens when the QuicSession - // receives the configuration and calls - // QuicStreamIdManager::OnConfigNegotiated() - session_->OnConfigNegotiated(); + EXPECT_CALL(delegate_, SendMaxStreams(_, IsUnidi())); + + // MAX_STREAMS will be sent now that the config has been negotiated. + stream_id_manager_.OnConfigNegotiated(); } // Following tests all are server-specific. They depend, in some way, on @@ -673,25 +558,21 @@ // set outgoing limit to 0, will cause the CanOpenNext... to fail // leading to a STREAMS_BLOCKED. - QuicStreamIdManagerPeer::set_outgoing_max_streams(stream_id_manager_, 0); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(0); - // We should not see a STREAMS-BLOCKED frame because we're not configured.. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + // We should not see a STREAMS_BLOCKED frame because we're not configured.. + EXPECT_CALL(delegate_, SendStreamsBlocked(_, _)).Times(0); // Since the stream limit is 0 and no sreams can be created this should return - // false and have forced a streams-blocked to be queued up, with the + // false and have forced a STREAMS_BLOCKED to be queued up, with the // blocked stream id == 0. - EXPECT_FALSE(stream_id_manager_->CanOpenNextOutgoingStream()); + EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream()); - // Simulate receipt of the configuration; This case does not update the - // outgoing stream limit, so the on-config should result in a streams-blocked - // being sent. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(1); - QuicConfigPeer::SetReceivedMaxIncomingUnidirectionalStreams( - session_->config(), 0); - QuicConfigPeer::SetReceivedMaxIncomingBidirectionalStreams(session_->config(), - 0); - session_->OnConfigNegotiated(); + // Since the steam limit has not been increased when the config was negotiated + // a STREAMS_BLOCKED frame should be sent. + EXPECT_CALL(delegate_, SendStreamsBlocked(_, IsUnidi())); + stream_id_manager_.OnConfigNegotiated(); } TEST_P(QuicStreamIdManagerTestClient, HoldStreamsBlockedFrameNoXmit) { @@ -700,28 +581,22 @@ } // Set outgoing limit to 0, will cause the CanOpenNext... to fail // leading to a STREAMS_BLOCKED. - QuicStreamIdManagerPeer::set_outgoing_max_streams(stream_id_manager_, 0); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(0); - // We should not see a STREAMS-BLOCKED frame because we're not configured.. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); + // We should not see a STREAMS_BLOCKED frame because we're not configured.. + EXPECT_CALL(delegate_, SendStreamsBlocked(_, IsUnidi())).Times(0); // Since the stream limit is 0 and no sreams can be created this should return - // false and have forced a streams-blocked to be queued up, with the + // false and have forced a STREAMS_BLOCKED to be queued up, with the // blocked stream id == 0. - EXPECT_FALSE(stream_id_manager_->CanOpenNextOutgoingStream()); + EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream()); - // Since the config gives some streams to create, we should not see - // a STREAMS-BLOCKED frame. - EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); - - // Do the configuration. The stream limits are increased, allowing this - // node to create more streams, so we should not see the pending - // STREAMS-BLOCKED frame get transmitted. - QuicConfigPeer::SetReceivedMaxIncomingUnidirectionalStreams( - session_->config(), 10); - QuicConfigPeer::SetReceivedMaxIncomingBidirectionalStreams(session_->config(), - 10); - session_->OnConfigNegotiated(); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(10); + // Since the stream limit has been increase which allows streams to be created + // no STREAMS_BLOCKED should be send. + stream_id_manager_.OnConfigNegotiated(); } INSTANTIATE_TEST_SUITE_P(Tests, @@ -733,86 +608,64 @@ // stream id is correct. TEST_P(QuicStreamIdManagerTestServer, CheckMaxAllowedOutgoing) { const size_t kIncomingStreamCount = 123; - stream_id_manager_->SetMaxOpenOutgoingStreams(kIncomingStreamCount); - EXPECT_EQ(kIncomingStreamCount, stream_id_manager_->outgoing_max_streams()); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(kIncomingStreamCount); + EXPECT_EQ(kIncomingStreamCount, stream_id_manager_.outgoing_max_streams()); } // Test that a MAX_STREAMS frame is generated when half the stream ids become // available. This has a useful side effect of testing that when streams are // closed, the number of available stream ids increases. TEST_P(QuicStreamIdManagerTestServer, MaxStreamsSlidingWindow) { - // Ignore OnStreamReset calls. - EXPECT_CALL(*connection_, OnStreamReset(_, _)).WillRepeatedly(Return()); - // Capture control frames for analysis. - EXPECT_CALL(*connection_, SendControlFrame(_)) - .WillRepeatedly(Invoke(session_.get(), &TestQuicSession::SaveFrame)); // Simulate config being negotiated, causing the limits all to be initialized. - session_->OnConfigNegotiated(); + stream_id_manager_.OnConfigNegotiated(); + QuicStreamCount first_advert = - stream_id_manager_->incoming_advertised_max_streams(); + stream_id_manager_.incoming_advertised_max_streams(); // Open/close enough streams to shrink the window without causing a MAX // STREAMS to be generated. The window will open (and a MAX STREAMS generated) // when max_streams_window() stream IDs have been made available. The loop // will make that many stream IDs available, so the last CloseStream should + // cause a MAX STREAMS frame to be generated. - int i = static_cast<int>(stream_id_manager_->max_streams_window()); + int i = static_cast<int>(stream_id_manager_.max_streams_window()); QuicStreamId id = - QuicStreamIdManagerPeer::GetFirstIncomingStreamId(stream_id_manager_); + QuicStreamIdManagerPeer::GetFirstIncomingStreamId(&stream_id_manager_); + EXPECT_CALL( + delegate_, + SendMaxStreams(first_advert + stream_id_manager_.max_streams_window(), + IsUnidi())); while (i) { - QuicStream* stream = session_->GetOrCreateStream(id); - EXPECT_NE(nullptr, stream); - // have to set the stream's fin-received flag to true so that it - // does not go into the has-not-received-byte-offset state, leading - // to the stream being added to the locally_closed_streams_highest_offset_ - // map, and therefore not counting as truly being closed. The test requires - // that the stream truly close, so that new streams become available, - // causing the MAX_STREAMS to be sent. - stream->set_fin_received(true); - EXPECT_EQ(id, stream->id()); - if (IsBidi()) { - // Only send reset for incoming bidirectional streams. - EXPECT_CALL(*session_, SendRstStream(_, _, _)); - } - CloseStream(stream->id()); + EXPECT_TRUE(stream_id_manager_.MaybeIncreaseLargestPeerStreamId(id)); + stream_id_manager_.OnStreamClosed(id); i--; id += kV99StreamIdIncrement; } - EXPECT_EQ(MAX_STREAMS_FRAME, session_->save_frame().type); - QuicStreamCount second_advert = - session_->save_frame().max_streams_frame.stream_count; - EXPECT_EQ(first_advert + stream_id_manager_->max_streams_window(), - second_advert); } // Tast that an attempt to create an outgoing stream does not exceed the limit // and that it generates an appropriate STREAMS_BLOCKED frame. TEST_P(QuicStreamIdManagerTestServer, NewStreamDoesNotExceedLimit) { - // Configure with some number of streams, and allow frame transmission - QuicConfigPeer::SetReceivedMaxIncomingUnidirectionalStreams( - session_->config(), 100); - QuicConfigPeer::SetReceivedMaxIncomingBidirectionalStreams(session_->config(), - 100); - session_->OnConfigNegotiated(); + EXPECT_CALL(delegate_, OnCanCreateNewOutgoingStream(IsUnidi())); + stream_id_manager_.SetMaxOpenOutgoingStreams(100); + stream_id_manager_.OnConfigNegotiated(); - size_t stream_count = stream_id_manager_->outgoing_max_streams(); + size_t stream_count = stream_id_manager_.outgoing_max_streams(); EXPECT_NE(0u, stream_count); - TestQuicStream* stream; + while (stream_count) { - stream = IsBidi() ? session_->CreateOutgoingBidirectionalStream() - : session_->CreateOutgoingUnidirectionalStream(); - EXPECT_NE(stream, nullptr); + EXPECT_TRUE(stream_id_manager_.CanOpenNextOutgoingStream()); + stream_id_manager_.GetNextOutgoingStreamId(); stream_count--; } - EXPECT_EQ(stream_id_manager_->outgoing_stream_count(), - stream_id_manager_->outgoing_max_streams()); + EXPECT_EQ(stream_id_manager_.outgoing_stream_count(), + stream_id_manager_.outgoing_max_streams()); // Create another, it should fail. Should also send a STREAMS_BLOCKED // control frame. - EXPECT_CALL(*connection_, SendControlFrame(_)); - stream = IsBidi() ? session_->CreateOutgoingBidirectionalStream() - : session_->CreateOutgoingUnidirectionalStream(); - EXPECT_EQ(nullptr, stream); + EXPECT_CALL(delegate_, SendStreamsBlocked(_, IsUnidi())); + EXPECT_FALSE(stream_id_manager_.CanOpenNextOutgoingStream()); } // Check that the parameters used by the stream ID manager are properly @@ -821,27 +674,27 @@ // These fields are inited via the QuicSession constructor to default // values defined as a constant. EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->incoming_initial_max_open_streams()); + stream_id_manager_.incoming_initial_max_open_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->incoming_actual_max_streams()); + stream_id_manager_.incoming_actual_max_streams()); EXPECT_EQ(kDefaultMaxStreamsPerConnection, - stream_id_manager_->outgoing_max_streams()); + stream_id_manager_.outgoing_max_streams()); // The window for advertising updates to the MAX STREAM ID is half the number // of stream allowed. EXPECT_EQ(kDefaultMaxStreamsPerConnection / kMaxStreamsWindowDivisor, - stream_id_manager_->max_streams_window()); + stream_id_manager_.max_streams_window()); } TEST_P(QuicStreamIdManagerTestServer, AvailableStreams) { - stream_id_manager_->MaybeIncreaseLargestPeerStreamId( + stream_id_manager_.MaybeIncreaseLargestPeerStreamId( IsBidi() ? GetNthClientInitiatedBidirectionalId(3) : GetNthClientInitiatedUnidirectionalId(3)); - EXPECT_TRUE(stream_id_manager_->IsAvailableStream( + EXPECT_TRUE(stream_id_manager_.IsAvailableStream( IsBidi() ? GetNthClientInitiatedBidirectionalId(1) : GetNthClientInitiatedUnidirectionalId(1))); - EXPECT_TRUE(stream_id_manager_->IsAvailableStream( + EXPECT_TRUE(stream_id_manager_.IsAvailableStream( IsBidi() ? GetNthClientInitiatedBidirectionalId(2) : GetNthClientInitiatedUnidirectionalId(2))); } @@ -851,7 +704,7 @@ // This is a regression for Chromium bugs 909987 and 910040 TEST_P(QuicStreamIdManagerTestServer, ExtremeMaybeIncreaseLargestPeerStreamId) { QuicStreamId too_big_stream_id = StreamCountToId( - stream_id_manager_->incoming_actual_max_streams() + 20, + stream_id_manager_.incoming_actual_max_streams() + 20, Perspective::IS_CLIENT); // This node is a server, incoming stream // ids must be client-originated. std::string error_details; @@ -865,10 +718,9 @@ error_details = "Stream id 478 would exceed stream count limit 100"; } - EXPECT_CALL(*connection_, - CloseConnection(QUIC_INVALID_STREAM_ID, error_details, _)); + EXPECT_CALL(delegate_, OnError(QUIC_INVALID_STREAM_ID, error_details)); EXPECT_FALSE( - stream_id_manager_->MaybeIncreaseLargestPeerStreamId(too_big_stream_id)); + stream_id_manager_.MaybeIncreaseLargestPeerStreamId(too_big_stream_id)); } } // namespace
diff --git a/quic/core/uber_quic_stream_id_manager.cc b/quic/core/uber_quic_stream_id_manager.cc index fa42643..98c6e8c 100644 --- a/quic/core/uber_quic_stream_id_manager.cc +++ b/quic/core/uber_quic_stream_id_manager.cc
@@ -11,17 +11,24 @@ UberQuicStreamIdManager::UberQuicStreamIdManager( QuicSession* session, + QuicStreamCount num_expected_unidirectiona_static_streams, QuicStreamCount max_open_outgoing_bidirectional_streams, QuicStreamCount max_open_outgoing_unidirectional_streams, QuicStreamCount max_open_incoming_bidirectional_streams, QuicStreamCount max_open_incoming_unidirectional_streams) : bidirectional_stream_id_manager_(session, /*unidirectional=*/false, + session->perspective(), + session->transport_version(), + 0, max_open_outgoing_bidirectional_streams, max_open_incoming_bidirectional_streams), unidirectional_stream_id_manager_( session, /*unidirectional=*/true, + session->perspective(), + session->transport_version(), + num_expected_unidirectiona_static_streams, max_open_outgoing_unidirectional_streams, max_open_incoming_unidirectional_streams) {}
diff --git a/quic/core/uber_quic_stream_id_manager.h b/quic/core/uber_quic_stream_id_manager.h index 5505475..dab571d 100644 --- a/quic/core/uber_quic_stream_id_manager.h +++ b/quic/core/uber_quic_stream_id_manager.h
@@ -22,6 +22,7 @@ public: UberQuicStreamIdManager( QuicSession* session, + QuicStreamCount num_expected_unidirectiona_static_streams, QuicStreamCount max_open_outgoing_bidirectional_streams, QuicStreamCount max_open_outgoing_unidirectional_streams, QuicStreamCount max_open_incoming_bidirectional_streams,
diff --git a/quic/core/uber_quic_stream_id_manager_test.cc b/quic/core/uber_quic_stream_id_manager_test.cc index 7e3cc8c..b4ef1cc 100644 --- a/quic/core/uber_quic_stream_id_manager_test.cc +++ b/quic/core/uber_quic_stream_id_manager_test.cc
@@ -315,7 +315,7 @@ TEST_P(UberQuicStreamIdManagerTest, OnStreamsBlockedFrame) { // Allow MAX_STREAMS frame transmission - QuicSessionPeer::set_is_configured(session_.get(), true); + manager_->OnConfigNegotiated(); // Set up to capture calls to SendControlFrame - when a STREAMS_BLOCKED // frame is received, it will result in a a new MAX_STREAMS frame being // sent (if new streams can be made available).