commit 2f71b61a34c86c5a267e671d36ad5c74f1a0fb69
author Henrik Boström <hbos@webrtc.org> Tue Mar 23 14:18:55 2021
committer Commit Bot <commit-bot@chromium.org> Tue Mar 23 15:27:46 2021
tree 535fc81c7daa443395a48d41cd21d05814553c21
parent ca18809ee52648335d7ea03df18fa3fd5d120bb2

Make sure "remote-inbound-rtp.jitter" and "packetsLost" is exposed to JS

In refactoring CL https://webrtc-review.googlesource.com/c/src/+/210340,
the RTCRemoteInboundRtpStreamStats hierarchy was updated to inherit from
RTCReceivedRtpStreamStats but we forgot to update the
WEBRTC_RTCSTATS_IMPL() macro to say that RTCReceivedRtpStreamStats is
the parent. As a consequence, RTCReceivedRtpStreamStats's members
(jitter and packetsLost) were not included when iterating over all
members of RTCRemoteInboundRtpStreamStats, which means these two merics
stopped being exposed to JavaScript in Chromium.

There is sadly no way to safe-guard against this, but the fix is simple.

TBR=hta@webrtc.org,meetwudi@gmail.com

Bug: webrtc:12532
Change-Id: I0179dad6eaa592ee36cfe48978f2fc22133b8f45
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/212866
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Alessio Bazzica <alessiob@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33543}

pc/rtc_stats_integrationtest.cc

diff --git a/pc/rtc_stats_integrationtest.cc b/pc/rtc_stats_integrationtest.cc
index 456ca72..a285555 100644
--- a/pc/rtc_stats_integrationtest.cc
+++ b/pc/rtc_stats_integrationtest.cc
@@ -771,10 +771,18 @@
   void VerifyRTCRTPStreamStats(const RTCRTPStreamStats& stream,
                                RTCStatsVerifier* verifier) {
     verifier->TestMemberIsDefined(stream.ssrc);
-    verifier->TestMemberIsDefined(stream.media_type);
     verifier->TestMemberIsDefined(stream.kind);
-    verifier->TestMemberIsIDReference(stream.track_id,
-                                      RTCMediaStreamTrackStats::kType);
+    // Some legacy metrics are only defined for some of the RTP types in the
+    // hierarcy.
+    if (stream.type() == RTCInboundRTPStreamStats::kType ||
+        stream.type() == RTCOutboundRTPStreamStats::kType) {
+      verifier->TestMemberIsDefined(stream.media_type);
+      verifier->TestMemberIsIDReference(stream.track_id,
+                                        RTCMediaStreamTrackStats::kType);
+    } else {
+      verifier->TestMemberIsUndefined(stream.media_type);
+      verifier->TestMemberIsUndefined(stream.track_id);
+    }
     verifier->TestMemberIsIDReference(stream.transport_id,
                                       RTCTransportStats::kType);
     verifier->TestMemberIsIDReference(stream.codec_id, RTCCodecStats::kType);
@@ -917,6 +925,9 @@
   bool VerifyRTCOutboundRTPStreamStats(
       const RTCOutboundRTPStreamStats& outbound_stream) {
     RTCStatsVerifier verifier(report_, &outbound_stream);
+    // TODO(https://crbug.com/webrtc/12532): Invoke
+    // VerifyRTCReceivedRtpStreamStats() instead of VerifyRTCRTPStreamStats()
+    // because they have a shared hierarchy now!
     VerifyRTCRTPStreamStats(outbound_stream, &verifier);
     if (outbound_stream.media_type.is_defined() &&
         *outbound_stream.media_type == "video") {
@@ -1008,20 +1019,19 @@
     return verifier.ExpectAllMembersSuccessfullyTested();
   }
 
+  void VerifyRTCReceivedRtpStreamStats(
+      const RTCReceivedRtpStreamStats& received_rtp,
+      RTCStatsVerifier* verifier) {
+    VerifyRTCRTPStreamStats(received_rtp, verifier);
+    verifier->TestMemberIsNonNegative<double>(received_rtp.jitter);
+    verifier->TestMemberIsDefined(received_rtp.packets_lost);
+  }
+
   bool VerifyRTCRemoteInboundRtpStreamStats(
       const RTCRemoteInboundRtpStreamStats& remote_inbound_stream) {
     RTCStatsVerifier verifier(report_, &remote_inbound_stream);
-    verifier.TestMemberIsDefined(remote_inbound_stream.ssrc);
-    verifier.TestMemberIsDefined(remote_inbound_stream.kind);
-    verifier.TestMemberIsIDReference(remote_inbound_stream.transport_id,
-                                     RTCTransportStats::kType);
-    verifier.TestMemberIsIDReference(remote_inbound_stream.codec_id,
-                                     RTCCodecStats::kType);
-    verifier.TestMemberIsDefined(remote_inbound_stream.packets_lost);
+    VerifyRTCReceivedRtpStreamStats(remote_inbound_stream, &verifier);
     verifier.TestMemberIsDefined(remote_inbound_stream.fraction_lost);
-    // Note that the existance of RTCCodecStats is needed for |codec_id| and
-    // |jitter| to be present.
-    verifier.TestMemberIsNonNegative<double>(remote_inbound_stream.jitter);
     verifier.TestMemberIsIDReference(remote_inbound_stream.local_id,
                                      RTCOutboundRTPStreamStats::kType);
     verifier.TestMemberIsNonNegative<double>(

stats/rtcstats_objects.cc

diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc
index 33492e7..3a12eea 100644
--- a/stats/rtcstats_objects.cc
+++ b/stats/rtcstats_objects.cc
@@ -874,7 +874,8 @@
 
 // clang-format off
 WEBRTC_RTCSTATS_IMPL(
-    RTCRemoteInboundRtpStreamStats, RTCStats, "remote-inbound-rtp",
+    RTCRemoteInboundRtpStreamStats, RTCReceivedRtpStreamStats,
+        "remote-inbound-rtp",
     &ssrc,
     &kind,
     &transport_id,