Fixes #2878 - Retain cycles on the ElementCall webView and correctly …

…tear down the call on dismissal.
element-hq · May 24, 2024 · f8d0945 · f8d0945
1 parent 8bfd802
commit f8d0945
Show file tree

Hide file tree

Showing 11 changed files with 77 additions and 21 deletions.
diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj
@@ -296,6 +296,7 @@
 		4799A852132F1744E2825994 /* CreateRoomViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 340179A0FC1AD4AEDA7FC134 /* CreateRoomViewModelProtocol.swift */; };
 		47FF70C051A991FB65CDBCF3 /* RoomScreenInteractionHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0135A608FFAD86E6674EE730 /* RoomScreenInteractionHandler.swift */; };
 		4807E8F51DB54F56B25E1C7E /* AppLockSetupSettingsScreenViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D8C38663020DF2EB2D13F5E /* AppLockSetupSettingsScreenViewModel.swift */; };
+		48416BBEB8DDF3E4DED0EDB6 /* ElementCallServiceProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FC8B21E86B137BE4E91F82A /* ElementCallServiceProtocol.swift */; };
 		484202C5D50983442D24D061 /* AttributedString.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52BD6ED18E2EB61E28C340AD /* AttributedString.swift */; };
 		489BB6A733D3DA0FE7062650 /* IdentityConfirmationScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7C71B9802433F1B4252291BB /* IdentityConfirmationScreenViewModelProtocol.swift */; };
 		491D62ACD19E6F134B1766AF /* RoomNotificationSettingsUserDefinedScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3203C6566DC17B7AECC1B7FD /* RoomNotificationSettingsUserDefinedScreen.swift */; };
@@ -361,6 +362,7 @@
 		565868808A1DA565707394ED /* CurrentValuePublisher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 127C8472672A5BA09EF1ACF8 /* CurrentValuePublisher.swift */; };
 		56F0A22972A3BB519DA2261C /* HomeScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 24F5530B2212862FA4BEFF2D /* HomeScreenViewModelProtocol.swift */; };
 		5710AAB27D5D866292C1FE06 /* SessionVerificationScreenModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = AF848B41DAF1066F3054D4A1 /* SessionVerificationScreenModels.swift */; };
+		5732395A4F71F51F9C754C5A /* ElementCallService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 33AE897D86784CCA5E4E9227 /* ElementCallService.swift */; };
 		5780E444F405AA1304E1C23E /* DeveloperOptionsScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = 38E521D6C2BF8DF0DFB35146 /* DeveloperOptionsScreen.swift */; };
 		57E115A8C33E599DE564F8C3 /* TimelineView.swift in Sources */ = {isa = PBXBuildFile; fileRef = BDEB27575FEBCF414D4DEE31 /* TimelineView.swift */; };
 		588411C8FD72B2A2DFE5F7DE /* XCUIElement.swift in Sources */ = {isa = PBXBuildFile; fileRef = E992D7B8BE54B2AB454613AF /* XCUIElement.swift */; };
@@ -441,6 +443,7 @@
 		6AECC84BE14A13440120FED8 /* NSESettingsProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9FB4F169D653296023ED65E6 /* NSESettingsProtocol.swift */; };
 		6B05AA5D9BBCD6D8D63B80EB /* TimelineItemAccessibilityModifier.swift in Sources */ = {isa = PBXBuildFile; fileRef = 74C6F3DAD167F972702C8893 /* TimelineItemAccessibilityModifier.swift */; };
 		6B31508C6334C617360C2EAB /* RoomMemberDetailsViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EC589E641AE46EFB2962534D /* RoomMemberDetailsViewModelTests.swift */; };
