Browse Source

Add a more efficient way to listen to classes changes.

Improves the situation in e.g. #8389 drastically.
Steven Kirk 3 years ago
parent
commit
e63aa46458

+ 51 - 2
src/Avalonia.Base/Controls/Classes.cs

@@ -14,6 +14,8 @@ namespace Avalonia.Controls
     /// </remarks>
     public class Classes : AvaloniaList<string>, IPseudoClasses
     {
+        private List<IClassesChangedListener>? _listeners;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="Classes"/> class.
         /// </summary>
@@ -39,6 +41,11 @@ namespace Avalonia.Controls
         {            
         }
 
+        /// <summary>
+        /// Gets the number of listeners subscribed to this collection for unit testing purposes.
+        /// </summary>
+        internal int ListenerCount => _listeners?.Count ?? 0;
+
         /// <summary>
         /// Parses a classes string.
         /// </summary>
@@ -62,6 +69,7 @@ namespace Avalonia.Controls
             if (!Contains(name))
             {
                 base.Add(name);
+                NotifyChanged();
             }
         }
 
@@ -89,6 +97,7 @@ namespace Avalonia.Controls
             }
 
             base.AddRange(c);
+            NotifyChanged();
         }
 
         /// <summary>
@@ -103,6 +112,8 @@ namespace Avalonia.Controls
                     RemoveAt(i);
                 }
             }
+
+            NotifyChanged();
         }
 
         /// <summary>
@@ -122,6 +133,7 @@ namespace Avalonia.Controls
             if (!Contains(name))
             {
                 base.Insert(index, name);
+                NotifyChanged();
             }
         }
 
@@ -154,6 +166,7 @@ namespace Avalonia.Controls
             if (toInsert != null)
             {
                 base.InsertRange(index, toInsert);
+                NotifyChanged();
             }
         }
 
@@ -169,7 +182,14 @@ namespace Avalonia.Controls
         public override bool Remove(string name)
         {
             ThrowIfPseudoclass(name, "removed");
-            return base.Remove(name);
+
+            if (base.Remove(name))
+            {
+                NotifyChanged();
+                return true;
+            }
+
+            return false;
         }
 
         /// <summary>
@@ -197,6 +217,7 @@ namespace Avalonia.Controls
             if (toRemove != null)
             {
                 base.RemoveAll(toRemove);
+                NotifyChanged();
             }
         }
 
@@ -214,6 +235,7 @@ namespace Avalonia.Controls
             var name = this[index];
             ThrowIfPseudoclass(name, "removed");
             base.RemoveAt(index);
+            NotifyChanged();
         }
 
         /// <summary>
@@ -224,6 +246,7 @@ namespace Avalonia.Controls
         public override void RemoveRange(int index, int count)
         {
             base.RemoveRange(index, count);
+            NotifyChanged();
         }
 
         /// <summary>
@@ -255,6 +278,7 @@ namespace Avalonia.Controls
             }
 
             base.AddRange(source);
+            NotifyChanged();
         }
 
         /// <inheritdoc/>
@@ -263,13 +287,38 @@ namespace Avalonia.Controls
             if (!Contains(name))
             {
                 base.Add(name);
+                NotifyChanged();
             }
         }
 
         /// <inheritdoc/>
         bool IPseudoClasses.Remove(string name)
         {
-            return base.Remove(name);
+            if (base.Remove(name))
+            {
+                NotifyChanged();
+                return true;
+            }
+
+            return false;
+        }
+
+        internal void AddListener(IClassesChangedListener listener)
+        {
+            (_listeners ??= new()).Add(listener);
+        }
+
+        internal void RemoveListener(IClassesChangedListener listener)
+        {
+            _listeners?.Remove(listener);
+        }
+
+        private void NotifyChanged()
+        {
+            if (_listeners is null)
+                return;
+            foreach (var listener in _listeners)
+                listener.Changed();
         }
 
         private void ThrowIfPseudoclass(string name, string operation)

+ 14 - 0
src/Avalonia.Base/Controls/IClassesChangedListener.cs

@@ -0,0 +1,14 @@
+namespace Avalonia.Controls
+{
+    /// <summary>
+    /// Internal interface for listening to changes in <see cref="Classes"/> in a more
+    /// performant manner than subscribing to CollectionChanged.
+    /// </summary>
+    internal interface IClassesChangedListener
+    {
+        /// <summary>
+        /// Notifies the listener that the <see cref="Classes"/> collection has changed.
+        /// </summary>
+        void Changed();
+    }
+}

