Merge branch 'mkirk/m71'

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>(
+            //       <OWSWindowRootViewController: 0x13de37550>
+            //     )
+            //
+            // 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<T> 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)
+        }
+    }
 }

Signal/src/views/RemoteVideoView.m

@@ -14,7 +14,7 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
-@interface RemoteVideoView () <RTCEAGLVideoViewDelegate>
+@interface RemoteVideoView () <RTCVideoViewDelegate>
 
 @property (nonatomic, readonly) __kindof UIView<RTCVideoRenderer> *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<RTCVideoRenderer>)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<NSLayoutConstraint *> *constraints = [NSMutableArray new];

ThirdParty/WebRTC

@@ -1 +1 @@
-Subproject commit ca71024b4993ba95e3e6b8d0758004cffc54ddaf
+Subproject commit aa8bee9bd6f69e388a9ca7506b8702ef8ab7f195