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.
