Skip to content

Commit b973e44

Browse files
committed
Fixes #2878 - Retain cycles on the ElementCall webView and correctly tear down the call on dismissal.
1 parent 137c7dc commit b973e44

File tree

8 files changed

+61
-16
lines changed

8 files changed

+61
-16
lines changed

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4313,6 +4313,11 @@ class ElementCallServiceMock: ElementCallServiceProtocol {
43134313
}
43144314
}
43154315
class ElementCallWidgetDriverMock: ElementCallWidgetDriverProtocol {
4316+
var widgetID: String {
4317+
get { return underlyingWidgetID }
4318+
set(value) { underlyingWidgetID = value }
4319+
}
4320+
var underlyingWidgetID: String!
43164321
var messagePublisher: PassthroughSubject<String, Never> {
43174322
get { return underlyingMessagePublisher }
43184323
set(value) { underlyingMessagePublisher = value }

ElementX/Sources/Screens/CallScreen/CallScreenCoordinator.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,10 @@ final class CallScreenCoordinator: CoordinatorProtocol {
5555
}
5656
.store(in: &cancellables)
5757
}
58+
59+
func stop() {
60+
viewModel.stop()
61+
}
5862

5963
func toPresentable() -> AnyView {
6064
AnyView(CallScreen(context: viewModel.context))

ElementX/Sources/Screens/CallScreen/CallScreenViewModel.swift

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,6 @@ class CallScreenViewModel: CallScreenViewModelType, CallScreenViewModelProtocol
3131
actionsSubject.eraseToAnyPublisher()
3232
}
3333

34-
deinit {
35-
elementCallService.tearDownCallSession()
36-
}
37-
3834
/// Designated initialiser
3935
/// - Parameters:
4036
/// - elementCallService: service responsible for setting up CallKit
@@ -120,7 +116,28 @@ class CallScreenViewModel: CallScreenViewModelType, CallScreenViewModelProtocol
120116
}
121117
}
122118

119+
func stop() {
120+
Task {
121+
await hangUp()
122+
}
123+
124+
elementCallService.tearDownCallSession()
125+
}
126+
123127
// MARK: - Private
128+
129+
private func hangUp() async {
130+
let hangUpMessage = """
131+
"api":"toWidget",
132+
"widgetId":"\(widgetDriver.widgetID)",
133+
"requestId":"widgetapi-\(UUID())",
134+
"action":"im.vector.hangup",
135+
"data":{}
136+
"""
137+
138+
let result = await widgetDriver.sendMessage(hangUpMessage)
139+
MXLog.error("Result yo: \(result)")
140+
}
124141

125142
private static let eventHandlerName = "elementx"
126143

ElementX/Sources/Screens/CallScreen/CallScreenViewModelProtocol.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,6 @@ import Combine
2020
protocol CallScreenViewModelProtocol {
2121
var actions: AnyPublisher<CallScreenViewModelAction, Never> { get }
2222
var context: CallScreenViewModelType.Context { get }
23+
24+
func stop()
2325
}

ElementX/Sources/Screens/CallScreen/View/CallScreen.swift

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ private struct WebView: UIViewRepresentable {
5353
}
5454

