diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 46f826980ed1..7af7ba7f5938 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -98,6 +98,35 @@ Always format code before committing: dotnet format Microsoft.Maui.sln --no-restore --exclude Templates/src --exclude-diagnostics CA1822 ``` +### Code Style Anti-Patterns + +**🚨 NEVER use these patterns:** + +| Anti-Pattern | Why It's Bad | Correct Approach | +|--------------|--------------|------------------| +| `obj.GetType().Name` | Fragile, breaks with refactoring, violates OOP | Use marker interfaces | +| `obj.GetType() == typeof(X)` | Same issues, also breaks inheritance | Use `is` operator with interfaces | +| `typeName.EndsWith("Page")` | String-based type checking is error-prone | Create internal marker interface | +| `obj.GetType().FullName.Contains("...")` | Extremely fragile, namespace changes break it | Use interfaces or base classes | + +**Correct pattern for type-specific behavior:** + +```csharp +// ❌ BAD - Never do this +if (obj.GetType().Name.EndsWith("ContentPage")) + +// ✅ GOOD - Use marker interface +internal interface IContentPageController { } +public partial class ContentPage : IContentPageController { } + +if (obj is IContentPageController) +``` + +When you need platform-specific type checking: +1. Create an internal marker interface in `src/Core/src/Platform/{Platform}/` +2. Have the specific type implement the interface (use platform-specific partial if needed) +3. Check for the interface using `is` operator + ## Contribution Guidelines ### Handling Existing PRs for Assigned Issues diff --git a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRenderer.cs b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRenderer.cs index b08347bc387e..51718f85e99e 100644 --- a/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRenderer.cs +++ b/src/Controls/src/Core/Compatibility/Handlers/Shell/Android/ShellFlyoutRenderer.cs @@ -17,7 +17,7 @@ namespace Microsoft.Maui.Controls.Platform.Compatibility { - public class ShellFlyoutRenderer : DrawerLayout, IShellFlyoutRenderer, IFlyoutBehaviorObserver, IAppearanceObserver + public class ShellFlyoutRenderer : DrawerLayout, IShellFlyoutRenderer, IFlyoutBehaviorObserver, IAppearanceObserver, Microsoft.Maui.Platform.IRequestInsetsOnTransition { #region IAppearanceObserver diff --git a/src/Controls/src/Core/ContentPage/ContentPage.Android.cs b/src/Controls/src/Core/ContentPage/ContentPage.Android.cs new file mode 100644 index 000000000000..fe32ba65e4d9 --- /dev/null +++ b/src/Controls/src/Core/ContentPage/ContentPage.Android.cs @@ -0,0 +1,9 @@ +#nullable enable +namespace Microsoft.Maui.Controls +{ + // Android-specific partial for ContentPage + // Implements IContentPageController marker interface for safe area transition handling + public partial class ContentPage : Microsoft.Maui.Platform.IContentPageController + { + } +} diff --git a/src/Controls/tests/TestCases.HostApp/GCMonitoringService.cs b/src/Controls/tests/TestCases.HostApp/GCMonitoringService.cs new file mode 100644 index 000000000000..82c7f8960ca7 --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/GCMonitoringService.cs @@ -0,0 +1,111 @@ +#nullable enable +using System.Diagnostics; + +namespace Maui.Controls.Sample; + +/// +/// A lightweight service that logs GC activity to Console.WriteLine for test infrastructure. +/// +/// This service runs automatically on Android and logs GC counts with the [MAUI_GC] prefix. +/// The UITest infrastructure can parse these logs from logcat to track GC activity per test. +/// +/// Since the app resets between test runs, GC counts are tracked from app startup, +/// giving us a clean per-test baseline. +/// +public static class GCMonitoringService +{ + private static int _initialGen0Count; + private static int _initialGen1Count; + private static int _initialGen2Count; + private static DateTime _startTime; + private static bool _isInitialized; + private static string? _currentTestPage; + + /// + /// Initialize the GC monitoring service. Call once at app startup. + /// + public static void Initialize() + { + if (_isInitialized) + return; + + // Force a GC to establish clean baseline + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + _initialGen0Count = GC.CollectionCount(0); + _initialGen1Count = GC.CollectionCount(1); + _initialGen2Count = GC.CollectionCount(2); + _startTime = DateTime.UtcNow; + _isInitialized = true; + + Log($"Initialized at {_startTime:HH:mm:ss.fff}, baseline Gen0={_initialGen0Count}, Gen1={_initialGen1Count}, Gen2={_initialGen2Count}"); + } + + /// + /// Log when a test page is navigated to. Helps correlate GC activity with specific tests. + /// + public static void OnTestPageNavigated(string pageName) + { + if (!_isInitialized) + Initialize(); + + _currentTestPage = pageName; + var (gen0, gen1, gen2) = GetCurrentCounts(); + Log($"TestPage={pageName}, Gen0={gen0}, Gen1={gen1}, Gen2={gen2}"); + } + + /// + /// Log when a test page is left. Shows GC activity during the test. + /// + public static void OnTestPageLeft(string pageName) + { + if (!_isInitialized) + return; + + var (gen0, gen1, gen2) = GetCurrentCounts(); + var elapsed = (DateTime.UtcNow - _startTime).TotalSeconds; + Log($"TestPageLeft={pageName}, Gen0={gen0}, Gen1={gen1}, Gen2={gen2}, ElapsedSec={elapsed:F1}"); + _currentTestPage = null; + } + + /// + /// Log current GC counts. Can be called at any time for debugging. + /// + public static void LogCurrentState(string? context = null) + { + if (!_isInitialized) + Initialize(); + + var (gen0, gen1, gen2) = GetCurrentCounts(); + var elapsed = (DateTime.UtcNow - _startTime).TotalSeconds; + var ctx = context ?? _currentTestPage ?? "Unknown"; + Log($"State={ctx}, Gen0={gen0}, Gen1={gen1}, Gen2={gen2}, ElapsedSec={elapsed:F1}"); + } + + /// + /// Get GC counts since initialization. + /// + public static (int Gen0, int Gen1, int Gen2) GetCurrentCounts() + { + if (!_isInitialized) + Initialize(); + + return ( + GC.CollectionCount(0) - _initialGen0Count, + GC.CollectionCount(1) - _initialGen1Count, + GC.CollectionCount(2) - _initialGen2Count + ); + } + + /// + /// Log with the [MAUI_GC] prefix that the test infrastructure will parse. + /// + private static void Log(string message) + { + // Use Console.WriteLine which goes to logcat on Android + // The [MAUI_GC] prefix allows the test infrastructure to easily grep these entries + Console.WriteLine($"[MAUI_GC] {message}"); + } +} diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33731.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33731.cs new file mode 100644 index 000000000000..2589dccea60d --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33731.cs @@ -0,0 +1,187 @@ +#nullable enable +namespace Maui.Controls.Sample.Issues; + +/// +/// Test for Issue #33731 - Continuous GC logs on TabbedPage in MAUI 10.0.30. +/// +/// The bug causes an infinite loop of RequestApplyInsets calls when Tab 2's content +/// is positioned off-screen (at x=screenWidth). This creates continuous lambda +/// allocations (~60/sec), triggering GC every ~5-6 seconds. +/// +/// This test monitors GC.CollectionCount(0) to detect excessive GC activity. +/// WITH BUG: 5+ GC events in 30 seconds +/// WITH FIX: 0-1 GC events in 30 seconds +/// +[Issue(IssueTracker.Github, 33731, "Continuous GC logs on TabbedPage in MAUI 10.0.30", PlatformAffected.Android)] +public class Issue33731 : TabbedPage +{ + private readonly Label _gcCountLabel; + private readonly Label _statusLabel; + private int _lastGcCount; + private int _gcEventsDetected; + private DateTime _startTime; + private IDispatcherTimer? _timer; + + public Issue33731() + { + AutomationId = "TabbedPageRoot"; + + // Force initial GC to establish baseline + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + _lastGcCount = GC.CollectionCount(0); + _gcEventsDetected = 0; + _startTime = DateTime.UtcNow; + + // Create the monitoring UI on Tab 1 + _gcCountLabel = new Label + { + AutomationId = "GCCountLabel", + Text = "GCCount: 0", + FontSize = 24, + FontAttributes = FontAttributes.Bold, + BackgroundColor = Colors.Yellow, + HorizontalOptions = LayoutOptions.Center, + Padding = new Thickness(10) + }; + + _statusLabel = new Label + { + AutomationId = "StatusLabel", + Text = "Monitoring...", + FontSize = 16, + HorizontalOptions = LayoutOptions.Center + }; + + var tab1 = new ContentPage + { + Title = "GC Monitor", + Content = new VerticalStackLayout + { + Padding = 20, + Spacing = 10, + Children = + { + new Label + { + Text = "Issue #33731 - TabbedPage GC Monitor", + AutomationId = "Tab1Label", + FontSize = 18, + FontAttributes = FontAttributes.Bold, + HorizontalOptions = LayoutOptions.Center + }, + new BoxView { HeightRequest = 2, Color = Colors.Gray }, + _gcCountLabel, + _statusLabel, + new Label + { + Text = "WITH BUG: GCCount increases rapidly (5+ in 30s)\n" + + "WITH FIX: GCCount stays at 0-1", + FontSize = 12, + HorizontalOptions = LayoutOptions.Center, + Margin = new Thickness(0, 20, 0, 0) + } + } + } + }; + + // Create second tab with Grid (matching the exact repro from the issue) + var tab2 = new ContentPage + { + Title = "Tab 2", + Content = new Grid + { + Children = + { + new Label + { + Text = "Tab 2 - Inactive tab", + AutomationId = "Tab2Label", + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center, + FontSize = 24 + } + } + } + }; + + // Create third tab (more tabs = higher likelihood of bug triggering) + var tab3 = new ContentPage + { + Title = "Tab 3", + Content = new Grid + { + Children = + { + new Label + { + Text = "Tab 3 - Another inactive tab", + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center, + FontSize = 24 + } + } + } + }; + + Children.Add(tab1); + Children.Add(tab2); + Children.Add(tab3); + + // Start GC monitoring timer + _timer = Application.Current?.Dispatcher.CreateTimer(); + if (_timer != null) + { + _timer.Interval = TimeSpan.FromMilliseconds(500); + _timer.Tick += OnTimerTick; + _timer.Start(); + } + } + + private void OnTimerTick(object? sender, EventArgs e) + { + int currentGcCount = GC.CollectionCount(0); + + if (currentGcCount > _lastGcCount) + { + int newGCs = currentGcCount - _lastGcCount; + _gcEventsDetected += newGCs; + _lastGcCount = currentGcCount; + } + + // Update UI - format must match what the test expects: "GCCount: X" + _gcCountLabel.Text = $"GCCount: {_gcEventsDetected}"; + + // Calculate elapsed time + double elapsedSeconds = (DateTime.UtcNow - _startTime).TotalSeconds; + + // Update status based on results + if (elapsedSeconds >= 30) + { + if (_gcEventsDetected >= 5) + { + _statusLabel.Text = $"BUG DETECTED: {_gcEventsDetected} GCs in {elapsedSeconds:F0}s"; + } + else if (_gcEventsDetected <= 1) + { + _statusLabel.Text = $"PASS: Only {_gcEventsDetected} GCs in {elapsedSeconds:F0}s"; + } + else + { + _statusLabel.Text = $"BORDERLINE: {_gcEventsDetected} GCs in {elapsedSeconds:F0}s"; + } + } + else + { + _statusLabel.Text = $"Monitoring... {elapsedSeconds:F0}s elapsed"; + } + } + + protected override void OnDisappearing() + { + base.OnDisappearing(); + _timer?.Stop(); + } +} diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue33768.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue33768.cs new file mode 100644 index 000000000000..e76ebb6693eb --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue33768.cs @@ -0,0 +1,253 @@ +#nullable enable +namespace Maui.Controls.Sample.Issues; + +/// +/// Test for Issue #33768 - Performance degradation caused by infinite RequestApplyInsets loop. +/// +/// This test reproduces the bug by using a CollectionView with a NEGATIVE MARGIN (-50). +/// The negative margin causes the native view bounds to extend beyond the screen edges, +/// which triggers the viewExtendsBeyondScreen check in SafeAreaExtensions.cs. +/// +/// WITHOUT FIX: The check triggers view.Post(() => RequestApplyInsets(view)), which +/// causes an infinite loop because the bounds never change, leading to ~60 allocations/sec +/// and triggering GC every 5-6 seconds. +/// +/// WITH FIX: The IRequestInsetsOnTransition guard ensures only Shell fragments during +/// transitions get the re-apply behavior, preventing the infinite loop. +/// +/// WITH BUG: 5+ GC events in 30 seconds +/// WITH FIX: 0-1 GC events in 30 seconds +/// +[Issue(IssueTracker.Github, 33768, "Performance degradation on Android caused by Infinite Layout Loop (RequestApplyInsets)", PlatformAffected.Android)] +public class Issue33768 : ContentPage +{ + private readonly Label _gcCountLabel; + private readonly Label _statusLabel; + private readonly Label _verdictLabel; + private int _initialGcCount; + private int _gcEventsDetected; + private DateTime _startTime; + private IDispatcherTimer? _timer; + + public Issue33768() + { + AutomationId = "Issue33768Root"; + + // Force initial GC to establish baseline + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + _initialGcCount = GC.CollectionCount(0); + _gcEventsDetected = 0; + _startTime = DateTime.UtcNow; + + _gcCountLabel = new Label + { + AutomationId = "GCCountLabel", + Text = "GCCount: 0", + FontSize = 24, + FontAttributes = FontAttributes.Bold, + BackgroundColor = Colors.Yellow, + HorizontalOptions = LayoutOptions.Center, + Padding = new Thickness(10) + }; + + _statusLabel = new Label + { + AutomationId = "StatusLabel", + Text = "Monitoring...", + FontSize = 16, + HorizontalOptions = LayoutOptions.Center + }; + + _verdictLabel = new Label + { + AutomationId = "VerdictLabel", + Text = "Testing...", + FontSize = 20, + FontAttributes = FontAttributes.Bold, + HorizontalOptions = LayoutOptions.Center + }; + + // Create a CollectionView with NEGATIVE MARGIN + // This is the key reproduction scenario from Issue #33768: + // "Add a heavily populated CollectionView with a negative margin to force + // some of the scrolling off-screen" + // The negative margin causes native view bounds to extend beyond screen edges + var collectionView = new CollectionView + { + AutomationId = "TestCollectionView", + Margin = new Thickness(-50), // NEGATIVE MARGIN - triggers viewExtendsBeyondScreen + BackgroundColor = Colors.LightBlue, + ItemTemplate = new DataTemplate(() => + { + var grid = new Grid { Padding = 10, HeightRequest = 80 }; + var border = new Border + { + Stroke = Colors.DarkBlue, + StrokeThickness = 2, + BackgroundColor = Colors.White, + Padding = 10 + }; + var stack = new VerticalStackLayout(); + var titleLabel = new Label { FontAttributes = FontAttributes.Bold, FontSize = 16 }; + titleLabel.SetBinding(Label.TextProperty, "Title"); + var descLabel = new Label { FontSize = 12, TextColor = Colors.Gray }; + descLabel.SetBinding(Label.TextProperty, "Description"); + stack.Children.Add(titleLabel); + stack.Children.Add(descLabel); + border.Content = stack; + grid.Children.Add(border); + return grid; + }) + }; + + // Populate with 100 items + var items = new List(); + for (int i = 1; i <= 100; i++) + { + items.Add(new CollectionItem + { + Title = $"Item {i}", + Description = "CollectionView with Margin=-50 extends native bounds beyond screen" + }); + } + collectionView.ItemsSource = items; + + Content = new Grid + { + RowDefinitions = + { + new RowDefinition { Height = GridLength.Auto }, + new RowDefinition { Height = GridLength.Star }, + new RowDefinition { Height = GridLength.Auto } + }, + Children = + { + // Header with monitoring UI + CreateHeader(), + // CollectionView with negative margin in the middle + CreateMiddle(collectionView), + // Footer with verdict + CreateFooter() + } + }; + + // Start GC monitoring timer + _timer = Application.Current?.Dispatcher.CreateTimer(); + if (_timer != null) + { + _timer.Interval = TimeSpan.FromMilliseconds(500); + _timer.Tick += OnTimerTick; + _timer.Start(); + } + } + + private View CreateHeader() + { + var header = new VerticalStackLayout + { + Padding = 20, + Spacing = 10, + BackgroundColor = Colors.LightBlue, + Children = + { + new Label + { + Text = "Issue #33768 - Negative Margin GC Test", + AutomationId = "TitleLabel", + FontSize = 18, + FontAttributes = FontAttributes.Bold, + HorizontalOptions = LayoutOptions.Center + }, + new BoxView { HeightRequest = 2, Color = Colors.Gray }, + _gcCountLabel, + _statusLabel, + new Label + { + Text = "CollectionView has Margin=-50.\n" + + "WITH BUG: GCCount increases rapidly (5+ in 30s)\n" + + "WITH FIX: GCCount stays at 0-1", + FontSize = 12, + HorizontalOptions = LayoutOptions.Center + } + } + }; + Grid.SetRow(header, 0); + return header; + } + + private View CreateMiddle(CollectionView collectionView) + { + Grid.SetRow(collectionView, 1); + return collectionView; + } + + private View CreateFooter() + { + var footer = new VerticalStackLayout + { + Padding = 10, + BackgroundColor = Colors.LightGreen, + Children = + { + new Label { Text = "Verdict:", FontAttributes = FontAttributes.Bold }, + _verdictLabel + } + }; + Grid.SetRow(footer, 2); + return footer; + } + + private void OnTimerTick(object? sender, EventArgs e) + { + int currentGcCount = GC.CollectionCount(0); + int newGCs = currentGcCount - _initialGcCount; + + if (newGCs > _gcEventsDetected) + { + _gcEventsDetected = newGCs; + } + + // Update UI - format must match what the test expects: "GCCount: X" + _gcCountLabel.Text = $"GCCount: {_gcEventsDetected}"; + + // Calculate elapsed time + double elapsedSeconds = (DateTime.UtcNow - _startTime).TotalSeconds; + + // Update status based on results + if (elapsedSeconds >= 10) + { + if (_gcEventsDetected >= 3) + { + _verdictLabel.Text = $"FAIL: {_gcEventsDetected} GCs in {elapsedSeconds:F0}s"; + _verdictLabel.TextColor = Colors.Red; + _statusLabel.Text = "BUG DETECTED: Infinite loop occurring"; + } + else + { + _verdictLabel.Text = $"PASS: {_gcEventsDetected} GCs in {elapsedSeconds:F0}s"; + _verdictLabel.TextColor = Colors.Green; + _statusLabel.Text = "No infinite loop detected"; + } + } + else + { + _statusLabel.Text = $"Monitoring... {elapsedSeconds:F0}s elapsed"; + } + } + + protected override void OnDisappearing() + { + base.OnDisappearing(); + _timer?.Stop(); + } + + // Simple data class for CollectionView items + private class CollectionItem + { + public string Title { get; set; } = string.Empty; + public string Description { get; set; } = string.Empty; + } +} diff --git a/src/Controls/tests/TestCases.HostApp/MauiProgram.cs b/src/Controls/tests/TestCases.HostApp/MauiProgram.cs index b4583ac95321..e0347575133d 100644 --- a/src/Controls/tests/TestCases.HostApp/MauiProgram.cs +++ b/src/Controls/tests/TestCases.HostApp/MauiProgram.cs @@ -19,6 +19,11 @@ static string GetFileLogPath() public static MauiApp CreateMauiApp() { + // Initialize GC monitoring at app startup (logs to Console.WriteLine/logcat) +#if ANDROID + GCMonitoringService.Initialize(); +#endif + var appBuilder = MauiApp.CreateBuilder(); #if IOS || ANDROID || MACCATALYST diff --git a/src/Controls/tests/TestCases.HostApp/TestCases.cs b/src/Controls/tests/TestCases.HostApp/TestCases.cs index 6e4aec66f541..d3d1d586351e 100644 --- a/src/Controls/tests/TestCases.HostApp/TestCases.cs +++ b/src/Controls/tests/TestCases.HostApp/TestCases.cs @@ -238,6 +238,11 @@ public bool TryToNavigateTo(string name) if (issue == null) return false; + // Log GC state when navigating to a test page +#if ANDROID + GCMonitoringService.OnTestPageNavigated(name); +#endif + issue.Action(); return true; } @@ -256,7 +261,17 @@ public bool TryToNavigateTo(string name) public Page TryToGetTestPage(string name) { var issue = _issues.SingleOrDefault(x => string.Equals(x.Description, name, StringComparison.OrdinalIgnoreCase)); - return issue?.PageFactory?.Invoke(); + var page = issue?.PageFactory?.Invoke(); + + // Log GC state when navigating to a test page +#if ANDROID + if (page != null) + { + GCMonitoringService.OnTestPageNavigated(name); + } +#endif + + return page; } public void FilterIssues(string filter = null) diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33731.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33731.cs new file mode 100644 index 000000000000..92403c2e9f89 --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33731.cs @@ -0,0 +1,71 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +public class Issue33731 : _IssuesUITest +{ + public override string Issue => "Continuous GC logs on TabbedPage in MAUI 10.0.30"; + + public Issue33731(TestDevice device) : base(device) { } + + [Test] + [Category(UITestCategories.TabbedPage)] + public void TabbedPageShouldNotCauseExcessiveGC() + { + // Wait for the TabbedPage to load + App.WaitForElement("GCCountLabel"); + App.WaitForElement("Tab1Label"); + + // Wait a moment for initial layout and GC baseline to stabilize + Task.Delay(1000).Wait(); + + // Get initial GC count + var initialStatus = App.FindElement("GCCountLabel").GetText() ?? ""; + int initialCount = GetCountFromStatus(initialStatus); + + System.Diagnostics.Debug.WriteLine($"Initial GCCount: {initialCount}"); + + // Wait 10 seconds while idle + // If the bug is present, GC happens every ~5-6 seconds due to lambda allocations + // from the infinite RequestApplyInsets loop + Task.Delay(10000).Wait(); + + // Get the count after waiting + var afterWaitStatus = App.FindElement("GCCountLabel").GetText() ?? ""; + int afterWaitCount = GetCountFromStatus(afterWaitStatus); + + System.Diagnostics.Debug.WriteLine($"After 10s GCCount: {afterWaitCount}"); + + // Calculate how many GC events happened during idle time + int gcsDuringIdle = afterWaitCount - initialCount; + + // If the bug is present: + // - Lambda allocations from view.Post(() => RequestApplyInsets) ~60 times/sec + // - This causes GC every ~5-6 seconds + // - In 10 seconds, we'd see 1-2 GC events + // + // If fixed: + // - No lambda allocations in the loop + // - 0 GC events during idle + // + // We use a threshold of 2 to detect the bug while allowing for minimal GC + // from normal app activity + Assert.That(gcsDuringIdle, Is.LessThan(2), + $"TabbedPage should not cause excessive GC. " + + $"Initial: {initialCount}, After 10s: {afterWaitCount}, GCs during idle: {gcsDuringIdle}. " + + $"Excessive GC indicates infinite RequestApplyInsets loop from PR #33285."); + } + + private static int GetCountFromStatus(string status) + { + // Format: "GCCount: X" + var parts = status.Split(':'); + if (parts.Length >= 2 && int.TryParse(parts[1].Trim(), out int count)) + { + return count; + } + return 0; + } +} diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33768.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33768.cs new file mode 100644 index 000000000000..e263835171d6 --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue33768.cs @@ -0,0 +1,71 @@ +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +public class Issue33768 : _IssuesUITest +{ + public override string Issue => "Performance degradation on Android caused by Infinite Layout Loop (RequestApplyInsets)"; + + public Issue33768(TestDevice device) : base(device) { } + + [Test] + [Category(UITestCategories.Layout)] + public void NegativeMarginCollectionViewShouldNotCauseExcessiveGC() + { + // Wait for the page to load + App.WaitForElement("GCCountLabel"); + App.WaitForElement("TitleLabel"); + + // Wait a moment for initial layout and GC baseline to stabilize + Task.Delay(1000).Wait(); + + // Get initial GC count + var initialStatus = App.FindElement("GCCountLabel").GetText() ?? ""; + int initialCount = GetCountFromStatus(initialStatus); + + System.Diagnostics.Debug.WriteLine($"Initial GCCount: {initialCount}"); + + // Wait 10 seconds while idle + // If bug is present, the negative margin causes viewExtendsBeyondScreen to be true, + // which triggers continuous RequestApplyInsets calls, causing lambda allocations and GC + Task.Delay(10000).Wait(); + + // Get the count after waiting + var afterWaitStatus = App.FindElement("GCCountLabel").GetText() ?? ""; + int afterWaitCount = GetCountFromStatus(afterWaitStatus); + + System.Diagnostics.Debug.WriteLine($"After 10s GCCount: {afterWaitCount}"); + + // Calculate how many GC events happened during idle time + int gcsDuringIdle = afterWaitCount - initialCount; + + // This test uses a CollectionView with Margin=-50 (negative margin). + // The negative margin causes native view bounds to extend beyond screen edges. + // + // WITHOUT FIX: viewExtendsBeyondScreen check in SafeAreaExtensions.cs would + // trigger infinite RequestApplyInsets loop, causing ~60 allocations/sec + // and 5+ GCs in 30 seconds. + // + // WITH FIX: The IRequestInsetsOnTransition guard ensures only Shell fragments + // during transitions get the re-apply behavior, so no infinite loop occurs. + // + // Threshold of 2 allows for minimal GC from normal app activity + Assert.That(gcsDuringIdle, Is.LessThan(2), + $"CollectionView with negative margin should not cause excessive GC. " + + $"Initial: {initialCount}, After 10s: {afterWaitCount}, GCs during idle: {gcsDuringIdle}. " + + $"Excessive GC indicates regression in RequestApplyInsets handling (Issue #33768)."); + } + + private static int GetCountFromStatus(string status) + { + // Format: "GCCount: X" + var parts = status.Split(':'); + if (parts.Length >= 2 && int.TryParse(parts[1].Trim(), out int count)) + { + return count; + } + return 0; + } +} diff --git a/src/Core/src/Platform/Android/IHandleWindowInsets.cs b/src/Core/src/Platform/Android/IHandleWindowInsets.cs index 9639bc1b6a82..763155826ee7 100644 --- a/src/Core/src/Platform/Android/IHandleWindowInsets.cs +++ b/src/Core/src/Platform/Android/IHandleWindowInsets.cs @@ -22,4 +22,34 @@ internal interface IHandleWindowInsets /// The view to reset void ResetWindowInsets(AView view); } + + /// + /// Interface for views that need window insets to be re-applied after layout transitions. + /// Views implementing this interface will have RequestApplyInsets posted when they extend + /// beyond screen bounds (e.g., during fragment transitions). + /// + /// + /// This is primarily used by Shell to handle fragment transitions where views are temporarily + /// positioned off-screen. Without this, views might get incorrect safe area calculations + /// during the transition. TabbedPage and other views that intentionally position content + /// off-screen should NOT implement this interface to avoid infinite inset request loops. + /// + internal interface IRequestInsetsOnTransition + { + } + + /// + /// Marker interface for ContentPage to indicate it should receive transition inset updates. + /// This is separate from IRequestInsetsOnTransition (which is for Shell hierarchy) because + /// we need to check that the actual view being processed is a ContentPage, not just that + /// it's under a Shell. + /// + /// + /// Only ContentPage should implement this interface. Other content views like ContentView, + /// ScrollView, Border, etc. should NOT implement this to avoid the infinite loop issue + /// when they are positioned off-screen (e.g., in TabbedPage inactive tabs). + /// + internal interface IContentPageController + { + } } \ No newline at end of file diff --git a/src/Core/src/Platform/Android/MauiWindowInsetListener.cs b/src/Core/src/Platform/Android/MauiWindowInsetListener.cs index 9763ec85ed32..3c9f12fb14db 100644 --- a/src/Core/src/Platform/Android/MauiWindowInsetListener.cs +++ b/src/Core/src/Platform/Android/MauiWindowInsetListener.cs @@ -33,6 +33,73 @@ internal class MauiWindowInsetListener : WindowInsetsAnimationCompat.Callback, I AView? _pendingView; + // Cached value for whether this listener's view hierarchy supports transition inset re-application. + // null = not yet computed, true/false = cached result + bool? _shouldRequestInsetsOnTransition; + + // Cached action for requesting insets re-application to avoid lambda allocations. + // Each view that needs deferred inset re-application gets its action cached here. + Action? _requestInsetsAction; + WeakReference? _requestInsetsViewRef; + + /// + /// Gets whether views in this listener's hierarchy should have insets re-applied during transitions. + /// This is determined by checking if any parent implements IRequestInsetsOnTransition. + /// The result is cached for performance and reset when the listener is disposed. + /// + internal bool ShouldRequestInsetsOnTransition(AView view) + { + if (_shouldRequestInsetsOnTransition.HasValue) + { + return _shouldRequestInsetsOnTransition.Value; + } + + // Walk parent hierarchy looking for IRequestInsetsOnTransition + bool result = false; + var parent = view.Parent; + while (parent != null) + { + if (parent is IRequestInsetsOnTransition) + { + result = true; + break; + } + parent = (parent as AView)?.Parent; + } + + _shouldRequestInsetsOnTransition = result; + return result; + } + + /// + /// Gets a cached Action that requests insets for the given view. + /// This avoids lambda allocations on each call, which can cause GC pressure + /// when called frequently (e.g., during animations). + /// + internal Action GetRequestInsetsAction(AView view) + { + // Check if we have a cached action for this view + if (_requestInsetsAction != null && _requestInsetsViewRef != null) + { + if (_requestInsetsViewRef.TryGetTarget(out var cachedView) && cachedView == view) + { + return _requestInsetsAction; + } + } + + // Create and cache a new action for this view + _requestInsetsViewRef = new WeakReference(view); + _requestInsetsAction = () => + { + if (_requestInsetsViewRef.TryGetTarget(out var targetView)) + { + ViewCompat.RequestApplyInsets(targetView); + } + }; + + return _requestInsetsAction; + } + // Static tracking for views that have local inset listeners. // This registry allows child views to find their appropriate listener without // relying on a global activity-level listener. diff --git a/src/Core/src/Platform/Android/SafeAreaExtensions.cs b/src/Core/src/Platform/Android/SafeAreaExtensions.cs index ab692611b6f1..060cb32bc3b9 100644 --- a/src/Core/src/Platform/Android/SafeAreaExtensions.cs +++ b/src/Core/src/Platform/Android/SafeAreaExtensions.cs @@ -8,6 +8,7 @@ namespace Microsoft.Maui.Platform; internal static class SafeAreaExtensions { + internal static ISafeAreaView2? GetSafeAreaView2(object? layout) => layout switch { @@ -24,6 +25,24 @@ internal static class SafeAreaExtensions _ => null }; + /// + /// Checks if the layout is associated with a ContentPage specifically. + /// This is used to limit the transition inset re-application to content pages only, + /// avoiding the infinite loop issue with other view types like TabbedPage tabs. + /// + internal static bool IsContentPageLayout(object? layout) + { + var virtualView = layout switch + { + IElementHandler handler => handler.VirtualView, + _ => layout + }; + + // Check if the virtual view implements IContentPageController (marker interface) + // Only ContentPage implements this interface on Android + return virtualView is IContentPageController; + } + internal static SafeAreaRegions GetSafeAreaRegionForEdge(int edge, ICrossPlatformLayout crossPlatformLayout) { @@ -149,13 +168,41 @@ internal static SafeAreaRegions GetSafeAreaRegionForEdge(int edge, ICrossPlatfor bool viewExtendsBeyondScreen = viewRight > screenWidth || viewBottom > screenHeight || viewLeft < 0 || viewTop < 0; - if (viewExtendsBeyondScreen) + if (viewExtendsBeyondScreen) { - // Request insets to be reapplied after the next layout pass - // when the view should be properly positioned. - // Don't return early - let processing continue with current insets - // to avoid visual popping, the re-apply will correct any issues. - view.Post(() => ViewCompat.RequestApplyInsets(view)); + // Only request re-apply for ContentView types (includes ContentPage) that + // opt-in via IRequestInsetsOnTransition AND are not completely off-screen. + // + // IRequestInsetsOnTransition is used by Shell for fragment transitions where + // views are temporarily off-screen. For other scenarios like TabbedPage inactive + // tabs, the view is intentionally off-screen and requesting insets would cause + // an infinite loop. See Issue #33731. + // + // The isCompletelyOffScreen check is an additional safety net - even for Shell, + // if a view has no intersection with the visible screen, there's no need to + // request inset re-application. + // + // The IsContentPageLayout check limits this behavior to ContentPage types only, + // which are the primary use case for Shell fragment transitions. + bool isCompletelyOffScreen = viewLeft >= screenWidth || viewRight <= 0 || + viewTop >= screenHeight || viewBottom <= 0; + + // Use the listener's cached method to check for IRequestInsetsOnTransition + bool shouldRequestInsets = globalWindowInsetsListener?.ShouldRequestInsetsOnTransition(view) == true; + + // Only apply to ContentPage types + bool isContentPage = IsContentPageLayout(layout); + + if (!isCompletelyOffScreen && shouldRequestInsets && isContentPage && globalWindowInsetsListener is not null) + { + // Request insets to be reapplied after the next layout pass + // when the view should be properly positioned. + // Don't return early - let processing continue with current insets + // to avoid visual popping, the re-apply will correct any issues. + // Use cached action to avoid lambda allocation on each call. + var requestInsetsAction = globalWindowInsetsListener.GetRequestInsetsAction(view); + view.Post(requestInsetsAction); + } } // Calculate actual overlap for each edge diff --git a/src/TestUtils/src/UITest.NUnit/UITestBase.cs b/src/TestUtils/src/UITest.NUnit/UITestBase.cs index 5206e2c7878d..3988d6bd49dc 100644 --- a/src/TestUtils/src/UITest.NUnit/UITestBase.cs +++ b/src/TestUtils/src/UITest.NUnit/UITestBase.cs @@ -35,6 +35,7 @@ public virtual void TestSetup() public virtual void TestTearDown() { RecordTestTeardown(); + LogGCCountFromDeviceLogs(); UITestBaseTearDown(); if (ResetAfterEachTest) { @@ -48,6 +49,65 @@ public void RecordTestTeardown() TestContext.Progress.WriteLine($">>>>> {DateTime.Now} {name} Stop"); } + /// + /// Parse [MAUI_GC] entries from device logs and log the GC count for this test. + /// This helps track GC activity across all UI tests to detect regressions. + /// + void LogGCCountFromDeviceLogs() + { + try + { + var types = App.GetLogTypes().ToArray(); + if (!types.Contains("logcat", StringComparer.InvariantCultureIgnoreCase)) + return; + + var entries = App.GetLogEntries("logcat"); + + // Look for [MAUI_GC] entries and extract the latest Gen0 count + int? latestGen0 = null; + string? testPage = null; + + foreach (var entry in entries) + { + if (!entry.Contains("[MAUI_GC]", StringComparison.Ordinal)) + continue; + + // Parse entries like: [MAUI_GC] TestPage=Issue33731, Gen0=5, Gen1=0, Gen2=0 + // or: [MAUI_GC] State=..., Gen0=5, Gen1=0, Gen2=0, ElapsedSec=10.5 + var match = System.Text.RegularExpressions.Regex.Match(entry, @"Gen0=(\d+)"); + if (match.Success && int.TryParse(match.Groups[1].Value, out int gen0)) + { + latestGen0 = gen0; + } + + // Also capture test page name if present + var pageMatch = System.Text.RegularExpressions.Regex.Match(entry, @"TestPage=([^,]+)"); + if (pageMatch.Success) + { + testPage = pageMatch.Groups[1].Value; + } + } + + if (latestGen0.HasValue) + { + var name = TestContext.CurrentContext.Test.MethodName ?? TestContext.CurrentContext.Test.Name; + var pageInfo = testPage != null ? $" (page: {testPage})" : ""; + TestContext.Progress.WriteLine($">>>>> [GC] {name}{pageInfo}: Gen0={latestGen0.Value} collections during test"); + + // Log a warning if GC count seems high (potential regression) + if (latestGen0.Value >= 5) + { + TestContext.Progress.WriteLine($">>>>> [GC WARNING] High GC count ({latestGen0.Value}) may indicate a memory issue!"); + } + } + } + catch (Exception e) + { + // Don't fail tests due to GC logging issues + TestContext.Progress.WriteLine($">>>>> [GC] Unable to parse GC logs: {e.Message}"); + } + } + protected virtual void FixtureSetup() { var name = TestContext.CurrentContext.Test.MethodName ?? TestContext.CurrentContext.Test.Name;