fix(grid): register assigned definition collections with their shared size group (#21848)

* test(grid): reproduce shared size groups ignoring assigned definitions

Definitions supplied through the ColumnDefinitions or RowDefinitions
setter are already in the collection when the grid claims it, so they
never pass through the collection-changed handler that joins them to the
parent tree. They never register with their shared size group, and the
definitions they replace never unregister from it.

* fix(grid): join assigned definition collections to the parent tree

DefinitionList.SetParent assigned each definition's Parent but never
called OnEnterParentTree, which only ran from the collection-changed
handler. Assigning Parent is not sufficient: OnEnterParentTree also sets
InheritanceParent, and a definition cannot read the inherited
PrivateSharedSizeScope that registers it with its group until that link
exists. Definitions supplied through the ColumnDefinitions setter - an
object initializer, a shared resource, or ColumnDefinitions="Auto,*" -
were therefore silently absent from their shared size group.

Enter and exit the parent tree from SetParent, and release the outgoing
collection when Grid swaps one in. Without that release the replaced
definitions stay registered with the group; nothing resets their measured
minimum any more, so they pin it at whatever they last contributed.

* test(grid): cover the definition ownership contract

Removing a definition leaves it holding its old Parent and its property
inheritance link, so it still reads the grid's shared size scope and can
re-register itself into a scope it has left. Also covers moving a
definition between grids, reassigning the same collection, and row
definitions, which the assignment fix reached but nothing exercised.

* refactor(grid): centralise definition parent-tree transitions

Definition ownership was implemented twice, and the two paths disagreed:
SetParent exited a definition and cleared its Parent, while removing one
from the collection called OnExitParentTree but left Parent set. Detach
was incomplete either way, since OnEnterParentTree establishes
InheritanceParent but OnExitParentTree never cleared it - so a removed
definition kept reading the grid's inherited PrivateSharedSizeScope, and
the grid kept it alive as an inheritance child.

Route every owner change through one transition that exits the old tree,
assigns Parent, and enters the new one, and clear InheritanceParent on
exit so detach mirrors attach.

---------

Co-authored-by: Julien Lebosquain <julien@lebosquain.net>

Author: NathanDrake2406

src/Avalonia.Controls/DefinitionBase.cs

@@ -60,6 +60,10 @@ internal void OnExitParentTree()
             }
 
             Parent?.InvalidateMeasure();
+
+            //  while this link survives the definition still reads the grid's inherited
+            //  PrivateSharedSizeScope, and the grid holds it as an inheritance child.
+            InheritanceParent = null;
         }
 
         /// <summary>

src/Avalonia.Controls/DefinitionList.cs

@@ -25,17 +25,53 @@ internal Grid? Parent
 
         private void SetParent(Grid? value)
         {
+            if (_parent == value)
+            {
+                return;
+            }
+
             _parent = value;
 
+            //  definitions already present when the grid claims the collection never pass through
+            //  OnCollectionChanged, so they have to change trees here.
             var idx = 0;
 
             foreach (T definition in this)
             {
-                definition.Parent = value;
+                SetDefinitionParent(definition, value);
                 definition.Index = idx++;
             }
         }
 
+        /// <summary>
+        /// Moves a definition from its current parent tree to <paramref name="parent"/>. Every route
+        /// that changes a definition's owner goes through here.
+        /// </summary>
+        /// <remarks>
+        /// Ownership is more than the Parent pointer: entering a tree also establishes the property
+        /// inheritance link a definition needs to see its shared size scope, and leaving one releases
+        /// that link and its shared size registration.
+        /// </remarks>
+        private static void SetDefinitionParent(DefinitionBase definition, Grid? parent)
+        {
+            if (definition.Parent == parent)
+            {
+                return;
+            }
+
+            if (definition.Parent is not null)
+            {
+                definition.OnExitParentTree();
+            }
+
+            definition.Parent = parent;
+
+            if (parent is not null)
+            {
+                definition.OnEnterParentTree();
+            }
+        }
+
         internal void OnCollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
         {
             var idx = 0;
@@ -62,17 +98,7 @@ private void UpdateDefinitionParent(IList? items, bool wasRemoved)
 
             for (var i = 0; i < count; i++)
             {
-                var definition = (DefinitionBase) items[i]!;
-
-                if (wasRemoved)
-                {
-                    definition.OnExitParentTree();
-                }
-                else
-                {
-                    definition.Parent = Parent;
-                    definition.OnEnterParentTree();                    
-                }
+                SetDefinitionParent((DefinitionBase)items[i]!, wasRemoved ? null : Parent);
             }
         }
     }

