Skip to content

Commit 2950e88

Browse files
filipnavaraCopilot
andauthored
Fix iOS CollectionView stale layout invalidations (#35245)
### Description of Change There are two related races in the iOS CollectionView handler when the item source changes while cells are still participating in UIKit layout. The first race happens after a visible templated cell invalidates its measure. MAUI records that state on the cell and the next ViewWillLayoutSubviews pass asks UICollectionViewFlowLayout to invalidate exactly those changed items. Previously the handler collected the cells first and converted them back to index paths later, at the point where the invalidation context was created. If the bound ItemsSource had changed in the meantime, for example an observable source inserted items and the view then cleared or replaced ItemsSource before the next layout pass, UIKit could still report a visible cell whose current index path no longer existed in MAUI's new ItemsSource. Passing that stale index path to InvalidateItems leaves UICollectionView and the data source with inconsistent item counts and can crash during layout. Fix that path by resolving each invalidated visible cell to an NSIndexPath immediately and keeping only paths that are still valid for the current ItemsSource. The invalidation context is then built from the validated paths. This preserves targeted invalidation for normal measure changes, while dropping cells that belong to the previous source state and cannot be safely invalidated by item path anymore. The second race involves measurement cells. ItemsViewController keeps prototype templated cells in _measurementCells so their realized content can be transferred to real UICollectionView cells. When an ItemsSource update, empty-source transition, or source disposal clears that dictionary, the old code only removed the references. Those measurement cells could still be bound to item view models and still subscribed to LayoutAttributesChanged. Later binding or property changes from that stale content could propagate measure/layout invalidations through cells that are no longer owned by the active source state. In the worst case this combines with UIKit's pending layout work after ReloadData or source clearing and contributes to the same stale layout invalidation problem; it can also keep disconnected measurement content behaving as if it were still live. Fix that by centralizing measurement-cell clearing. Before the cache is cleared, each cached cell is detached from the layout-attribute event and unbound so its BindingContext is removed and future measure invalidations from that stale measured content do not flow back into the CollectionView layout. The regression test reproduces the important ordering: a templated CollectionView is displayed, a visible label changes text so the cell measure is invalidated, the observable source mutates, and ItemsSource is immediately cleared before UIKit finishes its next layout pass. The test forces layout afterward and verifies this no longer crashes. ### Issues Fixed Fixes #35244 --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent 6e8d8e4 commit 2950e88

3 files changed

Lines changed: 113 additions & 14 deletions

File tree

src/Controls/src/Core/Handlers/Items/iOS/ItemsViewController.cs

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void CheckForEmptySource()
139139

140140
if (_isEmpty)
141141
{
142-
_measurementCells?.Clear();
142+
ClearMeasurementCells();
143143
ItemsViewLayout?.ClearCellSizeCache();
144144
}
145145

@@ -257,19 +257,23 @@ private protected virtual void LayoutSupplementaryViews()
257257
void InvalidateLayoutIfItemsMeasureChanged()
258258
{
259259
var visibleCells = CollectionView.VisibleCells;
260-
List<TemplatedCell> invalidatedCells = null;
260+
List<NSIndexPath> invalidatedIndexPaths = null;
261261

262262
var visibleCellsLength = visibleCells.Length;
263263
for (int n = 0; n < visibleCellsLength; n++)
264264
{
265265
if (visibleCells[n] is TemplatedCell { MeasureInvalidated: true } cell)
266266
{
267-
invalidatedCells ??= [];
268-
invalidatedCells.Add(cell);
267+
var indexPath = CollectionView.IndexPathForCell(cell);
268+
if (indexPath is not null && ItemsSource.IsIndexPathValid(indexPath))
269+
{
270+
invalidatedIndexPaths ??= [];
271+
invalidatedIndexPaths.Add(indexPath);
272+
}
269273
}
270274
}
271275

272-
if (invalidatedCells is not null)
276+
if (invalidatedIndexPaths is not null)
273277
{
274278
// GridLayout has a special positioning override when there's only one item
275279
// so we have to invalidate the layout entirely to trigger that special case.
@@ -280,7 +284,7 @@ void InvalidateLayoutIfItemsMeasureChanged()
280284
else
281285
{
282286
var layoutInvalidationContext = new UICollectionViewFlowLayoutInvalidationContext();
283-
layoutInvalidationContext.InvalidateItems(invalidatedCells.Select(CollectionView.IndexPathForCell).ToArray());
287+
layoutInvalidationContext.InvalidateItems(invalidatedIndexPaths.ToArray());
284288
CollectionView.CollectionViewLayout.InvalidateLayout(layoutInvalidationContext);
285289
}
286290
}
@@ -419,7 +423,7 @@ protected virtual IItemsViewSource CreateItemsViewSource()
419423

420424
public virtual void UpdateItemsSource()
421425
{
422-
_measurementCells?.Clear();
426+
ClearMeasurementCells();
423427
ItemsViewLayout?.ClearCellSizeCache();
424428
ItemsSource?.Dispose();
425429
ItemsSource = CreateItemsViewSource();
@@ -441,7 +445,7 @@ public virtual void UpdateItemsSource()
441445

442446
internal void DisposeItemsSource()
443447
{
444-
_measurementCells?.Clear();
448+
ClearMeasurementCells();
445449
ItemsViewLayout?.ClearCellSizeCache();
446450
ItemsSource?.Dispose();
447451
ItemsSource = new EmptySource();
@@ -530,6 +534,17 @@ protected virtual void UpdateTemplatedCell(TemplatedCell cell, NSIndexPath index
530534
ItemsViewLayout.PrepareCellForLayout(cell);
531535
}
532536

537+
void ClearMeasurementCells()
538+
{
539+
foreach (var measurementCell in _measurementCells.Values)
540+
{
541+
measurementCell.LayoutAttributesChanged -= CellLayoutAttributesChanged;
542+
measurementCell.Unbind();
543+
}
544+
545+
_measurementCells.Clear();
546+
}
547+
533548
public virtual NSIndexPath GetIndexForItem(object item)
534549
{
535550
return ItemsSource.GetIndexForItem(item);

src/Controls/src/Core/Handlers/Items2/iOS/ItemsViewController2.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,22 +214,26 @@ void InvalidateLayoutIfItemsMeasureChanged()
214214
{
215215
var collectionView = CollectionView;
216216
var visibleCells = collectionView.VisibleCells;
217-
List<TemplatedCell2> invalidatedCells = null;
217+
List<NSIndexPath> invalidatedIndexPaths = null;
218218

219219
var visibleCellsLength = visibleCells.Length;
220220
for (int n = 0; n < visibleCellsLength; n++)
221221
{
222222
if (visibleCells[n] is TemplatedCell2 { MeasureInvalidated: true } cell)
223223
{
224-
invalidatedCells ??= [];
225-
invalidatedCells.Add(cell);
224+
var indexPath = collectionView.IndexPathForCell(cell);
225+
if (indexPath is not null && Items.IndexPathHelpers.IsIndexPathValid(ItemsSource, indexPath))
226+
{
227+
invalidatedIndexPaths ??= [];
228+
invalidatedIndexPaths.Add(indexPath);
229+
}
226230
}
227231
}
228232

229-
if (invalidatedCells is not null)
233+
if (invalidatedIndexPaths is not null)
230234
{
231235
var layoutInvalidationContext = new UICollectionViewLayoutInvalidationContext();
232-
layoutInvalidationContext.InvalidateItems(invalidatedCells.Select(CollectionView.IndexPathForCell).ToArray());
236+
layoutInvalidationContext.InvalidateItems(invalidatedIndexPaths.ToArray());
233237
collectionView.CollectionViewLayout.InvalidateLayout(layoutInvalidationContext);
234238
}
235239
}

src/Controls/tests/DeviceTests/Elements/CollectionView/CollectionViewTests.iOS.cs

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,86 @@ public void IndexPathValidTest()
251251
Assert.False(source.IsIndexPathValid(invalidSection));
252252
}
253253

254+
private async Task ClearingItemsSourceAfterCellMeasureInvalidationDoesNotCrashHelper<THandler>()
255+
where THandler : class, IElementHandler
256+
{
257+
EnsureHandlerCreated(builder =>
258+
{
259+
builder.ConfigureMauiHandlers(handlers =>
260+
{
261+
handlers.AddHandler<CollectionView, THandler>();
262+
handlers.AddHandler<Label, LabelHandler>();
263+
});
264+
});
265+
266+
var labels = new List<Label>();
267+
var items = new ObservableCollection<string>
268+
{
269+
"one",
270+
"two",
271+
"three",
272+
"four"
273+
};
274+
275+
var collectionView = new CollectionView
276+
{
277+
HeightRequest = 200,
278+
WidthRequest = 300,
279+
ItemsSource = items,
280+
ItemTemplate = new DataTemplate(() =>
281+
{
282+
var label = new Label
283+
{
284+
LineBreakMode = LineBreakMode.WordWrap
285+
};
286+
287+
label.SetBinding(Label.TextProperty, ".");
288+
labels.Add(label);
289+
290+
return label;
291+
})
292+
};
293+
294+
var frame = collectionView.Frame;
295+
296+
await CreateHandlerAndAddToWindow<THandler>(collectionView, async handler =>
297+
{
298+
await WaitForUIUpdate(frame, collectionView);
299+
300+
Assert.NotEmpty(labels);
301+
302+
// Change text of all the labels to force a relayout, including those that were
303+
// only used for measurement cell. Now we should be sure that all visible cells
304+
// have their MeasureInvalidated == true.
305+
foreach (var label in labels)
306+
label.Text = label.Text + " with enough extra text to invalidate the measured cell size";
307+
// Add another item to force an animation
308+
items.Add("five");
309+
// Reset the data source to force another animation and a layout pass
310+
collectionView.ItemsSource = null;
311+
312+
var platformView = (UIView)handler.PlatformView;
313+
var uiCollectionView = platformView as UICollectionView ?? platformView.Subviews.OfType<UICollectionView>().FirstOrDefault();
314+
Assert.NotNull(uiCollectionView);
315+
// Force synchronous flush of the ItemsSource reloading
316+
await uiCollectionView.PerformBatchUpdatesAsync(() => { });
317+
// Force a layout
318+
platformView.LayoutIfNeeded();
319+
});
320+
}
321+
322+
[Fact(DisplayName = "CollectionView Does Not Crash After Resetting Source With Running Animation")]
323+
public Task ClearingItemsSourceAfterCellMeasureInvalidationDoesNotCrash()
324+
{
325+
return ClearingItemsSourceAfterCellMeasureInvalidationDoesNotCrashHelper<CollectionViewHandler>();
326+
}
327+
328+
[Fact(DisplayName = "CollectionViewHandler2 Does Not Crash After Resetting Source With Running Animation")]
329+
public Task ClearingItemsSourceAfterCellMeasureInvalidationDoesNotCrash2()
330+
{
331+
return ClearingItemsSourceAfterCellMeasureInvalidationDoesNotCrashHelper<CollectionViewHandler2>();
332+
}
333+
254334
[Fact(DisplayName = "CollectionView Does Not Leak With Default ItemsLayout")]
255335
public async Task CollectionViewDoesNotLeakWithDefaultItemsLayout()
256336
{
@@ -487,4 +567,4 @@ private static UIScrollView FindInternalScrollView(UICollectionView collectionVi
487567
return null;
488568
}
489569
}
490-
}
570+
}

0 commit comments

Comments
 (0)