5555
@MainActor
56-
class Coordinator: NSObject, WKScriptMessageHandler, WKUIDelegate, WKNavigationDelegate {
57-
private let viewModelContext: CallScreenViewModel.Context
56+
class Coordinator: NSObject, WKUIDelegate, WKNavigationDelegate {
57+
private weak var viewModelContext: CallScreenViewModel.Context?
5858

5959
private(set) var webView: WKWebView!
6060

@@ -73,7 +73,7 @@ private struct WebView: UIViewRepresentable {
7373
let configuration = WKWebViewConfiguration()
7474

7575
let userContentController = WKUserContentController()
76-
userContentController.add(self, name: viewModelContext.viewState.messageHandler)
76+
userContentController.add(WKScriptMessageHandlerWrapper(self), name: viewModelContext.viewState.messageHandler)
7777

7878
configuration.userContentController = userContentController
7979
configuration.allowsInlineMediaPlayback = true
@@ -98,8 +98,8 @@ private struct WebView: UIViewRepresentable {
9898
// After testing different scenarios it seems that when using async/await version of these
9999
// methods wkwebView expects JavaScript to return with a value (something other than Void),
100100
// if there is no value returning from the JavaScript that you evaluate you will have a crash.
101-
try await withCheckedThrowingContinuation { continuaton in
102-
webView.evaluateJavaScript(script) { result, error in
101+
try await withCheckedThrowingContinuation { [weak self] continuaton in
102+
self?.webView.evaluateJavaScript(script) { result, error in
103103
if let error {
104104
continuaton.resume(throwing: error)
105105
} else {
@@ -109,12 +109,10 @@ private struct WebView: UIViewRepresentable {
109109
}
110110
}
111111

112-
// MARK: - WKScriptMessageHandler
113-
114112
nonisolated func userContentController(_ userContentController: WKUserContentController,
115113
didReceive message: WKScriptMessage) {
116-
Task { @MainActor in
117-
viewModelContext.javaScriptMessageHandler?(message.body)
114+
Task { @MainActor [weak self] in
115+
self?.viewModelContext?.javaScriptMessageHandler?(message.body)
118116
}
119117
}
120118

@@ -148,10 +146,26 @@ private struct WebView: UIViewRepresentable {
148146

149147
nonisolated func webView(_ webView: WKWebView, didFinish navigation: WKNavigation!) {
150148
Task { @MainActor in
151-
viewModelContext.send(viewAction: .urlChanged(webView.url))
149+
viewModelContext?.send(viewAction: .urlChanged(webView.url))
152150
}
153151
}
154152
}
153+
154+
/// Avoids retain loops between the configuration and webView coordinator
155+
private class WKScriptMessageHandlerWrapper: NSObject, WKScriptMessageHandler {
156+
private weak var coordinator: Coordinator?
157+
158+
init(_ coordinator: Coordinator) {
159+
self.coordinator = coordinator
160+
}
161+
162+
// MARK: - WKScriptMessageHandler
163+
164+
nonisolated func userContentController(_ userContentController: WKUserContentController,
165+
didReceive message: WKScriptMessage) {
166+
coordinator?.userContentController(userContentController, didReceive: message)
167+
}
168+
}
155169
}
156170

157171
// MARK: - Previews

ElementX/Sources/Services/ElementCall/ElementCallWidgetDriver.swift

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class ElementCallWidgetDriver: WidgetCapabilitiesProvider, ElementCallWidgetDriv
4141
private let room: RoomProtocol
4242
private var widgetDriver: WidgetDriverAndHandle?
4343

44+
let widgetID = UUID().uuidString
4445
let messagePublisher = PassthroughSubject<String, Never>()
4546

4647
private let actionsSubject: PassthroughSubject<ElementCallWidgetDriverAction, Never> = .init()
@@ -60,7 +61,7 @@ class ElementCallWidgetDriver: WidgetCapabilitiesProvider, ElementCallWidgetDriv
6061
let useEncryption = (try? room.isEncrypted()) ?? false
6162

6263
guard let widgetSettings = try? newVirtualElementCallWidget(props: .init(elementCallUrl: baseURL.absoluteString,
63-
widgetId: UUID().uuidString,
64+
widgetId: widgetID,
6465
parentUrl: nil,
6566
hideHeader: nil,
6667
preload: nil,

ElementX/Sources/Services/ElementCall/ElementCallWidgetDriverProtocol.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ enum ElementCallWidgetDriverAction {
3232

3333
// sourcery: AutoMockable
3434
protocol ElementCallWidgetDriverProtocol {
35+
var widgetID: String { get }
36+
3537
var messagePublisher: PassthroughSubject<String, Never> { get }
3638
var actions: AnyPublisher<ElementCallWidgetDriverAction, Never> { get }
3739

project.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ packages:
4949
# Element/Matrix dependencies
5050
MatrixRustSDK:
5151
url: https://github.com/element-hq/matrix-rust-components-swift
52-
exactVersion: 1.0.2
52+
exactVersion: 1.0.3
5353
# path: ../matrix-rust-sdk
5454
Compound:
5555
url: https://github.com/element-hq/compound-ios

0 commit comments

Comments
 (0)