-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[Android] SafeArea: Skip RequestApplyInsets for completely off-screen views #33747
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Closed
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
0001afc
Checkpoint from Copilot CLI for coding agent session
PureWeen ce19634
Revert Sandbox test changes
Copilot cc2b94e
Fix TabbedPage continuous GC issue (#33731)
PureWeen 95e19e1
Improve try-fix skill with explicit output file creation
PureWeen e2c40ef
try-fix skill: Require actual test execution for Pass
PureWeen 7052387
try-fix skill: Add baseline.log requirement to prove baseline was est…
PureWeen 985c283
try-fix skill: Add sequential execution requirement - no parallel runs
PureWeen 79b3eee
ai-summary-comment: Fix dry-run mode to accumulate attempts instead o…
PureWeen f7e80a5
ai-summary-comment: Remove emojis and fix header deduplication regex
PureWeen 50f36c6
ai-summary-comment: Use ASCII-safe status indicators instead of emojis
PureWeen c702e50
Revert skill/script changes (not part of this fix)
PureWeen 746fb5c
Fix: Use geometric off-screen check to prevent infinite RequestApplyI…
PureWeen a06a742
Fix: Use internal IRequestInsetsOnTransition interface with caching a…
PureWeen 643e8ed
Refactor: Move transition insets cache from ConditionalWeakTable to M…
PureWeen cdd6558
Add UI test for Issue #33768 - ScrollView content GC regression test
PureWeen bb806f0
Add Issue #33768 UITest and GC monitoring infrastructure for all UITests
PureWeen 8df322c
Refactor: Add cached delegate, ContentView check, and improve comments
PureWeen d6a7df3
Revert Sandbox changes to match main
PureWeen 5b76f04
Restrict dispatch to ContentPage only (not ContentView)
PureWeen 6a7671e
Use IContentPageController marker interface instead of GetType()
PureWeen 34d9b95
Add code style anti-patterns: never use GetType() for type checking
PureWeen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
| { | ||
| } | ||
| } |
111 changes: 111 additions & 0 deletions
111
src/Controls/tests/TestCases.HostApp/GCMonitoringService.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,111 @@ | ||
| #nullable enable | ||
| using System.Diagnostics; | ||
|
|
||
| namespace Maui.Controls.Sample; | ||
|
|
||
| /// <summary> | ||
| /// 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. | ||
| /// </summary> | ||
| 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; | ||
|
|
||
| /// <summary> | ||
| /// Initialize the GC monitoring service. Call once at app startup. | ||
| /// </summary> | ||
| 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}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Log when a test page is navigated to. Helps correlate GC activity with specific tests. | ||
| /// </summary> | ||
| 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}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Log when a test page is left. Shows GC activity during the test. | ||
| /// </summary> | ||
| 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; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Log current GC counts. Can be called at any time for debugging. | ||
| /// </summary> | ||
| 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}"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Get GC counts since initialization. | ||
| /// </summary> | ||
| 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 | ||
| ); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Log with the [MAUI_GC] prefix that the test infrastructure will parse. | ||
| /// </summary> | ||
| 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}"); | ||
| } | ||
| } | ||
187 changes: 187 additions & 0 deletions
187
src/Controls/tests/TestCases.HostApp/Issues/Issue33731.cs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,187 @@ | ||
| #nullable enable | ||
| namespace Maui.Controls.Sample.Issues; | ||
|
|
||
| /// <summary> | ||
| /// 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 | ||
| /// </summary> | ||
| [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(); | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot anything you're doing in here with
DateTimemath should be using theStopwatchclass instead.