From 760fd5249488a7a8698679a026b247423caa7cce Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Wed, 22 Jan 2020 15:23:43 -0800 Subject: [PATCH] Replace MockAudioDeviceModule mock refcounting with real refcounting Bug: webrtc:11308 Change-Id: Ic55ec2c4b45f8fc709fe1348556bdeea6202e7a8 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166580 Reviewed-by: Henrik Andreassson Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#30366} --- .../null_webrtc_video_engine_unittest.cc | 5 +- media/engine/webrtc_voice_engine_unittest.cc | 96 ++++++++++--------- modules/audio_device/BUILD.gn | 1 + .../audio_device/include/mock_audio_device.h | 13 ++- 4 files changed, 64 insertions(+), 51 deletions(-) diff --git a/media/engine/null_webrtc_video_engine_unittest.cc b/media/engine/null_webrtc_video_engine_unittest.cc index 584cafe0a6..832bf8ad1a 100644 --- a/media/engine/null_webrtc_video_engine_unittest.cc +++ b/media/engine/null_webrtc_video_engine_unittest.cc @@ -29,9 +29,10 @@ namespace cricket { TEST(NullWebRtcVideoEngineTest, CheckInterface) { std::unique_ptr task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); auto audio_engine = std::make_unique( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, webrtc::AudioProcessingBuilder().Create()); diff --git a/media/engine/webrtc_voice_engine_unittest.cc b/media/engine/webrtc_voice_engine_unittest.cc index 1d82f0d2d0..488683dbec 100644 --- a/media/engine/webrtc_voice_engine_unittest.cc +++ b/media/engine/webrtc_voice_engine_unittest.cc @@ -96,7 +96,6 @@ void AdmSetupExpectations(webrtc::test::MockAudioDeviceModule* adm) { RTC_DCHECK(adm); // Setup. - EXPECT_CALL(*adm, AddRef()).Times(3); EXPECT_CALL(*adm, Init()).WillOnce(Return(0)); EXPECT_CALL(*adm, RegisterAudioCallback(_)).WillOnce(Return(0)); #if defined(WEBRTC_WIN) @@ -135,9 +134,6 @@ void AdmSetupExpectations(webrtc::test::MockAudioDeviceModule* adm) { EXPECT_CALL(*adm, StopRecording()).WillOnce(Return(0)); EXPECT_CALL(*adm, RegisterAudioCallback(nullptr)).WillOnce(Return(0)); EXPECT_CALL(*adm, Terminate()).WillOnce(Return(0)); - EXPECT_CALL(*adm, Release()) - .Times(3) - .WillRepeatedly(Return(rtc::RefCountReleaseStatus::kDroppedLastRef)); } } // namespace @@ -145,8 +141,9 @@ void AdmSetupExpectations(webrtc::test::MockAudioDeviceModule* adm) { TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) { std::unique_ptr task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - StrictMock adm; - AdmSetupExpectations(&adm); + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateStrict(); + AdmSetupExpectations(adm); rtc::scoped_refptr> apm = new rtc::RefCountedObject< StrictMock>(); @@ -157,7 +154,7 @@ TEST(WebRtcVoiceEngineTestStubLibrary, StartupShutdown) { EXPECT_CALL(*apm, DetachAecDump()); { cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); @@ -179,12 +176,13 @@ class WebRtcVoiceEngineTestFake : public ::testing::Test { explicit WebRtcVoiceEngineTestFake(const char* field_trials) : task_queue_factory_(webrtc::CreateDefaultTaskQueueFactory()), + adm_(webrtc::test::MockAudioDeviceModule::CreateStrict()), apm_(new rtc::RefCountedObject< StrictMock>()), call_(), override_field_trials_(field_trials) { // AudioDeviceModule. - AdmSetupExpectations(&adm_); + AdmSetupExpectations(adm_); // AudioProcessing. EXPECT_CALL(*apm_, GetConfig()).WillRepeatedly(ReturnPointee(&apm_config_)); EXPECT_CALL(*apm_, ApplyConfig(_)).WillRepeatedly(SaveArg<0>(&apm_config_)); @@ -197,7 +195,7 @@ class WebRtcVoiceEngineTestFake : public ::testing::Test { auto encoder_factory = webrtc::CreateBuiltinAudioEncoderFactory(); auto decoder_factory = webrtc::CreateBuiltinAudioDecoderFactory(); engine_.reset(new cricket::WebRtcVoiceEngine( - task_queue_factory_.get(), &adm_, encoder_factory, decoder_factory, + task_queue_factory_.get(), adm_, encoder_factory, decoder_factory, nullptr, apm_)); engine_->Init(); send_parameters_.codecs.push_back(kPcmuCodec); @@ -287,9 +285,9 @@ class WebRtcVoiceEngineTestFake : public ::testing::Test { void SetSend(bool enable) { ASSERT_TRUE(channel_); if (enable) { - EXPECT_CALL(adm_, RecordingIsInitialized()).WillOnce(Return(false)); - EXPECT_CALL(adm_, Recording()).WillOnce(Return(false)); - EXPECT_CALL(adm_, InitRecording()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, RecordingIsInitialized()).WillOnce(Return(false)); + EXPECT_CALL(*adm_, Recording()).WillOnce(Return(false)); + EXPECT_CALL(*adm_, InitRecording()).WillOnce(Return(0)); EXPECT_CALL(*apm_, SetExtraOptions(::testing::_)); } channel_->SetSend(enable); @@ -776,7 +774,7 @@ class WebRtcVoiceEngineTestFake : public ::testing::Test { protected: std::unique_ptr task_queue_factory_; - StrictMock adm_; + rtc::scoped_refptr adm_; rtc::scoped_refptr> apm_; cricket::FakeCall call_; std::unique_ptr engine_; @@ -2247,7 +2245,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStatsWithMultipleSendStreams) { // Check stats for the added streams. { - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); cricket::VoiceMediaInfo info; EXPECT_EQ(true, channel_->GetStats(&info)); @@ -2267,7 +2265,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStatsWithMultipleSendStreams) { { cricket::VoiceMediaInfo info; EXPECT_TRUE(channel_->RemoveRecvStream(kSsrcY)); - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); EXPECT_EQ(true, channel_->GetStats(&info)); EXPECT_EQ(static_cast(arraysize(kSsrcs4)), info.senders.size()); EXPECT_EQ(0u, info.receivers.size()); @@ -2279,7 +2277,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStatsWithMultipleSendStreams) { cricket::VoiceMediaInfo info; DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame)); SetAudioReceiveStreamStats(); - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); EXPECT_EQ(true, channel_->GetStats(&info)); EXPECT_EQ(static_cast(arraysize(kSsrcs4)), info.senders.size()); EXPECT_EQ(1u, info.receivers.size()); @@ -2331,7 +2329,7 @@ TEST_F(WebRtcVoiceEngineTestFake, PlayoutWithMultipleStreams) { TEST_F(WebRtcVoiceEngineTestFake, TxAgcConfigViaOptions) { EXPECT_TRUE(SetupSendStream()); - EXPECT_CALL(adm_, BuiltInAGCIsAvailable()) + EXPECT_CALL(*adm_, BuiltInAGCIsAvailable()) .Times(::testing::AtLeast(1)) .WillRepeatedly(Return(false)); const auto& agc_config = apm_config_.gain_controller1; @@ -2438,7 +2436,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStats) { // Check stats for the added streams. { - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); cricket::VoiceMediaInfo info; EXPECT_EQ(true, channel_->GetStats(&info)); @@ -2454,7 +2452,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStats) { { cricket::VoiceMediaInfo info; SetSend(true); - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); EXPECT_EQ(true, channel_->GetStats(&info)); VerifyVoiceSenderInfo(info.senders[0], true); VerifyVoiceSendRecvCodecs(info); @@ -2464,7 +2462,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStats) { { cricket::VoiceMediaInfo info; EXPECT_TRUE(channel_->RemoveRecvStream(kSsrcY)); - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); EXPECT_EQ(true, channel_->GetStats(&info)); EXPECT_EQ(1u, info.senders.size()); EXPECT_EQ(0u, info.receivers.size()); @@ -2476,7 +2474,7 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStats) { cricket::VoiceMediaInfo info; DeliverPacket(kPcmuFrame, sizeof(kPcmuFrame)); SetAudioReceiveStreamStats(); - EXPECT_CALL(adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); + EXPECT_CALL(*adm_, GetPlayoutUnderrunCount()).WillOnce(Return(0)); EXPECT_EQ(true, channel_->GetStats(&info)); EXPECT_EQ(1u, info.senders.size()); EXPECT_EQ(1u, info.receivers.size()); @@ -2820,13 +2818,13 @@ TEST_F(WebRtcVoiceEngineTestFake, SetExtmapAllowMixedDisabledAsCallee) { TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { EXPECT_TRUE(SetupSendStream()); EXPECT_TRUE(AddRecvStream(kSsrcY)); - EXPECT_CALL(adm_, BuiltInAECIsAvailable()) + EXPECT_CALL(*adm_, BuiltInAECIsAvailable()) .Times(8) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, BuiltInAGCIsAvailable()) + EXPECT_CALL(*adm_, BuiltInAGCIsAvailable()) .Times(4) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, BuiltInNSIsAvailable()) + EXPECT_CALL(*adm_, BuiltInNSIsAvailable()) .Times(2) .WillRepeatedly(Return(false)); @@ -2910,20 +2908,20 @@ TEST_F(WebRtcVoiceEngineTestFake, SetAudioOptions) { TEST_F(WebRtcVoiceEngineTestFake, SetOptionOverridesViaChannels) { EXPECT_TRUE(SetupSendStream()); - EXPECT_CALL(adm_, BuiltInAECIsAvailable()) + EXPECT_CALL(*adm_, BuiltInAECIsAvailable()) .Times(8) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, BuiltInAGCIsAvailable()) + EXPECT_CALL(*adm_, BuiltInAGCIsAvailable()) .Times(8) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, BuiltInNSIsAvailable()) + EXPECT_CALL(*adm_, BuiltInNSIsAvailable()) .Times(8) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, RecordingIsInitialized()) + EXPECT_CALL(*adm_, RecordingIsInitialized()) .Times(2) .WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, Recording()).Times(2).WillRepeatedly(Return(false)); - EXPECT_CALL(adm_, InitRecording()).Times(2).WillRepeatedly(Return(0)); + EXPECT_CALL(*adm_, Recording()).Times(2).WillRepeatedly(Return(false)); + EXPECT_CALL(*adm_, InitRecording()).Times(2).WillRepeatedly(Return(0)); EXPECT_CALL(*apm_, SetExtraOptions(::testing::_)).Times(10); std::unique_ptr channel1( @@ -3452,11 +3450,12 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) { // we never want it to create a decoder at this stage. std::unique_ptr task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); @@ -3477,16 +3476,15 @@ TEST(WebRtcVoiceEngineTest, StartupShutdown) { TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { std::unique_ptr task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - ::testing::NiceMock adm; - EXPECT_CALL(adm, AddRef()).Times(3); - EXPECT_CALL(adm, Release()) - .Times(3) - .WillRepeatedly(Return(rtc::RefCountReleaseStatus::kDroppedLastRef)); + rtc::scoped_refptr>> + adm(new rtc::RefCountedObject< + ::testing::NiceMock>()); { rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); @@ -3502,6 +3500,8 @@ TEST(WebRtcVoiceEngineTest, StartupShutdownWithExternalADM) { EXPECT_TRUE(channel != nullptr); delete channel; } + // The engine/channel should have dropped their references. + EXPECT_TRUE(adm->HasOneRef()); } // Verify the payload id of common audio codecs, including CN, ISAC, and G722. @@ -3510,11 +3510,12 @@ TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) { webrtc::CreateDefaultTaskQueueFactory(); // TODO(ossu): Why are the payload types of codecs with non-static payload // type assignments checked here? It shouldn't really matter. - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); @@ -3558,11 +3559,12 @@ TEST(WebRtcVoiceEngineTest, HasCorrectPayloadTypeMapping) { TEST(WebRtcVoiceEngineTest, Has32Channels) { std::unique_ptr task_queue_factory = webrtc::CreateDefaultTaskQueueFactory(); - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::MockAudioDecoderFactory::CreateUnusedFactory(), nullptr, apm); engine.Init(); @@ -3603,11 +3605,12 @@ TEST(WebRtcVoiceEngineTest, SetRecvCodecs) { // what we sent in - though it's probably reasonable to expect so, if // SetRecvParameters returns true. // I think it will become clear once audio decoder injection is completed. - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); cricket::WebRtcVoiceEngine engine( - task_queue_factory.get(), &adm, + task_queue_factory.get(), adm, webrtc::MockAudioEncoderFactory::CreateUnusedFactory(), webrtc::CreateBuiltinAudioDecoderFactory(), nullptr, apm); engine.Init(); @@ -3651,11 +3654,12 @@ TEST(WebRtcVoiceEngineTest, CollectRecvCodecs) { new rtc::RefCountedObject; EXPECT_CALL(*mock_decoder_factory.get(), GetSupportedDecoders()) .WillOnce(Return(specs)); - ::testing::NiceMock adm; + rtc::scoped_refptr adm = + webrtc::test::MockAudioDeviceModule::CreateNice(); rtc::scoped_refptr apm = webrtc::AudioProcessingBuilder().Create(); - cricket::WebRtcVoiceEngine engine(task_queue_factory.get(), &adm, + cricket::WebRtcVoiceEngine engine(task_queue_factory.get(), adm, unused_encoder_factory, mock_decoder_factory, nullptr, apm); engine.Init(); diff --git a/modules/audio_device/BUILD.gn b/modules/audio_device/BUILD.gn index de57db0e00..e5e23ce095 100644 --- a/modules/audio_device/BUILD.gn +++ b/modules/audio_device/BUILD.gn @@ -355,6 +355,7 @@ rtc_source_set("mock_audio_device") { ":audio_device", ":audio_device_buffer", ":audio_device_impl", + "../../rtc_base:refcount", "../../test:test_support", ] } diff --git a/modules/audio_device/include/mock_audio_device.h b/modules/audio_device/include/mock_audio_device.h index 8f9d9b61db..a05e64e6c9 100644 --- a/modules/audio_device/include/mock_audio_device.h +++ b/modules/audio_device/include/mock_audio_device.h @@ -14,6 +14,7 @@ #include #include "modules/audio_device/include/audio_device.h" +#include "rtc_base/ref_counted_object.h" #include "test/gmock.h" namespace webrtc { @@ -21,9 +22,15 @@ namespace test { class MockAudioDeviceModule : public AudioDeviceModule { public: - // RefCounted - MOCK_CONST_METHOD0(AddRef, void()); - MOCK_CONST_METHOD0(Release, rtc::RefCountReleaseStatus()); + static rtc::scoped_refptr CreateNice() { + return new rtc::RefCountedObject< + ::testing::NiceMock>(); + } + static rtc::scoped_refptr CreateStrict() { + return new rtc::RefCountedObject< + ::testing::StrictMock>(); + } + // AudioDeviceModule. MOCK_CONST_METHOD1(ActiveAudioLayer, int32_t(AudioLayer* audioLayer)); MOCK_METHOD1(RegisterAudioCallback, int32_t(AudioTransport* audioCallback));