diff --git a/logging/BUILD.gn b/logging/BUILD.gn index 5b91a39357..415c52789c 100644 --- a/logging/BUILD.gn +++ b/logging/BUILD.gn @@ -609,6 +609,7 @@ if (rtc_enable_protobuf) { ":rtc_event_video", ":rtc_stream_config", "../api:array_view", + "../api:field_trials_view", "../api:network_state_predictor_api", "../api:rtc_event_log_output_file", "../api:rtp_headers", @@ -617,7 +618,6 @@ if (rtc_enable_protobuf) { "../api/environment:environment_factory", "../api/rtc_event_log", "../api/rtc_event_log:rtc_event_log_factory", - "../api/transport:field_trial_based_config", "../api/units:time_delta", "../api/units:timestamp", "../call", @@ -631,9 +631,7 @@ if (rtc_enable_protobuf) { "../rtc_base:rtc_base_tests_utils", "../rtc_base:timeutils", "../system_wrappers", - "../system_wrappers:field_trial", "../test:explicit_key_value_config", - "../test:field_trial", "../test:fileutils", "../test:test_support", "../test/logging:log_writer", diff --git a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc index 73ff67e8b9..72611db8ce 100644 --- a/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc +++ b/logging/rtc_event_log/encoder/rtc_event_log_encoder_unittest.cc @@ -14,7 +14,7 @@ #include #include -#include "api/transport/field_trial_based_config.h" +#include "api/field_trials_view.h" #include "logging/rtc_event_log/encoder/rtc_event_log_encoder_legacy.h" #include "logging/rtc_event_log/encoder/rtc_event_log_encoder_new_format.h" #include "logging/rtc_event_log/encoder/rtc_event_log_encoder_v3.h" @@ -42,7 +42,6 @@ #include "rtc_base/fake_clock.h" #include "rtc_base/random.h" #include "test/explicit_key_value_config.h" -#include "test/field_trial.h" #include "test/gtest.h" namespace webrtc { @@ -63,15 +62,15 @@ class RtcEventLogEncoderTest verifier_(encoding_type_) {} ~RtcEventLogEncoderTest() override = default; - std::unique_ptr CreateEncoder() { + std::unique_ptr CreateEncoder( + const FieldTrialsView& field_trials = ExplicitKeyValueConfig("")) { std::unique_ptr encoder; switch (encoding_type_) { case RtcEventLog::EncodingType::Legacy: encoder = std::make_unique(); break; case RtcEventLog::EncodingType::NewFormat: - encoder = std::make_unique( - FieldTrialBasedConfig()); + encoder = std::make_unique(field_trials); break; case RtcEventLog::EncodingType::ProtoFree: encoder = std::make_unique(); @@ -98,7 +97,7 @@ class RtcEventLogEncoderTest uint32_t ssrc); template - void TestRtpPackets(); + void TestRtpPackets(RtcEventLogEncoder& encoder); std::deque> history_; ParsedRtcEventLog parsed_log_; @@ -174,14 +173,13 @@ RtcEventLogEncoderTest::GetRtpPacketsBySsrc(const ParsedRtcEventLog* parsed_log, } template -void RtcEventLogEncoderTest::TestRtpPackets() { +void RtcEventLogEncoderTest::TestRtpPackets(RtcEventLogEncoder& encoder) { // SSRCs will be randomly assigned out of this small pool, significant only // in that it also covers such edge cases as SSRC = 0 and SSRC = 0xffffffff. // The pool is intentionally small, so as to produce collisions. const std::vector kSsrcPool = {0x00000000, 0x12345678, 0xabcdef01, 0xffffffff, 0x20171024, 0x19840730, 0x19831230}; - std::unique_ptr encoder = CreateEncoder(); // TODO(terelius): Test extensions for legacy encoding, too. RtpHeaderExtensionMap extension_map; @@ -203,7 +201,7 @@ void RtcEventLogEncoderTest::TestRtpPackets() { } // Encode and parse. - encoded_ += encoder->EncodeBatch(history_.begin(), history_.end()); + encoded_ += encoder.EncodeBatch(history_.begin(), history_.end()); ASSERT_TRUE(parsed_log_.ParseString(encoded_).ok()); // For each SSRC, make sure the RTP packets associated with it to have been @@ -1339,25 +1337,31 @@ TEST_P(RtcEventLogEncoderTest, RtcEventRtcpLossNotification) { } TEST_P(RtcEventLogEncoderTest, RtcEventRtpPacketIncoming) { - TestRtpPackets(); + std::unique_ptr encoder = CreateEncoder(); + TestRtpPackets(*encoder); } TEST_P(RtcEventLogEncoderTest, RtcEventRtpPacketOutgoing) { - TestRtpPackets(); + std::unique_ptr encoder = CreateEncoder(); + TestRtpPackets(*encoder); } TEST_P(RtcEventLogEncoderTest, RtcEventRtpPacketIncomingNoDependencyDescriptor) { - test::ScopedFieldTrials no_dd( + ExplicitKeyValueConfig no_dd( "WebRTC-RtcEventLogEncodeDependencyDescriptor/Disabled/"); - TestRtpPackets(); + std::unique_ptr encoder = CreateEncoder(no_dd); + verifier_.ExpectDependencyDescriptorExtensionIsSet(false); + TestRtpPackets(*encoder); } TEST_P(RtcEventLogEncoderTest, RtcEventRtpPacketOutgoingNoDependencyDescriptor) { - test::ScopedFieldTrials no_dd( + ExplicitKeyValueConfig no_dd( "WebRTC-RtcEventLogEncodeDependencyDescriptor/Disabled/"); - TestRtpPackets(); + std::unique_ptr encoder = CreateEncoder(no_dd); + verifier_.ExpectDependencyDescriptorExtensionIsSet(false); + TestRtpPackets(*encoder); } // TODO(eladalon/terelius): Test with multiple events in the batch. diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc index df3586a3b6..be771e3cbb 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc @@ -42,8 +42,8 @@ #include "rtc_base/buffer.h" #include "rtc_base/checks.h" #include "rtc_base/time_utils.h" -#include "system_wrappers/include/field_trial.h" #include "system_wrappers/include/ntp_time.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { @@ -52,6 +52,9 @@ namespace test { namespace { +using ::testing::ElementsAreArray; +using ::testing::IsEmpty; + struct ExtensionPair { RTPExtensionType type; const char* name; @@ -1047,23 +1050,15 @@ void VerifyLoggedRtpHeader(const Event& original_header, } template -void VerifyLoggedDependencyDescriptor(const Event& packet, - const std::vector& logged_dd) { - if (webrtc::field_trial::IsDisabled( - "WebRTC-RtcEventLogEncodeDependencyDescriptor")) { - EXPECT_TRUE(logged_dd.empty()); - } else { +void EventVerifier::VerifyLoggedDependencyDescriptor( + const Event& packet, + const std::vector& logged_dd) const { + if (expect_dependency_descriptor_rtp_header_extension_is_set_) { rtc::ArrayView original = packet.template GetRawExtension(); - EXPECT_EQ(logged_dd.size(), original.size()); - bool dd_is_same = true; - for (size_t i = 0; i < logged_dd.size(); ++i) { - dd_is_same = logged_dd[i] == original[i]; - if (!dd_is_same) { - break; - } - } - EXPECT_TRUE(dd_is_same); + EXPECT_THAT(logged_dd, ElementsAreArray(original)); + } else { + EXPECT_THAT(logged_dd, IsEmpty()); } } diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.h b/logging/rtc_event_log/rtc_event_log_unittest_helper.h index 950a622f8b..e0fd3b741b 100644 --- a/logging/rtc_event_log/rtc_event_log_unittest_helper.h +++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.h @@ -160,6 +160,10 @@ class EventVerifier { explicit EventVerifier(RtcEventLog::EncodingType encoding_type) : encoding_type_(encoding_type) {} + void ExpectDependencyDescriptorExtensionIsSet(bool value) { + expect_dependency_descriptor_rtp_header_extension_is_set_ = value; + } + void VerifyLoggedAlrStateEvent(const RtcEventAlrState& original_event, const LoggedAlrStateEvent& logged_event) const; @@ -331,7 +335,13 @@ class EventVerifier { void VerifyReportBlock(const rtcp::ReportBlock& original_report_block, const rtcp::ReportBlock& logged_report_block); + template + void VerifyLoggedDependencyDescriptor( + const Event& packet, + const std::vector& logged_dd) const; + RtcEventLog::EncodingType encoding_type_; + bool expect_dependency_descriptor_rtp_header_extension_is_set_ = true; }; } // namespace test