[Android] Ensure disconnected ItemsViewHandler doesn't hold onto the items source

[filipnavara]

Description of Change

Any control derived from ItemsView (CarouselView, CollectionView) never disconnected from the ItemsSource.CollectionChanged callback when DisconnectHandler() is called. If the lifetime of the item source outlives the controls it can result in the CollectionChanged callback keeping part of the UI tree alive in a half-zombie state. Any change then calls the callback and it may cause crashes with NullReferenceException as seen in #24304.

Issues Fixed

Fixes #24304

[dotnet-policy-service]

Hey there @filipnavara! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

https://dev.azure.com/xamarin/public/_build/results?buildId=123299&view=ms.vss-test-web.build-test-results-tab

Looks like Unit tests are failing

[filipnavara]

Looks like Unit tests are failing

Apparently MarshalingObservableCollection tests depend on the collection changing without ever registering for the CollectionChanged event. That makes it tricky to implement the behavior on Android correctly since there's no explicit disposal of the MarshalingObservableCollection that is internally created by the ItemsSourceFactory.

[PureWeen]

Looks like Unit tests are failing

Apparently MarshalingObservableCollection tests depend on the collection changing without ever registering for the CollectionChanged event. That makes it tricky to implement the behavior on Android correctly since there's no explicit disposal of the MarshalingObservableCollection that is internally created by the ItemsSourceFactory.

Hmmm, maybe for now we just restore MarshalingObservableCollection so it subscribes in the ctor and make sure that calling disconnecthandler on Android will correctly unsubscribe. I realize that's not currently ideal but that would at least let us merge this PR and fix the crasher on iOS

[filipnavara]

Hmmm, maybe for now we just restore MarshalingObservableCollection so it subscribes in the ctor and make sure that calling disconnecthandler on Android will correctly unsubscribe.

Would it be okay to add implementation of IDisposable interface to MarshalingObservableCollection?

[rmarinho]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[filipnavara]

The test failure looks relevant. I don't see why it happens though, and I will need to reproduce it locally.

[rmarinho]

Hey @filipnavara since #24699 was merged, can we rebase this for the other platforms?

[filipnavara]

Sure. I am away until next week but I will revive the PR when I get back home.

[jsuarezruiz]

/rebase

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).