RTCIceCandidatePairStats.[total/current]RoundTripTime collected.

Collected in accordance with the spec:
https://w3c.github.io/webrtc-stats/#candidatepair-dict*

totalRoundTripTime is collected as the sum of rtt measurements, it was
previously not collected.
currentRoundTripTime is collected as the latest rtt measurement, it
was previously collected as a smoothed value, which was incorrect.

Connection is updated to collect these values which are surfaced
through ConnectionInfo.

BUG=webrtc:7062, webrtc:7204

Review-Url: https://codereview.webrtc.org/2719523002
Cr-Commit-Position: refs/heads/master@{#16905}

webrtc/api/stats/rtcstats_objects.h

@@ -144,14 +144,11 @@ class RTCIceCandidatePairStats final : public RTCStats {
   RTCStatsMember<bool> readable;
   RTCStatsMember<uint64_t> bytes_sent;
   RTCStatsMember<uint64_t> bytes_received;
-  // TODO(hbos): Collect and populate this value. https://bugs.webrtc.org/7062
   RTCStatsMember<double> total_round_trip_time;
-  // TODO(hbos): Collect this the way the spec describes it. We have a value for
-  // it but it is not spec-compliant. https://bugs.webrtc.org/7062
   RTCStatsMember<double> current_round_trip_time;
   RTCStatsMember<double> available_outgoing_bitrate;
   // TODO(hbos): Populate this value. It is wired up and collected the same way
-  // |VideoBwe.googAvailableReceiveBandwidth| is, but that value is always
+  // "VideoBwe.googAvailableReceiveBandwidth" is, but that value is always
   // undefined. https://bugs.webrtc.org/7062
   RTCStatsMember<double> available_incoming_bitrate;
   RTCStatsMember<uint64_t> requests_received;

webrtc/p2p/base/jseptransport.cc

@@ -61,7 +61,8 @@ ConnectionInfo::ConnectionInfo()
       key(nullptr),
       state(IceCandidatePairState::WAITING),
       priority(0),
-      nominated(false) {}
+      nominated(false),
+      total_round_trip_time_ms(0) {}
 
 bool BadTransportDescription(const std::string& desc, std::string* err_desc) {
   if (err_desc) {

webrtc/p2p/base/jseptransport.h

@@ -115,6 +115,10 @@ struct ConnectionInfo {
   uint64_t priority;
   // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-nominated
   bool nominated;
+  // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-totalroundtriptime
+  uint64_t total_round_trip_time_ms;
+  // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentroundtriptime
+  rtc::Optional<uint32_t> current_round_trip_time_ms;
 };
 
 // Information about all the connections of a channel.

webrtc/p2p/base/port.cc

@@ -1251,6 +1251,7 @@ void Connection::ReceivedPing() {
 }
 
 void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) {
+  RTC_DCHECK_GE(rtt, 0);
   // We've already validated that this is a STUN binding response with
   // the correct local and remote username for this connection.
   // So if we're not already, become writable. We may be bringing a pruned
@@ -1264,6 +1265,10 @@ void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) {
     acked_nomination_ = iter->nomination;
   }
 
+  total_round_trip_time_ms_ += rtt;
+  current_round_trip_time_ms_ = rtc::Optional<uint32_t>(
+      static_cast<uint32_t>(rtt));
+
   pings_since_last_response_.clear();
   last_ping_response_received_ = rtc::TimeMillis();
   UpdateReceiving(last_ping_response_received_);
@@ -1504,6 +1509,8 @@ ConnectionInfo Connection::stats() {
   stats_.state = state_;
   stats_.priority = priority();
   stats_.nominated = nominated();
+  stats_.total_round_trip_time_ms = total_round_trip_time_ms_;
+  stats_.current_round_trip_time_ms = current_round_trip_time_ms_;
   return stats_;
 }
 

webrtc/p2p/base/port.h

@@ -27,6 +27,7 @@
 #include "webrtc/base/asyncpacketsocket.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/network.h"
+#include "webrtc/base/optional.h"
 #include "webrtc/base/proxyinfo.h"
 #include "webrtc/base/ratetracker.h"
 #include "webrtc/base/sigslot.h"
@@ -706,6 +707,10 @@ class Connection : public CandidatePairInterface,
   StunRequestManager requests_;
   int rtt_;
   int rtt_samples_ = 0;
+  // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-totalroundtriptime
+  uint64_t total_round_trip_time_ms_ = 0;
+  // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentroundtriptime
+  rtc::Optional<uint32_t> current_round_trip_time_ms_;
   int64_t last_ping_sent_;      // last time we sent a ping to the other side
   int64_t last_ping_received_;  // last time we received a ping from the other
                                 // side

webrtc/p2p/base/port_unittest.cc

@@ -222,6 +222,23 @@ class TestPort : public Port {
   int type_preference_ = 0;
 };
 
+static void SendPingAndReceiveResponse(
+    Connection* lconn, TestPort* lport, Connection* rconn, TestPort* rport,
+    rtc::ScopedFakeClock* clock, int64_t ms) {
+  lconn->Ping(rtc::TimeMillis());
+  ASSERT_TRUE_WAIT(lport->last_stun_msg(), kDefaultTimeout);
+  ASSERT_TRUE(lport->last_stun_buf());
+  rconn->OnReadPacket(lport->last_stun_buf()->data<char>(),
+                      lport->last_stun_buf()->size(),
+                      rtc::PacketTime());
+  clock->AdvanceTime(rtc::TimeDelta::FromMilliseconds(ms));
+  ASSERT_TRUE_WAIT(rport->last_stun_msg(), kDefaultTimeout);
+  ASSERT_TRUE(rport->last_stun_buf());
+  lconn->OnReadPacket(rport->last_stun_buf()->data<char>(),
+                      rport->last_stun_buf()->size(),
+                      rtc::PacketTime());
+}
+
 class TestChannel : public sigslot::has_slots<> {
  public:
   // Takes ownership of |p1| (but not |p2|).
@@ -1844,6 +1861,49 @@ TEST_F(PortTest, TestNomination) {
   EXPECT_EQ(rconn->nominated(), rconn->stats().nominated);
 }
 
+TEST_F(PortTest, TestRoundTripTime) {
+  rtc::ScopedFakeClock clock;
+
+  std::unique_ptr<TestPort> lport(
+      CreateTestPort(kLocalAddr1, "lfrag", "lpass"));
+  std::unique_ptr<TestPort> rport(
+      CreateTestPort(kLocalAddr2, "rfrag", "rpass"));
+  lport->SetIceRole(cricket::ICEROLE_CONTROLLING);
+  lport->SetIceTiebreaker(kTiebreaker1);
+  rport->SetIceRole(cricket::ICEROLE_CONTROLLED);
+  rport->SetIceTiebreaker(kTiebreaker2);
+
+  lport->PrepareAddress();
+  rport->PrepareAddress();
+  ASSERT_FALSE(lport->Candidates().empty());
+  ASSERT_FALSE(rport->Candidates().empty());
+  Connection* lconn = lport->CreateConnection(rport->Candidates()[0],
+                                              Port::ORIGIN_MESSAGE);
+  Connection* rconn = rport->CreateConnection(lport->Candidates()[0],
+                                              Port::ORIGIN_MESSAGE);
+
+  EXPECT_EQ(0u, lconn->stats().total_round_trip_time_ms);
+  EXPECT_FALSE(lconn->stats().current_round_trip_time_ms);
+
+  SendPingAndReceiveResponse(
+      lconn, lport.get(), rconn, rport.get(), &clock, 10);
+  EXPECT_EQ(10u, lconn->stats().total_round_trip_time_ms);
+  ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+  EXPECT_EQ(10u, *lconn->stats().current_round_trip_time_ms);
+
+  SendPingAndReceiveResponse(
+      lconn, lport.get(), rconn, rport.get(), &clock, 20);
+  EXPECT_EQ(30u, lconn->stats().total_round_trip_time_ms);
+  ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+  EXPECT_EQ(20u, *lconn->stats().current_round_trip_time_ms);
+
+  SendPingAndReceiveResponse(
+      lconn, lport.get(), rconn, rport.get(), &clock, 30);
+  EXPECT_EQ(60u, lconn->stats().total_round_trip_time_ms);
+  ASSERT_TRUE(lconn->stats().current_round_trip_time_ms);
+  EXPECT_EQ(30u, *lconn->stats().current_round_trip_time_ms);
+}
+
 TEST_F(PortTest, TestUseCandidateAttribute) {
   std::unique_ptr<TestPort> lport(
       CreateTestPort(kLocalAddr1, "lfrag", "lpass"));

webrtc/pc/rtcstats_integrationtest.cc

@@ -372,7 +372,8 @@ class RTCStatsReportVerifier {
     verifier.TestMemberIsUndefined(candidate_pair.readable);
     verifier.TestMemberIsNonNegative<uint64_t>(candidate_pair.bytes_sent);
     verifier.TestMemberIsNonNegative<uint64_t>(candidate_pair.bytes_received);
-    verifier.TestMemberIsUndefined(candidate_pair.total_round_trip_time);
+    verifier.TestMemberIsNonNegative<double>(
+        candidate_pair.total_round_trip_time);
     verifier.TestMemberIsNonNegative<double>(
         candidate_pair.current_round_trip_time);
     if (is_selected_pair) {

webrtc/pc/rtcstatscollector.cc

@@ -875,11 +875,14 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
             static_cast<uint64_t>(info.sent_total_bytes);
         candidate_pair_stats->bytes_received =
             static_cast<uint64_t>(info.recv_total_bytes);
-        // TODO(hbos): The |info.rtt| measurement is smoothed. It shouldn't be
-        // smoothed according to the spec. crbug.com/633550. See
-        // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentrtt
-        candidate_pair_stats->current_round_trip_time =
-            static_cast<double>(info.rtt) / rtc::kNumMillisecsPerSec;
+        candidate_pair_stats->total_round_trip_time =
+            static_cast<double>(info.total_round_trip_time_ms) /
+            rtc::kNumMillisecsPerSec;
+        if (info.current_round_trip_time_ms) {
+          candidate_pair_stats->current_round_trip_time =
+              static_cast<double>(*info.current_round_trip_time_ms) /
+              rtc::kNumMillisecsPerSec;
+        }
         if (info.best_connection && video_media_info &&
             !video_media_info->bw_estimations.empty()) {
           // The bandwidth estimations we have are for the selected candidate

webrtc/pc/rtcstatscollector_unittest.cc

@@ -1241,7 +1241,8 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) {
   connection_info.writable = true;
   connection_info.sent_total_bytes = 42;
   connection_info.recv_total_bytes = 1234;
-  connection_info.rtt = 1337;
+  connection_info.total_round_trip_time_ms = 0;
+  connection_info.current_round_trip_time_ms = rtc::Optional<uint32_t>();
   connection_info.recv_ping_requests = 2020;
   connection_info.sent_ping_requests_total = 2020;
   connection_info.sent_ping_requests_before_first_response = 2000;
@@ -1294,12 +1295,14 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) {
   expected_pair.writable = true;
   expected_pair.bytes_sent = 42;
   expected_pair.bytes_received = 1234;
-  expected_pair.current_round_trip_time = 1.337;
+  expected_pair.total_round_trip_time = 0.0;
   expected_pair.requests_received = 2020;
   expected_pair.requests_sent = 2000;
   expected_pair.responses_received = 4321;
   expected_pair.responses_sent = 1000;
   expected_pair.consent_requests_sent = (2020 - 2000);
+  // |expected_pair.current_round_trip_time| should be undefined because the
+  // current RTT is not set.
   // |expected_pair.available_[outgoing/incoming]_bitrate| should be undefined
   // because is is not the current pair.
 
@@ -1325,6 +1328,24 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) {
       report->Get(expected_pair.id())->cast_to<RTCIceCandidatePairStats>());
   EXPECT_TRUE(report->Get(*expected_pair.transport_id));
 
+  // Set round trip times and "GetStats" again.
+  session_stats.transport_stats["transport"].channel_stats[0]
+      .connection_infos[0].total_round_trip_time_ms = 7331;
+  session_stats.transport_stats["transport"].channel_stats[0]
+      .connection_infos[0].current_round_trip_time_ms =
+          rtc::Optional<uint32_t>(1337);
+  EXPECT_CALL(*video_media_channel, GetStats(_))
+      .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true)));
+  collector_->ClearCachedStatsReport();
+  report = GetStatsReport();
+  expected_pair.total_round_trip_time = 7.331;
+  expected_pair.current_round_trip_time = 1.337;
+  ASSERT_TRUE(report->Get(expected_pair.id()));
+  EXPECT_EQ(
+      expected_pair,
+      report->Get(expected_pair.id())->cast_to<RTCIceCandidatePairStats>());
+  EXPECT_TRUE(report->Get(*expected_pair.transport_id));
+
   // Make pair the current pair, clear bandwidth and "GetStats" again.
   session_stats.transport_stats["transport"]
       .channel_stats[0]