PeerConnectionInterface::GetStats() with selector argument added.

This exposes the stats selection algorithm[1] on the PeerConnection. Per-spec, there are four flavors of getStats(): 1. RTCPeerConnection.getStats(). 2. RTCPeerConnection.getStats(MediaStreamTrack selector). 3. RTCRtpSender.getStats(). 4. RTCRtpReceiver.getStats(). 1) is the parameterless getStats() which is already shipped. 2) is the same as 3) and 4) except the track is used to look up the corresponding sender/receiver to use as the selector. 3) and 4) perform stats collection with a filter, which is implemented in RTCStatsCollector.GetStatsReport(selector). For technical reasons, it is easier to place GetStats() on the PeerConnection where the RTCStatsCollector lives than to place it on the sender/receiver. Passing the selector as an argument or as a "this" makes little difference other than style. Wiring Chrome up such that the JavaScript APIs is like the spec is trivial after GetStats() is added to PeerConnectionInterface. This CL also adds comments documenting our intent to deprecate and remove the legacy GetStats() APIs some time in the future. [1] https://w3c.github.io/webrtc-pc/#dfn-stats-selection-algorithm Bug: chromium:680172 Change-Id: I09316ba6f20b25d4f9c11785d0a1a1262d6062a1 Reviewed-on: https://webrtc-review.googlesource.com/62900 Reviewed-by: Taylor Brandstetter <[email protected]> Commit-Queue: Henrik Boström <[email protected]> Cr-Commit-Position: refs/heads/master@{#22602}
kellaJL · Mar 26, 2018 · 1df1bf8 · 1df1bf8
1 parent 0191441
commit 1df1bf8
Show file tree

Hide file tree

Showing 8 changed files with 222 additions and 13 deletions.
diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
@@ -805,15 +805,46 @@ class PeerConnectionInterface : public rtc::RefCountInterface {
     return {};
   }
 
+  // The legacy non-compliant GetStats() API. This correspond to the
+  // callback-based version of getStats() in JavaScript. The returned metrics
+  // are UNDOCUMENTED and many of them rely on implementation-specific details.
+  // The goal is to DELETE THIS VERSION but we can't today because it is heavily
+  // relied upon by third parties. See https://crbug.com/822696.
+  //
+  // This version is wired up into Chrome. Any stats implemented are
+  // automatically exposed to the Web Platform. This has BYPASSED the Chrome
+  // release processes for years and lead to cross-browser incompatibility
+  // issues and web application reliance on Chrome-only behavior.
+  //
+  // This API is in "maintenance mode", serious regressions should be fixed but
+  // adding new stats is highly discouraged.
+  //
+  // TODO(hbos): Deprecate and remove this when third parties have migrated to
+  // the spec-compliant GetStats() API. https://crbug.com/822696
   virtual bool GetStats(StatsObserver* observer,
-                        MediaStreamTrackInterface* track,
+                        MediaStreamTrackInterface* track,  // Optional
                         StatsOutputLevel level) = 0;
-  // Gets stats using the new stats collection API, see webrtc/api/stats/. These
-  // will replace old stats collection API when the new API has matured enough.
-  // TODO(hbos): Default implementation that does nothing only exists as to not
-  // break third party projects. As soon as they have been updated this should
-  // be changed to "= 0;".
+  // The spec-compliant GetStats() API. This correspond to the promise-based
+  // version of getStats() in JavaScript. Implementation status is described in
+  // api/stats/rtcstats_objects.h. For more details on stats, see spec:
+  // https://w3c.github.io/webrtc-pc/#dom-rtcpeerconnection-getstats
+  // TODO(hbos): Takes shared ownership, use rtc::scoped_refptr<> instead. This
+  // requires stop overriding the current version in third party or making third
+  // party calls explicit to avoid ambiguity during switch. Make the future
+  // version abstract as soon as third party projects implement it.
   virtual void GetStats(RTCStatsCollectorCallback* callback) {}
