Fix Loaded being incorrectly raised for unloaded controls (#18471)

src/Avalonia.Controls/Control.cs

@@ -103,7 +103,7 @@ namespace Avalonia.Controls
         private static readonly HashSet<Control> _loadedQueue = new HashSet<Control>();
         private static readonly HashSet<Control> _loadedProcessingQueue = new HashSet<Control>();
 
-        private bool _isLoaded = false;
+        private LoadState _loadState = LoadState.Unloaded;
         private DataTemplates? _dataTemplates;
         private Control? _focusAdorner;
         private AutomationPeer? _automationPeer;
@@ -151,7 +151,7 @@ namespace Avalonia.Controls
         /// <remarks>
         /// This is set to true while raising the <see cref="Loaded"/> event.
         /// </remarks>
-        public bool IsLoaded => _isLoaded;
+        public bool IsLoaded => _loadState == LoadState.Loaded;
 
         /// <summary>
         /// Gets or sets a user-defined object attached to the control.
@@ -293,9 +293,10 @@ namespace Avalonia.Controls
         /// </summary>
         internal void ScheduleOnLoadedCore()
         {
-            if (_isLoaded == false)
+            if (_loadState == LoadState.Unloaded)
             {
                 bool isAdded = _loadedQueue.Add(this);
+                _loadState = LoadState.LoadPending;
 
                 if (isAdded &&
                     _isLoadedProcessing == false)
@@ -312,13 +313,18 @@ namespace Avalonia.Controls
         /// </summary>
         internal void OnLoadedCore()
         {
-            if (_isLoaded == false &&
+            if (_loadState == LoadState.LoadPending &&
                 ((ILogical)this).IsAttachedToLogicalTree)
             {
-                _isLoaded = true;
+                _loadState = LoadState.Loaded;
 
                 OnLoaded(new RoutedEventArgs(LoadedEvent, this));
             }
+            else
+            {
+                // We somehow got here while being detached?
+                _loadState = LoadState.Unloaded;
+            }
         }
 
         /// <summary>
@@ -327,15 +333,21 @@ namespace Avalonia.Controls
         /// </summary>
         internal void OnUnloadedCore()
         {
-            if (_isLoaded)
+            switch (_loadState)
             {
-                // Remove from the loaded event queue here as a failsafe in case the control
+                case LoadState.Loaded:
-                // is detached before the dispatcher runs the Loaded jobs.
+                    _loadState = LoadState.Unloaded;
-                _loadedQueue.Remove(this);
+
+                    OnUnloaded(new RoutedEventArgs(UnloadedEvent, this));
+                    break;
 
-                _isLoaded = false;
+                case LoadState.LoadPending:
+                    // Remove from the loaded event queue here as a failsafe in case the control
+                    // is detached before the dispatcher runs the Loaded jobs.
+                    _loadedQueue.Remove(this);
 
-                OnUnloaded(new RoutedEventArgs(UnloadedEvent, this));
+                    _loadState = LoadState.Unloaded;
+                    break;
             }
         }
 
@@ -561,5 +573,12 @@ namespace Avalonia.Controls
             _loadedProcessingQueue.Clear();
             _isLoadedProcessing = false;
         }
+
+        private enum LoadState : byte
+        {
+            Unloaded,
+            Loaded,
+            LoadPending
+        }
     }
 }

tests/Avalonia.Controls.UnitTests/LoadedTests.cs

@@ -72,4 +72,38 @@ public class LoadedTests
             Assert.False(target.IsLoaded);
         }
     }
+
+    [Fact]
+    public void Loaded_Should_Not_Be_Raised_If_Detached_From_Visual_Tree()
+    {
+        using var app = UnitTestApplication.Start(TestServices.StyledWindow);
+
+        var loadedCount = 0;
+        var unloadedCount = 0;
+        var window = new Window();
+        window.Show();
+
+        var target = new Button();
+
+        target.Loaded += (_, _) => loadedCount++;
+        target.Unloaded += (_, _) => unloadedCount++;
+
+        Assert.Equal(0, loadedCount);
+        Assert.Equal(0, unloadedCount);
+
+        // Attach to, then immediately detach from the visual tree.
+        window.Content = target;
+        window.Content = null;
+
+        // Attach to another logical parent (this can actually happen outside tests with overlay popups)
+        ((ISetLogicalParent) target).SetParent(new Window());
+
+        Dispatcher.UIThread.RunJobs(DispatcherPriority.Loaded);
+
+        // At this point, the control shouldn't have been loaded at all.
+        Assert.Null(target.VisualParent);
+        Assert.False(target.IsLoaded);
+        Assert.Equal(0, loadedCount);
+        Assert.Equal(0, unloadedCount);
+    }
 }