[Android] Ensure disconnected ItemsViewHandler doesn't hold onto the items source
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
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.
/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).
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.
Looks like Unit tests are failing
Apparently
MarshalingObservableCollectiontests depend on the collection changing without ever registering for theCollectionChangedevent. That makes it tricky to implement the behavior on Android correctly since there's no explicit disposal of theMarshalingObservableCollectionthat is internally created by theItemsSourceFactory.
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
Hmmm, maybe for now we just restore
MarshalingObservableCollectionso 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?
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
The test failure looks relevant. I don't see why it happens though, and I will need to reproduce it locally.
Sure. I am away until next week but I will revive the PR when I get back home.
/rebase
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
/azp run
Azure Pipelines successfully started running 3 pipeline(s).
@filipnavara I screwed up this rebase.. I confused the branch on your side, do you mind rebase and reopen this one with your local copy.
Thanks
/azp run
Azure Pipelines successfully started running 3 pipeline(s).