Avoid generating a random id for candidate stats.
CandidateStats didn't use an initializer list which caused the
`candidate` member variable to be constructed with a random id
(calling an expensive rng method), only to be overwritten directly
thereafter.
Bug: webrtc:12840
Change-Id: I0366f674281d236896cb9539812dc2d88c1b37ef
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/221600
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34244}
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 0a123b4..a03a0d6 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -105,16 +105,6 @@
return rtc::ToString(rtc::ComputeCrc32(sb.Release()));
}
-CandidateStats::CandidateStats() = default;
-
-CandidateStats::CandidateStats(const CandidateStats&) = default;
-
-CandidateStats::CandidateStats(Candidate candidate) {
- this->candidate = candidate;
-}
-
-CandidateStats::~CandidateStats() = default;
-
Port::Port(rtc::Thread* thread,
const std::string& type,
rtc::PacketSocketFactory* factory,
diff --git a/p2p/base/port.h b/p2p/base/port.h
index 7759ade..2c18f1a 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -99,14 +99,24 @@
// Stats that we can return about a candidate.
class CandidateStats {
public:
- CandidateStats();
- explicit CandidateStats(Candidate candidate);
- CandidateStats(const CandidateStats&);
- ~CandidateStats();
+ CandidateStats() = default;
+ CandidateStats(const CandidateStats&) = default;
+ CandidateStats(CandidateStats&&) = default;
+ CandidateStats(Candidate candidate,
+ absl::optional<StunStats> stats = absl::nullopt)
+ : candidate_(std::move(candidate)), stun_stats_(std::move(stats)) {}
+ ~CandidateStats() = default;
- Candidate candidate;
+ CandidateStats& operator=(const CandidateStats& other) = default;
+
+ const Candidate& candidate() const { return candidate_; }
+
+ const absl::optional<StunStats>& stun_stats() const { return stun_stats_; }
+
+ private:
+ Candidate candidate_;
// STUN port stats if this candidate is a STUN candidate.
- absl::optional<StunStats> stun_stats;
+ absl::optional<StunStats> stun_stats_;
};
typedef std::vector<CandidateStats> CandidateStatsList;
diff --git a/p2p/client/basic_port_allocator.cc b/p2p/client/basic_port_allocator.cc
index 9d7a4f9..1d38a4c 100644
--- a/p2p/client/basic_port_allocator.cc
+++ b/p2p/client/basic_port_allocator.cc
@@ -487,8 +487,10 @@
for (auto* port : ports) {
auto candidates = port->Candidates();
for (const auto& candidate : candidates) {
- CandidateStats candidate_stats(allocator_->SanitizeCandidate(candidate));
- port->GetStunStats(&candidate_stats.stun_stats);
+ absl::optional<StunStats> stun_stats;
+ port->GetStunStats(&stun_stats);
+ CandidateStats candidate_stats(allocator_->SanitizeCandidate(candidate),
+ std::move(stun_stats));
candidate_stats_list->push_back(std::move(candidate_stats));
}
}
diff --git a/pc/stats_collector.cc b/pc/stats_collector.cc
index 1728cb4..7376e24 100644
--- a/pc/stats_collector.cc
+++ b/pc/stats_collector.cc
@@ -811,7 +811,7 @@
StatsReport* StatsCollector::AddCandidateReport(
const cricket::CandidateStats& candidate_stats,
bool local) {
- const auto& candidate = candidate_stats.candidate;
+ const auto& candidate = candidate_stats.candidate();
StatsReport::Id id(StatsReport::NewCandidateId(local, candidate.id()));
StatsReport* report = reports_.Find(id);
if (!report) {
@@ -834,8 +834,8 @@
}
report->set_timestamp(stats_gathering_started_);
- if (local && candidate_stats.stun_stats.has_value()) {
- const auto& stun_stats = candidate_stats.stun_stats.value();
+ if (local && candidate_stats.stun_stats().has_value()) {
+ const auto& stun_stats = candidate_stats.stun_stats().value();
report->AddInt64(StatsReport::kStatsValueNameSentStunKeepaliveRequests,
stun_stats.stun_binding_requests_sent);
report->AddInt64(StatsReport::kStatsValueNameRecvStunKeepaliveResponses,
