diff --git a/src/Avalonia.Base/StyledElement.cs b/src/Avalonia.Base/StyledElement.cs index f511d1de2a..0d449fcbf5 100644 --- a/src/Avalonia.Base/StyledElement.cs +++ b/src/Avalonia.Base/StyledElement.cs @@ -14,6 +14,7 @@ using Avalonia.Logging; using Avalonia.LogicalTree; using Avalonia.PropertyStore; using Avalonia.Styling; +using Avalonia.Utilities; namespace Avalonia { @@ -81,7 +82,7 @@ namespace Avalonia private string? _name; private Classes? _classes; private ILogicalRoot? _logicalRoot; - private AvaloniaList? _logicalChildren; + private SafeEnumerableList? _logicalChildren; private IResourceDictionary? _resources; private Styles? _styles; private bool _stylesApplied; @@ -267,7 +268,7 @@ namespace Avalonia { if (_logicalChildren == null) { - var list = new AvaloniaList + var list = new SafeEnumerableList { ResetBehavior = ResetBehavior.Remove, Validator = this @@ -712,12 +713,10 @@ namespace Avalonia element._dataContextUpdating = true; element.OnDataContextBeginUpdate(); - var logicalChildren = element.LogicalChildren; - var logicalChildrenCount = logicalChildren.Count; - for (var i = 0; i < logicalChildrenCount; i++) + foreach (var child in element.LogicalChildren) { - if (element.LogicalChildren[i] is StyledElement s && + if (child is StyledElement s && s.InheritanceParent == element && !s.IsSet(DataContextProperty)) { @@ -871,12 +870,10 @@ namespace Avalonia AttachedToLogicalTree?.Invoke(this, e); } - var logicalChildren = LogicalChildren; - var logicalChildrenCount = logicalChildren.Count; - for (var i = 0; i < logicalChildrenCount; i++) + foreach (var s in LogicalChildren) { - if (logicalChildren[i] is StyledElement child && child._logicalRoot != e.Root) // child may already have been attached within an event handler + if (s is StyledElement child && child._logicalRoot != e.Root) // child may already have been attached within an event handler { child.OnAttachedToLogicalTreeCore(e); } @@ -892,12 +889,10 @@ namespace Avalonia OnDetachedFromLogicalTree(e); DetachedFromLogicalTree?.Invoke(this, e); - var logicalChildren = LogicalChildren; - var logicalChildrenCount = logicalChildren.Count; - for (var i = 0; i < logicalChildrenCount; i++) + foreach (var s in LogicalChildren) { - if (logicalChildren[i] is StyledElement child) + if (s is StyledElement child) { child.OnDetachedFromLogicalTreeCore(e); } diff --git a/src/Avalonia.Base/Utilities/SafeEnumerableList.cs b/src/Avalonia.Base/Utilities/SafeEnumerableList.cs new file mode 100644 index 0000000000..4e0e7ddf9a --- /dev/null +++ b/src/Avalonia.Base/Utilities/SafeEnumerableList.cs @@ -0,0 +1,218 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; +using System.Linq; +using System.Runtime.CompilerServices; +using System.Text; +using System.Threading.Tasks; +using Avalonia.Collections; + +namespace Avalonia.Utilities +{ + internal class SafeEnumerableList : IAvaloniaList, IList, INotifyCollectionChanged + { + private AvaloniaList _list = new(); + private int _generation; + private int _enumCount = 0; + + public event NotifyCollectionChangedEventHandler? CollectionChanged; + public event PropertyChangedEventHandler? PropertyChanged; + + //for test purposes + internal AvaloniaList Inner => _list; + public SafeEnumerableList() + { + _list.CollectionChanged += List_CollectionChanged; + _list.PropertyChanged += List_PropertyChanged; + } + + private void List_PropertyChanged(object? sender, PropertyChangedEventArgs e) + { + PropertyChanged?.Invoke(this, e); + } + + private void List_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) + { + CollectionChanged?.Invoke(this, e); + } + + public SafeListEnumerator GetEnumerator() => new SafeListEnumerator(this, _list); + + public int IndexOf(T item) + { + return _list.IndexOf(item); + } + + public void Insert(int index, T item) + { + GetList().Insert(index, item); + } + + public void RemoveAt(int index) + { + GetList().RemoveAt(index); + } + + public void Add(T item) + { + GetList().Add(item); + } + + public void Clear() + { + GetList().Clear(); + } + + public bool Contains(T item) + { + return _list.Contains(item); + } + + public void CopyTo(T[] array, int arrayIndex) + { + _list.CopyTo(array, arrayIndex); + } + + public bool Remove(T item) + { + return GetList().Remove(item); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + private AvaloniaList GetList() + { + if (_enumCount > 0) + { + _list.CollectionChanged -= List_CollectionChanged; + _list = new(_list) + { + Validator = Validator, + ResetBehavior = ResetBehavior, + }; + _list.CollectionChanged += List_CollectionChanged; + ++_generation; + _enumCount = 0; + } + return _list; + } + + public void AddRange(IEnumerable items) + { + GetList().AddRange(items); + } + + public void InsertRange(int index, IEnumerable items) + { + GetList().InsertRange(index, items); + } + + public void Move(int oldIndex, int newIndex) + { + GetList().Move(oldIndex, newIndex); + } + + public void MoveRange(int oldIndex, int count, int newIndex) + { + GetList().MoveRange(oldIndex, count, newIndex); + } + + public void RemoveAll(IEnumerable items) + { + GetList().RemoveAll(items); + } + + public void RemoveRange(int index, int count) + { + GetList().RemoveRange(index, count); + } + + public int Add(object? value) + { + return ((IList)GetList()).Add(value); + } + + public bool Contains(object? value) + { + return ((IList)_list).Contains(value); + } + + public int IndexOf(object? value) + { + return ((IList)_list).IndexOf(value); + } + + public void Insert(int index, object? value) + { + ((IList)GetList()).Insert(index, value); + } + + public void Remove(object? value) + { + ((IList)GetList()).Remove(value); + } + + public void CopyTo(Array array, int index) + { + ((ICollection)_list).CopyTo(array, index); + } + + public int Count => _list.Count; + + public bool IsReadOnly => ((ICollection)_list).IsReadOnly; + + public ResetBehavior ResetBehavior { get => _list.ResetBehavior; set => _list.ResetBehavior = value; } + public IAvaloniaListItemValidator? Validator { get => _list.Validator; set => _list.Validator = value; } + + public bool IsFixedSize => ((IList)_list).IsFixedSize; + + public bool IsSynchronized => ((ICollection)_list).IsSynchronized; + + public object SyncRoot => ((ICollection)_list).SyncRoot; + + object? IList.this[int index] { get => ((IList)_list)[index]; set => ((IList)GetList())[index] = value; } + public T this[int index] { get => _list[index]; set => GetList()[index] = value; } + + public struct SafeListEnumerator : IEnumerator + { + private readonly SafeEnumerableList _owner; + private readonly int _generation; + private readonly IEnumerator _enumerator; + + public SafeListEnumerator(SafeEnumerableList owner, AvaloniaList list) + { + _owner = owner; + _generation = owner._generation; + ++owner._enumCount; + _enumerator = list.GetEnumerator(); + } + + public bool MoveNext() + => _enumerator.MoveNext(); + + public T Current => _enumerator.Current; + + object IEnumerator.Current => _enumerator.Current!; + + public void Reset() => throw new InvalidOperationException(); + + public void Dispose() + { + if (_generation == _owner._generation) + { + --_owner._enumCount; + } + } + } + } +} diff --git a/src/Avalonia.Base/Visual.Composition.cs b/src/Avalonia.Base/Visual.Composition.cs index be2d42782d..cafbc2d44e 100644 --- a/src/Avalonia.Base/Visual.Composition.cs +++ b/src/Avalonia.Base/Visual.Composition.cs @@ -3,7 +3,6 @@ using Avalonia.Collections.Pooled; using Avalonia.Media; using Avalonia.Rendering.Composition; using Avalonia.Rendering.Composition.Server; -using Avalonia.VisualTree; namespace Avalonia; @@ -45,7 +44,7 @@ public partial class Visual if(CompositionVisual == null) return; var compositionChildren = CompositionVisual.Children; - var visualChildren = (AvaloniaList)VisualChildren; + var visualChildren = VisualChildren; PooledList<(Visual visual, int index)>? sortedChildren = null; if (HasNonUniformZIndexChildren && visualChildren.Count > 1) diff --git a/src/Avalonia.Base/Visual.cs b/src/Avalonia.Base/Visual.cs index 4bd359fa78..5d720d90da 100644 --- a/src/Avalonia.Base/Visual.cs +++ b/src/Avalonia.Base/Visual.cs @@ -2,6 +2,7 @@ using System; using System.Collections; +using System.Collections.Generic; using System.Collections.Specialized; using Avalonia.Collections; using Avalonia.Data; @@ -159,7 +160,7 @@ namespace Avalonia // Disable transitions until we're added to the visual tree. DisableTransitions(); - var visualChildren = new AvaloniaList(); + var visualChildren = new SafeEnumerableList(); visualChildren.ResetBehavior = ResetBehavior.Remove; visualChildren.Validator = this; visualChildren.CollectionChanged += VisualChildrenChanged; @@ -561,12 +562,10 @@ namespace Avalonia _visualParent.HasNonUniformZIndexChildren = true; } - var visualChildren = VisualChildren; - var visualChildrenCount = visualChildren.Count; - for (var i = 0; i < visualChildrenCount; i++) + foreach (var child in VisualChildren) { - if (visualChildren[i] is { } child && child.PresentationSource != e.PresentationSource) // child may already have been attached within an event handler + if (child is { } && child.PresentationSource != e.PresentationSource) // child may already have been attached within an event handler { child.OnAttachedToVisualTreeCore(e); } @@ -599,12 +598,10 @@ namespace Avalonia PresentationSource = null; - var visualChildren = VisualChildren; - var visualChildrenCount = visualChildren.Count; - for (var i = 0; i < visualChildrenCount; i++) + foreach (var child in VisualChildren) { - if (visualChildren[i] is { } child) + if (child is { }) { child.OnDetachedFromVisualTreeCore(e); } @@ -767,7 +764,7 @@ namespace Avalonia for (var i = 0; i < count; i++) { - var visual = (Visual) children[i]!; + var visual = (Visual)children[i]!; visual.SetVisualParent(parent); } @@ -777,12 +774,11 @@ namespace Avalonia { base.OnTemplatedParentControlThemeChanged(); - var count = VisualChildren.Count; var templatedParent = TemplatedParent; - for (var i = 0; i < count; ++i) + foreach (var child in VisualChildren) { - if (VisualChildren[i] is StyledElement child && + if (child is not null && child.TemplatedParent == templatedParent) { child.OnTemplatedParentControlThemeChanged(); diff --git a/src/Avalonia.Controls/Primitives/TemplatedControl.cs b/src/Avalonia.Controls/Primitives/TemplatedControl.cs index d8ecfa99e8..397c27cb98 100644 --- a/src/Avalonia.Controls/Primitives/TemplatedControl.cs +++ b/src/Avalonia.Controls/Primitives/TemplatedControl.cs @@ -426,12 +426,9 @@ namespace Avalonia.Controls.Primitives { control.TemplatedParent = templatedParent; - var children = control.LogicalChildren; - var count = children.Count; - - for (var i = 0; i < count; i++) + foreach (var s in control.LogicalChildren) { - if (children[i] is StyledElement child && child.TemplatedParent is null) + if (s is StyledElement child && child.TemplatedParent is null) { ApplyTemplatedParent(child, templatedParent); } diff --git a/tests/Avalonia.Base.UnitTests/Collections/SafeEnumerateCollectionTests.cs b/tests/Avalonia.Base.UnitTests/Collections/SafeEnumerateCollectionTests.cs new file mode 100644 index 0000000000..e74b1d0bed --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Collections/SafeEnumerateCollectionTests.cs @@ -0,0 +1,93 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Linq.Expressions; +using System.Text; +using System.Threading.Tasks; +using Avalonia.Controls; +using Avalonia.Controls.Templates; +using Avalonia.Data; +using Avalonia.Layout; +using Avalonia.Media; +using Avalonia.Styling; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Base.UnitTests.Collections +{ + public class SafeEnumerateCollectionTests + { + private class ViewModel1 + { + public string FirstName { get; set; } = ""; + public string LastName { get; set; } = ""; + } + + private class ViewModel2 + { + public string CarProducer { get; set; } = ""; + public string CarModel { get; set; } = ""; + } + + [Fact] + public void NoExceptionWhenDetachingTabControlWithTemplate() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + TabControl tabControl; + IDataTemplate contentTemplate = new FuncDataTemplate( + (i, ns) => + { + return + new Border() + { + BorderBrush = Brushes.Red, + BorderThickness = new Thickness(4), + Padding = new Thickness(3), + Child = new ContentControl + { + Content = i, + } + }; + }); + + var window = new Window + { + Content = tabControl = new TabControl + { + ItemsSource = new object[] + { + new ViewModel1 + { + FirstName = "Vasily", + LastName = "Pupkin", + }, + new ViewModel2 + { + CarProducer = "Fiat", + CarModel = "Uno", + }, + }, + SelectedIndex = 0, + }, + Styles= + { + new Style(x => x.Is()) + { + Setters = + { + new Setter{ Property = TabItem.ContentTemplateProperty, + Value = contentTemplate, + } + } + } + }, + Width = 640, + Height = 480, + }; + window.Show(); + window.Close(); + } + } + } +} diff --git a/tests/Avalonia.Controls.UnitTests/Utils/SafeEnumerableListTests.cs b/tests/Avalonia.Controls.UnitTests/Utils/SafeEnumerableListTests.cs new file mode 100644 index 0000000000..7ac9f0006a --- /dev/null +++ b/tests/Avalonia.Controls.UnitTests/Utils/SafeEnumerableListTests.cs @@ -0,0 +1,131 @@ +using System.Collections.Generic; +using Avalonia.UnitTests; +using Avalonia.Utilities; +using Xunit; + +namespace Avalonia.Controls.UnitTests.Utils +{ + public class SafeEnumerableListTests : ScopedTestBase + { + [Fact] + public void List_Is_Not_Copied_Outside_Enumeration() + { + var target = new SafeEnumerableList(); + var inner = target.Inner; + + target.Add("foo"); + target.Add("bar"); + target.Remove("foo"); + + Assert.Same(inner, target.Inner); + } + + [Fact] + public void List_Is_Copied_Outside_Enumeration() + { + var target = new SafeEnumerableList(); + var inner = target.Inner; + + target.Add("foo"); + + foreach (var i in target) + { + Assert.Same(inner, target.Inner); + target.Add("bar"); + Assert.NotSame(inner, target.Inner); + Assert.Equal("foo", i); + } + + inner = target.Inner; + + foreach (var i in target) + { + target.Add("baz"); + Assert.NotSame(inner, target.Inner); + } + + Assert.Equal(new List { "foo", "bar", "baz", "baz" }, target); + } + + [Fact] + public void List_Is_Not_Copied_After_Enumeration() + { + var target = new SafeEnumerableList(); + var inner = target.Inner; + + target.Add("foo"); + + foreach (var i in target) + { + target.Add("bar"); + Assert.NotSame(inner, target.Inner); + inner = target.Inner; + Assert.Equal("foo", i); + } + + target.Add("baz"); + Assert.Same(inner, target.Inner); + } + + [Fact] + public void List_Is_Copied_Only_Once_During_Enumeration() + { + var target = new SafeEnumerableList(); + var inner = target.Inner; + + target.Add("foo"); + + foreach (var i in target) + { + target.Add("bar"); + Assert.NotSame(inner, target.Inner); + inner = target.Inner; + target.Add("baz"); + Assert.Same(inner, target.Inner); + } + + target.Add("baz"); + } + + [Fact] + public void List_Is_Copied_During_Nested_Enumerations() + { + var target = new SafeEnumerableList(); + var initialInner = target.Inner; + var firstItems = new List(); + var secondItems = new List(); + Collections.AvaloniaList firstInner; + Collections.AvaloniaList secondInner; + + target.Add("foo"); + + foreach (var i in target) + { + target.Add("bar"); + + firstInner = target.Inner; + Assert.NotSame(initialInner, firstInner); + + foreach (var j in target) + { + target.Add("baz"); + + secondInner = target.Inner; + Assert.NotSame(firstInner, secondInner); + + secondItems.Add(j); + } + + firstItems.Add(i); + } + + Assert.Equal(new List { "foo" }, firstItems); + Assert.Equal(new List { "foo", "bar" }, secondItems); + Assert.Equal(new List { "foo", "bar", "baz", "baz" }, target); + + var finalInner = target.Inner; + target.Add("final"); + Assert.Same(finalInner, target.Inner); + } + } +}