diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 7f4aa4cc4..08ff72d7f 100644 --- a/talk/app/webrtc/java/jni/peerconnection_jni.cc +++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc @@ -1002,14 +1002,14 @@ 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_type = JavaStringFromStdString(jni, report->TypeToString()); - jobjectArray j_values = ValuesToJava(jni, report->values()); + jstring j_id = JavaStringFromStdString(jni, report->id); + jstring j_type = JavaStringFromStdString(jni, report->type); + jobjectArray j_values = ValuesToJava(jni, report->values); jobject j_report = jni->NewObject(*j_stats_report_class_, j_stats_report_ctor_, j_id, j_type, - report->timestamp(), + report->timestamp, j_values); jni->SetObjectArrayElement(reports_array, i++, j_report); } @@ -1021,11 +1021,11 @@ class StatsObserverWrapper : public StatsObserver { values.size(), *j_value_class_, NULL); for (int i = 0; i < values.size(); ++i) { ScopedLocalRefFrame local_ref_frame(jni); - const auto& value = values[i]; + const StatsReport::Value& value = values[i]; // Should we use the '.name' enum value here instead of converting the // name to a string? - jstring j_name = JavaStringFromStdString(jni, value->display_name()); - jstring j_value = JavaStringFromStdString(jni, value->value); + jstring j_name = JavaStringFromStdString(jni, value.display_name()); + jstring j_value = JavaStringFromStdString(jni, value.value); jobject j_element_value = jni->NewObject(*j_value_class_, j_value_ctor_, j_name, j_value); jni->SetObjectArrayElement(j_values, i, j_element_value); diff --git a/talk/app/webrtc/objc/RTCStatsReport.mm b/talk/app/webrtc/objc/RTCStatsReport.mm index bbf18539b..98cf6548e 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()); - _type = @(statsReport.TypeToString()); - _timestamp = statsReport.timestamp(); + _reportId = @(statsReport.id.c_str()); + _type = @(statsReport.type.c_str()); + _timestamp = statsReport.timestamp; NSMutableArray* values = - [NSMutableArray arrayWithCapacity:statsReport.values().size()]; - for (const auto& v : statsReport.values()) { - RTCPair* pair = [[RTCPair alloc] initWithKey:@(v->display_name()) - value:@(v->value.c_str())]; + [NSMutableArray arrayWithCapacity:statsReport.values.size()]; + webrtc::StatsReport::Values::const_iterator it = statsReport.values.begin(); + for (; it != statsReport.values.end(); ++it) { + RTCPair* pair = [[RTCPair alloc] initWithKey:@(it->display_name()) + value:@(it->value.c_str())]; [values addObject:pair]; } _values = values; diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index e2d444e5e..05695c04e 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -102,10 +102,10 @@ bool ExtractValueFromReport( const StatsReport& report, StatsReport::StatsValueName name, std::string* value) { - StatsReport::Values::const_iterator it = report.values().begin(); - for (; it != report.values().end(); ++it) { - if ((*it)->name == name) { - *value = (*it)->value; + StatsReport::Values::const_iterator it = report.values.begin(); + for (; it != report.values.end(); ++it) { + if (it->name == name) { + *value = it->value; return true; } } @@ -284,12 +284,13 @@ void ExtractStats(const cricket::BandwidthEstimationInfo& info, double stats_gathering_started, PeerConnectionInterface::StatsOutputLevel level, StatsReport* report) { + ASSERT(report->id == StatsReport::kStatsReportVideoBweId); 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); + if (report->timestamp != stats_gathering_started) { + report->values.clear(); + report->timestamp = stats_gathering_started; } report->AddValue(StatsReport::kStatsValueNameAvailableSendBandwidth, @@ -323,13 +324,13 @@ void ExtractStats(const cricket::BandwidthEstimationInfo& info, void ExtractRemoteStats(const cricket::MediaSenderInfo& info, StatsReport* report) { - report->set_timestamp(info.remote_stats[0].timestamp); + report->timestamp = info.remote_stats[0].timestamp; // TODO(hta): Extract some stats here. } void ExtractRemoteStats(const cricket::MediaReceiverInfo& info, StatsReport* report) { - report->set_timestamp(info.remote_stats[0].timestamp); + report->timestamp = info.remote_stats[0].timestamp; // TODO(hta): Extract some stats here. } @@ -553,8 +554,8 @@ StatsReport* StatsCollector::PrepareLocalReport( // Having the old values in the report will lead to multiple values with // the same name. // TODO(xians): Consider changing StatsReport to use map instead of vector. - report->ResetValues(); - report->set_timestamp(stats_gathering_started_); + report->values.clear(); + report->timestamp = stats_gathering_started_; report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); @@ -594,8 +595,8 @@ StatsReport* StatsCollector::PrepareRemoteReport( // Clear out stats from previous GatherStats calls if any. // The timestamp will be added later. Zero it for debugging. - report->ResetValues(); - report->set_timestamp(0); + report->values.clear(); + report->timestamp = 0; report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); @@ -636,14 +637,14 @@ std::string StatsCollector::AddOneCertificateReport( StatsReport* report = reports_.ReplaceOrAddNew( StatsId(StatsReport::kStatsReportTypeCertificate, fingerprint)); report->type = StatsReport::kStatsReportTypeCertificate; - report->set_timestamp(stats_gathering_started_); + report->timestamp = stats_gathering_started_; report->AddValue(StatsReport::kStatsValueNameFingerprint, fingerprint); report->AddValue(StatsReport::kStatsValueNameFingerprintAlgorithm, digest_algorithm); report->AddValue(StatsReport::kStatsValueNameDer, der_base64); if (!issuer_id.empty()) report->AddValue(StatsReport::kStatsValueNameIssuerId, issuer_id); - return report->id().ToString(); + return report->id; } std::string StatsCollector::AddCertificateReports( @@ -686,7 +687,7 @@ std::string StatsCollector::AddCandidateReport( report->AddValue(StatsReport::kStatsValueNameCandidateNetworkType, AdapterTypeToStatsType(candidate.network_type())); } - report->set_timestamp(stats_gathering_started_); + report->timestamp = stats_gathering_started_; report->AddValue(StatsReport::kStatsValueNameCandidateIPAddress, candidate.address().ipaddr().ToString()); report->AddValue(StatsReport::kStatsValueNameCandidatePortNumber, @@ -708,8 +709,8 @@ void StatsCollector::ExtractSessionInfo() { StatsReport* report = reports_.ReplaceOrAddNew( StatsId(StatsReport::kStatsReportTypeSession, session_->id())); report->type = StatsReport::kStatsReportTypeSession; - report->set_timestamp(stats_gathering_started_); - report->ResetValues(); + report->timestamp = stats_gathering_started_; + report->values.clear(); report->AddBoolean(StatsReport::kStatsValueNameInitiator, session_->initiator()); @@ -754,7 +755,7 @@ void StatsCollector::ExtractSessionInfo() { << "-" << channel_iter->component; StatsReport* channel_report = reports_.ReplaceOrAddNew(ostc.str()); channel_report->type = StatsReport::kStatsReportTypeComponent; - channel_report->set_timestamp(stats_gathering_started_); + channel_report->timestamp = stats_gathering_started_; channel_report->AddValue(StatsReport::kStatsValueNameComponent, channel_iter->component); if (!local_cert_report_id.empty()) { @@ -775,10 +776,10 @@ void StatsCollector::ExtractSessionInfo() { << channel_iter->component << "-" << i; StatsReport* report = reports_.ReplaceOrAddNew(ost.str()); report->type = StatsReport::kStatsReportTypeCandidatePair; - report->set_timestamp(stats_gathering_started_); + report->timestamp = stats_gathering_started_; // Link from connection to its containing channel. report->AddValue(StatsReport::kStatsValueNameChannelId, - channel_report->id().ToString()); + channel_report->id); const cricket::ConnectionInfo& info = channel_iter->connection_infos[i]; @@ -918,6 +919,7 @@ StatsReport* StatsCollector::GetOrCreateReport(const std::string& type, if (report == NULL) { std::string statsid = StatsId(type, id, direction); report = reports_.FindOrAddNew(statsid); + ASSERT(report->id == statsid); report->type = type; } diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index e89b518f4..cba400390 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -155,9 +155,10 @@ class FakeAudioTrack bool GetValue(const StatsReport* report, StatsReport::StatsValueName name, std::string* value) { - for (const auto& v : report->values()) { - if (v->name == name) { - *value = v->value; + StatsReport::Values::const_iterator it = report->values.begin(); + for (; it != report->values.end(); ++it) { + if (it->name == name) { + *value = it->value; return true; } } @@ -198,12 +199,12 @@ const StatsReport* FindNthReportByType( const StatsReport* FindReportById(const StatsReports& reports, const std::string& id) { - for (const auto* r : reports) { - if (r->id().ToString() == id) { - return r; + for (size_t i = 0; i < reports.size(); ++i) { + if (reports[i]->id == id) { + return reports[i]; } } - return nullptr; + return NULL; } std::string ExtractSsrcStatsValue(StatsReports reports, @@ -1028,7 +1029,7 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { const StatsReport* remote_report = FindNthReportByType(reports, StatsReport::kStatsReportTypeRemoteSsrc, 1); EXPECT_FALSE(remote_report == NULL); - EXPECT_NE(0, remote_report->timestamp()); + EXPECT_NE(0, remote_report->timestamp); } // This test verifies that the empty track report exists in the returned stats diff --git a/talk/app/webrtc/statstypes.cc b/talk/app/webrtc/statstypes.cc index dc943f074..b15dba5d2 100644 --- a/talk/app/webrtc/statstypes.cc +++ b/talk/app/webrtc/statstypes.cc @@ -27,8 +27,6 @@ #include "talk/app/webrtc/statstypes.h" -using rtc::scoped_ptr; - namespace webrtc { const char StatsReport::kStatsReportTypeSession[] = "googLibjingleSession"; @@ -48,50 +46,37 @@ const char StatsReport::kStatsReportTypeDataChannel[] = "datachannel"; const char StatsReport::kStatsReportVideoBweId[] = "bweforvideo"; StatsReport::StatsReport(const StatsReport& src) - : id_(src.id_), + : id(src.id), type(src.type), - timestamp_(src.timestamp_), - values_(src.values_) { + timestamp(src.timestamp), + values(src.values) { } StatsReport::StatsReport(const std::string& id) - : id_(id), timestamp_(0) { -} - -StatsReport::StatsReport(scoped_ptr id) - : id_(id->ToString()), timestamp_(0) { -} - -// static -scoped_ptr StatsReport::NewTypedId( - StatsReport::StatsType type, const std::string& id) { - std::string internal_id(type); - internal_id += '_'; - internal_id += id; - return scoped_ptr(new Id(internal_id)).Pass(); + : id(id), timestamp(0) { } StatsReport& StatsReport::operator=(const StatsReport& src) { - ASSERT(id_ == src.id_); + ASSERT(id == src.id); type = src.type; - timestamp_ = src.timestamp_; - values_ = src.values_; + timestamp = src.timestamp; + values = src.values; return *this; } // Operators provided for STL container/algorithm support. bool StatsReport::operator<(const StatsReport& other) const { - return id_ < other.id_; + return id < other.id; } bool StatsReport::operator==(const StatsReport& other) const { - return id_ == other.id_; + return id == other.id; } // Special support for being able to use std::find on a container // without requiring a new StatsReport instance. bool StatsReport::operator==(const std::string& other_id) const { - return id_ == other_id; + return id == other_id; } // The copy ctor can't be declared as explicit due to problems with STL. @@ -333,7 +318,7 @@ const char* StatsReport::Value::display_name() const { void StatsReport::AddValue(StatsReport::StatsValueName name, const std::string& value) { - values_.push_back(ValuePtr(new Value(name, value))); + values.push_back(Value(name, value)); } void StatsReport::AddValue(StatsReport::StatsValueName name, int64 value) { @@ -375,21 +360,15 @@ void StatsReport::AddBoolean(StatsReport::StatsValueName name, bool value) { void StatsReport::ReplaceValue(StatsReport::StatsValueName name, const std::string& value) { - Values::iterator it = std::find_if(values_.begin(), values_.end(), - [&name](const ValuePtr& v)->bool { return v->name == name; }); - // Values are const once added since they may be used outside of the stats - // collection. So we remove it from values_ when replacing and add a new one. - if (it != values_.end()) { - if ((*it)->value == value) + for (Values::iterator it = values.begin(); it != values.end(); ++it) { + if ((*it).name == name) { + it->value = value; return; - values_.erase(it); + } } - - AddValue(name, value); -} - -void StatsReport::ResetValues() { - values_.clear(); + // It is not reachable here, add an ASSERT to make sure the overwriting is + // always a success. + ASSERT(false); } StatsSet::StatsSet() { diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h index d65d30d29..15a12d892 100644 --- a/talk/app/webrtc/statstypes.h +++ b/talk/app/webrtc/statstypes.h @@ -38,8 +38,6 @@ #include "webrtc/base/basictypes.h" #include "webrtc/base/common.h" -#include "webrtc/base/linked_ptr.h" -#include "webrtc/base/scoped_ptr.h" #include "webrtc/base/stringencode.h" namespace webrtc { @@ -48,7 +46,7 @@ class StatsReport { public: // TODO(tommi): Remove this ctor after removing reliance upon it in Chromium // (mock_peer_connection_impl.cc). - StatsReport() : timestamp_(0) {} + StatsReport() : timestamp(0) {} // TODO(tommi): Make protected and disallow copy completely once not needed. StatsReport(const StatsReport& src); @@ -71,7 +69,7 @@ class StatsReport { // This is used as a key for this report in ordered containers, // so it must never be changed. // TODO(tommi): Make this member variable const. - std::string id_; // See below for contents. + std::string id; // See below for contents. std::string type; // See below for contents. // StatsValue names. @@ -181,15 +179,6 @@ class StatsReport { kStatsValueNameWritable, }; - class Id { - public: - Id(const std::string& id) : id_(id) {} - Id(const Id& id) : id_(id.id_) {} - const std::string& ToString() const { return id_; } - private: - const std::string id_; - }; - struct Value { // The copy ctor can't be declared as explicit due to problems with STL. Value(const Value& other); @@ -209,15 +198,6 @@ class StatsReport { std::string value; }; - typedef rtc::linked_ptr ValuePtr; - typedef std::vector Values; - typedef const char* StatsType; - - // Ownership of |id| is passed to |this|. - explicit StatsReport(rtc::scoped_ptr id); - - static rtc::scoped_ptr NewTypedId(StatsType type, const std::string& id); - void AddValue(StatsValueName name, const std::string& value); void AddValue(StatsValueName name, int64 value); template @@ -226,17 +206,9 @@ class StatsReport { void ReplaceValue(StatsValueName name, const std::string& value); - void ResetValues(); - - const Id id() const { return Id(id_); } - double timestamp() const { return timestamp_; } - void set_timestamp(double t) { timestamp_ = t; } - const Values& values() const { return values_; } - - const char* TypeToString() const { return type.c_str(); } - - double timestamp_; // Time since 1970-01-01T00:00:00Z in milliseconds. - Values values_; + double timestamp; // Time since 1970-01-01T00:00:00Z in milliseconds. + typedef std::vector Values; + Values values; // TODO(tommi): These should all be enum values. diff --git a/talk/app/webrtc/test/mockpeerconnectionobservers.h b/talk/app/webrtc/test/mockpeerconnectionobservers.h index 5dbf92b57..56ca397f9 100644 --- a/talk/app/webrtc/test/mockpeerconnectionobservers.h +++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h @@ -174,9 +174,9 @@ class MockStatsObserver : public webrtc::StatsObserver { bool GetIntValue(const StatsReport* report, StatsReport::StatsValueName name, int* value) { - for (const auto& v : report->values()) { - if (v->name == name) { - *value = rtc::FromString(v->value); + for (const auto& v : report->values) { + if (v.name == name) { + *value = rtc::FromString(v.value); return true; } }