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

…tear down the call on dismissal.
element-hq · May 24, 2024 · b973e44 · b973e44
1 parent 137c7dc
commit b973e44
Show file tree

Hide file tree

Showing 8 changed files with 61 additions and 16 deletions.
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/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