Fixing a really obvious (in hindsight) bug in the quic truncation code which was incorrectly written and incorrectly tested.

Merge internal change: 40312314

BUG=


Review URL: https://chromiumcodereview.appspot.com/11818002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@176281 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 15 insertions, 11 deletions

diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc
index 9e2f4c5..37a7b47 100644
--- a/net/quic/quic_framer.cc
+++ b/net/quic/quic_framer.cc
@@ -769,14 +769,11 @@ QuicPacketSequenceNumber QuicFramer::CalculateLargestReceived(
   QuicPacketSequenceNumber previous_missing = *it;
   ++it;
 
-  // Try to find a gap in the missing packets: any gap indicates a non-missing
-  // packet which we can then return.
-  for (; it != missing_packets.end(); ++it) {
-    if (previous_missing + 1 != *it) {
+  // See if the next thing is a gap in the missing packets: if it's a
+  // non-missing packet we can return it.
+  if (it != missing_packets.end() && previous_missing + 1 != *it) {
       return *it - 1;
     }
-    previous_missing = *it;
-  }
 
   // If we've hit the end of the list, and we're not missing any packets, try
   // finding a gap between the largest written and the beginning of the set.
diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc
index 25c3f39..22eae23 100644
--- a/net/quic/quic_framer_test.cc
+++ b/net/quic/quic_framer_test.cc
@@ -1543,6 +1543,13 @@ TEST_F(QuicFramerTest, CalculateLargestReceived) {
   EXPECT_EQ(4u, QuicFramer::CalculateLargestReceived(missing, missing.find(1)));
   EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(5)));
 
+  missing.insert(2);
+  // For 1, we can't go forward as 2 would be implicitly acked.  We must go
+  // backwards.
+  EXPECT_EQ(0u, QuicFramer::CalculateLargestReceived(missing, missing.find(1)));
+  // For 2, we've seen 3 and 4, so can admit to a largest received.
+  EXPECT_EQ(4u, QuicFramer::CalculateLargestReceived(missing, missing.find(2)));
+
   // 7+ are fun because there is no subsequent gap: we have to walk before
   // them to find 6.
   EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(7)));
@@ -1560,12 +1567,12 @@ TEST_F(QuicFramerTest, CalculateLargestReceived) {
   missing.insert(2);
   EXPECT_EQ(0u, QuicFramer::CalculateLargestReceived(missing, missing.find(7)));
 
-  // If we add a gap after 7-9, we will return that.
+  // If we add a gap after 7-9, we will return that only for 9.
   missing.insert(11);
-  EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
-                                                      missing.find(7)));
-  EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
-                                                      missing.find(8)));
+  EXPECT_EQ(0u, QuicFramer::CalculateLargestReceived(missing,
+                                                     missing.find(7)));
+  EXPECT_EQ(0u, QuicFramer::CalculateLargestReceived(missing,
+                                                     missing.find(8)));
   EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
                                                       missing.find(9)));
 }