RTCStatsCollector: Utilize network thread to minimize thread hops.
(This is a re-upload of https://codereview.webrtc.org/2567243003/, the
CQ stopped working there.)
The previously used WebRtcSession::GetTransportStats did a synchronous
invoke per channel (voice, video, data) on the signaling thread to the
network thread - e.g. 3 blocking invokes.
It is replaced by WebRtcSession::GetStats[_s] which can be invoked on
the signaling thread or on any thread if a ChannelNamePairs argument is
present (provided by WebRtcSession::GetChannelNamePairs on the signaling
thread).
With these changes, and changes allowing the getting of certificates
from any thread, the RTCStatsCollector can turn the 3 blocking thread
invokes into 1 non-blocking invoke.
BUG=webrtc:6875, chromium:627816
Review-Url: https://codereview.webrtc.org/2583883002
Cr-Commit-Position: refs/heads/master@{#15672}
diff --git a/webrtc/api/rtcstatscollector.cc b/webrtc/api/rtcstatscollector.cc
index a6d691c..3628544 100644
--- a/webrtc/api/rtcstatscollector.cc
+++ b/webrtc/api/rtcstatscollector.cc
@@ -416,9 +416,32 @@
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, signaling_thread_,
rtc::Bind(&RTCStatsCollector::ProducePartialResultsOnSignalingThread,
rtc::scoped_refptr<RTCStatsCollector>(this), timestamp_us));
+
+ // TODO(hbos): No stats are gathered by
+ // |ProducePartialResultsOnWorkerThread|, remove it.
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, worker_thread_,
rtc::Bind(&RTCStatsCollector::ProducePartialResultsOnWorkerThread,
rtc::scoped_refptr<RTCStatsCollector>(this), timestamp_us));
+
+ // Prepare |channel_names_| and |media_info_| for use in
+ // |ProducePartialResultsOnNetworkThread|.
+ channel_name_pairs_.reset(new ChannelNamePairs());
+ if (pc_->session()->voice_channel()) {
+ channel_name_pairs_->voice = rtc::Optional<ChannelNamePair>(
+ ChannelNamePair(pc_->session()->voice_channel()->content_name(),
+ pc_->session()->voice_channel()->transport_name()));
+ }
+ if (pc_->session()->video_channel()) {
+ channel_name_pairs_->video = rtc::Optional<ChannelNamePair>(
+ ChannelNamePair(pc_->session()->video_channel()->content_name(),
+ pc_->session()->video_channel()->transport_name()));
+ }
+ if (pc_->session()->data_channel()) {
+ channel_name_pairs_->data = rtc::Optional<ChannelNamePair>(
+ ChannelNamePair(pc_->session()->data_channel()->content_name(),
+ pc_->session()->data_channel()->transport_name()));
+ }
+ media_info_.reset(PrepareMediaInfo_s().release());
invoker_.AsyncInvoke<void>(RTC_FROM_HERE, network_thread_,
rtc::Bind(&RTCStatsCollector::ProducePartialResultsOnNetworkThread,
rtc::scoped_refptr<RTCStatsCollector>(this), timestamp_us));
@@ -436,23 +459,6 @@
rtc::scoped_refptr<RTCStatsReport> report = RTCStatsReport::Create(
timestamp_us);
- SessionStats session_stats;
- if (pc_->session()->GetTransportStats(&session_stats)) {
- std::map<std::string, CertificateStatsPair> transport_cert_stats =
- PrepareTransportCertificateStats(session_stats);
- MediaInfo media_info = PrepareMediaInfo(session_stats);
-
- ProduceCertificateStats_s(
- timestamp_us, transport_cert_stats, report.get());
- ProduceCodecStats_s(
- timestamp_us, media_info, report.get());
- ProduceIceCandidateAndPairStats_s(
- timestamp_us, session_stats, report.get());
- ProduceRTPStreamStats_s(
- timestamp_us, session_stats, media_info, report.get());
- ProduceTransportStats_s(
- timestamp_us, session_stats, transport_cert_stats, report.get());
- }
ProduceDataChannelStats_s(timestamp_us, report.get());
ProduceMediaStreamAndTrackStats_s(timestamp_us, report.get());
ProducePeerConnectionStats_s(timestamp_us, report.get());
@@ -466,13 +472,8 @@
rtc::scoped_refptr<RTCStatsReport> report = RTCStatsReport::Create(
timestamp_us);
- // TODO(hbos): Gather stats on worker thread.
- // pc_->session()'s channels are owned by the signaling thread but there are
- // some stats that are gathered on the worker thread. Instead of a synchronous
- // invoke on "s->w" we could to the "w" work here asynchronously if it wasn't
- // for the ownership issue. Synchronous invokes in other places makes it
- // difficult to introduce locks without introducing deadlocks and the channels
- // are not reference counted.
+ // TODO(hbos): There are no stats to be gathered on this thread, remove this
+ // method.
AddPartialResults(report);
}
@@ -483,13 +484,23 @@
rtc::scoped_refptr<RTCStatsReport> report = RTCStatsReport::Create(
timestamp_us);
- // TODO(hbos): Gather stats on network thread.
- // pc_->session()'s channels are owned by the signaling thread but there are
- // some stats that are gathered on the network thread. Instead of a
- // synchronous invoke on "s->n" we could to the "n" work here asynchronously
- // if it wasn't for the ownership issue. Synchronous invokes in other places
- // makes it difficult to introduce locks without introducing deadlocks and the
- // channels are not reference counted.
+ std::unique_ptr<SessionStats> session_stats =
+ pc_->session()->GetStats(*channel_name_pairs_);
+ if (session_stats) {
+ std::map<std::string, CertificateStatsPair> transport_cert_stats =
+ PrepareTransportCertificateStats_n(*session_stats);
+
+ ProduceCertificateStats_n(
+ timestamp_us, transport_cert_stats, report.get());
+ ProduceCodecStats_n(
+ timestamp_us, *media_info_, report.get());
+ ProduceIceCandidateAndPairStats_n(
+ timestamp_us, *session_stats, report.get());
+ ProduceRTPStreamStats_n(
+ timestamp_us, *session_stats, *media_info_, report.get());
+ ProduceTransportStats_n(
+ timestamp_us, *session_stats, transport_cert_stats, report.get());
+ }
AddPartialResults(report);
}
@@ -534,11 +545,11 @@
callbacks_.clear();
}
-void RTCStatsCollector::ProduceCertificateStats_s(
+void RTCStatsCollector::ProduceCertificateStats_n(
int64_t timestamp_us,
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_thread_->IsCurrent());
for (const auto& transport_cert_stats_pair : transport_cert_stats) {
if (transport_cert_stats_pair.second.local) {
ProduceCertificateStatsFromSSLCertificateStats(
@@ -551,10 +562,10 @@
}
}
-void RTCStatsCollector::ProduceCodecStats_s(
+void RTCStatsCollector::ProduceCodecStats_n(
int64_t timestamp_us, const MediaInfo& media_info,
RTCStatsReport* report) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_thread_->IsCurrent());
// Audio
if (media_info.voice) {
// Inbound
@@ -605,10 +616,10 @@
}
}
-void RTCStatsCollector::ProduceIceCandidateAndPairStats_s(
+void RTCStatsCollector::ProduceIceCandidateAndPairStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
RTCStatsReport* report) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_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(
@@ -681,10 +692,10 @@
report->AddStats(std::move(stats));
}
-void RTCStatsCollector::ProduceRTPStreamStats_s(
+void RTCStatsCollector::ProduceRTPStreamStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
const MediaInfo& media_info, RTCStatsReport* report) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_thread_->IsCurrent());
// Audio
if (media_info.voice) {
@@ -788,11 +799,11 @@
}
}
-void RTCStatsCollector::ProduceTransportStats_s(
+void RTCStatsCollector::ProduceTransportStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_thread_->IsCurrent());
for (const auto& transport : session_stats.transport_stats) {
// Get reference to RTCP channel, if it exists.
std::string rtcp_transport_stats_id;
@@ -855,9 +866,9 @@
}
std::map<std::string, RTCStatsCollector::CertificateStatsPair>
-RTCStatsCollector::PrepareTransportCertificateStats(
+RTCStatsCollector::PrepareTransportCertificateStats_n(
const SessionStats& session_stats) const {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK(network_thread_->IsCurrent());
std::map<std::string, CertificateStatsPair> transport_cert_stats;
for (const auto& transport_stats : session_stats.transport_stats) {
CertificateStatsPair certificate_stats_pair;
@@ -880,20 +891,21 @@
return transport_cert_stats;
}
-RTCStatsCollector::MediaInfo RTCStatsCollector::PrepareMediaInfo(
- const SessionStats& session_stats) const {
- MediaInfo media_info;
+std::unique_ptr<RTCStatsCollector::MediaInfo>
+RTCStatsCollector::PrepareMediaInfo_s() const {
+ RTC_DCHECK(signaling_thread_->IsCurrent());
+ std::unique_ptr<MediaInfo> media_info(new MediaInfo());
if (pc_->session()->voice_channel()) {
cricket::VoiceMediaInfo voice_media_info;
if (pc_->session()->voice_channel()->GetStats(&voice_media_info)) {
- media_info.voice = rtc::Optional<cricket::VoiceMediaInfo>(
+ media_info->voice = rtc::Optional<cricket::VoiceMediaInfo>(
std::move(voice_media_info));
}
}
if (pc_->session()->video_channel()) {
cricket::VideoMediaInfo video_media_info;
if (pc_->session()->video_channel()->GetStats(&video_media_info)) {
- media_info.video = rtc::Optional<cricket::VideoMediaInfo>(
+ media_info->video = rtc::Optional<cricket::VideoMediaInfo>(
std::move(video_media_info));
}
}
diff --git a/webrtc/api/rtcstatscollector.h b/webrtc/api/rtcstatscollector.h
index c32d65f..7b23d24 100644
--- a/webrtc/api/rtcstatscollector.h
+++ b/webrtc/api/rtcstatscollector.h
@@ -41,6 +41,7 @@
class PeerConnection;
struct SessionStats;
+struct ChannelNamePairs;
class RTCStatsCollectorCallback : public virtual rtc::RefCountInterface {
public:
@@ -97,19 +98,19 @@
void DeliverCachedReport();
// Produces |RTCCertificateStats|.
- void ProduceCertificateStats_s(
+ void ProduceCertificateStats_n(
int64_t timestamp_us,
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const;
// Produces |RTCCodecStats|.
- void ProduceCodecStats_s(
+ void ProduceCodecStats_n(
int64_t timestamp_us, const MediaInfo& media_info,
RTCStatsReport* report) const;
// Produces |RTCDataChannelStats|.
void ProduceDataChannelStats_s(
int64_t timestamp_us, RTCStatsReport* report) const;
// Produces |RTCIceCandidatePairStats| and |RTCIceCandidateStats|.
- void ProduceIceCandidateAndPairStats_s(
+ void ProduceIceCandidateAndPairStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
RTCStatsReport* report) const;
// Produces |RTCMediaStreamStats| and |RTCMediaStreamTrackStats|.
@@ -119,19 +120,19 @@
void ProducePeerConnectionStats_s(
int64_t timestamp_us, RTCStatsReport* report) const;
// Produces |RTCInboundRTPStreamStats| and |RTCOutboundRTPStreamStats|.
- void ProduceRTPStreamStats_s(
+ void ProduceRTPStreamStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
const MediaInfo& media_info, RTCStatsReport* report) const;
// Produces |RTCTransportStats|.
- void ProduceTransportStats_s(
+ void ProduceTransportStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
const std::map<std::string, CertificateStatsPair>& transport_cert_stats,
RTCStatsReport* report) const;
// Helper function to stats-producing functions.
std::map<std::string, CertificateStatsPair>
- PrepareTransportCertificateStats(const SessionStats& session_stats) const;
- MediaInfo PrepareMediaInfo(const SessionStats& session_stats) const;
+ PrepareTransportCertificateStats_n(const SessionStats& session_stats) const;
+ std::unique_ptr<MediaInfo> PrepareMediaInfo_s() const;
// Slots for signals (sigslot) that are wired up to |pc_|.
void OnDataChannelCreated(DataChannel* channel);
@@ -150,6 +151,13 @@
rtc::scoped_refptr<RTCStatsReport> partial_report_;
std::vector<rtc::scoped_refptr<RTCStatsCollectorCallback>> callbacks_;
+ // Set in |GetStatsReport|, used in |ProducePartialResultsOnNetworkThread|
+ // (not passed as arguments to avoid copies). This is thread safe - it is set
+ // at the start of |GetStatsReport| after making sure there are no pending
+ // stats requests in progress.
+ std::unique_ptr<ChannelNamePairs> channel_name_pairs_;
+ std::unique_ptr<MediaInfo> media_info_;
+
// A timestamp, in microseconds, that is based on a timer that is
// monotonically increasing. That is, even if the system clock is modified the
// difference between the timer and this timestamp is how fresh the cached
diff --git a/webrtc/api/rtcstatscollector_unittest.cc b/webrtc/api/rtcstatscollector_unittest.cc
index 697cc30..4be7491 100644
--- a/webrtc/api/rtcstatscollector_unittest.cc
+++ b/webrtc/api/rtcstatscollector_unittest.cc
@@ -313,7 +313,7 @@
ReturnRef(data_channels_));
EXPECT_CALL(session_, video_channel()).WillRepeatedly(ReturnNull());
EXPECT_CALL(session_, voice_channel()).WillRepeatedly(ReturnNull());
- EXPECT_CALL(session_, GetTransportStats(_)).WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(ReturnNull());
EXPECT_CALL(session_, GetLocalCertificate(_, _)).WillRepeatedly(
Return(false));
EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_))
@@ -628,10 +628,11 @@
std::vector<std::string>({ "(remote) single certificate" }));
// Mock the session to return the local and remote certificates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this](SessionStats* stats) {
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ std::unique_ptr<SessionStats> stats(new SessionStats());
stats->transport_stats["transport"].transport_name = "transport";
- return true;
+ return stats;
}));
EXPECT_CALL(test_->session(), GetLocalCertificate(_, _)).WillRepeatedly(
Invoke([this, &local_certinfo](const std::string& transport_name,
@@ -713,8 +714,10 @@
session_stats.transport_stats["TransportName"].transport_name =
"TransportName";
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), voice_channel())
.WillRepeatedly(Return(&voice_channel));
EXPECT_CALL(test_->session(), video_channel())
@@ -791,11 +794,12 @@
video_remote_certinfo->ders);
// Mock the session to return the local and remote certificates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this](SessionStats* stats) {
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ std::unique_ptr<SessionStats> stats(new SessionStats());
stats->transport_stats["audio"].transport_name = "audio";
stats->transport_stats["video"].transport_name = "video";
- return true;
+ return stats;
}));
EXPECT_CALL(test_->session(), GetLocalCertificate(_, _)).WillRepeatedly(
Invoke([this, &audio_local_certinfo, &video_local_certinfo](
@@ -850,10 +854,11 @@
CreateFakeCertificateAndInfoFromDers(remote_ders);
// Mock the session to return the local and remote certificates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this](SessionStats* stats) {
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ std::unique_ptr<SessionStats> stats(new SessionStats());
stats->transport_stats["transport"].transport_name = "transport";
- return true;
+ return stats;
}));
EXPECT_CALL(test_->session(), GetLocalCertificate(_, _)).WillRepeatedly(
Invoke([this, &local_certinfo](const std::string& transport_name,
@@ -978,10 +983,9 @@
b_transport_channel_stats);
// Mock the session to return the desired candidates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this, &session_stats](SessionStats* stats) {
- *stats = session_stats;
- return true;
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
}));
rtc::scoped_refptr<const RTCStatsReport> report = GetStatsReport();
@@ -1022,10 +1026,9 @@
transport_channel_stats);
// Mock the session to return the desired candidates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this, &session_stats](SessionStats* stats) {
- *stats = session_stats;
- return true;
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
}));
rtc::scoped_refptr<const RTCStatsReport> report = GetStatsReport();
@@ -1387,8 +1390,10 @@
session_stats.transport_stats["TransportName"].channel_stats.push_back(
channel_stats);
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), voice_channel())
.WillRepeatedly(Return(&voice_channel));
@@ -1459,8 +1464,10 @@
session_stats.transport_stats["TransportName"].channel_stats.push_back(
channel_stats);
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), video_channel())
.WillRepeatedly(Return(&video_channel));
@@ -1529,8 +1536,10 @@
session_stats.transport_stats["TransportName"].channel_stats.push_back(
channel_stats);
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), voice_channel())
.WillRepeatedly(Return(&voice_channel));
@@ -1597,8 +1606,10 @@
session_stats.transport_stats["TransportName"].channel_stats.push_back(
channel_stats);
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), video_channel())
.WillRepeatedly(Return(&video_channel));
@@ -1677,8 +1688,10 @@
session_stats.transport_stats["TransportName"].channel_stats.push_back(
channel_stats);
- EXPECT_CALL(test_->session(), GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats), Return(true)));
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
+ }));
EXPECT_CALL(test_->session(), voice_channel())
.WillRepeatedly(Return(&voice_channel));
EXPECT_CALL(test_->session(), video_channel())
@@ -1754,10 +1767,9 @@
// Mock the session to return the desired candidates.
- EXPECT_CALL(test_->session(), GetTransportStats(_)).WillRepeatedly(Invoke(
- [this, &session_stats](SessionStats* stats) {
- *stats = session_stats;
- return true;
+ EXPECT_CALL(test_->session(), GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(new SessionStats(session_stats));
}));
// Get stats without RTCP, an active connection or certificates.
diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index 991ceb4..fb6583a 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -675,8 +675,8 @@
report->AddBoolean(StatsReport::kStatsValueNameInitiator,
pc_->session()->initial_offerer());
- SessionStats stats;
- if (!pc_->session()->GetTransportStats(&stats)) {
+ std::unique_ptr<SessionStats> stats = pc_->session()->GetStats_s();
+ if (!stats) {
return;
}
@@ -686,9 +686,9 @@
// the proxy map directly from the session stats.
// As is, if GetStats() failed, we could be using old (incorrect?) proxy
// data.
- proxy_to_transport_ = stats.proxy_to_transport;
+ proxy_to_transport_ = stats->proxy_to_transport;
- for (const auto& transport_iter : stats.transport_stats) {
+ for (const auto& transport_iter : stats->transport_stats) {
// Attempt to get a copy of the certificates from the transport and
// expose them in stats reports. All channels in a transport share the
// same local and remote certificates.
diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc
index bf60d0e..50ae51d 100644
--- a/webrtc/api/statscollector_unittest.cc
+++ b/webrtc/api/statscollector_unittest.cc
@@ -41,6 +41,7 @@
using testing::_;
using testing::DoAll;
using testing::Field;
+using testing::Invoke;
using testing::Return;
using testing::ReturnNull;
using testing::ReturnRef;
@@ -509,7 +510,7 @@
&event_log_)),
session_(media_controller_.get()) {
// By default, we ignore session GetStats calls.
- EXPECT_CALL(session_, GetTransportStats(_)).WillRepeatedly(Return(false));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(ReturnNull());
// Add default returns for mock classes.
EXPECT_CALL(session_, video_channel()).WillRepeatedly(ReturnNull());
EXPECT_CALL(session_, voice_channel()).WillRepeatedly(ReturnNull());
@@ -612,9 +613,11 @@
// Instruct the session to return stats containing the transport channel.
InitSessionStats(vc_name);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
// Constructs an ssrc stats update.
if (voice_sender_info)
@@ -712,9 +715,11 @@
EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(
transport_stats.transport_name))
.WillOnce(Return(remote_cert.release()));
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillOnce(DoAll(SetArgPointee<0>(session_stats),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillOnce(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats));
+ }));
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
@@ -853,9 +858,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
@@ -900,9 +907,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
@@ -1013,9 +1022,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
@@ -1110,9 +1121,11 @@
Return(true)));
InitSessionStats(kVcName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
StatsReports reports;
@@ -1181,9 +1194,11 @@
// Instruct the session to return stats containing the transport channel.
InitSessionStats(kVcName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
// Constructs an ssrc stats update.
cricket::VideoMediaInfo stats_read;
@@ -1224,9 +1239,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
@@ -1430,9 +1447,11 @@
transport_stats;
// Configure MockWebRtcSession
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillOnce(DoAll(SetArgPointee<0>(session_stats),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats));
+ }));
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
stats.GetStats(NULL, &reports);
@@ -1487,9 +1506,11 @@
transport_stats;
// Configure MockWebRtcSession
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillOnce(DoAll(SetArgPointee<0>(session_stats),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [&session_stats](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats));
+ }));
stats.UpdateStats(PeerConnectionInterface::kStatsOutputLevelStandard);
stats.GetStats(NULL, &reports);
@@ -1566,8 +1587,11 @@
// Instruct the session to return stats containing the transport channel.
InitSessionStats(kVcName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
cricket::VoiceSenderInfo voice_sender_info;
voice_sender_info.add_ssrc(kSsrcOfTrack);
@@ -1720,9 +1744,11 @@
// Instruct the session to return stats containing the transport channel.
InitSessionStats(kVcName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
stats.RemoveLocalAudioTrack(audio_track_.get(), kSsrcOfTrack);
cricket::VoiceSenderInfo voice_sender_info;
@@ -1794,9 +1820,11 @@
// Instruct the session to return stats containing the transport channel.
InitSessionStats(kVcName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
- Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
cricket::VoiceSenderInfo voice_sender_info;
InitVoiceSenderInfo(&voice_sender_info);
@@ -1914,8 +1942,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
@@ -1959,8 +1990,11 @@
const char kVideoChannelName[] = "video";
InitSessionStats(kVideoChannelName);
- EXPECT_CALL(session_, GetTransportStats(_))
- .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_), Return(true)));
+ EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Invoke(
+ [this](const ChannelNamePairs&) {
+ return std::unique_ptr<SessionStats>(
+ new SessionStats(session_stats_));
+ }));
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(
diff --git a/webrtc/api/test/mock_webrtcsession.h b/webrtc/api/test/mock_webrtcsession.h
index 108bdca..7fefad8 100644
--- a/webrtc/api/test/mock_webrtcsession.h
+++ b/webrtc/api/test/mock_webrtcsession.h
@@ -42,7 +42,8 @@
// track.
MOCK_METHOD2(GetLocalTrackIdBySsrc, bool(uint32_t, std::string*));
MOCK_METHOD2(GetRemoteTrackIdBySsrc, bool(uint32_t, std::string*));
- MOCK_METHOD1(GetTransportStats, bool(SessionStats*));
+ MOCK_METHOD1(GetStats,
+ std::unique_ptr<SessionStats>(const ChannelNamePairs&));
MOCK_METHOD2(GetLocalCertificate,
bool(const std::string& transport_name,
rtc::scoped_refptr<rtc::RTCCertificate>* certificate));
diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc
index b05862d..ea724a3 100644
--- a/webrtc/api/webrtcsession.cc
+++ b/webrtc/api/webrtcsession.cc
@@ -970,50 +970,43 @@
return true;
}
-bool WebRtcSession::GetTransportStats(SessionStats* stats) {
+std::unique_ptr<SessionStats> WebRtcSession::GetStats_s() {
ASSERT(signaling_thread()->IsCurrent());
- return (GetChannelTransportStats(voice_channel(), stats) &&
- GetChannelTransportStats(video_channel(), stats) &&
- GetChannelTransportStats(data_channel(), stats));
+ ChannelNamePairs channel_name_pairs;
+ if (voice_channel()) {
+ channel_name_pairs.voice = rtc::Optional<ChannelNamePair>(ChannelNamePair(
+ voice_channel()->content_name(), voice_channel()->transport_name()));
+ }
+ if (video_channel()) {
+ channel_name_pairs.video = rtc::Optional<ChannelNamePair>(ChannelNamePair(
+ video_channel()->content_name(), video_channel()->transport_name()));
+ }
+ if (data_channel()) {
+ channel_name_pairs.data = rtc::Optional<ChannelNamePair>(ChannelNamePair(
+ data_channel()->content_name(), data_channel()->transport_name()));
+ }
+ return GetStats(channel_name_pairs);
}
-bool WebRtcSession::GetChannelTransportStats(cricket::BaseChannel* ch,
- SessionStats* stats) {
- ASSERT(signaling_thread()->IsCurrent());
- if (!ch) {
- // Not using this channel.
- return true;
+std::unique_ptr<SessionStats> WebRtcSession::GetStats(
+ const ChannelNamePairs& channel_name_pairs) {
+ if (network_thread()->IsCurrent()) {
+ return GetStats_n(channel_name_pairs);
}
-
- const std::string& content_name = ch->content_name();
- const std::string& transport_name = ch->transport_name();
- stats->proxy_to_transport[content_name] = transport_name;
- if (stats->transport_stats.find(transport_name) !=
- stats->transport_stats.end()) {
- // Transport stats already done for this transport.
- return true;
- }
-
- cricket::TransportStats tstats;
- if (!transport_controller_->GetStats(transport_name, &tstats)) {
- return false;
- }
-
- stats->transport_stats[transport_name] = tstats;
- return true;
+ return network_thread()->Invoke<std::unique_ptr<SessionStats>>(
+ RTC_FROM_HERE,
+ rtc::Bind(&WebRtcSession::GetStats_n, this, channel_name_pairs));
}
bool WebRtcSession::GetLocalCertificate(
const std::string& transport_name,
rtc::scoped_refptr<rtc::RTCCertificate>* certificate) {
- ASSERT(signaling_thread()->IsCurrent());
return transport_controller_->GetLocalCertificate(transport_name,
certificate);
}
std::unique_ptr<rtc::SSLCertificate> WebRtcSession::GetRemoteSSLCertificate(
const std::string& transport_name) {
- ASSERT(signaling_thread()->IsCurrent());
return transport_controller_->GetRemoteSSLCertificate(transport_name);
}
@@ -1716,6 +1709,28 @@
return true;
}
+std::unique_ptr<SessionStats> WebRtcSession::GetStats_n(
+ const ChannelNamePairs& channel_name_pairs) {
+ ASSERT(network_thread()->IsCurrent());
+ std::unique_ptr<SessionStats> session_stats(new SessionStats());
+ for (const auto channel_name_pair : { &channel_name_pairs.voice,
+ &channel_name_pairs.video,
+ &channel_name_pairs.data }) {
+ if (*channel_name_pair) {
+ cricket::TransportStats transport_stats;
+ if (!transport_controller_->GetStats((*channel_name_pair)->transport_name,
+ &transport_stats)) {
+ return nullptr;
+ }
+ session_stats->proxy_to_transport[(*channel_name_pair)->content_name] =
+ (*channel_name_pair)->transport_name;
+ session_stats->transport_stats[(*channel_name_pair)->transport_name] =
+ std::move(transport_stats);
+ }
+ }
+ return session_stats;
+}
+
void WebRtcSession::OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp) {
SetError(ERROR_TRANSPORT,
rtcp ? kDtlsSetupFailureRtcp : kDtlsSetupFailureRtp);
diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h
index 06d3375..241fb62 100644
--- a/webrtc/api/webrtcsession.h
+++ b/webrtc/api/webrtcsession.h
@@ -115,6 +115,20 @@
TransportStatsMap transport_stats;
};
+struct ChannelNamePair {
+ ChannelNamePair(
+ const std::string& content_name, const std::string& transport_name)
+ : content_name(content_name), transport_name(transport_name) {}
+ std::string content_name;
+ std::string transport_name;
+};
+
+struct ChannelNamePairs {
+ rtc::Optional<ChannelNamePair> voice;
+ rtc::Optional<ChannelNamePair> video;
+ rtc::Optional<ChannelNamePair> data;
+};
+
// A WebRtcSession manages general session state. This includes negotiation
// of both the application-level and network-level protocols: the former
// defines what will be sent and the latter defines how it will be sent. Each
@@ -194,6 +208,7 @@
virtual cricket::DataChannel* data_channel() {
return data_channel_.get();
}
+ std::unique_ptr<ChannelNamePairs> GetChannelNamePairs();
cricket::BaseChannel* GetChannel(const std::string& content_name);
@@ -261,10 +276,14 @@
// Returns stats for all channels of all transports.
// This avoids exposing the internal structures used to track them.
- virtual bool GetTransportStats(SessionStats* stats);
-
- // Get stats for a specific channel
- bool GetChannelTransportStats(cricket::BaseChannel* ch, SessionStats* stats);
+ // The parameterless version creates |ChannelNamePairs| from |voice_channel|,
+ // |video_channel| and |voice_channel| if available - this requires it to be
+ // called on the signaling thread - and invokes the other |GetStats|. The
+ // other |GetStats| can be invoked on any thread; if not invoked on the
+ // network thread a thread hop will happen.
+ std::unique_ptr<SessionStats> GetStats_s();
+ virtual std::unique_ptr<SessionStats> GetStats(
+ const ChannelNamePairs& channel_name_pairs);
// virtual so it can be mocked in unit tests
virtual bool GetLocalCertificate(
@@ -415,6 +434,9 @@
bool CreateDataChannel(const cricket::ContentInfo* content,
const std::string* bundle_transport);
+ std::unique_ptr<SessionStats> GetStats_n(
+ const ChannelNamePairs& channel_name_pairs);
+
// Listens to SCTP CONTROL messages on unused SIDs and process them as OPEN
// messages.
void OnDataChannelMessageReceived(cricket::DataChannel* channel,
@@ -489,7 +511,7 @@
const std::string sid_;
bool initial_offerer_ = false;
- std::unique_ptr<cricket::TransportController> transport_controller_;
+ const std::unique_ptr<cricket::TransportController> transport_controller_;
MediaControllerInterface* media_controller_;
std::unique_ptr<cricket::VoiceChannel> voice_channel_;
std::unique_ptr<cricket::VideoChannel> video_channel_;
diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc
index fe34688..116c8d5 100644
--- a/webrtc/api/webrtcsession_unittest.cc
+++ b/webrtc/api/webrtcsession_unittest.cc
@@ -3039,9 +3039,8 @@
// Checks if one of the transport channels contains a connection using a given
// port.
auto connection_with_remote_port = [this, voice_channel](int port) {
- SessionStats stats;
- session_->GetChannelTransportStats(voice_channel, &stats);
- for (auto& kv : stats.transport_stats) {
+ std::unique_ptr<webrtc::SessionStats> stats = session_->GetStats_s();
+ for (auto& kv : stats->transport_stats) {
for (auto& chan_stat : kv.second.channel_stats) {
for (auto& conn_info : chan_stat.connection_infos) {
if (conn_info.remote_candidate.address().port() == port) {