Refactor QuicSentPacketManager::MarkPacketHandled to simplify

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 86aba1d74d5c4cfae735fa4829f356daf22861fe..748d6734d6dcc44a4227352075489adcc5aba763 100644
--- a/net/quic/quic_sent_packet_manager.cc
+++ b/net/quic/quic_sent_packet_manager.cc
@@ -368,11 +368,14 @@ bool QuicSentPacketManager::HasCryptoHandshake(
 
 QuicSentPacketManager::UnackedPacketMap::iterator
 QuicSentPacketManager::MarkPacketHandled(
-    QuicPacketSequenceNumber sequence_number, ReceivedByPeer received_by_peer) {
-  DCHECK(ContainsKey(unacked_packets_, sequence_number));
-
-  // If this packet is pending, remove it and inform the send algorithm.
+    QuicPacketSequenceNumber sequence_number,
+    ReceivedByPeer received_by_peer) {
   UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number);
+  if (it == unacked_packets_.end()) {
+    LOG(DFATAL) << "Packet is not unacked: " << sequence_number;
+    return it;
+  }
+  // If this packet is pending, remove it and inform the send algorithm.
   if (it->second.pending) {
     size_t bytes_sent = packet_history_map_[sequence_number]->bytes_sent();
     if (received_by_peer == RECEIVED_BY_PEER) {
@@ -384,70 +387,56 @@ QuicSentPacketManager::MarkPacketHandled(
     it->second.pending = false;
   }
 
-  // If this packet has never been retransmitted, then simply drop it.
-  if (it->second.previous_transmissions == NULL) {
-    ++it;
-    DiscardPacket(sequence_number);
-    return it;
-  }
-
   SequenceNumberSet* previous_transmissions = it->second.previous_transmissions;
+  if (previous_transmissions == NULL) {
+    previous_transmissions = new SequenceNumberSet;
+    previous_transmissions->insert(sequence_number);
+  }
   DCHECK(!previous_transmissions->empty());
   SequenceNumberSet::reverse_iterator previous_transmissions_it =
       previous_transmissions->rbegin();
   QuicPacketSequenceNumber newest_transmission = *previous_transmissions_it;
-  TransmissionInfo* transmission_info =
-      FindOrNull(unacked_packets_, newest_transmission);
   if (newest_transmission != sequence_number) {
     ++stats_->packets_spuriously_retransmitted;
   }
-  if (newest_transmission == sequence_number) {
-    DiscardPacket(newest_transmission);
-  } else if (HasCryptoHandshake(*transmission_info)) {
-    // If it's a crypto handshake packet, discard it and all retransmissions,
-    // since they won't be acked now that one has been processed.
-    if (transmission_info->pending) {
-      OnPacketAbandoned(unacked_packets_.find(newest_transmission));
-    }
-    DiscardPacket(newest_transmission);
-  } else {
-    // If we have received an ack for a previous transmission of a packet,
-    // we want to keep the "new" transmission of the packet unacked,
-    // but prevent the data from being retransmitted.
-    delete transmission_info->retransmittable_frames;
-    transmission_info->retransmittable_frames = NULL;
-    transmission_info->previous_transmissions = NULL;
-  }
 
-  // Clear out information all previous transmissions unless they're pending.
-  ++previous_transmissions_it;
+  bool has_cryto_handshake = HasCryptoHandshake(
+      *FindOrNull(unacked_packets_, newest_transmission));
+  if (has_cryto_handshake) {
+    --pending_crypto_packet_count_;
+  }
   while (previous_transmissions_it != previous_transmissions->rend()) {
     QuicPacketSequenceNumber previous_transmission = *previous_transmissions_it;
-    ++previous_transmissions_it;
-    // If the packet was TLP retransmitted, the old copy is still pending.
-    // Keep it until it is lost or acked.
-    if (unacked_packets_[previous_transmission].pending) {
-      // Previous transmissions will be deleted, so set it to NULL.
-      unacked_packets_[previous_transmission].previous_transmissions = NULL;
+    TransmissionInfo* transmission_info =
+        FindOrNull(unacked_packets_, previous_transmission);
+    if (transmission_info->retransmittable_frames != NULL) {
+      // Since some version of this packet has been acked, ensure that
+      // the data is not retransmitted again.
+      delete transmission_info->retransmittable_frames;
+      transmission_info->retransmittable_frames = NULL;
+    }
+    if (ContainsKey(pending_retransmissions_, previous_transmission)) {
+      // Don't bother retransmitting this packet, if it has been
+      // marked for retransmission.
+      pending_retransmissions_.erase(previous_transmission);
+    }
+    if (has_cryto_handshake) {
+      // If it's a crypto handshake packet, discard it and all retransmissions,
+      // since they won't be acked now that one has been processed.
+      if (transmission_info->pending) {
+        OnPacketAbandoned(unacked_packets_.find(newest_transmission));
+      }
+      transmission_info->pending = false;
+    }
+    if (!transmission_info->pending) {
+      unacked_packets_.erase(previous_transmission);
     } else {
-      DiscardPacket(previous_transmission);
+      transmission_info->previous_transmissions = NULL;
     }
+    ++previous_transmissions_it;
   }
-
   delete previous_transmissions;
 
-  if (ContainsKey(pending_retransmissions_, newest_transmission)) {
-    pending_retransmissions_.erase(newest_transmission);
-    if (!unacked_packets_[newest_transmission].pending) {
-      // 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) {
@@ -456,27 +445,6 @@ QuicSentPacketManager::MarkPacketHandled(
   return next_unacked;
 }
 
-void QuicSentPacketManager::DiscardPacket(
-    QuicPacketSequenceNumber sequence_number) {
-  UnackedPacketMap::iterator unacked_it =
-      unacked_packets_.find(sequence_number);
-  DCHECK(unacked_it != unacked_packets_.end());
-  // Ensure the packet is no longer pending when it's discarded.
-  DCHECK(!unacked_it->second.pending);
-
-  RetransmittableFrames* retransmittable_frames =
-      unacked_it->second.retransmittable_frames;
-  if (HasCryptoHandshake(unacked_it->second)) {
-    --pending_crypto_packet_count_;
-  }
-
-  // Delete the retransmittable frames.
-  delete retransmittable_frames;
-  unacked_packets_.erase(unacked_it);
-  pending_retransmissions_.erase(sequence_number);
-  return;
-}
-
 bool QuicSentPacketManager::IsUnacked(
     QuicPacketSequenceNumber sequence_number) const {
   return ContainsKey(unacked_packets_, sequence_number);