Merge pull request unoplatform#16183 from ramezgerges/treeview_drag_nre

fix(Treeview): throwing NRE on Dragging

src/Uno.UI.RuntimeTests/Tests/Microsoft_UI_Xaml_Controls/Given_TreeView.cs

@@ -29,6 +29,8 @@
 using Uno.UI.RuntimeTests.Helpers;
 using System.ComponentModel;
 using Windows.UI.Input.Preview.Injection;
+using Microsoft.UI.Xaml.Data;
+using MUXControlsTestApp.Utilities;
 using Uno.UI.RuntimeTests.Tests.Windows_UI_Xaml_Controls;
 
 using TreeView = Microsoft/* UWP don't rename */.UI.Xaml.Controls.TreeView;
@@ -94,6 +96,132 @@ public async Task When_TreeViewItem_Dragged_Near_the_Edge()
 		}
 #endif
 
+		[TestMethod]
+#if __ANDROID__ || __IOS__
+		[Ignore("The behaviour of virtualizing panels is only accurate for managed virtualizing panels.")]
+#endif
+		public async Task When_TreeViewItem_Collapsed_Children_Removed_From_Tree()
+		{
+			var treeView = new TreeView
+			{
+				RootNodes =
+				{
+					new TreeViewNode
+					{
+						Content = "Parent",
+						IsExpanded = true,
+						Children =
+						{
+							new TreeViewNode
+							{
+								Content = "Child",
+								IsExpanded = true
+							}
+						}
+					}
+				}
+			};
+
+			await UITestHelper.Load(treeView);
+
+			Assert.AreEqual(2, treeView.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+
+			treeView.RootNodes[0].IsExpanded = false;
+			await WindowHelper.WaitForIdle();
+
+			Assert.AreEqual(1, treeView.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+
+			treeView.RootNodes[0].IsExpanded = true;
+			await WindowHelper.WaitForIdle();
+
+			Assert.AreEqual(2, treeView.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+		}
+
+#if HAS_UNO
+		// https://github.com/unoplatform/uno/issues/16041
+		[TestMethod]
+#if !HAS_INPUT_INJECTOR
+		[Ignore("InputInjector is only supported on skia")]
+#elif HAS_UNO && !HAS_UNO_WINUI
+		[Ignore("Fails on UWP branch as mixing WUX and MUX types causes errors.")]
+#endif
+		public async Task When_TreeViewItem_Dragged_NRE()
+		{
+			using var _ = new DisposableAction(() => TestableTreeViewItem.DraggingThrewException = false);
+			var treeView = new TreeView
+			{
+				RootNodes =
+				{
+					new TreeViewNode
+					{
+						Content = "Parent",
+						IsExpanded = true,
+						Children =
+						{
+							new TreeViewNode
+							{
+								Content = "Child",
+								IsExpanded = true
+							}
+						}
+					}
+				},
+				Style = new Style
+				{
+					BasedOn = Style.GetDefaultStyleForType(typeof(TreeView)),
+					Setters =
+					{
+						new Setter(Control.TemplateProperty, new ControlTemplate(() =>
+						{
+							var tvl = new CustomTreeViewList
+							{
+								Name = "ListControl"
+							};
+							tvl.SetBinding(FrameworkElement.BackgroundProperty, new TemplateBinding(new PropertyPath("Background")));
+							tvl.SetBinding(ItemsControl.ItemTemplateProperty, new TemplateBinding(new PropertyPath("ItemTemplate")));
+							tvl.SetBinding(ItemsControl.ItemTemplateSelectorProperty, new TemplateBinding(new PropertyPath("ItemTemplateSelector")));
+							tvl.SetBinding(ItemsControl.ItemTemplateSelectorProperty, new TemplateBinding(new PropertyPath("ItemTemplateSelector")));
+							tvl.SetBinding(ItemsControl.ItemContainerStyleProperty, new TemplateBinding(new PropertyPath("ItemContainerStyle")));
+							tvl.SetBinding(ItemsControl.ItemContainerStyleSelectorProperty, new TemplateBinding(new PropertyPath("ItemContainerStyleSelector")));
+							tvl.SetBinding(ItemsControl.ItemContainerTransitionsProperty, new TemplateBinding(new PropertyPath("ItemContainerTransitions")));
+							tvl.SetBinding(ListViewBase.CanDragItemsProperty, new TemplateBinding(new PropertyPath("CanDragItems")));
+							tvl.SetBinding(UIElement.AllowDropProperty, new TemplateBinding(new PropertyPath("AllowDrop")));
+							tvl.SetBinding(ListViewBase.CanReorderItemsProperty, new TemplateBinding(new PropertyPath("CanReorderItems")));
+							return tvl;
+						}))
+					}
+				}
+			};
+
+			await UITestHelper.Load(treeView);
+
+			var injector = InputInjector.TryCreate() ?? throw new InvalidOperationException("Failed to init the InputInjector");
+			using var mouse = injector.GetMouse();
+
+			// The bug is extremely hard to reproduce. Even slight changes to the numbers will fail to repro.
+			// The main idea is to have the first PointerEnteredEvent in the "Parent" TreeViewItem
+			// and the very next "frame" (i.e. pointer event) should jump to the the "Child" TreeViewItem.
+			// At least, that's the leading hypothesis.
+			// Note that removing the WaitForIdle's will throw an NRE and this is somewhat expected. The TreeView
+			// removes the children "nodes" immediately when a parent node starts dragging, but the children
+			// are asynchronously removed, so if an "about to be removed" child node receives a DragOver/DragEnter,
+			// the event callback will run some logic that assumes that the node is still in the TreeView, when it
+			// technically isn't (it's in the tree, but the TreeView has internally discarded it), which throws the NRE.
+			mouse.Press(treeView.TransformToVisual(null).TransformPoint(new Point(54, 17)));
+			await WindowHelper.WaitForIdle();
+			mouse.MoveTo(treeView.TransformToVisual(null).TransformPoint(new Point(54, 29)), 1);
+			await WindowHelper.WaitForIdle();
+			mouse.MoveTo(treeView.TransformToVisual(null).TransformPoint(new Point(86, 42)), 1);
+			await WindowHelper.WaitForIdle();
+			mouse.MoveTo(treeView.TransformToVisual(null).TransformPoint(new Point(118, 55)), 1);
+			await WindowHelper.WaitForIdle();
+			mouse.MoveTo(treeView.TransformToVisual(null).TransformPoint(new Point(121, 55)), 1);
+			await WindowHelper.WaitForIdle();
+
+			Assert.IsFalse(TestableTreeViewItem.DraggingThrewException);
+		}
+#endif
+
 		[TestMethod]
 		public async Task When_Setting_SelectedItem_DoesNotTakeEffect()
 		{
@@ -232,4 +360,44 @@ public string Label
 			string _label;
 		}
 	}
+
+#if HAS_UNO
+	partial class CustomTreeViewList : TreeViewList
+	{
+		protected override DependencyObject GetContainerForItemOverride()
+		{
+			var targetItem = new TestableTreeViewItem() { IsGeneratedContainer = true }; // Uno specific IsGeneratedContainer
+			return targetItem;
+		}
+	}
+
+	partial class TestableTreeViewItem : TreeViewItem
+	{
+		public static bool DraggingThrewException { get; set; }
+
+		protected override void OnDragEnter(Microsoft.UI.Xaml.DragEventArgs args)
+		{
+			try
+			{
+				base.OnDragEnter(args);
+			}
+			catch (Exception)
+			{
+				DraggingThrewException = true;
+			}
+		}
+
+		protected override void OnDragOver(Microsoft.UI.Xaml.DragEventArgs args)
+		{
+			try
+			{
+				base.OnDragOver(args);
+			}
+			catch (Exception)
+			{
+				DraggingThrewException = true;
+			}
+		}
+	}
+#endif
 }

src/Uno.UI.RuntimeTests/Tests/Windows_UI_Xaml_Controls/Given_ListViewBase.cs

@@ -3975,6 +3975,38 @@ bool IsVisible(object x) => x is UIElement uie
 #endif
 		}
 
