Merge pull request #4578 from AvaloniaUI/fixes/devtools-tree-updates

Fix DevTools tree not updating

src/Avalonia.Controls/Utils/CollectionChangedEventManager.cs

@@ -17,12 +17,12 @@ namespace Avalonia.Controls.Utils
         void PostChanged(INotifyCollectionChanged sender, NotifyCollectionChangedEventArgs e);
     }
 
-    internal class CollectionChangedEventManager : IWeakSubscriber<NotifyCollectionChangedEventArgs>
+    internal class CollectionChangedEventManager
     {
         public static CollectionChangedEventManager Instance { get; } = new CollectionChangedEventManager();
 
-        private ConditionalWeakTable<INotifyCollectionChanged, List<WeakReference<ICollectionChangedListener>>> _entries =
-            new ConditionalWeakTable<INotifyCollectionChanged, List<WeakReference<ICollectionChangedListener>>>();
+        private ConditionalWeakTable<INotifyCollectionChanged, Entry> _entries =
+            new ConditionalWeakTable<INotifyCollectionChanged, Entry>();
 
         private CollectionChangedEventManager()
         {
@@ -34,17 +34,13 @@ namespace Avalonia.Controls.Utils
             listener = listener ?? throw new ArgumentNullException(nameof(listener));
             Dispatcher.UIThread.VerifyAccess();
 
-            if (!_entries.TryGetValue(collection, out var listeners))
+            if (!_entries.TryGetValue(collection, out var entry))
             {
-                listeners = new List<WeakReference<ICollectionChangedListener>>();
-                _entries.Add(collection, listeners);
-                WeakSubscriptionManager.Subscribe(
-                    collection,
-                    nameof(INotifyCollectionChanged.CollectionChanged),
-                    this);
+                entry = new Entry(collection);
+                _entries.Add(collection, entry);
             }
 
-            foreach (var l in listeners)
+            foreach (var l in entry.Listeners)
             {
                 if (l.TryGetTarget(out var target) && target == listener)
                 {
@@ -53,7 +49,7 @@ namespace Avalonia.Controls.Utils
                 }
             }
 
-            listeners.Add(new WeakReference<ICollectionChangedListener>(listener));
+            entry.Listeners.Add(new WeakReference<ICollectionChangedListener>(listener));
         }
 
         public void RemoveListener(INotifyCollectionChanged collection, ICollectionChangedListener listener)
@@ -62,8 +58,10 @@ namespace Avalonia.Controls.Utils
             listener = listener ?? throw new ArgumentNullException(nameof(listener));
             Dispatcher.UIThread.VerifyAccess();
 
-            if (_entries.TryGetValue(collection, out var listeners))
+            if (_entries.TryGetValue(collection, out var entry))
             {
+                var listeners = entry.Listeners;
+
                 for (var i = 0; i < listeners.Count; ++i)
                 {
                     if (listeners[i].TryGetTarget(out var target) && target == listener)
@@ -72,10 +70,7 @@ namespace Avalonia.Controls.Utils
 
                         if (listeners.Count == 0)
                         {
-                            WeakSubscriptionManager.Unsubscribe(
-                                collection,
-                                nameof(INotifyCollectionChanged.CollectionChanged),
-                                this);
+                            entry.Dispose();
                             _entries.Remove(collection);
                         }
 
@@ -88,51 +83,72 @@ namespace Avalonia.Controls.Utils
                 "Collection listener not registered for this collection/listener combination.");
         }
 
-        void IWeakSubscriber<NotifyCollectionChangedEventArgs>.OnEvent(object sender, NotifyCollectionChangedEventArgs e)
+        private class Entry : IWeakSubscriber<NotifyCollectionChangedEventArgs>, IDisposable
         {
-            static void Notify(
-                INotifyCollectionChanged incc,
-                NotifyCollectionChangedEventArgs args,
-                List<WeakReference<ICollectionChangedListener>> listeners)
+            private INotifyCollectionChanged _collection;
+
+            public Entry(INotifyCollectionChanged collection)
             {
-                foreach (var l in listeners)
+                _collection = collection;
+                Listeners = new List<WeakReference<ICollectionChangedListener>>();
+                WeakSubscriptionManager.Subscribe(
+                    _collection,
+                    nameof(INotifyCollectionChanged.CollectionChanged),
+                    this);
+            }
+
+            public List<WeakReference<ICollectionChangedListener>> Listeners { get; }
+
+            public void Dispose()
+            {
+                WeakSubscriptionManager.Unsubscribe(
+                    _collection,
+                    nameof(INotifyCollectionChanged.CollectionChanged),
+                    this);
+            }
+
+            void IWeakSubscriber<NotifyCollectionChangedEventArgs>.OnEvent(object sender, NotifyCollectionChangedEventArgs e)
+            {
+                static void Notify(
+                    INotifyCollectionChanged incc,
+                    NotifyCollectionChangedEventArgs args,
+                    List<WeakReference<ICollectionChangedListener>> listeners)
                 {
-                    if (l.TryGetTarget(out var target))
+                    foreach (var l in listeners)
                     {
-                        target.PreChanged(incc, args);
+                        if (l.TryGetTarget(out var target))
+                        {
+                            target.PreChanged(incc, args);
+                        }
                     }
-                }
 
-                foreach (var l in listeners)
-                {
-                    if (l.TryGetTarget(out var target))
+                    foreach (var l in listeners)
                     {
-                        target.Changed(incc, args);
+                        if (l.TryGetTarget(out var target))
+                        {
+                            target.Changed(incc, args);
+                        }
                     }
-                }
 
-                foreach (var l in listeners)
-                {
-                    if (l.TryGetTarget(out var target))
+                    foreach (var l in listeners)
                     {
-                        target.PostChanged(incc, args);
+                        if (l.TryGetTarget(out var target))
+                        {
+                            target.PostChanged(incc, args);
+                        }
                     }
                 }
-            }
 
-            if (sender is INotifyCollectionChanged incc && _entries.TryGetValue(incc, out var listeners))
-            {
-                var l = listeners.ToList();
+                var l = Listeners.ToList();
 
                 if (Dispatcher.UIThread.CheckAccess())
                 {
-                    Notify(incc, e, l);
+                    Notify(_collection, e, l);
                 }
                 else
                 {
-                    var inccCapture = incc;
                     var eCapture = e;
-                    Dispatcher.UIThread.Post(() => Notify(inccCapture, eCapture, l));
+                    Dispatcher.UIThread.Post(() => Notify(_collection, eCapture, l));
                 }
             }
         }

tests/Avalonia.Controls.UnitTests/Utils/CollectionChangedEventManagerTests.cs

@@ -0,0 +1,89 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.Specialized;
+using System.Text;
+using Avalonia.Collections;
+using Avalonia.Controls.Utils;
+using Xunit;
+using CollectionChangedEventManager = Avalonia.Controls.Utils.CollectionChangedEventManager;
+
+namespace Avalonia.Controls.UnitTests.Utils
+{
+    public class CollectionChangedEventManagerTests
+    {
+        [Fact]
+        public void AddListener_Listens_To_Events()
+        {
+            var source = new AvaloniaList<string>();
+            var listener = new Listener();
+
+            CollectionChangedEventManager.Instance.AddListener(source, listener);
+
+            Assert.Empty(listener.Received);
+
+            source.Add("foo");
+
+            Assert.Equal(1, listener.Received.Count);
+        }
+
+        [Fact]
+        public void RemoveListener_Stops_Listening_To_Events()
+        {
+            var source = new AvaloniaList<string>();
+            var listener = new Listener();
+
+            CollectionChangedEventManager.Instance.AddListener(source, listener);
+            CollectionChangedEventManager.Instance.RemoveListener(source, listener);
+
+            source.Add("foo");
+
+            Assert.Empty(listener.Received);
+        }
+
+        [Fact]
+        public void Receives_Events_From_Wrapped_Collection()
+        {
+            var source = new WrappingCollection();
+            var listener = new Listener();
+
+            CollectionChangedEventManager.Instance.AddListener(source, listener);
+
+            Assert.Empty(listener.Received);
+
+            source.Add("foo");
+
+            Assert.Equal(1, listener.Received.Count);
+        }
+
+        private class Listener : ICollectionChangedListener
+        {
+            public List<NotifyCollectionChangedEventArgs> Received { get; } = new List<NotifyCollectionChangedEventArgs>();
+
+            public void Changed(INotifyCollectionChanged sender, NotifyCollectionChangedEventArgs e)
+            {
+                Received.Add(e);
+            }
+
+            public void PostChanged(INotifyCollectionChanged sender, NotifyCollectionChangedEventArgs e)
+            {
+            }
+
+            public void PreChanged(INotifyCollectionChanged sender, NotifyCollectionChangedEventArgs e)
+            {
+            }
+        }
+
+        private class WrappingCollection : INotifyCollectionChanged
+        {
+            private AvaloniaList<string> _inner = new AvaloniaList<string>();
+
+            public void Add(string s) => _inner.Add(s);
+
+            public event NotifyCollectionChangedEventHandler CollectionChanged
+            {
+                add => _inner.CollectionChanged += value;
+                remove => _inner.CollectionChanged -= value;
+            }
+        }
+    }
+}