commit e9e94c3fee79b0b590e2e0fd40f8c55398d8f548
author zhihuang <zhihuang@webrtc.org> Fri Nov 04 11:38:15 2016 -0700
committer Commit bot <commit-bot@chromium.org> Fri Nov 04 18:38:19 2016 +0000
tree 7b964e2f4f8f6fd392ec9824cc2e3c9e2244cd52
parent 3326d0ac8232d6e9f6cd696b63f88bd817b0ec62

Return false if PeerConnection::GetStats() is called on invalid tracks

Before calling StatsCollctor::GetStats() in PeerConnection::GetStats(), check if the track is valid. If not, return false.
A track is invalid if it is not a nullptr and there is no report data for it.

BUG=webrtc:6652

Review-Url: https://codereview.webrtc.org/2470023004
Cr-Commit-Position: refs/heads/master@{#14934}

webrtc/api/peerconnection.cc

diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 34beeff..3fdbefb 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -886,6 +886,13 @@
   }
 
   stats_->UpdateStats(level);
+  // The StatsCollector is used to tell if a track is valid because it may
+  // remember tracks that the PeerConnection previously removed.
+  if (track && !stats_->IsValidTrack(track->id())) {
+    LOG(LS_WARNING) << "GetStats is called with an invalid track: "
+                    << track->id();
+    return false;
+  }
   signaling_thread()->Post(RTC_FROM_HERE, this, MSG_GETSTATS,
                            new GetStatsMsg(observer, track));
   return true;

webrtc/api/peerconnectioninterface_unittest.cc

diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index a4a11e1..3f84c3d 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -1577,10 +1577,7 @@
 }
 
 // Test that we don't get statistics for an invalid track.
-// TODO(tommi): Fix this test.  DoGetStats will return true
-// for the unknown track (since GetStats is async), but no
-// data is returned for the track.
-TEST_F(PeerConnectionInterfaceTest, DISABLED_GetStatsForInvalidTrack) {
+TEST_F(PeerConnectionInterfaceTest, GetStatsForInvalidTrack) {
   InitiateCall();
   rtc::scoped_refptr<AudioTrackInterface> unknown_audio_track(
       pc_factory_->CreateAudioTrack("unknown track", NULL));

webrtc/api/statscollector.cc

diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index a40835c..7dc17da 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -549,6 +549,11 @@
   return report;
 }
 
+bool StatsCollector::IsValidTrack(const std::string& track_id) {
+  return reports_.Find(StatsReport::NewTypedId(
+             StatsReport::kStatsReportTypeTrack, track_id)) != nullptr;
+}
+
 StatsReport* StatsCollector::AddCertificateReports(
     const rtc::SSLCertificate* cert) {
   RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent());

webrtc/api/statscollector.h

diff --git a/webrtc/api/statscollector.h b/webrtc/api/statscollector.h
index adbac44..7c19821 100644
--- a/webrtc/api/statscollector.h
+++ b/webrtc/api/statscollector.h
@@ -78,6 +78,9 @@
                              const StatsReport::Id& transport_id,
                              StatsReport::Direction direction);
 
+  // A track is invalid if there is no report data for it.
+  bool IsValidTrack(const std::string& track_id);
+
   // Method used by the unittest to force a update of stats since UpdateStats()
   // that occur less than kMinGatherStatsPeriod number of ms apart will be
   // ignored.