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.
This commit is contained in:
Shadowfacts 2020-10-26 22:55:58 -04:00
parent d638ff513b
commit 89b35fab6d
Signed by: shadowfacts
GPG Key ID: 94A5AB95422746E5
10 changed files with 174 additions and 87 deletions

View File

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

View File

@ -108,6 +108,10 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate {
// Use this method to save data, release shared resources, and store enough scene-specific state information // 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. // 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() try! scene.session.mastodonController?.persistentContainer.viewContext.save()
} }

View File

@ -349,3 +349,17 @@ extension MainSplitViewController: TuskerRootViewController {
} }
} }
} }
@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()
}
}
}
}

View File

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

View File

@ -19,15 +19,18 @@ class NotificationsTableViewController: EnhancedTableViewController {
weak var mastodonController: MastodonController! 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 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) { init(allowedTypes: [Pachyderm.Notification.Kind], mastodonController: MastodonController) {
self.excludedTypes = Array(Set(Pachyderm.Notification.Kind.allCases).subtracting(allowedTypes)) 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 // MARK: - Table view data source
override func numberOfSections(in tableView: UITableView) -> Int { override func numberOfSections(in tableView: UITableView) -> Int {
@ -137,32 +175,7 @@ class NotificationsTableViewController: EnhancedTableViewController {
// MARK: - Table view delegate // MARK: - Table view delegate
override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) { 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 { if indexPath.row == groups.count - 1 {
guard let older = older else { return } guard let older = older else { return }
@ -305,3 +318,9 @@ extension NotificationsTableViewController: UITableViewDataSourcePrefetching {
} }
} }
} }
extension NotificationsTableViewController: BackgroundableViewController {
func sceneDidEnterBackground() {
pruneOffscreenRows()
}
}

View File

@ -224,8 +224,6 @@ class ProfileStatusesViewController: EnhancedTableViewController {
} }
override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) { 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 // load older statuses if at bottom
if timelineSegments.count > 0, if timelineSegments.count > 0,
indexPath.section == timelineSegments.count, indexPath.section == timelineSegments.count,

View File

@ -18,8 +18,11 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
var timelineSegments: [[(id: String, state: StatusState)]] = [] 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) { init(for timeline: Timeline, mastodonController: MastodonController) {
self.timeline = timeline self.timeline = timeline
@ -73,6 +76,12 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
loadInitialStatuses() loadInitialStatuses()
} }
override func viewWillDisappear(_ animated: Bool) {
super.viewWillDisappear(animated)
pruneOffscreenRows()
}
func loadInitialStatuses() { func loadInitialStatuses() {
guard !loaded else { return } guard !loaded else { return }
loaded = true loaded = true
@ -91,6 +100,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 // MARK: - Table view data source
@ -117,58 +172,8 @@ class TimelineTableViewController: EnhancedTableViewController, StatusTableViewC
// MARK: - Table view delegate // MARK: - Table view delegate
override func tableView(_ tableView: UITableView, willDisplay cell: UITableViewCell, forRowAt indexPath: IndexPath) { 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 // load older statuses, if necessary
if indexPath.section == timelineSegments.count - 1, if indexPath.section == timelineSegments.count - 1,
@ -282,3 +287,9 @@ extension TimelineTableViewController: UITableViewDataSourcePrefetching {
} }
} }
} }
extension TimelineTableViewController: BackgroundableViewController {
func sceneDidEnterBackground() {
pruneOffscreenRows()
}
}

View File

@ -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()
}

View File

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

View File

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