ShowPopupAsync will now correctly return once the popup is closed under heavy garbage collection (#2756)

* Introduce unit test that reproduces the 'indefinite' ShowPopupAsync call

* Make PopupClosed event a stronger reference

* `dotnet format`

---------

Co-authored-by: Shaun Lawrence <[email protected]>
Co-authored-by: Brandon Minnick <[email protected]>

Author: bijington

src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs

@@ -81,7 +81,9 @@ static void InitializeServicesAndSetMockApplication(out IServiceProvider service
 		var mockPopup = new MockSelfClosingPopup(mockPageViewModel, new());
 
 		PopupService.AddPopup(mockPopup, mockPageViewModel, appBuilder.Services, ServiceLifetime.Transient);
+
 		appBuilder.Services.AddTransientPopup<MockPopup>();
+		appBuilder.Services.AddTransient<GarbageCollectionHeavySelfClosingPopup>();
 		#endregion
 
 		var mauiApp = appBuilder.Build();

src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs

@@ -946,6 +946,21 @@ public async Task ShowPopupAsyncWithView_Shell_ShouldValidateProperBindingContex
 		Assert.Equal(shellParameterViewModelTextValue, view.BindingContext.Text);
 	}
 
+	[Fact(Timeout = (int)TestDuration.Medium)]
+	public async Task ShowPopupAsync_ShouldSuccessfullyCompleteAndReturnResultUnderHeavyGarbageCollection()
+	{
+		// Arrange
+		var mockPopup = ServiceProvider.GetRequiredService<GarbageCollectionHeavySelfClosingPopup>();
+		var selfClosingPopup = ServiceProvider.GetRequiredService<GarbageCollectionHeavySelfClosingPopup>() ?? throw new InvalidOperationException();
+
+		// Act
+		var result = await navigation.ShowPopupAsync<object?>(selfClosingPopup, PopupOptions.Empty, TestContext.Current.CancellationToken);
+
+		// Assert
+		Assert.Same(mockPopup.Result, result.Result);
+		Assert.False(result.WasDismissedByTappingOutsideOfPopup);
+	}
+
 	[Fact(Timeout = (int)TestDuration.Medium)]
 	public async Task ShowPopupAsync_ShouldReturnResultOnceClosed()
 	{
@@ -1430,7 +1445,7 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult()
 		Assert.Equal(expectedResult, popupResult.Result);
 		Assert.False(popupResult.WasDismissedByTappingOutsideOfPopup);
 	}
-	
+
 	[Fact(Timeout = (int)TestDuration.Short)]
 	public async Task ShowPopupAsync_TaskShouldCompleteWhenCloseAsyncIsCalled()
 	{
@@ -1460,7 +1475,7 @@ public async Task ShowPopupAsync_TaskShouldCompleteWhenCloseAsyncIsCalled()
 		Assert.False(popupResult.WasDismissedByTappingOutsideOfPopup);
 	}
 
-	static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) => 
+	static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) =>
 		(TapGestureRecognizer)popupPage.Content.Children.OfType<BoxView>().Single().GestureRecognizers[0];
 }
 

src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs

@@ -522,7 +522,9 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult()
 	}
 }
 
-sealed class MockSelfClosingPopup : Popup<object?>, IQueryAttributable
+class GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : MockSelfClosingPopup(viewModel, result);
+
+class MockSelfClosingPopup : Popup<object?>, IQueryAttributable
 {
 	public const int ExpectedResult = 2;
 
@@ -548,7 +550,9 @@ async void HandleTick(object? sender, EventArgs e)
 			timer.Tick -= HandleTick;
 			try
 			{
+				GC.Collect();
 				await CloseAsync(Result);
+				GC.Collect();
 			}
 			catch (InvalidOperationException)
 			{

src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs

@@ -230,32 +230,32 @@ public void TapGestureRecognizer_VerifyCanBeDismissedByTappingOutsideOfPopup_Sho
 
 		// Assert
 		Assert.True(tapGestureRecognizer.Command?.CanExecute(null));
-		
+
 		// Act
 		view.CanBeDismissedByTappingOutsideOfPopup = false;
 		popupOptions.CanBeDismissedByTappingOutsideOfPopup = false;
-		
+
 		// Assert
 		Assert.False(tapGestureRecognizer.Command?.CanExecute(null));
-		
+
 		// Act
 		view.CanBeDismissedByTappingOutsideOfPopup = true;
 		popupOptions.CanBeDismissedByTappingOutsideOfPopup = false;
-		
+
 		// Assert
 		Assert.False(tapGestureRecognizer.Command?.CanExecute(null));
-		
+
 		// Act
 		view.CanBeDismissedByTappingOutsideOfPopup = false;
 		popupOptions.CanBeDismissedByTappingOutsideOfPopup = true;
-		
+
 		// Assert
 		Assert.False(tapGestureRecognizer.Command?.CanExecute(null));
-		
+
 		// Act
 		view.CanBeDismissedByTappingOutsideOfPopup = true;
 		popupOptions.CanBeDismissedByTappingOutsideOfPopup = true;
-		
+
 		// Assert
 		Assert.True(tapGestureRecognizer.Command?.CanExecute(null));
 
@@ -529,8 +529,8 @@ public void PopupPage_ShouldRespectLayoutOptions()
 		Assert.Equal(LayoutOptions.Start, border.VerticalOptions);
 		Assert.Equal(LayoutOptions.End, border.HorizontalOptions);
 	}
-	
-	static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) => 
+
+	static TapGestureRecognizer GetTapOutsideGestureRecognizer(PopupPage popupPage) =>
 		(TapGestureRecognizer)popupPage.Content.Children.OfType<BoxView>().Single().GestureRecognizers[0];
 
 	// Helper class for testing protected methods

