From f84c1d6644923251ee9ca2ed6723df86c3baa999 Mon Sep 17 00:00:00 2001 From: jtteh Date: Fri, 21 Apr 2017 13:56:39 -0700 Subject: [PATCH] Don't call unconfigureWebRTCSession if configureWebRTCSession fails. Otherwise, the activeCount will become negative. BUG=webrtc:7471 Review-Url: https://codereview.webrtc.org/2822233002 Cr-Commit-Position: refs/heads/master@{#17816} --- webrtc/modules/audio_device/BUILD.gn | 1 + .../audio_device/ios/audio_device_ios.h | 2 +- .../audio_device/ios/audio_device_ios.mm | 15 +++-- .../audio_device/ios/objc/RTCAudioSession.mm | 2 +- .../ios/objc/RTCAudioSessionTest.mm | 65 +++++++++++++++++++ 5 files changed, 78 insertions(+), 7 deletions(-) diff --git a/webrtc/modules/audio_device/BUILD.gn b/webrtc/modules/audio_device/BUILD.gn index 3fa06b5139..46f02821a2 100644 --- a/webrtc/modules/audio_device/BUILD.gn +++ b/webrtc/modules/audio_device/BUILD.gn @@ -295,6 +295,7 @@ if (rtc_include_tests) { if (target_cpu != "x64") { sources += [ "ios/audio_device_unittest_ios.cc" ] } + deps += [ "//third_party/ocmock" ] } if (!build_with_chromium && is_clang) { # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). diff --git a/webrtc/modules/audio_device/ios/audio_device_ios.h b/webrtc/modules/audio_device/ios/audio_device_ios.h index 5aa3b70e68..01dd7cd930 100644 --- a/webrtc/modules/audio_device/ios/audio_device_ios.h +++ b/webrtc/modules/audio_device/ios/audio_device_ios.h @@ -206,7 +206,7 @@ class AudioDeviceIOS : public AudioDeviceGeneric, void UpdateAudioUnit(bool can_play_or_record); // Configures the audio session for WebRTC. - void ConfigureAudioSession(); + bool ConfigureAudioSession(); // Unconfigures the audio session. void UnconfigureAudioSession(); diff --git a/webrtc/modules/audio_device/ios/audio_device_ios.mm b/webrtc/modules/audio_device/ios/audio_device_ios.mm index 6a1cd04fe0..ba4fe2a963 100644 --- a/webrtc/modules/audio_device/ios/audio_device_ios.mm +++ b/webrtc/modules/audio_device/ios/audio_device_ios.mm @@ -751,19 +751,24 @@ void AudioDeviceIOS::UpdateAudioUnit(bool can_play_or_record) { } } -void AudioDeviceIOS::ConfigureAudioSession() { +bool AudioDeviceIOS::ConfigureAudioSession() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); RTCLog(@"Configuring audio session."); if (has_configured_session_) { RTCLogWarning(@"Audio session already configured."); - return; + return false; } RTCAudioSession* session = [RTCAudioSession sharedInstance]; [session lockForConfiguration]; - [session configureWebRTCSession:nil]; + bool success = [session configureWebRTCSession:nil]; [session unlockForConfiguration]; - has_configured_session_ = true; - RTCLog(@"Configured audio session."); + if (success) { + has_configured_session_ = true; + RTCLog(@"Configured audio session."); + } else { + RTCLog(@"Failed to configure audio session."); + } + return success; } void AudioDeviceIOS::UnconfigureAudioSession() { diff --git a/webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm b/webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm index f4530167f9..84cc6da4e4 100644 --- a/webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm +++ b/webrtc/modules/audio_device/ios/objc/RTCAudioSession.mm @@ -688,7 +688,7 @@ NSInteger const kRTCAudioSessionErrorConfiguration = -2; if (![self setConfiguration:webRTCConfig active:YES error:&error]) { RTCLogError(@"Failed to set WebRTC audio configuration: %@", error.localizedDescription); - [self unconfigureWebRTCSession:nil]; + // Do not call setActive:NO if setActive:YES failed. if (outError) { *outError = error; } diff --git a/webrtc/modules/audio_device/ios/objc/RTCAudioSessionTest.mm b/webrtc/modules/audio_device/ios/objc/RTCAudioSessionTest.mm index 3f55077ed2..a4b49fc1c0 100644 --- a/webrtc/modules/audio_device/ios/objc/RTCAudioSessionTest.mm +++ b/webrtc/modules/audio_device/ios/objc/RTCAudioSessionTest.mm @@ -9,11 +9,13 @@ */ #import +#import #include "webrtc/test/gtest.h" #import "webrtc/modules/audio_device/ios/objc/RTCAudioSession.h" #import "webrtc/modules/audio_device/ios/objc/RTCAudioSession+Private.h" +#import "webrtc/modules/audio_device/ios/objc/RTCAudioSessionConfiguration.h" @interface RTCAudioSessionTestDelegate : NSObject @end @@ -185,6 +187,65 @@ EXPECT_EQ(0, audioSession.activationCount); } +// Hack - fixes OCMVerify link error +// Link error is: Undefined symbols for architecture i386: +// "OCMMakeLocation(objc_object*, char const*, int)", referenced from: +// -[RTCAudioSessionTest testConfigureWebRTCSession] in RTCAudioSessionTest.o +// ld: symbol(s) not found for architecture i386 +// REASON: https://github.com/erikdoe/ocmock/issues/238 +OCMLocation *OCMMakeLocation(id testCase, const char *fileCString, int line){ + return [OCMLocation locationWithTestCase:testCase + file:[NSString stringWithUTF8String:fileCString] + line:line]; +} + +- (void)testConfigureWebRTCSession { + NSError *error = nil; + + void (^setActiveBlock)(NSInvocation *invocation) = ^(NSInvocation *invocation) { + __autoreleasing NSError **retError; + [invocation getArgument:&retError atIndex:4]; + *retError = [NSError errorWithDomain:@"AVAudioSession" + code:AVAudioSessionErrorInsufficientPriority + userInfo:nil]; + BOOL failure = NO; + [invocation setReturnValue:&failure]; + }; + + id mockAVAudioSession = OCMPartialMock([AVAudioSession sharedInstance]); + OCMStub([[mockAVAudioSession ignoringNonObjectArgs] + setActive:YES withOptions:0 error:((NSError __autoreleasing **)[OCMArg anyPointer])]). + andDo(setActiveBlock); + + id mockAudioSession = OCMPartialMock([RTCAudioSession sharedInstance]); + OCMStub([mockAudioSession session]).andReturn(mockAVAudioSession); + + RTCAudioSession *audioSession = mockAudioSession; + EXPECT_EQ(0, audioSession.activationCount); + [audioSession lockForConfiguration]; + EXPECT_TRUE([audioSession checkLock:nil]); + // configureWebRTCSession is forced to fail in the above mock interface, + // so activationCount should remain 0 + OCMExpect([[mockAVAudioSession ignoringNonObjectArgs] + setActive:YES withOptions:0 error:((NSError __autoreleasing **)[OCMArg anyPointer])]). + andDo(setActiveBlock); + OCMExpect([mockAudioSession session]).andReturn(mockAVAudioSession); + EXPECT_FALSE([audioSession configureWebRTCSession:&error]); + EXPECT_EQ(0, audioSession.activationCount); + + id session = audioSession.session; + EXPECT_EQ(session, mockAVAudioSession); + EXPECT_EQ(NO, [mockAVAudioSession setActive:YES withOptions:0 error:&error]); + [audioSession unlockForConfiguration]; + + OCMVerify([mockAudioSession session]); + OCMVerify([[mockAVAudioSession ignoringNonObjectArgs] setActive:YES withOptions:0 error:&error]); + OCMVerify([[mockAVAudioSession ignoringNonObjectArgs] setActive:NO withOptions:0 error:&error]); + + [mockAVAudioSession stopMocking]; + [mockAudioSession stopMocking]; +} + @end namespace webrtc { @@ -229,5 +290,9 @@ TEST_F(AudioSessionTest, AudioSessionActivation) { [test testAudioSessionActivation]; } +TEST_F(AudioSessionTest, ConfigureWebRTCSession) { + RTCAudioSessionTest *test = [[RTCAudioSessionTest alloc] init]; + [test testConfigureWebRTCSession]; +} } // namespace webrtc