Remove ALL_UNACKED_RETRANSMISSION from TransmissionType because it's not used anymore. Unused code. not protected. PiperOrigin-RevId: 313307566 Change-Id: I615b2526c713e84c9bb14b24c1bd491dc8faeca7
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index 0190967..d39aa1e 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -2183,9 +2183,8 @@ } } -void QuicConnection::RetransmitUnackedPackets( - TransmissionType retransmission_type) { - sent_packet_manager_.RetransmitUnackedPackets(retransmission_type); +void QuicConnection::RetransmitZeroRttPackets() { + sent_packet_manager_.RetransmitZeroRttPackets(); WriteIfNotBlocked(); }
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index 9339fb0..c78cf0b 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -710,12 +710,9 @@ // remaining unacked packets. void OnRetransmissionTimeout(); - // Retransmits all unacked packets with retransmittable frames if - // |retransmission_type| is ALL_UNACKED_PACKETS, otherwise retransmits only - // initially encrypted packets. Used when the negotiated protocol version is - // different from what was initially assumed and when the initial encryption - // changes. - void RetransmitUnackedPackets(TransmissionType retransmission_type); + // Retransmits all sent 0-RTT encrypted packets. Called when new 0-RTT or + // 1-RTT key is available. + void RetransmitZeroRttPackets(); // Calls |sent_packet_manager_|'s NeuterUnencryptedPackets. Used when the // connection becomes forward secure and hasn't received acks for all packets.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index 34a4b4c..714db95 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -4476,47 +4476,6 @@ } } -TEST_P(QuicConnectionTest, RetransmitWithSameEncryptionLevel) { - use_tagging_decrypter(); - - // A TaggingEncrypter puts kTagSize copies of the given byte (0x01 here) at - // the end of the packet. We can test this to check which encrypter was used. - connection_.SetEncrypter(ENCRYPTION_INITIAL, - std::make_unique<TaggingEncrypter>(0x01)); - QuicByteCount packet_size; - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)) - .WillOnce(SaveArg<3>(&packet_size)); - connection_.SendCryptoDataWithString("foo", 0); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(AnyNumber()); - EXPECT_EQ(0x01010101u, writer_->final_bytes_of_last_packet()); - - connection_.SetEncrypter(ENCRYPTION_ZERO_RTT, - std::make_unique<TaggingEncrypter>(0x02)); - connection_.SetDefaultEncryptionLevel(ENCRYPTION_ZERO_RTT); - SendStreamDataToPeer(3, "foo", 0, NO_FIN, nullptr); - EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); - - { - InSequence s; - EXPECT_CALL(*send_algorithm_, - OnPacketSent(_, _, QuicPacketNumber(3), _, _)); - EXPECT_CALL(*send_algorithm_, - OnPacketSent(_, _, QuicPacketNumber(4), _, _)); - } - - // Manually mark both packets for retransmission. - connection_.RetransmitUnackedPackets(ALL_UNACKED_RETRANSMISSION); - if (!connection_.version().CanSendCoalescedPackets()) { - // Packet should have been sent with ENCRYPTION_INITIAL. - // If connection can send coalesced packet, both retransmissions will be - // coalesced in the same UDP datagram. - EXPECT_EQ(0x01010101u, writer_->final_bytes_of_previous_packet()); - } - - // Packet should have been sent with ENCRYPTION_ZERO_RTT. - EXPECT_EQ(0x02020202u, writer_->final_bytes_of_last_packet()); -} - TEST_P(QuicConnectionTest, SendHandshakeMessages) { use_tagging_decrypter(); // A TaggingEncrypter puts kTagSize copies of the given byte (0x01 here) at @@ -4590,7 +4549,7 @@ SendStreamDataToPeer(2, "bar", 0, NO_FIN, nullptr); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - connection_.RetransmitUnackedPackets(ALL_INITIAL_RETRANSMISSION); + connection_.RetransmitZeroRttPackets(); } TEST_P(QuicConnectionTest, BufferNonDecryptablePackets) {
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 3941a3e..b407b0b 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -453,30 +453,21 @@ } } -void QuicSentPacketManager::RetransmitUnackedPackets( - TransmissionType retransmission_type) { - DCHECK(retransmission_type == ALL_UNACKED_RETRANSMISSION || - retransmission_type == ALL_INITIAL_RETRANSMISSION); +void QuicSentPacketManager::RetransmitZeroRttPackets() { QuicPacketNumber packet_number = unacked_packets_.GetLeastUnacked(); for (QuicUnackedPacketMap::iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { - if ((retransmission_type == ALL_UNACKED_RETRANSMISSION || - it->encryption_level == ENCRYPTION_ZERO_RTT)) { + if (it->encryption_level == ENCRYPTION_ZERO_RTT) { if (it->in_flight) { // Remove 0-RTT packets and packets of the wrong version from flight, // because neither can be processed by the peer. unacked_packets_.RemoveFromInFlight(&*it); } if (unacked_packets_.HasRetransmittableFrames(*it)) { - MarkForRetransmission(packet_number, retransmission_type); + MarkForRetransmission(packet_number, ALL_INITIAL_RETRANSMISSION); } } } - if (retransmission_type == ALL_UNACKED_RETRANSMISSION && - unacked_packets_.bytes_in_flight() > 0) { - QUIC_BUG << "RetransmitUnackedPackets should remove all packets from flight" - << ", bytes_in_flight:" << unacked_packets_.bytes_in_flight(); - } } void QuicSentPacketManager::NeuterUnencryptedPackets() {
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index 371fa81..ac007fe 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -140,15 +140,11 @@ // TODO(fayang): Rename this function to OnHandshakeComplete. void SetHandshakeConfirmed(); - // Requests retransmission of all unacked packets of |retransmission_type|. - // The behavior of this method depends on the value of |retransmission_type|: - // ALL_UNACKED_RETRANSMISSION - All unacked packets will be retransmitted. - // This can happen, for example, after a version negotiation packet has been - // received and all packets needs to be retransmitted with the new version. - // ALL_INITIAL_RETRANSMISSION - Only initially encrypted packets will be - // retransmitted. This can happen, for example, when a CHLO has been rejected - // and the previously encrypted data needs to be encrypted with a new key. - void RetransmitUnackedPackets(TransmissionType retransmission_type); + // Requests retransmission of all unacked 0-RTT packets. + // Only initially encrypted packets will be retransmitted. This can happen, + // for example, when a CHLO has been rejected and the previously encrypted + // data needs to be encrypted with a new key. + void RetransmitZeroRttPackets(); // Notify the sent packet manager of an external network measurement or // prediction for either |bandwidth| or |rtt|; either can be empty.
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index 7125986..c0cbef9 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -1021,54 +1021,6 @@ EXPECT_FALSE(manager_.HasUnackedCryptoPackets()); } -TEST_F(QuicSentPacketManagerTest, CryptoHandshakeTimeoutVersionNegotiation) { - // Send 2 crypto packets and 3 data packets. - const size_t kNumSentCryptoPackets = 2; - for (size_t i = 1; i <= kNumSentCryptoPackets; ++i) { - SendCryptoPacket(i); - } - const size_t kNumSentDataPackets = 3; - for (size_t i = 1; i <= kNumSentDataPackets; ++i) { - SendDataPacket(kNumSentCryptoPackets + i); - } - EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); - - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(6); })) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(7); })); - manager_.OnRetransmissionTimeout(); - EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); - - // Now act like a version negotiation packet arrived, which would cause all - // unacked packets to be retransmitted. - // Mark packets [1, 7] lost. And the frames in 6 and 7 are same as packets 1 - // and 2, respectively. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(7); - manager_.RetransmitUnackedPackets(ALL_UNACKED_RETRANSMISSION); - - // Ensure the first two pending packets are the crypto retransmits. - RetransmitCryptoPacket(8); - RetransmitCryptoPacket(9); - RetransmitDataPacket(10, ALL_UNACKED_RETRANSMISSION); - RetransmitDataPacket(11, ALL_UNACKED_RETRANSMISSION); - RetransmitDataPacket(12, ALL_UNACKED_RETRANSMISSION); - - EXPECT_EQ(QuicPacketNumber(1u), manager_.GetLeastUnacked()); - // Least unacked isn't raised until an ack is received, so ack the - // crypto packets. - uint64_t acked[] = {8, 9}; - ExpectAcksAndLosses(true, acked, QUICHE_ARRAYSIZE(acked), nullptr, 0); - manager_.OnAckFrameStart(QuicPacketNumber(9), QuicTime::Delta::Infinite(), - clock_.Now()); - manager_.OnAckRange(QuicPacketNumber(8), QuicPacketNumber(10)); - EXPECT_EQ(PACKETS_NEWLY_ACKED, - manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), - ENCRYPTION_INITIAL)); - EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(false)); - EXPECT_EQ(QuicPacketNumber(10u), manager_.GetLeastUnacked()); -} - TEST_F(QuicSentPacketManagerTest, CryptoHandshakeSpuriousRetransmission) { // Send 1 crypto packet. SendCryptoPacket(1); @@ -1121,28 +1073,6 @@ } TEST_F(QuicSentPacketManagerTest, - CryptoHandshakeRetransmissionThenRetransmitAll) { - // Send 1 crypto packet. - SendCryptoPacket(1); - - EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); - - // Retransmit the crypto packet as 2. - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - manager_.OnRetransmissionTimeout(); - // Now retransmit all the unacked packets, which occurs when there is a - // version negotiation. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(2); - manager_.RetransmitUnackedPackets(ALL_UNACKED_RETRANSMISSION); - // Both packets 1 and 2 are unackable. - EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(1))); - EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(2))); - EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); - EXPECT_FALSE(manager_.HasInFlightPackets()); -} - -TEST_F(QuicSentPacketManagerTest, CryptoHandshakeRetransmissionThenNeuterAndAck) { // Send 1 crypto packet. SendCryptoPacket(1);
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index bfcd169..f019522 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -1537,7 +1537,7 @@ if (perspective() == Perspective::IS_CLIENT) { // Retransmit old 0-RTT data (if any) with the new 0-RTT keys, since // they can't be decrypted by the server. - connection_->RetransmitUnackedPackets(ALL_INITIAL_RETRANSMISSION); + connection_->RetransmitZeroRttPackets(); // Given any streams blocked by encryption a chance to write. OnCanWrite(); }
diff --git a/quic/core/quic_types.cc b/quic/core/quic_types.cc index 36614cd..b44ef84 100644 --- a/quic/core/quic_types.cc +++ b/quic/core/quic_types.cc
@@ -173,7 +173,6 @@ switch (transmission_type) { RETURN_STRING_LITERAL(NOT_RETRANSMISSION); RETURN_STRING_LITERAL(HANDSHAKE_RETRANSMISSION); - RETURN_STRING_LITERAL(ALL_UNACKED_RETRANSMISSION); RETURN_STRING_LITERAL(ALL_INITIAL_RETRANSMISSION); RETURN_STRING_LITERAL(LOSS_RETRANSMISSION); RETURN_STRING_LITERAL(RTO_RETRANSMISSION);
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h index e00ebb5..da7ea3b 100644 --- a/quic/core/quic_types.h +++ b/quic/core/quic_types.h
@@ -164,8 +164,6 @@ NOT_RETRANSMISSION, FIRST_TRANSMISSION_TYPE = NOT_RETRANSMISSION, HANDSHAKE_RETRANSMISSION, // Retransmits due to handshake timeouts. - // TODO(fayang): remove ALL_UNACKED_RETRANSMISSION. - ALL_UNACKED_RETRANSMISSION, // Retransmits all unacked packets. ALL_INITIAL_RETRANSMISSION, // Retransmits all initially encrypted packets. LOSS_RETRANSMISSION, // Retransmits due to loss detection. RTO_RETRANSMISSION, // Retransmits due to retransmit time out.
diff --git a/quic/core/quic_utils.cc b/quic/core/quic_utils.cc index 2d851d8..15424b6 100644 --- a/quic/core/quic_utils.cc +++ b/quic/core/quic_utils.cc
@@ -336,7 +336,6 @@ SentPacketState QuicUtils::RetransmissionTypeToPacketState( TransmissionType retransmission_type) { switch (retransmission_type) { - case ALL_UNACKED_RETRANSMISSION: case ALL_INITIAL_RETRANSMISSION: return UNACKABLE; case HANDSHAKE_RETRANSMISSION:
diff --git a/quic/core/quic_utils_test.cc b/quic/core/quic_utils_test.cc index 082a9bd..812c83e 100644 --- a/quic/core/quic_utils_test.cc +++ b/quic/core/quic_utils_test.cc
@@ -125,8 +125,7 @@ EXPECT_EQ(HANDSHAKE_RETRANSMITTED, state); } else if (i == LOSS_RETRANSMISSION) { EXPECT_EQ(LOST, state); - } else if (i == ALL_UNACKED_RETRANSMISSION || - i == ALL_INITIAL_RETRANSMISSION) { + } else if (i == ALL_INITIAL_RETRANSMISSION) { EXPECT_EQ(UNACKABLE, state); } else if (i == TLP_RETRANSMISSION) { EXPECT_EQ(TLP_RETRANSMITTED, state);