From cbd89a345f88db1b27fe352530dd270ed6979347 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javier=20Su=C3=A1rez?= <javiersuarezruiz@hotmail.com>
Date: Wed, 3 Feb 2021 13:15:02 +0100
Subject: [PATCH] Fix crash adding items to CarouselView on Android (#13617)
 fixes #13616

---
 .../Issue13616.xaml                           |  75 ++++++++++++
 .../Issue13616.xaml.cs                        | 115 ++++++++++++++++++
 ...rin.Forms.Controls.Issues.Shared.projitems |   4 +
 .../CollectionView/CarouselViewRenderer.cs    |  13 +-
 4 files changed, 202 insertions(+), 5 deletions(-)
 create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml
 create mode 100644 Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml.cs

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml
new file mode 100644
index 00000000000..a10c05bf486
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml
@@ -0,0 +1,75 @@
+<?xml version="1.0" encoding="utf-8" ?>
+<local:TestContentPage
+    xmlns="http://xamarin.com/schemas/2014/forms"
+    xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
+    xmlns:d="http://xamarin.com/schemas/2014/forms/design"
+    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
+    mc:Ignorable="d"
+    Title="Test 13616" xmlns:local="using:Xamarin.Forms.Controls"
+    x:Class="Xamarin.Forms.Controls.Issues.Issue13616">
+    <StackLayout>
+        <Label
+            Padding="12"
+            BackgroundColor="Black"
+            TextColor="White"
+            Text="Tap the Button to add a new Item, without exceptions, the test has passed."/>
+        <StackLayout Padding="20,40,20,20">
+            <Button
+                AutomationId="AddItemButtonId"
+                Text="Add new Item"
+                CornerRadius="10"
+                Command="{Binding AddCardCommand}"
+                HeightRequest="40"
+                WidthRequest="200"
+                HorizontalOptions="CenterAndExpand"/>
+            <CarouselView
+                x:Name="carouselView"
+                AutomationId="CarouselViewId"
+                HorizontalScrollBarVisibility="Never"
+                ItemsSource="{Binding Items}"
+                PeekAreaInsets="15"
+                Loop="False"
+                HeightRequest="250"
+                VerticalOptions="Start">
+                <CarouselView.ItemsLayout>
+                    <LinearItemsLayout
+                        Orientation="Horizontal"
+                        SnapPointsAlignment="Center"
+                        SnapPointsType="MandatorySingle"
+                        ItemSpacing="6"/>
+                </CarouselView.ItemsLayout>
+                <CarouselView.ItemTemplate>
+                    <DataTemplate>
+                        <Frame
+                            HeightRequest="250"
+                            CornerRadius="10"
+                            HasShadow="False"
+                            Padding="0"
+                            IsClippedToBounds="True"
+                            Margin="10">
+                            <Grid
+                                BackgroundColor="{Binding Color}">
+                                <Grid Margin="20">
+                                    <Grid.RowDefinitions>
+                                        <RowDefinition Height="Auto" />
+                                        <RowDefinition Height="*" />
+                                    </Grid.RowDefinitions>
+                                    <Label
+                                        Grid.Row="0"
+                                        Margin="0,0,0,20"
+                                        Text="{Binding Name}"
+                                        TextColor="Black" />
+                                    <Label
+                                        Grid.Row="1"
+                                        Margin="0,0,0,20"
+                                        Text="{Binding Desc}"
+                                        TextColor="Black" />
+                                </Grid>
+                            </Grid>
+                        </Frame>
+                    </DataTemplate>
+                </CarouselView.ItemTemplate>
+            </CarouselView>
+        </StackLayout>
+    </StackLayout>
+</local:TestContentPage>
\ No newline at end of file
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml.cs
new file mode 100644
index 00000000000..46ab31d50ea
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue13616.xaml.cs
@@ -0,0 +1,115 @@
+using System.Collections.ObjectModel;
+using System.Linq;
+using System.Windows.Input;
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+using Xamarin.Forms.Core.UITests;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+#if UITEST
+	[Category(UITestCategories.CarouselView)]
+#endif
+    [Preserve(AllMembers = true)]
+    [Issue(IssueTracker.Github, 13616,
+        "[Bug] After updating XF 5.0.0.1931 getting Java.Lang.IllegalArgumentException: Invalid target position at Java.Interop.JniEnvironment+InstanceMethods.CallVoidMethod",
+		PlatformAffected.Android)]
+	public partial class Issue13616 : TestContentPage
+	{
+		public Issue13616()
+		{
+#if APP
+			InitializeComponent();
+            BindingContext = new Issue13616ViewModel();
+#endif
+        }
+
+        protected override void Init()
+		{
+        }
+
+#if UITEST && __ANDROID__
+        [Test]
+        public void Issue13616Test()
+        {
+            RunningApp.WaitForElement("AddItemButtonId");
+            RunningApp.Tap("AddItemButtonId");
+            RunningApp.WaitForElement("CarouselViewId");
+        }
+#endif
+    }
+
+    [Preserve(AllMembers = true)]
+    public class Issue13616Model
+    {
+        public string Name { get; set; }
+        public string Desc { get; set; }
+        public Color Color { get; set; }
+        public double Scale { get; set; }
+    }
+
+    [Preserve(AllMembers = true)]
+    public class Issue13616ViewModel : BindableObject
+    {
+        int _i = 4;
+        ObservableCollection<Issue13616Model> _myList { get; set; } = new ObservableCollection<Issue13616Model>();
+
+        public Issue13616ViewModel()
+        {
+            Items = new ObservableCollection<Issue13616Model>
+            {
+                new Issue13616Model
+                {
+                    Name = "Card 1",
+                    Desc = "Card Holder Name 1",
+                    Color = Color.Yellow
+                },
+                new Issue13616Model
+                {
+                    Name = "Card 2",
+                    Desc = "Card Holder Name 2",
+                    Color = Color.Orange
+                },
+                new Issue13616Model
+                {
+                    Name = "Card 3",
+                    Desc = "Card Holder Name 3",
+                    Color = Color.Red
+                }
+            };
+        }
+
+        public ObservableCollection<Issue13616Model> Items
+        {
+            get
+            {
+                return _myList;
+            }
+            set
+            {
+                _myList = value;
+                OnPropertyChanged(nameof(Items));
+            }
+        }
+
+        public ICommand AddCardCommand { get { return new Command(AddCard); } }
+
+        void AddCard()
+        {
+            var tempList = Items.ToList();
+            tempList.Add(new Issue13616Model
+            {
+                Name = "Card " + _i,
+                Desc = "Card Holder Name " + _i,
+                Color = Color.Blue
+            });
+            Items = new ObservableCollection<Issue13616Model>(tempList);
+            _i++;
+        }
+    }
+}
\ No newline at end of file
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
index c9193b6a7bc..dd67bc6c1dc 100644
--- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems
@@ -1714,6 +1714,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue13436.xaml.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue8701.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue13390.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue13616.xaml.cs" />
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla22229.xaml">
@@ -2116,6 +2117,9 @@
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue13436.xaml">
       <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
     </EmbeddedResource>
+    <EmbeddedResource Include="$(MSBuildThisFileDirectory)Issue13616.xaml">
+      <Generator>MSBuild:UpdateDesignTimeXaml</Generator>
+    </EmbeddedResource>
   </ItemGroup>
   <ItemGroup>
     <EmbeddedResource Include="$(MSBuildThisFileDirectory)Bugzilla27417Xaml.xaml">
diff --git a/Xamarin.Forms.Platform.Android/CollectionView/CarouselViewRenderer.cs b/Xamarin.Forms.Platform.Android/CollectionView/CarouselViewRenderer.cs
index 4bf5d6a63d6..bef659c869c 100644
--- a/Xamarin.Forms.Platform.Android/CollectionView/CarouselViewRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/CollectionView/CarouselViewRenderer.cs
@@ -205,12 +205,15 @@ protected override void ScrollTo(ScrollToRequestEventArgs args)
 
 			if (_carouselViewLoopManager == null)
 				return;
-			//Special case here
-			//We could have a race condition where we are scrolling our collection to center the first item
-			//And at the same time the user is requesting we go to a particular item
-			if (position == -1 && Carousel.Loop)
+
+			// Special case here
+			// We could have a race condition where we are scrolling our collection to center the first item
+			// And at the same time the user is requesting we go to a particular item
+			if (position == -1)
 			{
-				_carouselViewLoopManager.AddPendingScrollTo(args);
+				if (Carousel.Loop)
+					_carouselViewLoopManager.AddPendingScrollTo(args);
+
 				return;
 			}