[Android] Refactor WindowInsetListener to per-view registry with MauiWindowInsetListener

[Copilot]

Improved implementation of WindowInsetListener for per-view layout based on PR #31898

Comparison with PR #31898

After analyzing PR #31898, I've developed my own implementation approach and then compared it with the existing PR. Here's a detailed comparison:

Original PR Approach:

• Introduces a static registry system for CoordinatorLayout instances with local listeners
• Removes global listener from MauiAppCompatActivity
• Lambda captures were identified as a potential issue (addressed in later commits)

My Improvements Over Original:

• ✅ Add proper nullability annotations (?) on fields in Core project where nullable is enabled
• ✅ Handle nullability properly in files with #nullable disable directive (Shell fragments)
• ✅ Optimize IsViewContainedIn with early termination when encountering another CoordinatorLayout - this prevents incorrect listener lookup when layouts are nested
• ✅ Add comprehensive thread safety documentation explaining Android's UI thread requirement - makes it clear that no locking is needed
• ✅ Refactor IME animation handling with IsImeAnimation helper method - reduces code duplication and improves readability
• ✅ Improve code organization and comments for better maintainability
• ✅ Use .ToArray() instead of new List<>() for better performance in ResetAllViews - eliminates unnecessary list allocation
• ✅ Simplify variable names (appBarHasContent vs appBarLayoutContainsSomething, hasBottomNav vs bottomNavigation) - improves code clarity
• ✅ Eliminate redundant field storage - Use typed CoordinatorLayout fields instead of separate tracking variables
• ✅ Rename to MauiWindowInsetListener - Better name that doesn't imply "global" scope, more accurately reflects per-view nature
• ✅ Add RegisterParentForChildViews API - Clean static method for registering parent views without attaching listeners to the parent itself
• ✅ Update all view handlers to use registry-based approach
• ✅ Remove modal-specific listener code
• ✅ Format all changed files according to .editorconfig
• ✅ Build and test successfully
• ✅ Pass code review and security scan
• ✅ Fix ScrollViewTests.ShouldGrow to isolate from safe area changes

Why Not Use the Existing PR

While PR #31898 provides a solid foundation, my implementation offers several key improvements:

1. Better Performance: Early termination optimization in view hierarchy traversal
2. Clearer Code: Helper methods, better variable names, comprehensive documentation, typed fields
3. More Robust: Better null handling, proper disposal patterns, eliminates redundant storage
4. Better Maintainability: Clearer comments explaining the "why" not just the "what"
5. Better Naming: MauiWindowInsetListener instead of GlobalWindowInsetListener reflects the per-view architecture
6. Cleaner API: RegisterParentForChildViews method for better encapsulation and avoids unnecessary listener instantiation

Key Changes

Renamed Class and Methods

• MauiWindowInsetListener: Renamed from GlobalWindowInsetListener to better reflect its per-view nature
• MauiWindowInsetListenerExtensions: Extension methods updated with new naming
• TrySetMauiWindowInsetListener: Extension method for setting listener on views
• RemoveMauiWindowInsetListener: Extension method for cleanup

New API

• RegisterParentForChildViews: Static method that registers a parent view so its children can find an inset listener, without attaching the listener to the parent itself. This improves encapsulation and reduces unnecessary listener instantiation in handlers like FlyoutViewHandler.

Test Fixes

• ScrollViewTests.ShouldGrow: Set SafeAreaEdges to None on all layouts and scrollviews to isolate the test from safe area inset handling changes introduced by this PR.

Implementation Plan

• Analyze existing GlobalWindowInsetListener.cs thoroughly
• Implement improved registry system with:
  • Proper nullability markers on ViewEntry record
  • Optimized view hierarchy traversal with early termination
  • Comprehensive thread safety documentation
  • Helper method for IME animation detection
