Init the flex item before adding it to the layout (#21737)

* Init the flex item before adding it to the layout

Fixes #21711

In some cases, adding an item before may trigger an update before
the add operation is complete. For example, adding a label with HTML
text may cause the layout to run while the label is being constructed.

To prevent that layout pass from crashing because the flex item is
not yet set, we make sure to first set the flex item, and then add it
to the layout. Then, when the layout pass runs, the item is ready to
be used.

This error might not typically appear in normal XAML layouts because
the items are first set, and then the entire page is inflated. However,
if the layout is part of a CollectionView or items are added
dynamically after the UI is loaded, this layout during add may occur.

* This too

* UI tests

* AutomationId is important :)

src/Controls/samples/Controls.Sample.UITests/Issues/Issue21711.cs

@@ -0,0 +1,94 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using Microsoft.Maui;
+using Microsoft.Maui.Controls;
+
+namespace Maui.Controls.Sample.Issues
+{
+	[Issue(IssueTracker.Github, 21711, "NullReferenceException from FlexLayout.InitItemProperties", PlatformAffected.iOS)]
+	public class Issue21711 : TestContentPage
+	{
+		protected override void Init()
+		{
+			FlexLayout flex = null!;
+			Content = new VerticalStackLayout
+			{
+				new Button
+				{
+					Text = "Add",
+					AutomationId = "Add",
+					Command = new Command(() =>
+					{
+						flex.Clear();
+						flex.Add(NewLabel(0));
+						flex.Add(NewLabel(1));
+
+						flex.Clear();
+						flex.Add(NewLabel(2));
+						flex.Add(NewLabel(3));
+					})
+				},
+				new Button
+				{
+					Text = "Insert",
+					AutomationId = "Insert",
+					Command = new Command(() =>
+					{
+						flex.Clear();
+						flex.Insert(0, NewLabel(1));
+						flex.Insert(0, NewLabel(0));
+
+						flex.Clear();
+						flex.Insert(0, NewLabel(3));
+						flex.Insert(0, NewLabel(2));
+					})
+				},
+				new Button
+				{
+					Text = "Update",
+					AutomationId = "Update",
+					Command = new Command(() =>
+					{
+						flex.Clear();
+						flex.Add(NewLabel(0));
+						flex[0] = NewLabel(1);
+
+						flex.Clear();
+						flex.Add(NewLabel(2));
+						flex[0] = NewLabel(3);
+					})
+				},
+				new Button
+				{
+					Text = "Remove",
+					AutomationId = "Remove",
+					Command = new Command(() =>
+					{
+						flex.Clear();
+						var label = NewLabel(0);
+						flex.Add(label);
+						flex.Remove(label);
+
+						flex.Clear();
+						label = NewLabel(1);
+						flex.Add(label);
+						flex.Remove(label);
+
+						flex.Add(NewLabel(2));
+					})
+				},
+				(flex = new FlexLayout { }),
+			};
+		}
+
+		Label NewLabel(int count) =>
+			new Label
+			{
+				Text = $"Item{count}",
+				AutomationId = $"Item{count}",
+				Background = Brush.Yellow,
+				TextType = TextType.Html
+			};
+	}
+}

src/Controls/src/Core/Layout/FlexLayout.cs

@@ -479,7 +479,7 @@ void InitItemProperties(IView view, Flex.Item item)
 
 		internal bool InMeasureMode { get; set; }
 
-		void AddFlexItem(IView child)
+		void AddFlexItem(int index, IView child)
 		{
 			if (_root == null)
 				return;
@@ -518,7 +518,7 @@ void AddFlexItem(IView child)
 				};
 			}
 
-			_root.InsertAt(Children.IndexOf(child), item);
+			_root.InsertAt(index, item);
 			SetFlexItem(child, item);
 		}
 
@@ -546,14 +546,8 @@ protected override ILayoutManager CreateLayoutManager()
 			return new FlexLayoutManager(this);
 		}
 
-		public Graphics.Rect GetFlexFrame(IView view)
-		{
-			return view switch
-			{
-				BindableObject bo => ((Flex.Item)bo.GetValue(FlexItemProperty)).GetFrame(),
-				_ => _viewInfo[view].FlexItem.GetFrame(),
-			};
-		}
+		public Graphics.Rect GetFlexFrame(IView view) =>
+			GetFlexItem(view).GetFrame();
 
 		void EnsureFlexItemPropertiesUpdated()
 		{
@@ -601,8 +595,10 @@ protected override void OnParentSet()
 		void PopulateLayout()
 		{
 			InitLayoutProperties(_root = new Flex.Item());
-			foreach (var child in Children)
-				AddFlexItem(child);
+			for (var i = 0; i < Children.Count; i++)
+			{
+				AddFlexItem(i, Children[i]);
+			}
 		}
 
 		void ClearLayout()
@@ -623,21 +619,21 @@ void InitLayoutProperties(Flex.Item item)
 
 		protected override void OnAdd(int index, IView view)
 		{
+			AddFlexItem(index, view);
 			base.OnAdd(index, view);
-			AddFlexItem(view);
 		}
 
 		protected override void OnInsert(int index, IView view)
 		{
+			AddFlexItem(index, view);
 			base.OnInsert(index, view);
-			AddFlexItem(view);
 		}
 
 		protected override void OnUpdate(int index, IView view, IView oldView)
 		{
-			base.OnUpdate(index, view, oldView);
 			RemoveFlexItem(oldView);
-			AddFlexItem(view);
+			AddFlexItem(index, view);
+			base.OnUpdate(index, view, oldView);
 		}
 
 		protected override void OnRemove(int index, IView view)

src/Controls/tests/UITests/Tests/Issues/Issue21711.cs

@@ -0,0 +1,57 @@
+using NUnit.Framework;
+using UITest.Appium;
+using UITest.Core;
+
+namespace Microsoft.Maui.AppiumTests.Issues
+{
+	public class Issue21711 : _IssuesUITest
+	{
+		public Issue21711(TestDevice device) : base(device)
+		{
+		}
+
+		public override string Issue => "NullReferenceException from FlexLayout.InitItemProperties";
+
+		[Test]
+		public void AddDoesNotCrash()
+		{
+			App.WaitForElement("Add");
+
+			App.Click("Add");
+
+			App.WaitForElement("Item2");
+			App.WaitForElement("Item3");
+		}
+
+		[Test]
+		public void InsertDoesNotCrash()
+		{
+			App.WaitForElement("Insert");
+
+			App.Click("Insert");
+
+			App.WaitForElement("Item2");
+			App.WaitForElement("Item3");
+		}
+
+		[Test]
+		public void UpdateDoesNotCrash()
+		{
+			App.WaitForElement("Update");
+
+			App.Click("Update");
+
+			App.WaitForElement("Item3");
+		}
+
+		[Test]
+		public void RemoveDoesNotCrash()
+		{
+			App.WaitForElement("Remove");
+
+			App.Click("Remove");
+
+			App.WaitForElement("Item2");
+		}
+	}
+}