Allows QuicSpdyClientSessionBase to take an QuicSession::Visitor instance to propagate server preferred address signal via a new interface OnServerPreferredAddressAvailable(). Move initiation of server preferred address validation from the client session to QUIC client. Client only change. The new interface is a no-op on the server side. PiperOrigin-RevId: 501579060
diff --git a/quiche/quic/core/http/quic_spdy_client_session.cc b/quiche/quic/core/http/quic_spdy_client_session.cc index b6fa05d..a30e10c 100644 --- a/quiche/quic/core/http/quic_spdy_client_session.cc +++ b/quiche/quic/core/http/quic_spdy_client_session.cc
@@ -26,7 +26,15 @@ QuicConnection* connection, const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config, QuicClientPushPromiseIndex* push_promise_index) - : QuicSpdyClientSessionBase(connection, push_promise_index, config, + : QuicSpdyClientSession(config, supported_versions, connection, nullptr, + server_id, crypto_config, push_promise_index) {} + +QuicSpdyClientSession::QuicSpdyClientSession( + const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, + QuicConnection* connection, QuicSession::Visitor* visitor, + const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config, + QuicClientPushPromiseIndex* push_promise_index) + : QuicSpdyClientSessionBase(connection, visitor, push_promise_index, config, supported_versions), server_id_(server_id), crypto_config_(crypto_config),
diff --git a/quiche/quic/core/http/quic_spdy_client_session.h b/quiche/quic/core/http/quic_spdy_client_session.h index df97adc..083baba 100644 --- a/quiche/quic/core/http/quic_spdy_client_session.h +++ b/quiche/quic/core/http/quic_spdy_client_session.h
@@ -31,6 +31,15 @@ const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config, QuicClientPushPromiseIndex* push_promise_index); + + QuicSpdyClientSession(const QuicConfig& config, + const ParsedQuicVersionVector& supported_versions, + QuicConnection* connection, + QuicSession::Visitor* visitor, + const QuicServerId& server_id, + QuicCryptoClientConfig* crypto_config, + QuicClientPushPromiseIndex* push_promise_index); + QuicSpdyClientSession(const QuicSpdyClientSession&) = delete; QuicSpdyClientSession& operator=(const QuicSpdyClientSession&) = delete; ~QuicSpdyClientSession() override;
diff --git a/quiche/quic/core/http/quic_spdy_client_session_base.cc b/quiche/quic/core/http/quic_spdy_client_session_base.cc index 3b38b1e..adc3348 100644 --- a/quiche/quic/core/http/quic_spdy_client_session_base.cc +++ b/quiche/quic/core/http/quic_spdy_client_session_base.cc
@@ -17,9 +17,10 @@ namespace quic { QuicSpdyClientSessionBase::QuicSpdyClientSessionBase( - QuicConnection* connection, QuicClientPushPromiseIndex* push_promise_index, - const QuicConfig& config, const ParsedQuicVersionVector& supported_versions) - : QuicSpdySession(connection, nullptr, config, supported_versions), + QuicConnection* connection, QuicSession::Visitor* visitor, + QuicClientPushPromiseIndex* push_promise_index, const QuicConfig& config, + const ParsedQuicVersionVector& supported_versions) + : QuicSpdySession(connection, visitor, config, supported_versions), push_promise_index_(push_promise_index), largest_promised_stream_id_( QuicUtils::GetInvalidStreamId(connection->transport_version())) {}
diff --git a/quiche/quic/core/http/quic_spdy_client_session_base.h b/quiche/quic/core/http/quic_spdy_client_session_base.h index 04a0d76..39746b6 100644 --- a/quiche/quic/core/http/quic_spdy_client_session_base.h +++ b/quiche/quic/core/http/quic_spdy_client_session_base.h
@@ -40,6 +40,7 @@ // Takes ownership of |connection|. Caller retains ownership of // |promised_by_url|. QuicSpdyClientSessionBase(QuicConnection* connection, + QuicSession::Visitor* visitor, QuicClientPushPromiseIndex* push_promise_index, const QuicConfig& config, const ParsedQuicVersionVector& supported_versions);
diff --git a/quiche/quic/core/quic_dispatcher.h b/quiche/quic/core/quic_dispatcher.h index 4c3e278..24602b3 100644 --- a/quiche/quic/core/quic_dispatcher.h +++ b/quiche/quic/core/quic_dispatcher.h
@@ -116,6 +116,11 @@ void OnConnectionIdRetired( const QuicConnectionId& server_connection_id) override; + void OnServerPreferredAddressAvailable( + const QuicSocketAddress& /*server_preferred_address*/) override { + QUICHE_DCHECK(false); + } + // QuicTimeWaitListManager::Visitor interface implementation // Called whenever the time wait list manager adds a new connection to the // time-wait list.
diff --git a/quiche/quic/core/quic_session.cc b/quiche/quic/core/quic_session.cc index 208c01b..7aaa65b 100644 --- a/quiche/quic/core/quic_session.cc +++ b/quiche/quic/core/quic_session.cc
@@ -27,6 +27,7 @@ #include "quiche/quic/platform/api/quic_logging.h" #include "quiche/quic/platform/api/quic_server_stats.h" #include "quiche/quic/platform/api/quic_stack_trace.h" +#include "quiche/common/platform/api/quiche_logging.h" #include "quiche/common/quiche_text_utils.h" namespace quic { @@ -2694,5 +2695,13 @@ return valid; } +void QuicSession::OnServerPreferredAddressAvailable( + const QuicSocketAddress& server_preferred_address) { + QUICHE_DCHECK_EQ(perspective_, Perspective::IS_CLIENT); + if (visitor_ != nullptr) { + visitor_->OnServerPreferredAddressAvailable(server_preferred_address); + } +} + #undef ENDPOINT // undef for jumbo builds } // namespace quic
diff --git a/quiche/quic/core/quic_session.h b/quiche/quic/core/quic_session.h index bf9531a..ec52225 100644 --- a/quiche/quic/core/quic_session.h +++ b/quiche/quic/core/quic_session.h
@@ -68,8 +68,11 @@ public QuicControlFrameManager::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 - // is closed, blocked, or added/removed from the time-wait list. + // This lets the session notify its owner when the connection + // is closed, blocked, etc. + // TODO(danzh): split this visitor to separate visitors for client and server + // respectively as not all methods in this class are interesting to both + // perspectives. class QUIC_EXPORT_PRIVATE Visitor { public: virtual ~Visitor() {} @@ -98,6 +101,9 @@ // Called when a ConnectionId has been retired. virtual void OnConnectionIdRetired( const QuicConnectionId& server_connection_id) = 0; + + virtual void OnServerPreferredAddressAvailable( + const QuicSocketAddress& /*server_preferred_address*/) = 0; }; // Does not take ownership of |connection| or |visitor|. @@ -180,7 +186,7 @@ return nullptr; } void OnServerPreferredAddressAvailable( - const QuicSocketAddress& /*server_preferred_address*/) override {} + const QuicSocketAddress& /*server_preferred_address*/) override; // QuicStreamFrameDataProducer WriteStreamDataResult WriteStreamData(QuicStreamId id,
diff --git a/quiche/quic/test_tools/mock_quic_session_visitor.h b/quiche/quic/test_tools/mock_quic_session_visitor.h index 61dc5d5..ab230b4 100644 --- a/quiche/quic/test_tools/mock_quic_session_visitor.h +++ b/quiche/quic/test_tools/mock_quic_session_visitor.h
@@ -35,6 +35,8 @@ (const quic::QuicConnectionId& server_connection_id), (override)); MOCK_METHOD(void, OnConnectionAddedToTimeWaitList, (QuicConnectionId connection_id), (override)); + MOCK_METHOD(void, OnServerPreferredAddressAvailable, + (const QuicSocketAddress& server_preferred_address), (override)); }; class MockQuicCryptoServerStreamHelper
diff --git a/quiche/quic/test_tools/quic_test_utils.cc b/quiche/quic/test_tools/quic_test_utils.cc index 62c8f69..1a63557 100644 --- a/quiche/quic/test_tools/quic_test_utils.cc +++ b/quiche/quic/test_tools/quic_test_utils.cc
@@ -738,8 +738,8 @@ QuicConnection* connection, const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config) - : QuicSpdyClientSessionBase(connection, &push_promise_index_, config, - supported_versions) { + : QuicSpdyClientSessionBase(connection, nullptr, &push_promise_index_, + config, supported_versions) { // TODO(b/153726130): Consider adding SetServerApplicationStateForResumption // calls in tests and set |has_application_state| to true. crypto_stream_ = std::make_unique<QuicCryptoClientStream>(
diff --git a/quiche/quic/tools/quic_client_base.cc b/quiche/quic/tools/quic_client_base.cc index de77089..02b56a4 100644 --- a/quiche/quic/tools/quic_client_base.cc +++ b/quiche/quic/tools/quic_client_base.cc
@@ -18,15 +18,20 @@ namespace quic { +namespace { + // Implements the basic feature of a result delegate for path validation for // connection migration. If the validation succeeds, migrate to the alternative // path. Otherwise, stay on the current path. class QuicClientSocketMigrationValidationResultDelegate : public QuicPathValidator::ResultDelegate { public: - QuicClientSocketMigrationValidationResultDelegate(QuicClientBase* client) + explicit QuicClientSocketMigrationValidationResultDelegate( + QuicClientBase* client) : QuicPathValidator::ResultDelegate(), client_(client) {} + virtual ~QuicClientSocketMigrationValidationResultDelegate() = default; + // QuicPathValidator::ResultDelegate // Overridden to start migration and takes the ownership of the writer in the // context. @@ -53,10 +58,33 @@ /*is_multi_port=*/false, *context); } + protected: + QuicClientBase* client() { return client_; } + private: QuicClientBase* client_; }; +class ServerPreferredAddressResultDelegateWithWriter + : public QuicClientSocketMigrationValidationResultDelegate { + public: + ServerPreferredAddressResultDelegateWithWriter(QuicClientBase* client) + : QuicClientSocketMigrationValidationResultDelegate(client) {} + + // Overridden to transfer the ownership of the new writer. + void OnPathValidationSuccess( + std::unique_ptr<QuicPathValidationContext> context, + QuicTime /*start_time*/) override { + client()->session()->connection()->OnServerPreferredAddressValidated( + *context, false); + auto migration_context = std::unique_ptr<PathMigrationContext>( + static_cast<PathMigrationContext*>(context.release())); + client()->set_writer(migration_context->ReleaseWriter()); + } +}; + +} // namespace + QuicClientBase::NetworkHelper::~NetworkHelper() = default; QuicClientBase::QuicClientBase( @@ -494,4 +522,23 @@ std::move(result_delegate)); } +void QuicClientBase::OnServerPreferredAddressAvailable( + const QuicSocketAddress& server_preferred_address) { + const auto self_address = session_->self_address(); + if (network_helper_ == nullptr || + !network_helper_->CreateUDPSocketAndBind(server_preferred_address, + self_address.host(), 0)) { + return; + } + QuicPacketWriter* writer = network_helper_->CreateQuicPacketWriter(); + if (writer == nullptr) { + return; + } + session()->ValidatePath( + std::make_unique<PathMigrationContext>( + std::unique_ptr<QuicPacketWriter>(writer), + network_helper_->GetLatestClientAddress(), server_preferred_address), + std::make_unique<ServerPreferredAddressResultDelegateWithWriter>(this)); +} + } // namespace quic
diff --git a/quiche/quic/tools/quic_client_base.h b/quiche/quic/tools/quic_client_base.h index ac29482..c3ec302 100644 --- a/quiche/quic/tools/quic_client_base.h +++ b/quiche/quic/tools/quic_client_base.h
@@ -52,7 +52,7 @@ // Subclasses derived from this class are responsible for creating the // actual QuicSession instance, as well as defining functions that // create and run the underlying network transport. -class QuicClientBase { +class QuicClientBase : public QuicSession::Visitor { public: // An interface to various network events that the QuicClient will need to // interact with. @@ -93,6 +93,26 @@ virtual ~QuicClientBase(); + // Implmenets QuicSession::Visitor + void OnConnectionClosed(QuicConnectionId /*server_connection_id*/, + QuicErrorCode /*error*/, + const std::string& /*error_details*/, + ConnectionCloseSource /*source*/) override {} + + void OnWriteBlocked(QuicBlockedWriterInterface* /*blocked_writer*/) override { + } + void OnRstStreamReceived(const QuicRstStreamFrame& /*frame*/) override {} + void OnStopSendingReceived(const QuicStopSendingFrame& /*frame*/) override {} + bool TryAddNewConnectionId( + const QuicConnectionId& /*server_connection_id*/, + const QuicConnectionId& /*new_connection_id*/) override { + return false; + } + void OnConnectionIdRetired( + const QuicConnectionId& /*server_connection_id*/) override {} + void OnServerPreferredAddressAvailable( + const QuicSocketAddress& server_preferred_address) override; + // Initializes the client to create a connection. Should be called exactly // once before calling StartConnect or Connect. Returns true if the // initialization succeeds, false otherwise.
diff --git a/quiche/quic/tools/quic_default_client.cc b/quiche/quic/tools/quic_default_client.cc index 0f43aa5..6a15fa9 100644 --- a/quiche/quic/tools/quic_default_client.cc +++ b/quiche/quic/tools/quic_default_client.cc
@@ -86,8 +86,8 @@ const ParsedQuicVersionVector& supported_versions, QuicConnection* connection) { return std::make_unique<QuicSimpleClientSession>( - *config(), supported_versions, connection, network_helper(), server_id(), - crypto_config(), push_promise_index(), drop_response_body(), + *config(), supported_versions, connection, this, network_helper(), + server_id(), crypto_config(), push_promise_index(), drop_response_body(), enable_web_transport()); }
diff --git a/quiche/quic/tools/quic_simple_client_session.cc b/quiche/quic/tools/quic_simple_client_session.cc index d6085d2..26e368b 100644 --- a/quiche/quic/tools/quic_simple_client_session.cc +++ b/quiche/quic/tools/quic_simple_client_session.cc
@@ -10,43 +10,26 @@ namespace quic { -namespace { - -class ServerPreferredAddressResultDelegateWithWriter - : public QuicPathValidator::ResultDelegate { - public: - explicit ServerPreferredAddressResultDelegateWithWriter( - QuicConnection* connection) - : connection_(connection) {} - - // Overridden to transfer the ownership of the new writer. - void OnPathValidationSuccess( - std::unique_ptr<QuicPathValidationContext> /*context*/, - QuicTime /*start_time*/) override { - // TODO(danzh) hand over the ownership of the released writer to client and - // call the connection to migrate. - } - - void OnPathValidationFailure( - std::unique_ptr<QuicPathValidationContext> context) override { - connection_->OnPathValidationFailureAtClient(/*is_multi_port=*/false, - *context); - } - - private: - QuicConnection* connection_ = nullptr; -}; - -} // namespace - QuicSimpleClientSession::QuicSimpleClientSession( const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, QuicConnection* connection, QuicClientBase::NetworkHelper* network_helper, const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config, QuicClientPushPromiseIndex* push_promise_index, bool drop_response_body, bool enable_web_transport) - : QuicSpdyClientSession(config, supported_versions, connection, server_id, - crypto_config, push_promise_index), + : QuicSimpleClientSession(config, supported_versions, connection, + /*visitor=*/nullptr, network_helper, server_id, + crypto_config, push_promise_index, + drop_response_body, enable_web_transport) {} + +QuicSimpleClientSession::QuicSimpleClientSession( + const QuicConfig& config, const ParsedQuicVersionVector& supported_versions, + QuicConnection* connection, QuicSession::Visitor* visitor, + QuicClientBase::NetworkHelper* network_helper, + const QuicServerId& server_id, QuicCryptoClientConfig* crypto_config, + QuicClientPushPromiseIndex* push_promise_index, bool drop_response_body, + bool enable_web_transport) + : QuicSpdyClientSession(config, supported_versions, connection, visitor, + server_id, crypto_config, push_promise_index), network_helper_(network_helper), drop_response_body_(drop_response_body), enable_web_transport_(enable_web_transport) {} @@ -87,24 +70,4 @@ network_helper_->GetLatestClientAddress(), peer_address()); } -void QuicSimpleClientSession::OnServerPreferredAddressAvailable( - const QuicSocketAddress& server_preferred_address) { - const auto self_address = connection()->self_address(); - if (network_helper_ == nullptr || - !network_helper_->CreateUDPSocketAndBind(server_preferred_address, - self_address.host(), 0)) { - return; - } - QuicPacketWriter* writer = network_helper_->CreateQuicPacketWriter(); - if (writer == nullptr) { - return; - } - connection()->ValidatePath( - std::make_unique<PathMigrationContext>( - std::unique_ptr<QuicPacketWriter>(writer), - network_helper_->GetLatestClientAddress(), server_preferred_address), - std::make_unique<ServerPreferredAddressResultDelegateWithWriter>( - connection())); -} - } // namespace quic
diff --git a/quiche/quic/tools/quic_simple_client_session.h b/quiche/quic/tools/quic_simple_client_session.h index 4e06cc2..1beb062 100644 --- a/quiche/quic/tools/quic_simple_client_session.h +++ b/quiche/quic/tools/quic_simple_client_session.h
@@ -22,13 +22,21 @@ QuicClientPushPromiseIndex* push_promise_index, bool drop_response_body, bool enable_web_transport); + QuicSimpleClientSession(const QuicConfig& config, + const ParsedQuicVersionVector& supported_versions, + QuicConnection* connection, + QuicSession::Visitor* visitor, + QuicClientBase::NetworkHelper* network_helper, + const QuicServerId& server_id, + QuicCryptoClientConfig* crypto_config, + QuicClientPushPromiseIndex* push_promise_index, + bool drop_response_body, bool enable_web_transport); + std::unique_ptr<QuicSpdyClientStream> CreateClientStream() override; bool ShouldNegotiateWebTransport() override; HttpDatagramSupport LocalHttpDatagramSupport() override; std::unique_ptr<QuicPathValidationContext> CreateContextForMultiPortPath() override; - void OnServerPreferredAddressAvailable( - const QuicSocketAddress& server_preferred_address) override; bool drop_response_body() const { return drop_response_body_; } private: