Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(397)

Unified Diff: net/quic/quic_sent_packet_manager.cc

Issue 125183003: Correctly handle NACK-based "retransmission" of packets which no longer (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/quic/quic_sent_packet_manager.h ('k') | net/quic/quic_sent_packet_manager_test.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/quic/quic_sent_packet_manager.cc
diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc
index b81f594cd7d009a0e0ba78ba0e8775d63e94dea3..c7a5663f0eea7cddbe7b3acfd63cb5102c5f8339 100644
--- a/net/quic/quic_sent_packet_manager.cc
+++ b/net/quic/quic_sent_packet_manager.cc
@@ -418,7 +418,6 @@ QuicSentPacketManager::MarkPacketHandled(
delete newest_it->second.retransmittable_frames;
newest_it->second.retransmittable_frames = NULL;
newest_it->second.previous_transmissions = NULL;
- pending_retransmissions_.erase(newest_transmission);
}
// Clear out information all previous transmissions.
@@ -437,6 +436,18 @@ QuicSentPacketManager::MarkPacketHandled(
delete previous_transmissions;
+ if (ContainsKey(pending_retransmissions_, newest_transmission)) {
+ pending_retransmissions_.erase(newest_transmission);
+ if (!ContainsKey(pending_packets_, newest_transmission)) {
+ // If the newest transmission has already been marked for retransmission
+ // and has already been abandoned, then we should remove it from
+ // unacked_packets_, as well as cancel the retransmission.
+ DCHECK(ContainsKey(unacked_packets_, newest_transmission));
+ DCHECK(!unacked_packets_[newest_transmission].previous_transmissions);
+ unacked_packets_.erase(newest_transmission);
+ }
+ }
+
UnackedPacketMap::iterator next_unacked = unacked_packets_.begin();
while (next_unacked != unacked_packets_.end() &&
next_unacked->first < sequence_number) {
@@ -740,7 +751,14 @@ void QuicSentPacketManager::MaybeRetransmitOnAckFrame(
lost_packets.insert(sequence_number);
if (transmission_info.retransmittable_frames) {
++num_retransmitted;
- MarkForRetransmission(*it, NACK_RETRANSMISSION);
+ MarkForRetransmission(sequence_number, NACK_RETRANSMISSION);
+ } else {
+ // Since we will not retransmit this, we need to remove it from
+ // unacked_packets_. This is either the current transmission of
+ // a packet whose previous transmission has been acked, or it
+ // is a packet that has been TLP retransmitted.
+ RemovePreviousTransmission(sequence_number);
+ unacked_packets_.erase(sequence_number);
}
++it;
@@ -755,8 +773,9 @@ void QuicSentPacketManager::MaybeRetransmitOnAckFrame(
// repair the loss, it should be recorded as a loss to the congestion
// manager, but not retransmitted until it's known whether the FEC packet
// arrived.
- send_algorithm_->OnPacketLost(*it, ack_receive_time);
- OnPacketAbandoned(*it);
+ QuicPacketSequenceNumber lost_packet = *it;
+ send_algorithm_->OnPacketLost(lost_packet, ack_receive_time);
+ OnPacketAbandoned(lost_packet);
}
}
@@ -937,4 +956,19 @@ void QuicSentPacketManager::MaybeEnablePacing() {
QuicTime::Delta::FromMicroseconds(1)));
}
+void QuicSentPacketManager::RemovePreviousTransmission(
+ QuicPacketSequenceNumber sequence_number) {
+ SequenceNumberSet* previous_transmissions =
+ unacked_packets_[sequence_number].previous_transmissions;
+ if (!previous_transmissions) {
+ return;
+ }
+ previous_transmissions->erase(sequence_number);
+ if (previous_transmissions->size() == 1) {
+ QuicPacketSequenceNumber current = *previous_transmissions->begin();
+ unacked_packets_[current].previous_transmissions = NULL;
+ delete previous_transmissions;
+ }
+}
+
} // namespace net
« no previous file with comments | « net/quic/quic_sent_packet_manager.h ('k') | net/quic/quic_sent_packet_manager_test.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698