Move pruning of offscreen rows to when the VC disappears, instead of

during scrolling

Prevents race when removing and adding cells in the willDisplay table
view delegate method.

Tusker.xcodeproj/project.pbxproj

@@ -145,6 +145,7 @@
 		D6531DEE246B81C9000F9538 /* GifvAttachmentView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6531DED246B81C9000F9538 /* GifvAttachmentView.swift */; };
 		D6531DF0246B867E000F9538 /* GifvAttachmentViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6531DEF246B867E000F9538 /* GifvAttachmentViewController.swift */; };
 		D6538945214D6D7500E3CEFC /* TableViewSwipeActionProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6538944214D6D7500E3CEFC /* TableViewSwipeActionProvider.swift */; };
+		D65C6BF525478A9C00A6E89C /* BackgroundableViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65C6BF425478A9C00A6E89C /* BackgroundableViewController.swift */; };
 		D65F613423AEAB6600F3CFD3 /* OnboardingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D65F613323AEAB6600F3CFD3 /* OnboardingTests.swift */; };
 		D6620ACE2511A0ED00312CA0 /* StatusStateResolver.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6620ACD2511A0ED00312CA0 /* StatusStateResolver.swift */; };
 		D663625D2135C74800C9CBA2 /* ConversationMainStatusTableViewCell.xib in Resources */ = {isa = PBXBuildFile; fileRef = D663625C2135C74800C9CBA2 /* ConversationMainStatusTableViewCell.xib */; };
@@ -482,6 +483,7 @@
 		D6531DED246B81C9000F9538 /* GifvAttachmentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GifvAttachmentView.swift; sourceTree = "<group>"; };
 		D6531DEF246B867E000F9538 /* GifvAttachmentViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GifvAttachmentViewController.swift; sourceTree = "<group>"; };
 		D6538944214D6D7500E3CEFC /* TableViewSwipeActionProvider.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TableViewSwipeActionProvider.swift; sourceTree = "<group>"; };
+		D65C6BF425478A9C00A6E89C /* BackgroundableViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BackgroundableViewController.swift; sourceTree = "<group>"; };
 		D65F612D23AE990C00F3CFD3 /* Embassy.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Embassy.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		D65F613023AE99E000F3CFD3 /* Ambassador.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Ambassador.framework; sourceTree = BUILT_PRODUCTS_DIR; };
 		D65F613323AEAB6600F3CFD3 /* OnboardingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OnboardingTests.swift; sourceTree = "<group>"; };
@@ -1327,6 +1329,7 @@
 				D693DE5823FE24300061E07D /* InteractivePushTransition.swift */,
 				D6DFC69D242C490400ACC392 /* TrackpadScrollGestureRecognizer.swift */,
 				D6412B0224AFF6A600F5412E /* TabBarScrollableViewController.swift */,
+				D65C6BF425478A9C00A6E89C /* BackgroundableViewController.swift */,
 			);
 			path = Utilities;
 			sourceTree = "<group>";
@@ -1864,6 +1867,7 @@
 				D6BC9DDA232D8BE5002CA326 /* SearchResultsViewController.swift in Sources */,
 				D627FF7F217E95E000CC0648 /* DraftTableViewCell.swift in Sources */,
 				D6DF95C12533F5DE0027A9B6 /* RelationshipMO.swift in Sources */,
+				D65C6BF525478A9C00A6E89C /* BackgroundableViewController.swift in Sources */,
 				D6AEBB4A23216F0400E5038B /* UnfollowAccountActivity.swift in Sources */,
 				D663626421360D2300C9CBA2 /* AvatarStyle.swift in Sources */,
 				D6D3FDE024F41B8400FF50A5 /* ComposeContainerView.swift in Sources */,

Tusker/SceneDelegate.swift

@@ -108,6 +108,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
         // Use this method to save data, release shared resources, and store enough scene-specific state information
         // to restore the scene back to its current state.
         
+        if let rootVC = window?.rootViewController as? BackgroundableViewController {
+            rootVC.sceneDidEnterBackground()
+        }
+        
         try! scene.session.mastodonController?.persistentContainer.viewContext.save()
     }
     

Tusker/Screens/Main/MainSplitViewController.swift

@@ -334,4 +334,32 @@ extension MainSplitViewController: TuskerRootViewController {
             }
         }
     }
