From 2a839b49d295f12ca4024957072bec09783ddab8 Mon Sep 17 00:00:00 2001 From: Alan Chu Date: Mon, 21 Jun 2021 17:24:50 -0700 Subject: [PATCH 1/3] Fixes StopViewController retain cycles Use weak-referencing block closure for reload timer Weakifes self references in list view actions --- .../OBA/AgencyAlertListViewConverters.swift | 3 +- OBAKit/Stops/StopViewController.swift | 28 ++++++++++++------- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift b/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift index b3ed1d963..db70d9d5c 100644 --- a/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift +++ b/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift @@ -45,7 +45,8 @@ extension AgencyAlertListViewConverters where Self: UIViewController { /// - sectionID: The section ID to use with `OBAListViewSection`. The default value is `service_alerts`. /// - Returns: An `OBAListViewSection` representing the array of `ServiceAlert`s for use with OBAListView. func listSection(serviceAlerts: [ServiceAlert], showSectionTitle: Bool, sectionID: String = "service_alerts") -> OBAListViewSection { - let items = serviceAlerts.map { TransitAlertDataListViewModel($0, isUnread: false, forLocale: .current, onSelectAction: presentAlert) }.uniqued + let onSelectAction: (TransitAlertDataListViewModel) -> Void = { [weak self] item in self?.presentAlert(item) } + let items = serviceAlerts.map { TransitAlertDataListViewModel($0, isUnread: false, forLocale: .current, onSelectAction: onSelectAction) }.uniqued let title: String? if showSectionTitle { if items.count > 1 { diff --git a/OBAKit/Stops/StopViewController.swift b/OBAKit/Stops/StopViewController.swift index b18af35d4..dc04ba789 100644 --- a/OBAKit/Stops/StopViewController.swift +++ b/OBAKit/Stops/StopViewController.swift @@ -151,7 +151,9 @@ public class StopViewController: UIViewController, registerDefaults() - Timer.scheduledTimer(timeInterval: StopViewController.defaultTimerReloadInterval / 2.0, target: self, selector: #selector(timerFired), userInfo: nil, repeats: true) + reloadTimer = Timer.scheduledTimer(withTimeInterval: StopViewController.defaultTimerReloadInterval / 2.0, repeats: true) { [weak self] _ in + self?.timerFired() + } navigationItem.backBarButtonItem = UIBarButtonItem.backButton } @@ -502,15 +504,21 @@ public class StopViewController: UIViewController, let alarmAvailable = canCreateAlarm(for: arrivalDeparture) let deepLinkingAvailable = application.features.deepLinking == .running let highlightTimeOnDisplay = shouldHighlight(arrivalDeparture: arrivalDeparture) + + let onSelectAction: OBAListViewAction = { [unowned self] item in self.didSelectArrivalDepartureItem(item) } + let addAlarmAction: OBAListViewAction = { [unowned self] item in self.addAlarm(viewModel: item) } + let bookmarkAction: OBAListViewAction = { [unowned self] item in self.addBookmark(viewModel: item) } + let shareAction: OBAListViewAction = { [unowned self] item in self.shareTripStatus(viewModel: item) } + return ArrivalDepartureItem( arrivalDeparture: arrivalDeparture, isAlarmAvailable: alarmAvailable, isDeepLinkingAvailable: deepLinkingAvailable, highlightTimeOnDisplay: highlightTimeOnDisplay, - onSelectAction: didSelectArrivalDepartureItem, - alarmAction: addAlarm, - bookmarkAction: addBookmark, - shareAction: shareTripStatus) + onSelectAction: onSelectAction, + alarmAction: addAlarmAction, + bookmarkAction: bookmarkAction, + shareAction: shareAction) } func sectionForGroup(groupRoute: Route?, showSectionHeader: Bool, arrDeps: [ArrivalDeparture]) -> OBAListViewSection { @@ -565,22 +573,22 @@ public class StopViewController: UIViewController, self.performPreviewStopArrival(viewModel) } - let menuProvider: OBAListViewMenuActions.MenuProvider = { _ in + let menuProvider: OBAListViewMenuActions.MenuProvider = { [unowned self] _ in var actions = [UIAction]() if viewModel.isAlarmAvailable { - let alarm = UIAction(title: Strings.addAlarm, image: Icons.addAlarm) { _ in + let alarm = UIAction(title: Strings.addAlarm, image: Icons.addAlarm) { [unowned self] _ in self.addAlarm(viewModel: viewModel) } actions.append(alarm) } - let addBookmark = UIAction(title: Strings.addBookmark, image: Icons.addBookmark) { _ in + let addBookmark = UIAction(title: Strings.addBookmark, image: Icons.addBookmark) { [unowned self] _ in self.addBookmark(viewModel: viewModel) } actions.append(addBookmark) - let shareTrip = UIAction(title: Strings.shareTrip, image: UIImage(systemName: "square.and.arrow.up")) { _ in + let shareTrip = UIAction(title: Strings.shareTrip, image: UIImage(systemName: "square.and.arrow.up")) { [unowned self] _ in self.shareTripStatus(viewModel: viewModel) } actions.append(shareTrip) @@ -734,7 +742,7 @@ public class StopViewController: UIViewController, // All Service Alerts if let alerts = stopArrivals?.serviceAlerts, alerts.count > 0 { - let row = OBAListRowView.DefaultViewModel(title: Strings.serviceAlerts, accessoryType: .disclosureIndicator) { _ in + let row = OBAListRowView.DefaultViewModel(title: Strings.serviceAlerts, accessoryType: .disclosureIndicator) { [unowned self] _ in let controller = ServiceAlertListController(application: self.application, serviceAlerts: alerts) self.application.viewRouter.navigate(to: controller, from: self) } From cd572fefe2ea2b9e6c025bcedd76b24d2d78896a Mon Sep 17 00:00:00 2001 From: Alan Chu Date: Mon, 21 Jun 2021 17:23:44 -0700 Subject: [PATCH 2/3] Fixes TripFloatingPanelController retain cycles Weakifes self references in list view actions --- OBAKit/Trip/TripFloatingPanelController.swift | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/OBAKit/Trip/TripFloatingPanelController.swift b/OBAKit/Trip/TripFloatingPanelController.swift index 7ac0086ba..cd5ef073d 100644 --- a/OBAKit/Trip/TripFloatingPanelController.swift +++ b/OBAKit/Trip/TripFloatingPanelController.swift @@ -256,7 +256,7 @@ class TripFloatingPanelController: UIViewController, } private func serviceAlertsListSection(_ alerts: [ServiceAlert]) -> OBAListViewSection { - let action: OBAListViewAction = { viewModel in + let action: OBAListViewAction = { [unowned self] viewModel in self.application.viewRouter.navigateTo(alert: viewModel.transitAlert, from: self) } @@ -275,19 +275,21 @@ class TripFloatingPanelController: UIViewController, private func tripStopListSection(tripDetails: TripDetails, arrivalDeparture: ArrivalDeparture?, showHeader: Bool) -> OBAListViewSection { var contents: [AnyOBAListViewItem] = [] + let selectAdjacentTripAction: OBAListViewAction = { [unowned self] item in self.onSelectAdjacentTrip(item) } // Previous trip, if any. if let previousTrip = tripDetails.previousTrip { - contents.append(AdjacentTripItem(order: .previous, trip: previousTrip, onSelectAction: onSelectAdjacentTrip).typeErased) + contents.append(AdjacentTripItem(order: .previous, trip: previousTrip, onSelectAction: selectAdjacentTripAction).typeErased) } // Stop times - let stopTimes: [AnyOBAListViewItem] = tripDetails.stopTimes.map { TripStopViewModel(stopTime: $0, arrivalDeparture: arrivalDeparture, onSelectAction: onSelectTripStop).typeErased } + let selectTripStopAction: OBAListViewAction = { [unowned self] item in self.onSelectTripStop(item) } + let stopTimes: [AnyOBAListViewItem] = tripDetails.stopTimes.map { TripStopViewModel(stopTime: $0, arrivalDeparture: arrivalDeparture, onSelectAction: selectTripStopAction).typeErased } contents.append(contentsOf: stopTimes) // Next trip, if any. if let nextTrip = tripDetails.nextTrip { - contents.append(AdjacentTripItem(order: .next, trip: nextTrip, onSelectAction: onSelectAdjacentTrip).typeErased) + contents.append(AdjacentTripItem(order: .next, trip: nextTrip, onSelectAction: selectAdjacentTripAction).typeErased) } let title: String? = showHeader ? OBALoc("trip_details_controller.service_alerts_footer", value: "Trip Details", comment: "Service alerts header in the trip details controller.") : nil @@ -298,7 +300,7 @@ class TripFloatingPanelController: UIViewController, private func viewOnMapAction(for viewModel: TripStopViewModel) -> UIAction? { guard parentTripViewController != nil else { return nil } - return UIAction(title: OBALoc("trip_details_controller.show_on_map", value: "Show on Map", comment: "Button that moves the map to focus on the selected stop"), image: UIImage(systemName: "mappin.circle")) { _ in + return UIAction(title: OBALoc("trip_details_controller.show_on_map", value: "Show on Map", comment: "Button that moves the map to focus on the selected stop"), image: UIImage(systemName: "mappin.circle")) { [unowned self] _ in self.showOnMap(viewModel) } } @@ -306,7 +308,7 @@ class TripFloatingPanelController: UIViewController, private func getWalkingDirections(for viewModel: TripStopViewModel) -> UIMenuElement? { let appleMapsAction: UIAction? if let appleMapsURL = AppInterop.appleMapsWalkingDirectionsURL(coordinate: viewModel.stop.coordinate) { - appleMapsAction = UIAction(title: OBALoc("stops_controller.walking_directions_apple", value: "Walking Directions (Apple Maps)", comment: "Button that launches Apple's maps.app with walking directions to this stop")) { _ in + appleMapsAction = UIAction(title: OBALoc("stops_controller.walking_directions_apple", value: "Walking Directions (Apple Maps)", comment: "Button that launches Apple's maps.app with walking directions to this stop")) { [unowned self] _ in self.application.open(appleMapsURL, options: [:], completionHandler: nil) } } else { @@ -316,7 +318,7 @@ class TripFloatingPanelController: UIViewController, let googleMapsAction: UIAction? #if !targetEnvironment(simulator) if let googleMapsURL = AppInterop.googleMapsWalkingDirectionsURL(coordinate: viewModel.stop.coordinate) { - googleMapsAction = UIAction(title: OBALoc("stops_controller.walking_directions_google", value: "Walking Directions (Google Maps)", comment: "Button that launches Google Maps with walking directions to this stop")) { _ in + googleMapsAction = UIAction(title: OBALoc("stops_controller.walking_directions_google", value: "Walking Directions (Google Maps)", comment: "Button that launches Google Maps with walking directions to this stop")) { [unowned self] _ in self.application.open(googleMapsURL, options: [:], completionHandler: nil) } } else { @@ -336,7 +338,7 @@ class TripFloatingPanelController: UIViewController, func contextMenu(_ listView: OBAListView, for item: AnyOBAListViewItem) -> OBAListViewMenuActions? { guard let tripStop = item.as(TripStopViewModel.self) else { return nil } - let menu: OBAListViewMenuActions.MenuProvider = { _ -> UIMenu? in + let menu: OBAListViewMenuActions.MenuProvider = { [unowned self] _ -> UIMenu? in let menuActions = [ self.viewOnMapAction(for: tripStop), self.getWalkingDirections(for: tripStop) @@ -345,13 +347,13 @@ class TripFloatingPanelController: UIViewController, return UIMenu(title: tripStop.title, children: menuActions) } - let previewProvider: OBAListViewMenuActions.PreviewProvider = { () -> UIViewController? in + let previewProvider: OBAListViewMenuActions.PreviewProvider = { [unowned self] () -> UIViewController? in let stopVC = StopViewController(application: self.application, stopID: tripStop.stop.id) self.currentPreviewingViewController = stopVC return stopVC } - let commitPreviewAction: VoidBlock = { + let commitPreviewAction: VoidBlock = { [unowned self] in guard let vc = self.currentPreviewingViewController else { return } (vc as? Previewable)?.exitPreviewMode() self.application.viewRouter.navigate(to: vc, from: self) From 63aa41090c95a314f2ca79a67348bde7d8879b8f Mon Sep 17 00:00:00 2001 From: Alan Chu Date: Mon, 21 Jun 2021 17:30:08 -0700 Subject: [PATCH 3/3] Fixes AgencyAlertsViewController retain cycles Weakifes self references in list view actions --- .../ListView/Helpers/OBA/AgencyAlertListViewConverters.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift b/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift index db70d9d5c..18179a3a5 100644 --- a/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift +++ b/OBAKit/Controls/ListView/Helpers/OBA/AgencyAlertListViewConverters.swift @@ -27,9 +27,10 @@ extension AgencyAlertListViewConverters where Self: UIViewController { func listSections(agencyAlerts: [AgencyAlert]) -> [OBAListViewSection] { let groupedAlerts = Dictionary(grouping: agencyAlerts, by: { $0.agency?.agency.name ?? "" }) return groupedAlerts.map { group -> OBAListViewSection in + let presentAlertAction: OBAListViewAction = { [weak self] item in self?.presentAlert(item) } let viewModels: [TransitAlertDataListViewModel] = group.value.map { alert -> TransitAlertDataListViewModel in let isUnread = application.alertsStore.isAlertUnread(alert) - return TransitAlertDataListViewModel(alert, isUnread: isUnread, forLocale: Locale.current, onSelectAction: presentAlert) + return TransitAlertDataListViewModel(alert, isUnread: isUnread, forLocale: Locale.current, onSelectAction: presentAlertAction) } let alerts = viewModels.uniqued.sorted(by: \.title) // remove duplicates @@ -45,7 +46,7 @@ extension AgencyAlertListViewConverters where Self: UIViewController { /// - sectionID: The section ID to use with `OBAListViewSection`. The default value is `service_alerts`. /// - Returns: An `OBAListViewSection` representing the array of `ServiceAlert`s for use with OBAListView. func listSection(serviceAlerts: [ServiceAlert], showSectionTitle: Bool, sectionID: String = "service_alerts") -> OBAListViewSection { - let onSelectAction: (TransitAlertDataListViewModel) -> Void = { [weak self] item in self?.presentAlert(item) } + let onSelectAction: OBAListViewAction = { [weak self] item in self?.presentAlert(item) } let items = serviceAlerts.map { TransitAlertDataListViewModel($0, isUnread: false, forLocale: .current, onSelectAction: onSelectAction) }.uniqued let title: String? if showSectionTitle {