• Update ShellContentFragment.cs with typed CoordinatorLayout field
• Update ShellSectionRenderer.cs with typed CoordinatorLayout field
• Update NavigationRootManager.cs with local listener support
• Remove global listener from MauiAppCompatActivity.cs
• Update ViewHandler.Android.cs to use registry-based approach
• Update all view groups (ContentViewGroup, LayoutViewGroup, MauiScrollView) to use new pattern
• Remove modal-specific listener code from ModalNavigationManager.Android.cs
• Update SafeAreaExtensions.cs
• Update WindowHandler.Android.cs
• Format all changed files according to .editorconfig
• Build and test the changes
• Run code review and security scan
• Address review feedback - simplify storage in Shell fragments
• Rename GlobalWindowInsetListener to MauiWindowInsetListener
• Add RegisterParentForChildViews method for better encapsulation
• Fix ScrollViewTests.ShouldGrow test to be isolated from safe area changes

Security Summary

✅ No security vulnerabilities detected by CodeQL analysis

• All null reference checks properly implemented
• Weak references used to prevent memory leaks
• Proper disposal patterns followed

Build Status

✅ All builds successful

• Core project for Android: ✅ Success
• Controls.Core project for Android: ✅ Success
• Zero warnings, zero errors

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Fixes

• [Android] Refactor WindowInsetListener to per-CoordinatorLayout registry with optimized lookups #32301
• [.NET 10] The gap in the bottom of the emulator increases while scrolling. #32436

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

[jsuarezruiz]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[Copilot]

Pull Request Overview

This PR refactors the GlobalWindowInsetListener architecture from a single activity-level instance to a local listener pattern attached to specific views. This improves isolation in complex navigation scenarios (modals, flyouts, shell navigation) and eliminates cross-contamination of inset tracking between different UI contexts.

Key changes:

• Introduces per-view local listeners with static registry for better isolation
• Removes activity-level GlobalWindowInsetListener singleton pattern
• Adds registration/unregistration methods for view lifecycle management
• Simplifies code with null-conditional operators and improved formatting
• Converts indentation from spaces to tabs in several files

Reviewed Changes

Copilot reviewed 21 out of 24 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
GlobalWindowInsetListener.cs	Major refactor: adds static registry, view registration/unregistration, local listener setup methods
SafeAreaExtensions.cs	Converts spacing to tabs, updates to use FindListenerForView
NavigationRootManager.cs	Manages CoordinatorLayout listener lifecycle, uses new local listener pattern
MauiAppCompatActivity.cs	Removes activity-level GlobalWindowInsetListener singleton
ViewHandler.Android.cs	Updates to use FindListenerForView instead of context extension
FlyoutViewHandler.Android.cs	Adds local listener setup/cleanup for flyout views
ModalNavigationManager.Android.cs	Removes modal-specific listener, relies on local pattern
Shell renderers	Updates to use SetupViewWithLocalListener for CoordinatorLayouts
Multiple extension files	Code cleanup: null-conditional operators, whitespace fixes

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

@copilot can you rename the GlobalWindowInsetListener to just MauiWindowInsetListener?

[Copilot]

@copilot can you rename the GlobalWindowInsetListener to just MauiWindowInsetListener?

Renamed GlobalWindowInsetListener to MauiWindowInsetListener throughout the codebase in commit daae097. This better reflects its purpose as a container-level listener rather than a global one.

[PureWeen]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[PureWeen]

/backport to release/10.0.1xx-sr1

[github-actions]

Started backporting to release/10.0.1xx-sr1 (link to workflow run)

[mattleibow]

Performance Analysis: FindListenerForView in PR #32278

Problem: Nested Loop O(n*m) Complexity

The current implementation has nested loops creating O(n*m) time complexity:

internal static MauiWindowInsetListener? FindListenerForView(AView view)
{
    // Walk up the view hierarchy looking for a registered view
    var parent = view.Parent;
    while (parent is not null)  // ← Outer loop: n = hierarchy depth
    {
        // Skip setting listener on views inside nested scroll containers or AppBarLayout (except MaterialToolbar)
        if (view is not MaterialToolbar &&
            (parent is AppBarLayout || parent is MauiScrollView || parent is IMauiRecyclerView))
        {
            return null;
        }

        if (parent is AView parentView)
        {
            // Check if this parent view is registered
            // Clean up dead references while searching
            for (int i = _registeredViews.Count - 1; i >= 0; i--)  // ← Inner loop: m = registry size
            {
                var entry = _registeredViews[i];
                if (!entry.View.TryGetTarget(out var registeredView))
                {
                    _registeredViews.RemoveAt(i);
                }
                else if (ReferenceEquals(registeredView, parentView))
                {
                    return entry.Listener;
                }
            }
        }

        parent = parent.Parent;
    }

    return null;
}

