Skip to content

Commit

Permalink
fix(leak): Ensure that ItemCollection is unregistering from ItemSource
Browse files Browse the repository at this point in the history
This change also moves the null check at the first position for performance
  • Loading branch information
jeromelaban committed Dec 21, 2020
1 parent ade644a commit 2058063
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 2 deletions.
8 changes: 7 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/ItemCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ internal void SetItemsSource(IEnumerable itemsSource)
if (itemsSource == null)
{
_itemsSource = null;
ObserveCollectionChanged(null);
}
else
{
Expand Down Expand Up @@ -182,7 +183,12 @@ internal void SetItemsSource(IEnumerable itemsSource)

private void ObserveCollectionChanged(object itemsSource)
{
if (itemsSource is INotifyCollectionChanged existingObservable)
if (itemsSource is null)
{
// fast path for null
_itemsSourceCollectionChangeDisposable.Disposable = null;
}
else if (itemsSource is INotifyCollectionChanged existingObservable)
{
// This is a workaround for a bug with EventRegistrationTokenTable on Xamarin, where subscribing/unsubscribing to a class method directly won't
// remove the handler.
Expand Down
6 changes: 5 additions & 1 deletion src/Uno.UI/UI/Xaml/Controls/ItemsControl/ItemsControl.cs
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,12 @@ internal void ObserveCollectionChanged()
{
var unwrappedSource = UnwrapItemsSource();

if(unwrappedSource is null)
{
_notifyCollectionChanged.Disposable = null;
}
//Subscribe to changes on grouped source that is an observable collection
if (unwrappedSource is CollectionView collectionView && collectionView.CollectionGroups != null && collectionView.InnerCollection is INotifyCollectionChanged observableGroupedSource)
else if (unwrappedSource is CollectionView collectionView && collectionView.CollectionGroups != null && collectionView.InnerCollection is INotifyCollectionChanged observableGroupedSource)
{
// This is a workaround for a bug with EventRegistrationTokenTable on Xamarin, where subscribing/unsubscribing to a class method directly won't
// remove the handler.
Expand Down

0 comments on commit 2058063

Please sign in to comment.