Refactor ItemsLayout handling: dynamic default + virtual view-managed subscriptions
[!NOTE] Are you waiting for the changes in this PR to be merged? It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
This PR resolves #29619 by eliminating the memory leak caused by CollectionViewHandler2's subscription to ItemsLayout property changes and reworking the property change flow.
Supersedes to #29635 , #28675 , #29190
Problems
- Previously, we used a static
LinearItemsLayout.Verticalinstance as the default for ItemsLayout - When the handler subscribed to property changes on this shared instance (without unsubscribing), it caused memory leaks (in the CV2 case)
- Additionally, Android and Windows used
WeakNotifyPropertyChangedProxyto work around this, but that added platform complexity and only masked the root cause.
Fix
- The default
ItemsLayoutis now created per-instance using defaultValueCreator, not a static object. - The
ItemsView(virtual view) subscribes directly to changes on itsItemsLayoutand raisesOnPropertyChanged("ItemsLayout")for relevant internal changes (Span, ItemSpacing, etc.). - This triggers the
MapItemsLayoutmapper, which applies platform-specific updates. - Removed
WeakNotifyPropertyChangedProxyfrom Android and Windows for ItemsLayout changes. - All layout updates now flow through the mapper via the virtual view
Issues Fixed
Fixes #29619 Fixes #27666 Fixes #27667 Fixes #28656 Fixes #29696 Fixes #28023
While enabling memory tests for CV2, I found that CarouselViewHandler2 is leaking 😢. It seems the cause is different from the issue we're addressing here — something else is triggering the leak.
Anyway, I’ll definitely look into it.
Aside from that, everything here looks great.
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
Only CarouselViewHandler2 memory tests are failing. #29719 should fix the leak.
I also noticed that CarouselView too uses a static instance as the default ItemsLayout. I'm updating it to use a per-instance value via defaultValueCreator and managing property changes in the VirtualView itself,just like we did with ItemsView for CollectionView.
Also, I need to write few UI tests to verify that property changes in ItemsLayout (e.g., ItemSpacing) are correctly reflected in the UI for both CollectionView and CarouselView
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
/rebase
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
Is it realistic to assume that this will come in SR11, related to the milestone?
https://github.com/dotnet/maui/pull/29683