Land Recent QUIC Changes.

net/quic/quic_unacked_packet_map.h

Index: net/quic/quic_unacked_packet_map.h
diff --git a/net/quic/quic_unacked_packet_map.h b/net/quic/quic_unacked_packet_map.h
index 011760a4807c6dd809899f8849c2252db00985dd..759bd7ba2c3328f7c9897121e93eb101709a92f0 100644
--- a/net/quic/quic_unacked_packet_map.h
+++ b/net/quic/quic_unacked_packet_map.h
@@ -31,9 +31,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
   // Returns true if the packet |sequence_number| is unacked.
   bool IsUnacked(QuicPacketSequenceNumber sequence_number) const;
 
-  // Returns true if the packet |sequence_number| is pending.
-  bool IsPending(QuicPacketSequenceNumber sequence_number) const;
-
   // Sets the nack count to the max of the current nack count and |min_nacks|.
   void NackPacket(QuicPacketSequenceNumber sequence_number,
                   size_t min_nacks);
@@ -72,10 +69,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
   // been acked by the peer.  If there are no unacked packets, returns 0.
   QuicPacketSequenceNumber GetLeastUnackedSentPacket() const;
 
-  // Returns the set of sequence numbers of all unacked packets.
-  // Test only.
-  SequenceNumberSet GetUnackedPackets() const;
-
   // Sets a packet as sent with the sent time |sent_time|.  Marks the packet
   // as pending and tracks the |bytes_sent| if |set_pending| is true.
   // Packets marked as pending are expected to be marked as missing when they
@@ -120,15 +113,21 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap {
   // Returns true if there are any pending crypto packets.
   bool HasPendingCryptoPackets() const;
 
-  // Deletes the retransmittable frames associated with the packet and removes
-  // it from unacked packets if it's not pending.
+  // Removes entries from the unacked packet map, and deletes
+  // the retransmittable frames associated with the packet.
   // Does not remove any previous or subsequent transmissions of this packet.
-  void NeuterIfPendingOrRemovePacket(QuicPacketSequenceNumber sequence_number);
+  void RemovePacket(QuicPacketSequenceNumber sequence_number);
+
+  // Neuters the specified packet.  Deletes any retransmittable
+  // frames, and sets all_transmissions to only include itself.

[wtc, 2014/05/13 18:49:23]

1. Unless we expect readers of this code to know all_transmissions is a member
of the TransmissionInfo struct, it would be nice to clarify that.

2. It's not clear what "itself" refers to. The specified packet or
all_transmissions?

3. Nit: If I only read the comments here, I don't really know the difference
between RemovePacket and NeuterPacket and hence don't know when I should use
which method. Unfortunately I can't suggest a way to clarify this.

[ramant (doing other things), 2014/05/14 05:33:29]

ianswett@: Would like to work with you to make this change in internal source
tree and then merge into chromium.

+  void NeuterPacket(QuicPacketSequenceNumber sequence_number);
 
   // Returns true if the packet has been marked as sent by SetSent.
   static bool IsSentAndNotPending(const TransmissionInfo& transmission_info);
 
  private:
+  void MaybeRemoveRetransmittableFrames(TransmissionInfo* transmission_info);
+
   QuicPacketSequenceNumber largest_sent_packet_;
 
   // Newly serialized retransmittable and fec packets are added to this map,