Revert 8095 "Update StatsCollector's interface in preparation of..."
> Update StatsCollector's interface in preparation of more changes.
>
> This CL is the first of three and this one contains interface additions (not deletion for backwards compatibility) as well as a few necessary updates to internal code.
>
> The next CL will be in Chromium to consume the new new methods and remove dependency on the old ones.
>
> The third CL will then contain the bulk of the updates and improvements and be compatible with this interface.
>
> BUG=2822
> R=perkj@webrtc.org
>
> Review URL: https://webrtc-codereview.appspot.com/36829004
TBR=tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/37669004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8096 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/java/jni/peerconnection_jni.cc b/talk/app/webrtc/java/jni/peerconnection_jni.cc
index 7f4aa4c..08ff72d 100644
--- a/talk/app/webrtc/java/jni/peerconnection_jni.cc
+++ b/talk/app/webrtc/java/jni/peerconnection_jni.cc
@@ -1002,14 +1002,14 @@
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 @@
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 bbf1853..98cf654 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 e2d444e..05695c0 100644
--- a/talk/app/webrtc/statscollector.cc
+++ b/talk/app/webrtc/statscollector.cc
@@ -102,10 +102,10 @@
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 @@
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 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 @@
// 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 @@
// 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 @@
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 @@
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 @@
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 @@
<< "-" << 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 @@
<< 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 @@
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 e89b518..cba4003 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -155,9 +155,10 @@
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* 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 @@
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 dc943f0..b15dba5 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::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<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();
+ : 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 @@
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::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 d65d30d..15a12d8 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 @@
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 @@
// 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 @@
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 @@
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>
@@ -226,17 +206,9 @@
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<Value> 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 5dbf92b..56ca397 100644
--- a/talk/app/webrtc/test/mockpeerconnectionobservers.h
+++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h
@@ -174,9 +174,9 @@
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;
}
}
