Serialize access to the UITableView for scrolling and diffable datasource updates. Issue #1806

iOS/MasterFeed/MasterFeedDataSource.swift

@@ -7,7 +7,6 @@
 //
 
 import UIKit
-import RSCore
 import RSTree
 import Account
 

iOS/MasterFeed/MasterFeedDataSourceOperation.swift

@@ -0,0 +1,39 @@
+//
+//  MasterFeedDataSourceOperation.swift
+//  NetNewsWire-iOS
+//
+//  Created by Maurice Parker on 2/23/20.
+//  Copyright © 2020 Ranchero Software. All rights reserved.
+//
+
+import UIKit
+import RSCore
+import RSTree
+
+class MasterFeedDataSourceOperation: MainThreadOperation {
+	
+	// MainThreadOperation
+	public var isCanceled = false
+	public var id: Int?
+	public weak var operationDelegate: MainThreadOperationDelegate?
+	public var name: String? = "MasterFeedDataSourceOperation"
+	public var completionBlock: MainThreadOperation.MainThreadOperationCompletionBlock?
+
+	private var dataSource: UITableViewDiffableDataSource<Node, Node>
+	private var snapshot: NSDiffableDataSourceSnapshot<Node, Node>
+	private var animating: Bool
+	
+	init(dataSource: UITableViewDiffableDataSource<Node, Node>, snapshot: NSDiffableDataSourceSnapshot<Node, Node>, animating: Bool) {
+		self.dataSource = dataSource
+		self.snapshot = snapshot
+		self.animating = animating
+	}
+	
+	func run() {
+		dataSource.apply(snapshot, animatingDifferences: animating) { [weak self] in
+			guard let self = self else { return }
+			self.operationDelegate?.operationDidComplete(self)
+		}
+	}
+	
+}

iOS/MasterFeed/MasterFeedViewController.swift

@@ -17,8 +17,10 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
 	@IBOutlet weak var filterButton: UIBarButtonItem!
 	private var refreshProgressView: RefreshProgressView?
 	@IBOutlet weak var addNewItemButton: UIBarButtonItem!
-	
+
+	private let operationQueue = MainThreadOperationQueue()
 	lazy var dataSource = makeDataSource()
+	
 	var undoableCommands = [UndoableCommand]()
 	weak var coordinator: SceneCoordinator!
 	
@@ -27,7 +29,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
 		return keyboardManager.keyCommands
 	}
 	
-	var restoreSelection = false
 	override var canBecomeFirstResponder: Bool {
 		return true
 	}
@@ -73,17 +74,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
 		updateUI()
 		super.viewWillAppear(animated)
 	}
-
-	override func viewDidLayoutSubviews() {
-		super.viewDidLayoutSubviews()
-		
-		// We have to delay the selection of the Feed until the subviews have been layed out
-		// so that the visible area is correct and we scroll correctly.
-		if restoreSelection {
-			restoreSelectionIfNecessary(adjustScroll: true)
-			restoreSelection = false
-		}
-	}
 	
 	// MARK: Notifications
 	
@@ -144,10 +134,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
 		}
 	}
 	
