ehealthsolutions/element-x-ios
3
0
Fork 0
mirror of https://github.com/Novaloop-AG/element-x-ios.git synced 2026-11-02 12:07:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

Memory management (#503)

Browse Source 
* Correctly tear down the user session on signing out

* Fix session verification <-> user session retain cycle, visible range debouncer leak

* Manually clean up coordinators retained within SwiftUI's NavigationStacks

* Slightly refactor the timeline content menu builder and prevent it from retaining the view model. Cleanup now unnecessarily optional RoomScreenCoordinator instance vars

* Move coordinator dismissal logic to the navigation modules
This commit is contained in:
Stefan Ceriu
2023-01-31 11:51:56 +02:00
committed by GitHub
parent 70cff446cf
commit d51a9b3a2b
13 changed files with 152 additions and 147 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ElementX/Sources/Application/AppCoordinator.swift
+17 -18
View File
@@ -29,8 +29,9 @@ class AppCoordinator: AppCoordinatorProtocol {
private var userSession: UserSessionProtocol! {
didSet {
deobserveUserSessionChanges()
if let userSession, !userSession.isSoftLogout {
userSessionCancellables.removeAll()
if userSession != nil {
configureNotificationManager()
observeUserSessionChanges()
}
@@ -145,14 +146,16 @@ class AppCoordinator: AppCoordinatorProtocol {
self.setupUserSession()
case (_, .signOut, .signingOut):
self.showLoadingIndicator()
self.tearDownUserSession()
self.logout(isSoftLogout: false)
case (.signingOut, .completedSigningOut, .signedOut):
self.tearDownUserSession()
self.presentSplashScreen()
self.hideLoadingIndicator()
case (_, .remoteSignOut(let isSoft), .remoteSigningOut):
self.showLoadingIndicator()
self.tearDownUserSession(isSoftLogout: isSoft)
self.logout(isSoftLogout: isSoft)
case (.remoteSigningOut(let isSoft), .completedSigningOut, .signedOut):
self.tearDownUserSession()
self.presentSplashScreen(isSoftLogout: isSoft)
self.hideLoadingIndicator()
default:
@@ -251,34 +254,34 @@ class AppCoordinator: AppCoordinatorProtocol {
navigationRootCoordinator.setRootCoordinator(navigationSplitCoordinator)
}
private func tearDownUserSession(isSoftLogout: Bool = false) {
private func logout(isSoftLogout: Bool) {
userSession.clientProxy.stopSync()
userSessionFlowCoordinator?.stop()
deobserveUserSessionChanges()
guard !isSoftLogout else {
stateMachine.processEvent(.completedSigningOut)
return
}
Task {
showLoadingIndicator()
// first log out from the server
_ = await userSession.clientProxy.logout()
// regardless of the result, clear user data
userSessionStore.logout(userSession: userSession)
userSession = nil
notificationManager?.delegate = nil
notificationManager = nil
stateMachine.processEvent(.completedSigningOut)
hideLoadingIndicator()
}
}
private func tearDownUserSession() {
userSession = nil
userSessionFlowCoordinator = nil
notificationManager?.delegate = nil
notificationManager = nil
}
private func presentSplashScreen(isSoftLogout: Bool = false) {
navigationRootCoordinator.setRootCoordinator(SplashScreenCoordinator())
@@ -340,10 +343,6 @@ class AppCoordinator: AppCoordinatorProtocol {
}
.store(in: &userSessionCancellables)
}
private func deobserveUserSessionChanges() {
userSessionCancellables.removeAll()
}
// MARK: Toasts and loading indicators
ElementX/Sources/Application/Navigation/NavigationCoordinators.swift
+58 -41
View File
@@ -28,13 +28,12 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove sidebar", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let sidebarModule {
logPresentationChange("Set sidebar", sidebarModule)
sidebarModule.coordinator.start()
sidebarModule.coordinator?.start()
}
updateCompactLayoutComponents()
@@ -50,13 +49,12 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove detail", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let detailModule {
logPresentationChange("Set detail", detailModule)
detailModule.coordinator.start()
detailModule.coordinator?.start()
}
updateCompactLayoutComponents()
@@ -72,13 +70,12 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove sheet", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let sheetModule {
logPresentationChange("Set sheet", sheetModule)
sheetModule.coordinator.start()
sheetModule.coordinator?.start()
}
updateCompactLayoutComponents()
@@ -94,13 +91,12 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove fullscreen cover", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let fullScreenCoverModule {
logPresentationChange("Set fullscreen cover", fullScreenCoverModule)
fullScreenCoverModule.coordinator.start()
fullScreenCoverModule.coordinator?.start()
}
updateCompactLayoutComponents()
@@ -123,7 +119,7 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
@Published internal var compactLayoutStackModules: [NavigationModule] = []
var compactLayoutStackCoordinators: [any CoordinatorProtocol] {
compactLayoutStackModules.map(\.coordinator)
compactLayoutStackModules.compactMap(\.coordinator)
}
/// Default NavigationSplitCoordinator initialiser
@@ -190,6 +186,10 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
AnyView(NavigationSplitCoordinatorView(navigationSplitCoordinator: self))
}
func stop() {
releaseAllCoordinatorReferences()
}
// MARK: - CustomStringConvertible
var description: String {
@@ -207,8 +207,27 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS
// MARK: - Private
/// The NavigationStack has a tendency to hold on to path items for longer than needed. We work around that by manually nilling the coordinator
/// when a NavigationModule is dismissed. As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence
/// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/
///
/// For added complexity, the NavigationSplitCoordinator has an internal compact layout NavigationStack for which we need to manually nil things again
private func releaseAllCoordinatorReferences() {
sidebarModule?.coordinator = nil
detailModule?.coordinator = nil
sheetModule?.coordinator = nil
fullScreenCoverModule?.coordinator = nil
compactLayoutRootModule?.coordinator = nil
compactLayoutStackModules.forEach { module in
module.coordinator = nil
}
}
private func logPresentationChange(_ change: String, _ module: NavigationModule) {
MXLog.info("\(self) \(change): \(module.coordinator)")
if let coordinator = module.coordinator {
MXLog.info("\(self) \(change): \(coordinator)")
}
}
/// We need to update the compact layout whenever anything changes within the split coordinator or
@@ -335,11 +354,11 @@ private struct NavigationSplitCoordinatorView: View {
// Embedded NavigationStackCoordinators will present their sheets
// through the NavigationSplitCoordinator as well.
.sheet(item: $navigationSplitCoordinator.sheetModule) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
.tint(.element.accent)
}
.fullScreenCover(item: $navigationSplitCoordinator.fullScreenCoverModule) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
.tint(.element.accent)
}
}
@@ -347,9 +366,9 @@ private struct NavigationSplitCoordinatorView: View {
/// The NavigationStack that will be used in compact layouts
var navigationStack: some View {
NavigationStack(path: $navigationSplitCoordinator.compactLayoutStackModules) {
navigationSplitCoordinator.compactLayoutRootModule?.coordinator.toPresentable()
navigationSplitCoordinator.compactLayoutRootModule?.coordinator?.toPresentable()
.navigationDestination(for: NavigationModule.self) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
}
}
}
@@ -358,20 +377,20 @@ private struct NavigationSplitCoordinatorView: View {
var navigationSplitView: some View {
NavigationSplitView(columnVisibility: $columnVisibility) {
if let sidebarModule = navigationSplitCoordinator.sidebarModule {
sidebarModule.coordinator.toPresentable()
sidebarModule.coordinator?.toPresentable()
} else {
navigationSplitCoordinator.placeholderModule.coordinator.toPresentable()
navigationSplitCoordinator.placeholderModule.coordinator?.toPresentable()
}
} detail: {
if let detailModule = navigationSplitCoordinator.detailModule {
detailModule.coordinator.toPresentable()
detailModule.coordinator?.toPresentable()
} else {
navigationSplitCoordinator.placeholderModule.coordinator.toPresentable()
navigationSplitCoordinator.placeholderModule.coordinator?.toPresentable()
}
}
.navigationSplitViewStyle(.balanced)
.navigationDestination(for: NavigationModule.self) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
}
}
}
@@ -386,13 +405,12 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove root", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let rootModule {
logPresentationChange("Set root", rootModule)
rootModule.coordinator.start()
rootModule.coordinator?.start()
}
}
}
@@ -406,13 +424,12 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove sheet", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let sheetModule {
logPresentationChange("Set sheet", sheetModule)
sheetModule.coordinator.start()
sheetModule.coordinator?.start()
}
}
}
@@ -433,13 +450,12 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
didSet {
if let oldValue {
logPresentationChange("Remove fullscreen cover", oldValue)
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let fullScreenCoverModule {
logPresentationChange("Set fullscreen cover", fullScreenCoverModule)
fullScreenCoverModule.coordinator.start()
fullScreenCoverModule.coordinator?.start()
}
}
}
@@ -461,11 +477,10 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
switch change {
case .insert(_, let module, _):
logPresentationChange("Push", module)
module.coordinator.start()
module.coordinator?.start()
case .remove(_, let module, _):
logPresentationChange("Pop", module)
module.coordinator.stop()
module.dismissalCallback?()
module.tearDown()
}
}
}
@@ -473,7 +488,7 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
// The current navigation stack. Excludes the rootCoordinator
var stackCoordinators: [any CoordinatorProtocol] {
stackModules.map(\.coordinator)
stackModules.compactMap(\.coordinator)
}
/// If this NavigationStackCoordinator will be embedded into a NavigationSplitCoordinator pass it here
@@ -592,7 +607,9 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS
// MARK: - Private
private func logPresentationChange(_ change: String, _ module: NavigationModule) {
MXLog.info("\(self) \(change): \(module.coordinator)")
if let coordinator = module.coordinator {
MXLog.info("\(self) \(change): \(coordinator)")
}
}
}
@@ -601,17 +618,17 @@ private struct NavigationStackCoordinatorView: View {
var body: some View {
NavigationStack(path: $navigationStackCoordinator.stackModules) {
navigationStackCoordinator.rootModule?.coordinator.toPresentable()
navigationStackCoordinator.rootModule?.coordinator?.toPresentable()
.navigationDestination(for: NavigationModule.self) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
}
}
.sheet(item: $navigationStackCoordinator.sheetModule) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
.tint(.element.accent)
}
.fullScreenCover(item: $navigationStackCoordinator.fullScreenCoverModule) { module in
module.coordinator.toPresentable()
module.coordinator?.toPresentable()
.tint(.element.accent)
}
}
ElementX/Sources/Application/Navigation/NavigationModule.swift
+16 -4
View File
@@ -18,9 +18,15 @@ import Foundation
/// A CoordinatorProtocol wrapper and type erasing component that allows
/// dynamically presenting arbitrary screens
struct NavigationModule: Identifiable, Hashable {
@MainActor
class NavigationModule: Identifiable, Hashable {
let id = UUID()
let coordinator: any CoordinatorProtocol
/// The NavigationStack has a tendency to hold on to path items for longer than needed. We work around that by manually nilling the coordinator
/// when a NavigationModule is dismissed, reason why this is an optional property
/// As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence
/// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/
var coordinator: (any CoordinatorProtocol)?
let dismissalCallback: (() -> Void)?
init(_ coordinator: any CoordinatorProtocol, dismissalCallback: (() -> Void)? = nil) {
@@ -28,11 +34,17 @@ struct NavigationModule: Identifiable, Hashable {
self.dismissalCallback = dismissalCallback
}
static func == (lhs: NavigationModule, rhs: NavigationModule) -> Bool {
func tearDown() {
coordinator?.stop()
dismissalCallback?()
coordinator = nil
}
nonisolated static func == (lhs: NavigationModule, rhs: NavigationModule) -> Bool {
lhs.id == rhs.id
}
func hash(into hasher: inout Hasher) {
nonisolated func hash(into hasher: inout Hasher) {
hasher.combine(id)
}
}
ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift
+6 -5
View File
@@ -20,13 +20,12 @@ class NavigationRootCoordinator: ObservableObject, CoordinatorProtocol, CustomSt
@Published fileprivate var rootModule: NavigationModule? {
didSet {
if let oldValue {
oldValue.coordinator.stop()
oldValue.dismissalCallback?()
oldValue.tearDown()
}
if let rootModule {
logPresentationChange("Set root", rootModule)
rootModule.coordinator.start()
rootModule.coordinator?.start()
}
}
}
@@ -66,7 +65,9 @@ class NavigationRootCoordinator: ObservableObject, CoordinatorProtocol, CustomSt
// MARK: - Private
private func logPresentationChange(_ change: String, _ module: NavigationModule) {
MXLog.info("\(self) \(change): \(module.coordinator)")
if let coordinator = module.coordinator {
MXLog.info("\(self) \(change): \(coordinator)")
}
}
}
@@ -75,7 +76,7 @@ private struct NavigationRootCoordinatorView: View {
var body: some View {
ZStack {
rootCoordinator.rootModule?.coordinator.toPresentable()
rootCoordinator.rootModule?.coordinator?.toPresentable()
}
.animation(.elementDefault, value: rootCoordinator.rootModule)
}