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}
diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index 6dbf320..a249d79 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -86,6 +86,7 @@
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 @@
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 2d6a7ac..7d75ce8 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -776,6 +776,9 @@
// |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 @@
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 @@
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 0018d60..743911e 100644
--- a/pc/rtcstatscollector.h
+++ b/pc/rtcstatscollector.h
@@ -209,6 +209,7 @@
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 @@
// 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_;