[Android, iOS] Fixes layout compression causes (#3698) fixes #3624

* [Android, iOS] Fixes layout compression causes

* [iOS] improve UpdateNewElement method

* Fix build error hopefully

diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3624.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3624.cs
new file mode 100644 (file)
index 0000000..be15293
--- /dev/null
+++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3624.cs
@@ -0,0 +1,146 @@
+using System;
+using Xamarin.Forms.CustomAttributes;
+using System.Collections.ObjectModel;
+using Xamarin.Forms.Internals;
+
+namespace Xamarin.Forms.Controls.Issues
+{
+       [Preserve (AllMembers = true)]
+       [Issue (IssueTracker.Github, 3624, "Layout Compression causes the app to crash when scrolling a ListView with ListViewCachingStrategy.RetainElement")]
+       public class Issue3624 : TestContentPage // or TestMasterDetailPage, etc ...
+       {
+               protected override void Init ()
+               {
+                       Content = new StackLayout
+                       {
+                               Children =
+                               {
+                                       new Button
+                                       {
+                                               Text = "With Compression (type 1) --- Crash while scrolling",
+                                               Command = new Command(() => {
+                                                       TestPage.ShouldUseCompressedLayout = true;
+                                                       Navigation.PushAsync(new TestPage());
+                                               })
+                                       },
+                                       new Button
+                                       {
+                                               Text = "Without Compression (type 1) --- Scrolls fine",
+                                               Command = new Command(() => {
+                                                       TestPage.ShouldUseCompressedLayout = false;
+                                                       Navigation.PushAsync(new TestPage());
+                                               })
+                                       },
+                                       new Button
+                                       {
+                                               Text = "With Compression (type 2) --- Crash while scrolling",
+                                               Command = new Command(() => {
+                                                       TestPage.ShouldUseCompressedLayout = true;
+                                                       Navigation.PushAsync(new TestPage(useType2: true));
+                                               })
+                                       },
+                                       new Button
+                                       {
+                                               Text = "Without Compression (type 2) --- Scrolls fine",
+                                               Command = new Command(() => {
+                                                       TestPage.ShouldUseCompressedLayout = false;
+                                                       Navigation.PushAsync(new TestPage(useType2: true));
+                                               })
+                                       }
+                               }
+                       };
+               }
+
+               [Preserve(AllMembers = true)]
+               public class TestPage : ContentPage
+               {
+                       public static bool ShouldUseCompressedLayout = false;
+                       public ObservableCollection<VeggieViewModel> veggies { get; set; }
+
+                       public TestPage(bool useType2 = false)
+                       {
+                               veggies = new ObservableCollection<VeggieViewModel>();
+                               ListView listView = new ListView
+                               {
+                                       RowHeight = 60
+                               };
+                               Title = ShouldUseCompressedLayout ? "Scroll & crash" : "Scrolls fine";
+                               listView.ItemTemplate = new DataTemplate(useType2 ? typeof(TestCell2) : typeof(TestCell1));
+
+                               for (int i = 0; i < 1000; i++)
+                               {
+                                       switch (i % 3)
+                                       {
+                                               case 0:
+                                                       veggies.Add(new VeggieViewModel { Name = $"#{i} Tomato" });
+                                                       break;
+                                               case 1:
+                                                       veggies.Add(new VeggieViewModel { Name = $"#{i} Romaine Lettuce" });
+                                                       break;
+                                               case 2:
+                                                       veggies.Add(new VeggieViewModel { Name = $"#{i} Zucchini" });
+                                                       break;
+                                       }
+                               }
+
+                               listView.ItemsSource = veggies;
+                               Content = listView;
+                       }
+               }
+
+               [Preserve(AllMembers = true)]
+               public class TestCell1 : ViewCell
+               {
+                       public TestCell1()
+                       {
+                               var nameLabel = new Label();
+                               var verticaLayout = new StackLayout();
+                               Forms.CompressedLayout.SetIsHeadless(verticaLayout, TestPage.ShouldUseCompressedLayout);
+                               var horizontalLayout = new StackLayout() { BackgroundColor = Color.Olive };
+
+                               nameLabel.SetBinding(Label.TextProperty, new Binding("Name"));
+
+                               horizontalLayout.Orientation = StackOrientation.Horizontal;
+                               horizontalLayout.HorizontalOptions = LayoutOptions.Fill;
+
+                               verticaLayout.Children.Add(nameLabel);
+                               horizontalLayout.Children.Add(verticaLayout);
+
+                               View = horizontalLayout;
+                       }
+               }
+
+               [Preserve(AllMembers = true)]
+               public partial class TestCell2 : ViewCell
+               {
+                       public TestCell2()
+                       {
+                               var layout = new AbsoluteLayout();
+                               var label = new Label();
+                               label.SetBinding(Label.TextProperty, new Binding("Name"));
+                               var stack = new StackLayout
+                               {
+                                       Children = { label }
+                               };
+                               var grid = new Grid()
+                               {
+                                       ColumnDefinitions = new ColumnDefinitionCollection {
+                                               new ColumnDefinition { Width = GridLength.Star }
+                                       }
+                               };
+                               Forms.CompressedLayout.SetIsHeadless(stack, true);
+                               grid.AddChild(stack, 0, 0);
+                               layout.Children.Add(grid, new Rectangle(0,0,1,1), AbsoluteLayoutFlags.All);
+
+                               View = layout;
+                               Forms.CompressedLayout.SetIsHeadless(stack, TestPage.ShouldUseCompressedLayout);
+                       }
+               }
+
+               [Preserve(AllMembers = true)]
+               public class VeggieViewModel
+               {
+                       public string Name { get; set; }
+               }
+       }
+}

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 d93a699..8da5d84 100644 (file)
--- 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
@@ -550,6 +550,7 @@
     </Compile>
     <Compile Include="$(MSBuildThisFileDirectory)Issue1664.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1680.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue3624.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1682.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1685.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1698.cs" />

diff --git a/Xamarin.Forms.Platform.Android/VisualElementPackager.cs b/Xamarin.Forms.Platform.Android/VisualElementPackager.cs
index 4ef3532..72e5ae9 100644 (file)
--- a/Xamarin.Forms.Platform.Android/VisualElementPackager.cs
+++ b/Xamarin.Forms.Platform.Android/VisualElementPackager.cs
@@ -37,9 +37,12 @@ namespace Xamarin.Forms.Platform.Android
                        _childReorderedHandler = OnChildrenReordered;
 
                        _renderer = renderer;
-                       _renderer.ElementChanged += (sender, args) => SetElement(args.OldElement, args.NewElement);
+                       _renderer.ElementChanged += OnElementChanged;
                }
 
