Collecting RTCIceCandidatePairStats.transport_id and improved unittests.
RTCIceCandidatePairStats.transport_id is set to the related
RTCTransportStats' id.
Unittest for RTCIceCandidatePairStats is updated to do EXPECT_EQ
between actual and an expected hardcoded dictionary. The previous way of
testing, ExpectReportContainsCandidatePair, is removed.
(ExpectReportContainsCandidate still exist, we might want to replace
this by EXPECT_EQ testing in a follow up.)
Unittest for RTCTransportStats is similarly updated and
ExpectReportContainsTransportStats is removed. A bug was uncovered where
the "rtcp_connection_info.best_connection = true" case was not tested
(a copy of rtcp_connection_info was used in the test, modifying that had
no affect on the test) - fixed.
rtcstats_integrationtest.cc updated to take transport_id into account.
In order to reuse an updated version of expected_rt[c]p_transport in the
unittest, timestamps are ignored by RTCStats::operator==.
BUG=chromium:627816, chromium:653873, chromium:653873, webrtc:6755
Review-Url: https://codereview.webrtc.org/2527113002
Cr-Commit-Position: refs/heads/master@{#15316}
diff --git a/webrtc/api/rtcstats_integrationtest.cc b/webrtc/api/rtcstats_integrationtest.cc
index a74d19f..a20f5b4 100644
--- a/webrtc/api/rtcstats_integrationtest.cc
+++ b/webrtc/api/rtcstats_integrationtest.cc
@@ -321,7 +321,8 @@
bool VerifyRTCIceCandidatePairStats(
const RTCIceCandidatePairStats& candidate_pair) {
RTCStatsVerifier verifier(report_, &candidate_pair);
- verifier.TestMemberIsUndefined(candidate_pair.transport_id);
+ verifier.TestMemberIsIDReference(
+ candidate_pair.transport_id, RTCTransportStats::kType);
verifier.TestMemberIsIDReference(
candidate_pair.local_candidate_id, RTCLocalIceCandidateStats::kType);
verifier.TestMemberIsIDReference(
diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc
index 0260e01..ec7cbe0 100644
--- a/webrtc/api/rtcstatscollector.cc
+++ b/webrtc/api/rtcstatscollector.cc
@@ -603,6 +603,8 @@
RTC_DCHECK(signaling_thread_->IsCurrent());
for (const auto& transport_stats : session_stats.transport_stats) {
for (const auto& channel_stats : transport_stats.second.channel_stats) {
+ std::string transport_id = RTCTransportStatsIDFromTransportChannel(
+ transport_stats.second.transport_name, channel_stats.component);
for (const cricket::ConnectionInfo& info :
channel_stats.connection_infos) {
std::unique_ptr<RTCIceCandidatePairStats> candidate_pair_stats(
@@ -610,6 +612,7 @@
RTCIceCandidatePairStatsIDFromConnectionInfo(info),
timestamp_us));
+ candidate_pair_stats->transport_id = transport_id;
// TODO(hbos): There could be other candidates that are not paired with
// anything. We don't have a complete list. Local candidates come from
// Port objects, and prflx candidates (both local and remote) are only
@@ -618,7 +621,6 @@
timestamp_us, info.local_candidate, true, report);
candidate_pair_stats->remote_candidate_id = ProduceIceCandidateStats(
timestamp_us, info.remote_candidate, false, report);
-
// TODO(hbos): This writable is different than the spec. It goes to
// false after a certain amount of time without a response passes.
// crbug.com/633550
diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc
index e4c3118..37356e0 100644
--- a/webrtc/api/rtcstatscollector_unittest.cc
+++ b/webrtc/api/rtcstatscollector_unittest.cc
@@ -516,68 +516,6 @@
return candidate_stats;
}
- void ExpectReportContainsCandidatePair(
- const rtc::scoped_refptr<const RTCStatsReport>& report,
- const cricket::TransportStats& transport_stats) {
- for (const auto& channel_stats : transport_stats.channel_stats) {
- for (const cricket::ConnectionInfo& info :
- channel_stats.connection_infos) {
- const std::string& id = "RTCIceCandidatePair_" +
- info.local_candidate.id() + "_" + info.remote_candidate.id();
- const RTCStats* stats = report->Get(id);
- ASSERT_TRUE(stats);
- const RTCIceCandidatePairStats& candidate_pair_stats =
- stats->cast_to<RTCIceCandidatePairStats>();
-
- // TODO(hbos): Define all the undefined |candidate_pair_stats| stats.
- // The EXPECT_FALSE are for the undefined stats, see also todos listed
- // in rtcstats_objects.h. crbug.com/633550
- EXPECT_FALSE(candidate_pair_stats.transport_id.is_defined());
- const RTCIceCandidateStats* local_candidate =
- ExpectReportContainsCandidate(report, info.local_candidate, true);
- EXPECT_EQ(*candidate_pair_stats.local_candidate_id,
- local_candidate->id());
- const RTCIceCandidateStats* remote_candidate =
- ExpectReportContainsCandidate(report, info.remote_candidate, false);
- EXPECT_EQ(*candidate_pair_stats.remote_candidate_id,
- remote_candidate->id());
-
- EXPECT_FALSE(candidate_pair_stats.state.is_defined());
- EXPECT_FALSE(candidate_pair_stats.priority.is_defined());
- EXPECT_FALSE(candidate_pair_stats.nominated.is_defined());
- EXPECT_EQ(*candidate_pair_stats.writable, info.writable);
- EXPECT_FALSE(candidate_pair_stats.readable.is_defined());
- EXPECT_EQ(*candidate_pair_stats.bytes_sent,
- static_cast<uint64_t>(info.sent_total_bytes));
- EXPECT_EQ(*candidate_pair_stats.bytes_received,
- static_cast<uint64_t>(info.recv_total_bytes));
- EXPECT_FALSE(candidate_pair_stats.total_rtt.is_defined());
- EXPECT_EQ(*candidate_pair_stats.current_rtt,
- static_cast<double>(info.rtt) / 1000.0);
- EXPECT_FALSE(
- candidate_pair_stats.available_outgoing_bitrate.is_defined());
- EXPECT_FALSE(
- candidate_pair_stats.available_incoming_bitrate.is_defined());
- EXPECT_FALSE(candidate_pair_stats.requests_received.is_defined());
- EXPECT_EQ(*candidate_pair_stats.requests_sent,
- static_cast<uint64_t>(info.sent_ping_requests_total));
- EXPECT_EQ(*candidate_pair_stats.responses_received,
- static_cast<uint64_t>(info.recv_ping_responses));
- EXPECT_EQ(*candidate_pair_stats.responses_sent,
- static_cast<uint64_t>(info.sent_ping_responses));
- EXPECT_FALSE(
- candidate_pair_stats.retransmissions_received.is_defined());
- EXPECT_FALSE(candidate_pair_stats.retransmissions_sent.is_defined());
- EXPECT_FALSE(
- candidate_pair_stats.consent_requests_received.is_defined());
- EXPECT_FALSE(candidate_pair_stats.consent_requests_sent.is_defined());
- EXPECT_FALSE(
- candidate_pair_stats.consent_responses_received.is_defined());
- EXPECT_FALSE(candidate_pair_stats.consent_responses_sent.is_defined());
- }
- }
- }
-
void ExpectReportContainsCertificateInfo(
const rtc::scoped_refptr<const RTCStatsReport>& report,
const CertificateInfo& cert_info) {
@@ -599,72 +537,6 @@
}
}
- void ExpectReportContainsTransportStats(
- const rtc::scoped_refptr<const RTCStatsReport>& report,
- const cricket::TransportStats& transport,
- const CertificateInfo* local_certinfo,
- const CertificateInfo* remote_certinfo) {
- std::string rtcp_transport_stats_id;
- for (const auto& channel_stats : transport.channel_stats) {
- if (channel_stats.component == cricket::ICE_CANDIDATE_COMPONENT_RTCP) {
- rtcp_transport_stats_id = "RTCTransport_" + transport.transport_name +
- "_" + rtc::ToString<>(cricket::ICE_CANDIDATE_COMPONENT_RTCP);
- }
- }
- for (const auto& channel_stats : transport.channel_stats) {
- const cricket::ConnectionInfo* best_connection_info = nullptr;
- const RTCStats* stats = report->Get(
- "RTCTransport_" + transport.transport_name + "_" +
- rtc::ToString<>(channel_stats.component));
- ASSERT_TRUE(stats);
- const RTCTransportStats& transport_stats =
- stats->cast_to<const RTCTransportStats>();
- uint64_t bytes_sent = 0;
- uint64_t bytes_received = 0;
- for (const cricket::ConnectionInfo& info :
- channel_stats.connection_infos) {
- bytes_sent += info.sent_total_bytes;
- bytes_received += info.recv_total_bytes;
- if (info.best_connection)
- best_connection_info = &info;
- }
- EXPECT_EQ(*transport_stats.bytes_sent, bytes_sent);
- EXPECT_EQ(*transport_stats.bytes_received, bytes_received);
- if (best_connection_info) {
- EXPECT_EQ(*transport_stats.active_connection, true);
- // TODO(hbos): Instead of testing how the ID looks, test that the
- // corresponding pair's IP addresses are equal to the IP addresses of
- // the |best_connection_info| data. crbug.com/653873
- EXPECT_EQ(*transport_stats.selected_candidate_pair_id,
- "RTCIceCandidatePair_" +
- best_connection_info->local_candidate.id() + "_" +
- best_connection_info->remote_candidate.id());
- EXPECT_TRUE(report->Get(*transport_stats.selected_candidate_pair_id));
- } else {
- EXPECT_EQ(*transport_stats.active_connection, false);
- EXPECT_FALSE(transport_stats.selected_candidate_pair_id.is_defined());
- }
- if (channel_stats.component != cricket::ICE_CANDIDATE_COMPONENT_RTCP &&
- !rtcp_transport_stats_id.empty()) {
- EXPECT_EQ(*transport_stats.rtcp_transport_stats_id,
- rtcp_transport_stats_id);
- } else {
- EXPECT_FALSE(transport_stats.rtcp_transport_stats_id.is_defined());
- }
- if (local_certinfo && remote_certinfo) {
- EXPECT_EQ(*transport_stats.local_certificate_id,
- "RTCCertificate_" + local_certinfo->fingerprints[0]);
- EXPECT_EQ(*transport_stats.remote_certificate_id,
- "RTCCertificate_" + remote_certinfo->fingerprints[0]);
- EXPECT_TRUE(report->Get(*transport_stats.local_certificate_id));
- EXPECT_TRUE(report->Get(*transport_stats.remote_certificate_id));
- } else {
- EXPECT_FALSE(transport_stats.local_certificate_id.is_defined());
- EXPECT_FALSE(transport_stats.remote_certificate_id.is_defined());
- }
- }
- }
-
void ExpectReportContainsDataChannel(
const rtc::scoped_refptr<const RTCStatsReport>& report,
const DataChannel& data_channel) {
@@ -1138,6 +1010,7 @@
connection_info.sent_ping_responses = 1000;
cricket::TransportChannelStats transport_channel_stats;
+ transport_channel_stats.component = cricket::ICE_CANDIDATE_COMPONENT_RTP;
transport_channel_stats.connection_infos.push_back(connection_info);
session_stats.transport_stats["transport"].transport_name = "transport";
session_stats.transport_stats["transport"].channel_stats.push_back(
@@ -1151,8 +1024,35 @@
}));
rtc::scoped_refptr<const RTCStatsReport> report = GetStatsReport();
- ExpectReportContainsCandidatePair(
- report, session_stats.transport_stats["transport"]);
+
+ RTCIceCandidatePairStats expected_pair("RTCIceCandidatePair_" +
+ local_candidate->id() + "_" +
+ remote_candidate->id(),
+ report->timestamp_us());
+ expected_pair.transport_id =
+ "RTCTransport_transport_" +
+ rtc::ToString<>(cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ expected_pair.local_candidate_id = "RTCIceCandidate_" + local_candidate->id();
+ expected_pair.remote_candidate_id =
+ "RTCIceCandidate_" + remote_candidate->id();
+ expected_pair.writable = true;
+ expected_pair.bytes_sent = 42;
+ expected_pair.bytes_received = 1234;
+ expected_pair.current_rtt = 1.337;
+ expected_pair.requests_sent = 1010;
+ expected_pair.responses_received = 4321;
+ expected_pair.responses_sent = 1000;
+
+ EXPECT_TRUE(report->Get(expected_pair.id()));
+ EXPECT_EQ(
+ expected_pair,
+ report->Get(expected_pair.id())->cast_to<RTCIceCandidatePairStats>());
+
+ EXPECT_TRUE(report->Get(*expected_pair.local_candidate_id));
+ ExpectReportContainsCandidate(report, connection_info.local_candidate, true);
+ EXPECT_TRUE(report->Get(*expected_pair.remote_candidate_id));
+ ExpectReportContainsCandidate(report, connection_info.remote_candidate,
+ false);
}
TEST_F(RTCStatsCollectorTest, CollectRTCPeerConnectionStats) {
@@ -1713,8 +1613,19 @@
// Get stats without RTCP, an active connection or certificates.
rtc::scoped_refptr<const RTCStatsReport> report = GetStatsReport();
- ExpectReportContainsTransportStats(
- report, session_stats.transport_stats["transport"], nullptr, nullptr);
+
+ RTCTransportStats expected_rtp_transport(
+ "RTCTransport_transport_" +
+ rtc::ToString<>(cricket::ICE_CANDIDATE_COMPONENT_RTP),
+ report->timestamp_us());
+ expected_rtp_transport.bytes_sent = 42;
+ expected_rtp_transport.bytes_received = 1337;
+ expected_rtp_transport.active_connection = false;
+
+ EXPECT_TRUE(report->Get(expected_rtp_transport.id()));
+ EXPECT_EQ(
+ expected_rtp_transport,
+ report->Get(expected_rtp_transport.id())->cast_to<RTCTransportStats>());
cricket::ConnectionInfo rtcp_connection_info;
rtcp_connection_info.best_connection = false;
@@ -1732,16 +1643,48 @@
collector_->ClearCachedStatsReport();
// Get stats with RTCP and without an active connection or certificates.
report = GetStatsReport();
- ExpectReportContainsTransportStats(
- report, session_stats.transport_stats["transport"], nullptr, nullptr);
+
+ RTCTransportStats expected_rtcp_transport(
+ "RTCTransport_transport_" +
+ rtc::ToString<>(cricket::ICE_CANDIDATE_COMPONENT_RTCP),
+ report->timestamp_us());
+ expected_rtcp_transport.bytes_sent = 1337;
+ expected_rtcp_transport.bytes_received = 42;
+ expected_rtcp_transport.active_connection = false;
+
+ expected_rtp_transport.rtcp_transport_stats_id = expected_rtcp_transport.id();
+
+ EXPECT_TRUE(report->Get(expected_rtp_transport.id()));
+ EXPECT_EQ(
+ expected_rtp_transport,
+ report->Get(expected_rtp_transport.id())->cast_to<RTCTransportStats>());
+ EXPECT_TRUE(report->Get(expected_rtcp_transport.id()));
+ EXPECT_EQ(
+ expected_rtcp_transport,
+ report->Get(expected_rtcp_transport.id())->cast_to<RTCTransportStats>());
// Get stats with an active connection.
- rtcp_connection_info.best_connection = true;
+ session_stats.transport_stats["transport"]
+ .channel_stats[1]
+ .connection_infos[0]
+ .best_connection = true;
collector_->ClearCachedStatsReport();
report = GetStatsReport();
- ExpectReportContainsTransportStats(
- report, session_stats.transport_stats["transport"], nullptr, nullptr);
+
+ expected_rtcp_transport.active_connection = true;
+ expected_rtcp_transport.selected_candidate_pair_id =
+ "RTCIceCandidatePair_" + rtcp_local_candidate->id() + "_" +
+ rtcp_remote_candidate->id();
+
+ EXPECT_TRUE(report->Get(expected_rtp_transport.id()));
+ EXPECT_EQ(
+ expected_rtp_transport,
+ report->Get(expected_rtp_transport.id())->cast_to<RTCTransportStats>());
+ EXPECT_TRUE(report->Get(expected_rtcp_transport.id()));
+ EXPECT_EQ(
+ expected_rtcp_transport,
+ report->Get(expected_rtcp_transport.id())->cast_to<RTCTransportStats>());
// Get stats with certificates.
std::unique_ptr<CertificateInfo> local_certinfo =
@@ -1769,9 +1712,25 @@
collector_->ClearCachedStatsReport();
report = GetStatsReport();
- ExpectReportContainsTransportStats(
- report, session_stats.transport_stats["transport"],
- local_certinfo.get(), remote_certinfo.get());
+
+ expected_rtp_transport.local_certificate_id =
+ "RTCCertificate_" + local_certinfo->fingerprints[0];
+ expected_rtp_transport.remote_certificate_id =
+ "RTCCertificate_" + remote_certinfo->fingerprints[0];
+
+ expected_rtcp_transport.local_certificate_id =
+ *expected_rtp_transport.local_certificate_id;
+ expected_rtcp_transport.remote_certificate_id =
+ *expected_rtp_transport.remote_certificate_id;
+
+ EXPECT_TRUE(report->Get(expected_rtp_transport.id()));
+ EXPECT_EQ(
+ expected_rtp_transport,
+ report->Get(expected_rtp_transport.id())->cast_to<RTCTransportStats>());
+ EXPECT_TRUE(report->Get(expected_rtcp_transport.id()));
+ EXPECT_EQ(
+ expected_rtcp_transport,
+ report->Get(expected_rtcp_transport.id())->cast_to<RTCTransportStats>());
}
class RTCStatsCollectorTestWithFakeCollector : public testing::Test {
diff --git a/webrtc/api/stats/rtcstats.h b/webrtc/api/stats/rtcstats.h
index aea47c2..b3afee0 100644
--- a/webrtc/api/stats/rtcstats.h
+++ b/webrtc/api/stats/rtcstats.h
@@ -67,7 +67,8 @@
// |Members| always returns the same members in the same order.
std::vector<const RTCStatsMemberInterface*> Members() const;
// Checks if the two stats objects are of the same type and have the same
- // member values. These operators are exposed for testing.
+ // member values. Timestamps are not compared. These operators are exposed for
+ // testing.
bool operator==(const RTCStats& other) const;
bool operator!=(const RTCStats& other) const;
diff --git a/webrtc/api/stats/rtcstats_objects.h b/webrtc/api/stats/rtcstats_objects.h
index 0ed02cb..8f5e038 100644
--- a/webrtc/api/stats/rtcstats_objects.h
+++ b/webrtc/api/stats/rtcstats_objects.h
@@ -115,7 +115,6 @@
RTCIceCandidatePairStats(const RTCIceCandidatePairStats& other);
~RTCIceCandidatePairStats() override;
- // TODO(hbos): Not collected by |RTCStatsCollector|. crbug.com/633550, 653873
RTCStatsMember<std::string> transport_id;
RTCStatsMember<std::string> local_candidate_id;
RTCStatsMember<std::string> remote_candidate_id;
diff --git a/webrtc/stats/rtcstats.cc b/webrtc/stats/rtcstats.cc
index ef36666..1968dd0 100644
--- a/webrtc/stats/rtcstats.cc
+++ b/webrtc/stats/rtcstats.cc
@@ -51,10 +51,8 @@
} // namespace
bool RTCStats::operator==(const RTCStats& other) const {
- if (type() != other.type() || id() != other.id() ||
- timestamp_us() != other.timestamp_us()) {
+ if (type() != other.type() || id() != other.id())
return false;
- }
std::vector<const RTCStatsMemberInterface*> members = Members();
std::vector<const RTCStatsMemberInterface*> other_members = other.Members();
RTC_DCHECK_EQ(members.size(), other_members.size());
diff --git a/webrtc/stats/rtcstats_unittest.cc b/webrtc/stats/rtcstats_unittest.cc
index 7854355..6fe8dd9 100644
--- a/webrtc/stats/rtcstats_unittest.cc
+++ b/webrtc/stats/rtcstats_unittest.cc
@@ -167,7 +167,7 @@
RTCTestStats empty_stats_different_id("testId2", 123);
EXPECT_NE(empty_stats, empty_stats_different_id);
RTCTestStats empty_stats_different_timestamp("testId", 321);
- EXPECT_NE(empty_stats, empty_stats_different_timestamp);
+ EXPECT_EQ(empty_stats, empty_stats_different_timestamp);
RTCChildStats child("childId", 42);
RTCGrandChildStats grandchild("grandchildId", 42);