Complexity Analysis:

• n = depth of view hierarchy (typically 10-20+ levels)
• m = number of registered views (grows with Shell tabs, pages, modals)
• Total: O(n*m) - Every hierarchy level checks every registered view

Why This Matters:

1. View hierarchies can be deep (10-20+ levels in Shell apps)
2. Registry grows with multiple Shell tabs, pages, modals
3. This runs every time a view needs to find its listener (during layout operations)
4. Cleanup happens inline during lookups, slowing hot path

---

Option 1: ConditionalWeakTable (RECOMMENDED)

Best choice: O(n) lookup, automatic cleanup, thread-safe, idiomatic .NET

static readonly ConditionalWeakTable<AView, MauiWindowInsetListener> _registeredViews = new();

internal static void SetupViewWithLocalListener(AView view)
{
    var listener = new MauiWindowInsetListener(view);
    _registeredViews.AddOrUpdate(view, listener);
    ViewCompat.SetOnApplyWindowInsetsListener(view, listener);
}

internal static MauiWindowInsetListener? FindListenerForView(AView view)
{
    var parent = view.Parent;
    while (parent is not null)
    {
        // Skip setting listener on views inside nested scroll containers or AppBarLayout (except MaterialToolbar)
        if (view is not MaterialToolbar &&
            (parent is AppBarLayout || parent is MauiScrollView || parent is IMauiRecyclerView))
        {
            return null;
        }

        if (parent is AView parentView && 
            _registeredViews.TryGetValue(parentView, out var listener))
        {
            return listener;
        }

        parent = parent.Parent;
    }

    return null;
}

internal static void RemoveViewWithLocalListener(AView view)
{
    if (_registeredViews.TryGetValue(view, out var listener))
    {
        listener.Dispose();
        _registeredViews.Remove(view);
    }
}

// Remove the old ViewEntry record - no longer needed

Benefits:

• ✅ O(n) complexity - Only walks hierarchy, no inner loop
• ✅ Automatic cleanup - Entries removed when views are garbage collected
• ✅ Thread-safe - Built-in synchronization
• ✅ Idiomatic .NET - Designed exactly for this use case (associating data with object lifetime)
• ✅ Simpler code - No manual weak reference management
• ✅ No cleanup overhead - GC handles it automatically

Drawbacks:

• Minor additional memory overhead per entry (negligible)

---

Option 2: Dictionary with RuntimeHelpers.GetHashCode

Good choice: O(n) lookup, manual cleanup, lightweight

static readonly Dictionary<int, WeakReference<MauiWindowInsetListener>> _registeredViews = new();
static readonly List<(int hash, WeakReference<AView> view)> _cleanupTracker = new();

internal static void SetupViewWithLocalListener(AView view)
{
    var listener = new MauiWindowInsetListener(view);
    int viewHash = System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(view);
    
    _registeredViews[viewHash] = new WeakReference<MauiWindowInsetListener>(listener);
    _cleanupTracker.Add((viewHash, new WeakReference<AView>(view)));
    
    ViewCompat.SetOnApplyWindowInsetsListener(view, listener);
}

internal static MauiWindowInsetListener? FindListenerForView(AView view)
{
    var parent = view.Parent;
    while (parent is not null)
    {
        if (view is not MaterialToolbar &&
            (parent is AppBarLayout || parent is MauiScrollView || parent is IMauiRecyclerView))
        {
            return null;
        }

        if (parent is AView parentView)
        {
            int parentHash = System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(parentView);
            if (_registeredViews.TryGetValue(parentHash, out var listenerRef) &&
                listenerRef.TryGetTarget(out var listener))
            {
                return listener;
            }
        }

        parent = parent.Parent;
    }

    return null;
}

