Merge pull request #747 from chenjt2001/fix-FAFrame

Decouple FAFrame BackStack from CacheSize and prevent memory leaks

src/FluentAvalonia/UI/Controls/Frame/FAFrame.cs

@@ -397,6 +397,8 @@ public partial class FAFrame : ContentControl
 
             if (addCurrentEntryToBackStack)
             {
+                // Set Instance to null to allow GC to collect it
+                CurrentEntry.Instance = null;
                 BackStack.Add(CurrentEntry);
                 CurrentEntry = null;
             }
@@ -458,13 +460,20 @@ public partial class FAFrame : ContentControl
             }
 
             // Navigate to new page
-            var prevEntry = CurrentEntry;
             bool wasPageSet = entry.Instance != null;
 
-            if (mode == FANavigationMode.New && !wasPageSet)
+            if (!wasPageSet)
             {
                 // Check if we already have an instance of the page in the cache
-                entry.Instance = CheckCacheAndGetPage(entry.SourcePageType);
+                // Context will not be null if NavigateCore is called from GoBack/GoForward and the entry was created from NavigateFromObject
+                if (entry.Context != null)
+                {
+                    entry.Instance = CheckCacheAndGetPage(null, entry.Context);
+                }
+                else
+                {
+                    entry.Instance = CheckCacheAndGetPage(entry.SourcePageType, null);
+                }
             }
 
             if (entry.Instance == null)
@@ -488,7 +497,7 @@ public partial class FAFrame : ContentControl
             CurrentEntry = entry;
 
             var navEA = new FluentAvalonia.UI.Navigation.FANavigationEventArgs(
-                CurrentEntry.Instance,
+                entry.Instance,
                 mode, entry.NavigationTransitionInfo,
                 entry.Parameter,
                 entry.SourcePageType);
@@ -498,6 +507,10 @@ public partial class FAFrame : ContentControl
             {
                 navEA.RoutedEvent = NavigatedFromEvent;
                 oldEntry.Instance.RaiseEvent(navEA);
+
+                // Set Instance to null to allow GC to collect it
+                // The page will still be retained in the cache if caching is enabled
+                oldEntry.Instance = null;
             }
 
             SetContentAndAnimate(entry);
@@ -510,28 +523,20 @@ public partial class FAFrame : ContentControl
                 {
                     case FANavigationMode.New:
                         ForwardStack.Clear();
-                        if (prevEntry != null)
+                        if (oldEntry != null)
                         {
-                            if (BackStack.Count == CacheSize)
-                            {
-                                if (BackStack.Count > 0)
-                                {
-                                    BackStack.RemoveAt(0);
-                                }
-                            }
-
-                            BackStack.Add(prevEntry);
+                            BackStack.Add(oldEntry);
                         }
                         break;
 
                     case FANavigationMode.Back:
-                        ForwardStack.Add(prevEntry);
-                        BackStack.Remove(CurrentEntry);
+                        ForwardStack.Add(oldEntry);
+                        BackStack.Remove(entry);
                         break;
 
                     case FANavigationMode.Forward:
-                        BackStack.Add(prevEntry);
-                        ForwardStack.Remove(CurrentEntry);
+                        BackStack.Add(oldEntry);
+                        ForwardStack.Remove(entry);
                         break;
 
                     case FANavigationMode.Refresh:

tests/FluentAvaloniaTests/ControlTests/FrameTests.cs

@@ -175,30 +175,35 @@ public class FrameTests
     }
 
     [AvaloniaFact]
-    public void PagesAreCachedWhenNavigatingIfUsingNavigationStack()
+    public void PagesAreDiscardedFromCacheWhenExceedingCacheSize()
     {
         var Context = GetTestContext();
 
         Assert.True(Context.Frame.IsNavigationStackEnabled);
 
+        Context.Frame.CacheSize = 2;
+
         Context.Frame.Navigate(typeof(TestPage1));
+        var page1 = Context.Frame.Content;
+
         Context.Frame.Navigate(typeof(TestPage2));
+        var page2 = Context.Frame.Content;
+
+        // Cause the cache to discard page 1
         Context.Frame.Navigate(typeof(TestPage3));
 
-        Assert.Equal(2, Context.Frame.BackStackDepth);
-
-        Assert.IsType<TestPage2>(Context.Frame.BackStack[1].Instance);
-        Assert.IsType<TestPage1>(Context.Frame.BackStack[0].Instance);
-
-        var prevPage = Context.Frame.BackStack[1].Instance;
-
+        // Test page are cached
         Context.Frame.GoBack();
+        Assert.Same(page2, Context.Frame.Content);
 
-        Assert.Equal(prevPage, Context.Frame.Content);
+        // Test cache size is respected
+        Context.Frame.GoBack();
+        Assert.NotSame(page1, Context.Frame.Content);
+        Assert.IsType<TestPage1>(Context.Frame.Content);
     }
 
     [AvaloniaFact]
-    public void DisablingNavigationStackPreventsPageCaching()
+    public void DisablingNavigationStackPreventsPageNavigation()
     {
         var Context = GetTestContext();