Fix crash using an object that implements IEnumerable and INotifyCollectionChanged...
authorJavier Suárez Ruiz <javiersuarezruiz@hotmail.com>
Mon, 4 Nov 2019 20:08:43 +0000 (21:08 +0100)
committerRui Marinho <me@ruimarinho.net>
Mon, 4 Nov 2019 20:08:42 +0000 (20:08 +0000)
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8200.cs [new file with mode: 0644]
Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
Xamarin.Forms.Platform.Android/CollectionView/ItemsSourceFactory.cs
Xamarin.Forms.Platform.Android/CollectionView/ObservableItemsSource.cs
Xamarin.Forms.Platform.iOS/CollectionView/ItemsSourceFactory.cs
Xamarin.Forms.Platform.iOS/CollectionView/ObservableGroupedSource.cs
Xamarin.Forms.Platform.iOS/CollectionView/ObservableItemsSource.cs

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8200.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue8200.cs
new file mode 100644 (file)
index 0000000..bc169ec
--- /dev/null
@@ -0,0 +1,169 @@
+using System.Collections;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using System.Linq;
+using System.Windows.Input;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using NUnit.Framework;
+using Xamarin.Forms.Core.UITests;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+       [Category(UITestCategories.CollectionView)]
+#endif
+       [Preserve(AllMembers = true)]
+       [Issue(IssueTracker.Github, 8200, "CollectionView on iOS assumes INotifyCollectionChanged is an IList", PlatformAffected.iOS)]
+       public class Issue8200 : TestContentPage
+       {
+               CollectionView _collectionView;
+
+               public Issue8200()
+               {
+                       Title = "Issue 8200";
+                       BindingContext = new Issue8200ViewModel();
+               }
+
+               protected override void Init()
+               {
+                       var instructions = new Label
+                       {
+                               Text = "If you can see a CollectionView below with items, the test has passed."
+                       };
+
+                       var buttonsLayout = new StackLayout
+                       {
+                               Orientation = StackOrientation.Horizontal
+                       };
+
+                       var addButton = new Button
+                       {
+                               Text = "Add Item"
+                       };
+
+                       addButton.SetBinding(Button.CommandProperty, "AddItemCommand");
+
+                       buttonsLayout.Children.Add(addButton);
+
+                       _collectionView = new CollectionView
+                       {
+                               BackgroundColor = Color.LightGreen,
+                               ItemTemplate = CreateDataGridTemplate(),
+                               SelectionMode = SelectionMode.None
+                       };
+
+                       _collectionView.SetBinding(ItemsView.ItemsSourceProperty, "Items");
+
+                       var stack = new StackLayout();
+
+                       stack.Children.Add(instructions);
+                       stack.Children.Add(buttonsLayout);
+                       stack.Children.Add(_collectionView);
+
+                       Content = stack;
+               }
+
+               DataTemplate CreateDataGridTemplate()
+               {
+                       var template = new DataTemplate(() =>
+                       {
+                               var grid = new Grid();
+                               var cell = new Label();
+                               cell.SetBinding(Label.TextProperty, "Text");
+                               cell.FontSize = 20;
+                               cell.BackgroundColor = Color.LightBlue;
+                               grid.Children.Add(cell);
+
+                               return grid;
+                       });
+
+                       return template;
+               }
+       }
+
+       [Preserve(AllMembers = true)]
+       public class Issue8200Model
+       {
+               public string Text { get; set; }
+       }
+
+       [Preserve(AllMembers = true)]
+       public class Issue8200ViewModel : BindableObject
+       {
+               Issue8200Collection _items;
+
+               public Issue8200ViewModel()
+               {
+                       LoadItems();
+               }
+
+               public Issue8200Collection Items
+               {
+                       get { return _items; }
+                       set
+                       {
+                               _items = value;
+                               OnPropertyChanged();
+                       }
+               }
+
+               public ICommand AddItemCommand => new Command(AddItem);
+
+               void LoadItems()
+               {
+                       Items = new Issue8200Collection();
+
+                       for (int i = 0; i < 30; i++)
+                       {
+                               Items.AddNewItem(new Issue8200Model { Text = i.ToString() });
+                       }
+               }
+
+               void AddItem()
+               {
+                       var itemsCount = Items.Count();
+                       Items.AddNewItem(new Issue8200Model { Text = itemsCount.ToString() });
+               }
+       }
+
+       public class Issue8200Collection : IEnumerable<Issue8200Model>, INotifyCollectionChanged
+       {
+               readonly List<Issue8200Model> _internalList = new List<Issue8200Model>();
+
+               public IEnumerable<Issue8200Model> GetItems()
+               {
+                       foreach (var item in _internalList)
+                       {
+                               yield return item;
+                       }
+               }
+
+               public IEnumerator<Issue8200Model> GetEnumerator()
+               {
+                       return GetItems().GetEnumerator();
+               }
+
+               public void AddNewItem(Issue8200Model newItem)
+               {
+                       int index = _internalList.Count;
+                       _internalList.Add(newItem);
+                       OnCollectionChanged(new NotifyCollectionChangedEventArgs(NotifyCollectionChangedAction.Add, newItem, index));
+               }
+
+               public event NotifyCollectionChangedEventHandler CollectionChanged;
+
+               protected void OnCollectionChanged(NotifyCollectionChangedEventArgs e)
+               {
+                       CollectionChanged?.Invoke(this, e);
+               }
+
+               IEnumerator IEnumerable.GetEnumerator()
+               {
+                       return GetEnumerator();
+               }
+       }
+}
\ No newline at end of file
index e1b207c..085a618 100644 (file)
       <DependentUpon>Issue7886.xaml</DependentUpon>
     </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)Issue7898.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue8200.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
