From e27d3ea3f81787b5042e2a9ff261f7ddc033a3fb Mon Sep 17 00:00:00 2001
From: Marten Seemann <martenseemann@gmail.com>
Date: Tue, 26 Apr 2016 22:38:10 +0700
Subject: [PATCH] ReceivedPacketHandler only generates ACKs if packets were
 received after the last ACK

---
 ackhandler/received_packet_handler.go      |  9 ++++
 ackhandler/received_packet_handler_test.go | 49 ++++++++++++++++++++++
 2 files changed, 58 insertions(+)

diff --git a/ackhandler/received_packet_handler.go b/ackhandler/received_packet_handler.go
index afe039756..2aeecb860 100644
--- a/ackhandler/received_packet_handler.go
+++ b/ackhandler/received_packet_handler.go
@@ -14,6 +14,7 @@ type receivedPacketHandler struct {
 	highestInOrderObservedEntropy EntropyAccumulator
 	largestObserved               protocol.PacketNumber
 	packetHistory                 map[protocol.PacketNumber]bool // the bool is the EntropyBit of the packet
+	stateChanged                  bool                           // has an ACK for this state already been sent? Will be set to false every time a new packet arrives, and to false every time an ACK is sent
 }
 
 // NewReceivedPacketHandler creates a new receivedPacketHandler
@@ -31,6 +32,8 @@ func (h *receivedPacketHandler) ReceivedPacket(packetNumber protocol.PacketNumbe
 		return ErrDuplicatePacket
 	}
 
+	h.stateChanged = true
+
 	if packetNumber > h.largestObserved {
 		h.largestObserved = packetNumber
 	}
@@ -84,6 +87,12 @@ func (h *receivedPacketHandler) getNackRanges() ([]frames.NackRange, EntropyAccu
 }
 
 func (h *receivedPacketHandler) DequeueAckFrame() *frames.AckFrame {
+	if !h.stateChanged {
+		return nil
+	}
+
+	h.stateChanged = false
+
 	nackRanges, entropy := h.getNackRanges()
 	return &frames.AckFrame{
 		LargestObserved: h.largestObserved,
diff --git a/ackhandler/received_packet_handler_test.go b/ackhandler/received_packet_handler_test.go
index 7939cb7fa..06815529e 100644
--- a/ackhandler/received_packet_handler_test.go
+++ b/ackhandler/received_packet_handler_test.go
@@ -229,4 +229,53 @@ var _ = Describe("receivedPacketHandler", func() {
 			Expect(ranges).To(HaveLen(0))
 		})
 	})
+
+	Context("ACK package generation", func() {
+		It("generates a simple ACK frame", func() {
+			entropy := EntropyAccumulator(0)
+			entropy.Add(1, true)
+			entropy.Add(2, true)
+			err := handler.ReceivedPacket(protocol.PacketNumber(1), true)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(protocol.PacketNumber(2), true)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.DequeueAckFrame()).To(Equal(&frames.AckFrame{LargestObserved: 2, Entropy: byte(entropy)}))
+		})
+
+		It("generates an ACK frame with a NACK range", func() {
+			entropy := EntropyAccumulator(0)
+			entropy.Add(1, true)
+			entropy.Add(4, true)
+			err := handler.ReceivedPacket(protocol.PacketNumber(1), true)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(protocol.PacketNumber(4), true)
+			Expect(err).ToNot(HaveOccurred())
+			expectedAck := frames.AckFrame{
+				LargestObserved: 4,
+				Entropy:         byte(entropy),
+				NackRanges:      []frames.NackRange{frames.NackRange{FirstPacketNumber: 2, LastPacketNumber: 3}},
+			}
+			Expect(handler.DequeueAckFrame()).To(Equal(&expectedAck))
+		})
+
+		It("does not generate an ACK if an ACK has already been sent for the largest Packet", func() {
+			err := handler.ReceivedPacket(protocol.PacketNumber(1), false)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(protocol.PacketNumber(2), false)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.DequeueAckFrame()).ToNot(BeNil())
+			Expect(handler.DequeueAckFrame()).To(BeNil())
+		})
+
+		It("generates an ACK when an out-of-order packet arrives", func() {
+			err := handler.ReceivedPacket(protocol.PacketNumber(1), false)
+			Expect(err).ToNot(HaveOccurred())
+			err = handler.ReceivedPacket(protocol.PacketNumber(3), false)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.DequeueAckFrame()).ToNot(BeNil())
+			err = handler.ReceivedPacket(protocol.PacketNumber(2), false)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(handler.DequeueAckFrame()).ToNot(BeNil())
+		})
+	})
 })