Land Recent QUIC changes.

net/quic/quic_ack_notifier_manager.h

Index: net/quic/quic_ack_notifier_manager.h
diff --git a/net/quic/quic_ack_notifier_manager.h b/net/quic/quic_ack_notifier_manager.h
index f3a7f63211452fe928de6d6472d307b1d1860161..ecba1be7bd5605d96bb4ae24af67a9a249a83453 100644
--- a/net/quic/quic_ack_notifier_manager.h
+++ b/net/quic/quic_ack_notifier_manager.h
@@ -5,20 +5,21 @@
 #ifndef NET_QUIC_QUIC_ACK_NOTIFIER_MANAGER_H_
 #define NET_QUIC_QUIC_ACK_NOTIFIER_MANAGER_H_
 
-#include <list>
 #include <map>
 #include <set>
 
+#include "base/compiler_specific.h"
+#include "base/containers/hash_tables.h"

[wtc, 2013/10/01 14:06:59]

Do we still need these two headers? It seems that you tried to use
base/containers/hash_tables.h but failed.

[ramant (doing other things), 2013/10/01 22:29:32]

Fixed the problem with hash_tables.h not compiling.

 #include "net/quic/quic_protocol.h"
 
 namespace net {
 
 class QuicAckNotifier;
 
-// The AckNotifierManager is used by the QuicConnection to keep track of all the
-// AckNotifiers currently active. It owns the AckNotifiers which it gets from
-// the serialized packets passed into OnSerializedPacket. It maintains both a
-// list of AckNotifiers and a map from sequence number to AckNotifier the sake
+// The AckNotifierManager is used by the QuicSentPacketManager to keep track of
+// all the AckNotifiers currently active. It owns the AckNotifiers which it gets
+// from the serialized packets passed into OnSerializedPacket. It maintains both
+// a list of AckNotifiers and a map from sequence number to AckNotifier the sake
 // of efficiency - we can quickly check the map to see if any AckNotifiers are
 // interested in a given sequence number.
 
@@ -27,7 +28,7 @@ class NET_EXPORT_PRIVATE AckNotifierManager {
   AckNotifierManager();
   virtual ~AckNotifierManager();
 
-  // Called from QuicConnection when it receives a new AckFrame. For each packet
+  // Called when the connection receives a new AckFrame. For each packet
   // in |acked_packets|, if the packet sequence number exists in
   // ack_notifier_map_ then the corresponding AckNotifiers will have their OnAck
   // method called.
@@ -39,27 +40,25 @@ class NET_EXPORT_PRIVATE AckNotifierManager {
   void UpdateSequenceNumber(QuicPacketSequenceNumber old_sequence_number,
                             QuicPacketSequenceNumber new_sequence_number);
 
-  // This is called after a packet has been serialized and is ready to be sent.
-  // If any of the frames in |serialized_packet| have AckNotifiers registered,
-  // then add them to our internal map and additionally inform the AckNotifier
-  // of the sequence number which it should track.
+  // This is called after a packet has been serialized, is ready to be sent, and
+  // contains retransmittable frames (which may have associated AckNotifiers).
+  // If any of the retransmittable frames included in |serialized_packet| have
+  // AckNotifiers registered, then add them to our internal map and additionally
+  // inform the AckNotifier of the sequence number which it should track.
   void OnSerializedPacket(const SerializedPacket& serialized_packet);
 
-  // Called from QuicConnection when data is sent which the sender would like to
-  // be notified on receipt of all ACKs. Adds the |notifier| to our map.
-  void AddAckNotifier(QuicAckNotifier* notifier);
-
  private:
-  typedef std::list<QuicAckNotifier*> AckNotifierList;
+  // TODO(rtenneti): base::hash_set doesn't compile and is not fully supported
+  // until C++11.

[wtc, 2013/10/01 14:06:59]

Nit: this comment needs more context. I don't know what it is referring to.

[ramant (doing other things), 2013/10/01 22:29:32]

Deleted the comment.

   typedef std::set<QuicAckNotifier*> AckNotifierSet;
   typedef std::map<QuicPacketSequenceNumber, AckNotifierSet> AckNotifierMap;
 
-  // On every ACK frame received by this connection, all the ack_notifiers_ will
+  // On every ACK frame received by the connection, all the ack_notifiers_ will
   // be told which sequeunce numbers were ACKed.
   // Once a given QuicAckNotifier has seen all the sequence numbers it is
   // interested in, it will be deleted, and removed from this list.
   // Owns the AckNotifiers in this list.

[wtc, 2013/10/01 14:06:59]

Nit: should we change all occurrences of "list" in this comment to "set"?

[ramant (doing other things), 2013/10/01 22:29:32]

Done.

-  AckNotifierList ack_notifiers_;
+  AckNotifierSet ack_notifiers_;
 
   // Maps from sequence number to the AckNotifiers which are registered
   // for that sequence number. On receipt of an ACK for a given sequence