internal static void RemoveViewWithLocalListener(AView view)
{
    int viewHash = System.Runtime.CompilerServices.RuntimeHelpers.GetHashCode(view);
    
    if (_registeredViews.TryGetValue(viewHash, out var listenerRef) &&
        listenerRef.TryGetTarget(out var listener))
    {
        listener.Dispose();
    }
    
    _registeredViews.Remove(viewHash);
    
    // Periodic cleanup
    CleanupDeadReferences();
}

static void CleanupDeadReferences()
{
    // Only cleanup periodically (e.g., every 10th removal) to avoid overhead
    if (_cleanupTracker.Count < 10 || _cleanupTracker.Count % 10 != 0)
        return;

    for (int i = _cleanupTracker.Count - 1; i >= 0; i--)
    {
        var (hash, viewRef) = _cleanupTracker[i];
        if (!viewRef.TryGetTarget(out _))
        {
            _cleanupTracker.RemoveAt(i);
            _registeredViews.Remove(hash);
        }
    }
}

Benefits:

• ✅ O(n) complexity - Dictionary lookup is O(1) average case
• ✅ Lightweight - Standard collections
• ✅ Predictable - Explicit control over cleanup timing

Drawbacks:

• ❌ Manual cleanup required - Must implement cleanup logic
• ❌ Hash collision risk - Small chance of collision (very unlikely in practice)
• ❌ More complex code - Dual tracking of hash and weak refs
• ⚠️ Not thread-safe - Would need locking if accessed from multiple threads

---

Option 3: Simple Cache for Last Result

Minimal change: Adds O(1) fast path for common case, keeps existing code

static AView? _lastQueriedView;
static MauiWindowInsetListener? _lastFoundListener;

internal static MauiWindowInsetListener? FindListenerForView(AView view)
{
    // Quick cache check for repeated lookups on same branch
    if (_lastQueriedView != null && IsViewContainedIn(view, _lastQueriedView))
        return _lastFoundListener;

    var parent = view.Parent;
    while (parent is not null)
    {
        if (view is not MaterialToolbar &&
            (parent is AppBarLayout || parent is MauiScrollView || parent is IMauiRecyclerView))
        {
            _lastQueriedView = null;
            _lastFoundListener = null;
            return null;
        }

        if (parent is AView parentView)
        {
            for (int i = _registeredViews.Count - 1; i >= 0; i--)
            {
                var entry = _registeredViews[i];
                if (!entry.View.TryGetTarget(out var registeredView))
                {
                    _registeredViews.RemoveAt(i);
                }
                else if (ReferenceEquals(registeredView, parentView))
                {
                    // Cache the result
                    _lastQueriedView = parentView;
                    _lastFoundListener = entry.Listener;
                    return entry.Listener;
                }
            }
        }

        parent = parent.Parent;
    }

    _lastQueriedView = null;
    _lastFoundListener = null;
    return null;
}

static bool IsViewContainedIn(AView child, AView potentialAncestor)
{
    var parent = child.Parent;
    while (parent is not null)
    {
        if (ReferenceEquals(parent, potentialAncestor))
            return true;
        parent = parent.Parent;
    }
    return false;
}

Benefits:

• ✅ O(1) for cache hits - Very fast for consecutive views in same hierarchy
• ✅ Minimal code change - Preserves existing logic
• ✅ Safe fallback - Falls back to existing algorithm on cache miss

Drawbacks:

• ❌ Still O(n*m) worst case - Doesn't solve fundamental problem
• ❌ Cache invalidation complexity - Single-entry cache is simplistic
• ⚠️ Limited effectiveness - Only helps when same parent queried repeatedly

---

Option 4: Separate Cleanup Pass

Hybrid approach: Keep list structure, decouple cleanup from lookup

static readonly List<ViewEntry> _registeredViews = new();
static int _lookupsSinceCleanup = 0;
const int CLEANUP_THRESHOLD = 100;