index a03fe0a..3053db0 100644 (file)
@@ -18,6 +18,8 @@ namespace Xamarin.Forms.Platform.Android
                        {
                                case IList _ when itemsSource is INotifyCollectionChanged:
                                        return new ObservableItemsSource(itemsSource as IList, notifier);
+                               case IEnumerable _ when itemsSource is INotifyCollectionChanged:
+                                       return new ObservableItemsSource(itemsSource as IEnumerable, notifier);
                                case IEnumerable<object> generic:
                                        return new ListSource(generic);
                        }
index 068ee0b..37ecb2b 100644 (file)
@@ -6,18 +6,18 @@ namespace Xamarin.Forms.Platform.Android
 {
        internal class ObservableItemsSource : IItemsViewSource
        {
-               readonly IList _itemsSource;
+               readonly IEnumerable _itemsSource;
                readonly ICollectionChangedNotifier _notifier;
                bool _disposed;
 
-               public ObservableItemsSource(IList itemSource, ICollectionChangedNotifier notifier)
+               public ObservableItemsSource(IEnumerable itemSource, ICollectionChangedNotifier notifier)
                {
-                       _itemsSource = itemSource;
+                       _itemsSource = itemSource as IList ?? itemSource as IEnumerable;
                        _notifier = notifier;
                        ((INotifyCollectionChanged)itemSource).CollectionChanged += CollectionChanged;
                }
 
-               public int Count => _itemsSource.Count + (HasHeader ? 1 : 0) + (HasFooter ? 1 : 0);
+               public int Count => ItemsCount() + (HasHeader ? 1 : 0) + (HasFooter ? 1 : 0);
 
                public bool HasHeader { get; set; }
                public bool HasFooter { get; set; }
@@ -39,9 +39,9 @@ namespace Xamarin.Forms.Platform.Android
 
                public int GetPosition(object item)
                {
-                       for (int n = 0; n < _itemsSource.Count; n++)
+                       for (int n = 0; n < ItemsCount(); n++)
                        {
-                               if (_itemsSource[n] == item)
+                               if (ElementAt(n) == item)
                                {
                                        return AdjustPositionForHeader(n);
                                }
@@ -52,7 +52,7 @@ namespace Xamarin.Forms.Platform.Android
 
                public object GetItem(int position)
                {
-                       return _itemsSource[AdjustIndexForHeader(position)];
+                       return ElementAt(AdjustIndexForHeader(position));
                }
 
                protected virtual void Dispose(bool disposing)
@@ -122,7 +122,7 @@ namespace Xamarin.Forms.Platform.Android
 
                void Add(NotifyCollectionChangedEventArgs args)
                {
-                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _itemsSource.IndexOf(args.NewItems[0]);
+                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]);
                        startIndex = AdjustPositionForHeader(startIndex);
                        var count = args.NewItems.Count;
 
@@ -163,7 +163,7 @@ namespace Xamarin.Forms.Platform.Android
 
                void Replace(NotifyCollectionChangedEventArgs args)
                {
-                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _itemsSource.IndexOf(args.NewItems[0]);
+                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]);
                        startIndex = AdjustPositionForHeader(startIndex);
                        var newCount = args.NewItems.Count;
 
@@ -187,5 +187,48 @@ namespace Xamarin.Forms.Platform.Android
                        // have to be updated. So we just have to use NotifyDataSetChanged and let the RecyclerView update everything
                        _notifier.NotifyDataSetChanged();
                }
+
+               internal int ItemsCount()
+               {
+                       if (_itemsSource is IList list)
+                               return list.Count;
+
+                       int count = 0;
+                       foreach (var item in _itemsSource)
+                               count++;
+                       return count;
+               }
+
+               internal object ElementAt(int index)
+               {
+                       if (_itemsSource is IList list)
+                               return list[index];
+
+                       int count = 0;
+                       foreach (var item in _itemsSource)
+                       {
+                               if (count == index)
+                                       return item;
+                               count++;
+                       }
+
+                       return -1;
+               }
+
+               internal int IndexOf(object item)
+               {
+                       if (_itemsSource is IList list)
+                               return list.IndexOf(item);
+
+                       int count = 0;
+                       foreach (var i in _itemsSource)
+                       {
+                               if (i == item)
+                                       return count;
+                               count++;
+                       }
+
+                       return -1;
+               }
        }
 }
