Change RTCStatsCollector from std::unique_ptr to direct member
Bug: None
Change-Id: Ic9f9d9a55395e2b9954cb7225b0fd065f5fab5ed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/436060
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#46541}
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index b7520b6..a810e29 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -608,7 +608,7 @@
worker_thread())),
call_ptr_(call_.get()),
legacy_stats_(std::make_unique<LegacyStatsCollector>(this, env_.clock())),
- stats_collector_(RTCStatsCollector::Create(this, env_)),
+ stats_collector_(this, env_),
// RFC 3264: The numeric value of the session id and version in the
// o line MUST be representable with a "64 bit signed integer".
// Due to this constraint session id `session_id_` is max limited to
@@ -694,10 +694,8 @@
sdp_handler_->GetMediaChannelTeardownTasks(network_tasks, worker_tasks);
legacy_stats_.reset(nullptr);
- if (stats_collector_) {
- network_tasks.push_back(
- stats_collector_->CancelPendingRequestAndGetShutdownTask());
- }
+ network_tasks.push_back(
+ stats_collector_.CancelPendingRequestAndGetShutdownTask());
CloseOnNetworkThread(network_tasks);
@@ -1342,7 +1340,6 @@
RTC_LOG(LS_ERROR) << "Legacy GetStats - observer is NULL.";
return false;
}
-
RTC_LOG_THREAD_BLOCK_COUNT();
legacy_stats_->UpdateStats(level);
@@ -1364,10 +1361,9 @@
void PeerConnection::GetStats(RTCStatsCollectorCallback* callback) {
TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
RTC_DCHECK_RUN_ON(signaling_thread());
- RTC_DCHECK(stats_collector_);
RTC_DCHECK(callback);
RTC_LOG_THREAD_BLOCK_COUNT();
- stats_collector_->GetStatsReport(
+ stats_collector_.GetStatsReport(
scoped_refptr<RTCStatsCollectorCallback>(callback));
RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2);
}
@@ -1378,7 +1374,6 @@
TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(callback);
- RTC_DCHECK(stats_collector_);
RTC_LOG_THREAD_BLOCK_COUNT();
scoped_refptr<RtpSenderInternal> internal_sender;
if (selector) {
@@ -1400,7 +1395,7 @@
// 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);
+ stats_collector_.GetStatsReport(internal_sender, callback);
RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2);
}
@@ -1410,7 +1405,6 @@
TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
RTC_DCHECK_RUN_ON(signaling_thread());
RTC_DCHECK(callback);
- RTC_DCHECK(stats_collector_);
RTC_LOG_THREAD_BLOCK_COUNT();
scoped_refptr<RtpReceiverInternal> internal_receiver;
if (selector) {
@@ -1432,7 +1426,7 @@
// 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);
+ stats_collector_.GetStatsReport(internal_receiver, callback);
RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(2);
}
@@ -1929,9 +1923,7 @@
}
// Ensure that all asynchronous stats requests are completed before destroying
// the transport controller below.
- if (stats_collector_) {
- stats_collector_->WaitForPendingRequest();
- }
+ stats_collector_.WaitForPendingRequest();
// Don't destroy BaseChannels until after stats has been cleaned up so that
// the last stats request can still read from the channels.
@@ -2308,8 +2300,7 @@
int channel_id,
DataChannelInterface::DataState state) {
RTC_DCHECK_RUN_ON(signaling_thread());
- if (stats_collector_)
- stats_collector_->OnSctpDataChannelStateChanged(channel_id, state);
+ stats_collector_.OnSctpDataChannelStateChanged(channel_id, state);
}
PeerConnection::InitializePortAllocatorResult
@@ -3184,9 +3175,7 @@
if (legacy_stats_) {
legacy_stats_->InvalidateCache();
}
- if (stats_collector_) {
- stats_collector_->ClearCachedStatsReport();
- }
+ stats_collector_.ClearCachedStatsReport();
}
bool PeerConnection::ShouldFireNegotiationNeededEvent(uint32_t event_id) {
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index 1cf1d51..2315f85 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -709,8 +709,7 @@
std::unique_ptr<LegacyStatsCollector> legacy_stats_
RTC_GUARDED_BY(signaling_thread()); // A pointer is passed to senders_
- std::unique_ptr<RTCStatsCollector> stats_collector_
- RTC_GUARDED_BY(signaling_thread());
+ RTCStatsCollector stats_collector_ RTC_GUARDED_BY(signaling_thread());
const std::string session_id_;
diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc
index 847918f..cfb0825 100644
--- a/pc/rtc_stats_collector.cc
+++ b/pc/rtc_stats_collector.cc
@@ -1193,14 +1193,6 @@
RTC_DCHECK(!sender_selector_ || !receiver_selector_);
}
-std::unique_ptr<RTCStatsCollector> RTCStatsCollector::Create(
- PeerConnectionInternal* pc,
- const Environment& env,
- int64_t cache_lifetime_us) {
- return std::unique_ptr<RTCStatsCollector>(
- new RTCStatsCollector(pc, env, cache_lifetime_us));
-}
-
RTCStatsCollector::RTCStatsCollector(PeerConnectionInternal* pc,
const Environment& env,
int64_t cache_lifetime_us)
diff --git a/pc/rtc_stats_collector.h b/pc/rtc_stats_collector.h
index 9084ca4..79aefc5 100644
--- a/pc/rtc_stats_collector.h
+++ b/pc/rtc_stats_collector.h
@@ -75,11 +75,6 @@
// reports are cached for `cache_lifetime_` ms.
class RTCStatsCollector {
public:
- static std::unique_ptr<RTCStatsCollector> Create(
- PeerConnectionInternal* pc,
- const Environment& env,
- int64_t cache_lifetime_us = 50 * kNumMicrosecsPerMillisec);
-
// Gets a recent stats report. If there is a report cached that is still fresh
// it is returned, otherwise new stats are gathered and returned. A report is
// considered fresh for `cache_lifetime_` ms. const RTCStatsReports are safe
@@ -119,11 +114,11 @@
virtual ~RTCStatsCollector();
- protected:
RTCStatsCollector(PeerConnectionInternal* pc,
const Environment& env,
- int64_t cache_lifetime_us);
+ int64_t cache_lifetime_us = 50 * kNumMicrosecsPerMillisec);
+ protected:
struct CertificateStatsPair {
std::unique_ptr<SSLCertificateStats> local;
std::unique_ptr<SSLCertificateStats> remote;
diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc
index f055a99..0fffc6c 100644
--- a/pc/rtc_stats_collector_unittest.cc
+++ b/pc/rtc_stats_collector_unittest.cc
@@ -391,17 +391,13 @@
explicit RTCStatsCollectorWrapper(
scoped_refptr<FakePeerConnectionForStats> pc,
const Environment& env)
- : pc_(pc),
- stats_collector_(
- RTCStatsCollector::Create(pc_.get(),
- env,
- 50 * kNumMicrosecsPerMillisec)) {}
+ : pc_(pc), stats_collector_(pc_.get(), env) {}
- RTCStatsCollector* stats_collector() { return stats_collector_.get(); }
+ RTCStatsCollector& stats_collector() { return stats_collector_; }
scoped_refptr<const RTCStatsReport> GetStatsReport(RunLoop& loop) {
scoped_refptr<const RTCStatsReport> report;
- stats_collector_->GetStatsReport(
+ stats_collector_.GetStatsReport(
RTCStatsObtainer::Create(&report, [&loop] { loop.Quit(); }));
loop.Run();
int64_t after = TimeUTCMicros();
@@ -417,7 +413,7 @@
}
scoped_refptr<const RTCStatsReport> GetFreshStatsReport(RunLoop& loop) {
- stats_collector_->ClearCachedStatsReport();
+ stats_collector_.ClearCachedStatsReport();
return GetStatsReport(loop);
}
@@ -610,7 +606,7 @@
private:
scoped_refptr<FakePeerConnectionForStats> pc_;
- std::unique_ptr<RTCStatsCollector> stats_collector_;
+ RTCStatsCollector stats_collector_;
};
class RTCStatsCollectorTest : public ::testing::Test {
@@ -874,7 +870,7 @@
TEST_F(RTCStatsCollectorTest, SingleCallback) {
scoped_refptr<const RTCStatsReport> result;
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&result, [&] { main_thread_.Quit(); }));
main_thread_.Run();
EXPECT_TRUE(result);
@@ -887,11 +883,11 @@
if (--remaining == 0)
main_thread_.Quit();
};
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&a, on_complete));
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&b, on_complete));
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&c, on_complete));
main_thread_.Run();
EXPECT_TRUE(a);
@@ -908,7 +904,7 @@
scoped_refptr<const RTCStatsReport> b = stats_->GetStatsReport(main_thread_);
EXPECT_EQ(a.get(), b.get());
// Invalidate cache by clearing it.
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
scoped_refptr<const RTCStatsReport> c = stats_->GetStatsReport(main_thread_);
EXPECT_NE(b.get(), c.get());
// Invalidate cache by advancing time.
@@ -925,14 +921,14 @@
if (--remaining == 0)
main_thread_.Quit();
};
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&a, on_complete));
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&b, on_complete));
// Cache is invalidated after 50 ms.
main_thread_.Flush();
fake_clock_.AdvanceTime(TimeDelta::Millis(51));
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
RTCStatsObtainer::Create(&c, on_complete));
main_thread_.Run();
EXPECT_TRUE(a);
@@ -1206,7 +1202,7 @@
video_media_info.receivers.clear();
video_channels.first->SetStats(video_media_info);
video_channels.second->SetStats(video_media_info);
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
report = stats_->GetStatsReport(main_thread_);
EXPECT_FALSE(report->Get(expected_inbound_audio_codec.id()));
EXPECT_FALSE(report->Get(expected_outbound_audio_codec.id()));
@@ -1274,7 +1270,7 @@
// If a second transport is added with the same PT information, this does
// count as different codec objects.
pc_->AddVideoChannel("Mid4", "SecondTransport", info_pt10_pt11);
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
report = stats_->GetStatsReport(main_thread_);
codec_stats = report->GetStatsOfType<RTCCodecStats>();
EXPECT_EQ(codec_stats.size(), 4u);
@@ -1335,7 +1331,7 @@
// in duplicates.
pc_->AddVideoChannel("Mid3", "BundledTransport", info_nofec);
pc_->AddVideoChannel("Mid4", "BundledTransport", info_fec);
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
report = stats_->GetStatsReport(main_thread_);
codec_stats = report->GetStatsOfType<RTCCodecStats>();
EXPECT_EQ(codec_stats.size(), 2u);
@@ -1358,7 +1354,7 @@
info_fec_pt112.receivers[0].codec_payload_type =
inbound_codec_pt112_fec.payload_type;
pc_->AddVideoChannel("Mid5", "BundledTransport", info_fec_pt112);
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
report = stats_->GetStatsReport(main_thread_);
codec_stats = report->GetStatsOfType<RTCCodecStats>();
EXPECT_EQ(codec_stats.size(), 3u);
@@ -1524,7 +1520,7 @@
second_report, updated_remote_certinfo->fingerprints[1]));
// Clear the cache, including the cached certificates.
- stats_->stats_collector()->ClearCachedStatsReport();
+ stats_->stats_collector().ClearCachedStatsReport();
scoped_refptr<const RTCStatsReport> third_report =
stats_->GetStatsReport(main_thread_);
// Now the old certificates stats should be deleted.
@@ -2100,10 +2096,10 @@
controller.weak_ptr(), "DummyChannelB", false, InternalDataChannelInit(),
Thread::Current(), Thread::Current());
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_a->internal_id(), DataChannelInterface::DataState::kOpen);
// Closing a channel that is not opened should not affect the counts.
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_b->internal_id(), DataChannelInterface::DataState::kClosed);
{
@@ -2116,9 +2112,9 @@
EXPECT_EQ(expected, report->Get("P")->cast_to<RTCPeerConnectionStats>());
}
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_b->internal_id(), DataChannelInterface::DataState::kOpen);
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_b->internal_id(), DataChannelInterface::DataState::kClosed);
{
@@ -2133,7 +2129,7 @@
// Re-opening a data channel (or opening a new data channel that is re-using
// the same address in memory) should increase the opened count.
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_b->internal_id(), DataChannelInterface::DataState::kOpen);
{
@@ -2146,9 +2142,9 @@
EXPECT_EQ(expected, report->Get("P")->cast_to<RTCPeerConnectionStats>());
}
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_a->internal_id(), DataChannelInterface::DataState::kClosed);
- stats_->stats_collector()->OnSctpDataChannelStateChanged(
+ stats_->stats_collector().OnSctpDataChannelStateChanged(
dummy_channel_b->internal_id(), DataChannelInterface::DataState::kClosed);
{
@@ -3713,7 +3709,7 @@
// v v
// codec (send) transport
scoped_refptr<const RTCStatsReport> sender_report;
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
graph.sender,
RTCStatsObtainer::Create(&sender_report, [&] { main_thread_.Quit(); }));
main_thread_.Run();
@@ -3740,7 +3736,7 @@
// v v
// transport codec (recv)
scoped_refptr<const RTCStatsReport> receiver_report;
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
graph.receiver,
RTCStatsObtainer::Create(&receiver_report, [&] { main_thread_.Quit(); }));
main_thread_.Run();
@@ -3759,7 +3755,7 @@
TEST_F(RTCStatsCollectorTest, GetStatsWithNullSenderSelector) {
ExampleStatsGraph graph = SetupExampleStatsGraphForSelectorTests();
scoped_refptr<const RTCStatsReport> empty_report;
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
scoped_refptr<RtpSenderInternal>(nullptr),
RTCStatsObtainer::Create(&empty_report, [&] { main_thread_.Quit(); }));
main_thread_.Run();
@@ -3771,7 +3767,7 @@
TEST_F(RTCStatsCollectorTest, GetStatsWithNullReceiverSelector) {
ExampleStatsGraph graph = SetupExampleStatsGraphForSelectorTests();
scoped_refptr<const RTCStatsReport> empty_report;
- stats_->stats_collector()->GetStatsReport(
+ stats_->stats_collector().GetStatsReport(
scoped_refptr<RtpReceiverInternal>(nullptr),
RTCStatsObtainer::Create(&empty_report, [&] { main_thread_.Quit(); }));
main_thread_.Run();
@@ -3857,8 +3853,8 @@
stats_.get(), main_thread_, on_complete);
auto callback2 = make_ref_counted<RecursiveCallback>(
stats_.get(), main_thread_, on_complete);
- stats_->stats_collector()->GetStatsReport(callback1);
- stats_->stats_collector()->GetStatsReport(callback2);
+ stats_->stats_collector().GetStatsReport(callback1);
+ stats_->stats_collector().GetStatsReport(callback2);
main_thread_.Run();
EXPECT_TRUE(callback1->called());
EXPECT_TRUE(callback2->called());
@@ -3997,7 +3993,7 @@
RTCStatsCollectorWrapper wrapper(pc, env);
auto callback = make_ref_counted<MockStatsCollectorCallback>();
EXPECT_CALL(*callback, OnStatsDelivered(_)).WillOnce([&] { loop.Quit(); });
- wrapper.stats_collector()->GetStatsReport(callback);
+ wrapper.stats_collector().GetStatsReport(callback);
loop.Run();
}
@@ -4016,12 +4012,12 @@
EXPECT_CALL(*callback, OnStatsDelivered(_)).Times(0);
// At this point, cancellation has not been made, this posts a task to the
// network thread.
- wrapper.stats_collector()->GetStatsReport(callback);
+ wrapper.stats_collector().GetStatsReport(callback);
// Now cancel any ongoing stats gathering operations. This should have the
// effect that the gathering that is ongoing on the network thread, will queue
// up a task for the signaling thread, but that task will be dropped.
auto network_task =
- wrapper.stats_collector()->CancelPendingRequestAndGetShutdownTask();
+ wrapper.stats_collector().CancelPendingRequestAndGetShutdownTask();
loop.Flush();
// Run the network cleanup task for posterity.
std::move(network_task)();
@@ -4041,13 +4037,13 @@
// Start by canceling any ongoing tasks. There aren't actually any ongoing
// tasks, but this gives us the network cleanup task.
auto network_task =
- wrapper.stats_collector()->CancelPendingRequestAndGetShutdownTask();
+ wrapper.stats_collector().CancelPendingRequestAndGetShutdownTask();
// Clean up the state on the network thread. This will have the effect of
// dropping any tasks targeting the network thread.
std::move(network_task)();
// Now, attempt to get a stats report. This will try to post a task to the
// network thread, which will be dropped.
- wrapper.stats_collector()->GetStatsReport(callback);
+ wrapper.stats_collector().GetStatsReport(callback);
loop.Flush();
}