+    
+    func getTabController(tab: MainTabBarViewController.Tab) -> UIViewController? {
+        if traitCollection.horizontalSizeClass == .compact {
+            return tabBarViewController?.getTabController(tab: tab)
+        } else {
+            if tab == .compose {
+                return nil
+            } else if case .tab(tab) = sidebar.selectedItem {
+                return viewController(for: .secondary)
+            } else {
+                return nil
+            }
+        }
+    }
+}
+
+@available(iOS 14.0, *)
+extension MainSplitViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        if traitCollection.horizontalSizeClass == .compact {
+            tabBarViewController.sceneDidEnterBackground()
+        } else {
+            // todo: should this do the same for the sidebar VC as well?
+            if let contentVC = viewController(for: .secondary) as? BackgroundableViewController {
+                contentVC.sceneDidEnterBackground()
+            }
+        }
+    }
 }

Tusker/Screens/Main/MainTabBarViewController.swift

@@ -136,3 +136,11 @@ extension MainTabBarViewController: TuskerRootViewController {
         }
     }
 }
+
+extension MainTabBarViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        if let selectedVC = selectedViewController as? BackgroundableViewController {
+            selectedVC.sceneDidEnterBackground()
+        }
+    }
+}

Tusker/Screens/Main/TuskerRootViewController.swift

@@ -11,4 +11,5 @@ import UIKit
 protocol TuskerRootViewController: UIViewController {
     func presentCompose()
     func select(tab: MainTabBarViewController.Tab)
+    func getTabController(tab: MainTabBarViewController.Tab) -> UIViewController?
 }

Tusker/Screens/Notifications/NotificationsTableViewController.swift

@@ -19,15 +19,18 @@ class NotificationsTableViewController: EnhancedTableViewController {
     
     weak var mastodonController: MastodonController!
     
-    let excludedTypes: [Pachyderm.Notification.Kind]
+    private let excludedTypes: [Pachyderm.Notification.Kind]
-    let groupTypes = [Notification.Kind.favourite, .reblog, .follow]
+    private let groupTypes = [Notification.Kind.favourite, .reblog, .follow]
     
     private var loaded = false
 
-    var groups: [NotificationGroup] = []
+    private var groups: [NotificationGroup] = []
     
-    var newer: RequestRange?
+    private let pageSize = 20
-    var older: RequestRange?
+    private var newer: RequestRange?
+    private var older: RequestRange?
+    
+    private var lastLastVisibleRow: IndexPath?
     
     init(allowedTypes: [Pachyderm.Notification.Kind], mastodonController: MastodonController) {
         self.excludedTypes = Array(Set(Pachyderm.Notification.Kind.allCases).subtracting(allowedTypes))
@@ -84,6 +87,41 @@ class NotificationsTableViewController: EnhancedTableViewController {
         }
     }
     
+    override func viewWillDisappear(_ animated: Bool) {
+        super.viewWillDisappear(animated)
+        
+        pruneOffscreenRows()
+    }
+    
+    private func pruneOffscreenRows() {
+        guard let lastVisibleRow = lastLastVisibleRow else {
+            return
+        }
+        let lastRowIndex = groups.count - 1
+        
+        if lastVisibleRow.row < lastRowIndex - pageSize {
+            // if there are more than 20 rows below the lats visible one
+            
+            let rowIndicesToRemove = (lastVisibleRow.row + pageSize)..<groups.count
+            
+            let groupsToRemove = groups[rowIndicesToRemove]
+            for group in groupsToRemove {
+                for notification in group.notifications {
+                    if let id = notification.status?.id {
+                        mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
+                    }
+                }
+            }
+            
+            groups.removeSubrange(rowIndicesToRemove)
+            
+            let removedIndexPaths = rowIndicesToRemove.map { IndexPath(row: $0, section: 0) }
+            UIView.performWithoutAnimation {
+                tableView.deleteRows(at: removedIndexPaths, with: .none)
+            }
+        }
+    }
+    
     // MARK: - Table view data source
 
     override func numberOfSections(in tableView: UITableView) -> Int {
@@ -137,32 +175,7 @@ class NotificationsTableViewController: EnhancedTableViewController {
     // MARK: - Table view delegate
 
     override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
-        // see TimelineTableViewController.tableView(_:willDisplay:forRowAt:)
+        lastLastVisibleRow = tableView.indexPathsForVisibleRows?.last
-        if !isCurrentlyScrollingToTop, scrollViewDirection < 0 {
-            let pageSize = 20
-            if groups.count > 2 * pageSize,
-                indexPath.row < groups.count - (2 * pageSize) {
-                let groupsToRemove = groups[groups.count - pageSize..<groups.count]
-                for group in groupsToRemove {
-                    for notification in group.notifications {
-                        // todo: reference count accounts
-//                        mastodonController.persistentContainer.account(for: notification.account.id)?.decrementReferenceCount()
-                        if let id = notification.status?.id {
-                            mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
-                        }
-                    }
-                }
-                let removedIndexPaths = (groups.count - 20..<groups.count).map { IndexPath(row: $0, section: 0) }
-                
-                groups.removeLast(pageSize)
-                
-                DispatchQueue.main.async {
-                    UIView.performWithoutAnimation {
-                        tableView.deleteRows(at: removedIndexPaths, with: .none)
-                    }
-                }
-            }
-        }
         
         if indexPath.row == groups.count - 1 {
             guard let older = older else { return }
@@ -305,3 +318,9 @@ extension NotificationsTableViewController: UITableViewDataSourcePrefetching {
         }
     }
 }
+
+extension NotificationsTableViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        pruneOffscreenRows()
+    }
+}

