Instagram / IGListKit / 26610259200

99%

master: 100%

Build Type

push

github

Committed by meta-codesync[bot]

Commit Message

Add IGListExperimentSkipNilIndexPathFiltering to opt into nil-index-path-aware visible cell filtering

Summary:
There's a subtle bug in `-[IGListAdapter visibleCellsForSectionController:]` and `-[IGListAdapter fullyVisibleCellsForSectionController:]`... both methods filter `[collectionView visibleCells]` by `[collectionView indexPathForCell:cell].section == section`, but `indexPathForCell:` is allowed to return `nil` for cells in transitional states — mid-`prepareForReuse`, mid-detach during a `performBatchUpdates`, or in flight between dequeue and final attachment. When that happens, sending `-section` to a `nil` `NSIndexPath` returns `0` in Objective-C, so any visible cell with a cleared index path is silently treated as belonging to **section 0**.

For most consumers this is harmless or invisible. But callers that walk these results and then read layout / window-relative geometry from each returned cell can end up touching cells whose model has been swapped out. We've seen this surface as crashes when a consumer triggers a layout pass on a cell whose backing data is mid-replacement (for example, text typesetting on an `NSAttributedString` that's being swapped out as the cell prepares for reuse). It's also a logical correctness issue: section 0 is the only section that can ever observe transitional cells from other sections leaking in.

The fix itself is essentially one line: skip cells where `indexPathForCell:` returned `nil` before doing the section comparison. The interesting question is how to roll it out.

Given how far-reaching this could be for code depending on IGListKit, I've introduced this as an opt-in experiment so we can effectively test this in Instagram and Threads before iterating or making an informed decision.

The fix is strictly subtractive — it can only narrow the result set, never expand it. Most consumers will just get more correct results. But these methods are called from a wide variety of places across multiple apps that de... (continued)

Coverage Stats

10 of 10 new or added lines in 1 file covered. (100.0%)

11 existing lines in 1 file now uncovered.

4512 of 4569 relevant lines covered (98.75%)

703.73 hits per line