[controls] fix memory leak in Window (#13400) (#14073)

Fixes: #10029

I could reproduce a memory leak by doing:

    App.Current.OpenWindow(new Window { Page = new ContentPage() });

I found that `App.Current._requestedWindows` just held onto every
`Window` object forever.

Note that we *can't* use `ConditionalWeakTable` here:

https://learn.microsoft.com/dotnet/api/system.runtime.compilerservices.conditionalweaktable-2

After initially trying this, I added a test showing not to use it. The
problem is if the `string` is GC'd the `Window` will be lost.

We can use `Dictionary<string, WeakReference<Window>>` instead.

The only other change I made was to use `Guid.ToString("n")` as it
removes `{`, `}`, and `-` characters from the string.

After these changes, I still found `Window` objects hanging around that
appeared to be held onto by many other objects.

Then I reviewed:

```csharp
void IWindow.Destroying()
{
    SendWindowDisppearing();
    Destroying?.Invoke(this, EventArgs.Empty);
    OnDestroying();

    Application?.RemoveWindow(this);
    // This wasn't here!
    Handler?.DisconnectHandler();
}
```

It appears that upon `Window`'s closing, we didn't have any code that
"disconnected" the MAUI handler. After this change, I could see `Window`
objects properly go away.

I added a unit test as well.
# Conflicts:
#	src/Controls/tests/Core.UnitTests/WindowsTests.cs

Co-authored-by: Shane Neuville <shneuvil@microsoft.com>

src/Controls/src/Core/HandlerImpl/Application/Application.Impl.cs

@@ -1,6 +1,7 @@
 #nullable enable
 using System;
 using System.Collections.Generic;
+using System.Runtime.CompilerServices;
 using Microsoft.Extensions.Logging;
 using Microsoft.Maui.ApplicationModel;
 using Microsoft.Maui.Devices;
@@ -10,10 +11,10 @@ namespace Microsoft.Maui.Controls
 {
 	public partial class Application : IApplication
 	{
-		const string MauiWindowIdKey = "__MAUI_WINDOW_ID__";
+		internal const string MauiWindowIdKey = "__MAUI_WINDOW_ID__";
 
 		readonly List<Window> _windows = new();
-		readonly Dictionary<string, Window> _requestedWindows = new();
+		readonly Dictionary<string, WeakReference<Window>> _requestedWindows = new();
 		ILogger<Application>? _logger;
 
 		ILogger<Application>? Logger =>
@@ -30,8 +31,14 @@ IWindow IApplication.CreateWindow(IActivationState? activationState)
 			// try get the window that is pending
 			if (activationState?.State?.TryGetValue(MauiWindowIdKey, out var requestedWindowId) ?? false)
 			{
-				if (requestedWindowId != null && _requestedWindows.TryGetValue(requestedWindowId, out var w))
-					window = w;
+				if (requestedWindowId != null && _requestedWindows.TryGetValue(requestedWindowId, out var r))
+				{
+					if (r.TryGetTarget(out var w))
+					{
+						window = w;
+					}
+					_requestedWindows.Remove(requestedWindowId);
+				}
 			}
 
 			// create a new one if there is no pending windows
@@ -87,9 +94,8 @@ internal void RemoveWindow(Window window)
 
 		public virtual void OpenWindow(Window window)
 		{
-			var id = Guid.NewGuid().ToString();
-
-			_requestedWindows[id] = window;
+			var id = Guid.NewGuid().ToString("n");
+			_requestedWindows.Add(id, new WeakReference<Window>(window));
 
 			var state = new PersistedState
 			{

src/Controls/src/Core/HandlerImpl/Window/Window.Impl.cs

@@ -500,6 +500,7 @@ void IWindow.Destroying()
 			OnDestroying();
 
 			Application?.RemoveWindow(this);
+			Handler?.DisconnectHandler();
 		}
 
 		void IWindow.Resumed()

src/Controls/tests/Core.UnitTests/WindowsTests.cs

@@ -1,5 +1,9 @@
-using System.Collections.Generic;
+using System;
+using System.Collections;
+using System.Collections.Generic;
 using System.Linq;
+using System.Reflection;
+using System.Threading.Tasks;
 using Microsoft.Maui.Graphics;
 using Xunit;
 
@@ -554,5 +558,63 @@ public void MinDimensionsArePassedToCoreCorrectly(double inW, double inH, double
 			Assert.Equal(outW, coreWindow.MinimumWidth);
 			Assert.Equal(outH, coreWindow.MinimumHeight);
 		}
+
+		[Fact]
+		public async Task WindowDoesNotLeak()
+		{
+			var application = new Application();
+			WeakReference reference;
+
+			// Scope for window
+			{
+				var window = new Window { Page = new ContentPage() };
+				reference = new WeakReference(window);
+				application.OpenWindow(window);
+				((IWindow)window).Destroying();
+			}
+
+			// GC
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			Assert.False(reference.IsAlive, "Window should not be alive!");
+		}
+
+		// NOTE: this test is here to show `ConditionalWeakTable _requestedWindows` was a bad idea
+		[Fact]
+		public async Task TwoKeysSameWindow()
+		{
+			var application = new Application();
+			var window = new Window { Page = new ContentPage() };
+
+			application.OpenWindow(window);
+
+			// Access Application._requestedWindows
+			var flags = BindingFlags.NonPublic | BindingFlags.Instance;
+			var table = typeof(Application).GetField("_requestedWindows", flags).GetValue(application) as IDictionary;
+			Assert.NotNull(table);
+
+			// A "cloned" key should still work
+			string key;
+			{
+				var originalKey = table.Keys.OfType<string>().Single();
+				key = new string(originalKey);
+				Assert.NotSame(originalKey, key);
+			}
+
+			// GC collect the original key
+			await Task.Yield();
+			GC.Collect();
+			GC.WaitForPendingFinalizers();
+
+			// Same window, doesn't create a new one
+			var actual = ((IApplication)application).CreateWindow(new ActivationState(new MockMauiContext(), new PersistedState
+			{
+				{ Application.MauiWindowIdKey, key }
+			}));
+			Assert.Same(window, actual);
+			Assert.Empty(table);
+		}
 	}
 }