Expose packet loss counts from QUIC to NetworkQualityEstimator

net/quic/quic_connection_logger.cc

Index: net/quic/quic_connection_logger.cc
diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc
index 96c6425d1620b7a379cc1f8b69126682ce979da2..863b8ec5d601e835f34500aa395c42a0fda3f1f2 100644
--- a/net/quic/quic_connection_logger.cc
+++ b/net/quic/quic_connection_logger.cc
@@ -503,6 +503,9 @@ void QuicConnectionLogger::OnProtocolVersionMismatch(
 void QuicConnectionLogger::OnPacketHeader(const QuicPacketHeader& header) {
   net_log_.AddEvent(NetLog::TYPE_QUIC_SESSION_PACKET_AUTHENTICATED);
   ++num_packets_received_;
+
+  const QuicPacketCount previous_largest_received_packet_number =
+      largest_received_packet_number_;
   if (largest_received_packet_number_ < header.packet_number) {
     QuicPacketNumber delta =
         header.packet_number - largest_received_packet_number_;
@@ -528,6 +531,40 @@ void QuicConnectionLogger::OnPacketHeader(const QuicPacketHeader& header) {
                                                  header.packet_number));
   }
   last_received_packet_number_ = header.packet_number;
+  ReportPacketCounts(previous_largest_received_packet_number);
+}
+
+void QuicConnectionLogger::ReportPacketCounts(
+    QuicPacketCount previous_largest_received_packet_number) const {
+  DCHECK_GE(largest_received_packet_number_,
+            previous_largest_received_packet_number);
+  const uint64_t diff =

[bengr, 2016/02/19 00:45:08]

Move this to after 548

[tbansal1, 2016/02/26 01:15:00]

Done.

+      largest_received_packet_number_ - previous_largest_received_packet_number;
+
+  uint64_t num_packets_lost = 0;
+  uint64_t num_packets_received_in_order = 0;
+  uint64_t num_packets_received_not_in_order = 0;
+
+  if (socket_performance_watcher_) {

[bengr, 2016/02/19 00:45:08]

if (!socket_performance_watcher)
  return;
...

[tbansal1, 2016/02/26 01:15:00]

Done.

+    if (diff == 0) {
+      // The received packet number has alower sequence number than the expected

[bengr, 2016/02/19 00:45:08]

alower -> a lower

[tbansal1, 2016/02/26 01:15:00]

Done.

+      // sequence number.
+      num_packets_received_not_in_order = 1;
+    } else if (diff == 1) {
+      // The received packet number has the same sequence number as the expected
+      // sequence number.
+      num_packets_received_in_order = 1;
+    } else {
+      // The received packet number has higher sequence number as the expected
+      // sequence number. Report all packets with intermediate sequence numbers
+      // as lost.
+      num_packets_lost = diff - 1;
+      num_packets_received_in_order = 1;
+    }
+    socket_performance_watcher_->OnUpdatedPacketCountAvailable(
+        num_packets_lost, num_packets_received_in_order,
+        num_packets_received_not_in_order);

[bengr, 2016/02/19 00:45:08]

I don't think you need "num_" but could be convinced you could replace it with
the suffix "_count"

I'd change "not_in" to "out_of"

[tbansal1, 2016/02/26 01:15:00]

Done. This file uses num_ at multiple places. I was just going with the
convention in this file.

+  }
 }
 
 void QuicConnectionLogger::OnStreamFrame(const QuicStreamFrame& frame) {
@@ -775,8 +812,8 @@ void QuicConnectionLogger::AddTo21CumulativeHistogram(
 float QuicConnectionLogger::ReceivedPacketLossRate() const {
   if (largest_received_packet_number_ <= num_packets_received_)
     return 0.0f;
-  float num_received = largest_received_packet_number_ - num_packets_received_;
-  return num_received / largest_received_packet_number_;
+  float num_lost = largest_received_packet_number_ - num_packets_received_;

[bengr, 2016/02/19 00:45:08]

I don't like "num." Maybe lost_count? Please remove "num_" everywhere.

[tbansal1, 2016/02/26 01:15:00]

num is already there in this code (also, this is not my code). I changed this
line in this CL because I found the variable name to be wrong and confusing
(num_received has almost opposite meaning of num_lost).

Not done.

+  return num_lost / largest_received_packet_number_;
 }
 
 void QuicConnectionLogger::OnRttChanged(QuicTime::Delta rtt) const {