From 87ed662116ebc5078de67f7c165afc547672db02 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Thu, 19 Jan 2017 09:38:50 -0500 Subject: [PATCH] Persist AudioService if CallViewController is dismissed ...in response to CR, move the AudioService off of the CallViewController Adopt multiple observer pattern vs. a singular delegate. Doing so required implementing some machinery to address the ARC (see: Weak.swift) // FREEBIE --- Signal.xcodeproj/project.pbxproj | 4 ++ Signal/src/call/CallAudioService.swift | 45 ++++++++++------ Signal/src/call/CallService.swift | 1 + Signal/src/call/SignalCall.swift | 52 ++++++++++++++++--- .../call/UserInterface/CallUIAdapter.swift | 15 ++++++ Signal/src/util/Weak.swift | 29 +++++++++++ .../view controllers/CallViewController.swift | 24 +++++---- 7 files changed, 139 insertions(+), 31 deletions(-) create mode 100644 Signal/src/util/Weak.swift diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index ba9e295dc..c44ef0761 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -104,6 +104,7 @@ 45F170AF1E2F0393003FC1F2 /* CallAudioSessionTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170AE1E2F0393003FC1F2 /* CallAudioSessionTest.swift */; }; 45F170BB1E2FC5D3003FC1F2 /* CallAudioService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */; }; 45F170BC1E2FC5D3003FC1F2 /* CallAudioService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */; }; + 45F170D61E315310003FC1F2 /* Weak.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45F170D51E315310003FC1F2 /* Weak.swift */; }; 45F2B1941D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m in Sources */ = {isa = PBXBuildFile; fileRef = 45F2B1931D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m */; }; 45F2B1971D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 45F2B1951D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib */; }; 45F2B1981D9CA207000D2C69 /* OWSOutgoingMessageCollectionViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = 45F2B1961D9CA207000D2C69 /* OWSOutgoingMessageCollectionViewCell.xib */; }; @@ -699,6 +700,7 @@ 45F170AE1E2F0393003FC1F2 /* CallAudioSessionTest.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; name = CallAudioSessionTest.swift; path = test/call/CallAudioSessionTest.swift; sourceTree = ""; }; 45F170B31E2F0A6A003FC1F2 /* RTCAudioSession.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RTCAudioSession.h; sourceTree = ""; }; 45F170BA1E2FC5D3003FC1F2 /* CallAudioService.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CallAudioService.swift; sourceTree = ""; }; + 45F170D51E315310003FC1F2 /* Weak.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Weak.swift; sourceTree = ""; }; 45F2B1921D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = OWSOutgoingMessageCollectionViewCell.h; sourceTree = ""; }; 45F2B1931D9C9F48000D2C69 /* OWSOutgoingMessageCollectionViewCell.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = OWSOutgoingMessageCollectionViewCell.m; sourceTree = ""; }; 45F2B1951D9CA207000D2C69 /* OWSIncomingMessageCollectionViewCell.xib */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.xib; path = OWSIncomingMessageCollectionViewCell.xib; sourceTree = ""; }; @@ -1948,6 +1950,7 @@ 45CD81F01DC03A22004C9430 /* OWSLogger.h */, 45CD81F11DC03A22004C9430 /* OWSLogger.m */, 450DF2041E0D74AC003D14BE /* Platform.swift */, + 45F170D51E315310003FC1F2 /* Weak.swift */, ); path = util; sourceTree = ""; @@ -3220,6 +3223,7 @@ BFB074C719A5611000F2947C /* FutureUtil.m in Sources */, 45E1F3A51DEF20A100852CF1 /* NoSignalContactsView.swift in Sources */, FCD274E21A5AFD8000202277 /* PrivacySettingsTableViewController.m in Sources */, + 45F170D61E315310003FC1F2 /* Weak.swift in Sources */, 76EB057218170B33006006FC /* RecentCall.m in Sources */, B97CBFA818860EA3008E0DE9 /* CountryCodeViewController.m in Sources */, B6B1013C196D213F007E3930 /* SignalKeyingStorage.m in Sources */, diff --git a/Signal/src/call/CallAudioService.swift b/Signal/src/call/CallAudioService.swift index 4e80bd1df..70cd7ab95 100644 --- a/Signal/src/call/CallAudioService.swift +++ b/Signal/src/call/CallAudioService.swift @@ -4,7 +4,8 @@ import Foundation -@objc class CallAudioService: NSObject { +@objc class CallAudioService: NSObject, CallObserver { + private let TAG = "[CallAudioService]" private var vibrateTimer: Timer? private let soundPlayer = JSQSystemSoundPlayer.shared()! @@ -21,21 +22,41 @@ import Foundation // `pulseDuration` is the small pause between the two vibrations in the pair. private let pulseDuration = 0.2 - public var isSpeakerphoneEnabled = false { - didSet { - handleUpdatedSpeakerphone() - } - } - // MARK: - Initializers init(handleRinging: Bool) { self.handleRinging = handleRinging } + // MARK: - CallObserver + + internal func stateDidChange(call: SignalCall, state: CallState) { + DispatchQueue.main.async { + self.handleState(state) + } + } + + internal func muteDidChange(call: SignalCall, isMuted: Bool) { + Logger.verbose("\(TAG) in \(#function) is no-op") + } + + internal func speakerphoneDidChange(call: SignalCall, isEnabled: Bool) { + if isEnabled { + setAudioSession(category: AVAudioSessionCategoryPlayAndRecord, options: .defaultToSpeaker) + } else { + setAudioSession(category: AVAudioSessionCategoryPlayAndRecord) + } + } + + internal func hasVideoDidChange(call: SignalCall, hasVideo: Bool) { + // no-op + } + // MARK: - Service action handlers public func handleState(_ state: CallState) { + Logger.verbose("\(TAG) in \(#function) new state: \(state)") + switch state { case .idle: handleIdle() case .dialing: handleDialing() @@ -100,14 +121,6 @@ import Foundation stopRinging() } - private func handleUpdatedSpeakerphone() { - if isSpeakerphoneEnabled { - setAudioSession(category: AVAudioSessionCategoryPlayAndRecord, options: .defaultToSpeaker) - } else { - setAudioSession(category: AVAudioSessionCategoryPlayAndRecord) - } - } - // MARK: - Ringing private func startRinging() { @@ -151,7 +164,7 @@ import Foundation } } - // MARK: - AVAudioSession Mgmt + // MARK: - AVAudioSession Helpers private func setAudioSession(category: String, options: AVAudioSessionCategoryOptions) { do { diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 3d553d39a..857424b01 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -924,6 +924,7 @@ fileprivate let timeoutSeconds = 60 peerConnectionClient?.terminate() peerConnectionClient = nil + call?.removeAllObservers() call = nil thread = nil incomingCallPromise = nil diff --git a/Signal/src/call/SignalCall.swift b/Signal/src/call/SignalCall.swift index 98bb18b56..22fcabb26 100644 --- a/Signal/src/call/SignalCall.swift +++ b/Signal/src/call/SignalCall.swift @@ -17,10 +17,11 @@ enum CallState: String { case remoteBusy // terminal } -protocol CallDelegate: class { +protocol CallObserver: class { func stateDidChange(call: SignalCall, state: CallState) func hasVideoDidChange(call: SignalCall, hasVideo: Bool) func muteDidChange(call: SignalCall, isMuted: Bool) + func speakerphoneDidChange(call: SignalCall, isEnabled: Bool) } /** @@ -30,7 +31,7 @@ protocol CallDelegate: class { let TAG = "[SignalCall]" - weak var delegate: CallDelegate? + var observers = [Weak]() let remotePhoneNumber: String // Signal Service identifier for this Call. Used to coordinate the call across remote clients. @@ -38,12 +39,16 @@ protocol CallDelegate: class { // Distinguishes between calls locally, e.g. in CallKit let localId: UUID + var hasVideo = false { didSet { Logger.debug("\(TAG) hasVideo changed: \(oldValue) -> \(hasVideo)") - delegate?.hasVideoDidChange(call: self, hasVideo: hasVideo) + for observer in observers { + observer.value?.hasVideoDidChange(call: self, hasVideo: hasVideo) + } } } + var state: CallState { didSet { Logger.debug("\(TAG) state changed: \(oldValue) -> \(state)") @@ -56,20 +61,35 @@ protocol CallDelegate: class { } else { connectedDate = nil } - - delegate?.stateDidChange(call: self, state: state) + for observer in observers { + observer.value?.stateDidChange(call: self, state: state) + } } } + var isMuted = false { didSet { Logger.debug("\(TAG) muted changed: \(oldValue) -> \(isMuted)") - delegate?.muteDidChange(call: self, isMuted: isMuted) + for observer in observers { + observer.value?.muteDidChange(call: self, isMuted: isMuted) + } + } + } + + var isSpeakerphoneEnabled = false { + didSet { + Logger.debug("\(TAG) isSpeakerphoneEnabled changed: \(oldValue) -> \(isSpeakerphoneEnabled)") + for observer in observers { + observer.value?.speakerphoneDidChange(call: self, isEnabled: isSpeakerphoneEnabled) + } } } var connectedDate: NSDate? var error: CallError? + // MARK: Initializers and Factory Methods + init(localId: UUID, signalingId: UInt64, state: CallState, remotePhoneNumber: String) { self.localId = localId self.signalingId = signalingId @@ -85,7 +105,27 @@ protocol CallDelegate: class { return SignalCall(localId: localId, signalingId: signalingId, state: .answering, remotePhoneNumber: remotePhoneNumber) } + // - + + func addObserverAndSyncState(observer: CallObserver) { + observers.append(Weak(value: observer)) + + // Synchronize observer with current call state + observer.stateDidChange(call: self, state: self.state) + } + + func removeObserver(_ observer: CallObserver) { + while let index = observers.index(where: { $0.value === observer }) { + observers.remove(at: index) + } + } + + func removeAllObservers() { + observers = [] + } + // MARK: Equatable + static func == (lhs: SignalCall, rhs: SignalCall) -> Bool { return lhs.localId == rhs.localId } diff --git a/Signal/src/call/UserInterface/CallUIAdapter.swift b/Signal/src/call/UserInterface/CallUIAdapter.swift index dded773a2..030b82405 100644 --- a/Signal/src/call/UserInterface/CallUIAdapter.swift +++ b/Signal/src/call/UserInterface/CallUIAdapter.swift @@ -42,6 +42,7 @@ class CallUIAdapter { let TAG = "[CallUIAdapter]" private let adaptee: CallUIAdaptee private let contactsManager: OWSContactsManager + private let audioService: CallAudioService required init(callService: CallService, contactsManager: OWSContactsManager, notificationsAdapter: CallNotificationsAdapter) { self.contactsManager = contactsManager @@ -58,9 +59,13 @@ class CallUIAdapter { Logger.info("\(TAG) choosing non-callkit adaptee for older iOS") adaptee = NonCallKitCallUIAdaptee(callService: callService, notificationsAdapter: notificationsAdapter) } + + audioService = CallAudioService(handleRinging: adaptee.hasManualRinger) } internal func reportIncomingCall(_ call: SignalCall, thread: TSContactThread) { + call.addObserverAndSyncState(observer: audioService) + let callerName = self.contactsManager.displayName(forPhoneIdentifier: call.remotePhoneNumber) adaptee.reportIncomingCall(call, callerName: callerName) } @@ -72,6 +77,8 @@ class CallUIAdapter { internal func startOutgoingCall(handle: String) -> SignalCall { let call = SignalCall.outgoingCall(localId: UUID(), remotePhoneNumber: handle) + call.addObserverAndSyncState(observer: audioService) + adaptee.startOutgoingCall(call) return call } @@ -97,6 +104,7 @@ class CallUIAdapter { } internal func setIsMuted(call: SignalCall, isMuted: Bool) { + // With CallKit, muting is handled by a CXAction, so it must go through the adaptee adaptee.setIsMuted(call: call, isMuted: isMuted) } @@ -104,6 +112,13 @@ class CallUIAdapter { adaptee.setHasVideo(call: call, hasVideo: hasVideo) } + internal func toggleSpeakerphone(call: SignalCall, isEnabled: Bool) { + // Speakerphone is not handled by CallKit (e.g. there is no CXAction), so we handle it w/o going through the + // adaptee, relying on the AudioService CallObserver to put the system in a state consistent with the call's + // assigned property. + call.isSpeakerphoneEnabled = isEnabled + } + // CallKit handles ringing state on it's own. But for non-call kit we trigger ringing start/stop manually. internal var hasManualRinger: Bool { return adaptee.hasManualRinger diff --git a/Signal/src/util/Weak.swift b/Signal/src/util/Weak.swift new file mode 100644 index 000000000..cefa890f6 --- /dev/null +++ b/Signal/src/util/Weak.swift @@ -0,0 +1,29 @@ +// +// Copyright © 2017 Open Whisper Systems. All rights reserved. +// + +/** + * Container for a weakly referenced object. + * + * Only use this for |T| with reference-semantic entities + * e.g. inheriting from AnyObject or Class-only protocols, but not structs or enums. + * + * + * Based on https://devforums.apple.com/message/981472#981472, but also supports class-only protocols + */ +struct Weak { + private weak var _value: AnyObject? + + var value: T? { + get { + return _value as? T + } + set { + _value = newValue as AnyObject + } + } + + init(value: T) { + self.value = value + } +} diff --git a/Signal/src/view controllers/CallViewController.swift b/Signal/src/view controllers/CallViewController.swift index e66336d3c..ce8da785b 100644 --- a/Signal/src/view controllers/CallViewController.swift +++ b/Signal/src/view controllers/CallViewController.swift @@ -10,7 +10,7 @@ import PromiseKit // TODO: Add logic to button handlers. // TODO: Ensure buttons enabled & disabled as necessary. @objc(OWSCallViewController) -class CallViewController: UIViewController, CallDelegate { +class CallViewController: UIViewController, CallObserver { enum CallDirection { case unspecified, outgoing, incoming @@ -22,7 +22,6 @@ class CallViewController: UIViewController, CallDelegate { let callUIAdapter: CallUIAdapter let contactsManager: OWSContactsManager - let audioService: CallAudioService // MARK: Properties @@ -81,7 +80,6 @@ class CallViewController: UIViewController, CallDelegate { contactsManager = Environment.getCurrent().contactsManager let callService = Environment.getCurrent().callService! callUIAdapter = callService.callUIAdapter - audioService = CallAudioService(handleRinging: callUIAdapter.hasManualRinger) super.init(coder: aDecoder) } @@ -89,7 +87,6 @@ class CallViewController: UIViewController, CallDelegate { contactsManager = Environment.getCurrent().contactsManager let callService = Environment.getCurrent().callService! callUIAdapter = callService.callUIAdapter - audioService = CallAudioService(handleRinging: callUIAdapter.hasManualRinger) super.init(nibName: nil, bundle: nil) } @@ -133,8 +130,8 @@ class CallViewController: UIViewController, CallDelegate { // No-op, since call service is already set up at this point, the result of which was presenting this viewController. } - call.delegate = self - stateDidChange(call: call, state: call.state) + // Subscribe for future call updates + call.addObserverAndSyncState(observer: self) } func createViews() { @@ -488,7 +485,11 @@ class CallViewController: UIViewController, CallDelegate { func didPressSpeakerphone(sender speakerphoneButton: UIButton) { Logger.info("\(TAG) called \(#function)") speakerphoneButton.isSelected = !speakerphoneButton.isSelected - audioService.isSpeakerphoneEnabled = speakerphoneButton.isSelected + if let call = self.call { + callUIAdapter.toggleSpeakerphone(call: call, isEnabled: speakerphoneButton.isSelected) + } else { + Logger.warn("\(TAG) pressed mute, but call was unexpectedly nil") + } } func didPressTextMessage(sender speakerphoneButton: UIButton) { @@ -537,14 +538,13 @@ class CallViewController: UIViewController, CallDelegate { self.dismiss(animated: true) } - // MARK: - CallDelegate + // MARK: - CallObserver internal func stateDidChange(call: SignalCall, state: CallState) { DispatchQueue.main.async { Logger.info("\(self.TAG) new call status: \(state)") self.updateCallUI(callState: state) } - self.audioService.handleState(state) } internal func hasVideoDidChange(call: SignalCall, hasVideo: Bool) { @@ -558,4 +558,10 @@ class CallViewController: UIViewController, CallDelegate { self.updateCallUI(callState: call.state) } } + + internal func speakerphoneDidChange(call: SignalCall, isEnabled: Bool) { + DispatchQueue.main.async { + self.updateCallUI(callState: call.state) + } + } }