-	@objc func webFeedMetadataDidChange(_ note: Notification) {
-		reloadAllVisibleCells()
-	}
-	
 	@objc func contentSizeCategoryDidChange(_ note: Notification) {
 		resetEstimatedRowHeight()
 		applyChanges(animated: false)
@@ -517,24 +503,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner {
 	}
 
 	func updateFeedSelection(animations: Animations) {
-		if dataSource.snapshot().numberOfItems > 0 {
-			if let indexPath = coordinator.currentFeedIndexPath {
-				if tableView.indexPathForSelectedRow != indexPath {
-					tableView.selectRowAndScrollIfNotVisible(at: indexPath, animations: animations)
-				}
-			} else {
-				if animations.contains(.select) {
-					// This nasty bit of duct tape is because there is something, somewhere
-					// interrupting the deselection animation, which will leave the row selected.
-					// This seems to get it far enough away the problem that it always works.
-					DispatchQueue.main.async {
-						self.tableView.selectRow(at: nil, animated: true, scrollPosition: .none)
-					}
-				} else {
-					self.tableView.selectRow(at: nil, animated: false, scrollPosition: .none)
-				}
-			}
-		}
+		operationQueue.add(UpdateSelectionOperation(coordinator: coordinator, dataSource: dataSource, tableView: tableView, animations: animations))
 	}
 
 	func reloadFeeds(initialLoad: Bool, completion: (() -> Void)? = nil) {
@@ -646,7 +615,7 @@ private extension MasterFeedViewController {
 	func reloadNode(_ node: Node) {
 		var snapshot = dataSource.snapshot()
 		snapshot.reloadItems([node])
-		dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
+		queueApply(snapshot: snapshot, animatingDifferences: false) { [weak self] in
 			self?.restoreSelectionIfNecessary(adjustScroll: false)
 		}
 	}
@@ -661,13 +630,22 @@ private extension MasterFeedViewController {
 			snapshot.appendItems(shadowTableNodes, toSection: sectionNode)
 		}
         
-		dataSource.apply(snapshot, animatingDifferences: animated) { [weak self] in
+		queueApply(snapshot: snapshot, animatingDifferences: animated) { [weak self] in
 			self?.restoreSelectionIfNecessary(adjustScroll: adjustScroll)
 			completion?()
 		}
 	}
+	
+	func queueApply(snapshot: NSDiffableDataSourceSnapshot<Node, Node>, animatingDifferences: Bool = true, completion: (() -> Void)? = nil) {
+		let operation = MasterFeedDataSourceOperation(dataSource: dataSource, snapshot: snapshot, animating: animatingDifferences)
+		operation.completionBlock = { _ in
+			completion?()
+		}
+		operationQueue.add(operation)
+	}
 
-    func makeDataSource() -> UITableViewDiffableDataSource<Node, Node> {
+
+    func makeDataSource() -> MasterFeedDataSource {
 		let dataSource = MasterFeedDataSource(tableView: tableView, cellProvider: { [weak self] tableView, indexPath, node in
 			let cell = tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! MasterFeedTableViewCell
 			self?.configure(cell, node)
@@ -774,7 +752,7 @@ private extension MasterFeedViewController {
 	private func reloadCells(_ nodes: [Node], completion: (() -> Void)? = nil) {
 		var snapshot = dataSource.snapshot()
 		snapshot.reloadItems(nodes)
-		dataSource.apply(snapshot, animatingDifferences: false) { [weak self] in
+		queueApply(snapshot: snapshot, animatingDifferences: false) { [weak self] in
 			self?.restoreSelectionIfNecessary(adjustScroll: false)
 			completion?()
 		}

iOS/MasterFeed/UpdateSelectionOperation.swift

@@ -0,0 +1,51 @@
+//
+//  UpdateSelectionOperation.swift
+//  NetNewsWire-iOS
+//
+//  Created by Maurice Parker on 2/22/20.
+//  Copyright © 2020 Ranchero Software. All rights reserved.
+//
+
+import UIKit
+import RSCore
+
+class UpdateSelectionOperation: MainThreadOperation {
+	
+	// MainThreadOperation
+	public var isCanceled = false
+	public var id: Int?
+	public weak var operationDelegate: MainThreadOperationDelegate?
+	public var name: String? = "UpdateSelectionOperation"
+	public var completionBlock: MainThreadOperation.MainThreadOperationCompletionBlock?
+
+	private var coordinator: SceneCoordinator
+	private var dataSource: MasterFeedDataSource
+	private var tableView: UITableView
+	private var animations: Animations
+	
+	init(coordinator: SceneCoordinator, dataSource: MasterFeedDataSource, tableView: UITableView, animations: Animations) {
+		self.coordinator = coordinator
+		self.dataSource = dataSource
+		self.tableView = tableView
+		self.animations = animations
+	}
+	
+	func run() {
+		if dataSource.snapshot().numberOfItems > 0 {
+			if let indexPath = coordinator.currentFeedIndexPath {
+				CATransaction.begin()
+				CATransaction.setCompletionBlock {
+					self.operationDelegate?.operationDidComplete(self)
+				}
+				tableView.selectRowAndScrollIfNotVisible(at: indexPath, animations: animations)
+				CATransaction.commit()
+			} else {
+				tableView.selectRow(at: nil, animated: animations.contains(.select), scrollPosition: .none)
+				self.operationDelegate?.operationDidComplete(self)
+			}
+		} else {
+			self.operationDelegate?.operationDidComplete(self)
+		}
+	}
+	
+}

iOS/SceneCoordinator.swift

@@ -1738,7 +1738,9 @@ private extension SceneCoordinator {
 	}
 
 	@objc func fetchAndMergeArticlesAsync() {
-		fetchAndMergeArticlesAsync(animated: true, completion: nil)
+		fetchAndMergeArticlesAsync(animated: true) {
+			self.masterTimelineViewController?.reinitializeArticles(resetScroll: false)
+		}
 	}
 	
 	func fetchAndMergeArticlesAsync(animated: Bool = true, completion: (() -> Void)? = nil) {
@@ -2002,7 +2004,6 @@ private extension SceneCoordinator {
 		}
 
 		treeControllerDelegate.addFilterException(feedIdentifier)
-		masterFeedViewController.restoreSelection = true
 		
 		switch feedIdentifier {
 		
@@ -2087,8 +2088,6 @@ private extension SceneCoordinator {
 				return false
 		}
 
-		masterFeedViewController.restoreSelection = true
-		
 		switch feedIdentifier {
 
 		case .smartFeed: