From 710de1f85a7da094607066c2e38d8d558ebf2532 Mon Sep 17 00:00:00 2001
From: Steven Kirk <grokys@gmail.com>
Date: Thu, 19 May 2016 21:25:49 +0100
Subject: [PATCH] Fix an edge case in visual parenting.

If a child that already has a parent was added as a visual child of
another control and the exception caught, the control would still be
present in the VisualChildren collection.
---
 src/Avalonia.Base/Collections/AvaloniaList.cs | 20 +++++++++++--------
 src/Avalonia.SceneGraph/Visual.cs             | 16 +++++++--------
 .../VisualTests.cs                            | 13 ++++++++++++
 3 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/src/Avalonia.Base/Collections/AvaloniaList.cs b/src/Avalonia.Base/Collections/AvaloniaList.cs
index b064032e1c..e893fcb75b 100644
--- a/src/Avalonia.Base/Collections/AvaloniaList.cs
+++ b/src/Avalonia.Base/Collections/AvaloniaList.cs
@@ -150,16 +150,20 @@ namespace Avalonia.Collections
                 Validate?.Invoke(value);
 
                 T old = _inner[index];
-                _inner[index] = value;
 
-                if (_collectionChanged != null)
+                if (!object.Equals(old, value))
                 {
-                    var e = new NotifyCollectionChangedEventArgs(
-                        NotifyCollectionChangedAction.Replace,
-                        value,
-                        old,
-                        index);
-                    _collectionChanged(this, e);
+                    _inner[index] = value;
+
+                    if (_collectionChanged != null)
+                    {
+                        var e = new NotifyCollectionChangedEventArgs(
+                            NotifyCollectionChangedAction.Replace,
+                            value,
+                            old,
+                            index);
+                        _collectionChanged(this, e);
+                    }
                 }
             }
         }
diff --git a/src/Avalonia.SceneGraph/Visual.cs b/src/Avalonia.SceneGraph/Visual.cs
index 6ecab23d6a..9cf97c2767 100644
--- a/src/Avalonia.SceneGraph/Visual.cs
+++ b/src/Avalonia.SceneGraph/Visual.cs
@@ -94,7 +94,7 @@ namespace Avalonia
         {
             var visualChildren = new AvaloniaList<IVisual>();
             visualChildren.ResetBehavior = ResetBehavior.Remove;
-            visualChildren.Validate = ValidateLogicalChild;
+            visualChildren.Validate = ValidateVisualChild;
             visualChildren.CollectionChanged += VisualChildrenChanged;
             VisualChildren = visualChildren;
         }
@@ -414,15 +414,20 @@ namespace Avalonia
         }
 
         /// <summary>
-        /// Ensures a visual child is not null.
+        /// Ensures a visual child is not null and not already parented.
         /// </summary>
         /// <param name="c">The visual child.</param>
-        private static void ValidateLogicalChild(IVisual c)
+        private static void ValidateVisualChild(IVisual c)
         {
             if (c == null)
             {
                 throw new ArgumentNullException("Cannot add null to VisualChildren.");
             }
+
+            if (c.VisualParent != null)
+            {
+                throw new InvalidOperationException("The control already has a visual parent.");
+            }
         }
 
         /// <summary>
@@ -442,11 +447,6 @@ namespace Avalonia
         /// <param name="value">The visual parent.</param>
         private void SetVisualParent(Visual value)
         {
-            if (value != null && _visualParent != null)
-            {
-                throw new InvalidOperationException("The control already has a visual parent.");
-            }
-
             if (_visualParent == value)
             {
                 return;
diff --git a/tests/Avalonia.SceneGraph.UnitTests/VisualTests.cs b/tests/Avalonia.SceneGraph.UnitTests/VisualTests.cs
index c7d0de6f56..6679138e6d 100644
--- a/tests/Avalonia.SceneGraph.UnitTests/VisualTests.cs
+++ b/tests/Avalonia.SceneGraph.UnitTests/VisualTests.cs
@@ -98,5 +98,18 @@ namespace Avalonia.SceneGraph.UnitTests
             Assert.True(called1);
             Assert.True(called2);
         }
+
+        [Fact]
+        public void Adding_Already_Parented_Control_Should_Throw()
+        {
+            var root1 = new TestRoot();
+            var root2 = new TestRoot();
+            var child = new TestVisual();
+
+            root1.AddChild(child);
+
+            Assert.Throws<InvalidOperationException>(() => root2.AddChild(child));
+            Assert.Equal(0, root2.GetVisualChildren().Count());
+        }
     }
 }