diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index ed5fbc75b7..9defeb772b 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -641,7 +641,7 @@ class StatsObserverWrapper : public StatsObserver { 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 9b5662f155..04a3d274d1 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 badc6091b8..0567c3fe34 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -57,55 +57,40 @@ const char* STATSREPORT_ADAPTER_TYPE_WWAN = "wwan"; const char* STATSREPORT_ADAPTER_TYPE_VPN = "vpn"; const char* STATSREPORT_ADAPTER_TYPE_LOOPBACK = "loopback"; -struct FloatForAdd { +template +struct TypeForAdd { const StatsReport::StatsValueName name; - const float& value; + const ValueType& value; }; -struct IntForAdd { - const StatsReport::StatsValueName name; - const int value; -}; - -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; - } +typedef TypeForAdd BoolForAdd; +typedef TypeForAdd FloatForAdd; +typedef TypeForAdd Int64ForAdd; +typedef TypeForAdd IntForAdd; +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 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 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 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 @@ void ExtractStats(const cricket::BandwidthEstimationInfo& info, 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 @@ void ExtractRemoteStats(const cricket::MediaReceiverInfo& info, // for each type. template void ExtractStatsFromList(const std::vector& 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 @@ void StatsCollector::AddLocalAudioTrack(AudioTrackInterface* audio_track, 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 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::AddLocalAudioTrack(AudioTrackInterface* audio_track, 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 @@ void StatsCollector::GetStats(MediaStreamTrackInterface* track, 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 @@ void StatsCollector::GetStats(MediaStreamTrackInterface* track, 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 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { 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 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { 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(ssrc); - rtc::scoped_ptr id(StatsReport::NewIdWithDirection( + StatsReport::Id id(StatsReport::NewIdWithDirection( local ? StatsReport::kStatsReportTypeSsrc : StatsReport::kStatsReportTypeRemoteSsrc, - ssrc_id, direction)); - StatsReport* report = reports_.Find(*id.get()); + rtc::ToString(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 @@ StatsReport* StatsCollector::PrepareReport( 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 StatsCollector::AddOneCertificateReport( std::string digest_algorithm; if (!cert->GetSignatureDigestAlgorithm(&digest_algorithm)) - return std::string(); + return nullptr; rtc::scoped_ptr ssl_fingerprint( rtc::SSLFingerprint::Create(digest_algorithm, cert)); @@ -561,7 +530,7 @@ std::string StatsCollector::AddOneCertificateReport( // 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 @@ std::string StatsCollector::AddOneCertificateReport( rtc::Base64::EncodeFromArray( der_buffer.data(), der_buffer.length(), &der_base64); - rtc::scoped_ptr 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 @@ std::string StatsCollector::AddCertificateReports( // empty. ASSERT(cert != NULL); - std::string issuer_id; + StatsReport* issuer = nullptr; rtc::scoped_ptr chain; if (cert->GetChain(chain.accept())) { // This loop runs in reverse, i.e. from root to leaf, so that each @@ -605,21 +572,68 @@ std::string StatsCollector::AddCertificateReports( // 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 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 @@ std::string StatsCollector::AddCandidateReport( 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 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 @@ void StatsCollector::ExtractSessionInfo() { // 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 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 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 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 id( - StatsReport::NewCandidatePairId(transport_iter->first, - channel_iter->component, static_cast(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 @@ void StatsCollector::ExtractVoiceInfo() { 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 @@ void StatsCollector::ExtractVideoInfo( 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 @@ void StatsCollector::ExtractVideoInfo( if (video_info.bw_estimations.size() != 1) { LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size(); } else { - rtc::scoped_ptr 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::ExtractVideoInfo( 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 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 @@ StatsReport* StatsCollector::GetReport(const StatsReport::StatsType& type, 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(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(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 @@ void StatsCollector::UpdateStatsFromExistingLocalAudioTracks() { // 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 b4ca1fc12d..3c0aaf9b8e 100644 --- a/talk/app/webrtc/statscollector.h +++ b/talk/app/webrtc/statscollector.h @@ -87,7 +87,7 @@ class StatsCollector { // 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 @@ class StatsCollector { 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 @@ class StatsCollector { 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 > LocalAudioTrackVector; LocalAudioTrackVector local_audio_tracks_; diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index e16ff335d1..5d41dc8db6 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -168,12 +168,12 @@ std::string ExtractStatsValue(const StatsReport::StatsType& type, return kNotFound; } -scoped_ptr TypedIdFromIdString(StatsReport::StatsType type, - const std::string& value) { +StatsReport::Id TypedIdFromIdString(StatsReport::StatsType type, + const std::string& value) { EXPECT_FALSE(value.empty()); - scoped_ptr 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 @@ scoped_ptr TypedIdFromIdString(StatsReport::StatsType type, 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 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* FindNthReportByType( 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 @@ std::vector DersToPems( void CheckCertChainReports(const StatsReports& reports, const std::vector& ders, const StatsReport::Id& start_id) { - scoped_ptr cert_id; + StatsReport::Id cert_id; const StatsReport::Id* certificate_id = &start_id; size_t i = 0; while (true) { @@ -278,8 +277,8 @@ void CheckCertChainReports(const StatsReports& reports, 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 @@ class StatsCollectorTest : public testing::Test { .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 @@ class StatsCollectorTest : public testing::Test { StatsReport::kStatsValueNameLocalCertificateId); if (local_ders.size() > 0) { EXPECT_NE(kNotFound, local_certificate_id); - scoped_ptr 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 @@ class StatsCollectorTest : public testing::Test { StatsReport::kStatsValueNameRemoteCertificateId); if (remote_ders.size() > 0) { EXPECT_NE(kNotFound, remote_certificate_id); - scoped_ptr 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 @@ TEST_F(StatsCollectorTest, ExtractDataInfo) { const StatsReport* report = FindNthReportByType(reports, StatsReport::kStatsReportTypeDataChannel, 1); - scoped_ptr reportId = StatsReport::NewTypedIntId( + StatsReport::Id reportId = StatsReport::NewTypedIntId( StatsReport::kStatsReportTypeDataChannel, id); EXPECT_TRUE(reportId->Equals(report->id())); @@ -780,9 +779,18 @@ TEST_F(StatsCollectorTest, ExtractDataInfo) { 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(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, BytesCounterHandles64Bits) { 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(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 @@ TEST_F(StatsCollectorTest, TrackObjectExistsWithoutUpdateStats) { 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, TrackObjectExistsWithoutUpdateStats) { 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(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 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { index = content.rfind('-'); ASSERT_NE(std::string::npos, index); content = content.substr(0, index); - scoped_ptr 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, RemoteSsrcInfoIsPresent) { 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(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 @@ TEST_F(StatsCollectorTest, IceCandidateReport) { 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 @@ TEST_F(StatsCollectorTest, IceCandidateReport) { 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 af81ed025f..cc6eaa0e57 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 + +#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 @@ const char* InternalTypeToString(StatsReport::StatsType type) { 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(other).id_ == id_; } @@ -92,13 +103,13 @@ class TypedId : public StatsReport::Id { 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(other).id_ == id_; } @@ -118,7 +129,7 @@ class IdWithDirection : public TypedId { 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(other).direction_ == direction_; } @@ -148,14 +159,14 @@ class CandidateId : public TypedId { } }; -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(other).component_ == component_ && static_cast(other).content_name_ == content_name_; } @@ -167,7 +178,7 @@ class ComponentId : public StatsReport::Id { 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 @@ class CandidatePairId : public ComponentId { component), index_(index) {} - bool Equals(const Id& other) const override { + bool Equals(const IdBase& other) const override { return ComponentId::Equals(other) && static_cast(other).index_ == index_; } @@ -209,17 +220,160 @@ class CandidatePairId : public ComponentId { } // 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(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(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 @@ const char* StatsReport::Value::display_name() const { 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.Pass()), timestamp_(0.0) { - ASSERT(id_.get()); +StatsReport::StatsReport(const Id& id) : id_(id), timestamp_(0.0) { + DCHECK(id_.get()); } // static -scoped_ptr StatsReport::NewBandwidthEstimationId() { - return scoped_ptr(new BandwidthEstimationId()).Pass(); +StatsReport::Id StatsReport::NewBandwidthEstimationId() { + return Id(new RefCountedObject()); } // static -scoped_ptr StatsReport::NewTypedId( - StatsType type, const std::string& id) { - return scoped_ptr(new TypedId(type, id)).Pass(); +StatsReport::Id StatsReport::NewTypedId(StatsType type, const std::string& id) { + return Id(new RefCountedObject(type, id)); } // static -scoped_ptr StatsReport::NewTypedIntId(StatsType type, int id) { - return scoped_ptr(new TypedIntId(type, id)).Pass(); +StatsReport::Id StatsReport::NewTypedIntId(StatsType type, int id) { + return Id(new RefCountedObject(type, id)); } // static -scoped_ptr StatsReport::NewIdWithDirection( +StatsReport::Id StatsReport::NewIdWithDirection( StatsType type, const std::string& id, StatsReport::Direction direction) { - return scoped_ptr(new IdWithDirection(type, id, direction)).Pass(); + return Id(new RefCountedObject(type, id, direction)); } // static -scoped_ptr StatsReport::NewCandidateId( - bool local, const std::string& id) { - return scoped_ptr(new CandidateId(local, id)).Pass(); +StatsReport::Id StatsReport::NewCandidateId(bool local, const std::string& id) { + return Id(new RefCountedObject(local, id)); } // static -scoped_ptr StatsReport::NewComponentId( +StatsReport::Id StatsReport::NewComponentId( const std::string& content_name, int component) { - return scoped_ptr(new ComponentId(content_name, component)).Pass(); + return Id(new RefCountedObject(content_name, component)); } // static -scoped_ptr StatsReport::NewCandidatePairId( +StatsReport::Id StatsReport::NewCandidatePairId( const std::string& content_name, int component, int index) { - return scoped_ptr(new CandidatePairId(content_name, component, index)) - .Pass(); + return Id(new RefCountedObject( + content_name, component, index)); } const char* StatsReport::TypeToString() const { @@ -499,28 +668,47 @@ const char* StatsReport::TypeToString() const { 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(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(value)); + const Value* found = FindValue(name); + if (!found || !(*found == static_cast(value))) + values_[name] = ValuePtr(new Value(name, value, Value::kInt)); } void StatsReport::AddFloat(StatsReport::StatsValueName name, float value) { - AddString(name, rtc::ToString(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 @@ size_t StatsCollection::size() const { return list_.size(); } -StatsReport* StatsCollection::InsertNew(scoped_ptr 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 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 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& id) { - return Find(*id.get()); -} - } // namespace webrtc diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h index 22fc9d7348..de4fb5fba3 100644 --- a/talk/app/webrtc/statstypes.h +++ b/talk/app/webrtc/statstypes.h @@ -37,11 +37,11 @@ #include #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 @@ class StatsReport { 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& 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 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 ValuePtr; typedef std::map Values; // Ownership of |id| is passed to |this|. - explicit StatsReport(rtc::scoped_ptr id); + explicit StatsReport(const Id& id); // Factory functions for various types of stats IDs. - static rtc::scoped_ptr NewBandwidthEstimationId(); - static rtc::scoped_ptr NewTypedId(StatsType type, const std::string& id); - static rtc::scoped_ptr NewTypedIntId(StatsType type, int id); - static rtc::scoped_ptr 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 NewCandidateId(bool local, const std::string& id); - static rtc::scoped_ptr NewComponentId( + static Id NewCandidateId(bool local, const std::string& id); + static Id NewComponentId( const std::string& content_name, int component); - static rtc::scoped_ptr 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 @@ class StatsReport { 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 @@ class StatsReport { // 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_; + const Id id_; double timestamp_; // Time since 1970-01-01T00:00:00Z in milliseconds. Values values_; @@ -317,7 +389,6 @@ class StatsCollection { StatsCollection(); ~StatsCollection(); - // TODO(tommi): shared_ptr (or linked_ptr)? typedef std::list Container; typedef Container::iterator iterator; typedef Container::const_iterator const_iterator; @@ -328,15 +399,13 @@ class StatsCollection { // Creates a new report object with |id| that does not already // exist in the list of reports. - StatsReport* InsertNew(rtc::scoped_ptr id); - StatsReport* FindOrAddNew(rtc::scoped_ptr id); - StatsReport* ReplaceOrAddNew(rtc::scoped_ptr 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& id); private: Container list_;