src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs

@@ -14,7 +14,7 @@ public void PopupBackgroundColor_DefaultValue_ShouldBeWhite()
 	{
 		Assert.Equal(PopupDefaults.BackgroundColor, Colors.White);
 	}
-	
+
 	[Fact]
 	public void CanBeDismissedByTappingOutsideOfPopup_DefaultValue_ShouldBeTrue()
 	{
@@ -158,7 +158,7 @@ public async Task PopupT_Close_ShouldNotThrowExceptionWhenCloseIsOverridden()
 		await popup.CloseAsync(TestContext.Current.CancellationToken);
 		await popup.CloseAsync("Hello", TestContext.Current.CancellationToken);
 	}
-	
+
 	[Fact(Timeout = (int)TestDuration.Short)]
 	public async Task ShowPopupAsync_TaskShouldCompleteWhenPopupCloseAsyncIsCalled()
 	{

src/CommunityToolkit.Maui/Primitives/Defaults/PopupDefaults.shared.cs

@@ -31,7 +31,7 @@ static class PopupDefaults
 	/// Default value for <see cref="VisualElement.BackgroundColor"/> BackgroundColor 
 	/// </summary>
 	public static Color BackgroundColor { get; } = Colors.White;
-	
+
 	/// <summary>
 	/// Default value for <see cref="Popup.CanBeDismissedByTappingOutsideOfPopup"/>
 	/// </summary>

src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs

@@ -26,7 +26,7 @@ public partial class Popup : ContentView
 	/// Bindable property to set the vertical position of the <see cref="Popup"/> when displayed on screen
 	/// </summary>
 	public static new readonly BindableProperty VerticalOptionsProperty = View.VerticalOptionsProperty;
-	
+
 	/// <summary>
 	///  Backing BindableProperty for the <see cref="CanBeDismissedByTappingOutsideOfPopup"/> property.
 	/// </summary>
@@ -88,7 +88,7 @@ public Popup()
 		get => base.VerticalOptions;
 		set => base.VerticalOptions = value;
 	}
-	
+
 	/// <inheritdoc cref="IPopupOptions.CanBeDismissedByTappingOutsideOfPopup"/> />
 	/// <remarks>
 	/// When true and the user taps outside the popup, it will dismiss.

src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs

@@ -26,7 +26,6 @@ partial class PopupPage : ContentPage, IQueryAttributable
 	readonly Popup popup;
 	readonly IPopupOptions popupOptions;
 	readonly Command tapOutsideOfPopupCommand;
-	readonly WeakEventManager popupClosedEventManager = new();
 
 	public PopupPage(View view, IPopupOptions popupOptions)
 		: this(view as Popup ?? CreatePopupFromView<Popup>(view), popupOptions)
@@ -64,11 +63,7 @@ public PopupPage(Popup popup, IPopupOptions popupOptions)
 		On<iOS>().SetModalPresentationStyle(UIModalPresentationStyle.OverFullScreen);
 	}
 
-	public event EventHandler<IPopupResult> PopupClosed
-	{
-		add => popupClosedEventManager.AddEventHandler(value);
-		remove => popupClosedEventManager.RemoveEventHandler(value);
-	}
+	public event EventHandler<IPopupResult>? PopupClosed;
 
 	// Prevent Content from being set by external class
 	// Casts `PopupPage.Content` to return typeof(PopupPageLayout)
@@ -102,7 +97,7 @@ public async Task CloseAsync(PopupResult result, CancellationToken token = defau
 		token.ThrowIfCancellationRequested();
 		await Navigation.PopModalAsync(false).WaitAsync(token);
 
-		popupClosedEventManager.HandleEvent(this, result, nameof(PopupClosed));
+		PopupClosed?.Invoke(this, result);
 	}
 
 	protected override bool OnBackButtonPressed()
@@ -164,7 +159,7 @@ void HandlePopupOptionsPropertyChanged(object? sender, PropertyChangedEventArgs
 			tapOutsideOfPopupCommand.ChangeCanExecute();
 		}
 	}
-	
+
 	void HandlePopupPropertyChanged(object? sender, PropertyChangedEventArgs e)
 	{
 		if (e.PropertyName == Popup.CanBeDismissedByTappingOutsideOfPopupProperty.PropertyName)