+  // Spec-compliant getStats() performing the stats selection algorithm with the
+  // sender. https://w3c.github.io/webrtc-pc/#dom-rtcrtpsender-getstats
+  // TODO(hbos): Make abstract as soon as third party projects implement it.
+  virtual void GetStats(
+      rtc::scoped_refptr<RtpSenderInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) {}
+  // Spec-compliant getStats() performing the stats selection algorithm with the
+  // receiver. https://w3c.github.io/webrtc-pc/#dom-rtcrtpreceiver-getstats
+  // TODO(hbos): Make abstract as soon as third party projects implement it.
+  virtual void GetStats(
+      rtc::scoped_refptr<RtpReceiverInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) {}
   // Clear cached stats in the RTCStatsCollector.
   // Exposed for testing while waiting for automatic cache clear to work.
   // https://bugs.webrtc.org/8693

diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h
@@ -70,6 +70,14 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection)
                 MediaStreamTrackInterface*,
                 StatsOutputLevel)
   PROXY_METHOD1(void, GetStats, RTCStatsCollectorCallback*)
+  PROXY_METHOD2(void,
+                GetStats,
+                rtc::scoped_refptr<RtpSenderInterface>,
+                rtc::scoped_refptr<RTCStatsCollectorCallback>);
+  PROXY_METHOD2(void,
+                GetStats,
+                rtc::scoped_refptr<RtpReceiverInterface>,
+                rtc::scoped_refptr<RTCStatsCollectorCallback>);
   PROXY_METHOD2(rtc::scoped_refptr<DataChannelInterface>,
                 CreateDataChannel,
                 const std::string&,

diff --git a/api/stats/rtcstats_objects.h b/api/stats/rtcstats_objects.h
@@ -11,6 +11,7 @@
 #ifndef API_STATS_RTCSTATS_OBJECTS_H_
 #define API_STATS_RTCSTATS_OBJECTS_H_
 
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -217,11 +218,13 @@ class RTCIceCandidateStats : public RTCStats {
 // But here we define them as subclasses of |RTCIceCandidateStats| because the
 // |kType| need to be different ("RTCStatsType type") in the local/remote case.
 // https://w3c.github.io/webrtc-stats/#rtcstatstype-str*
+// This forces us to have to override copy() and type().
 class RTCLocalIceCandidateStats final : public RTCIceCandidateStats {
  public:
   static const char kType[];
   RTCLocalIceCandidateStats(const std::string& id, int64_t timestamp_us);
   RTCLocalIceCandidateStats(std::string&& id, int64_t timestamp_us);
+  std::unique_ptr<RTCStats> copy() const override;
   const char* type() const override;
 };
 
@@ -230,6 +233,7 @@ class RTCRemoteIceCandidateStats final : public RTCIceCandidateStats {
   static const char kType[];
   RTCRemoteIceCandidateStats(const std::string& id, int64_t timestamp_us);
   RTCRemoteIceCandidateStats(std::string&& id, int64_t timestamp_us);
+  std::unique_ptr<RTCStats> copy() const override;
   const char* type() const override;
 };
 

diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
@@ -1592,10 +1592,68 @@ bool PeerConnection::GetStats(StatsObserver* observer,
 }
 
 void PeerConnection::GetStats(RTCStatsCollectorCallback* callback) {
+  TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
   RTC_DCHECK(stats_collector_);
+  RTC_DCHECK(callback);
   stats_collector_->GetStatsReport(callback);
 }
 
+void PeerConnection::GetStats(
+    rtc::scoped_refptr<RtpSenderInterface> selector,
+    rtc::scoped_refptr<RTCStatsCollectorCallback> callback) {
+  TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
+  RTC_DCHECK(callback);
+  RTC_DCHECK(stats_collector_);
+  rtc::scoped_refptr<RtpSenderInternal> internal_sender;
+  if (selector) {
+    for (const auto& proxy_transceiver : transceivers_) {
+      for (const auto& proxy_sender :
+           proxy_transceiver->internal()->senders()) {
+        if (proxy_sender == selector) {
+          internal_sender = proxy_sender->internal();
+          break;
+        }
+      }
+      if (internal_sender)
+        break;
+    }
+  }
+  // If there is no |internal_sender| then |selector| is either null or does not
+  // belong to the PeerConnection (in Plan B, senders can be removed from the
+  // PeerConnection). This means that "all the stats objects representing the
+  // selector" is an empty set. Invoking GetStatsReport() with a null selector
+  // produces an empty stats report.
+  stats_collector_->GetStatsReport(internal_sender, callback);
+}
+
+void PeerConnection::GetStats(
+    rtc::scoped_refptr<RtpReceiverInterface> selector,
+    rtc::scoped_refptr<RTCStatsCollectorCallback> callback) {
+  TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
+  RTC_DCHECK(callback);
+  RTC_DCHECK(stats_collector_);
+  rtc::scoped_refptr<RtpReceiverInternal> internal_receiver;
+  if (selector) {
+    for (const auto& proxy_transceiver : transceivers_) {
+      for (const auto& proxy_receiver :
+           proxy_transceiver->internal()->receivers()) {
+        if (proxy_receiver == selector) {
+          internal_receiver = proxy_receiver->internal();
+          break;
+        }
+      }
+      if (internal_receiver)
+        break;
+    }
+  }
+  // If there is no |internal_receiver| then |selector| is either null or does
+  // not belong to the PeerConnection (in Plan B, receivers can be removed from
+  // the PeerConnection). This means that "all the stats objects representing
+  // the selector" is an empty set. Invoking GetStatsReport() with a null
+  // selector produces an empty stats report.
+  stats_collector_->GetStatsReport(internal_receiver, callback);
+}
+
 PeerConnectionInterface::SignalingState PeerConnection::signaling_state() {
   return signaling_state_;
 }

diff --git a/pc/peerconnection.h b/pc/peerconnection.h
@@ -116,10 +116,18 @@ class PeerConnection : public PeerConnectionInternal,
   rtc::scoped_refptr<DataChannelInterface> CreateDataChannel(
       const std::string& label,
       const DataChannelInit* config) override;
+  // WARNING: LEGACY. See peerconnectioninterface.h
   bool GetStats(StatsObserver* observer,
                 webrtc::MediaStreamTrackInterface* track,
                 StatsOutputLevel level) override;
+  // Spec-complaint GetStats(). See peerconnectioninterface.h
   void GetStats(RTCStatsCollectorCallback* callback) override;
+  void GetStats(
+      rtc::scoped_refptr<RtpSenderInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) override;
+  void GetStats(
+      rtc::scoped_refptr<RtpReceiverInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) override;
   void ClearStatsCache() override;
 
   SignalingState signaling_state() override;

diff --git a/pc/rtcstats_integrationtest.cc b/pc/rtcstats_integrationtest.cc
@@ -137,10 +137,26 @@ class RTCStatsIntegrationTest : public testing::Test {
   rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCaller() {
     return GetStats(caller_->pc());
   }
+  rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCaller(
+      rtc::scoped_refptr<RtpSenderInterface> selector) {
+    return GetStats(caller_->pc(), selector);
+  }
+  rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCaller(
+      rtc::scoped_refptr<RtpReceiverInterface> selector) {
+    return GetStats(caller_->pc(), selector);
+  }
 
   rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCallee() {
     return GetStats(callee_->pc());
   }
+  rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCallee(
+      rtc::scoped_refptr<RtpSenderInterface> selector) {
+    return GetStats(callee_->pc(), selector);
+  }
+  rtc::scoped_refptr<const RTCStatsReport> GetStatsFromCallee(
+      rtc::scoped_refptr<RtpReceiverInterface> selector) {
+    return GetStats(callee_->pc(), selector);
+  }
 
  protected:
   static rtc::scoped_refptr<const RTCStatsReport> GetStats(
@@ -152,6 +168,17 @@ class RTCStatsIntegrationTest : public testing::Test {
     return stats_obtainer->report();
   }
 
+  template <typename T>
+  static rtc::scoped_refptr<const RTCStatsReport> GetStats(
+      PeerConnectionInterface* pc,
+      rtc::scoped_refptr<T> selector) {
+    rtc::scoped_refptr<RTCStatsObtainer> stats_obtainer =
+        RTCStatsObtainer::Create();
+    pc->GetStats(selector, stats_obtainer);
+    EXPECT_TRUE_WAIT(stats_obtainer->report(), kGetStatsTimeoutMs);
+    return stats_obtainer->report();
+  }
+
   // |network_thread_| uses |virtual_socket_server_| so they must be
   // constructed/destructed in the correct order.
   rtc::VirtualSocketServer virtual_socket_server_;
@@ -315,7 +342,7 @@ class RTCStatsReportVerifier {
       : report_(report) {
   }
 
-  void VerifyReport() {
+  void VerifyReport(std::vector<const char*> allowed_missing_stats) {
     std::set<const char*> missing_stats = StatsTypes();
     bool verify_successful = true;
     std::vector<const RTCTransportStats*> transport_stats =
@@ -367,9 +394,10 @@ class RTCStatsReportVerifier {
         verify_successful = false;
       }
     }
-    if (!missing_stats.empty()) {
-      verify_successful = false;
-      for (const char* missing : missing_stats) {
+    for (const char* missing : missing_stats) {
+      if (std::find(allowed_missing_stats.begin(), allowed_missing_stats.end(),
+                    missing) == allowed_missing_stats.end()) {
+        verify_successful = false;
         EXPECT_TRUE(false) << "Missing expected stats type: " << missing;
       }
     }
@@ -718,18 +746,76 @@ TEST_F(RTCStatsIntegrationTest, GetStatsFromCaller) {
   StartCall();
 
   rtc::scoped_refptr<const RTCStatsReport> report = GetStatsFromCaller();
-  RTCStatsReportVerifier(report.get()).VerifyReport();
+  RTCStatsReportVerifier(report.get()).VerifyReport({});
   EXPECT_EQ(report->ToJson(), RTCStatsReportTraceListener::last_trace());
 }
 
 TEST_F(RTCStatsIntegrationTest, GetStatsFromCallee) {
   StartCall();
 
   rtc::scoped_refptr<const RTCStatsReport> report = GetStatsFromCallee();
-  RTCStatsReportVerifier(report.get()).VerifyReport();
+  RTCStatsReportVerifier(report.get()).VerifyReport({});
   EXPECT_EQ(report->ToJson(), RTCStatsReportTraceListener::last_trace());
 }
 
+// These tests exercise the integration of the stats selection algorithm inside
+// of PeerConnection. See rtcstatstraveral_unittest.cc for more detailed stats
+// traversal tests on particular stats graphs.
+TEST_F(RTCStatsIntegrationTest, GetStatsWithSenderSelector) {
+  StartCall();
+  ASSERT_FALSE(caller_->pc()->GetSenders().empty());
+  rtc::scoped_refptr<const RTCStatsReport> report =
+      GetStatsFromCaller(caller_->pc()->GetSenders()[0]);
+  std::vector<const char*> allowed_missing_stats = {
+      // TODO(hbos): Include RTC[Audio/Video]ReceiverStats when implemented.
+      // TODO(hbos): Include RTCRemoteOutboundRtpStreamStats when implemented.
+      // TODO(hbos): Include RTCRtpContributingSourceStats when implemented.
+      RTCInboundRTPStreamStats::kType, RTCPeerConnectionStats::kType,
+      RTCMediaStreamStats::kType, RTCDataChannelStats::kType,
+  };
+  RTCStatsReportVerifier(report.get()).VerifyReport(allowed_missing_stats);
+  EXPECT_TRUE(report->size());
+}
+
+TEST_F(RTCStatsIntegrationTest, GetStatsWithReceiverSelector) {
+  StartCall();
+
+  ASSERT_FALSE(caller_->pc()->GetReceivers().empty());
+  rtc::scoped_refptr<const RTCStatsReport> report =
+      GetStatsFromCaller(caller_->pc()->GetReceivers()[0]);
+  std::vector<const char*> allowed_missing_stats = {
+      // TODO(hbos): Include RTC[Audio/Video]SenderStats when implemented.
+      // TODO(hbos): Include RTCRemoteInboundRtpStreamStats when implemented.
+      // TODO(hbos): Include RTCRtpContributingSourceStats when implemented.
+      RTCOutboundRTPStreamStats::kType, RTCPeerConnectionStats::kType,
+      RTCMediaStreamStats::kType, RTCDataChannelStats::kType,
+  };
+  RTCStatsReportVerifier(report.get()).VerifyReport(allowed_missing_stats);
+  EXPECT_TRUE(report->size());
+}
+
+TEST_F(RTCStatsIntegrationTest, GetStatsWithInvalidSenderSelector) {
+  StartCall();
+
+  ASSERT_FALSE(callee_->pc()->GetSenders().empty());
+  // The selector is invalid for the caller because it belongs to the callee.
+  auto invalid_selector = callee_->pc()->GetSenders()[0];
+  rtc::scoped_refptr<const RTCStatsReport> report =
+      GetStatsFromCaller(invalid_selector);
+  EXPECT_FALSE(report->size());
+}
+
+TEST_F(RTCStatsIntegrationTest, GetStatsWithInvalidReceiverSelector) {
+  StartCall();
+
+  ASSERT_FALSE(callee_->pc()->GetReceivers().empty());
+  // The selector is invalid for the caller because it belongs to the callee.
+  auto invalid_selector = callee_->pc()->GetReceivers()[0];
+  rtc::scoped_refptr<const RTCStatsReport> report =
+      GetStatsFromCaller(invalid_selector);
+  EXPECT_FALSE(report->size());
+}
+
 TEST_F(RTCStatsIntegrationTest, GetsStatsWhileDestroyingPeerConnections) {
   StartCall();
 

diff --git a/pc/test/fakepeerconnectionbase.h b/pc/test/fakepeerconnectionbase.h
@@ -110,6 +110,12 @@ class FakePeerConnectionBase : public PeerConnectionInternal {
   }
 
   void GetStats(RTCStatsCollectorCallback* callback) override {}
+  void GetStats(
+      rtc::scoped_refptr<RtpSenderInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) override {}
+  void GetStats(
+      rtc::scoped_refptr<RtpReceiverInterface> selector,
+      rtc::scoped_refptr<RTCStatsCollectorCallback> callback) override {}
 
   void ClearStatsCache() override {}
 

diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc
@@ -262,7 +262,7 @@ RTCIceCandidatePairStats::~RTCIceCandidatePairStats() {
 }
 
 // clang-format off
-WEBRTC_RTCSTATS_IMPL(RTCIceCandidateStats, RTCStats, "ice-candidate",
+WEBRTC_RTCSTATS_IMPL(RTCIceCandidateStats, RTCStats, "abstract-ice-candidate",
     &transport_id,
     &is_remote,
     &network_type,
@@ -323,6 +323,10 @@ RTCLocalIceCandidateStats::RTCLocalIceCandidateStats(
     : RTCIceCandidateStats(std::move(id), timestamp_us, false) {
 }
 
+std::unique_ptr<RTCStats> RTCLocalIceCandidateStats::copy() const {
+  return std::unique_ptr<RTCStats>(new RTCLocalIceCandidateStats(*this));
+}
+
 const char* RTCLocalIceCandidateStats::type() const {
   return kType;
 }
@@ -339,6 +343,10 @@ RTCRemoteIceCandidateStats::RTCRemoteIceCandidateStats(
     : RTCIceCandidateStats(std::move(id), timestamp_us, true) {
 }
 
+std::unique_ptr<RTCStats> RTCRemoteIceCandidateStats::copy() const {
+  return std::unique_ptr<RTCStats>(new RTCRemoteIceCandidateStats(*this));
+}
+
 const char* RTCRemoteIceCandidateStats::type() const {
   return kType;
 }