Skip to content

Commit 992a1f1

Browse files
mojganiimojganii
committed
Fix memory issues in custom list
1 parent 03ef6a8 commit 992a1f1

14 files changed

+114
-90
lines changed

‎ios/MullvadVPN/Coordinators/CustomLists/AddCustomListCoordinator.swift

+12-8
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class AddCustomListCoordinator: Coordinator, Presentable, Presenting {
2323
navigationController
2424
}
2525

26-
var didFinish: (() -> Void)?
26+
var didFinish: ((AddCustomListCoordinator) -> Void)?
2727

2828
init(
2929
navigationController: UINavigationController,
@@ -59,8 +59,11 @@ class AddCustomListCoordinator: Coordinator, Presentable, Presenting {
5959

6060
controller.navigationItem.leftBarButtonItem = UIBarButtonItem(
6161
systemItem: .cancel,
62-
primaryAction: UIAction(handler: { _ in
63-
self.didFinish?()
62+
primaryAction: UIAction(handler: { [weak self] _ in
63+
guard let self else {
64+
return
65+
}
66+
didFinish?(self)
6467
})
6568
)
6669

@@ -70,7 +73,7 @@ class AddCustomListCoordinator: Coordinator, Presentable, Presenting {
7073

7174
extension AddCustomListCoordinator: CustomListViewControllerDelegate {
7275
func customListDidSave(_ list: CustomList) {
73-
didFinish?()
76+
didFinish?(self)
7477
}
7578

7679
func customListDidDelete(_ list: CustomList) {
@@ -84,14 +87,15 @@ extension AddCustomListCoordinator: CustomListViewControllerDelegate {
8487
customList: list
8588
)
8689

87-
coordinator.didFinish = { customList in
88-
self.subject.send(CustomListViewModel(
90+
coordinator.didFinish = { [weak self] locationsCoordinator, customList in
91+
guard let self else { return }
92+
subject.send(CustomListViewModel(
8993
id: customList.id,
9094
name: customList.name,
9195
locations: customList.locations,
92-
tableSections: self.subject.value.tableSections
96+
tableSections: subject.value.tableSections
9397
))
94-
self.removeFromParent()
98+
locationsCoordinator.removeFromParent()
9599
}
96100

97101
coordinator.start()

‎ios/MullvadVPN/Coordinators/CustomLists/AddLocationsCoordinator.swift

+6-2
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ class AddLocationsCoordinator: Coordinator, Presentable, Presenting {
1818
private let nodes: [LocationNode]
1919
private var customList: CustomList
2020

21-
var didFinish: ((CustomList) -> Void)?
21+
var didFinish: ((AddLocationsCoordinator, CustomList) -> Void)?
2222

2323
var presentedViewController: UIViewController {
2424
navigationController
@@ -54,6 +54,10 @@ class AddLocationsCoordinator: Coordinator, Presentable, Presenting {
5454

5555
extension AddLocationsCoordinator: AddLocationsViewControllerDelegate {
5656
func didUpdateSelectedLocations(locations: [RelayLocation]) {
57-
didFinish?(CustomList(id: customList.id, name: customList.name, locations: locations))
57+
customList.locations = locations
58+
}
59+
60+
func didOnBack() {
61+
didFinish?(self, customList)
5862
}
5963
}

‎ios/MullvadVPN/Coordinators/CustomLists/AddLocationsDataSource.swift

+2-15
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,7 @@ class AddLocationsDataSource: UITableViewDiffableDataSource<AddLocationsSectionI
3333
customList: CustomList
3434
) {
3535
self.tableView = tableView
36-
self.nodes = allLocations.map {
37-
let copy = $0.copy()
38-
copy.showsChildren = false
39-
copy.allChildren.forEach { $0.showsChildren = false }
40-
return copy
41-
}
36+
self.nodes = allLocations
4237

4338
self.customListLocationNode = CustomListLocationNodeBuilder(
4439
customList: customList,
@@ -71,7 +66,7 @@ class AddLocationsDataSource: UITableViewDiffableDataSource<AddLocationsSectionI
7166
)
7267
locationsList.append(viewModel)
7368

74-
// Determine the node should be expanded.
69+
// Determine if the node should be expanded.
7570
guard customListLocationNode.children.contains(where: { node.allChildren.contains($0) }) else {
7671
return
7772
}
@@ -159,14 +154,6 @@ extension AddLocationsDataSource: UITableViewDelegate {
159154
func tableView(_ tableView: UITableView, indentationLevelForRowAt indexPath: IndexPath) -> Int {
160155
itemIdentifier(for: indexPath)?.indentationLevel ?? 0
161156
}
162-
163-
func tableView(_ tableView: UITableView, heightForHeaderInSection section: Int) -> CGFloat {
164-
.zero
165-
}
166-
167-
func tableView(_ tableView: UITableView, viewForHeaderInSection section: Int) -> UIView? {
168-
nil
169-
}
170157
}
171158

172159
extension AddLocationsDataSource: AddLocationCellDelegate {

‎ios/MullvadVPN/Coordinators/CustomLists/AddLocationsViewController.swift

+20
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import UIKit
1212

1313
protocol AddLocationsViewControllerDelegate: AnyObject {
1414
func didUpdateSelectedLocations(locations: [RelayLocation])
15+
func didOnBack()
1516
}
1617

1718
class AddLocationsViewController: UIViewController {
@@ -28,9 +29,27 @@ class AddLocationsViewController: UIViewController {
2829
tableView.indicatorStyle = .white
2930
tableView.accessibilityIdentifier = .addLocationsView
3031
tableView.allowsMultipleSelection = true
32+
tableView.tableHeaderView = nil
33+
tableView.sectionHeaderHeight = .zero
3134
return tableView
3235
}()
3336

37+
private lazy var backBarButton: UIBarButtonItem = {
38+
let backBarButton = UIBarButtonItem(
39+
primaryAction: UIAction(
40+
image: UIImage(resource: .iconBack),
41+
handler: { [weak self] _ in
42+
guard let self else { return }
43+
delegate?.didOnBack()
44+
navigationController?.popViewController(animated: true)
45+
}
46+
)
47+
)
48+
backBarButton.style = .done
49+
50+
return backBarButton
51+
}()
52+
3453
init(
3554
allLocationsNodes: [LocationNode],
3655
customList: CustomList
@@ -48,6 +67,7 @@ class AddLocationsViewController: UIViewController {
4867
super.viewDidLoad()
4968
tableView.backgroundColor = view.backgroundColor
5069
view.backgroundColor = .secondaryColor
70+
navigationItem.leftBarButtonItem = backBarButton
5171
addConstraints()
5272
setUpDataSource()
5373
}

‎ios/MullvadVPN/Coordinators/CustomLists/CustomListDataSourceConfiguration.swift

+8-6
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
import UIKit
1010

1111
class CustomListDataSourceConfiguration: NSObject {
12-
let dataSource: UITableViewDiffableDataSource<CustomListSectionIdentifier, CustomListItemIdentifier>
12+
private weak var dataSource: UITableViewDiffableDataSource<CustomListSectionIdentifier, CustomListItemIdentifier>?
1313
var validationErrors: Set<CustomListFieldValidationError> = []
1414

1515
var didSelectItem: ((CustomListItemIdentifier) -> Void)?
@@ -43,13 +43,15 @@ class CustomListDataSourceConfiguration: NSObject {
4343
}
4444
}
4545

46-
dataSource.apply(snapshot, animatingDifferences: animated)
46+
dataSource?.apply(snapshot, animatingDifferences: animated)
4747
}
4848

4949
func set(validationErrors: Set<CustomListFieldValidationError>) {
5050
self.validationErrors = validationErrors
5151

52-
var snapshot = dataSource.snapshot()
52+
guard var snapshot = dataSource?.snapshot() else {
53+
return
54+
}
5355

5456
validationErrors.forEach { error in
5557
switch error {
@@ -58,7 +60,7 @@ class CustomListDataSourceConfiguration: NSObject {
5860
}
5961
}
6062

61-
dataSource.apply(snapshot, animatingDifferences: false)
63+
dataSource?.apply(snapshot, animatingDifferences: false)
6264
}
6365
}
6466

@@ -68,7 +70,7 @@ extension CustomListDataSourceConfiguration: UITableViewDelegate {
6870
}
6971

7072
func tableView(_ tableView: UITableView, viewForFooterInSection section: Int) -> UIView? {
71-
let snapshot = dataSource.snapshot()
73+
guard let snapshot = dataSource?.snapshot() else { return nil }
7274

7375
let sectionIdentifier = snapshot.sectionIdentifiers[section]
7476
let itemsInSection = snapshot.itemIdentifiers(inSection: sectionIdentifier)
@@ -99,7 +101,7 @@ extension CustomListDataSourceConfiguration: UITableViewDelegate {
99101
func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) {
100102
tableView.deselectRow(at: indexPath, animated: false)
101103

102-
if let item = dataSource.itemIdentifier(for: indexPath) {
104+
if let item = dataSource?.itemIdentifier(for: indexPath) {
103105
didSelectItem?(item)
104106
}
105107
}

‎ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift

+10-8
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ class CustomListViewController: UIViewController {
4545
value: "Save",
4646
comment: ""
4747
),
48-
primaryAction: UIAction { _ in
49-
self.onSave()
48+
primaryAction: UIAction { [weak self] _ in
49+
self?.onSave()
5050
}
5151
)
5252
barButtonItem.style = .done
@@ -101,29 +101,31 @@ class CustomListViewController: UIViewController {
101101
}
102102

103103
private func configureDataSource() {
104-
cellConfiguration.onDelete = {
105-
self.onDelete()
104+
cellConfiguration.onDelete = { [weak self] in
105+
self?.onDelete()
106106
}
107107

108108
dataSource = DataSource(
109109
tableView: tableView,
110-
cellProvider: { _, indexPath, itemIdentifier in
111-
self.cellConfiguration.dequeueCell(
110+
cellProvider: { [weak self] _, indexPath, itemIdentifier in
111+
guard let self else { return nil }
112+
return cellConfiguration.dequeueCell(
112113
at: indexPath,
113114
for: itemIdentifier,
114115
validationErrors: self.validationErrors
115116
)
116117
}
117118
)
118119

119-
dataSourceConfiguration?.didSelectItem = { item in
120+
dataSourceConfiguration?.didSelectItem = { [weak self] item in
121+
guard let self else { return }
120122
self.view.endEditing(false)
121123

122124
switch item {
123125
case .name, .deleteList:
124126
break
125127
case .addLocations, .editLocations:
126-
self.delegate?.showLocations(self.subject.value.customList)
128+
delegate?.showLocations(self.subject.value.customList)
127129
}
128130
}
129131

‎ios/MullvadVPN/Coordinators/CustomLists/EditCustomListCoordinator.swift

+8-7
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class EditCustomListCoordinator: Coordinator, Presentable, Presenting {
2626
navigationController
2727
}
2828

29-
var didFinish: ((FinishAction, CustomList) -> Void)?
29+
var didFinish: ((EditCustomListCoordinator, FinishAction, CustomList) -> Void)?
3030

3131
init(
3232
navigationController: UINavigationController,
@@ -67,11 +67,11 @@ class EditCustomListCoordinator: Coordinator, Presentable, Presenting {
6767

6868
extension EditCustomListCoordinator: CustomListViewControllerDelegate {
6969
func customListDidSave(_ list: CustomList) {
70-
didFinish?(.save, list)
70+
didFinish?(self, .save, list)
7171
}
7272

7373
func customListDidDelete(_ list: CustomList) {
74-
didFinish?(.delete, list)
74+
didFinish?(self, .delete, list)
7575
}
7676

7777
func showLocations(_ list: CustomList) {
@@ -81,14 +81,15 @@ extension EditCustomListCoordinator: CustomListViewControllerDelegate {
8181
customList: list
8282
)
8383

84-
coordinator.didFinish = { customList in
85-
self.subject.send(CustomListViewModel(
84+
coordinator.didFinish = { [weak self] locationsCoordinator, customList in
85+
guard let self else { return }
86+
subject.send(CustomListViewModel(
8687
id: customList.id,
8788
name: customList.name,
8889
locations: customList.locations,
89-
tableSections: self.subject.value.tableSections
90+
tableSections: subject.value.tableSections
9091
))
91-
coordinator.removeFromParent()
92+
locationsCoordinator.removeFromParent()
9293
}
9394

9495
coordinator.start()

‎ios/MullvadVPN/Coordinators/CustomLists/EditLocationsCoordinator.swift

+6-2
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ class EditLocationsCoordinator: Coordinator, Presentable, Presenting {
1717
private let nodes: [LocationNode]
1818
private var customList: CustomList
1919

20-
var didFinish: ((CustomList) -> Void)?
20+
var didFinish: ((EditLocationsCoordinator, CustomList) -> Void)?
2121

2222
var presentedViewController: UIViewController {
2323
navigationController
@@ -52,6 +52,10 @@ class EditLocationsCoordinator: Coordinator, Presentable, Presenting {
5252

5353
extension EditLocationsCoordinator: AddLocationsViewControllerDelegate {
5454
func didUpdateSelectedLocations(locations: [RelayLocation]) {
55-
didFinish?(CustomList(id: customList.id, name: customList.name, locations: locations))
55+
customList.locations = locations
56+
}
57+
58+
func didOnBack() {
59+
didFinish?(self, customList)
5660
}
5761
}

‎ios/MullvadVPN/Coordinators/CustomLists/ListCustomListCoordinator.swift

+13-12
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class ListCustomListCoordinator: Coordinator, Presentable, Presenting {
2222
navigationController
2323
}
2424

25-
var didFinish: (() -> Void)?
25+
var didFinish: ((ListCustomListCoordinator) -> Void)?
2626

2727
init(
2828
navigationController: UINavigationController,
@@ -39,28 +39,29 @@ class ListCustomListCoordinator: Coordinator, Presentable, Presenting {
3939
}
4040

4141
func start() {
42-
listViewController.didFinish = didFinish
43-
listViewController.didSelectItem = {
44-
self.edit(list: $0)
42+
listViewController.didFinish = { [weak self] in
43+
guard let self else { return }
44+
didFinish?(self)
45+
}
46+
listViewController.didSelectItem = { [weak self] in
47+
self?.edit(list: $0)
4548
}
4649

4750
navigationController.pushViewController(listViewController, animated: false)
4851
}
4952

5053
private func edit(list: CustomList) {
51-
// Remove previous edit coordinator to prevent accumulation.
52-
childCoordinators.filter { $0 is EditCustomListCoordinator }.forEach { $0.removeFromParent() }
53-
5454
let coordinator = EditCustomListCoordinator(
5555
navigationController: navigationController,
5656
customListInteractor: interactor,
5757
customList: list,
5858
nodes: nodes
5959
)
6060

61-
coordinator.didFinish = { action, list in
62-
self.popToList()
63-
coordinator.removeFromParent()
61+
coordinator.didFinish = { [weak self] editCustomListCoordinator, action, list in
62+
guard let self else { return }
63+
popToList()
64+
editCustomListCoordinator.removeFromParent()
6465

6566
self.updateRelayConstraints(for: action, in: list)
6667
self.listViewController.updateDataSource(reloadExisting: action == .save)
@@ -90,8 +91,8 @@ class ListCustomListCoordinator: Coordinator, Presentable, Presenting {
9091
relayConstraints.locations = .only(UserSelectedRelays(locations: []))
9192
}
9293

93-
tunnelManager.updateSettings([.relayConstraints(relayConstraints)]) {
94-
self.tunnelManager.startTunnel()
94+
tunnelManager.updateSettings([.relayConstraints(relayConstraints)]) { [weak self] in
95+
self?.tunnelManager.startTunnel()
9596
}
9697
}
9798

‎ios/MullvadVPN/Coordinators/CustomLists/ListCustomListViewController.swift

-10
Original file line numberDiff line numberDiff line change
@@ -108,16 +108,6 @@ class ListCustomListViewController: UIViewController {
108108
tableView.registerReusableViews(from: CellReuseIdentifier.self)
109109
}
110110

111-
private func setEmptyMessage(_ message: String) {
112-
tableView.backgroundView = emptyListLabel
113-
tableView.separatorStyle = .none
114-
}
115-
116-
private func restore() {
117-
tableView.backgroundView = nil
118-
tableView.separatorStyle = .singleLine
119-
}
120-
121111
private func configureNavigationItem() {
122112
navigationItem.title = NSLocalizedString(
123113
"LIST_CUSTOM_LIST_NAVIGATION_TITLE",

0 commit comments

Comments
 (0)