Tusker/Screens/Profile/ProfileStatusesViewController.swift

@@ -224,8 +224,6 @@ class ProfileStatusesViewController: EnhancedTableViewController {
     }
     
     override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
-        // todo: if scrolling up, remove statuses at bottom like timeline VC
-        
         // load older statuses if at bottom
         if timelineSegments.count > 0,
            indexPath.section == timelineSegments.count,

Tusker/Screens/Timeline/TimelineTableViewController.swift

@@ -18,8 +18,11 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
     
     var timelineSegments: [[(id: String, state: StatusState)]] = []
     
-    var newer: RequestRange?
+    private let pageSize = 20
-    var older: RequestRange?
+    private var newer: RequestRange?
+    private var older: RequestRange?
+    
+    private var lastLastVisibleRow: IndexPath?
     
     init(for timeline: Timeline, mastodonController: MastodonController) {
         self.timeline = timeline
@@ -73,6 +76,12 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
         loadInitialStatuses()
     }
     
+    override func viewWillDisappear(_ animated: Bool) {
+        super.viewWillDisappear(animated)
+        
+        pruneOffscreenRows()
+    }
+    
     func loadInitialStatuses() {
         guard !loaded else { return }
         loaded = true
@@ -92,6 +101,52 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
         }
     }
     
