Correctly handle NACK-based "retransmission" of packets which no longer

net/quic/quic_sent_packet_manager.cc

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