fix: address remaining review findings from PR #55's floating feature

src/AsyncNavigation.Avalonia.Floating/AvaloniaFloatingWindowHost.cs

@@ -31,7 +31,7 @@ public IFloatingWindowHost Create(FloatingWindowOptions options)
 internal sealed class AvaloniaFloatingWindowHost : IFloatingWindowHost
 {
     private readonly Window _window;
-    private bool _allowClose;
+    private readonly FloatingWindowCloseGuard _closeGuard = new();
     private readonly ContentControl _content = new();
 
     public AvaloniaFloatingWindowHost(FloatingWindowOptions options)
@@ -71,7 +71,7 @@ public Task ActivateAsync(CancellationToken cancellationToken = default) =>
     public Task CloseAsync(CancellationToken cancellationToken = default) =>
         InvokeAsync(() =>
         {
-            _allowClose = true;
+            _closeGuard.AllowClose();
             _window.Close();
         }, cancellationToken);
 
@@ -82,15 +82,15 @@ await InvokeAsync(() =>
             _window.Closing -= OnClosing;
             if (_window.IsVisible)
             {
-                _allowClose = true;
+                _closeGuard.AllowClose();
                 _window.Close();
             }
         }, CancellationToken.None);
     }
 
     private void OnClosing(object? sender, WindowClosingEventArgs e)
     {
-        if (_allowClose)
+        if (!_closeGuard.ShouldCancelAndNotify())
             return;
 
         e.Cancel = true;

src/AsyncNavigation.Floating/FloatingWindowCloseGuard.cs

@@ -0,0 +1,24 @@
+namespace AsyncNavigation.Floating;
+
+/// <summary>
+/// Encapsulates the "cancel the OS window's close, then raise <c>CloseRequested</c> instead"
+/// dance shared by every <see cref="IFloatingWindowHost"/> backed by a real window: closing the
+/// window via the OS chrome (or Alt+F4 etc.) must not close it outright - it has to be turned
+/// into a request that the caller can still veto or handle asynchronously via
+/// <c>CloseAsync</c>/<c>DisposeAsync</c>. Shared so this exact state transition - easy to get
+/// subtly wrong per platform - only has to be implemented once.
+/// </summary>
+public sealed class FloatingWindowCloseGuard
+{
+    private volatile bool _allowClose;
+
+    /// <summary>Call before actually closing the window from <c>CloseAsync</c>/<c>DisposeAsync</c>.</summary>
+    public void AllowClose() => _allowClose = true;
+
+    /// <summary>
+    /// Call from the window's own Closing handler. Returns true when the close was not
+    /// explicitly requested via <see cref="AllowClose"/> - the caller must then cancel the
+    /// window's own close and raise its <c>CloseRequested</c> event instead.
+    /// </summary>
+    public bool ShouldCancelAndNotify() => !_allowClose;
+}

src/AsyncNavigation.Wpf.Floating/WpfFloatingWindowHost.cs

@@ -21,7 +21,7 @@ public IFloatingWindowHost Create(FloatingWindowOptions options)
 internal sealed class WpfFloatingWindowHost : IFloatingWindowHost
 {
     private readonly Window _window;
-    private bool _allowClose;
+    private readonly FloatingWindowCloseGuard _closeGuard = new();
     private readonly ContentControl _content = new();
 
     public WpfFloatingWindowHost(FloatingWindowOptions options)
@@ -61,7 +61,7 @@ public Task ActivateAsync(CancellationToken cancellationToken = default) =>
     public Task CloseAsync(CancellationToken cancellationToken = default) =>
         InvokeAsync(() =>
         {
-            _allowClose = true;
+            _closeGuard.AllowClose();
             _window.Close();
         }, cancellationToken);
 
@@ -72,15 +72,15 @@ await InvokeAsync(() =>
             _window.Closing -= OnClosing;
             if (_window.IsVisible)
             {
-                _allowClose = true;
+                _closeGuard.AllowClose();
                 _window.Close();
             }
         }, CancellationToken.None);
     }
 
     private void OnClosing(object? sender, CancelEventArgs e)
     {
-        if (_allowClose)
+        if (!_closeGuard.ShouldCancelAndNotify())
             return;
         e.Cancel = true;
         _ = _window.Dispatcher.InvokeAsync(

src/AsyncNavigation/Abstractions/IViewManager.cs

@@ -8,5 +8,7 @@ Task<IView> ResolveViewAsync(string key,
         Func<IView, Task>? initialize = null);
     void AddView(string key, IView view);
     void Clear();
-    void Remove(string key, bool dispose = false);
+    /// <summary>Removes the cache entry for <paramref name="key"/>. Returns false without removing
+    /// anything if the cached instance is currently floating (pinned) or the key isn't cached.</summary>
+    bool Remove(string key, bool dispose = false);
 }

src/AsyncNavigation/RegionNavigationService.cs

@@ -220,7 +220,14 @@ private async Task OnAfterNavigationAsync(NavigationContext navigationContext)
         {
             SubscribeUnload(navigationContext);
             await aware.OnNavigatedToAsync(navigationContext);
-            navigationContext.CancellationToken.ThrowIfCancellationRequested();
+            if (navigationContext.CancellationToken.IsCancellationRequested)
+            {
+                // Activation succeeded but the operation is being cancelled/rolled back right
+                // after - notify the view it's leaving too, so its lifecycle state doesn't end
+                // up out of sync with the region reverting to whatever was active before.
+                await aware.OnNavigatedFromAsync(navigationContext);
+                navigationContext.CancellationToken.ThrowIfCancellationRequested();
+            }
             _current = (view, navigationContext);
         }
         if (navigationContext.Target.Value is { } target)

src/AsyncNavigation/ViewManager.cs

@@ -100,10 +100,10 @@ public async Task<IView> ResolveViewAsync(string key,
         }
     }
 
