Fix the logic for computing the number of truncated QUIC ACK frames sent.

Review URL: https://codereview.chromium.org/549433006

Cr-Commit-Position: refs/heads/master@{#294661}

net/net.gypi

@@ -1527,6 +1527,7 @@
       'quic/quic_clock_test.cc',
       'quic/quic_config_test.cc',
       'quic/quic_connection_helper_test.cc',
+      'quic/quic_connection_logger_unittest.cc',
       'quic/quic_connection_test.cc',
       'quic/quic_crypto_client_stream_test.cc',
       'quic/quic_crypto_server_stream_test.cc',

net/quic/quic_connection_logger.cc

@@ -392,13 +392,32 @@ void QuicConnectionLogger::OnFrameAddedToPacket(const QuicFrame& frame) {
           NetLog::TYPE_QUIC_SESSION_STREAM_FRAME_SENT,
           base::Bind(&NetLogQuicStreamFrameCallback, frame.stream_frame));
       break;
-    case ACK_FRAME:
+    case ACK_FRAME: {
       net_log_.AddEvent(
           NetLog::TYPE_QUIC_SESSION_ACK_FRAME_SENT,
           base::Bind(&NetLogQuicAckFrameCallback, frame.ack_frame));
-      if (frame.ack_frame->is_truncated)
-        ++num_truncated_acks_sent_;
+      const SequenceNumberSet& missing_packets =
+          frame.ack_frame->missing_packets;
+      const uint8 max_ranges = std::numeric_limits<uint8>::max();
+      // Compute an upper bound on the number of NACK ranges. If the bound
+      // is below the max, then it clearly isn't truncated.
+      if (missing_packets.size() < max_ranges ||
+          (*missing_packets.rbegin() - *missing_packets.begin() -
+           missing_packets.size() + 1) < max_ranges) {
+        break;
+      }
+      size_t num_ranges = 0;
+      QuicPacketSequenceNumber last_missing = 0;
+      for (SequenceNumberSet::const_iterator it = missing_packets.begin();
+           it != missing_packets.end(); ++it) {
+        if (*it != last_missing + 1 && ++num_ranges >= max_ranges) {
+          ++num_truncated_acks_sent_;
+          break;
+        }
+        last_missing = *it;
+      }
       break;
+    }
     case CONGESTION_FEEDBACK_FRAME:
       net_log_.AddEvent(
           NetLog::TYPE_QUIC_SESSION_CONGESTION_FEEDBACK_FRAME_SENT,

net/quic/quic_connection_logger.h

@@ -14,6 +14,9 @@
 #include "net/quic/quic_protocol.h"
 
 namespace net {
+namespace test {
+class QuicConnectionLoggerPeer;
+}  // namespace test
 
 class CryptoHandshakeMessage;
 class CertVerifyResult;
@@ -81,6 +84,8 @@ class NET_EXPORT_PRIVATE QuicConnectionLogger
   void OnCertificateVerified(const CertVerifyResult& result);
 
  private:
+  friend class test::QuicConnectionLoggerPeer;
+
   // Do a factory get for a histogram for recording data, about individual
   // packet sequence numbers, that was gathered in the vectors
   // received_packets_ and received_acks_. |statistic_name| identifies which

net/quic/quic_connection_logger_unittest.cc

+// 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.
+
+#include "net/quic/quic_connection_logger.h"
+
+#include "net/quic/quic_protocol.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace net {
+namespace test {
+
+class QuicConnectionLoggerPeer {
+ public:
+  static size_t num_truncated_acks_sent(const QuicConnectionLogger& logger) {
+    return logger.num_truncated_acks_sent_;
+  }
+};
+
+class QuicConnectionLoggerTest : public ::testing::Test {
+ protected:
+  QuicConnectionLoggerTest() : logger_(net_log_) {}
+
+  BoundNetLog net_log_;
+  QuicConnectionLogger logger_;
+};
+
+TEST_F(QuicConnectionLoggerTest, TruncatedAcksSentNotChanged) {
+  QuicAckFrame frame;
+  logger_.OnFrameAddedToPacket(QuicFrame(&frame));
+  EXPECT_EQ(0u, QuicConnectionLoggerPeer::num_truncated_acks_sent(logger_));
+
+  for (QuicPacketSequenceNumber i = 0; i < 256; ++i) {
+    frame.missing_packets.insert(i);
+  }
+  logger_.OnFrameAddedToPacket(QuicFrame(&frame));
+  EXPECT_EQ(0u, QuicConnectionLoggerPeer::num_truncated_acks_sent(logger_));
+}
+
+TEST_F(QuicConnectionLoggerTest, TruncatedAcksSent) {
+  QuicAckFrame frame;
+  for (QuicPacketSequenceNumber i = 0; i < 512; i += 2) {
+    frame.missing_packets.insert(i);
+  }
+  logger_.OnFrameAddedToPacket(QuicFrame(&frame));
+  EXPECT_EQ(1u, QuicConnectionLoggerPeer::num_truncated_acks_sent(logger_));
+}
+
+}  // namespace test
+}  // namespace net