Merge pull request #7111 from unoplatform/dev/jela/xload-race

fix(xLoad): [Android] Fix invalid x:Load reentrancy issue

src/Uno.UI.RemoteControl.VS/Uno.UI.RemoteControl.VS.csproj

@@ -68,4 +68,7 @@
 		</ItemGroup>
 	</Target>
 
+	<!-- VS2022 build issue -->
+	<Target Name="GetTargetPath" />
+
 </Project>

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml

@@ -0,0 +1,17 @@
+<UserControl
+    x:Class="Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls.When_xLoad_Visibility_While_Materializing"
+    xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
+	  xmlns:local="using:Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls"
+    xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
+    xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
+    xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
+    mc:Ignorable="d">
+
+	<Grid>
+		<local:When_xLoad_Visibility_While_Materializing_Content
+			x:Name="item1"
+			x:FieldModifier="public"
+			x:Load="false"
+			Visibility="{x:Bind Model.IsVisible, Mode=OneWay, FallbackValue=false}" />
+	</Grid>
+</UserControl>

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Controls/When_xLoad_Visibility_While_Materializing.xaml.cs

@@ -0,0 +1,64 @@
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices.WindowsRuntime;
+using Windows.Foundation;
+using Windows.Foundation.Collections;
+using Windows.UI.Xaml;
+using Windows.UI.Xaml.Controls;
+using Windows.UI.Xaml.Controls.Primitives;
+using Windows.UI.Xaml.Data;
+using Windows.UI.Xaml.Input;
+using Windows.UI.Xaml.Media;
+using Windows.UI.Xaml.Navigation;
+
+// The User Control item template is documented at https://go.microsoft.com/fwlink/?LinkId=234236
+
+namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml.Controls
+{
+	public sealed partial class When_xLoad_Visibility_While_Materializing : UserControl
+	{
+		public When_xLoad_Visibility_While_Materializing()
+		{
+			Model = new Model();
+
+			Model.PropertyChanged += delegate {
+				FindName("item1");
+			};
+
+			this.InitializeComponent();
+		}
+
+		public Model Model { get; }
+	}
+
+	public class Model : System.ComponentModel.INotifyPropertyChanged
+	{
+		public event System.ComponentModel.PropertyChangedEventHandler PropertyChanged;
+
+		private bool _isVisible;
+
+		public bool IsVisible
+		{
+			get { return _isVisible; }
+			set
+			{
+				_isVisible = value;
+
+				PropertyChanged.Invoke(this, new System.ComponentModel.PropertyChangedEventArgs(nameof(IsVisible)));
+			}
+		}
+
+	}
+
+	public partial class When_xLoad_Visibility_While_Materializing_Content : UserControl
+	{
+		public When_xLoad_Visibility_While_Materializing_Content()
+		{
+			Instances++;
+		}
+
+		public static int Instances { get; set; }
+	}
+}

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml/Given_xLoad.cs

@@ -10,6 +10,7 @@
 namespace Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml
 {
 	[TestClass]
+	[RunsOnUIThread]
 	public class Given_xLoad
 	{
 		[TestMethod]
@@ -47,6 +48,24 @@ public async Task When_xLoad_xBind()
 
 			Assert.IsFalse((parent.Child as ElementStub).Load);
 		}
+
+		[TestMethod]
+		[RunsOnUIThread]
+		public void When_xLoad_Visibility_While_Materializing()
+		{
+			var SUT = new When_xLoad_Visibility_While_Materializing();
+
+			Assert.AreEqual(0, When_xLoad_Visibility_While_Materializing_Content.Instances);
+
+			TestServices.WindowHelper.WindowContent = SUT;
+
+			Assert.AreEqual(0, When_xLoad_Visibility_While_Materializing_Content.Instances);
+
+			SUT.Model.IsVisible = true;
+
+			Assert.AreEqual(1, When_xLoad_Visibility_While_Materializing_Content.Instances);
+		}
+
 	}
 }
 #endif

src/Uno.UI/UI/Xaml/ElementStub.cs

@@ -45,6 +45,14 @@ private View _content
 		private View _content;
 #endif
 
+		/// <summary>
+		/// Ensures that materialization handles reentrancy properly.
+		/// This scenario can happen on android specifically because the Parent
+		/// property is not immediately set to null once a view is removed from
+		/// the tree.
+		/// </summary>
+		private bool _isMaterializing;
+
 		/// <summary>
 		/// A delegate used to raise materialization changes in <see cref="ElementStub.MaterializationChanged"/>
 		/// </summary>
@@ -144,22 +152,36 @@ public void Materialize()
 
 		private void Materialize(bool isVisibilityChanged)
 		{
-			if (_content == null)
+			if (_content == null && !_isMaterializing)
 			{
-				_content = SwapViews(oldView: (FrameworkElement)this, newViewProvider: ContentBuilder);
-				var targetDependencyObject = _content as DependencyObject;
-
-				if (isVisibilityChanged && targetDependencyObject != null)
+#if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783
+				try
 				{
-					var visibilityProperty = GetVisibilityProperty(_content);
+#endif
+					_isMaterializing = true;
 
-					// Set the visibility at the same precedence it was currently set with on the stub.
-					var precedence = this.GetCurrentHighestValuePrecedence(visibilityProperty);
+					_content = SwapViews(oldView: (FrameworkElement)this, newViewProvider: ContentBuilder);
+					var targetDependencyObject = _content as DependencyObject;
 
-					targetDependencyObject.SetValue(visibilityProperty, Visibility.Visible, precedence);
-				}
+					if (isVisibilityChanged && targetDependencyObject != null)
+					{
+						var visibilityProperty = GetVisibilityProperty(_content);
 
-				MaterializationChanged?.Invoke(this);
+						// Set the visibility at the same precedence it was currently set with on the stub.
+						var precedence = this.GetCurrentHighestValuePrecedence(visibilityProperty);
+
+						targetDependencyObject.SetValue(visibilityProperty, Visibility.Visible, precedence);
+					}
+
+					MaterializationChanged?.Invoke(this);
+
+#if !HAS_EXPENSIVE_TRYFINALLY // Try/finally incurs a very large performance hit in mono-wasm - https://github.com/dotnet/runtime/issues/50783
+				}
+				finally
+				{
+					_isMaterializing = false;
+				}
+#endif
 			}
 		}