internal static MauiWindowInsetListener? FindListenerForView(AView view)
{
    var parent = view.Parent;
    while (parent is not null)
    {
        if (view is not MaterialToolbar &&
            (parent is AppBarLayout || parent is MauiScrollView || parent is IMauiRecyclerView))
        {
            return null;
        }

        if (parent is AView parentView)
        {
            // Fast lookup without cleanup
            foreach (var entry in _registeredViews)
            {
                if (entry.View.TryGetTarget(out var registeredView) && 
                    ReferenceEquals(registeredView, parentView))
                {
                    return entry.Listener;
                }
            }
        }

        parent = parent.Parent;
    }

    // Periodic cleanup after many lookups
    if (++_lookupsSinceCleanup >= CLEANUP_THRESHOLD)
    {
        CleanupDeadReferences();
        _lookupsSinceCleanup = 0;
    }

    return null;
}

static void CleanupDeadReferences()
{
    for (int i = _registeredViews.Count - 1; i >= 0; i--)
    {
        if (!_registeredViews[i].View.TryGetTarget(out _))
        {
            _registeredViews.RemoveAt(i);
        }
    }
}

internal static void RemoveViewWithLocalListener(AView view)
{
    for (int i = _registeredViews.Count - 1; i >= 0; i--)
    {
        if (_registeredViews[i].View.TryGetTarget(out var registeredView) && 
            ReferenceEquals(registeredView, view))
        {
            _registeredViews[i].Listener?.Dispose();
            _registeredViews.RemoveAt(i);
            break;
        }
    }
    
    // Explicit cleanup on removal
    CleanupDeadReferences();
}

Benefits:

• ✅ Faster hot path - Lookup doesn't do cleanup
• ✅ Keeps existing structure - Minimal refactoring
• ✅ Predictable cleanup - Happens at known intervals

Drawbacks:

• ❌ Still O(n*m) - Fundamental complexity unchanged
• ⚠️ Memory buildup - Dead entries accumulate between cleanups
• ⚠️ Tuning required - CLEANUP_THRESHOLD affects memory vs CPU tradeoff

---

Performance Comparison

Option	Lookup Complexity	Memory Overhead	Cleanup	Thread-Safe	Code Complexity
Current	O(n*m)	Low	Inline (slow)	No	Medium
Option 1: ConditionalWeakTable	O(n)	Low	Automatic	Yes	Low
Option 2: Dictionary	O(n)	Low	Manual	No	High
Option 3: Cache	O(1)/O(n*m)	Minimal	Inline (slow)	No	Low
Option 4: Deferred Cleanup	O(n*m)	Medium	Batched	No	Medium

---

Recommendation

Use Option 1: ConditionalWeakTable

This is the best solution because:

1. ✅ Solves the root problem - Eliminates the inner loop entirely (O(n) vs O(n*m))
2. ✅ Built for this use case - ConditionalWeakTable is specifically designed to associate data with object lifetimes
3. ✅ Production-ready - Thread-safe, automatic cleanup, well-tested in .NET runtime
4. ✅ Minimal code - Actually simpler than the current implementation
5. ✅ No tuning needed - No cleanup thresholds or cache invalidation logic
6. ✅ Future-proof - Scales better as apps grow more complex

The current implementation likely works fine with small registry sizes (< 10 views), but for foundational infrastructure that runs on every layout pass, the optimization is worthwhile and eliminates a potential performance cliff as apps scale.

---

Migration Notes

If choosing Option 1 (ConditionalWeakTable):

1. Remove the ViewEntry record definition
2. Replace List<ViewEntry> _registeredViews with ConditionalWeakTable<AView, MauiWindowInsetListener> _registeredViews
3. Update SetupViewWithLocalListener to use AddOrUpdate
4. Simplify FindListenerForView to remove inner loop
5. Simplify RemoveViewWithLocalListener to just call Remove
6. Remove all cleanup logic (automatic now)
7. Consider removing ResetAllViews() or rewriting with ConditionalWeakTable iteration

Breaking Changes: None - API surface remains identical
Risk Level: Low - ConditionalWeakTable is well-tested runtime feature
Testing: Ensure Shell tab switching, modal presentation, and memory leaks tests pass

[mattleibow]

Overall I think it is good.