\ No newline at end of file
index b38261b..98fd16d 100644 (file)
@@ -16,12 +16,15 @@ namespace Xamarin.Forms.Platform.iOS
 
                        switch (itemsSource)
                        {
-                               case INotifyCollectionChanged _:
+                               case IList _ when itemsSource is INotifyCollectionChanged:
                                        return new ObservableItemsSource(itemsSource as IList, collectionViewController);
-                               case IEnumerable _:
-                               default:
-                                       return new ListSource(itemsSource);
+                               case IEnumerable _ when itemsSource is INotifyCollectionChanged:
+                                       return new ObservableItemsSource(itemsSource as IEnumerable, collectionViewController);
+                               case IEnumerable<object> generic:
+                                       return new ListSource(generic);
                        }
+
+                       return new ListSource(itemsSource);
                }
 
                public static IItemsViewSource CreateGrouped(IEnumerable itemsSource, UICollectionViewController collectionViewController)
index fe2f166..f85623f 100644 (file)
@@ -121,7 +121,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                        for (int n = 0; n < _groupSource.Count; n++)
                        {
-                               if (_groupSource[n] is INotifyCollectionChanged && _groupSource[n] is IList list)
+                               if (_groupSource[n] is INotifyCollectionChanged && _groupSource[n] is IEnumerable list)
                                {
                                        _groups.Add(new ObservableItemsSource(list, _collectionViewController, n));
                                }
index 1d9d749..d493321 100644 (file)
@@ -12,10 +12,10 @@ namespace Xamarin.Forms.Platform.iOS
                readonly UICollectionView _collectionView;
                readonly bool _grouped;
                readonly int _section;
-               readonly IList _itemsSource;
+               readonly IEnumerable _itemsSource;
                bool _disposed;
 
-               public ObservableItemsSource(IList itemSource, UICollectionViewController collectionViewController, int group = -1)
+               public ObservableItemsSource(IEnumerable itemSource, UICollectionViewController collectionViewController, int group = -1)
                {
                        _collectionViewController = collectionViewController;
                        _collectionView = _collectionViewController.CollectionView;
@@ -23,14 +23,14 @@ namespace Xamarin.Forms.Platform.iOS
                        _section = group < 0 ? 0 : group;
                        _grouped = group >= 0;
 
-                       _itemsSource = itemSource;
-                       
+                       _itemsSource = itemSource as IList ?? itemSource as IEnumerable;
+
                        ((INotifyCollectionChanged)itemSource).CollectionChanged += CollectionChanged;
                }
 
-               public int Count => _itemsSource.Count;
+               public int Count => ItemsCount();
 
-               public object this[int index] => _itemsSource[index];
+               public object this[int index] => ElementAt(index);
 
                public void Dispose()
                {
@@ -52,7 +52,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                public int ItemCountInGroup(nint group)
                {
-                       return _itemsSource.Count;
+                       return ItemsCount();
                }
 
                public object Group(NSIndexPath indexPath)
@@ -62,7 +62,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                public NSIndexPath GetIndexForItem(object item)
                {
-                       for (int n = 0; n < _itemsSource.Count; n++)
+                       for (int n = 0; n < ItemsCount(); n++)
                        {
                                if (this[n] == item)
                                {
@@ -75,7 +75,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                public int GroupCount => 1;
 
-               public int ItemCount => _itemsSource.Count;
+               public int ItemCount => ItemsCount();
 
                public object this[NSIndexPath indexPath]
                {
@@ -141,7 +141,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                void Add(NotifyCollectionChangedEventArgs args)
                {
-                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _itemsSource.IndexOf(args.NewItems[0]);
+                       var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]);
                        var count = args.NewItems.Count;
 
                        if (NotLoadedYet())
@@ -198,7 +198,7 @@ namespace Xamarin.Forms.Platform.iOS
 
                        if (newCount == args.OldItems.Count)
                        {
-                               var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : _itemsSource.IndexOf(args.NewItems[0]);
+                               var startIndex = args.NewStartingIndex > -1 ? args.NewStartingIndex : IndexOf(args.NewItems[0]);
 
                                // We are replacing one set of items with a set of equal size; we can do a simple item range update
                                _collectionView.ReloadItems(CreateIndexesFrom(startIndex, newCount));
@@ -228,5 +228,48 @@ namespace Xamarin.Forms.Platform.iOS
                        var end = Math.Max(args.OldStartingIndex, args.NewStartingIndex) + count;
                        _collectionView.ReloadItems(CreateIndexesFrom(start, end));
                }
+
+               internal int ItemsCount()
+               {
+                       if (_itemsSource is IList list)
+                               return list.Count;
+
+                       int count = 0;
+                       foreach (var item in _itemsSource)
+                               count++;
+                       return count;
+               }
+
+               internal object ElementAt(int index)
+               {
+                       if (_itemsSource is IList list)
+                               return list[index];
+
+                       int count = 0;
+                       foreach (var item in _itemsSource)
+                       {
+                               if (count == index)
+                                       return item;
+                               count++;
+                       }
+
+                       return -1;
+               }
+
+               internal int IndexOf(object item)
+               {
+                       if (_itemsSource is IList list)
+                               return list.IndexOf(item);
+
+                       int count = 0;
+                       foreach (var i in _itemsSource)
+                       {
+                               if (i == item)
+                                       return count;
+                               count++;
+                       }
+
+                       return -1;
+               }
        }
 }