gfe-relnote: In QUIC, let connection determine handshake complete by asking session. Protected by gfe2_reloadable_flag_quic_use_get_handshake_state. Also change handshake_state_ to handshake_finished_ in sent packet manager. PiperOrigin-RevId: 291129658 Change-Id: I3ad4f1aad39d8eeb3761471f8f5e7f4cb4f18aac
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 754db3a..2c3e43c 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -4040,6 +4040,15 @@ return SEND_TO_WRITER; } +bool QuicConnection::IsHandshakeComplete() const { + if (use_handshake_delegate_ && + GetQuicReloadableFlag(quic_use_get_handshake_state)) { + QUIC_RELOADABLE_FLAG_COUNT(quic_use_get_handshake_state); + return visitor_->GetHandshakeState() >= HANDSHAKE_COMPLETE; + } + return sent_packet_manager_.handshake_finished(); +} + bool QuicConnection::IsHandshakeConfirmed() const { DCHECK_EQ(PROTOCOL_TLS1_3, version().handshake_protocol); return visitor_->GetHandshakeState() == HANDSHAKE_CONFIRMED;
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index ef09e01..7fc528f 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -871,9 +871,7 @@ }; // Whether the handshake completes from this connection's perspective. - bool IsHandshakeComplete() const { - return sent_packet_manager_.handshake_state() >= HANDSHAKE_COMPLETE; - } + bool IsHandshakeComplete() const; // Whether peer completes handshake. Only used with TLS handshake. bool IsHandshakeConfirmed() const;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index fc51c79..0cb109e 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -7984,6 +7984,11 @@ EXPECT_CALL(*send_algorithm_, OnApplicationLimited(_)).Times(0); ASSERT_EQ(0u, connection_.GetStats().packets_sent); connection_.set_fill_up_link_during_probing(true); + if (GetQuicReloadableFlag(quic_use_handshaker_delegate2) && + GetQuicReloadableFlag(quic_use_get_handshake_state)) { + EXPECT_CALL(visitor_, GetHandshakeState()) + .WillRepeatedly(Return(HANDSHAKE_COMPLETE)); + } connection_.OnHandshakeComplete(); connection_.SendStreamData3();
diff --git a/quic/core/quic_dispatcher_test.cc b/quic/core/quic_dispatcher_test.cc index b3ec02c..ac35793 100644 --- a/quic/core/quic_dispatcher_test.cc +++ b/quic/core/quic_dispatcher_test.cc
@@ -68,8 +68,9 @@ nullptr, nullptr, crypto_config, - compressed_certs_cache), - crypto_stream_(QuicServerSessionBase::GetMutableCryptoStream()) {} + compressed_certs_cache) { + Initialize(); + } TestQuicSpdyServerSession(const TestQuicSpdyServerSession&) = delete; TestQuicSpdyServerSession& operator=(const TestQuicSpdyServerSession&) = delete; @@ -91,24 +92,9 @@ stream_helper()); } - void SetCryptoStream(QuicCryptoServerStream* crypto_stream) { - crypto_stream_ = crypto_stream; - } - - QuicCryptoServerStreamBase* GetMutableCryptoStream() override { - return crypto_stream_; - } - - const QuicCryptoServerStreamBase* GetCryptoStream() const override { - return crypto_stream_; - } - QuicCryptoServerStream::Helper* stream_helper() { return QuicServerSessionBase::stream_helper(); } - - private: - QuicCryptoServerStreamBase* crypto_stream_; }; class TestDispatcher : public QuicDispatcher {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index a6b3cab..f4b7973 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -92,7 +92,7 @@ min_rto_timeout_( QuicTime::Delta::FromMilliseconds(kMinRetransmissionTimeMs)), largest_mtu_acked_(0), - handshake_state_(HANDSHAKE_START), + handshake_finished_(false), peer_max_ack_delay_( QuicTime::Delta::FromMilliseconds(kDefaultDelayedAckTimeMs)), rtt_updated_(false), @@ -344,8 +344,8 @@ } void QuicSentPacketManager::SetHandshakeConfirmed() { - if (handshake_state_ < HANDSHAKE_COMPLETE) { - handshake_state_ = HANDSHAKE_COMPLETE; + if (!handshake_finished_) { + handshake_finished_ = true; NeuterHandshakePackets(); } } @@ -668,7 +668,7 @@ QuicSentPacketManager::RetransmissionTimeoutMode QuicSentPacketManager::OnRetransmissionTimeout() { DCHECK(unacked_packets_.HasInFlightPackets() || - (handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE)); + (handshake_mode_disabled_ && !handshake_finished_)); DCHECK_EQ(0u, pending_timer_transmission_count_); // Handshake retransmission, timer based loss detection, TLP, and RTO are // implemented with a single alarm. The handshake alarm is set when the @@ -846,8 +846,8 @@ QuicSentPacketManager::RetransmissionTimeoutMode QuicSentPacketManager::GetRetransmissionMode() const { DCHECK(unacked_packets_.HasInFlightPackets() || - (handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE)); - if (!handshake_mode_disabled_ && handshake_state_ < HANDSHAKE_COMPLETE && + (handshake_mode_disabled_ && !handshake_finished_)); + if (!handshake_mode_disabled_ && !handshake_finished_ && unacked_packets_.HasPendingCryptoPackets()) { return HANDSHAKE_MODE; } @@ -932,7 +932,7 @@ const QuicTime QuicSentPacketManager::GetRetransmissionTime() const { if (!unacked_packets_.HasInFlightPackets() && - (!handshake_mode_disabled_ || handshake_state_ >= HANDSHAKE_COMPLETE || + (!handshake_mode_disabled_ || handshake_finished_ || unacked_packets_.perspective() == Perspective::IS_SERVER)) { // Do not set the timer if there is nothing in flight. However, to avoid // handshake deadlock due to anti-amplification limit, client needs to set
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 62c8b64..fdbd697 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -345,7 +345,9 @@ return largest_packet_peer_knows_is_acked_; } - HandshakeState handshake_state() const { return handshake_state_; } + // TODO(fayang): Stop exposing this when deprecating + // quic_use_get_handshake_state. + bool handshake_finished() const { return handshake_finished_; } size_t pending_timer_transmission_count() const { return pending_timer_transmission_count_; @@ -576,9 +578,9 @@ // Calls into |send_algorithm_| for the underlying congestion control. PacingSender pacing_sender_; - // Indicates current handshake state. - // TODO(fayang): Change this to a bool. - HandshakeState handshake_state_; + // Indicates whether handshake is finished. This is purely used to determine + // retransmission mode. DONOT use this to infer handshake state. + bool handshake_finished_; // Records bandwidth from server to client in normal operation, over periods // of time with no loss events.