From 2057d7177574e67c6f2ba1b06c1d5cf460b2e842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 24 May 2023 11:18:56 +0200 Subject: [PATCH] [Stats] Delete unused NonStandardGroupId. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NonStandardGroupId is no longer wired up to Chrome, but if we did want to only expose certain metrics if a field trial was enabled then the right place to do that would be in blink, where WebIDL lives. This was only used prior to the WebRtcStatsReportIdl launch and experiments haven't been active in several years so its dead code. Blocked on: - https://chromium-review.googlesource.com/c/chromium/src/+/4514754 Bug: webrtc:15162 Change-Id: Ia41a4d21d7e5f029ddb121183fbd69ae7f98fac4 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/304720 Commit-Queue: Henrik Boström Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#40132} --- api/stats/rtc_stats.h | 31 ++----------------------------- stats/rtc_stats_unittest.cc | 10 ---------- stats/rtcstats_objects.cc | 13 +++---------- 3 files changed, 5 insertions(+), 49 deletions(-) diff --git a/api/stats/rtc_stats.h b/api/stats/rtc_stats.h index 30a725bb66..0bd70a0fb5 100644 --- a/api/stats/rtc_stats.h +++ b/api/stats/rtc_stats.h @@ -206,21 +206,6 @@ class RTC_EXPORT RTCStats { return parent_class::MembersOfThisObjectAndAncestors(0); \ } -// Non-standard stats members can be exposed to the JavaScript API in Chrome -// e.g. through origin trials. The group ID can be used by the blink layer to -// determine if a stats member should be exposed or not. Multiple non-standard -// stats members can share the same group ID so that they are exposed together. -enum class NonStandardGroupId { - // Group ID used for testing purposes only. - kGroupIdForTesting, - // I2E: - // https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/hE2B1iItPDk - kRtcAudioJitterBufferMaxPackets, - // I2E: - // https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/YbhMyqLXXXo - kRtcStatsRelativePacketArrivalDelay, -}; - // Certain stat members should only be exposed to the JavaScript API in // certain circumstances as to avoid passive fingerprinting. enum class StatExposureCriteria : uint8_t { @@ -274,9 +259,6 @@ class RTCStatsMemberInterface { bool is_standardized() const { return exposure_criteria() != StatExposureCriteria::kNonStandard; } - // Non-standard stats members can have group IDs in order to be exposed in - // JavaScript through experiments. Standardized stats have no group IDs. - virtual std::vector group_ids() const { return {}; } // The conditions for exposing the statistic to JavaScript. Stats with // criteria that is not kAlways has some restriction and should be filtered // in accordance to the spec. @@ -512,22 +494,14 @@ class RTCNonStandardStatsMember public: explicit RTCNonStandardStatsMember(const char* name) : RTCRestrictedStatsBase(name) {} - RTCNonStandardStatsMember(const char* name, - std::initializer_list group_ids) - : RTCRestrictedStatsBase(name), group_ids_(group_ids) {} RTCNonStandardStatsMember(const char* name, const T& value) : RTCRestrictedStatsBase(name, value) {} RTCNonStandardStatsMember(const char* name, T&& value) : RTCRestrictedStatsBase(name, std::move(value)) {} RTCNonStandardStatsMember(const RTCNonStandardStatsMember& other) - : RTCRestrictedStatsBase(other), group_ids_(other.group_ids_) {} + : RTCRestrictedStatsBase(other) {} RTCNonStandardStatsMember(RTCNonStandardStatsMember&& other) - : RTCRestrictedStatsBase(std::move(other)), - group_ids_(std::move(other.group_ids_)) {} - - std::vector group_ids() const override { - return group_ids_; - } + : RTCRestrictedStatsBase(std::move(other)) {} T& operator=(const T& value) { return RTCRestrictedStatsMember< @@ -541,7 +515,6 @@ class RTCNonStandardStatsMember private: using RTCRestrictedStatsBase = RTCRestrictedStatsMember; - std::vector group_ids_; }; extern template class RTC_EXPORT_TEMPLATE_DECLARE(RTC_EXPORT) diff --git a/stats/rtc_stats_unittest.cc b/stats/rtc_stats_unittest.cc index 84b9a63ed4..5e4b5f12a4 100644 --- a/stats/rtc_stats_unittest.cc +++ b/stats/rtc_stats_unittest.cc @@ -518,16 +518,6 @@ TEST(RTCStatsTest, RestrictedStatsTest) { << "These can not be equal as they have different exposure criteria."; } -TEST(RTCStatsTest, NonStandardGroupId) { - auto group_id = NonStandardGroupId::kGroupIdForTesting; - RTCNonStandardStatsMember with_group_id("stat", {group_id}); - std::vector expected_ids({group_id}); - EXPECT_EQ(expected_ids, with_group_id.group_ids()); - - RTCNonStandardStatsMember without_group_id("stat"); - EXPECT_TRUE(without_group_id.group_ids().empty()); -} - // Death tests. // Disabled on Android because death tests misbehave on Android, see // base/test/gtest_util.h. diff --git a/stats/rtcstats_objects.cc b/stats/rtcstats_objects.cc index adf3f8c18e..888a77b37e 100644 --- a/stats/rtcstats_objects.cc +++ b/stats/rtcstats_objects.cc @@ -481,16 +481,9 @@ RTCInboundRtpStreamStats::RTCInboundRtpStreamStats(std::string id, qp_sum("qpSum"), goog_timing_frame_info("googTimingFrameInfo"), power_efficient_decoder("powerEfficientDecoder"), - jitter_buffer_flushes( - "jitterBufferFlushes", - {NonStandardGroupId::kRtcAudioJitterBufferMaxPackets}), - delayed_packet_outage_samples( - "delayedPacketOutageSamples", - {NonStandardGroupId::kRtcAudioJitterBufferMaxPackets, - NonStandardGroupId::kRtcStatsRelativePacketArrivalDelay}), - relative_packet_arrival_delay( - "relativePacketArrivalDelay", - {NonStandardGroupId::kRtcStatsRelativePacketArrivalDelay}), + jitter_buffer_flushes("jitterBufferFlushes"), + delayed_packet_outage_samples("delayedPacketOutageSamples"), + relative_packet_arrival_delay("relativePacketArrivalDelay"), interruption_count("interruptionCount"), total_interruption_duration("totalInterruptionDuration"), min_playout_delay("minPlayoutDelay"),