From e9e94c3fee79b0b590e2e0fd40f8c55398d8f548 Mon Sep 17 00:00:00 2001
From: zhihuang <zhihuang@webrtc.org>
Date: Fri, 4 Nov 2016 11:38:15 -0700
Subject: [PATCH] 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                   | 7 +++++++
 webrtc/api/peerconnectioninterface_unittest.cc | 5 +----
 webrtc/api/statscollector.cc                   | 5 +++++
 webrtc/api/statscollector.h                    | 3 +++
 4 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc
index 34beeffa7c..3fdbefb8a9 100644
--- a/webrtc/api/peerconnection.cc
+++ b/webrtc/api/peerconnection.cc
@@ -886,6 +886,13 @@ bool PeerConnection::GetStats(StatsObserver* observer,
   }
 
   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;
diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc
index a4a11e1252..3f84c3d75f 100644
--- a/webrtc/api/peerconnectioninterface_unittest.cc
+++ b/webrtc/api/peerconnectioninterface_unittest.cc
@@ -1577,10 +1577,7 @@ TEST_F(PeerConnectionInterfaceTest, GetStatsForVideoTrack) {
 }
 
 // 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));
diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc
index a40835c77e..7dc17da159 100644
--- a/webrtc/api/statscollector.cc
+++ b/webrtc/api/statscollector.cc
@@ -549,6 +549,11 @@ StatsReport* StatsCollector::PrepareReport(
   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());
diff --git a/webrtc/api/statscollector.h b/webrtc/api/statscollector.h
index adbac44b80..7c198217e8 100644
--- a/webrtc/api/statscollector.h
+++ b/webrtc/api/statscollector.h
@@ -78,6 +78,9 @@ class StatsCollector {
                              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.