+    private func pruneOffscreenRows() {
+        guard let lastVisibleRow = lastLastVisibleRow else {
+            return
+        }
+        let lastSectionIndex = timelineSegments.count - 1
+        
+        if lastVisibleRow.section < lastSectionIndex {
+            // if there is a section below the last visible one
+            
+            let sectionsToRemove = (lastVisibleRow.section + 1)...lastSectionIndex
+            
+            for section in sectionsToRemove {
+                for (id, _) in timelineSegments.remove(at: section) {
+                    mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
+                }
+            }
+            
+            UIView.performWithoutAnimation {
+                tableView.deleteSections(IndexSet(sectionsToRemove), with: .none)
+            }
+            
+        } else if lastVisibleRow.section == lastSectionIndex {
+            let lastSection = timelineSegments.last!
+            let lastRowIndex = lastSection.count - 1
+            
+            if lastVisibleRow.row < lastRowIndex - 20 {
+                // if there are more than 20 rows in the current section below the last visible one
+                
+                let rowIndicesInLastSectionToRemove = (lastVisibleRow.row + 20)..<lastSection.count
+                
+                let statusesToRemove = lastSection[rowIndicesInLastSectionToRemove]
+                for (id, _) in statusesToRemove {
+                    mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
+                }
+                
+                timelineSegments[lastSectionIndex].removeSubrange(rowIndicesInLastSectionToRemove)
+
+                let removedIndexPaths = rowIndicesInLastSectionToRemove.map { IndexPath(row: $0, section: lastSectionIndex) }
+                UIView.performWithoutAnimation {
+                    tableView.deleteRows(at: removedIndexPaths, with: .none)
+                }
+
+            }
+        }
+    }
+
     // MARK: - Table view data source
 
     override func numberOfSections(in tableView: UITableView) -> Int {
@@ -117,58 +172,8 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
     // MARK: - Table view delegate
     
     override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) {
-        // don't remove rows when jumping to the top, otherwise jumping back down might try to show removed rows
+        // this assumes that indexPathsForVisibleRows is always in order
-        // when scrolling upwards, decrement reference counts for old statuses, if necessary
+        lastLastVisibleRow = tableView.indexPathsForVisibleRows?.last
-        if !isCurrentlyScrollingToTop, scrollViewDirection < 0 {
-            if indexPath.section <= timelineSegments.count - 2 {
-                // decrement ref counts for all sections below the section below the current section
-                // (e.g., there exist sections 0, 1, 2 and we're currently scrolling upwards in section 0, we want to remove section 2)
-                
-                // todo: this is in the hot path for scrolling, possibly move this to a background thread?
-                let sectionsToRemove = indexPath.section + 1..<timelineSegments.count
-                for section in sectionsToRemove {
-                    for (id, _) in timelineSegments.remove(at: section) {
-                        mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
-                    }
-                }
-                // see below comment about DispatchQueue.main.async
-                DispatchQueue.main.async {
-                    UIView.performWithoutAnimation {
-                        tableView.deleteSections(IndexSet(sectionsToRemove), with: .none)
-                    }
-                }
-            } else {
-                // we are scrolling in the last or second to last section
-                // grab the last section and, if there is more than one page in the last section
-                // (we never want to remove the only page in the last section unless there is another section between it and the user),
-                // remove the last page and decrement reference counts
-                let pageSize = 20 // todo: this should come from somewhere in Pachyderm
-                let lastSection = timelineSegments.last!
-                if lastSection.count > 2 * pageSize,
-                    indexPath.row < lastSection.count - (2 * pageSize) {
-                    // todo: this is in the hot path for scrolling, possibly move this to a background thread?
-                    let statusesToRemove = lastSection[lastSection.count - pageSize..<lastSection.count]
-                    for (id, _) in statusesToRemove {
-                        mastodonController.persistentContainer.status(for: id)?.decrementReferenceCount()
-                    }
-                    timelineSegments[timelineSegments.count - 1].removeLast(pageSize)
-                    
-                    let removedIndexPaths = (lastSection.count - 20..<lastSection.count).map { IndexPath(row: $0, section: timelineSegments.count - 1) }
-                    // Removing this DispatchQueue.main.async call causes things to break when scrolling
-                    // back down towards the removed rows. There would be a index out of bounds crash
-                    // because, while we've already removed the statuses from our model, the table view doesn't seem to know that.
-                    // It seems like tableView update calls made from inside tableView(_:willDisplay:forRowAt:) are silently ignored.
-                    // Calling tableView.numberOfRows(inSection: 0) when trapped in the debugger after the aforementioned IOOB
-                    // will produce an incorrect value (it will be some multiple of pageSize too high).
-                    // Deferring the tableView update until the next runloop iteration seems to solve that.
-                    DispatchQueue.main.async {
-                        UIView.performWithoutAnimation {
-                            tableView.deleteRows(at: removedIndexPaths, with: .none)
-                        }
-                    }
-                }
-            }
-        }
         
         // load older statuses, if necessary
         if indexPath.section == timelineSegments.count - 1,
@@ -282,3 +287,9 @@ extension TimelineTableViewController: UITableViewDataSourcePrefetching {
         }
     }
 }
+
+extension TimelineTableViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        pruneOffscreenRows()
+    }
+}

Tusker/Screens/Utilities/BackgroundableViewController.swift

@@ -0,0 +1,13 @@
+//
+//  BackgroundableViewController.swift
+//  Tusker
+//
+//  Created by Shadowfacts on 10/26/20.
+//  Copyright © 2020 Shadowfacts. All rights reserved.
+//
+
+import Foundation
+
+protocol BackgroundableViewController {
+    func sceneDidEnterBackground()
+}

Tusker/Screens/Utilities/EnhancedNavigationViewController.swift

@@ -74,3 +74,11 @@ class EnhancedNavigationViewController: UINavigationController {
     }
 
 }
+
+extension EnhancedNavigationViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        if let topVC = topViewController as? BackgroundableViewController {
+            topVC.sceneDidEnterBackground()
+        }
+    }
+}

Tusker/Screens/Utilities/SegmentedPageViewController.swift

@@ -71,3 +71,11 @@ extension SegmentedPageViewController: TabBarScrollableViewController {
         }
     }
 }
