From 7a1abbaca38aa94948a1642c424250f22c19aa10 Mon Sep 17 00:00:00 2001
From: Steve Sanderson <SteveSandersonMS@users.noreply.github.com>
Date: Sun, 21 Jan 2018 19:37:34 +0000
Subject: [PATCH] In tree diffing, better handle scenario where both old and
 new sequences loop back but to different locations

---
 .../RenderTree/RenderTreeDiff.cs              |  9 +++
 .../RenderTreeDiffTest.cs                     | 58 +++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/src/Microsoft.Blazor/RenderTree/RenderTreeDiff.cs b/src/Microsoft.Blazor/RenderTree/RenderTreeDiff.cs
index af0835980f..899417e627 100644
--- a/src/Microsoft.Blazor/RenderTree/RenderTreeDiff.cs
+++ b/src/Microsoft.Blazor/RenderTree/RenderTreeDiff.cs
@@ -63,12 +63,20 @@ namespace Microsoft.Blazor.RenderTree
                         // preordered merge join (picking from whichever side brings us closer to being
                         // back in sync)
                         treatAsInsert = newSeq < oldSeq;
+
+                        if (oldLoopedBack)
+                        {
+                            // If both old and new have now looped back, we must reset their 'looped back'
+                            // tracker so we can treat them as proceeding through the same loop block
+                            prevOldSeq = prevNewSeq = -1;
+                        }
                     }
                     else if (oldLoopedBack)
                     {
                         // Old sequence looped back but new one didn't
                         // The new sequence either has some extra trailing elements in the current loop block
                         // which we should insert, or omits some old trailing loop blocks which we should delete
+                        // TODO: Find a way of not recomputing this next flag on every iteration
                         var newLoopsBackLater = false;
                         for (var testIndex = newStartIndex + 1; testIndex < newEndIndexExcl; testIndex++)
                         {
@@ -91,6 +99,7 @@ namespace Microsoft.Blazor.RenderTree
                         // The old sequence either has some extra trailing elements in the current loop block
                         // which we should delete, or the new sequence has extra trailing loop blocks which we
                         // should insert
+                        // TODO: Find a way of not recomputing this next flag on every iteration
                         var oldLoopsBackLater = false;
                         for (var testIndex = oldStartIndex + 1; testIndex < oldEndIndexExcl; testIndex++)
                         {
diff --git a/test/Microsoft.Blazor.Test/RenderTreeDiffTest.cs b/test/Microsoft.Blazor.Test/RenderTreeDiffTest.cs
index ba18ce3d9b..84868258f3 100644
--- a/test/Microsoft.Blazor.Test/RenderTreeDiffTest.cs
+++ b/test/Microsoft.Blazor.Test/RenderTreeDiffTest.cs
@@ -212,6 +212,64 @@ namespace Microsoft.Blazor.Test
                 });
         }
 
+        [Fact]
+        public void RecognizesLeadingLoopBlockItemsBeingAdded()
+        {
+            // Arrange
+            var oldTree = new RenderTreeBuilder(new FakeRenderer());
+            var newTree = new RenderTreeBuilder(new FakeRenderer());
+            var diff = new RenderTreeDiff();
+            oldTree.AddText(2, "x");
+            oldTree.AddText(2, "x"); // Note that the '0' and '1' items are not present on this iteration
+            newTree.AddText(2, "x");
+            newTree.AddText(0, "x");
+            newTree.AddText(1, "x");
+            newTree.AddText(2, "x");
+
+            // Act
+            var result = diff.ComputeDifference(oldTree.GetNodes(), newTree.GetNodes());
+
+            // Assert
+            Assert.Collection(result,
+                entry => Assert.Equal(RenderTreeDiffEntryType.Continue, entry.Type),
+                entry =>
+                {
+                    Assert.Equal(RenderTreeDiffEntryType.PrependNode, entry.Type);
+                    Assert.Equal(1, entry.NewTreeIndex);
+                },
+                entry =>
+                {
+                    Assert.Equal(RenderTreeDiffEntryType.PrependNode, entry.Type);
+                    Assert.Equal(2, entry.NewTreeIndex);
+                },
+                entry => Assert.Equal(RenderTreeDiffEntryType.Continue, entry.Type));
+        }
+
+        [Fact]
+        public void RecognizesLeadingLoopBlockItemsBeingRemoved()
+        {
+            // Arrange
+            var oldTree = new RenderTreeBuilder(new FakeRenderer());
+            var newTree = new RenderTreeBuilder(new FakeRenderer());
+            var diff = new RenderTreeDiff();
+            oldTree.AddText(2, "x");
+            oldTree.AddText(0, "x");
+            oldTree.AddText(1, "x");
+            oldTree.AddText(2, "x");
+            newTree.AddText(2, "x");
+            newTree.AddText(2, "x"); // Note that the '0' and '1' items are not present on this iteration
+
+            // Act
+            var result = diff.ComputeDifference(oldTree.GetNodes(), newTree.GetNodes());
+
+            // Assert
+            Assert.Collection(result,
+                entry => Assert.Equal(RenderTreeDiffEntryType.Continue, entry.Type),
+                entry => Assert.Equal(RenderTreeDiffEntryType.RemoveNode, entry.Type),
+                entry => Assert.Equal(RenderTreeDiffEntryType.RemoveNode, entry.Type),
+                entry => Assert.Equal(RenderTreeDiffEntryType.Continue, entry.Type));
+        }
+
         [Fact]
         public void HandlesAdjacentItemsBeingRemovedAndInsertedAtOnce()
         {