gfe-relnote: In QUIC, let connection determine handshake confirmed by asking session and stop marking HANDSHAKE_CONFIRMED in sent packet manager. Not affecting prod, not protected.
PiperOrigin-RevId: 291006942
Change-Id: I1caf90ff1dfe638126fdff1fd65abcab48246c46
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index 7ba472a..754db3a 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -2480,8 +2480,7 @@
}
char* QuicConnection::GetPacketBuffer() {
- if (version().CanSendCoalescedPackets() &&
- sent_packet_manager_.handshake_state() < HANDSHAKE_CONFIRMED) {
+ if (version().CanSendCoalescedPackets() && !IsHandshakeConfirmed()) {
// Do not use writer's packet buffer for coalesced packets which may contain
// multiple QUIC packets.
return nullptr;
@@ -4025,8 +4024,7 @@
SerializedPacketFate QuicConnection::DeterminePacketFate(
bool is_mtu_discovery) {
- if (version().CanSendCoalescedPackets() &&
- sent_packet_manager_.handshake_state() < HANDSHAKE_CONFIRMED &&
+ if (version().CanSendCoalescedPackets() && !IsHandshakeConfirmed() &&
!is_mtu_discovery) {
// Before receiving ACK for any 1-RTT packets, always try to coalesce
// packet (except MTU discovery packet).
@@ -4042,6 +4040,11 @@
return SEND_TO_WRITER;
}
+bool QuicConnection::IsHandshakeConfirmed() const {
+ DCHECK_EQ(PROTOCOL_TLS1_3, version().handshake_protocol);
+ return visitor_->GetHandshakeState() == HANDSHAKE_CONFIRMED;
+}
+
size_t QuicConnection::min_received_before_ack_decimation() const {
return uber_received_packet_manager_.min_received_before_ack_decimation();
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 23910e5..ef09e01 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -875,6 +875,9 @@
return sent_packet_manager_.handshake_state() >= HANDSHAKE_COMPLETE;
}
+ // Whether peer completes handshake. Only used with TLS handshake.
+ bool IsHandshakeConfirmed() const;
+
// Returns the largest received packet number sent by peer.
QuicPacketNumber GetLargestReceivedPacket() const;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 9441e86..fc51c79 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1076,7 +1076,8 @@
.WillRepeatedly(Return(QuicTime::Zero()));
EXPECT_CALL(*loss_algorithm_, DetectLosses(_, _, _, _, _, _))
.Times(AnyNumber());
-
+ EXPECT_CALL(visitor_, GetHandshakeState())
+ .WillRepeatedly(Return(HANDSHAKE_START));
if (connection_.version().KnowsWhichDecrypterToUse()) {
connection_.InstallDecrypter(
ENCRYPTION_FORWARD_SECURE,
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc
index 81bbbec..a6b3cab 100644
--- a/quic/core/quic_sent_packet_manager.cc
+++ b/quic/core/quic_sent_packet_manager.cc
@@ -1233,9 +1233,6 @@
return PACKETS_ACKED_IN_WRONG_PACKET_NUMBER_SPACE;
}
last_ack_frame_.packets.Add(acked_packet.packet_number);
- if (info->encryption_level == ENCRYPTION_FORWARD_SECURE) {
- handshake_state_ = HANDSHAKE_CONFIRMED;
- }
largest_packet_peer_knows_is_acked_.UpdateMax(info->largest_acked);
if (supports_multiple_packet_number_spaces()) {
largest_packets_peer_knows_is_acked_[packet_number_space].UpdateMax(
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h
index 69a4d23..62c8b64 100644
--- a/quic/core/quic_sent_packet_manager.h
+++ b/quic/core/quic_sent_packet_manager.h
@@ -577,9 +577,7 @@
PacingSender pacing_sender_;
// Indicates current handshake state.
- // TODO(fayang): Stop inferring handshake state, instead, retrieve handshake
- // state from SessionNotifier::GetHandshakeState and use this variable purely
- // for performance purpose.
+ // TODO(fayang): Change this to a bool.
HandshakeState handshake_state_;
// Records bandwidth from server to client in normal operation, over periods
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc
index 25102a0..f007562 100644
--- a/quic/core/quic_sent_packet_manager_test.cc
+++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -2851,42 +2851,6 @@
EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime());
}
-TEST_F(QuicSentPacketManagerTest, ForwardSecurePacketAcked) {
- EXPECT_LT(manager_.handshake_state(), HANDSHAKE_CONFIRMED);
- SendDataPacket(1, ENCRYPTION_INITIAL);
- // Ack packet 1.
- ExpectAck(1);
- manager_.OnAckFrameStart(QuicPacketNumber(1), QuicTime::Delta::Infinite(),
- clock_.Now());
- manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2));
- EXPECT_EQ(PACKETS_NEWLY_ACKED,
- manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1),
- ENCRYPTION_INITIAL));
- EXPECT_LT(manager_.handshake_state(), HANDSHAKE_CONFIRMED);
-
- SendDataPacket(2, ENCRYPTION_ZERO_RTT);
- // Ack packet 2.
- ExpectAck(2);
- manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(),
- clock_.Now());
- manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(3));
- EXPECT_EQ(PACKETS_NEWLY_ACKED,
- manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2),
- ENCRYPTION_FORWARD_SECURE));
- EXPECT_LT(manager_.handshake_state(), HANDSHAKE_CONFIRMED);
-
- SendDataPacket(3, ENCRYPTION_FORWARD_SECURE);
- // Ack packet 3.
- ExpectAck(3);
- manager_.OnAckFrameStart(QuicPacketNumber(3), QuicTime::Delta::Infinite(),
- clock_.Now());
- manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(4));
- EXPECT_EQ(PACKETS_NEWLY_ACKED,
- manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3),
- ENCRYPTION_FORWARD_SECURE));
- EXPECT_EQ(manager_.handshake_state(), HANDSHAKE_CONFIRMED);
-}
-
TEST_F(QuicSentPacketManagerTest, PtoTimeoutIncludesMaxAckDelay) {
EnablePto(k1PTO);
// Use PTOS and PTOA.