+		6B67AC7AA41136FC9804C136 /* ElementCallServiceProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6FC8B21E86B137BE4E91F82A /* ElementCallServiceProtocol.swift */; };
 		6BAD956B909A6E29F6CC6E7C /* ButtonStyle.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8CC23C63849452BC86EA2852 /* ButtonStyle.swift */; };
 		6BB6944443C421C722ED1E7D /* portrait_test_video.mp4 in Resources */ = {isa = PBXBuildFile; fileRef = F2D513D2477B57F90E98EEC0 /* portrait_test_video.mp4 */; };
 		6C34237AFB808E38FC8776B9 /* RoomStateEventStringBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8D55702474F279D910D2D162 /* RoomStateEventStringBuilder.swift */; };
@@ -1331,6 +1334,7 @@
 		32C5DAA1773F57653BF1C4F9 /* SoftLogoutViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SoftLogoutViewModelTests.swift; sourceTree = "<group>"; };
 		330AF4D121C3396F7A14B21D /* id */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = id; path = id.lproj/SAS.strings; sourceTree = "<group>"; };
 		3368395F06AA180138E185B6 /* PollFormScreenUITests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PollFormScreenUITests.swift; sourceTree = "<group>"; };
+		33AE897D86784CCA5E4E9227 /* ElementCallService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ElementCallService.swift; sourceTree = "<group>"; };
 		33E49C5C6F802B4D94CA78D1 /* ro */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = ro; path = ro.lproj/Localizable.strings; sourceTree = "<group>"; };
 		340179A0FC1AD4AEDA7FC134 /* CreateRoomViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CreateRoomViewModelProtocol.swift; sourceTree = "<group>"; };
 		342BEBC3C5FC3F9943C41C4C /* TemplateScreenViewModelProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TemplateScreenViewModelProtocol.swift; sourceTree = "<group>"; };
@@ -1558,6 +1562,7 @@
 		6F3DFE5B444F131648066F05 /* StateStoreViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StateStoreViewModel.swift; sourceTree = "<group>"; };
 		6F6E6EDC4BBF962B2ED595A4 /* MessageForwardingScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MessageForwardingScreenViewModelTests.swift; sourceTree = "<group>"; };
 		6FC5015B9634698BDB8701AF /* it */ = {isa = PBXFileReference; lastKnownFileType = text.plist.stringsdict; name = it; path = it.lproj/Localizable.stringsdict; sourceTree = "<group>"; };
