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.