diff options
| -rw-r--r-- | talk/app/webrtc/java/jni/peerconnection_jni.cc | 14 | ||||
| -rw-r--r-- | talk/app/webrtc/objc/RTCStatsReport.mm | 15 | ||||
| -rw-r--r-- | talk/app/webrtc/statscollector.cc | 44 | ||||
| -rw-r--r-- | talk/app/webrtc/statscollector_unittest.cc | 17 | ||||
| -rw-r--r-- | talk/app/webrtc/statstypes.cc | 57 | ||||
| -rw-r--r-- | talk/app/webrtc/statstypes.h | 38 | ||||
| -rw-r--r-- | talk/app/webrtc/test/mockpeerconnectionobservers.h | 6 |
7 files changed, 118 insertions, 73 deletions
diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc index 08ff72d7fd..7f4aa4cc41 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); - jstring j_type = JavaStringFromStdString(jni, report->type); - jobjectArray j_values = ValuesToJava(jni, report->values); + 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_, 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 StatsReport::Value& value = values[i]; + const auto& 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 98cf6548e3..bbf18539b9 100644 --- a/talk/app/webrtc/objc/RTCStatsReport.mm +++ b/talk/app/webrtc/objc/RTCStatsReport.mm @@ -50,15 +50,14 @@ - (instancetype)initWithStatsReport:(const webrtc::StatsReport&)statsReport { if (self = [super init]) { - _reportId = @(statsReport.id.c_str()); - _type = @(statsReport.type.c_str()); - _timestamp = statsReport.timestamp; + _reportId = @(statsReport.id().ToString().c_str()); + _type = @(statsReport.TypeToString()); + _timestamp = statsReport.timestamp(); NSMutableArray* values = - [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())]; + [NSMutableArray arrayWithCapacity:statsReport.values().size()]; + for (const auto& v : statsReport.values()) { + RTCPair* pair = [[RTCPair alloc] initWithKey:@(v->display_name()) + value:@(v->value.c_str())]; [values addObject:pair]; } _values = values; diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index 05695c04eb..e2d444e5e1 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,13 +284,12 @@ 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->values.clear(); - report->timestamp = stats_gathering_started; + if (report->timestamp() != stats_gathering_started) { + report->ResetValues(); + report->set_timestamp(stats_gathering_started); } report->AddValue(StatsReport::kStatsValueNameAvailableSendBandwidth, @@ -324,13 +323,13 @@ void ExtractStats(const cricket::BandwidthEstimationInfo& info, void ExtractRemoteStats(const cricket::MediaSenderInfo& info, StatsReport* report) { - report->timestamp = info.remote_stats[0].timestamp; + report->set_timestamp(info.remote_stats[0].timestamp); // TODO(hta): Extract some stats here. } void ExtractRemoteStats(const cricket::MediaReceiverInfo& info, StatsReport* report) { - report->timestamp = info.remote_stats[0].timestamp; + report->set_timestamp(info.remote_stats[0].timestamp); // TODO(hta): Extract some stats here. } @@ -554,8 +553,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->values.clear(); - report->timestamp = stats_gathering_started_; + report->ResetValues(); + report->set_timestamp(stats_gathering_started_); report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); @@ -595,8 +594,8 @@ StatsReport* StatsCollector::PrepareRemoteReport( // Clear out stats from previous GatherStats calls if any. // The timestamp will be added later. Zero it for debugging. - report->values.clear(); - report->timestamp = 0; + report->ResetValues(); + report->set_timestamp(0); report->AddValue(StatsReport::kStatsValueNameSsrc, ssrc_id); report->AddValue(StatsReport::kStatsValueNameTrackId, track_id); @@ -637,14 +636,14 @@ std::string StatsCollector::AddOneCertificateReport( StatsReport* report = reports_.ReplaceOrAddNew( StatsId(StatsReport::kStatsReportTypeCertificate, fingerprint)); report->type = StatsReport::kStatsReportTypeCertificate; - report->timestamp = stats_gathering_started_; + report->set_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; + return report->id().ToString(); } std::string StatsCollector::AddCertificateReports( @@ -687,7 +686,7 @@ std::string StatsCollector::AddCandidateReport( report->AddValue(StatsReport::kStatsValueNameCandidateNetworkType, AdapterTypeToStatsType(candidate.network_type())); } - report->timestamp = stats_gathering_started_; + report->set_timestamp(stats_gathering_started_); report->AddValue(StatsReport::kStatsValueNameCandidateIPAddress, candidate.address().ipaddr().ToString()); report->AddValue(StatsReport::kStatsValueNameCandidatePortNumber, @@ -709,8 +708,8 @@ void StatsCollector::ExtractSessionInfo() { StatsReport* report = reports_.ReplaceOrAddNew( StatsId(StatsReport::kStatsReportTypeSession, session_->id())); report->type = StatsReport::kStatsReportTypeSession; - report->timestamp = stats_gathering_started_; - report->values.clear(); + report->set_timestamp(stats_gathering_started_); + report->ResetValues(); report->AddBoolean(StatsReport::kStatsValueNameInitiator, session_->initiator()); @@ -755,7 +754,7 @@ void StatsCollector::ExtractSessionInfo() { << "-" << channel_iter->component; StatsReport* channel_report = reports_.ReplaceOrAddNew(ostc.str()); channel_report->type = StatsReport::kStatsReportTypeComponent; - channel_report->timestamp = stats_gathering_started_; + channel_report->set_timestamp(stats_gathering_started_); channel_report->AddValue(StatsReport::kStatsValueNameComponent, channel_iter->component); if (!local_cert_report_id.empty()) { @@ -776,10 +775,10 @@ void StatsCollector::ExtractSessionInfo() { << channel_iter->component << "-" << i; StatsReport* report = reports_.ReplaceOrAddNew(ost.str()); report->type = StatsReport::kStatsReportTypeCandidatePair; - report->timestamp = stats_gathering_started_; + report->set_timestamp(stats_gathering_started_); // Link from connection to its containing channel. report->AddValue(StatsReport::kStatsValueNameChannelId, - channel_report->id); + channel_report->id().ToString()); const cricket::ConnectionInfo& info = channel_iter->connection_infos[i]; @@ -919,7 +918,6 @@ 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 cba4003906..e89b518f44 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -155,10 +155,9 @@ class FakeAudioTrack bool GetValue(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; + for (const auto& v : report->values()) { + if (v->name == name) { + *value = v->value; return true; } } @@ -199,12 +198,12 @@ const StatsReport* FindNthReportByType( const StatsReport* FindReportById(const StatsReports& reports, const std::string& id) { - for (size_t i = 0; i < reports.size(); ++i) { - if (reports[i]->id == id) { - return reports[i]; + for (const auto* r : reports) { + if (r->id().ToString() == id) { + return r; } } - return NULL; + return nullptr; } std::string ExtractSsrcStatsValue(StatsReports reports, @@ -1029,7 +1028,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 b15dba5d2d..dc943f0740 100644 --- a/talk/app/webrtc/statstypes.cc +++ b/talk/app/webrtc/statstypes.cc @@ -27,6 +27,8 @@ #include "talk/app/webrtc/statstypes.h" +using rtc::scoped_ptr; + namespace webrtc { const char StatsReport::kStatsReportTypeSession[] = "googLibjingleSession"; @@ -46,37 +48,50 @@ 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) { + : id_(id), timestamp_(0) { +} + +StatsReport::StatsReport(scoped_ptr<StatsReport::Id> id) + : id_(id->ToString()), timestamp_(0) { +} + +// static +scoped_ptr<StatsReport::Id> StatsReport::NewTypedId( + StatsReport::StatsType type, const std::string& id) { + std::string internal_id(type); + internal_id += '_'; + internal_id += id; + return scoped_ptr<Id>(new Id(internal_id)).Pass(); } 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. @@ -318,7 +333,7 @@ const char* StatsReport::Value::display_name() const { void StatsReport::AddValue(StatsReport::StatsValueName name, const std::string& value) { - values.push_back(Value(name, value)); + values_.push_back(ValuePtr(new Value(name, value))); } void StatsReport::AddValue(StatsReport::StatsValueName name, int64 value) { @@ -360,15 +375,21 @@ void StatsReport::AddBoolean(StatsReport::StatsValueName name, bool value) { void StatsReport::ReplaceValue(StatsReport::StatsValueName name, const std::string& value) { - for (Values::iterator it = values.begin(); it != values.end(); ++it) { - if ((*it).name == name) { - it->value = 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) return; - } + values_.erase(it); } - // It is not reachable here, add an ASSERT to make sure the overwriting is - // always a success. - ASSERT(false); + + AddValue(name, value); +} + +void StatsReport::ResetValues() { + values_.clear(); } StatsSet::StatsSet() { diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h index 15a12d892f..d65d30d291 100644 --- a/talk/app/webrtc/statstypes.h +++ b/talk/app/webrtc/statstypes.h @@ -38,6 +38,8 @@ #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 { @@ -46,7 +48,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); @@ -69,7 +71,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. @@ -179,6 +181,15 @@ 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); @@ -198,6 +209,15 @@ class StatsReport { std::string value; }; + typedef rtc::linked_ptr<Value> ValuePtr; + typedef std::vector<ValuePtr> Values; + typedef const char* StatsType; + + // Ownership of |id| is passed to |this|. + explicit StatsReport(rtc::scoped_ptr<Id> id); + + static rtc::scoped_ptr<Id> NewTypedId(StatsType type, const std::string& id); + void AddValue(StatsValueName name, const std::string& value); void AddValue(StatsValueName name, int64 value); template <typename T> @@ -206,9 +226,17 @@ class StatsReport { void ReplaceValue(StatsValueName name, const std::string& value); - double timestamp; // Time since 1970-01-01T00:00:00Z in milliseconds. - typedef std::vector<Value> Values; - Values values; + 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_; // 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 56ca397f9d..5dbf92b57e 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<int>(v.value); + for (const auto& v : report->values()) { + if (v->name == name) { + *value = rtc::FromString<int>(v->value); return true; } } |