Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Fixed

- Fixed a crash when a list left the window (or was deallocated) while an interactive reorder was still in progress — for example, navigating away while a drag was held. The native interactive-movement session outlived the content it was started against, so a later content update or layout pass read a now-stale index and trapped in the layout. `ListView` now cancels any in-progress reorder on `didMoveToWindow` (when leaving the window) and in `deinit`, while the data source and layout are still in sync.

### Added

### Removed
Expand Down
19 changes: 17 additions & 2 deletions ListableUI/Sources/ListView/ListView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,15 @@ public final class ListView : UIView
// because the display link driving it is retained by the main runloop.
self.cancelScrollAnimation()

// If the list is deallocated while a reorder gesture is still in flight, the native
// interactive-movement session would otherwise outlive the data source and layout.
// UIKit would then try to resolve the move against content that no longer matches the
// drag's index paths, reading a stale index and crashing in the layout. Cancel it here,
// while everything is still in sync.
if self.hasInProgressReorders {
self.cancelAllInProgressReorders()
}

/**
Even though these are zeroing weak references in UIKIt as of iOS 9.0,

Expand Down Expand Up @@ -1567,9 +1576,15 @@ public final class ListView : UIView
public override func didMoveToWindow()
{
super.didMoveToWindow()

if self.window != nil {
self.updateScrollViewInsets()
} else if self.hasInProgressReorders {
// Leaving the window — for example, navigating away while a drag is still held — ends
// any chance of the reorder gesture completing normally. Cancel it now, while the
// content and the drag's index paths still agree, so a later content update or layout
// pass cannot read a stale index and crash.
self.cancelAllInProgressReorders()
}
}

Expand Down Expand Up @@ -2467,7 +2482,7 @@ extension ListView : ReorderingActionsDelegate
self.collectionView.cancelInteractiveMovement()
}

private var hasInProgressReorders : Bool {
var hasInProgressReorders : Bool {

for section in self.storage.presentationState.sections {
for item in section.items {
Expand Down
128 changes: 128 additions & 0 deletions ListableUI/Tests/ListView/ListView.ReorderTeardownTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
//
// ListView.ReorderTeardownTests.swift
// ListableUI-Unit-Tests
//

@testable import ListableUI
import XCTest


class ListView_ReorderTeardownTests: XCTestCase {

/// A reorder gesture that is still in flight when the list leaves the window — for example,
/// navigating away while a drag is held — must be cancelled during teardown. Otherwise the
/// native interactive-movement session outlives the content it was started against, and a
/// later content update or layout pass reads a now-stale index and crashes.
func test_reorder_is_cancelled_when_list_leaves_window() {

let viewController = ReorderTestViewController()

show(vc: viewController) { viewController in
let listView = viewController.list

let indexPath = IndexPath(item: 0, section: 0)
let item = listView.storage.presentationState.item(at: indexPath)

// Simulate an in-progress reorder without depending on the flakier native
// `beginInteractiveMovementForItem` gesture plumbing.
item.beginReorder(from: indexPath, with: listView.environment)

XCTAssertTrue(listView.hasInProgressReorders)

// Leaving the window is what navigating away mid-drag does to the list.
listView.removeFromSuperview()

XCTAssertFalse(
listView.hasInProgressReorders,
"Leaving the window should cancel any in-progress reorder."
)
}
}

/// A list deallocated with a reorder still in progress must not crash: teardown cancels the
/// interactive movement while the data source and layout are still valid.
func test_reorder_in_progress_does_not_crash_on_deinit() {

weak var weakList: ListView?

autoreleasepool {
let listView = ListView(frame: CGRect(x: 0, y: 0, width: 400, height: 600))
weakList = listView

listView.configure { list in
list.animatesChanges = false
list("section") { section in
for number in 1...10 {
section += Item(
ReorderTestContent(title: "Item \(number)"),
reordering: ItemReordering(sections: .all)
)
}
}
}

listView.collectionView.layoutIfNeeded()

let indexPath = IndexPath(item: 0, section: 0)
let item = listView.storage.presentationState.item(at: indexPath)
item.beginReorder(from: indexPath, with: listView.environment)

XCTAssertTrue(listView.hasInProgressReorders)
}

// Reaching here without a crash — and with the list deallocated — is the assertion.
XCTAssertNil(weakList)
}
}


fileprivate final class ReorderTestViewController: UIViewController {

let list = ListView()

override func loadView() {
view = UIView()
view.addSubview(list)
list.frame = CGRect(x: 0, y: 0, width: 400, height: 600)

list.configure { list in
list.animatesChanges = false
list("section") { section in
for number in 1...10 {
section += Item(
ReorderTestContent(title: "Item \(number)"),
reordering: ItemReordering(sections: .all)
)
}
}
}
}
}


fileprivate struct ReorderTestContent: ItemContent, Equatable {

var title: String

var identifierValue: String { title }

func apply(
to views: ItemContentViews<Self>,
for reason: ApplyReason,
with info: ApplyItemContentInfo
) {
views.content.backgroundColor = .red
}

typealias ContentView = UIView

static func createReusableContentView(frame: CGRect) -> UIView {
UIView(frame: frame)
}

var defaultItemProperties: DefaultProperties {
.defaults { defaults in
defaults.sizing = .fixed(height: 50)
}
}
}