-    public void Remove(string cacheKey, bool dispose = false)
+    public bool Remove(string cacheKey, bool dispose = false)
     {
         if (_viewCache.TryGetValue(cacheKey, out var pinned) &&
-            pinned.TryGetTarget(out var active) && _placement?.IsFloating(active) == true) return;
+            pinned.TryGetTarget(out var active) && _placement?.IsFloating(active) == true) return false;
         if (_viewCache.TryRemove(cacheKey, out var viewRef))
         {
             lock (_lruLock)
@@ -119,7 +119,9 @@ public void Remove(string cacheKey, bool dispose = false)
             {
                 DisposeView(view);
             }
+            return true;
         }
+        return false;
     }
 
     public void RemoveInstance(IView view)
@@ -169,12 +171,18 @@ private void AddToLru(string key)
 
     private void TrimCache(string protectedKey)
     {
+        // Resume scanning from wherever the previous eviction in this call left off, instead of
+        // restarting from the tail every outer-loop iteration - floating (pinned) entries don't
+        // change status mid-call, so re-walking past the same ones on every eviction is wasted
+        // work that turns this into O(evictions * pinned) when floating views sit at the tail.
+        LinkedListNode<string>? node = null;
         while (_viewCache.Count > _maxCacheSize)
         {
             string? oldestKey = null;
             lock (_lruLock)
             {
-                var node = _lruList.Last;
+                if (node?.List != _lruList)
+                    node = _lruList.Last;
                 while (node is not null)
                 {
                     if (node.Value != protectedKey &&
@@ -185,8 +193,10 @@ private void TrimCache(string protectedKey)
                 }
                 if (node is null) break; // Active windows are pinned, even above the cache limit.
                 oldestKey = node.Value;
+                var toRemove = node;
+                node = node.Previous;
                 _lruIndex.Remove(oldestKey);
-                _lruList.Remove(node);
+                _lruList.Remove(toRemove);
             }
 
             if (oldestKey != null && _viewCache.TryRemove(oldestKey, out var viewRef))

src/AsyncNavigation/WeakUnloadObserver.cs

@@ -16,7 +16,7 @@ internal sealed class WeakUnloadObserver
     private static readonly ConditionalWeakTable<INavigationAware, AsyncEventHandler<AsyncEventArgs>> _subscriptions = new();
     private static readonly object _subscriptionsLock = new();
 
-    public static void Subscribe(INavigationAware navigationAware, Action<INavigationAware> onUnloadCallback)
+    public static void Subscribe(INavigationAware navigationAware, Func<INavigationAware, Task> onUnloadCallback)
     {
         var weakReference = new WeakReference<INavigationAware>(navigationAware);
 
@@ -32,7 +32,7 @@ async Task HandleRequestUnloadAsync(object? sender, AsyncEventArgs args)
             }
 
             await target.OnUnloadAsync(args.CancellationToken);
-            onUnloadCallback?.Invoke(target);
+            await onUnloadCallback(target);
         }
 
         // TryGetValue/Remove/Add aren't individually atomic against each other, and

tests/AsyncNavigation.Tests/Mocks/GuardTestNavigationAware.cs

@@ -7,16 +7,31 @@ public class GuardTestNavigationAware : INavigationAware, INavigationGuard
 {
     public static GuardTestNavigationAware? LastCreated { get; private set; }
 
+    /// <summary>Set before navigating (no view caching means a fresh instance is created per
+    /// navigation, so a static hook is what lets the next-created instance(s) pick it up) to run
+    /// <see cref="OnActivatedCallback"/> on whichever instance gets created next. Not consumed/
+    /// cleared on construction: GuardTestView's own constructor parameter causes the DI container
+    /// to construct a throwaway instance before the one actually wired as its DataContext, so
+    /// every instance built while this is set picks up the same reference - only the one that
+    /// ends up as the real DataContext ever has OnNavigatedToAsync invoked on it. Callers should
+    /// reset this back to null themselves once done to avoid leaking into other tests.</summary>
+    public static Action? NextActivatedCallback { get; set; }
+
     public GuardTestNavigationAware()
     {
         LastCreated = this;
+        OnActivatedCallback = NextActivatedCallback;
     }
 
     public event AsyncEventHandler<AsyncEventArgs>? AsyncRequestUnloadEvent;
     public bool AllowNavigation { get; set; } = true;
     public bool GuardWasCalled { get; private set; }
     public bool NavigatedFromWasCalled { get; private set; }
 
+    /// <summary>Invoked at the end of <see cref="OnNavigatedToAsync"/>, before it returns - lets a
+    /// test simulate cancellation racing with a just-completed activation.</summary>
+    public Action? OnActivatedCallback { get; set; }
+
     public Task<bool> CanNavigateAsync(NavigationContext context, CancellationToken cancellationToken)
     {
         GuardWasCalled = true;
@@ -26,6 +41,10 @@ public Task<bool> CanNavigateAsync(NavigationContext context, CancellationToken
     public Task InitializeAsync(NavigationContext context) => Task.CompletedTask;
     public Task<bool> IsNavigationTargetAsync(NavigationContext context) => Task.FromResult(true);
     public Task OnNavigatedFromAsync(NavigationContext context) { NavigatedFromWasCalled = true; return Task.CompletedTask; }
-    public Task OnNavigatedToAsync(NavigationContext context) => Task.CompletedTask;
+    public Task OnNavigatedToAsync(NavigationContext context)
+    {
+        OnActivatedCallback?.Invoke();
+        return Task.CompletedTask;
+    }
     public Task OnUnloadAsync(CancellationToken cancellationToken) => Task.CompletedTask;
 }

tests/AsyncNavigation.Tests/RegionManagerTests.cs

@@ -207,4 +207,35 @@ public async Task NavigationGuard_WhenBlocksNavigation_DoesNotCallOnNavigatedFro
         Assert.True(guardVm.GuardWasCalled);
         Assert.False(guardVm.NavigatedFromWasCalled);
     }
+
+    [Fact]
+    public async Task Navigation_CancelledRightAfterActivation_StillCallsOnNavigatedFrom()
+    {
+        var region = TestRegion.Build(_serviceProvider);
+        _regionManager.TryRemoveRegion("Guard4", out _);
+        _regionManager.AddRegion("Guard4", region);
+
+        using var cts = new CancellationTokenSource();
+        var callbackRan = false;
+        // Simulates cancellation racing in right after OnNavigatedToAsync succeeds but before the
+        // pipeline's own cancellation check - the view was told it's active, so it must also be
+        // told it's leaving when the operation is rolled back, or its lifecycle state goes out of
+        // sync with the region reverting.
+        GuardTestNavigationAware.NextActivatedCallback = () =>
+        {
+            callbackRan = true;
+            cts.Cancel();
+        };
+
+        var result = await _regionManager.RequestNavigateAsync("Guard4", "GuardTestView", cancellationToken: cts.Token);
+        var guardVm = GuardTestNavigationAware.LastCreated!;
+        GuardTestNavigationAware.NextActivatedCallback = null;
+
+        _regionManager.TryRemoveRegion("Guard4", out _);
+
+        Assert.True(callbackRan, "activation callback never ran");
+        Assert.True(cts.IsCancellationRequested, "cts was not cancelled");
+        Assert.True(result.IsCancelled, result.ToString());
+        Assert.True(guardVm.NavigatedFromWasCalled);
+    }
 }