+ 10 - 16
src/Avalonia.Base/Styling/Activators/StyleClassActivator.cs

@@ -1,6 +1,7 @@
 using System.Collections.Generic;
 using System.Collections.Specialized;
 using Avalonia.Collections;
+using Avalonia.Controls;
 
 #nullable enable
 
@@ -10,21 +11,17 @@ namespace Avalonia.Styling.Activators
     /// An <see cref="IStyleActivator"/> which is active when a set of classes match those on a
     /// control.
     /// </summary>
-    internal sealed class StyleClassActivator : StyleActivatorBase
+    internal sealed class StyleClassActivator : StyleActivatorBase, IClassesChangedListener
     {
         private readonly IList<string> _match;
-        private readonly IAvaloniaReadOnlyList<string> _classes;
-        private NotifyCollectionChangedEventHandler? _classesChangedHandler;
+        private readonly Classes _classes;
 
-        public StyleClassActivator(IAvaloniaReadOnlyList<string> classes, IList<string> match)
+        public StyleClassActivator(Classes classes, IList<string> match)
         {
             _classes = classes;
             _match = match;
         }
 
-        private NotifyCollectionChangedEventHandler ClassesChangedHandler =>
-            _classesChangedHandler ??= ClassesChanged;
-
         public static bool AreClassesMatching(IReadOnlyList<string> classes, IList<string> toMatch)
         {
             int remainingMatches = toMatch.Count;
@@ -55,23 +52,20 @@ namespace Avalonia.Styling.Activators
             return remainingMatches == 0;
         }
 
-        protected override void Initialize()
+        void IClassesChangedListener.Changed()
         {
             PublishNext(IsMatching());
-            _classes.CollectionChanged += ClassesChangedHandler;
         }
 
-        protected override void Deinitialize()
+        protected override void Initialize()
         {
-            _classes.CollectionChanged -= ClassesChangedHandler;
+            PublishNext(IsMatching());
+            _classes.AddListener(this);
         }
 
-        private void ClassesChanged(object? sender, NotifyCollectionChangedEventArgs e)
+        protected override void Deinitialize()
         {
-            if (e.Action != NotifyCollectionChangedAction.Move)
-            {
-                PublishNext(IsMatching());
-            }
+            _classes.RemoveListener(this);
         }
 
         private bool IsMatching() => AreClassesMatching(_classes, _match);

+ 2 - 1
src/Avalonia.Base/Styling/TypeNameAndClassSelector.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Text;
+using Avalonia.Controls;
 using Avalonia.Styling.Activators;
 
 #nullable enable
@@ -125,7 +126,7 @@ namespace Avalonia.Styling
             {
                 if (subscribe)
                 {
-                    var observable = new StyleClassActivator(control.Classes, _classes.Value);
+                    var observable = new StyleClassActivator((Classes)control.Classes, _classes.Value);
 
                     return new SelectorMatch(observable);
                 }

+ 2 - 3
tests/Avalonia.Base.UnitTests/Styling/SelectorTests_Template.cs

@@ -142,14 +142,13 @@ namespace Avalonia.Base.UnitTests.Styling
             var border = (Border)target.Object.VisualChildren.Single();
             var selector = default(Selector).OfType(templatedControl.Object.GetType()).Class("foo").Template().OfType<Border>();
             var activator = selector.Match(border).Activator;
-            var inccDebug = (INotifyCollectionChangedDebug)styleable.Object.Classes;
 
             using (activator.Subscribe(_ => { }))
             {
-                Assert.Single(inccDebug.GetCollectionChangedSubscribers());
+                Assert.Equal(1, ((Classes)styleable.Object.Classes).ListenerCount);
             }
 
-            Assert.Null(inccDebug.GetCollectionChangedSubscribers());
+            Assert.Equal(0, ((Classes)styleable.Object.Classes).ListenerCount);
         }
 
         private void BuildVisualTree<T>(Mock<T> templatedControl) where T : class, IVisual

+ 1 - 1
tests/Avalonia.LeakTests/ControlTests.cs

@@ -313,7 +313,7 @@ namespace Avalonia.LeakTests
 
                 // The TextBox should have subscriptions to its Classes collection from the
                 // default theme.
-                Assert.NotEmpty(((INotifyCollectionChangedDebug)textBox.Classes).GetCollectionChangedSubscribers());
+                Assert.NotEqual(0, textBox.Classes.ListenerCount);
 
                 // Clear the content and ensure the TextBox is removed.
                 window.Content = null;