Land Recent QUIC Changes.

net/quic/quic_unacked_packet_map.h

 // Copyright 2014 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.
 
 #ifndef NET_QUIC_QUIC_UNACKED_PACKET_MAP_H_
 #define NET_QUIC_QUIC_UNACKED_PACKET_MAP_H_
 
 #include "net/base/linked_hash_map.h"
 #include "net/quic/quic_protocol.h"
 
@@ skipping 13 matching lines @@
   // Called when a packet is retransmitted with a new sequence number.
   // |old_sequence_number| will remain unacked, but will have no
   // retransmittable data associated with it. |new_sequence_number| will
   // be both unacked and associated with retransmittable data.
   void OnRetransmittedPacket(QuicPacketSequenceNumber old_sequence_number,
                              QuicPacketSequenceNumber new_sequence_number);
 
   // 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);
 
   // Marks |sequence_number| as no longer pending.
   void SetNotPending(QuicPacketSequenceNumber sequence_number);
 
   // Returns true if the unacked packet |sequence_number| has retransmittable
   // frames.  This will return false if the packet has been acked, if a
   // previous transmission of this packet was ACK'd, or if this packet has been
@@ skipping 18 matching lines @@
 
   // Returns the sum of the bytes in all pending packets.
   QuicByteCount bytes_in_flight() const {
     return bytes_in_flight_;
   }
 
   // Returns the smallest sequence number of a serialized packet which has not
   // 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
   // don't arrive, indicating the need for retransmission.
   void SetSent(QuicPacketSequenceNumber sequence_number,
                QuicTime sent_time,
                QuicByteCount bytes_sent,
                bool set_pending);
 
   // Clears up to |num_to_clear| previous transmissions in order to make room
@@ skipping 24 matching lines @@
 
   // Returns the number of unacked packets.
   size_t GetNumUnackedPackets() const;
 
   // Returns true if there are multiple packet pending.
   bool HasMultiplePendingPackets() const;
 
   // 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,
   // which contains owning pointers to any contained frames.  If a packet is
   // retransmitted, this map will contain entries for both the old and the new
   // packet. The old packet's retransmittable frames entry will be NULL, while
   // the new packet's entry will contain the frames to retransmit.
   // If the old packet is acked before the new packet, then the old entry will
   // be removed from the map and the new entry's retransmittable frames will be
   // set to NULL.
   UnackedPacketMap unacked_packets_;
 
   size_t bytes_in_flight_;
   // Number of outstanding crypto handshake packets.
   size_t pending_crypto_packet_count_;
 
   DISALLOW_COPY_AND_ASSIGN(QuicUnackedPacketMap);
 };
 
 }  // namespace net
 
 #endif  // NET_QUIC_QUIC_UNACKED_PACKET_MAP_H_