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

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_id = JavaStringFromStdString(jni, report->id);
-      jstring j_type = JavaStringFromStdString(jni, report->TypeToString());
+      jstring j_type = JavaStringFromStdString(jni, report->type);
-      jobjectArray j_values = ValuesToJava(jni, report->values());
+      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_name = JavaStringFromStdString(jni, value.display_name());
-      jstring j_value = JavaStringFromStdString(jni, value->value);
+      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);

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.c_str());
-    _type = @(statsReport.TypeToString());
+    _type = @(statsReport.type.c_str());
-    _timestamp = statsReport.timestamp();
+    _timestamp = statsReport.timestamp;
     NSMutableArray* values =
-        [NSMutableArray arrayWithCapacity:statsReport.values().size()];
+        [NSMutableArray arrayWithCapacity:statsReport.values.size()];
-    for (const auto& v : statsReport.values()) {
+    webrtc::StatsReport::Values::const_iterator it = statsReport.values.begin();
-      RTCPair* pair = [[RTCPair alloc] initWithKey:@(v->display_name())
+    for (; it != statsReport.values.end(); ++it) {
-                                             value:@(v->value.c_str())];
+      RTCPair* pair = [[RTCPair alloc] initWithKey:@(it->display_name())
+                                             value:@(it->value.c_str())];
       [values addObject:pair];
     }
     _values = values;

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();
+  StatsReport::Values::const_iterator it = report.values.begin();
-  for (; it != report.values().end(); ++it) {
+  for (; it != report.values.end(); ++it) {
-    if ((*it)->name == name) {
+    if (it->name == name) {
-      *value = (*it)->value;
+      *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) {
+  if (report->timestamp != stats_gathering_started) {
-    report->ResetValues();
+    report->values.clear();
-    report->set_timestamp(stats_gathering_started);
+    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->values.clear();
-  report->set_timestamp(stats_gathering_started_);
+  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->values.clear();
-  report->set_timestamp(0);
+  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->timestamp = stats_gathering_started_;
-  report->ResetValues();
+  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;
   }
 

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()) {
+  StatsReport::Values::const_iterator it = report->values.begin();
-    if (v->name == name) {
+  for (; it != report->values.end(); ++it) {
-      *value = v->value;
+    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) {
+  for (size_t i = 0; i < reports.size(); ++i) {
-    if (r->id().ToString() == id) {
+    if (reports[i]->id == id) {
-      return r;
+      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

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_),
+    timestamp(src.timestamp),
-    values_(src.values_) {
+    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_;
+  timestamp = src.timestamp;
-  values_ = src.values_;
+  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(),
+  for (Values::iterator it = values.begin(); it != values.end(); ++it) {
-      [&name](const ValuePtr& v)->bool { return v->name == name; });
+    if ((*it).name == name) {
-  // Values are const once added since they may be used outside of the stats
+      it->value = value;
-  // 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
-  AddValue(name, value);
+  // always a success.
-}
+  ASSERT(false);
-
-void StatsReport::ResetValues() {
-  values_.clear();
 }
 
 StatsSet::StatsSet() {

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<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 @@ class StatsReport {
 
   void ReplaceValue(StatsValueName name, const std::string& value);
 
-  void ResetValues();
+  double timestamp;  // Time since 1970-01-01T00:00:00Z in milliseconds.
-
+  typedef std::vector<Value> Values;
-  const Id id() const { return Id(id_); }
+  Values values;
-  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.
 

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()) {
+    for (const auto& v : report->values) {
-      if (v->name == name) {
+      if (v.name == name) {
-        *value = rtc::FromString<int>(v->value);
+        *value = rtc::FromString<int>(v.value);
         return true;
       }
     }