+
+extension SegmentedPageViewController: BackgroundableViewController {
+    func sceneDidEnterBackground() {
+        if let current = pageControllers[currentIndex] as? BackgroundableViewController {
+            current.sceneDidEnterBackground()
+        }
+    }
+}

Tusker/Shortcuts/UserActivityManager.swift

@@ -21,14 +21,14 @@ class UserActivityManager {
         return scene.session.mastodonController!
     }
     
-    private static func getMainTabBarController() -> MainTabBarViewController {
+    private static func getMainViewController() -> TuskerRootViewController {
         let scene = UIApplication.shared.connectedScenes.compactMap { $0 as? UIWindowScene }.first!
         let window = scene.windows.first { $0.isKeyWindow }!
-        return window.rootViewController as! MainTabBarViewController
+        return window.rootViewController as! TuskerRootViewController
     }
     
     private static func present(_ vc: UIViewController, animated: Bool = true) {
-        getMainTabBarController().present(vc, animated: animated)
+        getMainViewController().present(vc, animated: animated)
     }
 
     // MARK: - New Post
@@ -66,9 +66,9 @@ class UserActivityManager {
     }
     
     static func handleCheckNotifications(activity: NSUserActivity) {
-        let tabBarController = getMainTabBarController()
+        let mainViewController = getMainViewController()
-        tabBarController.select(tab: .notifications)
+        mainViewController.select(tab: .notifications)
-        if let navigationController = tabBarController.getTabController(tab: .notifications) as? UINavigationController,
+        if let navigationController = mainViewController.getTabController(tab: .notifications) as? UINavigationController,
             let notificationsPageController = navigationController.viewControllers.first as? NotificationsPageViewController {
             navigationController.popToRootViewController(animated: false)
             notificationsPageController.loadViewIfNeeded()
@@ -86,9 +86,9 @@ class UserActivityManager {
     }
     
     static func handleCheckMentions(activity: NSUserActivity) {
-        let tabBarController = getMainTabBarController()
+        let mainViewController = getMainViewController()
-        tabBarController.select(tab: .notifications)
+        mainViewController.select(tab: .notifications)
-        if let navController = tabBarController.getTabController(tab: .notifications) as? UINavigationController,
+        if let navController = mainViewController.getTabController(tab: .notifications) as? UINavigationController,
             let notificationsPageController = navController.viewControllers.first as? NotificationsPageViewController {
             navController.popToRootViewController(animated: false)
             notificationsPageController.loadViewIfNeeded()
@@ -133,9 +133,12 @@ class UserActivityManager {
                 return
         }
         
-        let tabBarController = getMainTabBarController()
+        let mainViewController = getMainViewController()
-        tabBarController.select(tab: .timelines)
+        mainViewController.select(tab: .timelines)
-        let navigationController = tabBarController.viewControllers![0] as! UINavigationController
+        guard let navigationController = mainViewController.getTabController(tab: .timelines) as? UINavigationController else {
+            return
+        }
+        
         switch timeline {
         case .home, .public(true), .public(false):
             navigationController.popToRootViewController(animated: false)
@@ -170,9 +173,9 @@ class UserActivityManager {
     }
     
     static func handleSearch(activity: NSUserActivity) {
-        let tabBarController = getMainTabBarController()
+        let mainViewController = getMainViewController()
-        tabBarController.select(tab: .explore)
+        mainViewController.select(tab: .explore)
-        if let navigationController = tabBarController.getTabController(tab: .explore) as? UINavigationController,
+        if let navigationController = mainViewController.getTabController(tab: .explore) as? UINavigationController,
             let exploreController = navigationController.viewControllers.first as? ExploreViewController {
             navigationController.popToRootViewController(animated: false)
             exploreController.searchController.isActive = true
@@ -189,9 +192,9 @@ class UserActivityManager {
     }
     
     static func handleBookmarks(activity: NSUserActivity) {
-        let tabBarController = getMainTabBarController()
+        let mainViewController = getMainViewController()
-        tabBarController.select(tab: .explore)
+        mainViewController.select(tab: .explore)
-        if let navigationController = tabBarController.getTabController(tab: .explore) as? UINavigationController {
+        if let navigationController = mainViewController.getTabController(tab: .explore) as? UINavigationController {
             navigationController.popToRootViewController(animated: false)
             navigationController.pushViewController(BookmarksTableViewController(mastodonController: mastodonController), animated: false)
         }