+		[TestMethod]
+#if __ANDROID__ || __IOS__
+		[Ignore("The behaviour of virtualizing panels is only accurate for managed virtualizing panels.")]
+#endif
+		public async Task When_Item_Removed_From_ItemsSource_Item_Removed_From_Tree()
+		{
+			var source = new ObservableCollection<string>()
+			{
+				"1",
+				"2",
+				"3"
+			};
+
+			var SUT = new ListView
+			{
+				ItemsSource = source
+			};
+
+			await UITestHelper.Load(SUT);
+
+			Assert.AreEqual(3, SUT.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+			source.RemoveAt(2);
+			await WindowHelper.WaitForIdle();
+			Assert.AreEqual(2, SUT.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+			source.RemoveAt(1);
+			await WindowHelper.WaitForIdle();
+			Assert.AreEqual(1, SUT.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+			source.RemoveAt(0);
+			await WindowHelper.WaitForIdle();
+			Assert.AreEqual(0, SUT.FindVisualChildByType<ItemsStackPanel>().Children.Count);
+		}
+
 		[TestMethod]
 		public async Task When_ItemsSource_INCC_Reset()
 		{

src/Uno.UI/UI/Xaml/Controls/ListViewBase/VirtualizingPanelGenerator.managed.cs

@@ -203,6 +203,11 @@ public void RecycleViewForItem(FrameworkElement container, int index, bool clear
 						// https://github.com/unoplatform/uno/issues/11957
 						container.PrepareForRecycle();
 					}
+
+					if (container.Parent is Panel parent)
+					{
+						parent.Children.Remove(container);
+					}
 				}
 				else
 				{