src/Avalonia.Controls/Grid.cs

@@ -198,6 +198,12 @@ public ColumnDefinitions ColumnDefinitions
             set
             {
                 if (_extData == null) { _extData = new ExtendedData(); }
+                //  otherwise the outgoing definitions stay registered with their shared size
+                //  group and keep contributing to its minimum.
+                if (_extData.ColumnDefinitions is { } oldDefinitions && !ReferenceEquals(oldDefinitions, value))
+                {
+                    oldDefinitions.Parent = null;
+                }
                 _extData.ColumnDefinitions = value;
                 _extData.ColumnDefinitions.Parent = this;
                 InvalidateMeasure();
@@ -220,6 +226,12 @@ public RowDefinitions RowDefinitions
             set
             {
                 if (_extData == null) { _extData = new ExtendedData(); }
+                //  otherwise the outgoing definitions stay registered with their shared size
+                //  group and keep contributing to its minimum.
+                if (_extData.RowDefinitions is { } oldDefinitions && !ReferenceEquals(oldDefinitions, value))
+                {
+                    oldDefinitions.Parent = null;
+                }
                 _extData.RowDefinitions = value;
                 _extData.RowDefinitions.Parent = this;
                 InvalidateMeasure();

tests/Avalonia.Controls.UnitTests/GridTests.cs

@@ -1309,6 +1309,244 @@ void ExecuteSharedSizeLayoutPass()
             Assert.Equal(10, plainGrid.ColumnDefinitions[0].ActualWidth);
         }
 
+        [Fact]
+        public void Shared_Size_Group_Is_Registered_For_Definitions_Assigned_As_A_Collection()
+        {
+            // Definitions supplied through the ColumnDefinitions setter - an object initializer, a
+            // shared resource, or ColumnDefinitions="Auto,*" - are already in the collection when the
+            // grid claims it, so they never pass through the collection-changed handler that joins
+            // them to the parent tree.
+            var grids = new[]
+            {
+                new Grid
+                {
+                    ColumnDefinitions = new ColumnDefinitions
+                    {
+                        new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" },
+                        new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) },
+                    },
+                },
+                new Grid
+                {
+                    ColumnDefinitions = new ColumnDefinitions
+                    {
+                        new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" },
+                        new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) },
+                    },
+                },
+            };
+            grids[0].Children.Add(new Border { Width = 50, Height = 10 });
+
+            var scope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { grids[0], grids[1] },
+            };
+            var root = new TestRoot(scope);
+
+            root.ExecuteInitialLayoutPass();
+            // Shared groups validate after layout and apply any resulting invalidation on the next pass.
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Equal(50, grids[0].ColumnDefinitions[0].ActualWidth);
+            Assert.Equal(50, grids[1].ColumnDefinitions[0].ActualWidth);
+        }
+
+        [Fact]
+        public void Replacing_Definition_Collection_Releases_Its_Shared_Size_Group()
+        {
+            // The outgoing definitions are no longer reachable from the grid, so nothing resets their
+            // measured minimum. Left registered, they keep the group pinned at whatever size they
+            // last contributed.
+            var grid = new Grid();
+            grid.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+            grid.Children.Add(new Border { Width = 50, Height = 10 });
+
+            var other = new Grid();
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+
+            var scope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { grid, other },
+            };
+            var root = new TestRoot(scope);
+
+            root.ExecuteInitialLayoutPass();
+            // Shared groups validate after layout and apply any resulting invalidation on the next pass.
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth);
+
+            grid.ColumnDefinitions = new ColumnDefinitions
+            {
+                new ColumnDefinition { Width = GridLength.Auto },
+                new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) },
+            };
+            root.LayoutManager.ExecuteLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth);
+        }
+
+        [Fact]
+        public void Removing_Definition_Detaches_It_From_The_Grid()
+        {
+            var shared = new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" };
+            var grid = new Grid();
+            grid.ColumnDefinitions.Add(shared);
+            grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+            grid.Children.Add(new Border { Width = 50, Height = 10 });
+
+            var other = new Grid();
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+
+            var scope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { grid, other },
+            };
+            var root = new TestRoot(scope);
+
+            root.ExecuteInitialLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth);
+
+            grid.ColumnDefinitions.Remove(shared);
+            root.LayoutManager.ExecuteLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth);
+            Assert.Null(shared.Parent);
+
+            // A definition that has left the grid must no longer see the grid's scope.
+            shared.SharedSizeGroup = null;
+            shared.SharedSizeGroup = "A";
+            root.LayoutManager.ExecuteLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(0, other.ColumnDefinitions[0].ActualWidth);
+        }
+
+        [Fact]
+        public void Moving_Definition_Between_Grids_Moves_Its_Shared_Size_Registration()
+        {
+            var shared = new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" };
+            var source = new Grid();
+            source.ColumnDefinitions.Add(shared);
+            source.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+            source.Children.Add(new Border { Width = 50, Height = 10 });
+
+            var sourcePartner = new Grid();
+            sourcePartner.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            sourcePartner.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+
+            var target = new Grid();
+            target.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+
+            var targetPartner = new Grid();
+            targetPartner.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            targetPartner.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+            targetPartner.Children.Add(new Border { Width = 20, Height = 10 });
+
+            var sourceScope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { source, sourcePartner },
+            };
+            var targetScope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { target, targetPartner },
+            };
+            var root = new TestRoot(new StackPanel { Children = { sourceScope, targetScope } });
+
+            root.ExecuteInitialLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(50, sourcePartner.ColumnDefinitions[0].ActualWidth);
+            Assert.Equal(20, targetPartner.ColumnDefinitions[0].ActualWidth);
+
+            source.ColumnDefinitions.Remove(shared);
+            target.ColumnDefinitions.Insert(0, shared);
+            root.LayoutManager.ExecuteLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Same(target, shared.Parent);
+            Assert.Equal(0, sourcePartner.ColumnDefinitions[0].ActualWidth);
+            Assert.Equal(20, target.ColumnDefinitions[0].ActualWidth);
+        }
+
+        [Fact]
+        public void Reassigning_The_Same_Definition_Collection_Is_Inert()
+        {
+            var grid = new Grid();
+            grid.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            grid.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+            grid.Children.Add(new Border { Width = 50, Height = 10 });
+
+            var other = new Grid();
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = GridLength.Auto, SharedSizeGroup = "A" });
+            other.ColumnDefinitions.Add(new ColumnDefinition { Width = new GridLength(1, GridUnitType.Star) });
+
+            var scope = new StackPanel
+            {
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { grid, other },
+            };
+            var root = new TestRoot(scope);
+
+            root.ExecuteInitialLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+            Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth);
+
+            var definitions = grid.ColumnDefinitions;
+            grid.ColumnDefinitions = definitions;
+            root.LayoutManager.ExecuteLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Same(definitions, grid.ColumnDefinitions);
+            Assert.Equal(50, other.ColumnDefinitions[0].ActualWidth);
+        }
+
+        [Fact]
+        public void Shared_Size_Group_Is_Registered_For_Row_Definitions_Assigned_As_A_Collection()
+        {
+            var grids = new[]
+            {
+                new Grid
+                {
+                    RowDefinitions = new RowDefinitions
+                    {
+                        new RowDefinition { Height = GridLength.Auto, SharedSizeGroup = "A" },
+                        new RowDefinition { Height = new GridLength(1, GridUnitType.Star) },
+                    },
+                },
+                new Grid
+                {
+                    RowDefinitions = new RowDefinitions
+                    {
+                        new RowDefinition { Height = GridLength.Auto, SharedSizeGroup = "A" },
+                        new RowDefinition { Height = new GridLength(1, GridUnitType.Star) },
+                    },
+                },
+            };
+            grids[0].Children.Add(new Border { Width = 10, Height = 50 });
+
+            var scope = new StackPanel
+            {
+                Orientation = Layout.Orientation.Horizontal,
+                [Grid.IsSharedSizeScopeProperty] = true,
+                Children = { grids[0], grids[1] },
+            };
+            var root = new TestRoot(scope);
+
+            root.ExecuteInitialLayoutPass();
+            root.LayoutManager.ExecuteLayoutPass();
+
+            Assert.Equal(50, grids[0].RowDefinitions[0].ActualHeight);
+            Assert.Equal(50, grids[1].RowDefinitions[0].ActualHeight);
+        }
+
         [Fact]
         public void Collection_Changes_Are_Tracked()
         {