Refactor SelectedItemsSync.

src/Avalonia.Controls/Primitives/SelectingItemsControl.cs

@@ -167,8 +167,8 @@ namespace Avalonia.Controls.Primitives
         /// </summary>
         protected IList SelectedItems
         {
-            get => SelectedItemsSync.GetOrCreateSelectedItems();
-            set => SelectedItemsSync.SetSelectedItems(value);
+            get => SelectedItemsSync.SelectedItems;
+            set => SelectedItemsSync.SelectedItems = value;
         }
 
         /// <summary>

src/Avalonia.Controls/Utils/SelectedItemsSync.cs

@@ -13,114 +13,107 @@ namespace Avalonia.Controls.Utils
     /// <summary>
     /// Synchronizes an <see cref="ISelectionModel"/> with a list of SelectedItems.
     /// </summary>
-    internal class SelectedItemsSync
+    internal class SelectedItemsSync : IDisposable
     {
-        private IList? _selectedItems;
+        private ISelectionModel _selectionModel;
+        private IList _selectedItems;
         private bool _updatingItems;
         private bool _updatingModel;
-        private bool _initializeOnSourceAssignment;
 
         public SelectedItemsSync(ISelectionModel model)
         {
-            model = model ?? throw new ArgumentNullException(nameof(model));
-            Model = model;
+            _selectionModel = model ?? throw new ArgumentNullException(nameof(model));
+            _selectedItems = new AvaloniaList<object?>();
+            SyncSelectedItemsWithSelectionModel();
+            SubscribeToSelectedItems(_selectedItems);
+            SubscribeToSelectionModel(model);
         }
 
-        public ISelectionModel Model { get; private set; }
-
-        public IList GetOrCreateSelectedItems()
+        public ISelectionModel SelectionModel 
         {
-            if (_selectedItems == null)
+            get => _selectionModel;
+            set
             {
-                var items = new AvaloniaList<object?>(Model.SelectedItems);
-                items.CollectionChanged += ItemsCollectionChanged;
-                Model.SelectionChanged += SelectionModelSelectionChanged;
-                _selectedItems = items;
+                value = value ?? throw new ArgumentNullException(nameof(value));
+                UnsubscribeFromSelectionModel(_selectionModel);
+                _selectionModel = value;
+                SubscribeToSelectionModel(_selectionModel);
+                SyncSelectedItemsWithSelectionModel();
             }
-
-            return _selectedItems;
         }
-
-        public void SetSelectedItems(IList? items)
+        
+        public IList SelectedItems 
         {
-            items ??= new AvaloniaList<object>();
-
-            if (items.IsFixedSize)
+            get => _selectedItems;
+            set
             {
-                throw new NotSupportedException(
-                    "Cannot assign fixed size selection to SelectedItems.");
-            }
+                value ??= new AvaloniaList<object?>();
 
-            if (_selectedItems is INotifyCollectionChanged incc)
-            {
-                incc.CollectionChanged -= ItemsCollectionChanged;
+                if (value.IsFixedSize)
+                {
+                    throw new NotSupportedException(
+                        "Cannot assign fixed size selection to SelectedItems.");
+                }
+
+                UnsubscribeFromSelectedItems(_selectedItems);
+                _selectedItems = value;
+                SubscribeToSelectedItems(_selectedItems);
+                SyncSelectionModelWithSelectedItems();
             }
+        }
 
-            if (_selectedItems == null)
+        public void Dispose()
+        {
+            UnsubscribeFromSelectedItems(_selectedItems);
+            UnsubscribeFromSelectionModel(_selectionModel);
+        }
+
+        private void SyncSelectedItemsWithSelectionModel()
+        {
+            if (_selectionModel.Source is null)
             {
-                Model.SelectionChanged += SelectionModelSelectionChanged;
+                return;
             }
 
+            _updatingItems = true;
+
             try
             {
-                _updatingModel = true;
-                _selectedItems = items;
+                _selectedItems.Clear();
 
-                if (Model.Source is object)
-                {
-                    using (Model.BatchUpdate())
-                    {
-                        Model.Clear();
-                        Add(items);
-                    }
-                }
-                else if (!_initializeOnSourceAssignment)
+                foreach (var i in SelectionModel.SelectedItems)
                 {
-                    Model.PropertyChanged += SelectionModelPropertyChanged;
-                    _initializeOnSourceAssignment = true;
-                }
-
-                if (_selectedItems is INotifyCollectionChanged incc2)
-                {
-                    incc2.CollectionChanged += ItemsCollectionChanged;
+                    _selectedItems.Add(i);
                 }
             }
             finally
             {
-                _updatingModel = false;
+                _updatingItems = false;
             }
         }
 
-        public void SetModel(ISelectionModel model)
+        private void SyncSelectionModelWithSelectedItems()
         {
-            model = model ?? throw new ArgumentNullException(nameof(model));
+            _updatingModel = true;
 
-            if (_selectedItems != null)
+            try
             {
-                Model.PropertyChanged -= SelectionModelPropertyChanged;
-                Model.SelectionChanged -= SelectionModelSelectionChanged;
-                Model = model;
-                Model.SelectionChanged += SelectionModelSelectionChanged;
-                _initializeOnSourceAssignment = false;
-
-                try
+                if (_selectionModel.Source is object)
                 {
-                    _updatingItems = true;
-                    _selectedItems.Clear();
-
-                    foreach (var i in model.SelectedItems)
+                    using (_selectionModel.BatchUpdate())
                     {
-                        _selectedItems.Add(i);
+                        SelectionModel.Clear();
+                        Add(_selectedItems);
                     }
                 }
-                finally
-                {
-                    _updatingItems = false;
-                }
+            }
+            finally
+            {
+                _updatingModel = false;
             }
         }
 
-        private void ItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
+        private void SelectedItemsCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
         {
             if (_updatingItems)
             {
@@ -136,18 +129,18 @@ namespace Avalonia.Controls.Utils
             {
                 foreach (var i in e.OldItems)
                 {
-                    var index = IndexOf(Model.Source, i);
+                    var index = IndexOf(SelectionModel.Source, i);
 
                     if (index != -1)
                     {
-                        Model.Deselect(index);
+                        SelectionModel.Deselect(index);
                     }
                 }
             }
 
             try
             {
-                using var operation = Model.BatchUpdate();
+                using var operation = SelectionModel.BatchUpdate();
 
                 _updatingModel = true;
 
@@ -164,7 +157,7 @@ namespace Avalonia.Controls.Utils
                         Add(e.NewItems);
                         break;
                     case NotifyCollectionChangedAction.Reset:
-                        Model.Clear();
+                        SelectionModel.Clear();
                         Add(_selectedItems);
                         break;
                 }
@@ -179,46 +172,37 @@ namespace Avalonia.Controls.Utils
         {
             foreach (var i in newItems)
             {
-                var index = IndexOf(Model.Source, i);
+                var index = IndexOf(SelectionModel.Source, i);
 
                 if (index != -1)
                 {
-                    Model.Select(index);
+                    SelectionModel.Select(index);
                 }
             }
         }
 
         private void SelectionModelPropertyChanged(object sender, PropertyChangedEventArgs e)
         {
-            if (_initializeOnSourceAssignment &&
-                _selectedItems != null &&
-                e.PropertyName == nameof(ISelectionModel.Source))
+            if (e.PropertyName == nameof(ISelectionModel.Source))
             {
-                try
+                if (_selectedItems.Count > 0)
                 {
-                    _updatingModel = true;
-                    Add(_selectedItems);
-                    _initializeOnSourceAssignment = false;
+                    SyncSelectionModelWithSelectedItems();
                 }
-                finally
+                else
                 {
-                    _updatingModel = false;
+                    SyncSelectedItemsWithSelectionModel();
                 }
             }
         }
 
         private void SelectionModelSelectionChanged(object sender, SelectionModelSelectionChangedEventArgs e)
         {
-            if (_updatingModel)
+            if (_updatingModel || _selectionModel.Source is null)
             {
                 return;
             }
 
-            if (_selectedItems == null)
-            {
-                throw new AvaloniaInternalException("SelectionModelChanged raised but we don't have items.");
-            }
-
             try
             {
                 var deselected = e.DeselectedItems.ToList();
@@ -242,6 +226,34 @@ namespace Avalonia.Controls.Utils
             }
         }
 
+        private void SubscribeToSelectedItems(IList selectedItems)
+        {
+            if (selectedItems is INotifyCollectionChanged incc)
+            {
+                incc.CollectionChanged += SelectedItemsCollectionChanged;
+            }
+        }
+
+        private void SubscribeToSelectionModel(ISelectionModel model)
+        {
+            model.PropertyChanged += SelectionModelPropertyChanged;
+            model.SelectionChanged += SelectionModelSelectionChanged;
+        }
+
+        private void UnsubscribeFromSelectedItems(IList selectedItems)
+        {
+            if (selectedItems is INotifyCollectionChanged incc)
+            {
+                incc.CollectionChanged -= SelectedItemsCollectionChanged;
+            }
+        }
+
+        private void UnsubscribeFromSelectionModel(ISelectionModel model)
+        {
+            model.PropertyChanged -= SelectionModelPropertyChanged;
+            model.SelectionChanged -= SelectionModelSelectionChanged;
+        }
+
         private static int IndexOf(object? source, object? item)
         {
             if (source is IList l)

tests/Avalonia.Controls.UnitTests/ListBoxTests.cs

@@ -385,6 +385,7 @@ namespace Avalonia.Controls.UnitTests
 
             // First an item that is not index 0 must be selected.
             _mouse.Click(target.Presenter.Panel.Children[1]);
+            Assert.Equal(1, target.Selection.AnchorIndex);
 
             // We're going to be clicking on item 9.
             var item = (ListBoxItem)target.Presenter.Panel.Children[9];

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

@@ -0,0 +1,245 @@
+using System;
+using System.Collections.Generic;
+using Avalonia.Collections;
+using Avalonia.Controls.Selection;
+using Avalonia.Controls.Utils;
+using Xunit;
+
+namespace Avalonia.Controls.UnitTests.Utils
+{
+    public class SelectedItemsSyncTests
+    {
+        [Fact]
+        public void Initial_Items_Are_From_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            Assert.Equal(new[] { "bar", "baz" }, items);
+        }
+
+        [Fact]
+        public void Selecting_On_Model_Adds_Item()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            target.SelectionModel.Select(0);
+
+            Assert.Equal(new[] { "bar", "baz", "foo" }, items);
+        }
+
+        [Fact]
+        public void Selecting_Duplicate_On_Model_Adds_Item()
+        {
+            var target = CreateTarget(new[] { "foo", "bar", "baz", "foo", "bar", "baz" });
+            var items = target.SelectedItems;
+
+            target.SelectionModel.Select(4);
+
+            Assert.Equal(new[] { "bar", "baz", "bar" }, items);
+        }
+
+        [Fact]
+        public void Deselecting_On_Model_Removes_Item()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            target.SelectionModel.Deselect(1);
+
+            Assert.Equal(new[] { "baz" }, items);
+        }
+
+        [Fact]
+        public void Deselecting_Duplicate_On_Model_Removes_Item()
+        {
+            var target = CreateTarget(new[] { "foo", "bar", "baz", "foo", "bar", "baz" });
+            var items = target.SelectedItems;
+
+            target.SelectionModel.Select(4);
+            target.SelectionModel.Deselect(4);
+
+            Assert.Equal(new[] { "baz", "bar" }, items);
+        }
+
+        [Fact]
+        public void Reassigning_Model_Resets_Items()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            var newModel = new SelectionModel<string> 
+            { 
+                Source = (string[])target.SelectionModel.Source,
+                SingleSelect = false 
+            };
+
+            newModel.Select(0);
+            newModel.Select(1);
+
+            target.SelectionModel = newModel;
+
+            Assert.Equal(new[] { "foo", "bar" }, items);
+        }
+
+        [Fact]
+        public void Reassigning_Model_Tracks_New_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            var newModel = new SelectionModel<string>
+            {
+                Source = (string[])target.SelectionModel.Source,
+                SingleSelect = false
+            };
+
+            target.SelectionModel = newModel;
+
+            newModel.Select(0);
+            newModel.Select(1);
+
+            Assert.Equal(new[] { "foo", "bar" }, items);
+        }
+
+        [Fact]
+        public void Adding_To_Items_Selects_On_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            items.Add("foo");
+
+            Assert.Equal(new[] { 0, 1, 2 }, target.SelectionModel.SelectedIndexes);
+            Assert.Equal(new[] { "bar", "baz", "foo" }, items);
+        }
+
+        [Fact]
+        public void Removing_From_Items_Deselects_On_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            items.Remove("baz");
+
+            Assert.Equal(new[] { 1 }, target.SelectionModel.SelectedIndexes);
+            Assert.Equal(new[] { "bar" }, items);
+        }
+
+        [Fact]
+        public void Replacing_Item_Updates_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            items[0] = "foo";
+
+            Assert.Equal(new[] { 0, 2 }, target.SelectionModel.SelectedIndexes);
+            Assert.Equal(new[] { "foo", "baz" }, items);
+        }
+
+        [Fact]
+        public void Clearing_Items_Updates_Model()
+        {
+            var target = CreateTarget();
+            var items = target.SelectedItems;
+
+            items.Clear();
+
+            Assert.Empty(target.SelectionModel.SelectedIndexes);
+        }
+
+        [Fact]
+        public void Setting_Items_Updates_Model()
+        {
+            var target = CreateTarget();
+            var oldItems = target.SelectedItems;
+
+            var newItems = new AvaloniaList<string> { "foo", "baz" };
+            target.SelectedItems = newItems;
+
+            Assert.Equal(new[] { 0, 2 }, target.SelectionModel.SelectedIndexes);
+            Assert.Same(newItems, target.SelectedItems);
+            Assert.NotSame(oldItems, target.SelectedItems);
+            Assert.Equal(new[] { "foo", "baz" }, newItems);
+        }
+
+        [Fact]
+        public void Setting_Items_Subscribes_To_Model()
+        {
+            var target = CreateTarget();
+            var items = new AvaloniaList<string> { "foo", "baz" };
+
+            target.SelectedItems = items;
+            target.SelectionModel.Select(1);
+
+            Assert.Equal(new[] { "foo", "baz", "bar" }, items);
+        }
+
+        [Fact]
+        public void Setting_Items_To_Null_Creates_Empty_Items()
+        {
+            var target = CreateTarget();
+            var oldItems = target.SelectedItems;
+
+            target.SelectedItems = null;
+
+            var newItems = Assert.IsType<AvaloniaList<object>>(target.SelectedItems);
+
+            Assert.NotSame(oldItems, newItems);
+        }
+
+        [Fact]
+        public void Handles_Null_Model_Source()
+        {
+            var model = new SelectionModel<string> { SingleSelect = false };
+            model.Select(1);
+
+            var target = new SelectedItemsSync(model);
+            var items = target.SelectedItems;
+
+            Assert.Empty(items);
+
+            model.Select(2);
+            model.Source = new[] { "foo", "bar", "baz" };
+
+            Assert.Equal(new[] { "bar", "baz" }, items);
+        }
+
+        [Fact]
+        public void Does_Not_Accept_Fixed_Size_Items()
+        {
+            var target = CreateTarget();
+
+            Assert.Throws<NotSupportedException>(() =>
+                target.SelectedItems = new[] { "foo", "bar", "baz" });
+        }
+
+        [Fact]
+        public void Selected_Items_Can_Be_Set_Before_SelectionModel_Source()
+        {
+            var model = new SelectionModel<string>();
+            var target = new SelectedItemsSync(model);
+            var items = new AvaloniaList<string> { "foo", "bar", "baz" };
+            var selectedItems = new AvaloniaList<string> { "bar" };
+
+            target.SelectedItems = selectedItems;
+            model.Source = items;
+
+            Assert.Equal(1, model.SelectedIndex);
+        }
+
+        private static SelectedItemsSync CreateTarget(
+            IEnumerable<string> items = null)
+        {
+            items ??= new[] { "foo", "bar", "baz" };
+
+            var model = new SelectionModel<string> { Source = items, SingleSelect = false };
+            model.SelectRange(1, 2);
+
+            var target = new SelectedItemsSync(model);
+            return target;
+        }
+    }
+}