diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h index e58713ff12..cf9a1f564c 100644 --- a/api/peerconnectioninterface.h +++ b/api/peerconnectioninterface.h @@ -805,15 +805,46 @@ class PeerConnectionInterface : public rtc::RefCountInterface { return {}; } + // The legacy non-compliant GetStats() API. This correspond to the + // callback-based version of getStats() in JavaScript. The returned metrics + // are UNDOCUMENTED and many of them rely on implementation-specific details. + // The goal is to DELETE THIS VERSION but we can't today because it is heavily + // relied upon by third parties. See https://crbug.com/822696. + // + // This version is wired up into Chrome. Any stats implemented are + // automatically exposed to the Web Platform. This has BYPASSED the Chrome + // release processes for years and lead to cross-browser incompatibility + // issues and web application reliance on Chrome-only behavior. + // + // This API is in "maintenance mode", serious regressions should be fixed but + // adding new stats is highly discouraged. + // + // TODO(hbos): Deprecate and remove this when third parties have migrated to + // the spec-compliant GetStats() API. https://crbug.com/822696 virtual bool GetStats(StatsObserver* observer, - MediaStreamTrackInterface* track, + MediaStreamTrackInterface* track, // Optional StatsOutputLevel level) = 0; - // Gets stats using the new stats collection API, see webrtc/api/stats/. These - // will replace old stats collection API when the new API has matured enough. - // TODO(hbos): Default implementation that does nothing only exists as to not - // break third party projects. As soon as they have been updated this should - // be changed to "= 0;". + // The spec-compliant GetStats() API. This correspond to the promise-based + // version of getStats() in JavaScript. Implementation status is described in + // api/stats/rtcstats_objects.h. For more details on stats, see spec: + // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-getstats + // TODO(hbos): Takes shared ownership, use rtc::scoped_refptr<> instead. This + // requires stop overriding the current version in third party or making third + // party calls explicit to avoid ambiguity during switch. Make the future + // version abstract as soon as third party projects implement it. virtual void GetStats(RTCStatsCollectorCallback* callback) {} + // Spec-compliant getStats() performing the stats selection algorithm with the + // sender. https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-getstats + // TODO(hbos): Make abstract as soon as third party projects implement it. + virtual void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) {} + // Spec-compliant getStats() performing the stats selection algorithm with the + // receiver. https://w3c.github.io/webrtc-pc/#dom-rtcrtpreceiver-getstats + // TODO(hbos): Make abstract as soon as third party projects implement it. + virtual void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) {} // Clear cached stats in the RTCStatsCollector. // Exposed for testing while waiting for automatic cache clear to work. // https://bugs.webrtc.org/8693 diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h index 7235f5b4b2..9325adcec3 100644 --- a/api/peerconnectionproxy.h +++ b/api/peerconnectionproxy.h @@ -70,6 +70,14 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) MediaStreamTrackInterface*, StatsOutputLevel) PROXY_METHOD1(void, GetStats, RTCStatsCollectorCallback*) + PROXY_METHOD2(void, + GetStats, + rtc::scoped_refptr, + rtc::scoped_refptr); + PROXY_METHOD2(void, + GetStats, + rtc::scoped_refptr, + rtc::scoped_refptr); PROXY_METHOD2(rtc::scoped_refptr, CreateDataChannel, const std::string&, diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h index 208a3001da..842fca8cb2 100644 --- a/api/stats/rtcstats_objects.h +++ b/api/stats/rtcstats_objects.h @@ -11,6 +11,7 @@ #ifndef API_STATS_RTCSTATS_OBJECTS_H_ #define API_STATS_RTCSTATS_OBJECTS_H_ +#include #include #include @@ -217,11 +218,13 @@ class RTCIceCandidateStats : public RTCStats { // But here we define them as subclasses of |RTCIceCandidateStats| because the // |kType| need to be different ("RTCStatsType type") in the local/remote case. // https://w3c.github.io/webrtc-stats/#rtcstatstype-str* +// This forces us to have to override copy() and type(). class RTCLocalIceCandidateStats final : public RTCIceCandidateStats { public: static const char kType[]; RTCLocalIceCandidateStats(const std::string& id, int64_t timestamp_us); RTCLocalIceCandidateStats(std::string&& id, int64_t timestamp_us); + std::unique_ptr copy() const override; const char* type() const override; }; @@ -230,6 +233,7 @@ class RTCRemoteIceCandidateStats final : public RTCIceCandidateStats { static const char kType[]; RTCRemoteIceCandidateStats(const std::string& id, int64_t timestamp_us); RTCRemoteIceCandidateStats(std::string&& id, int64_t timestamp_us); + std::unique_ptr copy() const override; const char* type() const override; }; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index d8ec0c949a..6fb64fcbdd 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -1592,10 +1592,68 @@ bool PeerConnection::GetStats(StatsObserver* observer, } void PeerConnection::GetStats(RTCStatsCollectorCallback* callback) { + TRACE_EVENT0("webrtc", "PeerConnection::GetStats"); RTC_DCHECK(stats_collector_); + RTC_DCHECK(callback); stats_collector_->GetStatsReport(callback); } +void PeerConnection::GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) { + TRACE_EVENT0("webrtc", "PeerConnection::GetStats"); + RTC_DCHECK(callback); + RTC_DCHECK(stats_collector_); + rtc::scoped_refptr internal_sender; + if (selector) { + for (const auto& proxy_transceiver : transceivers_) { + for (const auto& proxy_sender : + proxy_transceiver->internal()->senders()) { + if (proxy_sender == selector) { + internal_sender = proxy_sender->internal(); + break; + } + } + if (internal_sender) + break; + } + } + // If there is no |internal_sender| then |selector| is either null or does not + // belong to the PeerConnection (in Plan B, senders can be removed from the + // PeerConnection). This means that "all the stats objects representing the + // selector" is an empty set. Invoking GetStatsReport() with a null selector + // produces an empty stats report. + stats_collector_->GetStatsReport(internal_sender, callback); +} + +void PeerConnection::GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) { + TRACE_EVENT0("webrtc", "PeerConnection::GetStats"); + RTC_DCHECK(callback); + RTC_DCHECK(stats_collector_); + rtc::scoped_refptr internal_receiver; + if (selector) { + for (const auto& proxy_transceiver : transceivers_) { + for (const auto& proxy_receiver : + proxy_transceiver->internal()->receivers()) { + if (proxy_receiver == selector) { + internal_receiver = proxy_receiver->internal(); + break; + } + } + if (internal_receiver) + break; + } + } + // If there is no |internal_receiver| then |selector| is either null or does + // not belong to the PeerConnection (in Plan B, receivers can be removed from + // the PeerConnection). This means that "all the stats objects representing + // the selector" is an empty set. Invoking GetStatsReport() with a null + // selector produces an empty stats report. + stats_collector_->GetStatsReport(internal_receiver, callback); +} + PeerConnectionInterface::SignalingState PeerConnection::signaling_state() { return signaling_state_; } diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 9051910d15..d1ba212e9c 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -116,10 +116,18 @@ class PeerConnection : public PeerConnectionInternal, rtc::scoped_refptr CreateDataChannel( const std::string& label, const DataChannelInit* config) override; + // WARNING: LEGACY. See peerconnectioninterface.h bool GetStats(StatsObserver* observer, webrtc::MediaStreamTrackInterface* track, StatsOutputLevel level) override; + // Spec-complaint GetStats(). See peerconnectioninterface.h void GetStats(RTCStatsCollectorCallback* callback) override; + void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) override; + void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) override; void ClearStatsCache() override; SignalingState signaling_state() override; diff --git a/pc/rtcstats_integrationtest.cc b/pc/rtcstats_integrationtest.cc index 40d07f7607..cdc46d7d81 100644 --- a/pc/rtcstats_integrationtest.cc +++ b/pc/rtcstats_integrationtest.cc @@ -137,10 +137,26 @@ class RTCStatsIntegrationTest : public testing::Test { rtc::scoped_refptr GetStatsFromCaller() { return GetStats(caller_->pc()); } + rtc::scoped_refptr GetStatsFromCaller( + rtc::scoped_refptr selector) { + return GetStats(caller_->pc(), selector); + } + rtc::scoped_refptr GetStatsFromCaller( + rtc::scoped_refptr selector) { + return GetStats(caller_->pc(), selector); + } rtc::scoped_refptr GetStatsFromCallee() { return GetStats(callee_->pc()); } + rtc::scoped_refptr GetStatsFromCallee( + rtc::scoped_refptr selector) { + return GetStats(callee_->pc(), selector); + } + rtc::scoped_refptr GetStatsFromCallee( + rtc::scoped_refptr selector) { + return GetStats(callee_->pc(), selector); + } protected: static rtc::scoped_refptr GetStats( @@ -152,6 +168,17 @@ class RTCStatsIntegrationTest : public testing::Test { return stats_obtainer->report(); } + template + static rtc::scoped_refptr GetStats( + PeerConnectionInterface* pc, + rtc::scoped_refptr selector) { + rtc::scoped_refptr stats_obtainer = + RTCStatsObtainer::Create(); + pc->GetStats(selector, stats_obtainer); + EXPECT_TRUE_WAIT(stats_obtainer->report(), kGetStatsTimeoutMs); + return stats_obtainer->report(); + } + // |network_thread_| uses |virtual_socket_server_| so they must be // constructed/destructed in the correct order. rtc::VirtualSocketServer virtual_socket_server_; @@ -315,7 +342,7 @@ class RTCStatsReportVerifier { : report_(report) { } - void VerifyReport() { + void VerifyReport(std::vector allowed_missing_stats) { std::set missing_stats = StatsTypes(); bool verify_successful = true; std::vector transport_stats = @@ -367,9 +394,10 @@ class RTCStatsReportVerifier { verify_successful = false; } } - if (!missing_stats.empty()) { - verify_successful = false; - for (const char* missing : missing_stats) { + for (const char* missing : missing_stats) { + if (std::find(allowed_missing_stats.begin(), allowed_missing_stats.end(), + missing) == allowed_missing_stats.end()) { + verify_successful = false; EXPECT_TRUE(false) << "Missing expected stats type: " << missing; } } @@ -718,7 +746,7 @@ TEST_F(RTCStatsIntegrationTest, GetStatsFromCaller) { StartCall(); rtc::scoped_refptr report = GetStatsFromCaller(); - RTCStatsReportVerifier(report.get()).VerifyReport(); + RTCStatsReportVerifier(report.get()).VerifyReport({}); EXPECT_EQ(report->ToJson(), RTCStatsReportTraceListener::last_trace()); } @@ -726,10 +754,68 @@ TEST_F(RTCStatsIntegrationTest, GetStatsFromCallee) { StartCall(); rtc::scoped_refptr report = GetStatsFromCallee(); - RTCStatsReportVerifier(report.get()).VerifyReport(); + RTCStatsReportVerifier(report.get()).VerifyReport({}); EXPECT_EQ(report->ToJson(), RTCStatsReportTraceListener::last_trace()); } +// These tests exercise the integration of the stats selection algorithm inside +// of PeerConnection. See rtcstatstraveral_unittest.cc for more detailed stats +// traversal tests on particular stats graphs. +TEST_F(RTCStatsIntegrationTest, GetStatsWithSenderSelector) { + StartCall(); + ASSERT_FALSE(caller_->pc()->GetSenders().empty()); + rtc::scoped_refptr report = + GetStatsFromCaller(caller_->pc()->GetSenders()[0]); + std::vector allowed_missing_stats = { + // TODO(hbos): Include RTC[Audio/Video]ReceiverStats when implemented. + // TODO(hbos): Include RTCRemoteOutboundRtpStreamStats when implemented. + // TODO(hbos): Include RTCRtpContributingSourceStats when implemented. + RTCInboundRTPStreamStats::kType, RTCPeerConnectionStats::kType, + RTCMediaStreamStats::kType, RTCDataChannelStats::kType, + }; + RTCStatsReportVerifier(report.get()).VerifyReport(allowed_missing_stats); + EXPECT_TRUE(report->size()); +} + +TEST_F(RTCStatsIntegrationTest, GetStatsWithReceiverSelector) { + StartCall(); + + ASSERT_FALSE(caller_->pc()->GetReceivers().empty()); + rtc::scoped_refptr report = + GetStatsFromCaller(caller_->pc()->GetReceivers()[0]); + std::vector allowed_missing_stats = { + // TODO(hbos): Include RTC[Audio/Video]SenderStats when implemented. + // TODO(hbos): Include RTCRemoteInboundRtpStreamStats when implemented. + // TODO(hbos): Include RTCRtpContributingSourceStats when implemented. + RTCOutboundRTPStreamStats::kType, RTCPeerConnectionStats::kType, + RTCMediaStreamStats::kType, RTCDataChannelStats::kType, + }; + RTCStatsReportVerifier(report.get()).VerifyReport(allowed_missing_stats); + EXPECT_TRUE(report->size()); +} + +TEST_F(RTCStatsIntegrationTest, GetStatsWithInvalidSenderSelector) { + StartCall(); + + ASSERT_FALSE(callee_->pc()->GetSenders().empty()); + // The selector is invalid for the caller because it belongs to the callee. + auto invalid_selector = callee_->pc()->GetSenders()[0]; + rtc::scoped_refptr report = + GetStatsFromCaller(invalid_selector); + EXPECT_FALSE(report->size()); +} + +TEST_F(RTCStatsIntegrationTest, GetStatsWithInvalidReceiverSelector) { + StartCall(); + + ASSERT_FALSE(callee_->pc()->GetReceivers().empty()); + // The selector is invalid for the caller because it belongs to the callee. + auto invalid_selector = callee_->pc()->GetReceivers()[0]; + rtc::scoped_refptr report = + GetStatsFromCaller(invalid_selector); + EXPECT_FALSE(report->size()); +} + TEST_F(RTCStatsIntegrationTest, GetsStatsWhileDestroyingPeerConnections) { StartCall(); diff --git a/pc/test/fakepeerconnectionbase.h b/pc/test/fakepeerconnectionbase.h index 2a46bf5481..9f670484e5 100644 --- a/pc/test/fakepeerconnectionbase.h +++ b/pc/test/fakepeerconnectionbase.h @@ -110,6 +110,12 @@ class FakePeerConnectionBase : public PeerConnectionInternal { } void GetStats(RTCStatsCollectorCallback* callback) override {} + void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) override {} + void GetStats( + rtc::scoped_refptr selector, + rtc::scoped_refptr callback) override {} void ClearStatsCache() override {} diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index 0a176b4f64..7b697f0f77 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -262,7 +262,7 @@ RTCIceCandidatePairStats::~RTCIceCandidatePairStats() { } // clang-format off -WEBRTC_RTCSTATS_IMPL(RTCIceCandidateStats, RTCStats, "ice-candidate", +WEBRTC_RTCSTATS_IMPL(RTCIceCandidateStats, RTCStats, "abstract-ice-candidate", &transport_id, &is_remote, &network_type, @@ -323,6 +323,10 @@ RTCLocalIceCandidateStats::RTCLocalIceCandidateStats( : RTCIceCandidateStats(std::move(id), timestamp_us, false) { } +std::unique_ptr RTCLocalIceCandidateStats::copy() const { + return std::unique_ptr(new RTCLocalIceCandidateStats(*this)); +} + const char* RTCLocalIceCandidateStats::type() const { return kType; } @@ -339,6 +343,10 @@ RTCRemoteIceCandidateStats::RTCRemoteIceCandidateStats( : RTCIceCandidateStats(std::move(id), timestamp_us, true) { } +std::unique_ptr RTCRemoteIceCandidateStats::copy() const { + return std::unique_ptr(new RTCRemoteIceCandidateStats(*this)); +} + const char* RTCRemoteIceCandidateStats::type() const { return kType; }