Use a variant for storing stats values in StatsCollector code.
This cuts down on the amount of string copying we currently do and paves the way for separating the code that fetches the stats from the code that populates the stats reports. As is, that code is intertwined, so we populate the stats on both signaling and worker thread.
I'm also adding some documentation and TODOs for further improvements.
BUG=2822
R=pthatcher@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/47459004
Cr-Commit-Position: refs/heads/master@{#8700}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8700 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc
index ed5fbc7..9defeb7 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -641,7 +641,7 @@
int i = 0;
for (const auto* report : reports) {
ScopedLocalRefFrame local_ref_frame(jni);
- jstring j_id = JavaStringFromStdString(jni, report->id().ToString());
+ jstring j_id = JavaStringFromStdString(jni, report->id()->ToString());
jstring j_type = JavaStringFromStdString(jni, report->TypeToString());
jobjectArray j_values = ValuesToJava(jni, report->values());
jobject j_report = jni->NewObject(*j_stats_report_class_,
diff --git a/talk/app/webrtc/objc/RTCStatsReport.mm b/talk/app/webrtc/objc/RTCStatsReport.mm
index 9b5662f..04a3d27 100644
--- a/talk/app/webrtc/objc/RTCStatsReport.mm
+++ b/talk/app/webrtc/objc/RTCStatsReport.mm
@@ -50,14 +50,15 @@
- (instancetype)initWithStatsReport:(const webrtc::StatsReport&)statsReport {
if (self = [super init]) {
- _reportId = @(statsReport.id().ToString().c_str());
+ _reportId = @(statsReport.id()->ToString().c_str());
_type = @(statsReport.TypeToString());
_timestamp = statsReport.timestamp();
NSMutableArray* values =
[NSMutableArray arrayWithCapacity:statsReport.values().size()];
for (const auto& it : statsReport.values()) {
- RTCPair* pair = [[RTCPair alloc] initWithKey:@(it.second->display_name())
- value:@(it.second->value.c_str())];
+ RTCPair* pair =
+ [[RTCPair alloc] initWithKey:@(it.second->display_name())
+ value:@(it.second->ToString().c_str())];
[values addObject:pair];
}
_values = values;
diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc
index badc609..0567c3f 100644
--- a/talk/app/webrtc/statscollector.cc
+++ b/talk/app/webrtc/statscollector.cc
@@ -57,55 +57,40 @@
const char* STATSREPORT_ADAPTER_TYPE_VPN = "vpn";
const char* STATSREPORT_ADAPTER_TYPE_LOOPBACK = "loopback";
-struct FloatForAdd {
+template<typename ValueType>
+struct TypeForAdd {
const StatsReport::StatsValueName name;
- const float& value;
+ const ValueType& value;
};
-struct IntForAdd {
- const StatsReport::StatsValueName name;
- const int value;
-};
+typedef TypeForAdd<bool> BoolForAdd;
+typedef TypeForAdd<float> FloatForAdd;
+typedef TypeForAdd<int64> Int64ForAdd;
+typedef TypeForAdd<int> IntForAdd;
-bool GetTransportIdFromProxy(const cricket::ProxyTransportMap& map,
- const std::string& proxy,
- std::string* transport) {
- // TODO(hta): Remove handling of empty proxy name once tests do not use it.
- if (proxy.empty()) {
- transport->clear();
- return true;
- }
-
+StatsReport::Id GetTransportIdFromProxy(const cricket::ProxyTransportMap& map,
+ const std::string& proxy) {
+ DCHECK(!proxy.empty());
cricket::ProxyTransportMap::const_iterator found = map.find(proxy);
- if (found == map.end()) {
- LOG(LS_ERROR) << "No transport ID mapping for " << proxy;
- return false;
- }
+ if (found == map.end())
+ return StatsReport::Id();
- // Component 1 is always used for RTP.
- scoped_ptr<StatsReport::Id> id(
- StatsReport::NewComponentId(found->second, 1));
- // TODO(tommi): Should |transport| simply be of type StatsReport::Id?
- // When we support more value types than string (e.g. int, double, vector etc)
- // we should also support a value type for Id.
- *transport = id->ToString();
- return true;
+ return StatsReport::NewComponentId(
+ found->second, cricket::ICE_CANDIDATE_COMPONENT_RTP);
}
void AddTrackReport(StatsCollection* reports, const std::string& track_id) {
// Adds an empty track report.
- rtc::scoped_ptr<StatsReport::Id> id(
+ StatsReport::Id id(
StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack, track_id));
- StatsReport* report = reports->ReplaceOrAddNew(id.Pass());
+ StatsReport* report = reports->ReplaceOrAddNew(id);
report->AddString(StatsReport::kStatsValueNameTrackId, track_id);
}
template <class TrackVector>
void CreateTrackReports(const TrackVector& tracks, StatsCollection* reports) {
- for (size_t j = 0; j < tracks.size(); ++j) {
- webrtc::MediaStreamTrackInterface* track = tracks[j];
+ for (const auto& track : tracks)
AddTrackReport(reports, track->id());
- }
}
void ExtractCommonSendProperties(const cricket::MediaSenderInfo& info,
@@ -265,26 +250,20 @@
StatsReport* report) {
ASSERT(report->type() == StatsReport::kStatsReportTypeBwe);
- // Clear out stats from previous GatherStats calls if any.
- if (report->timestamp() != stats_gathering_started) {
- report->ResetValues();
- report->set_timestamp(stats_gathering_started);
- }
-
- report->AddInt(StatsReport::kStatsValueNameAvailableSendBandwidth,
- info.available_send_bandwidth);
- report->AddInt(StatsReport::kStatsValueNameAvailableReceiveBandwidth,
- info.available_recv_bandwidth);
- report->AddInt(StatsReport::kStatsValueNameTargetEncBitrate,
- info.target_enc_bitrate);
- report->AddInt(StatsReport::kStatsValueNameActualEncBitrate,
- info.actual_enc_bitrate);
- report->AddInt(StatsReport::kStatsValueNameRetransmitBitrate,
- info.retransmit_bitrate);
- report->AddInt(StatsReport::kStatsValueNameTransmitBitrate,
- info.transmit_bitrate);
- report->AddInt64(StatsReport::kStatsValueNameBucketDelay,
- info.bucket_delay);
+ report->set_timestamp(stats_gathering_started);
+ const IntForAdd ints[] = {
+ { StatsReport::kStatsValueNameAvailableSendBandwidth,
+ info.available_send_bandwidth },
+ { StatsReport::kStatsValueNameAvailableReceiveBandwidth,
+ info.available_recv_bandwidth },
+ { StatsReport::kStatsValueNameTargetEncBitrate, info.target_enc_bitrate },
+ { StatsReport::kStatsValueNameActualEncBitrate, info.actual_enc_bitrate },
+ { StatsReport::kStatsValueNameRetransmitBitrate, info.retransmit_bitrate },
+ { StatsReport::kStatsValueNameTransmitBitrate, info.transmit_bitrate },
+ };
+ for (const auto& i : ints)
+ report->AddInt(i.name, i.value);
+ report->AddInt64(StatsReport::kStatsValueNameBucketDelay, info.bucket_delay);
}
void ExtractRemoteStats(const cricket::MediaSenderInfo& info,
@@ -305,7 +284,7 @@
// for each type.
template<typename T>
void ExtractStatsFromList(const std::vector<T>& data,
- const std::string& transport_id,
+ const StatsReport::Id& transport_id,
StatsCollector* collector,
StatsReport::Direction direction) {
for (const auto& d : data) {
@@ -394,21 +373,20 @@
uint32 ssrc) {
ASSERT(session_->signaling_thread()->IsCurrent());
ASSERT(audio_track != NULL);
- for (LocalAudioTrackVector::iterator it = local_audio_tracks_.begin();
- it != local_audio_tracks_.end(); ++it) {
- ASSERT(it->first != audio_track || it->second != ssrc);
- }
+#if ENABLE_DEBUG
+ for (const auto& track : local_audio_tracks_)
+ ASSERT(track.first != audio_track || track.second != ssrc);
+#endif
local_audio_tracks_.push_back(std::make_pair(audio_track, ssrc));
// Create the kStatsReportTypeTrack report for the new track if there is no
// report yet.
- rtc::scoped_ptr<StatsReport::Id> id(
- StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack,
- audio_track->id()));
- StatsReport* report = reports_.Find(*id.get());
+ StatsReport::Id id(StatsReport::NewTypedId(StatsReport::kStatsReportTypeTrack,
+ audio_track->id()));
+ StatsReport* report = reports_.Find(id);
if (!report) {
- report = reports_.InsertNew(id.Pass());
+ report = reports_.InsertNew(id);
report->AddString(StatsReport::kStatsValueNameTrackId, audio_track->id());
}
}
@@ -416,15 +394,11 @@
void StatsCollector::RemoveLocalAudioTrack(AudioTrackInterface* audio_track,
uint32 ssrc) {
ASSERT(audio_track != NULL);
- for (LocalAudioTrackVector::iterator it = local_audio_tracks_.begin();
- it != local_audio_tracks_.end(); ++it) {
- if (it->first == audio_track && it->second == ssrc) {
- local_audio_tracks_.erase(it);
- return;
- }
- }
-
- ASSERT(false);
+ local_audio_tracks_.erase(std::remove_if(local_audio_tracks_.begin(),
+ local_audio_tracks_.end(),
+ [audio_track, ssrc](const LocalAudioTrackVector::value_type& track) {
+ return track.first == audio_track && track.second == ssrc;
+ }));
}
void StatsCollector::GetStats(MediaStreamTrackInterface* track,
@@ -433,6 +407,8 @@
ASSERT(reports != NULL);
ASSERT(reports->empty());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
if (!track) {
reports->reserve(reports_.size());
for (auto* r : reports_)
@@ -460,7 +436,7 @@
const StatsReport::Value* v =
r->FindValue(StatsReport::kStatsValueNameTrackId);
- if (v && v->value == track->id())
+ if (v && v->string_val() == track->id())
reports->push_back(r);
}
}
@@ -479,6 +455,12 @@
stats_gathering_started_ = time_now;
if (session_) {
+ // TODO(tommi): All of these hop over to the worker thread to fetch
+ // information. We could use an AsyncInvoker to run all of these and post
+ // the information back to the signaling thread where we can create and
+ // update stats reports. That would also clean up the threading story a bit
+ // since we'd be creating/updating the stats report objects consistently on
+ // the same thread (this class has no locks right now).
ExtractSessionInfo();
ExtractVoiceInfo();
ExtractVideoInfo(level);
@@ -489,15 +471,14 @@
StatsReport* StatsCollector::PrepareReport(
bool local,
uint32 ssrc,
- const std::string& transport_id,
+ const StatsReport::Id& transport_id,
StatsReport::Direction direction) {
ASSERT(session_->signaling_thread()->IsCurrent());
- const std::string ssrc_id = rtc::ToString<uint32>(ssrc);
- rtc::scoped_ptr<StatsReport::Id> id(StatsReport::NewIdWithDirection(
+ StatsReport::Id id(StatsReport::NewIdWithDirection(
local ? StatsReport::kStatsReportTypeSsrc :
StatsReport::kStatsReportTypeRemoteSsrc,
- ssrc_id, direction));
- StatsReport* report = reports_.Find(*id.get());
+ rtc::ToString<uint32>(ssrc), direction));
+ StatsReport* report = reports_.Find(id);
// Use the ID of the track that is currently mapped to the SSRC, if any.
std::string track_id;
@@ -513,36 +494,24 @@
const StatsReport::Value* v =
report->FindValue(StatsReport::kStatsValueNameTrackId);
if (v)
- track_id = v->value;
+ track_id = v->string_val();
}
- if (!report) {
- report = reports_.InsertNew(id.Pass());
- } else {
- // Clear out stats from previous GatherStats calls if any.
- // This is required since the report will be returned for the new values.
- // Having the old values in the report will lead to multiple values with
- // the same name.
- // TODO(tommi): This seems to be pretty wasteful if some of these values
- // have not changed (we basically throw them away just to recreate them).
- // Figure out a way to not have to do this while not breaking the existing
- // functionality.
- report->ResetValues();
- }
+ if (!report)
+ report = reports_.InsertNew(id);
- ASSERT(report->empty());
// FYI - for remote reports, the timestamp will be overwritten later.
report->set_timestamp(stats_gathering_started_);
- report->AddString(StatsReport::kStatsValueNameSsrc, ssrc_id);
+ report->AddInt64(StatsReport::kStatsValueNameSsrc, ssrc);
report->AddString(StatsReport::kStatsValueNameTrackId, track_id);
// Add the mapping of SSRC to transport.
- report->AddString(StatsReport::kStatsValueNameTransportId, transport_id);
+ report->AddId(StatsReport::kStatsValueNameTransportId, transport_id);
return report;
}
-std::string StatsCollector::AddOneCertificateReport(
- const rtc::SSLCertificate* cert, const std::string& issuer_id) {
+StatsReport* StatsCollector::AddOneCertificateReport(
+ const rtc::SSLCertificate* cert, const StatsReport* issuer) {
ASSERT(session_->signaling_thread()->IsCurrent());
// TODO(bemasc): Move this computation to a helper class that caches these
@@ -551,7 +520,7 @@
std::string digest_algorithm;
if (!cert->GetSignatureDigestAlgorithm(&digest_algorithm))
- return std::string();
+ return nullptr;
rtc::scoped_ptr<rtc::SSLFingerprint> ssl_fingerprint(
rtc::SSLFingerprint::Create(digest_algorithm, cert));
@@ -561,7 +530,7 @@
// implementation of SSLCertificate::ComputeDigest. This currently happens
// with MD5- and SHA-224-signed certificates when linked to libNSS.
if (!ssl_fingerprint)
- return std::string();
+ return nullptr;
std::string fingerprint = ssl_fingerprint->GetRfc4572Fingerprint();
@@ -571,22 +540,20 @@
rtc::Base64::EncodeFromArray(
der_buffer.data(), der_buffer.length(), &der_base64);
- rtc::scoped_ptr<StatsReport::Id> id(
- StatsReport::NewTypedId(
- StatsReport::kStatsReportTypeCertificate, fingerprint));
- StatsReport* report = reports_.ReplaceOrAddNew(id.Pass());
+ StatsReport::Id id(StatsReport::NewTypedId(
+ StatsReport::kStatsReportTypeCertificate, fingerprint));
+ StatsReport* report = reports_.ReplaceOrAddNew(id);
report->set_timestamp(stats_gathering_started_);
report->AddString(StatsReport::kStatsValueNameFingerprint, fingerprint);
report->AddString(StatsReport::kStatsValueNameFingerprintAlgorithm,
digest_algorithm);
report->AddString(StatsReport::kStatsValueNameDer, der_base64);
- if (!issuer_id.empty())
- report->AddString(StatsReport::kStatsValueNameIssuerId, issuer_id);
- // TODO(tommi): Can we avoid this?
- return report->id().ToString();
+ if (issuer)
+ report->AddId(StatsReport::kStatsValueNameIssuerId, issuer->id());
+ return report;
}
-std::string StatsCollector::AddCertificateReports(
+StatsReport* StatsCollector::AddCertificateReports(
const rtc::SSLCertificate* cert) {
ASSERT(session_->signaling_thread()->IsCurrent());
// Produces a chain of StatsReports representing this certificate and the rest
@@ -596,7 +563,7 @@
// empty.
ASSERT(cert != NULL);
- std::string issuer_id;
+ StatsReport* issuer = nullptr;
rtc::scoped_ptr<rtc::SSLCertChain> chain;
if (cert->GetChain(chain.accept())) {
// This loop runs in reverse, i.e. from root to leaf, so that each
@@ -605,21 +572,68 @@
// value.
for (ptrdiff_t i = chain->GetSize() - 1; i >= 0; --i) {
const rtc::SSLCertificate& cert_i = chain->Get(i);
- issuer_id = AddOneCertificateReport(&cert_i, issuer_id);
+ issuer = AddOneCertificateReport(&cert_i, issuer);
}
}
// Add the leaf certificate.
- return AddOneCertificateReport(cert, issuer_id);
+ return AddOneCertificateReport(cert, issuer);
}
-std::string StatsCollector::AddCandidateReport(
+StatsReport* StatsCollector::AddConnectionInfoReport(
+ const std::string& content_name, int component, int connection_id,
+ const StatsReport::Id& channel_report_id,
+ const cricket::ConnectionInfo& info) {
+ StatsReport::Id id(StatsReport::NewCandidatePairId(content_name, component,
+ connection_id));
+ StatsReport* report = reports_.ReplaceOrAddNew(id);
+ report->set_timestamp(stats_gathering_started_);
+
+ const BoolForAdd bools[] = {
+ { StatsReport::kStatsValueNameActiveConnection, info.best_connection },
+ { StatsReport::kStatsValueNameReadable, info.readable },
+ { StatsReport::kStatsValueNameWritable, info.writable },
+ };
+ for (const auto& b : bools)
+ report->AddBoolean(b.name, b.value);
+
+ report->AddId(StatsReport::kStatsValueNameChannelId, channel_report_id);
+ report->AddId(StatsReport::kStatsValueNameLocalCandidateId,
+ AddCandidateReport(info.local_candidate, true)->id());
+ report->AddId(StatsReport::kStatsValueNameRemoteCandidateId,
+ AddCandidateReport(info.remote_candidate, false)->id());
+
+ const Int64ForAdd int64s[] = {
+ { StatsReport::kStatsValueNameBytesReceived, info.recv_total_bytes },
+ { StatsReport::kStatsValueNameBytesSent, info.sent_total_bytes },
+ { StatsReport::kStatsValueNamePacketsSent, info.sent_total_packets },
+ { StatsReport::kStatsValueNameRtt, info.rtt },
+ { StatsReport::kStatsValueNameSendPacketsDiscarded,
+ info.sent_discarded_packets },
+ };
+ for (const auto& i : int64s)
+ report->AddInt64(i.name, i.value);
+
+ report->AddString(StatsReport::kStatsValueNameLocalAddress,
+ info.local_candidate.address().ToString());
+ report->AddString(StatsReport::kStatsValueNameLocalCandidateType,
+ info.local_candidate.type());
+ report->AddString(StatsReport::kStatsValueNameRemoteAddress,
+ info.remote_candidate.address().ToString());
+ report->AddString(StatsReport::kStatsValueNameRemoteCandidateType,
+ info.remote_candidate.type());
+ report->AddString(StatsReport::kStatsValueNameTransportType,
+ info.local_candidate.protocol());
+
+ return report;
+}
+
+StatsReport* StatsCollector::AddCandidateReport(
const cricket::Candidate& candidate,
bool local) {
- scoped_ptr<StatsReport::Id> id(
- StatsReport::NewCandidateId(local, candidate.id()));
- StatsReport* report = reports_.Find(*id.get());
+ StatsReport::Id id(StatsReport::NewCandidateId(local, candidate.id()));
+ StatsReport* report = reports_.Find(id);
if (!report) {
- report = reports_.InsertNew(id.Pass());
+ report = reports_.InsertNew(id);
report->set_timestamp(stats_gathering_started_);
if (local) {
report->AddString(StatsReport::kStatsValueNameCandidateNetworkType,
@@ -637,30 +651,31 @@
candidate.protocol());
}
- // TODO(tommi): Necessary?
- return report->id().ToString();
+ return report;
}
void StatsCollector::ExtractSessionInfo() {
ASSERT(session_->signaling_thread()->IsCurrent());
+
// Extract information from the base session.
- rtc::scoped_ptr<StatsReport::Id> id(
- StatsReport::NewTypedId(
- StatsReport::kStatsReportTypeSession, session_->id()));
- StatsReport* report = reports_.ReplaceOrAddNew(id.Pass());
+ StatsReport::Id id(StatsReport::NewTypedId(
+ StatsReport::kStatsReportTypeSession, session_->id()));
+ StatsReport* report = reports_.ReplaceOrAddNew(id);
report->set_timestamp(stats_gathering_started_);
- report->ResetValues();
report->AddBoolean(StatsReport::kStatsValueNameInitiator,
session_->initiator());
cricket::SessionStats stats;
if (session_->GetStats(&stats)) {
// Store the proxy map away for use in SSRC reporting.
+ // TODO(tommi): This shouldn't be necessary if we post the stats back to the
+ // signaling thread after fetching them on the worker thread, then just use
+ // 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;
- for (cricket::TransportStatsMap::iterator transport_iter
- = stats.transport_stats.begin();
- transport_iter != stats.transport_stats.end(); ++transport_iter) {
+ 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.
@@ -669,104 +684,61 @@
// invoke method calls on the worker thread and block this thread, but
// messages are still processed on this thread, which may blow way the
// existing transports. So we cannot reuse |transport| after these calls.
- std::string local_cert_report_id, remote_cert_report_id;
+ StatsReport::Id local_cert_report_id, remote_cert_report_id;
cricket::Transport* transport =
- session_->GetTransport(transport_iter->second.content_name);
+ session_->GetTransport(transport_iter.second.content_name);
rtc::scoped_ptr<rtc::SSLIdentity> identity;
if (transport && transport->GetIdentity(identity.accept())) {
- local_cert_report_id =
- AddCertificateReports(&(identity->certificate()));
+ StatsReport* r = AddCertificateReports(&(identity->certificate()));
+ if (r)
+ local_cert_report_id = r->id();
}
- transport = session_->GetTransport(transport_iter->second.content_name);
+ transport = session_->GetTransport(transport_iter.second.content_name);
rtc::scoped_ptr<rtc::SSLCertificate> cert;
if (transport && transport->GetRemoteCertificate(cert.accept())) {
- remote_cert_report_id = AddCertificateReports(cert.get());
+ StatsReport* r = AddCertificateReports(cert.get());
+ if (r)
+ remote_cert_report_id = r->id();
}
- for (cricket::TransportChannelStatsList::iterator channel_iter
- = transport_iter->second.channel_stats.begin();
- channel_iter != transport_iter->second.channel_stats.end();
- ++channel_iter) {
- rtc::scoped_ptr<StatsReport::Id> id(
- StatsReport::NewComponentId(transport_iter->second.content_name,
- channel_iter->component));
- StatsReport* channel_report = reports_.ReplaceOrAddNew(id.Pass());
+ for (const auto& channel_iter : transport_iter.second.channel_stats) {
+ StatsReport::Id id(StatsReport::NewComponentId(
+ transport_iter.second.content_name, channel_iter.component));
+ StatsReport* channel_report = reports_.ReplaceOrAddNew(id);
channel_report->set_timestamp(stats_gathering_started_);
channel_report->AddInt(StatsReport::kStatsValueNameComponent,
- channel_iter->component);
- if (!local_cert_report_id.empty()) {
- channel_report->AddString(
- StatsReport::kStatsValueNameLocalCertificateId,
- local_cert_report_id);
+ channel_iter.component);
+ if (local_cert_report_id.get()) {
+ channel_report->AddId(StatsReport::kStatsValueNameLocalCertificateId,
+ local_cert_report_id);
}
- if (!remote_cert_report_id.empty()) {
- channel_report->AddString(
- StatsReport::kStatsValueNameRemoteCertificateId,
- remote_cert_report_id);
+ if (remote_cert_report_id.get()) {
+ channel_report->AddId(StatsReport::kStatsValueNameRemoteCertificateId,
+ remote_cert_report_id);
}
- const std::string& srtp_cipher = channel_iter->srtp_cipher;
+ const std::string& srtp_cipher = channel_iter.srtp_cipher;
if (!srtp_cipher.empty()) {
- channel_report->AddString(
- StatsReport::kStatsValueNameSrtpCipher,
- srtp_cipher);
+ channel_report->AddString(StatsReport::kStatsValueNameSrtpCipher,
+ srtp_cipher);
}
- const std::string& ssl_cipher = channel_iter->ssl_cipher;
+ const std::string& ssl_cipher = channel_iter.ssl_cipher;
if (!ssl_cipher.empty()) {
- channel_report->AddString(
- StatsReport::kStatsValueNameDtlsCipher,
- ssl_cipher);
+ channel_report->AddString(StatsReport::kStatsValueNameDtlsCipher,
+ ssl_cipher);
}
- for (size_t i = 0;
- i < channel_iter->connection_infos.size();
- ++i) {
- rtc::scoped_ptr<StatsReport::Id> id(
- StatsReport::NewCandidatePairId(transport_iter->first,
- channel_iter->component, static_cast<int>(i)));
- StatsReport* report = reports_.ReplaceOrAddNew(id.Pass());
- report->set_timestamp(stats_gathering_started_);
- // Link from connection to its containing channel.
- // TODO(tommi): Any way to avoid ToString here?
- report->AddString(StatsReport::kStatsValueNameChannelId,
- channel_report->id().ToString());
- const cricket::ConnectionInfo& info =
- channel_iter->connection_infos[i];
- report->AddInt64(StatsReport::kStatsValueNameBytesSent,
- info.sent_total_bytes);
- report->AddInt64(StatsReport::kStatsValueNameSendPacketsDiscarded,
- info.sent_discarded_packets);
- report->AddInt64(StatsReport::kStatsValueNamePacketsSent,
- info.sent_total_packets);
- report->AddInt64(StatsReport::kStatsValueNameBytesReceived,
- info.recv_total_bytes);
- report->AddBoolean(StatsReport::kStatsValueNameWritable,
- info.writable);
- report->AddBoolean(StatsReport::kStatsValueNameReadable,
- info.readable);
- report->AddString(StatsReport::kStatsValueNameLocalCandidateId,
- AddCandidateReport(info.local_candidate, true));
- report->AddString(
- StatsReport::kStatsValueNameRemoteCandidateId,
- AddCandidateReport(info.remote_candidate, false));
- report->AddString(StatsReport::kStatsValueNameLocalAddress,
- info.local_candidate.address().ToString());
- report->AddString(StatsReport::kStatsValueNameRemoteAddress,
- info.remote_candidate.address().ToString());
- report->AddInt64(StatsReport::kStatsValueNameRtt, info.rtt);
- report->AddString(StatsReport::kStatsValueNameTransportType,
- info.local_candidate.protocol());
- report->AddString(StatsReport::kStatsValueNameLocalCandidateType,
- info.local_candidate.type());
- report->AddString(StatsReport::kStatsValueNameRemoteCandidateType,
- info.remote_candidate.type());
- report->AddBoolean(StatsReport::kStatsValueNameActiveConnection,
- info.best_connection);
+ int connection_id = 0;
+ for (const cricket::ConnectionInfo& info :
+ channel_iter.connection_infos) {
+ StatsReport* connection_report = AddConnectionInfoReport(
+ transport_iter.first, channel_iter.component, connection_id++,
+ channel_report->id(), info);
if (info.best_connection) {
- channel_report->AddString(
+ channel_report->AddId(
StatsReport::kStatsValueNameSelectedCandidatePairId,
- report->id().ToString());
+ connection_report->id());
}
}
}
@@ -785,14 +757,19 @@
LOG(LS_ERROR) << "Failed to get voice channel stats.";
return;
}
- std::string transport_id;
- if (!GetTransportIdFromProxy(proxy_to_transport_,
- session_->voice_channel()->content_name(),
- &transport_id)) {
+
+ // TODO(tommi): The above code should run on the worker thread and post the
+ // results back to the signaling thread, where we can add data to the reports.
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
+ StatsReport::Id transport_id(GetTransportIdFromProxy(proxy_to_transport_,
+ session_->voice_channel()->content_name()));
+ if (!transport_id.get()) {
LOG(LS_ERROR) << "Failed to get transport name for proxy "
<< session_->voice_channel()->content_name();
return;
}
+
ExtractStatsFromList(voice_info.receivers, transport_id, this,
StatsReport::kReceive);
ExtractStatsFromList(voice_info.senders, transport_id, this,
@@ -813,10 +790,14 @@
LOG(LS_ERROR) << "Failed to get video channel stats.";
return;
}
- std::string transport_id;
- if (!GetTransportIdFromProxy(proxy_to_transport_,
- session_->video_channel()->content_name(),
- &transport_id)) {
+
+ // TODO(tommi): The above code should run on the worker thread and post the
+ // results back to the signaling thread, where we can add data to the reports.
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
+ StatsReport::Id transport_id(GetTransportIdFromProxy(proxy_to_transport_,
+ session_->video_channel()->content_name()));
+ if (!transport_id.get()) {
LOG(LS_ERROR) << "Failed to get transport name for proxy "
<< session_->video_channel()->content_name();
return;
@@ -828,9 +809,8 @@
if (video_info.bw_estimations.size() != 1) {
LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size();
} else {
- rtc::scoped_ptr<StatsReport::Id> report_id(
- StatsReport::NewBandwidthEstimationId());
- StatsReport* report = reports_.FindOrAddNew(report_id.Pass());
+ StatsReport::Id report_id(StatsReport::NewBandwidthEstimationId());
+ StatsReport* report = reports_.FindOrAddNew(report_id);
ExtractStats(
video_info.bw_estimations[0], stats_gathering_started_, level, report);
}
@@ -839,11 +819,13 @@
void StatsCollector::ExtractDataInfo() {
ASSERT(session_->signaling_thread()->IsCurrent());
+ rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls;
+
for (const auto& dc :
session_->mediastream_signaling()->sctp_data_channels()) {
- rtc::scoped_ptr<StatsReport::Id> id(StatsReport::NewTypedIntId(
+ StatsReport::Id id(StatsReport::NewTypedIntId(
StatsReport::kStatsReportTypeDataChannel, dc->id()));
- StatsReport* report = reports_.ReplaceOrAddNew(id.Pass());
+ StatsReport* report = reports_.ReplaceOrAddNew(id);
report->set_timestamp(stats_gathering_started_);
report->AddString(StatsReport::kStatsValueNameLabel, dc->label());
report->AddInt(StatsReport::kStatsValueNameDataChannelId, dc->id());
@@ -865,13 +847,11 @@
void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() {
ASSERT(session_->signaling_thread()->IsCurrent());
// Loop through the existing local audio tracks.
- for (LocalAudioTrackVector::const_iterator it = local_audio_tracks_.begin();
- it != local_audio_tracks_.end(); ++it) {
- AudioTrackInterface* track = it->first;
- uint32 ssrc = it->second;
- const std::string ssrc_id = rtc::ToString<uint32>(ssrc);
+ for (const auto& it : local_audio_tracks_) {
+ AudioTrackInterface* track = it.first;
+ uint32 ssrc = it.second;
StatsReport* report = GetReport(StatsReport::kStatsReportTypeSsrc,
- ssrc_id,
+ rtc::ToString<uint32>(ssrc),
StatsReport::kSend);
if (report == NULL) {
// This can happen if a local audio track is added to a stream on the
@@ -883,7 +863,7 @@
// The same ssrc can be used by both local and remote audio tracks.
const StatsReport::Value* v =
report->FindValue(StatsReport::kStatsValueNameTrackId);
- if (!v || v->value != track->id())
+ if (!v || v->string_val() != track->id())
continue;
UpdateReportFromAudioTrack(track, report);
diff --git a/talk/app/webrtc/statscollector.h b/talk/app/webrtc/statscollector.h
index b4ca1fc..3c0aaf9 100644
--- a/talk/app/webrtc/statscollector.h
+++ b/talk/app/webrtc/statscollector.h
@@ -87,7 +87,7 @@
// Prepare a local or remote SSRC report for the given ssrc. Used internally
// in the ExtractStatsFromList template.
StatsReport* PrepareReport(bool local, uint32 ssrc,
- const std::string& transport_id, StatsReport::Direction direction);
+ const StatsReport::Id& transport_id, StatsReport::Direction direction);
// Method used by the unittest to force a update of stats since UpdateStats()
// that occur less than kMinGatherStatsPeriod number of ms apart will be
@@ -103,17 +103,22 @@
bool CopySelectedReports(const std::string& selector, StatsReports* reports);
// Helper method for AddCertificateReports.
- std::string AddOneCertificateReport(
- const rtc::SSLCertificate* cert, const std::string& issuer_id);
+ StatsReport* AddOneCertificateReport(
+ const rtc::SSLCertificate* cert, const StatsReport* issuer);
// Helper method for creating IceCandidate report. |is_local| indicates
// whether this candidate is local or remote.
- std::string AddCandidateReport(const cricket::Candidate& candidate,
- bool local);
+ StatsReport* AddCandidateReport(const cricket::Candidate& candidate,
+ bool local);
// Adds a report for this certificate and every certificate in its chain, and
- // returns the leaf certificate's report's ID.
- std::string AddCertificateReports(const rtc::SSLCertificate* cert);
+ // returns the leaf certificate's report.
+ StatsReport* AddCertificateReports(const rtc::SSLCertificate* cert);
+
+ StatsReport* AddConnectionInfoReport(const std::string& content_name,
+ int component, int connection_id,
+ const StatsReport::Id& channel_report_id,
+ const cricket::ConnectionInfo& info);
void ExtractDataInfo();
void ExtractSessionInfo();
@@ -141,6 +146,8 @@
double stats_gathering_started_;
cricket::ProxyTransportMap proxy_to_transport_;
+ // TODO(tommi): We appear to be holding on to raw pointers to reference
+ // counted objects? We should be using scoped_refptr here.
typedef std::vector<std::pair<AudioTrackInterface*, uint32> >
LocalAudioTrackVector;
LocalAudioTrackVector local_audio_tracks_;
diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc
index e16ff33..5d41dc8 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -168,12 +168,12 @@
return kNotFound;
}
-scoped_ptr<StatsReport::Id> TypedIdFromIdString(StatsReport::StatsType type,
- const std::string& value) {
+StatsReport::Id TypedIdFromIdString(StatsReport::StatsType type,
+ const std::string& value) {
EXPECT_FALSE(value.empty());
- scoped_ptr<StatsReport::Id> id;
+ StatsReport::Id id;
if (value.empty())
- return id.Pass();
+ return id;
// This has assumptions about how the ID is constructed. As is, this is
// OK since this is for testing purposes only, but if we ever need this
@@ -181,16 +181,15 @@
size_t index = value.find('_');
EXPECT_NE(index, std::string::npos);
if (index == std::string::npos || index == (value.length() - 1))
- return id.Pass();
+ return id;
id = StatsReport::NewTypedId(type, value.substr(index + 1));
EXPECT_EQ(id->ToString(), value);
- return id.Pass();
+ return id;
}
-scoped_ptr<StatsReport::Id> IdFromCertIdString(const std::string& cert_id) {
- return TypedIdFromIdString(StatsReport::kStatsReportTypeCertificate, cert_id)
- .Pass();
+StatsReport::Id IdFromCertIdString(const std::string& cert_id) {
+ return TypedIdFromIdString(StatsReport::kStatsReportTypeCertificate, cert_id);
}
// Finds the |n|-th report of type |type| in |reports|.
@@ -210,7 +209,7 @@
const StatsReport* FindReportById(const StatsReports& reports,
const StatsReport::Id& id) {
for (const auto* r : reports) {
- if (r->id().Equals(id))
+ if (r->id()->Equals(id))
return r;
}
return nullptr;
@@ -244,7 +243,7 @@
void CheckCertChainReports(const StatsReports& reports,
const std::vector<std::string>& ders,
const StatsReport::Id& start_id) {
- scoped_ptr<StatsReport::Id> cert_id;
+ StatsReport::Id cert_id;
const StatsReport::Id* certificate_id = &start_id;
size_t i = 0;
while (true) {
@@ -278,8 +277,8 @@
break;
}
- cert_id = IdFromCertIdString(issuer_id).Pass();
- certificate_id = cert_id.get();
+ cert_id = IdFromCertIdString(issuer_id);
+ certificate_id = &cert_id;
}
EXPECT_EQ(ders.size(), i);
}
@@ -543,9 +542,9 @@
.WillOnce(DoAll(SetArgPointee<1>(kRemoteTrackId), Return(true)));
}
- std::string AddCandidateReport(StatsCollector* collector,
- const cricket::Candidate& candidate,
- bool local) {
+ StatsReport* AddCandidateReport(StatsCollector* collector,
+ const cricket::Candidate& candidate,
+ bool local) {
return collector->AddCandidateReport(candidate, local);
}
@@ -688,8 +687,8 @@
StatsReport::kStatsValueNameLocalCertificateId);
if (local_ders.size() > 0) {
EXPECT_NE(kNotFound, local_certificate_id);
- scoped_ptr<StatsReport::Id> id(IdFromCertIdString(local_certificate_id));
- CheckCertChainReports(reports, local_ders, *id.get());
+ StatsReport::Id id(IdFromCertIdString(local_certificate_id));
+ CheckCertChainReports(reports, local_ders, id);
} else {
EXPECT_EQ(kNotFound, local_certificate_id);
}
@@ -701,8 +700,8 @@
StatsReport::kStatsValueNameRemoteCertificateId);
if (remote_ders.size() > 0) {
EXPECT_NE(kNotFound, remote_certificate_id);
- scoped_ptr<StatsReport::Id> id(IdFromCertIdString(remote_certificate_id));
- CheckCertChainReports(reports, remote_ders, *id.get());
+ StatsReport::Id id(IdFromCertIdString(remote_certificate_id));
+ CheckCertChainReports(reports, remote_ders, id);
} else {
EXPECT_EQ(kNotFound, remote_certificate_id);
}
@@ -755,7 +754,7 @@
const StatsReport* report =
FindNthReportByType(reports, StatsReport::kStatsReportTypeDataChannel, 1);
- scoped_ptr<StatsReport::Id> reportId = StatsReport::NewTypedIntId(
+ StatsReport::Id reportId = StatsReport::NewTypedIntId(
StatsReport::kStatsReportTypeDataChannel, id);
EXPECT_TRUE(reportId->Equals(report->id()));
@@ -780,9 +779,18 @@
TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) {
StatsCollectorForTest stats(&session_);
+ const char kVideoChannelName[] = "video";
+
+ InitSessionStats(kVideoChannelName);
+ EXPECT_CALL(session_, GetStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
+ Return(true)));
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
- media_engine_, media_channel, &session_, "", false, NULL);
+ media_engine_, media_channel, &session_, kVideoChannelName, false, NULL);
StatsReports reports; // returned values.
cricket::VideoSenderInfo video_sender_info;
cricket::VideoMediaInfo stats_read;
@@ -814,9 +822,19 @@
TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) {
StatsCollectorForTest stats(&session_);
+ const char kVideoChannelName[] = "video";
+
+ InitSessionStats(kVideoChannelName);
+ EXPECT_CALL(session_, GetStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
+ Return(true)));
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
- media_engine_, media_channel, &session_, "", false, NULL);
+ media_engine_, media_channel, &session_, kVideoChannelName, false, NULL);
+
StatsReports reports; // returned values.
cricket::VideoSenderInfo video_sender_info;
cricket::VideoMediaInfo stats_read;
@@ -894,7 +912,7 @@
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
- media_engine_, media_channel, &session_, "", false, NULL);
+ media_engine_, media_channel, &session_, "video", false, NULL);
AddOutgoingVideoTrackStats();
stats.AddStream(stream_);
@@ -916,9 +934,17 @@
TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) {
StatsCollectorForTest stats(&session_);
+ const char kVideoChannelName[] = "video";
+ InitSessionStats(kVideoChannelName);
+ EXPECT_CALL(session_, GetStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
+ Return(true)));
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
- media_engine_, media_channel, &session_, "", false, NULL);
+ media_engine_, media_channel, &session_, kVideoChannelName, false, NULL);
AddOutgoingVideoTrackStats();
stats.AddStream(stream_);
@@ -1022,9 +1048,9 @@
index = content.rfind('-');
ASSERT_NE(std::string::npos, index);
content = content.substr(0, index);
- scoped_ptr<StatsReport::Id> id(StatsReport::NewComponentId(content, 1));
+ StatsReport::Id id(StatsReport::NewComponentId(content, 1));
ASSERT_EQ(transport_id, id->ToString());
- const StatsReport* transport_report = FindReportById(reports, *id.get());
+ const StatsReport* transport_report = FindReportById(reports, id);
ASSERT_FALSE(transport_report == NULL);
}
@@ -1106,9 +1132,17 @@
TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) {
StatsCollectorForTest stats(&session_);
+ const char kVideoChannelName[] = "video";
+ InitSessionStats(kVideoChannelName);
+ EXPECT_CALL(session_, GetStats(_))
+ .WillRepeatedly(DoAll(SetArgPointee<0>(session_stats_),
+ Return(true)));
+ EXPECT_CALL(session_, GetTransport(_))
+ .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
+
MockVideoMediaChannel* media_channel = new MockVideoMediaChannel();
cricket::VideoChannel video_channel(rtc::Thread::Current(),
- media_engine_, media_channel, &session_, "", false, NULL);
+ media_engine_, media_channel, &session_, kVideoChannelName, false, NULL);
AddIncomingVideoTrackStats();
stats.AddStream(stream_);
@@ -1171,7 +1205,7 @@
c.set_address(local_address);
c.set_priority(priority);
c.set_network_type(network_type);
- std::string report_id = AddCandidateReport(&stats, c, true);
+ std::string report_id = AddCandidateReport(&stats, c, true)->id()->ToString();
EXPECT_EQ("Cand-" + c.id(), report_id);
c = cricket::Candidate();
@@ -1181,7 +1215,7 @@
c.set_address(remote_address);
c.set_priority(priority);
c.set_network_type(network_type);
- report_id = AddCandidateReport(&stats, c, false);
+ report_id = AddCandidateReport(&stats, c, false)->id()->ToString();
EXPECT_EQ("Cand-" + c.id(), report_id);
stats.GetStats(NULL, &reports);
diff --git a/talk/app/webrtc/statstypes.cc b/talk/app/webrtc/statstypes.cc
index af81ed0..cc6eaa0 100644
--- a/talk/app/webrtc/statstypes.cc
+++ b/talk/app/webrtc/statstypes.cc
@@ -27,7 +27,17 @@
#include "talk/app/webrtc/statstypes.h"
-using rtc::scoped_ptr;
+#include <string.h>
+
+#include "webrtc/base/checks.h"
+
+// TODO(tommi): Could we have a static map of value name -> expected type
+// and use this to DCHECK on correct usage (somewhat strongly typed values)?
+// Alternatively, we could define the names+type in a separate document and
+// generate strongly typed inline C++ code that forces the correct type to be
+// used for a given name at compile time.
+
+ using rtc::RefCountedObject;
namespace webrtc {
namespace {
@@ -64,23 +74,24 @@
case StatsReport::kStatsReportTypeDataChannel:
return "datachannel";
}
- ASSERT(false);
+ DCHECK(false);
return nullptr;
}
-class BandwidthEstimationId : public StatsReport::Id {
+class BandwidthEstimationId : public StatsReport::IdBase {
public:
- BandwidthEstimationId() : StatsReport::Id(StatsReport::kStatsReportTypeBwe) {}
+ BandwidthEstimationId()
+ : StatsReport::IdBase(StatsReport::kStatsReportTypeBwe) {}
std::string ToString() const override { return kStatsReportVideoBweId; }
};
-class TypedId : public StatsReport::Id {
+class TypedId : public StatsReport::IdBase {
public:
TypedId(StatsReport::StatsType type, const std::string& id)
- : StatsReport::Id(type), id_(id) {}
+ : StatsReport::IdBase(type), id_(id) {}
- bool Equals(const Id& other) const override {
- return Id::Equals(other) &&
+ bool Equals(const IdBase& other) const override {
+ return IdBase::Equals(other) &&
static_cast<const TypedId&>(other).id_ == id_;
}
@@ -92,13 +103,13 @@
const std::string id_;
};
-class TypedIntId : public StatsReport::Id {
+class TypedIntId : public StatsReport::IdBase {
public:
TypedIntId(StatsReport::StatsType type, int id)
- : StatsReport::Id(type), id_(id) {}
+ : StatsReport::IdBase(type), id_(id) {}
- bool Equals(const Id& other) const override {
- return Id::Equals(other) &&
+ bool Equals(const IdBase& other) const override {
+ return IdBase::Equals(other) &&
static_cast<const TypedIntId&>(other).id_ == id_;
}
@@ -118,7 +129,7 @@
StatsReport::Direction direction)
: TypedId(type, id), direction_(direction) {}
- bool Equals(const Id& other) const override {
+ bool Equals(const IdBase& other) const override {
return TypedId::Equals(other) &&
static_cast<const IdWithDirection&>(other).direction_ == direction_;
}
@@ -148,14 +159,14 @@
}
};
-class ComponentId : public StatsReport::Id {
+class ComponentId : public StatsReport::IdBase {
public:
ComponentId(const std::string& content_name, int component)
: ComponentId(StatsReport::kStatsReportTypeComponent, content_name,
component) {}
- bool Equals(const Id& other) const override {
- return Id::Equals(other) &&
+ bool Equals(const IdBase& other) const override {
+ return IdBase::Equals(other) &&
static_cast<const ComponentId&>(other).component_ == component_ &&
static_cast<const ComponentId&>(other).content_name_ == content_name_;
}
@@ -167,7 +178,7 @@
protected:
ComponentId(StatsReport::StatsType type, const std::string& content_name,
int component)
- : Id(type),
+ : IdBase(type),
content_name_(content_name),
component_(component) {}
@@ -191,7 +202,7 @@
component),
index_(index) {}
- bool Equals(const Id& other) const override {
+ bool Equals(const IdBase& other) const override {
return ComponentId::Equals(other) &&
static_cast<const CandidatePairId&>(other).index_ == index_;
}
@@ -209,17 +220,160 @@
} // namespace
-StatsReport::Id::Id(StatsType type) : type_(type) {}
-StatsReport::Id::~Id() {}
+StatsReport::IdBase::IdBase(StatsType type) : type_(type) {}
+StatsReport::IdBase::~IdBase() {}
-StatsReport::StatsType StatsReport::Id::type() const { return type_; }
+StatsReport::StatsType StatsReport::IdBase::type() const { return type_; }
-bool StatsReport::Id::Equals(const Id& other) const {
+bool StatsReport::IdBase::Equals(const IdBase& other) const {
return other.type_ == type_;
}
+StatsReport::Value::Value(StatsValueName name, int64 value, Type int_type)
+ : name(name), type_(int_type) {
+ DCHECK(type_ == kInt || type_ == kInt64);
+ type_ == kInt ? value_.int_ = static_cast<int>(value) : value_.int64_ = value;
+}
+
+StatsReport::Value::Value(StatsValueName name, float f)
+ : name(name), type_(kFloat) {
+ value_.float_ = f;
+}
+
StatsReport::Value::Value(StatsValueName name, const std::string& value)
- : name(name), value(value) {
+ : name(name), type_(kString) {
+ value_.string_ = new std::string(value);
+}
+
+StatsReport::Value::Value(StatsValueName name, const char* value)
+ : name(name), type_(kStaticString) {
+ value_.static_string_ = value;
+}
+
+StatsReport::Value::Value(StatsValueName name, bool b)
+ : name(name), type_(kBool) {
+ value_.bool_ = b;
+}
+
+StatsReport::Value::Value(StatsValueName name, const Id& value)
+ : name(name), type_(kId) {
+ value_.id_ = new Id(value);
+}
+
+StatsReport::Value::~Value() {
+ switch (type_) {
+ case kInt:
+ case kInt64:
+ case kFloat:
+ case kBool:
+ case kStaticString:
+ break;
+ case kString:
+ delete value_.string_;
+ break;
+ case kId:
+ delete value_.id_;
+ break;
+ }
+}
+
+bool StatsReport::Value::Equals(const Value& other) const {
+ if (name != other.name)
+ return false;
+
+ // There's a 1:1 relation between a name and a type, so we don't have to
+ // check that.
+ DCHECK_EQ(type_, other.type_);
+
+ switch (type_) {
+ case kInt:
+ return value_.int_ == other.value_.int_;
+ case kInt64:
+ return value_.int64_ == other.value_.int64_;
+ case kFloat:
+ return value_.float_ == other.value_.float_;
+ case kStaticString: {
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ if (value_.static_string_ != other.value_.static_string_) {
+ DCHECK(strcmp(value_.static_string_, other.value_.static_string_) != 0)
+ << "Duplicate global?";
+ }
+#endif
+ return value_.static_string_ == other.value_.static_string_;
+ }
+ case kString:
+ return *value_.string_ == *other.value_.string_;
+ case kBool:
+ return value_.bool_ == other.value_.bool_;
+ case kId:
+ return (*value_.id_)->Equals(*other.value_.id_);
+ }
+ RTC_NOTREACHED();
+ return false;
+}
+
+bool StatsReport::Value::operator==(const std::string& value) const {
+ return (type_ == kString && value_.string_->compare(value) == 0) ||
+ (type_ == kStaticString && value.compare(value_.static_string_) == 0);
+}
+
+bool StatsReport::Value::operator==(const char* value) const {
+ if (type_ == kString)
+ return value_.string_->compare(value) == 0;
+ if (type_ != kStaticString)
+ return false;
+#if (!defined(NDEBUG) || defined(DCHECK_ALWAYS_ON))
+ if (value_.static_string_ != value)
+ DCHECK(strcmp(value_.static_string_, value) != 0) << "Duplicate global?";
+#endif
+ return value == value_.static_string_;
+}
+
+bool StatsReport::Value::operator==(int64 value) const {
+ return type_ == kInt ? value_.int_ == static_cast<int>(value) :
+ (type_ == kInt64 ? value_.int64_ == value : false);
+}
+
+bool StatsReport::Value::operator==(bool value) const {
+ return type_ == kBool && value_.bool_ == value;
+}
+
+bool StatsReport::Value::operator==(float value) const {
+ return type_ == kFloat && value_.float_ == value;
+}
+
+bool StatsReport::Value::operator==(const Id& value) const {
+ return type_ == kId && (*value_.id_)->Equals(value);
+}
+
+int StatsReport::Value::int_val() const {
+ DCHECK(type_ == kInt);
+ return value_.int_;
+}
+
+int64 StatsReport::Value::int64_val() const {
+ DCHECK(type_ == kInt64);
+ return value_.int64_;
+}
+
+float StatsReport::Value::float_val() const {
+ DCHECK(type_ == kFloat);
+ return value_.float_;
+}
+
+const char* StatsReport::Value::static_string_val() const {
+ DCHECK(type_ == kStaticString);
+ return value_.static_string_;
+}
+
+const std::string& StatsReport::Value::string_val() const {
+ DCHECK(type_ == kString);
+ return *value_.string_;
+}
+
+bool StatsReport::Value::bool_val() const {
+ DCHECK(type_ == kBool);
+ return value_.bool_;
}
const char* StatsReport::Value::display_name() const {
@@ -437,60 +591,75 @@
case kStatsValueNameWritable:
return "googWritable";
default:
- ASSERT(false);
+ DCHECK(false);
break;
}
return nullptr;
}
-const std::string& StatsReport::Value::ToString() const {
- return value;
+std::string StatsReport::Value::ToString() const {
+ switch (type_) {
+ case kInt:
+ return rtc::ToString(value_.int_);
+ case kInt64:
+ return rtc::ToString(value_.int64_);
+ case kFloat:
+ return rtc::ToString(value_.float_);
+ case kStaticString:
+ return std::string(value_.static_string_);
+ case kString:
+ return *value_.string_;
+ case kBool:
+ return value_.bool_ ? "true" : "false";
+ case kId:
+ return (*value_.id_)->ToString();
+ }
+ RTC_NOTREACHED();
+ return std::string();
}
-StatsReport::StatsReport(scoped_ptr<Id> id) : id_(id.Pass()), timestamp_(0.0) {
- ASSERT(id_.get());
+StatsReport::StatsReport(const Id& id) : id_(id), timestamp_(0.0) {
+ DCHECK(id_.get());
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewBandwidthEstimationId() {
- return scoped_ptr<Id>(new BandwidthEstimationId()).Pass();
+StatsReport::Id StatsReport::NewBandwidthEstimationId() {
+ return Id(new RefCountedObject<BandwidthEstimationId>());
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewTypedId(
- StatsType type, const std::string& id) {
- return scoped_ptr<Id>(new TypedId(type, id)).Pass();
+StatsReport::Id StatsReport::NewTypedId(StatsType type, const std::string& id) {
+ return Id(new RefCountedObject<TypedId>(type, id));
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewTypedIntId(StatsType type, int id) {
- return scoped_ptr<Id>(new TypedIntId(type, id)).Pass();
+StatsReport::Id StatsReport::NewTypedIntId(StatsType type, int id) {
+ return Id(new RefCountedObject<TypedIntId>(type, id));
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewIdWithDirection(
+StatsReport::Id StatsReport::NewIdWithDirection(
StatsType type, const std::string& id, StatsReport::Direction direction) {
- return scoped_ptr<Id>(new IdWithDirection(type, id, direction)).Pass();
+ return Id(new RefCountedObject<IdWithDirection>(type, id, direction));
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewCandidateId(
- bool local, const std::string& id) {
- return scoped_ptr<Id>(new CandidateId(local, id)).Pass();
+StatsReport::Id StatsReport::NewCandidateId(bool local, const std::string& id) {
+ return Id(new RefCountedObject<CandidateId>(local, id));
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewComponentId(
+StatsReport::Id StatsReport::NewComponentId(
const std::string& content_name, int component) {
- return scoped_ptr<Id>(new ComponentId(content_name, component)).Pass();
+ return Id(new RefCountedObject<ComponentId>(content_name, component));
}
// static
-scoped_ptr<StatsReport::Id> StatsReport::NewCandidatePairId(
+StatsReport::Id StatsReport::NewCandidatePairId(
const std::string& content_name, int component, int index) {
- return scoped_ptr<Id>(new CandidatePairId(content_name, component, index))
- .Pass();
+ return Id(new RefCountedObject<CandidatePairId>(
+ content_name, component, index));
}
const char* StatsReport::TypeToString() const {
@@ -499,28 +668,47 @@
void StatsReport::AddString(StatsReport::StatsValueName name,
const std::string& value) {
- values_[name] = ValuePtr(new Value(name, value));
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value));
+}
+
+void StatsReport::AddString(StatsReport::StatsValueName name,
+ const char* value) {
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value));
}
void StatsReport::AddInt64(StatsReport::StatsValueName name, int64 value) {
- AddString(name, rtc::ToString<int64>(value));
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value, Value::kInt64));
}
void StatsReport::AddInt(StatsReport::StatsValueName name, int value) {
- AddString(name, rtc::ToString<int>(value));
+ const Value* found = FindValue(name);
+ if (!found || !(*found == static_cast<int64>(value)))
+ values_[name] = ValuePtr(new Value(name, value, Value::kInt));
}
void StatsReport::AddFloat(StatsReport::StatsValueName name, float value) {
- AddString(name, rtc::ToString<float>(value));
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value));
}
void StatsReport::AddBoolean(StatsReport::StatsValueName name, bool value) {
- // TODO(tommi): Store bools as bool.
- AddString(name, value ? "true" : "false");
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value));
}
-void StatsReport::ResetValues() {
- values_.clear();
+void StatsReport::AddId(StatsReport::StatsValueName name,
+ const Id& value) {
+ const Value* found = FindValue(name);
+ if (!found || !(*found == value))
+ values_[name] = ValuePtr(new Value(name, value));
}
const StatsReport::Value* StatsReport::FindValue(StatsValueName name) const {
@@ -548,41 +736,37 @@
return list_.size();
}
-StatsReport* StatsCollection::InsertNew(scoped_ptr<StatsReport::Id> id) {
- ASSERT(Find(*id.get()) == NULL);
- StatsReport* report = new StatsReport(id.Pass());
+StatsReport* StatsCollection::InsertNew(const StatsReport::Id& id) {
+ DCHECK(Find(id) == nullptr);
+ StatsReport* report = new StatsReport(id);
list_.push_back(report);
return report;
}
-StatsReport* StatsCollection::FindOrAddNew(scoped_ptr<StatsReport::Id> id) {
- StatsReport* ret = Find(*id.get());
- return ret ? ret : InsertNew(id.Pass());
+StatsReport* StatsCollection::FindOrAddNew(const StatsReport::Id& id) {
+ StatsReport* ret = Find(id);
+ return ret ? ret : InsertNew(id);
}
-StatsReport* StatsCollection::ReplaceOrAddNew(scoped_ptr<StatsReport::Id> id) {
- ASSERT(id.get());
+StatsReport* StatsCollection::ReplaceOrAddNew(const StatsReport::Id& id) {
+ DCHECK(id.get());
Container::iterator it = std::find_if(list_.begin(), list_.end(),
- [&id](const StatsReport* r)->bool { return r->id().Equals(*id.get()); });
+ [&id](const StatsReport* r)->bool { return r->id()->Equals(id); });
if (it != end()) {
+ StatsReport* report = new StatsReport((*it)->id());
delete *it;
- StatsReport* report = new StatsReport(id.Pass());
*it = report;
return report;
}
- return InsertNew(id.Pass());
+ return InsertNew(id);
}
// Looks for a report with the given |id|. If one is not found, NULL
// will be returned.
StatsReport* StatsCollection::Find(const StatsReport::Id& id) {
Container::iterator it = std::find_if(list_.begin(), list_.end(),
- [&id](const StatsReport* r)->bool { return r->id().Equals(id); });
+ [&id](const StatsReport* r)->bool { return r->id()->Equals(id); });
return it == list_.end() ? nullptr : *it;
}
-StatsReport* StatsCollection::Find(const scoped_ptr<StatsReport::Id>& id) {
- return Find(*id.get());
-}
-
} // namespace webrtc
diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h
index 22fc9d7..de4fb5f 100644
--- a/talk/app/webrtc/statstypes.h
+++ b/talk/app/webrtc/statstypes.h
@@ -37,11 +37,11 @@
#include <string>
#include "webrtc/base/basictypes.h"
-#include "webrtc/base/linked_ptr.h"
-#include "webrtc/base/scoped_ptr.h"
#include "webrtc/base/common.h"
-#include "webrtc/base/linked_ptr.h"
+#include "webrtc/base/refcount.h"
#include "webrtc/base/scoped_ptr.h"
+#include "webrtc/base/linked_ptr.h"
+#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/base/stringencode.h"
namespace webrtc {
@@ -221,57 +221,129 @@
kStatsValueNameWritable,
};
- class Id {
+ class IdBase : public rtc::RefCountInterface {
public:
- virtual ~Id();
+ ~IdBase() override;
StatsType type() const;
- virtual bool Equals(const Id& other) const;
+
+ // Users of IdBase will be using the Id typedef, which is compatible with
+ // this Equals() function. It simply calls the protected (and overridden)
+ // Equals() method.
+ bool Equals(const rtc::scoped_refptr<IdBase>& other) const {
+ return Equals(*other.get());
+ }
+
virtual std::string ToString() const = 0;
- // TODO(tommi): Remove this after rolling into Chrome.
- const Id* operator->() const { return this; }
-
protected:
- Id(StatsType type); // Only meant for derived classes.
+ // Protected since users of the IdBase type will be using the Id typedef.
+ virtual bool Equals(const IdBase& other) const;
+
+ IdBase(StatsType type); // Only meant for derived classes.
const StatsType type_;
static const char kSeparator = '_';
};
+ typedef rtc::scoped_refptr<IdBase> Id;
+
struct Value {
+ enum Type {
+ kInt, // int.
+ kInt64, // int64.
+ kFloat, // float.
+ kString, // std::string
+ kStaticString, // const char*.
+ kBool, // bool.
+ kId, // Id.
+ };
+
+ Value(StatsValueName name, int64 value, Type int_type);
+ Value(StatsValueName name, float f);
Value(StatsValueName name, const std::string& value);
+ Value(StatsValueName name, const char* value);
+ Value(StatsValueName name, bool b);
+ Value(StatsValueName name, const Id& value);
+
+ ~Value();
+
+ // TODO(tommi): This compares name as well as value...
+ // I think we should only need to compare the value part and
+ // move the name part into a hash map.
+ bool Equals(const Value& other) const;
+
+ // Comparison operators. Return true iff the current instance is of the
+ // correct type and holds the same value. No conversion is performed so
+ // a string value of "123" is not equal to an int value of 123 and an int
+ // value of 123 is not equal to a float value of 123.0f.
+ // One exception to this is that types kInt and kInt64 can be compared and
+ // kString and kStaticString too.
+ bool operator==(const std::string& value) const;
+ bool operator==(const char* value) const;
+ bool operator==(int64 value) const;
+ bool operator==(bool value) const;
+ bool operator==(float value) const;
+ bool operator==(const Id& value) const;
+
+ // Getters that allow getting the native value directly.
+ // The caller must know the type beforehand or else hit a check.
+ int int_val() const;
+ int64 int64_val() const;
+ float float_val() const;
+ const char* static_string_val() const;
+ const std::string& string_val() const;
+ bool bool_val() const;
+ const Id& id_val() const;
// Returns the string representation of |name|.
const char* display_name() const;
- const std::string& ToString() const;
+
+ // Converts the native value to a string representation of the value.
+ std::string ToString() const;
+
+ Type type() const { return type_; }
+
// TODO(tommi): Move |name| and |display_name| out of the Value struct.
const StatsValueName name;
- // TODO(tommi): Support more value types than string.
- const std::string value;
+
+ private:
+ const Type type_;
+ // TODO(tommi): Use C++ 11 union and make value_ const.
+ union InternalType {
+ int int_;
+ int64 int64_;
+ float float_;
+ bool bool_;
+ std::string* string_;
+ const char* static_string_;
+ Id* id_;
+ } value_;
private:
DISALLOW_COPY_AND_ASSIGN(Value);
};
+ // TODO(tommi): Consider using a similar approach to how we store Ids using
+ // scoped_refptr for values.
typedef rtc::linked_ptr<Value> ValuePtr;
typedef std::map<StatsValueName, ValuePtr> Values;
// Ownership of |id| is passed to |this|.
- explicit StatsReport(rtc::scoped_ptr<Id> id);
+ explicit StatsReport(const Id& id);
// Factory functions for various types of stats IDs.
- static rtc::scoped_ptr<Id> NewBandwidthEstimationId();
- static rtc::scoped_ptr<Id> NewTypedId(StatsType type, const std::string& id);
- static rtc::scoped_ptr<Id> NewTypedIntId(StatsType type, int id);
- static rtc::scoped_ptr<Id> NewIdWithDirection(
+ static Id NewBandwidthEstimationId();
+ static Id NewTypedId(StatsType type, const std::string& id);
+ static Id NewTypedIntId(StatsType type, int id);
+ static Id NewIdWithDirection(
StatsType type, const std::string& id, Direction direction);
- static rtc::scoped_ptr<Id> NewCandidateId(bool local, const std::string& id);
- static rtc::scoped_ptr<Id> NewComponentId(
+ static Id NewCandidateId(bool local, const std::string& id);
+ static Id NewComponentId(
const std::string& content_name, int component);
- static rtc::scoped_ptr<Id> NewCandidatePairId(
+ static Id NewCandidatePairId(
const std::string& content_name, int component, int index);
- const Id& id() const { return *id_.get(); }
+ const Id& id() const { return id_; }
StatsType type() const { return id_->type(); }
double timestamp() const { return timestamp_; }
void set_timestamp(double t) { timestamp_ = t; }
@@ -281,12 +353,12 @@
const char* TypeToString() const;
void AddString(StatsValueName name, const std::string& value);
+ void AddString(StatsValueName name, const char* value);
void AddInt64(StatsValueName name, int64 value);
void AddInt(StatsValueName name, int value);
void AddFloat(StatsValueName name, float value);
void AddBoolean(StatsValueName name, bool value);
-
- void ResetValues();
+ void AddId(StatsValueName name, const Id& value);
const Value* FindValue(StatsValueName name) const;
@@ -294,7 +366,7 @@
// The unique identifier for this object.
// This is used as a key for this report in ordered containers,
// so it must never be changed.
- const rtc::scoped_ptr<Id> id_;
+ const Id id_;
double timestamp_; // Time since 1970-01-01T00:00:00Z in milliseconds.
Values values_;
@@ -317,7 +389,6 @@
StatsCollection();
~StatsCollection();
- // TODO(tommi): shared_ptr (or linked_ptr)?
typedef std::list<StatsReport*> Container;
typedef Container::iterator iterator;
typedef Container::const_iterator const_iterator;
@@ -328,15 +399,13 @@
// Creates a new report object with |id| that does not already
// exist in the list of reports.
- StatsReport* InsertNew(rtc::scoped_ptr<StatsReport::Id> id);
- StatsReport* FindOrAddNew(rtc::scoped_ptr<StatsReport::Id> id);
- StatsReport* ReplaceOrAddNew(rtc::scoped_ptr<StatsReport::Id> id);
+ StatsReport* InsertNew(const StatsReport::Id& id);
+ StatsReport* FindOrAddNew(const StatsReport::Id& id);
+ StatsReport* ReplaceOrAddNew(const StatsReport::Id& id);
// Looks for a report with the given |id|. If one is not found, NULL
// will be returned.
StatsReport* Find(const StatsReport::Id& id);
- // TODO(tommi): we should only need one of these.
- StatsReport* Find(const rtc::scoped_ptr<StatsReport::Id>& id);
private:
Container list_;
