From 17e78556e8a3ec9d7667cfc74a0f479f74b775ab Mon Sep 17 00:00:00 2001
From: Martin Zikmund <martin@zikmund.dev>
Date: Tue, 5 Sep 2023 17:32:37 +0200
Subject: [PATCH] feat(NavigationView): Include Fix issue with exception being
 thrown during cleanup of NavigationView

---
 .../NavigationView/NavigationViewTests.cs     | 32 +++++++++++++++++++
 .../Controls/NavigationView/NavigationView.cs | 31 ++++++++++++++++--
 2 files changed, 61 insertions(+), 2 deletions(-)

diff --git a/src/Uno.UI.RuntimeTests/MUX/Microsoft_UI_Xaml_Controls/NavigationView/NavigationViewTests.cs b/src/Uno.UI.RuntimeTests/MUX/Microsoft_UI_Xaml_Controls/NavigationView/NavigationViewTests.cs
index 8826757b0e7c..6c2271d53d1b 100644
--- a/src/Uno.UI.RuntimeTests/MUX/Microsoft_UI_Xaml_Controls/NavigationView/NavigationViewTests.cs
+++ b/src/Uno.UI.RuntimeTests/MUX/Microsoft_UI_Xaml_Controls/NavigationView/NavigationViewTests.cs
@@ -1333,5 +1333,37 @@ async Task SetPaneConfigAndVerifyToolTips(NavigationViewPaneDisplayMode paneDisp
 				}
 			});
 		}
+
+		[TestMethod]
+		public void VerifyNVIOutlivingNVDoesNotCrash()
+		{
+			NavigationViewItem menuItem1 = null;
+			RunOnUIThread.Execute(() =>
+			{
+				var navView = new NavigationView();
+				menuItem1 = new NavigationViewItem();
+				navView.MenuItems.Add(menuItem1);
+				Content = navView;
+				Content.UpdateLayout();
+				navView.MenuItems.Clear();
+				Content = menuItem1;
+				Content.UpdateLayout();
+			});
+
+			IdleSynchronizer.Wait();
+
+			RunOnUIThread.Execute(() =>
+			{
+				GC.Collect();
+			});
+
+			IdleSynchronizer.Wait();
+
+			RunOnUIThread.Execute(() =>
+			{
+				// NavigationView has a handler on NVI's IsSelected DependencyPropertyChangedEvent.
+				menuItem1.IsSelected = !menuItem1.IsSelected;
+			});
+		}
 	}
 }
diff --git a/src/Uno.UI/Microsoft/UI/Xaml/Controls/NavigationView/NavigationView.cs b/src/Uno.UI/Microsoft/UI/Xaml/Controls/NavigationView/NavigationView.cs
index d3622b8fad63..ac49aa4d8409 100644
--- a/src/Uno.UI/Microsoft/UI/Xaml/Controls/NavigationView/NavigationView.cs
+++ b/src/Uno.UI/Microsoft/UI/Xaml/Controls/NavigationView/NavigationView.cs
@@ -1,6 +1,6 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT License. See LICENSE in the project root for license information.
-// MUX Reference NavigationView.cpp, commit 4e2990d
+// MUX Reference NavigationView.cpp, commit d8d4f4f
 
 #pragma warning disable 105 // remove when moving to WinUI tree
 
@@ -13,6 +13,7 @@
 using Microsoft.UI.Xaml.Automation.Peers;
 using Microsoft.UI.Xaml.Controls.AnimatedVisuals;
 using Uno.Disposables;
+using Uno.Foundation.Logging;
 using Uno.UI.Helpers.WinUI;
 using Windows.ApplicationModel.Core;
 using Windows.Foundation;
@@ -3699,6 +3700,8 @@ private void SetNavigationViewItemRevokers(NavigationViewItem nvi)
 
 	private void ClearNavigationViewItemRevokers(NavigationViewItem nvi)
 	{
+		RevokeNavigationViewItemRevokers(nvi);
+
 		nvi.EventRevoker.Disposable = null;
 		m_itemsWithRevokerObjects.Remove(nvi);
 	}
@@ -3707,11 +3710,35 @@ private void ClearAllNavigationViewItemRevokers()
 	{
 		foreach (var nvi in m_itemsWithRevokerObjects)
 		{
-			nvi.EventRevoker.Disposable = null;
+			// ClearAllNavigationViewItemRevokers is only called in the destructor, where exceptions cannot be thrown.
+			// If the associated NV has not yet been cleaned up, we must detach these revokers or risk a call into freed
+			// memory being made.  However if they have been cleaned up these calls will throw. In this case we can ignore
+			// those exceptions.
+			try
+			{
+				RevokeNavigationViewItemRevokers(nvi);
+
+#if !HAS_UNO // TODO Uno specific: Revokers are implemented differently than in WinUI.
+				nvi.SetValue(s_NavigationViewItemRevokersProperty, nullptr);
+#endif
+			}
+			catch (Exception ex)
+			{
+				if (this.Log().IsEnabled(LogLevel.Error))
+				{
+					this.Log().LogError("Failed to clear revokers for NavigationViewItem.", ex);
+				}
+			}
 		}
 		m_itemsWithRevokerObjects.Clear();
 	}
 
+	private void RevokeNavigationViewItemRevokers(NavigationViewItem nvi)
+	{
+		// TODO Uno specific: Revokers are implemented differently than in WinUI.
+		nvi.EventRevoker.Disposable = null;
+	}
+
 	private void InvalidateTopNavPrimaryLayout()
 	{
 		if (m_appliedTemplate && IsTopNavigationView())