From 7e67adf8b5cd12562d4636bca995ee5a6e5a5223 Mon Sep 17 00:00:00 2001 From: Shaun Lawrence <17139988+bijington@users.noreply.github.com> Date: Fri, 20 Jun 2025 21:47:31 +0100 Subject: [PATCH 1/3] Introduce unit test that reproduces the 'indefinite' ShowPopupAsync call --- .../BaseHandlerTest.cs | 2 ++ .../Extensions/PopupExtensionsTests.cs | 15 +++++++++++++++ .../Services/PopupServiceTests.cs | 11 ++++++++++- 3 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs b/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs index 5c0cd45d84..b04e214f6f 100644 --- a/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs +++ b/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs @@ -82,6 +82,8 @@ static void InitializeServicesAndSetMockApplication(out IServiceProvider service PopupService.AddPopup(mockPopup, mockPageViewModel, appBuilder.Services, ServiceLifetime.Transient); appBuilder.Services.AddTransientPopup(); + + appBuilder.Services.AddTransient(); #endregion var mauiApp = appBuilder.Build(); diff --git a/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs b/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs index cef2f4423c..244d6ffce0 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs @@ -945,6 +945,21 @@ public async Task ShowPopupAsyncWithView_Shell_ShouldValidateProperBindingContex Assert.Equal(shellParameterBackgroundColorValue, view.BackgroundColor); Assert.Equal(shellParameterViewModelTextValue, view.BindingContext.Text); } + + [Fact(Timeout = (int)TestDuration.Medium)] + public async Task ShowPopupAsync_ShouldSuccessfullyCompleteAndReturnResultUnderHeavyGarbageCollection() + { + // Arrange + var mockPopup = ServiceProvider.GetRequiredService(); + var selfClosingPopup = ServiceProvider.GetRequiredService() ?? throw new InvalidOperationException(); + + // Act + var result = await navigation.ShowPopupAsync(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() diff --git a/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs b/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs index 41f929d100..995bc178fd 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs @@ -522,7 +522,14 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult() } } -sealed class MockSelfClosingPopup : Popup, IQueryAttributable +class GarbageCollectionHeavySelfClosingPopup : MockSelfClosingPopup +{ + public GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : base(viewModel, result) + { + } +} + +class MockSelfClosingPopup : Popup, IQueryAttributable { public const int ExpectedResult = 2; @@ -548,7 +555,9 @@ async void HandleTick(object? sender, EventArgs e) timer.Tick -= HandleTick; try { + GC.Collect(); await CloseAsync(Result); + GC.Collect(); } catch (InvalidOperationException) { From 7094820efcd21712d0dd78aadf79401e8bc05645 Mon Sep 17 00:00:00 2001 From: Shaun Lawrence <17139988+bijington@users.noreply.github.com> Date: Fri, 20 Jun 2025 21:49:27 +0100 Subject: [PATCH 2/3] Make PopupClosed event a stronger reference --- .../Views/Popup/PopupPage.shared.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs b/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs index 13cacf49b0..0babb63ba0 100644 --- a/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs +++ b/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(view), popupOptions) @@ -63,11 +62,7 @@ public PopupPage(Popup popup, IPopupOptions popupOptions) On().SetModalPresentationStyle(UIModalPresentationStyle.OverFullScreen); } - public event EventHandler PopupClosed - { - add => popupClosedEventManager.AddEventHandler(value); - remove => popupClosedEventManager.RemoveEventHandler(value); - } + public event EventHandler? PopupClosed; // Prevent Content from being set by external class // Casts `PopupPage.Content` to return typeof(PopupPageLayout) @@ -101,7 +96,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() From 122fc28976de62394307f62a2f2014ecbe5e61d3 Mon Sep 17 00:00:00 2001 From: Brandon Minnick <13558917+TheCodeTraveler@users.noreply.github.com> Date: Fri, 20 Jun 2025 14:24:07 -0700 Subject: [PATCH 3/3] `dotnet format` --- .../BaseHandlerTest.cs | 4 ++-- .../Extensions/PopupExtensionsTests.cs | 6 +++--- .../Services/PopupServiceTests.cs | 7 +------ .../Views/Popup/PopupPageTests.cs | 20 +++++++++---------- .../Views/Popup/PopupTests.cs | 4 ++-- .../Defaults/PopupDefaults.shared.cs | 2 +- .../Views/Popup/Popup.shared.cs | 4 ++-- .../Views/Popup/PopupPage.shared.cs | 2 +- 8 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs b/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs index b04e214f6f..87ae78be4f 100644 --- a/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs +++ b/src/CommunityToolkit.Maui.UnitTests/BaseHandlerTest.cs @@ -81,9 +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(); - appBuilder.Services.AddTransient(); + appBuilder.Services.AddTransientPopup(); + appBuilder.Services.AddTransient(); #endregion var mauiApp = appBuilder.Build(); diff --git a/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs b/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs index 244d6ffce0..5c1099f499 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Extensions/PopupExtensionsTests.cs @@ -945,7 +945,7 @@ public async Task ShowPopupAsyncWithView_Shell_ShouldValidateProperBindingContex Assert.Equal(shellParameterBackgroundColorValue, view.BackgroundColor); Assert.Equal(shellParameterViewModelTextValue, view.BindingContext.Text); } - + [Fact(Timeout = (int)TestDuration.Medium)] public async Task ShowPopupAsync_ShouldSuccessfullyCompleteAndReturnResultUnderHeavyGarbageCollection() { @@ -1445,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() { @@ -1475,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().Single().GestureRecognizers[0]; } diff --git a/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs b/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs index 995bc178fd..fb0b29bba6 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs +++ b/src/CommunityToolkit.Maui.UnitTests/Services/PopupServiceTests.cs @@ -522,12 +522,7 @@ public async Task ClosePopupAsyncT_ShouldClosePopupUsingPageAndReturnResult() } } -class GarbageCollectionHeavySelfClosingPopup : MockSelfClosingPopup -{ - public GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : base(viewModel, result) - { - } -} +class GarbageCollectionHeavySelfClosingPopup(MockPageViewModel viewModel, object? result = null) : MockSelfClosingPopup(viewModel, result); class MockSelfClosingPopup : Popup, IQueryAttributable { diff --git a/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs b/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs index f75256bbe1..14c1ebdd0e 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupPageTests.cs +++ b/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().Single().GestureRecognizers[0]; // Helper class for testing protected methods diff --git a/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs b/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs index 7d989d9898..09f6154cc6 100644 --- a/src/CommunityToolkit.Maui.UnitTests/Views/Popup/PopupTests.cs +++ b/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() { diff --git a/src/CommunityToolkit.Maui/Primitives/Defaults/PopupDefaults.shared.cs b/src/CommunityToolkit.Maui/Primitives/Defaults/PopupDefaults.shared.cs index 31ebddac61..79a5e6dde3 100644 --- a/src/CommunityToolkit.Maui/Primitives/Defaults/PopupDefaults.shared.cs +++ b/src/CommunityToolkit.Maui/Primitives/Defaults/PopupDefaults.shared.cs @@ -31,7 +31,7 @@ static class PopupDefaults /// Default value for BackgroundColor /// public static Color BackgroundColor { get; } = Colors.White; - + /// /// Default value for /// diff --git a/src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs b/src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs index 5d35a7bf10..c971c92294 100644 --- a/src/CommunityToolkit.Maui/Views/Popup/Popup.shared.cs +++ b/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 when displayed on screen /// public static new readonly BindableProperty VerticalOptionsProperty = View.VerticalOptionsProperty; - + /// /// Backing BindableProperty for the property. /// @@ -88,7 +88,7 @@ public Popup() get => base.VerticalOptions; set => base.VerticalOptions = value; } - + /// /> /// /// When true and the user taps outside the popup, it will dismiss. diff --git a/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs b/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs index f4c1e24edd..ebf1a76e75 100644 --- a/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs +++ b/src/CommunityToolkit.Maui/Views/Popup/PopupPage.shared.cs @@ -159,7 +159,7 @@ void HandlePopupOptionsPropertyChanged(object? sender, PropertyChangedEventArgs tapOutsideOfPopupCommand.ChangeCanExecute(); } } - + void HandlePopupPropertyChanged(object? sender, PropertyChangedEventArgs e) { if (e.PropertyName == Popup.CanBeDismissedByTappingOutsideOfPopupProperty.PropertyName)