Refactor StatsCollector and associated types.
* Due to the type changes, I'm going to update the OnCompleted event in two phases to sync with Chrome.  This is the first phase.
* Reports are now managed in a set, not a map, since it's enough to store the id in one place.
* Report ids are now const.
* Copying of data has been greatly reduced.
* This change includes preparation work for making GetStats fully async.

R=xians@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/18819004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6745 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/talk/app/webrtc/peerconnection.cc b/talk/app/webrtc/peerconnection.cc
index ec20593..bb5f0d4 100644
--- a/talk/app/webrtc/peerconnection.cc
+++ b/talk/app/webrtc/peerconnection.cc
@@ -85,11 +85,12 @@
 };
 
 struct GetStatsMsg : public talk_base::MessageData {
-  explicit GetStatsMsg(webrtc::StatsObserver* observer)
-      : observer(observer) {
+  GetStatsMsg(webrtc::StatsObserver* observer,
+              webrtc::MediaStreamTrackInterface* track)
+      : observer(observer), track(track) {
   }
-  webrtc::StatsReports reports;
   talk_base::scoped_refptr<webrtc::StatsObserver> observer;
+  talk_base::scoped_refptr<webrtc::MediaStreamTrackInterface> track;
 };
 
 // |in_str| should be of format
@@ -446,17 +447,15 @@
 bool PeerConnection::GetStats(StatsObserver* observer,
                               MediaStreamTrackInterface* track,
                               StatsOutputLevel level) {
+  ASSERT(signaling_thread()->IsCurrent());
   if (!VERIFY(observer != NULL)) {
     LOG(LS_ERROR) << "GetStats - observer is NULL.";
     return false;
   }
 
   stats_->UpdateStats(level);
-  talk_base::scoped_ptr<GetStatsMsg> msg(new GetStatsMsg(observer));
-  if (!stats_->GetStats(track, &(msg->reports))) {
-    return false;
-  }
-  signaling_thread()->Post(this, MSG_GETSTATS, msg.release());
+  signaling_thread()->Post(this, MSG_GETSTATS,
+                           new GetStatsMsg(observer, track));
   return true;
 }
 
@@ -702,7 +701,9 @@
     }
     case MSG_GETSTATS: {
       GetStatsMsg* param = static_cast<GetStatsMsg*>(msg->pdata);
-      param->observer->OnComplete(param->reports);
+      StatsReports reports;
+      stats_->GetStats(param->track, &reports);
+      param->observer->OnComplete(reports);
       delete param;
       break;
     }
diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h
index ed4033c..171c10b 100644
--- a/talk/app/webrtc/peerconnectioninterface.h
+++ b/talk/app/webrtc/peerconnectioninterface.h
@@ -113,7 +113,18 @@
 
 class StatsObserver : public talk_base::RefCountInterface {
  public:
-  virtual void OnComplete(const std::vector<StatsReport>& reports) = 0;
+  // TODO(tommi): Remove.
+  virtual void OnComplete(const std::vector<StatsReport>& reports) {}
+
+  // TODO(tommi): Make pure virtual and remove implementation.
+  virtual void OnComplete(const StatsReports& reports) {
+    std::vector<StatsReportCopyable> report_copies;
+    for (size_t i = 0; i < reports.size(); ++i)
+      report_copies.push_back(StatsReportCopyable(*reports[i]));
+    std::vector<StatsReport>* r =
+        reinterpret_cast<std::vector<StatsReport>*>(&report_copies);
+     OnComplete(*r);
+   }
 
  protected:
   virtual ~StatsObserver() {}
diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc
index b165841..751e1fc 100644
--- a/talk/app/webrtc/statscollector.cc
+++ b/talk/app/webrtc/statscollector.cc
@@ -234,7 +234,6 @@
 }
 
 namespace {
-typedef std::map<std::string, StatsReport> StatsMap;
 
 double GetTimeNow() {
   return talk_base::Timing::WallTimeNow() * talk_base::kNumMillisecsPerSec;
@@ -294,17 +293,16 @@
   return false;
 }
 
-void AddTrackReport(StatsMap* reports, const std::string& track_id) {
+void AddTrackReport(StatsSet* reports, const std::string& track_id) {
   // Adds an empty track report.
-  StatsReport report;
-  report.type = StatsReport::kStatsReportTypeTrack;
-  report.id = StatsId(StatsReport::kStatsReportTypeTrack, track_id);
-  report.AddValue(StatsReport::kStatsValueNameTrackId, track_id);
-  (*reports)[report.id] = report;
+  StatsReport* report = reports->FindOrAddNew(
+      StatsId(StatsReport::kStatsReportTypeTrack, track_id));
+  report->type = StatsReport::kStatsReportTypeTrack;
+  report->AddValue(StatsReport::kStatsValueNameTrackId, track_id);
 }
 
 template <class TrackVector>
-void CreateTrackReports(const TrackVector& tracks, StatsMap* reports) {
+void CreateTrackReports(const TrackVector& tracks, StatsSet* reports) {
   for (size_t j = 0; j < tracks.size(); ++j) {
     webrtc::MediaStreamTrackInterface* track = tracks[j];
     AddTrackReport(reports, track->id());
@@ -469,7 +467,7 @@
                   double stats_gathering_started,
                   PeerConnectionInterface::StatsOutputLevel level,
                   StatsReport* report) {
-  report->id = StatsReport::kStatsReportVideoBweId;
+  ASSERT(report->id == StatsReport::kStatsReportVideoBweId);
   report->type = StatsReport::kStatsReportTypeBwe;
 
   // Clear out stats from previous GatherStats calls if any.
@@ -583,9 +581,9 @@
 
   // Create the kStatsReportTypeTrack report for the new track if there is no
   // report yet.
-  StatsMap::iterator it = reports_.find(
+  StatsReport* found = reports_.Find(
       StatsId(StatsReport::kStatsReportTypeTrack, audio_track->id()));
-  if (it == reports_.end())
+  if (!found)
     AddTrackReport(&reports_, audio_track->id());
 }
 
@@ -603,49 +601,46 @@
   ASSERT(false);
 }
 
-bool StatsCollector::GetStats(MediaStreamTrackInterface* track,
+void StatsCollector::GetStats(MediaStreamTrackInterface* track,
                               StatsReports* reports) {
   ASSERT(reports != NULL);
-  reports->clear();
+  ASSERT(reports->empty());
 
-  StatsMap::iterator it;
   if (!track) {
-    for (it = reports_.begin(); it != reports_.end(); ++it) {
-      reports->push_back(it->second);
-    }
-    return true;
+    StatsSet::const_iterator it;
+    for (it = reports_.begin(); it != reports_.end(); ++it)
+      reports->push_back(&(*it));
+    return;
   }
 
-  it = reports_.find(StatsId(StatsReport::kStatsReportTypeSession,
-                             session_->id()));
-  if (it != reports_.end()) {
-    reports->push_back(it->second);
-  }
+  StatsReport* report =
+      reports_.Find(StatsId(StatsReport::kStatsReportTypeSession,
+                            session_->id()));
+  if (report)
+    reports->push_back(report);
 
-  it = reports_.find(StatsId(StatsReport::kStatsReportTypeTrack, track->id()));
+  report = reports_.Find(
+      StatsId(StatsReport::kStatsReportTypeTrack, track->id()));
 
-  if (it == reports_.end()) {
-    LOG(LS_WARNING) << "No StatsReport is available for "<< track->id();
-    return false;
-  }
+  if (!report)
+    return;
 
-  reports->push_back(it->second);
+  reports->push_back(report);
 
   std::string track_id;
-  for (it = reports_.begin(); it != reports_.end(); ++it) {
-    if (it->second.type != StatsReport::kStatsReportTypeSsrc) {
+  for (StatsSet::const_iterator it = reports_.begin(); it != reports_.end();
+       ++it) {
+    if (it->type != StatsReport::kStatsReportTypeSsrc)
       continue;
-    }
-    if (ExtractValueFromReport(it->second,
+
+    if (ExtractValueFromReport(*it,
                                StatsReport::kStatsValueNameTrackId,
                                &track_id)) {
       if (track_id == track->id()) {
-        reports->push_back(it->second);
+        reports->push_back(&(*it));
       }
     }
   }
-
-  return true;
 }
 
 void
@@ -672,13 +667,13 @@
     const std::string& transport_id,
     TrackDirection direction) {
   const std::string ssrc_id = talk_base::ToString<uint32>(ssrc);
-  StatsMap::iterator it = reports_.find(StatsId(
-      StatsReport::kStatsReportTypeSsrc, ssrc_id, direction));
+  StatsReport* report = reports_.Find(
+      StatsId(StatsReport::kStatsReportTypeSsrc, ssrc_id, direction));
 
   // Use the ID of the track that is currently mapped to the SSRC, if any.
   std::string track_id;
   if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) {
-    if (it == reports_.end()) {
+    if (!report) {
       // The ssrc is not used by any track or existing report, return NULL
       // in such case to indicate no report is prepared for the ssrc.
       return NULL;
@@ -686,13 +681,13 @@
 
     // The ssrc is not used by any existing track. Keeps the old track id
     // since we want to report the stats for inactive ssrc.
-    ExtractValueFromReport(it->second,
+    ExtractValueFromReport(*report,
                            StatsReport::kStatsValueNameTrackId,
                            &track_id);
   }
 
-  StatsReport* report = GetOrCreateReport(StatsReport::kStatsReportTypeSsrc,
-                                          ssrc_id, direction);
+  report = GetOrCreateReport(
+      StatsReport::kStatsReportTypeSsrc, ssrc_id, direction);
 
   // Clear out stats from previous GatherStats calls if any.
   // This is required since the report will be returned for the new values.
@@ -715,13 +710,13 @@
     const std::string& transport_id,
     TrackDirection direction) {
   const std::string ssrc_id = talk_base::ToString<uint32>(ssrc);
-  StatsMap::iterator it = reports_.find(StatsId(
-      StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction));
+  StatsReport* report = reports_.Find(
+      StatsId(StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction));
 
   // Use the ID of the track that is currently mapped to the SSRC, if any.
   std::string track_id;
   if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) {
-    if (it == reports_.end()) {
+    if (!report) {
       // The ssrc is not used by any track or existing report, return NULL
       // in such case to indicate no report is prepared for the ssrc.
       return NULL;
@@ -729,12 +724,12 @@
 
     // The ssrc is not used by any existing track. Keeps the old track id
     // since we want to report the stats for inactive ssrc.
-    ExtractValueFromReport(it->second,
+    ExtractValueFromReport(*report,
                            StatsReport::kStatsValueNameTrackId,
                            &track_id);
   }
 
-  StatsReport* report = GetOrCreateReport(
+  report = GetOrCreateReport(
       StatsReport::kStatsReportTypeRemoteSsrc, ssrc_id, direction);
 
   // Clear out stats from previous GatherStats calls if any.
@@ -778,18 +773,17 @@
   talk_base::Base64::EncodeFromArray(
       der_buffer.data(), der_buffer.length(), &der_base64);
 
-  StatsReport report;
-  report.type = StatsReport::kStatsReportTypeCertificate;
-  report.id = StatsId(report.type, fingerprint);
-  report.timestamp = stats_gathering_started_;
-  report.AddValue(StatsReport::kStatsValueNameFingerprint, fingerprint);
-  report.AddValue(StatsReport::kStatsValueNameFingerprintAlgorithm,
-                  digest_algorithm);
-  report.AddValue(StatsReport::kStatsValueNameDer, der_base64);
+  StatsReport* report = reports_.FindOrAddNew(
+      StatsId(StatsReport::kStatsReportTypeCertificate, fingerprint));
+  report->type = StatsReport::kStatsReportTypeCertificate;
+  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);
-  reports_[report.id] = report;
-  return report.id;
+    report->AddValue(StatsReport::kStatsValueNameIssuerId, issuer_id);
+  return report->id;
 }
 
 std::string StatsCollector::AddCertificateReports(
@@ -819,15 +813,13 @@
 
 void StatsCollector::ExtractSessionInfo() {
   // Extract information from the base session.
-  StatsReport report;
-  report.id = StatsId(StatsReport::kStatsReportTypeSession, session_->id());
-  report.type = StatsReport::kStatsReportTypeSession;
-  report.timestamp = stats_gathering_started_;
-  report.values.clear();
-  report.AddBoolean(StatsReport::kStatsValueNameInitiator,
-                    session_->initiator());
-
-  reports_[report.id] = report;
+  StatsReport* report = reports_.FindOrAddNew(StatsId(
+        StatsReport::kStatsReportTypeSession, session_->id()));
+  report->type = StatsReport::kStatsReportTypeSession;
+  report->timestamp = stats_gathering_started_;
+  report->values.clear();
+  report->AddBoolean(StatsReport::kStatsValueNameInitiator,
+                     session_->initiator());
 
   cricket::SessionStats stats;
   if (session_->GetStats(&stats)) {
@@ -857,61 +849,58 @@
                = transport_iter->second.channel_stats.begin();
            channel_iter != transport_iter->second.channel_stats.end();
            ++channel_iter) {
-        StatsReport channel_report;
         std::ostringstream ostc;
         ostc << "Channel-" << transport_iter->second.content_name
              << "-" << channel_iter->component;
-        channel_report.id = ostc.str();
-        channel_report.type = StatsReport::kStatsReportTypeComponent;
-        channel_report.timestamp = stats_gathering_started_;
-        channel_report.AddValue(StatsReport::kStatsValueNameComponent,
-                                channel_iter->component);
+        StatsReport* channel_report = reports_.FindOrAddNew(ostc.str());
+        channel_report->type = StatsReport::kStatsReportTypeComponent;
+        channel_report->timestamp = stats_gathering_started_;
+        channel_report->AddValue(StatsReport::kStatsValueNameComponent,
+                                 channel_iter->component);
         if (!local_cert_report_id.empty())
-          channel_report.AddValue(
+          channel_report->AddValue(
               StatsReport::kStatsValueNameLocalCertificateId,
               local_cert_report_id);
         if (!remote_cert_report_id.empty())
-          channel_report.AddValue(
+          channel_report->AddValue(
               StatsReport::kStatsValueNameRemoteCertificateId,
               remote_cert_report_id);
-        reports_[channel_report.id] = channel_report;
         for (size_t i = 0;
              i < channel_iter->connection_infos.size();
              ++i) {
-          StatsReport report;
-          const cricket::ConnectionInfo& info
-              = channel_iter->connection_infos[i];
           std::ostringstream ost;
           ost << "Conn-" << transport_iter->first << "-"
               << channel_iter->component << "-" << i;
-          report.id = ost.str();
-          report.type = StatsReport::kStatsReportTypeCandidatePair;
-          report.timestamp = stats_gathering_started_;
+          StatsReport* report = reports_.FindOrAddNew(ost.str());
+          report->type = StatsReport::kStatsReportTypeCandidatePair;
+          report->timestamp = stats_gathering_started_;
           // Link from connection to its containing channel.
-          report.AddValue(StatsReport::kStatsValueNameChannelId,
-                          channel_report.id);
-          report.AddValue(StatsReport::kStatsValueNameBytesSent,
-                          info.sent_total_bytes);
-          report.AddValue(StatsReport::kStatsValueNameBytesReceived,
-                          info.recv_total_bytes);
-          report.AddBoolean(StatsReport::kStatsValueNameWritable,
-                            info.writable);
-          report.AddBoolean(StatsReport::kStatsValueNameReadable,
-                            info.readable);
-          report.AddBoolean(StatsReport::kStatsValueNameActiveConnection,
-                            info.best_connection);
-          report.AddValue(StatsReport::kStatsValueNameLocalAddress,
-                          info.local_candidate.address().ToString());
-          report.AddValue(StatsReport::kStatsValueNameRemoteAddress,
-                          info.remote_candidate.address().ToString());
-          report.AddValue(StatsReport::kStatsValueNameRtt, info.rtt);
-          report.AddValue(StatsReport::kStatsValueNameTransportType,
-                          info.local_candidate.protocol());
-          report.AddValue(StatsReport::kStatsValueNameLocalCandidateType,
-                          info.local_candidate.type());
-          report.AddValue(StatsReport::kStatsValueNameRemoteCandidateType,
-                          info.remote_candidate.type());
-          reports_[report.id] = report;
+          report->AddValue(StatsReport::kStatsValueNameChannelId,
+                           channel_report->id);
+
+          const cricket::ConnectionInfo& info =
+              channel_iter->connection_infos[i];
+          report->AddValue(StatsReport::kStatsValueNameBytesSent,
+                           info.sent_total_bytes);
+          report->AddValue(StatsReport::kStatsValueNameBytesReceived,
+                           info.recv_total_bytes);
+          report->AddBoolean(StatsReport::kStatsValueNameWritable,
+                             info.writable);
+          report->AddBoolean(StatsReport::kStatsValueNameReadable,
+                             info.readable);
+          report->AddBoolean(StatsReport::kStatsValueNameActiveConnection,
+                             info.best_connection);
+          report->AddValue(StatsReport::kStatsValueNameLocalAddress,
+                           info.local_candidate.address().ToString());
+          report->AddValue(StatsReport::kStatsValueNameRemoteAddress,
+                           info.remote_candidate.address().ToString());
+          report->AddValue(StatsReport::kStatsValueNameRtt, info.rtt);
+          report->AddValue(StatsReport::kStatsValueNameTransportType,
+                           info.local_candidate.protocol());
+          report->AddValue(StatsReport::kStatsValueNameLocalCandidateType,
+                           info.local_candidate.type());
+          report->AddValue(StatsReport::kStatsValueNameRemoteCandidateType,
+                           info.remote_candidate.type());
         }
       }
     }
@@ -968,7 +957,8 @@
   if (video_info.bw_estimations.size() != 1) {
     LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size();
   } else {
-    StatsReport* report = &reports_[StatsReport::kStatsReportVideoBweId];
+    StatsReport* report =
+        reports_.FindOrAddNew(StatsReport::kStatsReportVideoBweId);
     ExtractStats(
         video_info.bw_estimations[0], stats_gathering_started_, level, report);
   }
@@ -979,13 +969,7 @@
                                        TrackDirection direction) {
   ASSERT(type == StatsReport::kStatsReportTypeSsrc ||
          type == StatsReport::kStatsReportTypeRemoteSsrc);
-  std::string statsid = StatsId(type, id, direction);
-  StatsReport* report = NULL;
-  std::map<std::string, StatsReport>::iterator it = reports_.find(statsid);
-  if (it != reports_.end())
-    report = &(it->second);
-
-  return report;
+  return reports_.Find(StatsId(type, id, direction));
 }
 
 StatsReport* StatsCollector::GetOrCreateReport(const std::string& type,
@@ -996,8 +980,8 @@
   StatsReport* report = GetReport(type, id, direction);
   if (report == NULL) {
     std::string statsid = StatsId(type, id, direction);
-    report = &reports_[statsid];  // Create new element.
-    report->id = statsid;
+    report = reports_.FindOrAddNew(statsid);
+    ASSERT(report->id == statsid);
     report->type = type;
   }
 
diff --git a/talk/app/webrtc/statscollector.h b/talk/app/webrtc/statscollector.h
index a444da4..adfbfee 100644
--- a/talk/app/webrtc/statscollector.h
+++ b/talk/app/webrtc/statscollector.h
@@ -72,7 +72,11 @@
   // be called before this function to get the most recent stats. |selector| is
   // a track label or empty string. The most recent reports are stored in
   // |reports|.
-  bool GetStats(MediaStreamTrackInterface* track,
+  // TODO(tommi): Change this contract to accept a callback object instead
+  // of filling in |reports|.  As is, there's a requirement that the caller
+  // uses |reports| immediately without allowing any async activity on
+  // the thread (message handling etc) and then discard the results.
+  void GetStats(MediaStreamTrackInterface* track,
                 StatsReports* reports);
 
   // Prepare an SSRC report for the given ssrc. Used internally
@@ -121,7 +125,7 @@
                         TrackDirection direction);
 
   // A map from the report id to the report.
-  std::map<std::string, StatsReport> reports_;
+  StatsSet reports_;
   // Raw pointer to the session the statistics are gathered from.
   WebRtcSession* const session_;
   double stats_gathering_started_;
diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc
index fc0ca0e..58caf28 100644
--- a/talk/app/webrtc/statscollector_unittest.cc
+++ b/talk/app/webrtc/statscollector_unittest.cc
@@ -168,10 +168,10 @@
     return kNoReports;
   }
   for (size_t i = 0; i < reports.size(); ++i) {
-    if (reports[i].type != type)
+    if (reports[i]->type != type)
       continue;
     std::string ret;
-    if (GetValue(&reports[i], name, &ret)) {
+    if (GetValue(reports[i], name, &ret)) {
       return ret;
     }
   }
@@ -184,10 +184,10 @@
 const StatsReport* FindNthReportByType(
     const StatsReports& reports, const std::string& type, int n) {
   for (size_t i = 0; i < reports.size(); ++i) {
-    if (reports[i].type == type) {
+    if (reports[i]->type == type) {
       n--;
       if (n == 0)
-        return &reports[i];
+        return reports[i];
     }
   }
   return NULL;
@@ -196,8 +196,8 @@
 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];
+    if (reports[i]->id == id) {
+      return reports[i];
     }
   }
   return NULL;
@@ -433,7 +433,7 @@
 class StatsCollectorTest : public testing::Test {
  protected:
   StatsCollectorTest()
-    : media_engine_(new cricket::FakeMediaEngine),
+    : media_engine_(new cricket::FakeMediaEngine()),
       channel_manager_(
           new cricket::ChannelManager(media_engine_,
                                       new cricket::FakeDeviceManager(),
@@ -443,6 +443,8 @@
     EXPECT_CALL(session_, GetStats(_)).WillRepeatedly(Return(false));
   }
 
+  ~StatsCollectorTest() {}
+
   // This creates a standard setup with a transport called "trspname"
   // having one transport channel
   // and the specified virtual connection name.
@@ -786,7 +788,7 @@
   stats.GetStats(NULL, &reports);
   EXPECT_EQ((size_t)1, reports.size());
   EXPECT_EQ(std::string(StatsReport::kStatsReportTypeTrack),
-            reports[0].type);
+            reports[0]->type);
 
   std::string trackValue =
       ExtractStatsValue(StatsReport::kStatsReportTypeTrack,
@@ -832,6 +834,7 @@
   EXPECT_TRUE(track_report);
 
   // Get report for the specific |track|.
+  reports.clear();
   stats.GetStats(track_, &reports);
   // |reports| should contain at least one session report, one track report,
   // and one ssrc report.
@@ -1333,7 +1336,7 @@
   webrtc::StatsCollector stats(&session_);  // Implementation under test.
   // Ignore unused callback (logspam).
   EXPECT_CALL(session_, GetTransport(_))
-      .WillOnce(Return(static_cast<cricket::Transport*>(NULL)));
+      .WillRepeatedly(Return(static_cast<cricket::Transport*>(NULL)));
   MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel();
   // The content_name known by the voice channel.
   const std::string kVcName("vcname");
@@ -1396,6 +1399,7 @@
   VerifyVoiceSenderInfoReport(track_report, voice_sender_info);
 
   // Get stats for the remote track.
+  reports.clear();
   stats.GetStats(remote_track.get(), &reports);
   track_report = FindNthReportByType(reports,
                                      StatsReport::kStatsReportTypeSsrc, 1);
@@ -1452,6 +1456,7 @@
   cricket::VoiceSenderInfo new_voice_sender_info;
   InitVoiceSenderInfo(&new_voice_sender_info);
   cricket::VoiceMediaInfo new_stats_read;
+  reports.clear();
   SetupAndVerifyAudioTrackStats(
       new_audio_track.get(), stream_.get(), &stats, &voice_channel, kVcName,
       media_channel, &new_voice_sender_info, NULL, &new_stats_read, &reports);
diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h
index 828b9f5..65cbe1f 100644
--- a/talk/app/webrtc/statstypes.h
+++ b/talk/app/webrtc/statstypes.h
@@ -31,6 +31,7 @@
 #ifndef TALK_APP_WEBRTC_STATSTYPES_H_
 #define TALK_APP_WEBRTC_STATSTYPES_H_
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -39,15 +40,45 @@
 
 namespace webrtc {
 
+// TODO(tommi): Move all the implementation that's in this file and
+// statscollector.cc related to these types, into a new, statstypes.cc file.
+
 class StatsReport {
+ protected:
+  // TODO(tommi): Disallow copy ctor etc completely once not needed.
+  explicit StatsReport(const StatsReport& src)
+    : id(src.id),
+      type(src.type),
+      timestamp(src.timestamp),
+      values(src.values) {}
+
+  StatsReport& operator=(const StatsReport& src) {
+    ASSERT(id == src.id);
+    type = src.type;
+    timestamp = src.timestamp;
+    values = src.values;
+    return *this;
+  }
+
+  // Constructor is protected to force use of StatsSet.
+  explicit StatsReport(const std::string& id) : id(id), timestamp(0) {}
+
  public:
-  StatsReport() : timestamp(0) { }
+  // Operators provided for STL container/algorithm support.
+  bool operator<(const StatsReport& other) const { return id < other.id; }
+  bool operator==(const StatsReport& other) const { return id == other.id; }
+  // Special support for being able to use std::find on a container
+  // without requiring a new StatsReport instance.
+  bool operator==(const std::string& other_id) const { return id == other_id; }
 
   // TODO(tommi): Change this to be an enum type that holds all the
   // kStatsValueName constants.
   typedef const char* StatsValueName;
 
-  std::string id;  // See below for contents.
+  // 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 std::string id;  // See below for contents.
   std::string type;  // See below for contents.
 
   struct Value {
@@ -239,7 +270,65 @@
   static const char kStatsValueNameDecodingPLCCNG[];
 };
 
-typedef std::vector<StatsReport> StatsReports;
+// This class is provided for the cases where we need to keep
+// snapshots of reports around.  This is an edge case.
+// TODO(tommi): Move into the private section of StatsSet.
+class StatsReportCopyable : public StatsReport {
+ public:
+  StatsReportCopyable(const std::string& id) : StatsReport(id) {}
+  explicit StatsReportCopyable(const StatsReport& src)
+      : StatsReport(src) {}
+
+  using StatsReport::operator=;
+};
+
+// Typedef for an array of const StatsReport pointers.
+// Ownership of the pointers held by this implementation is assumed to lie
+// elsewhere and lifetime guarantees are made by the implementation that uses
+// this type.  In the StatsCollector, object ownership lies with the StatsSet
+// class.
+typedef std::vector<const StatsReport*> StatsReports;
+
+// A map from the report id to the report.
+// This class wraps an STL container and provides a limited set of
+// functionality in order to keep things simple.
+// TODO(tommi): Use a thread checker here (currently not in libjingle).
+class StatsSet {
+ public:
+  StatsSet() {}
+  ~StatsSet() {}
+
+  typedef std::set<StatsReportCopyable> Container;
+  typedef Container::iterator iterator;
+  typedef Container::const_iterator const_iterator;
+
+  const_iterator begin() const { return list_.begin(); }
+  const_iterator end() const { return list_.end(); }
+
+  // Creates a new report object with |id| that does not already
+  // exist in the list of reports.
+  StatsReport* InsertNew(const std::string& id) {
+    ASSERT(Find(id) == NULL);
+    const StatsReport* ret = &(*list_.insert(StatsReportCopyable(id)).first);
+    return const_cast<StatsReport*>(ret);
+  }
+
+  StatsReport* FindOrAddNew(const std::string& id) {
+    StatsReport* ret = Find(id);
+    return ret ? ret : InsertNew(id);
+  }
+
+  // Looks for a report with the given |id|.  If one is not found, NULL
+  // will be returned.
+  StatsReport* Find(const std::string& id) {
+    iterator it = std::find(begin(), end(), id);
+    return it == end() ? NULL :
+        const_cast<StatsReport*>(static_cast<const StatsReport*>(&(*it)));
+  }
+
+ private:
+  Container list_;
+};
 
 }  // namespace webrtc
 
diff --git a/talk/app/webrtc/test/mockpeerconnectionobservers.h b/talk/app/webrtc/test/mockpeerconnectionobservers.h
index 3ae2162..2bb187e 100644
--- a/talk/app/webrtc/test/mockpeerconnectionobservers.h
+++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h
@@ -120,9 +120,13 @@
   MockStatsObserver()
       : called_(false) {}
   virtual ~MockStatsObserver() {}
-  virtual void OnComplete(const std::vector<webrtc::StatsReport>& reports) {
+  virtual void OnComplete(const StatsReports& reports) {
     called_ = true;
-    reports_ = reports;
+    reports_.clear();
+    reports_.reserve(reports.size());
+    StatsReports::const_iterator it;
+    for (it = reports.begin(); it != reports.end(); ++it)
+      reports_.push_back(StatsReportCopyable(*(*it)));
   }
 
   bool called() const { return called_; }
@@ -148,7 +152,7 @@
   }
 
  private:
-  int GetSsrcStatsValue(const std::string name) {
+  int GetSsrcStatsValue(StatsReport::StatsValueName name) {
     if (reports_.empty()) {
       return 0;
     }
@@ -167,7 +171,7 @@
   }
 
   bool called_;
-  std::vector<webrtc::StatsReport> reports_;
+  std::vector<StatsReportCopyable> reports_;
 };
 
 }  // namespace webrtc