ItemsRepeater crashes when scrolling back and forth

[KPixel]

Describe the bug
The following sample causes ItemsRepeater to fail internally and crash the app.

Steps to reproduce the bug

1. Run the following sample:

public sealed class ItemsRepeaterPage : Page
{
    public ItemsRepeaterPage()
    {
        var repeater = new ItemsRepeater
        {
            ItemsSource = Enumerable.Range(0, 1000).Select(i => new Border
            {
                Background = new SolidColorBrush(Colors.Blue),
                Child = new TextBlock { Text = "#" + i }
            }).ToArray(),
            Layout = new UniformGridLayout
            {
                MinItemWidth = 100, MinItemHeight = 40,
                MinRowSpacing = 10, MinColumnSpacing = 10
            }
        };
        Content = new ScrollViewer { Content = repeater };
    }
}

2. When you scroll to the end then scroll back to the start, Item #0 is not visible.
3. Then, trying to scroll to the end again causes COMException: "ItemsRepeater's child not found in its Children collection."

Expected behavior
The app should not crash.

Version Info
Tested with version 2.4.2 and 2.5.0.prerelease on Windows 10 1909.

[StephenLPeters]

@ranjeshj FYI

[StephenLPeters]

could be related to #1923

[marcelwgn]

Unless somebody else is already working on this, I would like to look into this issue @ranjeshj.

[ranjeshj]

@chingucoding go for it. 👍

[marcelwgn]

So it seems that the ElementManager is assuming it is owning the first item, when actually the UniformGridLayout is owning it. And since it is not expecting that, it is crashing. Should we update the UniformGridLayout here to not hold a reference to the first item and instead maybe save it's size or get it dynamically? After all this issue only exists because the first item is special for the UniformGridLayout.

[StephenLPeters]

In general it seems weird to me that a layout would be the owner of an element. I would expect the layout to subscribe to the first elements size changed event.

[ranjeshj]

Possibly the same issue as #535. Ownership here is with respect to who is responsible for recycling the element. UniformWrapGrid is a bit complicated in this sense because it internally uses flow layout algorithm and the algorithm class is responsible for keeping track of which items are realized and which are not, but for uniform wrap grid we need to track the first items size and keep it realized all the time. There is some complex back and forth happening for that.

[marcelwgn]

Would it be a good solution to just not cache that item? After all, when we cache that item we would need some logic to return that item when it is request (which does not happen right now and results in the first item not being shown), which will be quite complex and potentially computation expensive. Realizing an item every time we are trying to measure is also not ideal, but at least we wouldn't need to keep track of the lifecycle of the first element.

[marcelwgn]

In general it seems weird to me that a layout would be the owner of an element. I would expect the layout to subscribe to the first elements size changed event.

Listening to the elements size changed also doesn't really work, since that would get cleared as soon as we leave the realization rect, so we would need to keep that item alive in that case too, which means move the ownership from ElementManager to the layout.

[ranjeshj]

Would it be a good solution to just not cache that item? After all, when we cache that item we would need some logic to return that item when it is request (which does not happen right now and results in the first item not being shown), which will be quite complex and potentially computation expensive. Realizing an item every time we are trying to measure is also not ideal, but at least we wouldn't need to keep track of the lifecycle of the first element.

We could maybe force create item 0 and recycle it every time. That is worse perf wise than caching, but if the elements are being recycled, that might be an acceptable tradeoff and will get rid of dealing with the ownership issue. Its worth a try.