diff --git a/Signal/src/ViewControllers/CallViewController.swift b/Signal/src/ViewControllers/CallViewController.swift index abf8f6c02..cb2872c0b 100644 --- a/Signal/src/ViewControllers/CallViewController.swift +++ b/Signal/src/ViewControllers/CallViewController.swift @@ -156,6 +156,7 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, } deinit { + Logger.info("") NotificationCenter.default.removeObserver(self) self.proximityMonitoringManager.remove(lifetime: self) } @@ -177,7 +178,8 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, override func viewWillAppear(_ animated: Bool) { super.viewWillAppear(animated) - self.proximityMonitoringManager.add(lifetime: self) + + ensureProximityMonitoring() updateCallUI(callState: call.state) @@ -1130,4 +1132,41 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, updateLocalVideo(captureSession: localCaptureSession) updateRemoteVideoTrack(remoteVideoTrack: remoteVideoTrack) } + + // MARK: - Proximity Monitoring + + func ensureProximityMonitoring() { + if #available(iOS 11, *) { + // BUG: Adding `self` as a Weak reference to the proximityMonitoringManager results in + // the CallViewController never being deallocated, which, besides being a memory leak + // can interfere with subsequent video capture - presumably because the old capture + // session is still retained via the callViewController.localVideoView. + // + // A code audit has not revealed a retain cycle. + // + // Using the XCode memory debugger shows that a strong reference is held by + // windowManager.callNavigationController->_childViewControllers. + // Even though, when inspecting via the debugger, the CallViewController is not shown as + // a childViewController. + // + // (lldb) po [[[OWSWindowManager sharedManager] callNavigationController] childViewControllers] + // <__NSSingleObjectArrayI 0x1c0418bd0>( + // + // ) + // + // Weirder still, when presenting another CallViewController, the old one remains unallocated + // and inspecting it in the memory debugger shows _no_ strong references to it (yet it + // is not deallocated). Some weak references do remain - from the proximityMonitoringManager + // and the callObserver, both of which use the Weak struct, which could be related. + // + // In any case, we can apparently avoid this behavior by not adding self as a Weak lifetime + // and as of iOS11, the system automatically managages proximityMonitoring + // via CallKit and AudioSessions. Proximity monitoring will be enabled whenever a call + // is active, unless we switch to VideoChat audio mode (which is actually desirable + // behavior), so the proximityMonitoringManager is redundant for calls on iOS11+. + } else { + // before iOS11, manually enable proximityMonitoring while we're on a call. + self.proximityMonitoringManager.add(lifetime: self) + } + } } diff --git a/Signal/src/views/RemoteVideoView.m b/Signal/src/views/RemoteVideoView.m index 018d35979..f0926155d 100644 --- a/Signal/src/views/RemoteVideoView.m +++ b/Signal/src/views/RemoteVideoView.m @@ -14,7 +14,7 @@ NS_ASSUME_NONNULL_BEGIN -@interface RemoteVideoView () +@interface RemoteVideoView () @property (nonatomic, readonly) __kindof UIView *videoRenderer; @@ -35,7 +35,7 @@ NS_ASSUME_NONNULL_BEGIN _remoteVideoConstraints = @[]; // Currently RTC only supports metal on 64bit machines -#if defined(RTC_SUPPORTS_METAL) +#if defined(__arm64__) // On 64-bit, iOS9+: uses the MetalKit backed view for improved battery/rendering performance. if (_videoRenderer == nil) { @@ -59,9 +59,6 @@ NS_ASSUME_NONNULL_BEGIN } } } -#elif defined(__arm64__) - // Canary incase the upstream RTC_SUPPORTS_METAL macro changes semantics - OWSFailDebug(@"should only use legacy video view on 32bit systems"); #endif // On 32-bit iOS9+ systems, use the legacy EAGL backed view. @@ -89,11 +86,18 @@ NS_ASSUME_NONNULL_BEGIN [self.videoRenderer setSize:size]; } -#pragma mark - RTCEAGLVideoViewDelegate +#pragma mark - RTCVideoViewDelegate -- (void)videoView:(RTCEAGLVideoView *)videoView didChangeVideoSize:(CGSize)remoteVideoSize +- (void)videoView:(id)videoRenderer didChangeVideoSize:(CGSize)remoteVideoSize { OWSAssertIsOnMainThread(); + + if (![videoRenderer isKindOfClass:[RTCEAGLVideoView class]]) { + OWSFailDebug(@"Unexpected videoRenderer: %@", videoRenderer); + return; + } + RTCEAGLVideoView *videoView = (RTCEAGLVideoView *)videoRenderer; + if (remoteVideoSize.height <= 0) { OWSFailDebug(@"Illegal video height: %f", remoteVideoSize.height); return; @@ -112,11 +116,6 @@ NS_ASSUME_NONNULL_BEGIN return; } - if (![self.videoRenderer isKindOfClass:[RTCEAGLVideoView class]]) { - OWSFailDebug(@"Unexpected video renderer: %@", self.videoRenderer); - return; - } - [NSLayoutConstraint deactivateConstraints:self.remoteVideoConstraints]; NSMutableArray *constraints = [NSMutableArray new]; diff --git a/ThirdParty/WebRTC b/ThirdParty/WebRTC index ca71024b4..aa8bee9bd 160000 --- a/ThirdParty/WebRTC +++ b/ThirdParty/WebRTC @@ -1 +1 @@ -Subproject commit ca71024b4993ba95e3e6b8d0758004cffc54ddaf +Subproject commit aa8bee9bd6f69e388a9ca7506b8702ef8ab7f195