Refactor QuicSentPacketManager::MarkPacketHandled to simplify

net/quic/quic_sent_packet_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/quic/quic_sent_packet_manager.h"
 
 #include "base/logging.h"
 #include "base/stl_util.h"
 #include "net/quic/congestion_control/pacing_sender.h"
 #include "net/quic/crypto/crypto_protocol.h"
@@ skipping 350 matching lines @@
     const TransmissionInfo& transmission_info) {
   if (transmission_info.retransmittable_frames == NULL) {
     return false;
   }
   return transmission_info.retransmittable_frames->HasCryptoHandshake() ==
       IS_HANDSHAKE;
 }
 
 QuicSentPacketManager::UnackedPacketMap::iterator
 QuicSentPacketManager::MarkPacketHandled(
-    QuicPacketSequenceNumber sequence_number, ReceivedByPeer received_by_peer) {
-  DCHECK(ContainsKey(unacked_packets_, sequence_number));
-
+    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.
-  UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number);
   if (it->second.pending) {
     size_t bytes_sent = packet_history_map_[sequence_number]->bytes_sent();
     if (received_by_peer == RECEIVED_BY_PEER) {
       send_algorithm_->OnPacketAcked(sequence_number, bytes_sent);
     } else {
       // It's been abandoned.
       send_algorithm_->OnPacketAbandoned(sequence_number, bytes_sent);
     }
     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);
   }
-
-  SequenceNumberSet* previous_transmissions = it->second.previous_transmissions;
   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;
+
+  bool has_cryto_handshake = HasCryptoHandshake(
+      *FindOrNull(unacked_packets_, newest_transmission));
+  if (has_cryto_handshake) {
+    --pending_crypto_packet_count_;
   }
-
-  // Clear out information all previous transmissions unless they're pending.
-  ++previous_transmissions_it;
   while (previous_transmissions_it != previous_transmissions->rend()) {
     QuicPacketSequenceNumber previous_transmission = *previous_transmissions_it;
+    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 {
+      transmission_info->previous_transmissions = NULL;
+    }
     ++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;
-    } else {
-      DiscardPacket(previous_transmission);
-    }
   }
-
   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) {
     ++next_unacked;
   }
   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);
 }
 
 bool QuicSentPacketManager::HasUnackedPackets() const {
   return !unacked_packets_.empty();
 }
 
 bool QuicSentPacketManager::HasPendingPackets() const {
@@ skipping 480 matching lines @@
   }
   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