From 7cb69d5cc71a496b0e9372001ddd22f471d4d076 Mon Sep 17 00:00:00 2001 From: zstein Date: Mon, 8 May 2017 11:52:38 -0700 Subject: [PATCH] This will allow me to test that Call invokes SendSideCongestionController::SetBweBitrates as expected (for https://codereview.chromium.org/2793913008). FakeRtpTransportController moves to a common header and its constructor is changed to take a SendSideCongestionController to enable injecting the mock. BUG=webrtc:7395 Review-Url: https://codereview.webrtc.org/2834663003 Cr-Commit-Position: refs/heads/master@{#18055} --- webrtc/audio/BUILD.gn | 1 + webrtc/audio/audio_send_stream_unittest.cc | 45 ++++++----------- webrtc/call/BUILD.gn | 1 + webrtc/call/call.cc | 20 +++++--- webrtc/call/call.h | 7 +++ webrtc/call/call_unittest.cc | 28 +++++++++++ .../call/fake_rtp_transport_controller_send.h | 48 +++++++++++++++++++ webrtc/call/rtp_transport_controller_send.cc | 6 --- webrtc/call/rtp_transport_controller_send.h | 3 -- webrtc/modules/congestion_controller/BUILD.gn | 13 +++++ .../include/congestion_controller.h | 2 +- .../include/mock/mock_congestion_observer.h | 3 -- .../mock_send_side_congestion_controller.h | 39 +++++++++++++++ .../include/send_side_congestion_controller.h | 3 +- 14 files changed, 167 insertions(+), 52 deletions(-) create mode 100644 webrtc/call/fake_rtp_transport_controller_send.h create mode 100644 webrtc/modules/congestion_controller/include/mock/mock_send_side_congestion_controller.h diff --git a/webrtc/audio/BUILD.gn b/webrtc/audio/BUILD.gn index 897c65f10b..eba088050c 100644 --- a/webrtc/audio/BUILD.gn +++ b/webrtc/audio/BUILD.gn @@ -81,6 +81,7 @@ if (rtc_include_tests) { "../modules/audio_device:mock_audio_device", "../modules/audio_mixer:audio_mixer_impl", "../modules/congestion_controller:congestion_controller", + "../modules/congestion_controller:mock_congestion_controller", "../modules/pacing:pacing", "../test:test_common", "../test:test_support", diff --git a/webrtc/audio/audio_send_stream_unittest.cc b/webrtc/audio/audio_send_stream_unittest.cc index 99e7bd5254..c66efea674 100644 --- a/webrtc/audio/audio_send_stream_unittest.cc +++ b/webrtc/audio/audio_send_stream_unittest.cc @@ -15,7 +15,9 @@ #include "webrtc/audio/audio_send_stream.h" #include "webrtc/audio/audio_state.h" #include "webrtc/audio/conversion.h" +#include "webrtc/base/ptr_util.h" #include "webrtc/base/task_queue.h" +#include "webrtc/call/fake_rtp_transport_controller_send.h" #include "webrtc/call/rtp_transport_controller_send_interface.h" #include "webrtc/logging/rtc_event_log/mock/mock_rtc_event_log.h" #include "webrtc/modules/audio_mixer/audio_mixer_impl.h" @@ -23,8 +25,8 @@ #include "webrtc/modules/congestion_controller/include/mock/mock_congestion_observer.h" #include "webrtc/modules/congestion_controller/include/send_side_congestion_controller.h" #include "webrtc/modules/pacing/paced_sender.h" -#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "webrtc/modules/rtp_rtcp/mocks/mock_rtcp_rtt_stats.h" +#include "webrtc/modules/rtp_rtcp/mocks/mock_rtp_rtcp.h" #include "webrtc/test/gtest.h" #include "webrtc/test/mock_audio_encoder.h" #include "webrtc/test/mock_audio_encoder_factory.h" @@ -124,36 +126,15 @@ rtc::scoped_refptr SetupEncoderFactoryMock() { } struct ConfigHelper { - class FakeRtpTransportController - : public RtpTransportControllerSendInterface { - public: - explicit FakeRtpTransportController(RtcEventLog* event_log) - : simulated_clock_(123456), - send_side_cc_(&simulated_clock_, - &bitrate_observer_, - event_log, - &packet_router_) {} - PacketRouter* packet_router() override { return &packet_router_; } - - SendSideCongestionController* send_side_cc() override { - return &send_side_cc_; - } - TransportFeedbackObserver* transport_feedback_observer() override { - return &send_side_cc_; - } - - RtpPacketSender* packet_sender() override { return send_side_cc_.pacer(); } - - private: - SimulatedClock simulated_clock_; - testing::NiceMock bitrate_observer_; - PacketRouter packet_router_; - SendSideCongestionController send_side_cc_; - }; - ConfigHelper(bool audio_bwe_enabled, bool expect_set_encoder_call) : stream_config_(nullptr), - fake_transport_(&event_log_), + simulated_clock_(123456), + send_side_cc_(rtc::MakeUnique( + &simulated_clock_, + nullptr /* observer */, + &event_log_, + &packet_router_)), + fake_transport_(send_side_cc_.get()), bitrate_allocator_(&limit_observer_), worker_queue_("ConfigHelper_worker_queue") { using testing::Invoke; @@ -322,11 +303,13 @@ struct ConfigHelper { rtc::scoped_refptr audio_state_; AudioSendStream::Config stream_config_; testing::StrictMock* channel_proxy_ = nullptr; - testing::NiceMock bitrate_observer_; MockAudioProcessing audio_processing_; MockTransmitMixer transmit_mixer_; AudioProcessing::AudioProcessingStatistics audio_processing_stats_; - FakeRtpTransportController fake_transport_; + SimulatedClock simulated_clock_; + PacketRouter packet_router_; + std::unique_ptr send_side_cc_; + FakeRtpTransportControllerSend fake_transport_; MockRtcEventLog event_log_; MockRtpRtcp rtp_rtcp_; MockRtcpRttStats rtcp_rtt_stats_; diff --git a/webrtc/call/BUILD.gn b/webrtc/call/BUILD.gn index 826ef65873..da94de536b 100644 --- a/webrtc/call/BUILD.gn +++ b/webrtc/call/BUILD.gn @@ -93,6 +93,7 @@ if (rtc_include_tests) { "../modules/audio_device:mock_audio_device", "../modules/audio_mixer", "../modules/bitrate_controller", + "../modules/congestion_controller:mock_congestion_controller", "../modules/pacing", "../modules/rtp_rtcp", "../system_wrappers", diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc index 0e64269d4e..e594dcc6ca 100644 --- a/webrtc/call/call.cc +++ b/webrtc/call/call.cc @@ -26,6 +26,7 @@ #include "webrtc/base/location.h" #include "webrtc/base/logging.h" #include "webrtc/base/optional.h" +#include "webrtc/base/ptr_util.h" #include "webrtc/base/task_queue.h" #include "webrtc/base/thread_annotations.h" #include "webrtc/base/thread_checker.h" @@ -97,7 +98,7 @@ class Call : public webrtc::Call, public BitrateAllocator::LimitObserver { public: Call(const Call::Config& config, - std::unique_ptr transport_send); + std::unique_ptr transport_send); virtual ~Call(); // Implements webrtc::Call. @@ -298,16 +299,21 @@ std::string Call::Stats::ToString(int64_t time_ms) const { } Call* Call::Create(const Call::Config& config) { - return new internal::Call( - config, std::unique_ptr( - new RtpTransportControllerSend(Clock::GetRealTimeClock(), - config.event_log))); + return new internal::Call(config, + rtc::MakeUnique( + Clock::GetRealTimeClock(), config.event_log)); +} + +Call* Call::Create( + const Call::Config& config, + std::unique_ptr transport_send) { + return new internal::Call(config, std::move(transport_send)); } namespace internal { Call::Call(const Call::Config& config, - std::unique_ptr transport_send) + std::unique_ptr transport_send) : clock_(Clock::GetRealTimeClock()), num_cpu_cores_(CpuInfo::DetectNumberOfCores()), module_process_thread_(ProcessThread::Create("ModuleProcessThread")), @@ -342,7 +348,7 @@ Call::Call(const Call::Config& config, config.bitrate_config.start_bitrate_bps); } Trace::CreateTrace(); - transport_send->RegisterNetworkObserver(this); + transport_send->send_side_cc()->RegisterNetworkObserver(this); transport_send_ = std::move(transport_send); transport_send_->send_side_cc()->SignalNetworkState(kNetworkDown); transport_send_->send_side_cc()->SetBweBitrates( diff --git a/webrtc/call/call.h b/webrtc/call/call.h index caf0ee2d88..f67b6907e5 100644 --- a/webrtc/call/call.h +++ b/webrtc/call/call.h @@ -10,6 +10,7 @@ #ifndef WEBRTC_CALL_CALL_H_ #define WEBRTC_CALL_CALL_H_ +#include #include #include @@ -20,6 +21,7 @@ #include "webrtc/call/audio_send_stream.h" #include "webrtc/call/audio_state.h" #include "webrtc/call/flexfec_receive_stream.h" +#include "webrtc/call/rtp_transport_controller_send_interface.h" #include "webrtc/common_types.h" #include "webrtc/video_receive_stream.h" #include "webrtc/video_send_stream.h" @@ -98,6 +100,11 @@ class Call { static Call* Create(const Call::Config& config); + // Allows mocking |transport_send| for testing. + static Call* Create( + const Call::Config& config, + std::unique_ptr transport_send); + virtual AudioSendStream* CreateAudioSendStream( const AudioSendStream::Config& config) = 0; virtual void DestroyAudioSendStream(AudioSendStream* send_stream) = 0; diff --git a/webrtc/call/call_unittest.cc b/webrtc/call/call_unittest.cc index 27d2329811..564f6bd032 100644 --- a/webrtc/call/call_unittest.cc +++ b/webrtc/call/call_unittest.cc @@ -11,11 +11,15 @@ #include #include #include +#include +#include "webrtc/base/ptr_util.h" #include "webrtc/call/audio_state.h" #include "webrtc/call/call.h" +#include "webrtc/call/fake_rtp_transport_controller_send.h" #include "webrtc/logging/rtc_event_log/rtc_event_log.h" #include "webrtc/modules/audio_mixer/audio_mixer_impl.h" +#include "webrtc/modules/congestion_controller/include/mock/mock_send_side_congestion_controller.h" #include "webrtc/test/gtest.h" #include "webrtc/test/mock_audio_decoder_factory.h" #include "webrtc/test/mock_transport.h" @@ -305,4 +309,28 @@ TEST(CallTest, MultipleFlexfecReceiveStreamsProtectingSingleVideoStream) { } } +// TODO(zstein): This is just a motivating example for +// MockSendSideCongestionController. It should be deleted once we have more +// meaningful tests. +TEST(CallTest, MockSendSideCongestionControllerExample) { + RtcEventLogNullImpl event_log; + Call::Config config(&event_log); + + SimulatedClock clock(123456); + PacketRouter packet_router; + testing::NiceMock mock_cc( + &clock, &event_log, &packet_router); + auto transport_send = + rtc::MakeUnique(&mock_cc); + std::unique_ptr call(Call::Create(config, std::move(transport_send))); + + Call::Config::BitrateConfig bitrate_config; + bitrate_config.min_bitrate_bps = 1; + bitrate_config.start_bitrate_bps = 2; + bitrate_config.max_bitrate_bps = 3; + + EXPECT_CALL(mock_cc, SetBweBitrates(1, 2, 3)); + call->SetBitrateConfig(bitrate_config); +} + } // namespace webrtc diff --git a/webrtc/call/fake_rtp_transport_controller_send.h b/webrtc/call/fake_rtp_transport_controller_send.h new file mode 100644 index 0000000000..42462d9062 --- /dev/null +++ b/webrtc/call/fake_rtp_transport_controller_send.h @@ -0,0 +1,48 @@ +/* + * Copyright (c) 2015 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_CALL_FAKE_RTP_TRANSPORT_CONTROLLER_SEND_H_ +#define WEBRTC_CALL_FAKE_RTP_TRANSPORT_CONTROLLER_SEND_H_ + +#include "webrtc/call/rtp_transport_controller_send_interface.h" +#include "webrtc/modules/congestion_controller/include/send_side_congestion_controller.h" +#include "webrtc/modules/pacing/packet_router.h" + +namespace webrtc { + +class FakeRtpTransportControllerSend + : public RtpTransportControllerSendInterface { + public: + explicit FakeRtpTransportControllerSend( + SendSideCongestionController* send_side_cc) + : send_side_cc_(send_side_cc) { + RTC_DCHECK(send_side_cc); + } + + PacketRouter* packet_router() override { return &packet_router_; } + + SendSideCongestionController* send_side_cc() override { + return send_side_cc_; + } + + TransportFeedbackObserver* transport_feedback_observer() override { + return send_side_cc_; + } + + RtpPacketSender* packet_sender() override { return send_side_cc_->pacer(); } + + private: + PacketRouter packet_router_; + SendSideCongestionController* send_side_cc_; +}; + +} // namespace webrtc + +#endif // WEBRTC_CALL_FAKE_RTP_TRANSPORT_CONTROLLER_SEND_H_ diff --git a/webrtc/call/rtp_transport_controller_send.cc b/webrtc/call/rtp_transport_controller_send.cc index a006f289f6..1040218c03 100644 --- a/webrtc/call/rtp_transport_controller_send.cc +++ b/webrtc/call/rtp_transport_controller_send.cc @@ -18,10 +18,4 @@ RtpTransportControllerSend::RtpTransportControllerSend( : send_side_cc_(clock, nullptr /* observer */, event_log, &packet_router_) { } -void RtpTransportControllerSend::RegisterNetworkObserver( - SendSideCongestionController::Observer* observer) { - // Must be called only once. - send_side_cc_.RegisterNetworkObserver(observer); -} - } // namespace webrtc diff --git a/webrtc/call/rtp_transport_controller_send.h b/webrtc/call/rtp_transport_controller_send.h index 2bbb547ffd..b626fa887e 100644 --- a/webrtc/call/rtp_transport_controller_send.h +++ b/webrtc/call/rtp_transport_controller_send.h @@ -26,9 +26,6 @@ class RtpTransportControllerSend : public RtpTransportControllerSendInterface { public: RtpTransportControllerSend(Clock* clock, webrtc::RtcEventLog* event_log); - void RegisterNetworkObserver( - SendSideCongestionController::Observer* observer); - // Implements RtpTransportControllerSendInterface PacketRouter* packet_router() override { return &packet_router_; } SendSideCongestionController* send_side_cc() override { diff --git a/webrtc/modules/congestion_controller/BUILD.gn b/webrtc/modules/congestion_controller/BUILD.gn index 3dcbb27b24..352d41083d 100644 --- a/webrtc/modules/congestion_controller/BUILD.gn +++ b/webrtc/modules/congestion_controller/BUILD.gn @@ -85,6 +85,7 @@ if (rtc_include_tests) { ] deps = [ ":congestion_controller", + ":mock_congestion_controller", "../../base:rtc_base_approved", "../../system_wrappers:system_wrappers", "../../test:field_trial", @@ -101,4 +102,16 @@ if (rtc_include_tests) { suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] } } + + rtc_source_set("mock_congestion_controller") { + testonly = true + sources = [ + "include/mock/mock_congestion_observer.h", + "include/mock/mock_send_side_congestion_controller.h", + ] + deps = [ + ":congestion_controller", + "../../test:test_support", + ] + } } diff --git a/webrtc/modules/congestion_controller/include/congestion_controller.h b/webrtc/modules/congestion_controller/include/congestion_controller.h index 0e69047b9a..00eb0d7f41 100644 --- a/webrtc/modules/congestion_controller/include/congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/congestion_controller.h @@ -58,7 +58,7 @@ class CongestionController : public CallStatsObserver, receive_side_cc_(clock, packet_router) {} CongestionController(const Clock* clock, Observer* observer, - RemoteBitrateObserver* remote_bitrate_observer, + RemoteBitrateObserver* /* remote_bitrate_observer */, RtcEventLog* event_log, PacketRouter* packet_router, std::unique_ptr pacer) diff --git a/webrtc/modules/congestion_controller/include/mock/mock_congestion_observer.h b/webrtc/modules/congestion_controller/include/mock/mock_congestion_observer.h index 7b4c7f2ecf..6b14aec5c7 100644 --- a/webrtc/modules/congestion_controller/include/mock/mock_congestion_observer.h +++ b/webrtc/modules/congestion_controller/include/mock/mock_congestion_observer.h @@ -11,9 +11,6 @@ #ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_INCLUDE_MOCK_MOCK_CONGESTION_OBSERVER_H_ #define WEBRTC_MODULES_CONGESTION_CONTROLLER_INCLUDE_MOCK_MOCK_CONGESTION_OBSERVER_H_ -#include "webrtc/base/constructormagic.h" -#include "webrtc/base/socket.h" -#include "webrtc/common_types.h" #include "webrtc/modules/congestion_controller/include/congestion_controller.h" #include "webrtc/test/gmock.h" diff --git a/webrtc/modules/congestion_controller/include/mock/mock_send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/mock/mock_send_side_congestion_controller.h new file mode 100644 index 0000000000..e71ec8fff4 --- /dev/null +++ b/webrtc/modules/congestion_controller/include/mock/mock_send_side_congestion_controller.h @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2017 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef WEBRTC_MODULES_CONGESTION_CONTROLLER_INCLUDE_MOCK_MOCK_SEND_SIDE_CONGESTION_CONTROLLER_H_ +#define WEBRTC_MODULES_CONGESTION_CONTROLLER_INCLUDE_MOCK_MOCK_SEND_SIDE_CONGESTION_CONTROLLER_H_ + +#include "webrtc/modules/congestion_controller/include/send_side_congestion_controller.h" +#include "webrtc/test/gmock.h" + +namespace webrtc { +namespace test { + +class MockSendSideCongestionController : public SendSideCongestionController { + public: + MockSendSideCongestionController(const Clock* clock, + RtcEventLog* event_log, + PacketRouter* packet_router) + : SendSideCongestionController(clock, + nullptr /* observer */, + event_log, + packet_router) {} + + MOCK_METHOD3(SetBweBitrates, + void(int min_bitrate_bps, + int start_bitrate_bps, + int max_bitrate_bps)); +}; + +} // namespace test +} // namespace webrtc + +#endif // WEBRTC_MODULES_CONGESTION_CONTROLLER_INCLUDE_MOCK_MOCK_SEND_SIDE_CONGESTION_CONTROLLER_H_ diff --git a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h index 66bcb314d2..b779b13168 100644 --- a/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h +++ b/webrtc/modules/congestion_controller/include/send_side_congestion_controller.h @@ -57,7 +57,8 @@ class SendSideCongestionController : public CallStatsObserver, protected: virtual ~Observer() {} }; - // TODO(nisse): Consider deleting the |observer| argument to constructors. + // TODO(nisse): Consider deleting the |observer| argument to constructors + // once CongestionController is deleted. SendSideCongestionController(const Clock* clock, Observer* observer, RtcEventLog* event_log,