Fixed an issue where a crash would occur when applying an external update to list content while a live reorder event was occurring

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Fixed
 
 - Fixed an issue where supplementary views (headers or footers) that contained a first responder would result in the view being duplicated when scrolled off-screen.
+- Fixed an issue where a crash would occur when applying an external update to list content while a live reorder event was occurring.
 
 ### Added
 

Demo/Sources/Demos/Demo Screens/PaymentTypesViewController.swift

@@ -12,12 +12,19 @@ import BlueprintUICommonControls
 
 final class PaymentTypesViewController : ListViewController {
 
+    override func viewDidLoad() {
+        super.viewDidLoad()
+
+        self.updateNavigationItems()
+    }
+
     override func configure(list: inout ListProperties) {
 
         list.layout = .table { table in
             table.layout.interSectionSpacingWithNoFooter = 10.0
             table.bounds = .init(
-                padding: UIEdgeInsets(top: 0, left: 0, bottom: 100, right: 0)
+                padding: UIEdgeInsets(top: 0, left: 0, bottom: 100, right: 0),
+                width: .atMost(600)
             )
         }
 
@@ -30,21 +37,31 @@ final class PaymentTypesViewController : ListViewController {
         list += Section(SectionID.main) { section in
 
             section.header = PaymentTypeHeader(title: SectionID.main.title)
-
+                        
             section += types.filter { $0.isEnabled }
             .filter { $0.isMain }
             .sorted { $0.sortOrder < $1.sortOrder }
             .map(makeItem(with:))
+
+            if section.items.isEmpty {
+                section += EmptyRow()
+            }
         }
 
         list += Section(SectionID.more) { section in
 
             section.header = PaymentTypeHeader(title: SectionID.more.title)
 
+            section.reordering.minItemCount = 0
+
             section += types.filter { $0.isEnabled }
             .filter { $0.isMain == false }
             .sorted { $0.sortOrder < $1.sortOrder }
             .map(makeItem(with:))
+
+            if section.items.isEmpty {
+                section += EmptyRow()
+            }
         }
 
         list += Section(SectionID.disabled) { section in
@@ -63,6 +80,56 @@ final class PaymentTypesViewController : ListViewController {
         }
     }
 
+    private var reloadingListTimer : Timer? = nil {
+        didSet {
+            oldValue?.invalidate()
+        }
+    }
+
+    private func updateNavigationItems() {
+
+        if reloadingListTimer == nil {
+            self.navigationItem.rightBarButtonItem = UIBarButtonItem(
+                title: "Start Reloading",
+                style: .plain,
+                target: self,
+                action: #selector(beginReloading)
+            )
+        } else {
+            self.navigationItem.rightBarButtonItem = UIBarButtonItem(
+                title: "Stop Reloading",
+                style: .plain,
+                target: self,
+                action: #selector(endReloading)
+            )
+        }
+    }
+
+    @objc private func beginReloading() {
+
+        let alert = UIAlertController(
+            title: "Will Repeatedly Reload List",
+            message: "The list will be reloaded every 0.5 seconds to verify that reordering + reloading does not cause a crash.",
+            preferredStyle: .alert
+        )
+
+        alert.addAction(.init(title: "OK", style: .default) { [weak self] _ in
+            self?.reloadingListTimer = Timer.scheduledTimer(withTimeInterval: 0.5, repeats: true, block: { _ in
+                self?.reload(animated: true)
+            })
+
+            self?.updateNavigationItems()
+        })
+
+        self.present(alert, animated: true)
+    }
+
+    @objc private func endReloading() {
+        self.reloadingListTimer = nil
+
+        updateNavigationItems()
+    }
+
     private func save(with info : ListStateObserver.ItemReordered) {
         let main = info.sections.first { $0.identifier == Section.identifier(with: SectionID.main) }!
         let more = info.sections.first { $0.identifier == Section.identifier(with: SectionID.more) }!
@@ -136,7 +203,7 @@ fileprivate struct PaymentTypeHeader : BlueprintHeaderFooterContent, Equatable {
 
     var elementRepresentation: Element {
         Label(text: title) {
-            $0.font = .systemFont(ofSize: 18.0, weight: .medium)
+            $0.font = .systemFont(ofSize: 18.0, weight: .bold)
         }
         .inset(uniform: 15.0)
     }

ListableUI/Sources/ListView/ListChangesQueue.swift

@@ -11,16 +11,54 @@ import Foundation
 /// A queue used to synchronized and serialize changes made to the backing collection view,
 /// to work around either bugs or confusing behavior.
 ///
-/// ## Handling Re-ordering (`isQueuingForReorderEvent`)
+/// ## Handling Applying Re-ordering / Move Events (`isQueuingToApplyReorderEvent`)
 /// Collection View has an issue wherein if you perform a re-order event, and then within
 /// the same runloop, deliver an update to the collection view as a result of that re-order event
 /// that removes a row or section, the collection view will crash because it's internal index path
 /// cache / data model has not yet been updated. Thus, in `collectionView(_:moveItemAt:to:)`,
 /// we set this value to `true`, and then after one runloop, we set it back to `false`, after
 /// the collection view's updates have "settled". Please see `sendEndQueuingEditsAfterDelay` for more.
 ///
+/// ## Disabling Updates During In-Progress Re-orders (`listHasUncommittedReorderUpdates`)
+/// If an update is pushed into a `UICollectionView` while a reorder is in progress, there will be a crash
+/// as the collection view tries to layout an index path that does not exist in the data source, as the reordering event
+/// has not yet been committed. As such, we'll queue external updates while reordering is in progress.
+///
+/// ```
+/// 💥
+/// Array.subscript.getter ()
+/// ListLayoutContent.item(at:)
+/// ListLayoutContent.layoutAttributes(at:)
+/// CollectionViewLayout.layoutAttributesForItem(at:)
+/// @objc CollectionViewLayout.layoutAttributesForItem(at:)
+/// -[UICollectionViewData layoutAttributesForItemAtIndexPath:]
+/// -[UICollectionViewData layoutAttributesForGlobalItemIndex:]
+/// __107-[UICollectionView _attributesForItemsVisibleDuringCurrentUpdateWithOldVisibleViews:attributesForNewModel:]_block_invoke
+/// __NSDICTIONARY_IS_CALLING_OUT_TO_A_BLOCK__
+/// -[__NSDictionaryM enumerateKeysAndObjectsWithOptions:usingBlock:]
+/// -[_UICollectionViewSubviewManager enumerateCellsWithEnumerator:]
+/// -[UICollectionView _attributesForItemsVisibleDuringCurrentUpdateWithOldVisibleViews:attributesForNewModel:]
+/// -[UICollectionView /// _createAndAppendViewAnimationsForExistingAndNewlyVisibleItemsInCurrentUpdate:animationsForOnScreenViews:newSubviewManager:oldVisibleViews:attributesF/// orNewModel:]
+/// -[UICollectionView _viewAnimationsForCurrentUpdateWithCollectionViewAnimator:]
+/// __102-[UICollectionView _updateWithItems:tentativelyForReordering:propertyAnimator:collectionViewAnimator:]_block_invoke.632
+/// +[UIView(Animation) performWithoutAnimation:]
+/// -[UICollectionView _updateWithItems:tentativelyForReordering:propertyAnimator:collectionViewAnimator:]
+/// -[UICollectionView _endItemAnimationsWithInvalidationContext:tentativelyForReordering:animator:collectionViewAnimator:]
+/// -[UICollectionView _performBatchUpdates:completion:invalidationContext:tentativelyForReordering:animator:animationHandler:]
+/// ListView.IOS16_4_First_Responder_Bug_CollectionView.performBatchUpdates(_:changes:completion:)
+/// closure #3 in ListView.performBatchUpdates(with:animated:updateBackingData:collectionViewUpdateCompletion:animationCompletion:)
+/// ListView.performBatchUpdates(with:animated:updateBackingData:collectionViewUpdateCompletion:animationCompletion:)
+/// closure #1 in ListView.updatePresentationStateWith(firstVisibleIndexPath:for:completion:)
+/// closure #1 in ListChangesQueue.add(async:)
+/// closure #2 in ListChangesQueue.runIfNeeded()
+/// ListChangesQueue.Operation.ifSynchronous(_:ifAsynchronous:)
+/// ListChangesQueue.runIfNeeded()
+/// ListChangesQueue.add(sync:)
+/// ListView.configure(with:)
+/// ```
+///
 /// ## Handling async batch updates (`add(async:)`)
-/// Because we peform updates to _our_ backing data model (`PresentationState`) alongside
+/// Because we perform updates to _our_ backing data model (`PresentationState`) alongside
 /// our collection view in order to make sure they remain in sync, we need to handle cases where
 /// `UICollectionView.performBatchUpdates(_:completion:)` does not synchronously
 /// invoke its `update` block, which means state can get out of sync.
@@ -104,18 +142,24 @@ final class ListChangesQueue {
         self.runIfNeeded()
     }
 
-    /// Set by consumers to enable and disable queueing during a reorder event.
-    var isQueuingForReorderEvent : Bool = false {
+    /// Set by consumers to enable and disable queueing when a reorder event is being applied.
+    var isQueuingToApplyReorderEvent : Bool = false {
         didSet {
             self.runIfNeeded()
         }
     }
 
-    /// Prevents processing other events in the queue.
+    /// Should be set to `{ collectionView.hasUncommittedUpdates }`.
     ///
-    /// Note: Right now this just checks `isQueuingForReorderEvent`, but may check more props in the future.
+    /// When this closure returns `true`, the queue is paused, to avoid crashes when applying
+    /// content updates while there are index-changing reorder events in process.
+    var listHasUncommittedReorderUpdates : () -> Bool = {
+        fatalError("Must set `listHasUncommittedReorderUpdates` before using `ListChangesQueue`.")
+    }
+
+    /// Prevents processing other events in the queue.
     var isPaused : Bool {
-        self.isQueuingForReorderEvent
+        self.isQueuingToApplyReorderEvent || self.listHasUncommittedReorderUpdates()
     }
 
     var isEmpty : Bool {

ListableUI/Sources/ListView/ListView.DataSource.swift

@@ -152,7 +152,7 @@ internal extension ListView
             ///
             /// See `sendEndQueuingEditsAfterDelay` for a more in-depth explanation.
             ///
-            self.view.updateQueue.isQueuingForReorderEvent = true
+            self.view.updateQueue.isQueuingToApplyReorderEvent = true
 
             /// Perform the change in our data source.
 

ListableUI/Sources/ListView/ListView.Delegate.swift

@@ -288,7 +288,7 @@ extension ListView
         }
 
         func listViewShouldEndQueueingEditsForReorder() {
-            self.view.updateQueue.isQueuingForReorderEvent = false
+            self.view.updateQueue.isQueuingToApplyReorderEvent = false
         }
 
         // MARK: UIScrollViewDelegate

ListableUI/Sources/ListView/ListView.swift

@@ -67,6 +67,8 @@ public final class ListView : UIView
 
         self.closeActiveSwipesGesture = TouchDownGestureRecognizer()
 
+        self.updateQueue = ListChangesQueue()
+
         // Super init.
 
         super.init(frame: frame)
@@ -91,6 +93,10 @@ public final class ListView : UIView
             self?.shouldRecognizeCloseSwipeTouch(touch) ?? false
         }
 
+        self.updateQueue.listHasUncommittedReorderUpdates = { [weak collectionView] in
+            collectionView?.hasUncommittedUpdates ?? false
+        }
+
         // Register supplementary views.
 
         SupplementaryKind.allCases.forEach {
@@ -846,7 +852,7 @@ public final class ListView : UIView
         self.configure(with: description)
     }
 
-    let updateQueue = ListChangesQueue()
+    let updateQueue : ListChangesQueue
 
     public func configure(with properties : ListProperties)
     {

ListableUI/Tests/ListView/ListChangesQueueTests.swift

@@ -14,9 +14,10 @@ class ListChangesQueueTests : XCTestCase {
     func test_pausing() {
 
         let queue = ListChangesQueue()
+        queue.listHasUncommittedReorderUpdates = { false }
 
         XCTAssertFalse(queue.isPaused)
-        XCTAssertFalse(queue.isQueuingForReorderEvent)
+        XCTAssertFalse(queue.isQueuingToApplyReorderEvent)
 
         var calls : [Int] = []
 
@@ -35,10 +36,10 @@ class ListChangesQueueTests : XCTestCase {
         XCTAssertEqual(queue.count, 0)
         XCTAssertEqual(calls, [1, 2])
 
-        queue.isQueuingForReorderEvent = true
+        queue.isQueuingToApplyReorderEvent = true
 
         XCTAssertTrue(queue.isPaused)
-        XCTAssertTrue(queue.isQueuingForReorderEvent)
+        XCTAssertTrue(queue.isQueuingToApplyReorderEvent)
 
         queue.add {
             calls += [3]
@@ -57,10 +58,10 @@ class ListChangesQueueTests : XCTestCase {
         XCTAssertEqual(queue.count, 3)
         XCTAssertEqual(calls, [1, 2])
 
-        queue.isQueuingForReorderEvent = false
+        queue.isQueuingToApplyReorderEvent = false
 
         XCTAssertFalse(queue.isPaused)
-        XCTAssertFalse(queue.isQueuingForReorderEvent)
+        XCTAssertFalse(queue.isQueuingToApplyReorderEvent)
 
         waitFor {
             queue.isEmpty
@@ -72,6 +73,7 @@ class ListChangesQueueTests : XCTestCase {
     func test_synchronous() {
 
         let queue = ListChangesQueue()
+        queue.listHasUncommittedReorderUpdates = { false }
 
         var calls : [Int] = []
 
@@ -99,6 +101,7 @@ class ListChangesQueueTests : XCTestCase {
     func test_asynchronous() {
 
         let queue = ListChangesQueue()
+        queue.listHasUncommittedReorderUpdates = { false }
 
         var calls : [Int] = []
 
@@ -157,6 +160,7 @@ class ListChangesQueueTests : XCTestCase {
     func test_both() {
 
         let queue = ListChangesQueue()
+        queue.listHasUncommittedReorderUpdates = { false }
 
         var calls : [Int] = []
 
@@ -225,6 +229,7 @@ class ListChangesQueueTests : XCTestCase {
     func test_fuzzing() {
 
         let queue = ListChangesQueue()
+        queue.listHasUncommittedReorderUpdates = { false }
 
         var calls : [Int] = []