+               void OnElementChanged(object sender, VisualElementChangedEventArgs e)
+                       => SetElement(e.OldElement, e.NewElement);
+
                public void Dispose()
                {
                        Dispose(true);
@@ -70,6 +73,7 @@ namespace Xamarin.Forms.Platform.Android
                                        _childPackagers = null;
                                }
 
+                               _renderer.ElementChanged -= OnElementChanged;
                                if (_renderer.Element != null)
                                {
                                        _renderer.Element.ChildAdded -= _childAddedHandler;
@@ -258,7 +262,7 @@ namespace Xamarin.Forms.Platform.Android
                                for (var i = 0; i < newChildren.Count; i++)
                                {
                                        IVisualElementRenderer oldRenderer = null;
-                                       if (oldChildren != null && sameChildrenTypes)
+                                       if (oldChildren != null && sameChildrenTypes && _childViews != null)
                                                oldRenderer = _childViews[i];
 
                                        AddChild((VisualElement)newChildren[i], oldRenderer, pool, sameChildrenTypes);

diff --git a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs
index c5f7b51..5e7c718 100644 (file)
--- a/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs
+++ b/Xamarin.Forms.Platform.Android/VisualElementRenderer.cs
@@ -262,7 +262,11 @@ namespace Xamarin.Forms.Platform.Android
                protected virtual void OnElementChanged(ElementChangedEventArgs<TElement> e)
                {
                        var args = new VisualElementChangedEventArgs(e.OldElement, e.NewElement);
-                       foreach (EventHandler<VisualElementChangedEventArgs> handler in _elementChangedHandlers)
+
+                       // The list of event handlers can be changed inside the handlers. (ex.: are used CompressedLayout)
+                       // To avoid an exception, a copy of the handlers is called.
+                       var handlers = _elementChangedHandlers.ToArray();
+                       foreach (var handler in handlers)
                                handler(this, args);
 
                        ElementChanged?.Invoke(this, e);

diff --git a/Xamarin.Forms.Platform.iOS/RendererPool.cs b/Xamarin.Forms.Platform.iOS/RendererPool.cs
index 6396018..421ee04 100644 (file)
--- a/Xamarin.Forms.Platform.iOS/RendererPool.cs
+++ b/Xamarin.Forms.Platform.iOS/RendererPool.cs
@@ -52,14 +52,16 @@ namespace Xamarin.Forms.Platform.MacOS
 
                        var sameChildrenTypes = true;
 
-                       var oldChildren = ((IElementController)_oldElement).LogicalChildren;
+                       var oldChildren = _oldElement.LogicalChildren;
+                       var oldNativeChildren = _parent.NativeView.Subviews;
                        var newChildren = ((IElementController)newElement).LogicalChildren;
 
-                       if (oldChildren.Count == newChildren.Count)
+                       if (oldChildren.Count == newChildren.Count && oldNativeChildren.Length >= oldChildren.Count)
                        {
                                for (var i = 0; i < oldChildren.Count; i++)
                                {
-                                       if (oldChildren[i].GetType() != newChildren[i].GetType())
+                                       var oldChildType = (oldNativeChildren[i] as IVisualElementRenderer)?.Element.GetType();
+                                       if (oldChildType != newChildren[i].GetType())
                                        {
                                                sameChildrenTypes = false;
                                                break;