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}
This commit is contained in:
Tommi 2021-06-07 21:16:38 +02:00 committed by WebRTC LUCI CQ
parent 0fb949702b
commit 2e1546887b
4 changed files with 23 additions and 21 deletions

View File

@ -105,16 +105,6 @@ std::string Port::ComputeFoundation(const std::string& type,
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,

View File

@ -99,14 +99,24 @@ class StunStats {
// 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;

View File

@ -487,8 +487,10 @@ void BasicPortAllocatorSession::GetCandidateStatsFromReadyPorts(
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));
}
}

View File

@ -811,7 +811,7 @@ StatsReport* StatsCollector::AddConnectionInfoReport(
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 @@ StatsReport* StatsCollector::AddCandidateReport(
}
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,