+		6FC8B21E86B137BE4E91F82A /* ElementCallServiceProtocol.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ElementCallServiceProtocol.swift; sourceTree = "<group>"; };
 		7023EB4F3B7C7D1FBA68638B /* TimelineItemDebugView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineItemDebugView.swift; sourceTree = "<group>"; };
 		7061BE2C0BF427C38AEDEF5E /* SecureBackupRecoveryKeyScreenViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecureBackupRecoveryKeyScreenViewModel.swift; sourceTree = "<group>"; };
 		70C86696AC9521F8ED88FBEB /* MediaUploadPreviewScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MediaUploadPreviewScreen.swift; sourceTree = "<group>"; };
@@ -4021,6 +4026,8 @@
 		92E99C57D7F92ED16F73282C /* ElementCall */ = {
 			isa = PBXGroup;
 			children = (
+				33AE897D86784CCA5E4E9227 /* ElementCallService.swift */,
+				6FC8B21E86B137BE4E91F82A /* ElementCallServiceProtocol.swift */,
 				309AD8BAE6437C31BA7157BF /* ElementCallWidgetDriver.swift */,
 				A6C11AD9813045E44F950410 /* ElementCallWidgetDriverProtocol.swift */,
 			);
@@ -5623,6 +5630,7 @@
 				B5618E3C948584E5C1F67033 /* DTHTMLElement+AttributedStringBuilder.swift in Sources */,
 				DFCA89C4EC2A5332ED6B441F /* DataProtectionManager.swift in Sources */,
 				24A75F72EEB7561B82D726FD /* Date.swift in Sources */,
+				6B67AC7AA41136FC9804C136 /* ElementCallServiceProtocol.swift in Sources */,
 				CFEC53440C572CEEABC4A6A0 /* ElementXAttributeScope.swift in Sources */,
 				A33784831AD880A670CAA9F9 /* FileManager.swift in Sources */,
 				59F940FCBE6BC343AECEF75E /* ImageCache.swift in Sources */,
@@ -5972,6 +5980,8 @@
 				2955F4C160CFD7794D819C64 /* EffectsScene.swift in Sources */,
 				AE1160076F663BF14E0E893A /* EffectsView.swift in Sources */,
 				FE4593FC2A02AAF92E089565 /* ElementAnimations.swift in Sources */,
+				5732395A4F71F51F9C754C5A /* ElementCallService.swift in Sources */,
+				48416BBEB8DDF3E4DED0EDB6 /* ElementCallServiceProtocol.swift in Sources */,
 				07CC13C5729C24255348CBBD /* ElementCallWidgetDriver.swift in Sources */,
 				370AF5BFCD4384DD455479B6 /* ElementCallWidgetDriverProtocol.swift in Sources */,
 				7C1A7B594B2F8143F0DD0005 /* ElementXAttributeScope.swift in Sources */,
@@ -7313,7 +7323,7 @@
 			repositoryURL = "https://github.com/element-hq/matrix-rust-components-swift";
 			requirement = {
 				kind = exactVersion;
-				version = 1.0.2;
+				version = 1.0.3;
 			};
 		};
 		701C7BEF8F70F7A83E852DCC /* XCRemoteSwiftPackageReference "GZIP" */ = {

diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -3,7 +3,7 @@
     {
       "identity" : "compound-design-tokens",
       "kind" : "remoteSourceControl",
-      "location" : "https://github.com/element-hq/compound-design-tokens",
+      "location" : "https://github.com/element-hq/compound-design-tokens.git",
       "state" : {
         "revision" : "c3fff1f2b042295cd5f4bcf8d4fe68ec47ca4061",
         "version" : "1.2.0"
@@ -139,8 +139,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/element-hq/matrix-rust-components-swift",
       "state" : {
-        "revision" : "e87ffe044e74a74930386c2185f5fbcfce3c7877",
-        "version" : "1.0.2"
+        "revision" : "4e74b98191dbbc36025b23baa6cc272298e59de5",
+        "version" : "1.0.3"
       }
     },
     {
@@ -191,7 +191,7 @@
     {
       "identity" : "sfsafesymbols",
       "kind" : "remoteSourceControl",
-      "location" : "https://github.com/SFSafeSymbols/SFSafeSymbols",
+      "location" : "https://github.com/SFSafeSymbols/SFSafeSymbols.git",
       "state" : {
         "revision" : "7cca2d60925876b5953a2cf7341cd80fbeac983c",
         "version" : "4.1.1"

diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
@@ -4313,6 +4313,11 @@ class ElementCallServiceMock: ElementCallServiceProtocol {
     }
 }
 class ElementCallWidgetDriverMock: ElementCallWidgetDriverProtocol {
+    var widgetID: String {
+        get { return underlyingWidgetID }
+        set(value) { underlyingWidgetID = value }
+    }
+    var underlyingWidgetID: String!
     var messagePublisher: PassthroughSubject<String, Never> {
         get { return underlyingMessagePublisher }
         set(value) { underlyingMessagePublisher = value }

diff --git a/ElementX/Sources/Screens/CallScreen/CallScreenCoordinator.swift b/ElementX/Sources/Screens/CallScreen/CallScreenCoordinator.swift
@@ -55,6 +55,10 @@ final class CallScreenCoordinator: CoordinatorProtocol {
         }
         .store(in: &cancellables)
     }
+
+    func stop() {
+        viewModel.stop()
+    }
 
     func toPresentable() -> AnyView {
         AnyView(CallScreen(context: viewModel.context))

diff --git a/ElementX/Sources/Screens/CallScreen/CallScreenViewModel.swift b/ElementX/Sources/Screens/CallScreen/CallScreenViewModel.swift
@@ -31,10 +31,6 @@ class CallScreenViewModel: CallScreenViewModelType, CallScreenViewModelProtocol
         actionsSubject.eraseToAnyPublisher()
     }
 
-    deinit {
-        elementCallService.tearDownCallSession()
-    }
-
     /// Designated initialiser
     /// - Parameters:
     ///   - elementCallService: service responsible for setting up CallKit
@@ -120,7 +116,28 @@ class CallScreenViewModel: CallScreenViewModelType, CallScreenViewModelProtocol
         }
     }
 
+    func stop() {
+        Task {
+            await hangUp()
+        }
+
+        elementCallService.tearDownCallSession()
+    }
+
     // MARK: - Private
+
+    private func hangUp() async {
+        let hangUpMessage = """
+        "api":"toWidget",
+        "widgetId":"\(widgetDriver.widgetID)",
+        "requestId":"widgetapi-\(UUID())",
+        "action":"im.vector.hangup",
+        "data":{}
+        """
+
+        let result = await widgetDriver.sendMessage(hangUpMessage)
+        MXLog.error("Result yo: \(result)")
+    }
 
     private static let eventHandlerName = "elementx"
 

diff --git a/ElementX/Sources/Screens/CallScreen/CallScreenViewModelProtocol.swift b/ElementX/Sources/Screens/CallScreen/CallScreenViewModelProtocol.swift
@@ -20,4 +20,6 @@ import Combine
 protocol CallScreenViewModelProtocol {
     var actions: AnyPublisher<CallScreenViewModelAction, Never> { get }
     var context: CallScreenViewModelType.Context { get }
+
+    func stop()
 }
diff --git a/ElementX/Sources/Screens/CallScreen/View/CallScreen.swift b/ElementX/Sources/Screens/CallScreen/View/CallScreen.swift
@@ -53,8 +53,8 @@ private struct WebView: UIViewRepresentable {
     }
 
     @MainActor
-    class Coordinator: NSObject, WKScriptMessageHandler, WKUIDelegate, WKNavigationDelegate {
-        private let viewModelContext: CallScreenViewModel.Context
+    class Coordinator: NSObject, WKUIDelegate, WKNavigationDelegate {
+        private weak var viewModelContext: CallScreenViewModel.Context?
 
         private(set) var webView: WKWebView!
 
@@ -73,7 +73,7 @@ private struct WebView: UIViewRepresentable {
             let configuration = WKWebViewConfiguration()
 
             let userContentController = WKUserContentController()
-            userContentController.add(self, name: viewModelContext.viewState.messageHandler)
+            userContentController.add(WKScriptMessageHandlerWrapper(self), name: viewModelContext.viewState.messageHandler)
 
             configuration.userContentController = userContentController
             configuration.allowsInlineMediaPlayback = true
@@ -98,8 +98,8 @@ private struct WebView: UIViewRepresentable {
             // After testing different scenarios it seems that when using async/await version of these
             // methods wkwebView expects JavaScript to return with a value (something other than Void),
             // if there is no value returning from the JavaScript that you evaluate you will have a crash.
-            try await withCheckedThrowingContinuation { continuaton in
-                webView.evaluateJavaScript(script) { result, error in
+            try await withCheckedThrowingContinuation { [weak self] continuaton in
+                self?.webView.evaluateJavaScript(script) { result, error in
                     if let error {
                         continuaton.resume(throwing: error)
                     } else {
@@ -109,12 +109,10 @@ private struct WebView: UIViewRepresentable {
             }
         }
 
-        // MARK: - WKScriptMessageHandler
-
         nonisolated func userContentController(_ userContentController: WKUserContentController,
                                                didReceive message: WKScriptMessage) {
-            Task { @MainActor in
-                viewModelContext.javaScriptMessageHandler?(message.body)
+            Task { @MainActor [weak self] in
+                self?.viewModelContext?.javaScriptMessageHandler?(message.body)
             }
         }
 
@@ -148,10 +146,26 @@ private struct WebView: UIViewRepresentable {
 
         nonisolated func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
             Task { @MainActor in
-                viewModelContext.send(viewAction: .urlChanged(webView.url))
+                viewModelContext?.send(viewAction: .urlChanged(webView.url))
             }
         }
     }
+
+    /// Avoids retain loops between the configuration and webView coordinator
+    private class WKScriptMessageHandlerWrapper: NSObject, WKScriptMessageHandler {
+        private weak var coordinator: Coordinator?
+
+        init(_ coordinator: Coordinator) {
+            self.coordinator = coordinator
+        }
+
+        // MARK: - WKScriptMessageHandler
+
+        nonisolated func userContentController(_ userContentController: WKUserContentController,
+                                               didReceive message: WKScriptMessage) {
+            coordinator?.userContentController(userContentController, didReceive: message)
+        }
+    }
 }
 
 // MARK: - Previews

diff --git a/ElementX/Sources/Services/ElementCall/ElementCallWidgetDriver.swift b/ElementX/Sources/Services/ElementCall/ElementCallWidgetDriver.swift
@@ -41,6 +41,7 @@ class ElementCallWidgetDriver: WidgetCapabilitiesProvider, ElementCallWidgetDriv
     private let room: RoomProtocol
     private var widgetDriver: WidgetDriverAndHandle?
 
+    let widgetID = UUID().uuidString
     let messagePublisher = PassthroughSubject<String, Never>()
 
     private let actionsSubject: PassthroughSubject<ElementCallWidgetDriverAction, Never> = .init()
@@ -60,7 +61,7 @@ class ElementCallWidgetDriver: WidgetCapabilitiesProvider, ElementCallWidgetDriv
         let useEncryption = (try? room.isEncrypted()) ?? false
 
         guard let widgetSettings = try? newVirtualElementCallWidget(props: .init(elementCallUrl: baseURL.absoluteString,
-                                                                                 widgetId: UUID().uuidString,
+                                                                                 widgetId: widgetID,
                                                                                  parentUrl: nil,
                                                                                  hideHeader: nil,
                                                                                  preload: nil,

diff --git a/ElementX/Sources/Services/ElementCall/ElementCallWidgetDriverProtocol.swift b/ElementX/Sources/Services/ElementCall/ElementCallWidgetDriverProtocol.swift
@@ -32,6 +32,8 @@ enum ElementCallWidgetDriverAction {
 
 // sourcery: AutoMockable
 protocol ElementCallWidgetDriverProtocol {
+    var widgetID: String { get }
+
     var messagePublisher: PassthroughSubject<String, Never> { get }
     var actions: AnyPublisher<ElementCallWidgetDriverAction, Never> { get }
 

diff --git a/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift b/UnitTests/Sources/UserSessionFlowCoordinatorTests.swift
@@ -51,6 +51,7 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
                                                                 navigationRootCoordinator: navigationRootCoordinator,
                                                                 appLockService: AppLockServiceMock(),
                                                                 bugReportService: BugReportServiceMock(),
+                                                                elementCallService: ElementCallServiceMock(),
                                                                 roomTimelineControllerFactory: timelineControllerFactory,
                                                                 appMediator: AppMediatorMock.default,
                                                                 appSettings: ServiceLocator.shared.settings,

diff --git a/project.yml b/project.yml
@@ -49,7 +49,7 @@ packages:
   # Element/Matrix dependencies
   MatrixRustSDK:
     url: https://github.com/element-hq/matrix-rust-components-swift
-    exactVersion: 1.0.2
+    exactVersion: 1.0.3
     # path: ../matrix-rust-sdk
   Compound:
     url: https://github.com/element-hq/compound-ios