From 7eca09361d6d49df7bcb7bdbeba3e030ce73b5c0 Mon Sep 17 00:00:00 2001
From: Steve Anton <steveanton@webrtc.org>
Date: Fri, 30 Mar 2018 15:18:41 -0700
Subject: [PATCH] Ensure that data channel transport stats are included

The RTCStatsCollector was only iterating through RtpTransceivers
in order to find the active transports for which to generate stats.
But for data channel only connections, there were no
RtpTransceivers so no transports were being identified.

This CL changes the stats collector to include the transport names
of the SCTP and RTP data channel if active.

Bug: chromium:826972
Change-Id: I762b253b3bbf0f0d7861bc281b8908decbb9b0d9
Reviewed-on: https://webrtc-review.googlesource.com/65788
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22697}
---
 pc/peerconnection_integrationtest.cc | 22 ++++++++++++++++++++++
 pc/rtcstatscollector.cc              | 28 +++++++++++++++++++++-------
 pc/rtcstatscollector.h               |  2 ++
 3 files changed, 45 insertions(+), 7 deletions(-)

diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index 6dbf32080d..a249d79dd8 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -86,6 +86,7 @@ using RTCConfiguration = PeerConnectionInterface::RTCConfiguration;
 using webrtc::PeerConnectionFactory;
 using webrtc::PeerConnectionProxy;
 using webrtc::RTCErrorType;
+using webrtc::RTCTransportStats;
 using webrtc::RtpSenderInterface;
 using webrtc::RtpReceiverInterface;
 using webrtc::RtpSenderInterface;
@@ -4076,6 +4077,27 @@ TEST_P(PeerConnectionIntegrationTest, ClosingConnectionStopsPacketFlow) {
   EXPECT_EQ(sent_packets_a, sent_packets_b);
 }
 
+// Test that transport stats are generated by the RTCStatsCollector for a
+// connection that only involves data channels. This is a regression test for
+// crbug.com/826972.
+#ifdef HAVE_SCTP
+TEST_P(PeerConnectionIntegrationTest,
+       TransportStatsReportedForDataChannelOnlyConnection) {
+  ASSERT_TRUE(CreatePeerConnectionWrappers());
+  ConnectFakeSignaling();
+  caller()->CreateDataChannel();
+
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  ASSERT_TRUE_WAIT(callee()->data_channel(), kDefaultTimeout);
+
+  auto caller_report = caller()->NewGetStats();
+  EXPECT_EQ(1u, caller_report->GetStatsOfType<RTCTransportStats>().size());
+  auto callee_report = callee()->NewGetStats();
+  EXPECT_EQ(1u, callee_report->GetStatsOfType<RTCTransportStats>().size());
+}
+#endif  // HAVE_SCTP
+
 INSTANTIATE_TEST_CASE_P(PeerConnectionIntegrationTest,
                         PeerConnectionIntegrationTest,
                         Values(SdpSemantics::kPlanB,
diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc
index 2d6a7ac767..7d75ce8e53 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -776,6 +776,9 @@ void RTCStatsCollector::GetStatsReportInternal(
     // |ProducePartialResultsOnNetworkThread| and
     // |ProducePartialResultsOnSignalingThread|.
     transceiver_stats_infos_ = PrepareTransceiverStatsInfos_s();
+    // Prepare |transport_names_| for use in
+    // |ProducePartialResultsOnNetworkThread|.
+    transport_names_ = PrepareTransportNames_s();
 
     // Prepare |call_stats_| here since GetCallStats() will hop to the worker
     // thread.
@@ -827,14 +830,8 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
   rtc::scoped_refptr<RTCStatsReport> report = RTCStatsReport::Create(
       timestamp_us);
 
-  std::set<std::string> transport_names;
-  for (const auto& stats : transceiver_stats_infos_) {
-    if (stats.transport_name) {
-      transport_names.insert(*stats.transport_name);
-    }
-  }
   std::map<std::string, cricket::TransportStats> transport_stats_by_name =
-      pc_->GetTransportStatsByNames(transport_names);
+      pc_->GetTransportStatsByNames(transport_names_);
 
   std::map<std::string, CertificateStatsPair> transport_cert_stats =
       PrepareTransportCertificateStats_n(transport_stats_by_name);
@@ -1483,6 +1480,23 @@ RTCStatsCollector::PrepareTransceiverStatsInfos_s() const {
   return transceiver_stats_infos;
 }
 
+std::set<std::string> RTCStatsCollector::PrepareTransportNames_s() const {
+  std::set<std::string> transport_names;
+  for (const auto& transceiver : pc_->GetTransceiversInternal()) {
+    if (transceiver->internal()->channel()) {
+      transport_names.insert(
+          transceiver->internal()->channel()->transport_name());
+    }
+  }
+  if (pc_->rtp_data_channel()) {
+    transport_names.insert(pc_->rtp_data_channel()->transport_name());
+  }
+  if (pc_->sctp_transport_name()) {
+    transport_names.insert(*pc_->sctp_transport_name());
+  }
+  return transport_names;
+}
+
 void RTCStatsCollector::OnDataChannelCreated(DataChannel* channel) {
   channel->SignalOpened.connect(this, &RTCStatsCollector::OnDataChannelOpened);
   channel->SignalClosed.connect(this, &RTCStatsCollector::OnDataChannelClosed);
diff --git a/pc/rtcstatscollector.h b/pc/rtcstatscollector.h
index 0018d60326..743911e5e7 100644
--- a/pc/rtcstatscollector.h
+++ b/pc/rtcstatscollector.h
@@ -209,6 +209,7 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
       const std::map<std::string, cricket::TransportStats>&
           transport_stats_by_name) const;
   std::vector<RtpTransceiverStatsInfo> PrepareTransceiverStatsInfos_s() const;
+  std::set<std::string> PrepareTransportNames_s() const;
 
   // Slots for signals (sigslot) that are wired up to |pc_|.
   void OnDataChannelCreated(DataChannel* channel);
@@ -232,6 +233,7 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
   // passed as arguments to avoid copies. This is thread safe - when we
   // set/reset we know there are no pending stats requests in progress.
   std::vector<RtpTransceiverStatsInfo> transceiver_stats_infos_;
+  std::set<std::string> transport_names_;
 
   Call::Stats call_stats_;