Make RenderTreeFrame properly readonly to allow more pass-by-ref cases

src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs

@@ -221,6 +221,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine
                     case HtmlTokenType.EndTag:
                         {
                             var nextTag = nextToken.AsTag();
+                            var isComponent = false;
                             if (nextToken.Type == HtmlTokenType.StartTag)
                             {
                                 var tagNameOriginalCase = GetTagNameWithOriginalCase(originalHtmlContent, nextTag);
@@ -230,6 +231,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine
                                         .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.OpenComponentElement)}<{componentTypeName}>")
                                         .Write((_sourceSequence++).ToString())
                                         .WriteEndMethodInvocation();
+                                    isComponent = true;
                                 }
                                 else
                                 {
@@ -274,8 +276,11 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine
                                 || nextTag.IsSelfClosing
                                 || htmlVoidElementsLookup.Contains(nextTag.Data))
                             {
+                                var closeMethodName = isComponent
+                                    ? nameof(RenderTreeBuilder.CloseComponent)
+                                    : nameof(RenderTreeBuilder.CloseElement);
                                 codeWriter
-                                    .WriteStartMethodInvocation($"{builderVarName}.{nameof(RenderTreeBuilder.CloseElement)}")
+                                    .WriteStartMethodInvocation($"{builderVarName}.{closeMethodName}")
                                     .WriteEndMethodInvocation();
                             }
                             break;

src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeBuilder.cs

@@ -48,7 +48,19 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
         public void CloseElement()
         {
             var indexOfEntryBeingClosed = _openElementIndices.Pop();
-            _entries.Buffer[indexOfEntryBeingClosed].CloseElement(_entries.Count - 1);
+            ref var entry = ref _entries.Buffer[indexOfEntryBeingClosed];
+            entry = entry.WithElementDescendantsEndIndex(_entries.Count - 1);
+        }
+
+        /// <summary>
+        /// Marks a previously appended component frame as closed. Calls to this method
+        /// must be balanced with calls to <see cref="OpenComponentElement{TComponent}"/>.
+        /// </summary>
+        public void CloseComponent()
+        {
+            var indexOfEntryBeingClosed = _openElementIndices.Pop();
+            ref var entry = ref _entries.Buffer[indexOfEntryBeingClosed];
+            entry = entry.WithComponentDescendantsEndIndex(_entries.Count - 1);
         }
 
         /// <summary>
@@ -57,7 +69,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
         /// <param name="sequence">An integer that represents the position of the instruction in the source code.</param>
         /// <param name="textContent">Content for the new text frame.</param>
         public void AddText(int sequence, string textContent)
-            => Append(RenderTreeFrame.Text(sequence, textContent));
+            => Append(RenderTreeFrame.Text(sequence, textContent ?? string.Empty));
 
         /// <summary>
         /// Appends a frame representing text content.
@@ -133,8 +145,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
             }
 
             AssertCanAddAttribute();
-            frame.SetSequence(sequence);
+            Append(frame.WithAttributeSequence(sequence));
-            Append(frame);
         }
 
         /// <summary>

src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffComputer.cs

@@ -197,7 +197,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
             var hasSetAnyProperty = false;
 
             // Preserve the actual componentInstance
-            newComponentFrame.SetChildComponentInstance(componentId, componentInstance);
+            newComponentFrame = newComponentFrame.WithComponentInstance(componentId, componentInstance);
 
             // Now locate any added/changed/removed properties
             var oldStartIndex = oldComponentIndex + 1;

src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeFrame.cs

@@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
     /// Represents an entry in a tree of user interface (UI) items.
     /// </summary>
     [StructLayout(LayoutKind.Explicit)]
-    public struct RenderTreeFrame
+    public readonly struct RenderTreeFrame
     {
         // Note that the struct layout has to be valid in both 32-bit and 64-bit runtime platforms,
         // which means that all reference-type fields need to take up 8 bytes (except for the last
@@ -25,25 +25,25 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
         // 8 bytes here.
 
         // Common
-        [FieldOffset(0)] int _sequence;
+        [FieldOffset(0)] readonly int _sequence;
-        [FieldOffset(4)] RenderTreeFrameType _frameType;
+        [FieldOffset(4)] readonly RenderTreeFrameType _frameType;
 
         // RenderTreeFrameType.Element
-        [FieldOffset(8)] private int _elementDescendantsEndIndex;
+        [FieldOffset(8)] readonly int _elementDescendantsEndIndex;
-        [FieldOffset(16)] string _elementName;
+        [FieldOffset(16)] readonly string _elementName;
 
         // RenderTreeFrameType.Text
-        [FieldOffset(16)] private string _textContent;
+        [FieldOffset(16)] readonly string _textContent;
 
         // RenderTreeFrameType.Attribute
-        [FieldOffset(16)] private string _attributeName;
+        [FieldOffset(16)] readonly string _attributeName;
-        [FieldOffset(24)] private object _attributeValue;
+        [FieldOffset(24)] readonly object _attributeValue;
 
         // RenderTreeFrameType.Component
-        [FieldOffset(8)] private int _componentDescendantsEndIndex;
+        [FieldOffset(8)] readonly int _componentDescendantsEndIndex;
-        [FieldOffset(12)] private int _componentId;
+        [FieldOffset(12)] readonly int _componentId;
-        [FieldOffset(16)] private Type _componentType;
+        [FieldOffset(16)] readonly Type _componentType;
-        [FieldOffset(24)] private IComponent _component;
+        [FieldOffset(24)] readonly IComponent _component;
 
         /// <summary>
         /// Gets the sequence number of the frame. Sequence numbers indicate the relative source
@@ -106,68 +106,73 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
         /// </summary>
         public IComponent Component => _component;
 
-        internal static RenderTreeFrame Element(int sequence, string elementName) => new RenderTreeFrame
+        private RenderTreeFrame(int sequence, string elementName, int descendantsEndIndex)
-        {
+            : this()
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Element,
-            _elementName = elementName,
-        };
-
-        internal static RenderTreeFrame Text(int sequence, string textContent) => new RenderTreeFrame
-        {
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Text,
-            _textContent = textContent ?? string.Empty,
-        };
-
-        internal static RenderTreeFrame Attribute(int sequence, string name, string value) => new RenderTreeFrame
-        {
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Attribute,
-            _attributeName = name,
-            _attributeValue = value
-        };
-
-        internal static RenderTreeFrame Attribute(int sequence, string name, UIEventHandler value) => new RenderTreeFrame
-        {
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Attribute,
-            _attributeName = name,
-            _attributeValue = value
-        };
-
-        internal static RenderTreeFrame Attribute(int sequence, string name, object value) => new RenderTreeFrame
-        {
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Attribute,
-            _attributeName = name,
-            _attributeValue = value
-        };
-
-        internal static RenderTreeFrame ChildComponent<T>(int sequence) where T : IComponent => new RenderTreeFrame
-        {
-            _sequence = sequence,
-            _frameType = RenderTreeFrameType.Component,
-            _componentType = typeof(T)
-        };
-
-        internal void CloseElement(int descendantsEndIndex)
         {
+            _frameType = RenderTreeFrameType.Element;
+            _sequence = sequence;
+            _elementName = elementName;
             _elementDescendantsEndIndex = descendantsEndIndex;
         }
 
-        internal void SetChildComponentInstance(int componentId, IComponent component)
+        private RenderTreeFrame(int sequence, Type componentType, int descendantsEndIndex)
+            : this()
+        {
+            _frameType = RenderTreeFrameType.Component;
+            _sequence = sequence;
+            _componentType = componentType;
+            _componentDescendantsEndIndex = descendantsEndIndex;
+        }
+
+        private RenderTreeFrame(int sequence, Type componentType, int descendantsEndIndex, int componentId, IComponent component)
+            : this(sequence, componentType, descendantsEndIndex)
         {
             _componentId = componentId;
             _component = component;
         }
 
-        internal void SetSequence(int sequence)
+        private RenderTreeFrame(int sequence, string textContent)
+            : this()
         {
-            // This is only used when appending attribute frames, because helpers such as @onclick
+            _frameType = RenderTreeFrameType.Text;
-            // need to construct the attribute frame in a context where they don't know the sequence
-            // number, so we assign it later
             _sequence = sequence;
+            _textContent = textContent;
         }
+
+        private RenderTreeFrame(int sequence, string attributeName, object attributeValue)
+            : this()
+        {
+            _frameType = RenderTreeFrameType.Attribute;
+            _sequence = sequence;
+            _attributeName = attributeName;
+            _attributeValue = attributeValue;
+        }
+
+        internal static RenderTreeFrame Element(int sequence, string elementName)
+            => new RenderTreeFrame(sequence, elementName: elementName, descendantsEndIndex: 0);
+
+        internal static RenderTreeFrame Text(int sequence, string textContent)
+            => new RenderTreeFrame(sequence, textContent: textContent);
+
+        internal static RenderTreeFrame Attribute(int sequence, string name, UIEventHandler value)
+             => new RenderTreeFrame(sequence, attributeName: name, attributeValue: value);
+
+        internal static RenderTreeFrame Attribute(int sequence, string name, object value)
+            => new RenderTreeFrame(sequence, attributeName: name, attributeValue: value);
+
+        internal static RenderTreeFrame ChildComponent<T>(int sequence) where T : IComponent
+            => new RenderTreeFrame(sequence, typeof(T), 0);
+
+        internal RenderTreeFrame WithElementDescendantsEndIndex(int descendantsEndIndex)
+            => new RenderTreeFrame(_sequence, elementName: _elementName, descendantsEndIndex: descendantsEndIndex);
+
+        internal RenderTreeFrame WithComponentDescendantsEndIndex(int descendantsEndIndex)
+            => new RenderTreeFrame(_sequence, componentType: _componentType, descendantsEndIndex: descendantsEndIndex);
+
+        internal RenderTreeFrame WithAttributeSequence(int sequence)
+            => new RenderTreeFrame(sequence, attributeName: _attributeName, attributeValue: _attributeValue);
+
+        internal RenderTreeFrame WithComponentInstance(int componentId, IComponent component)
+            => new RenderTreeFrame(_sequence, _componentType, _componentDescendantsEndIndex, componentId, component);
     }
 }

src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs

@@ -126,7 +126,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
 
             var newComponent = (IComponent)Activator.CreateInstance(frame.ComponentType);
             var newComponentId = AssignComponentId(newComponent);
-            frame.SetChildComponentInstance(newComponentId, newComponent);
+            frame = frame.WithComponentInstance(newComponentId, newComponent);
         }
 
         private ComponentState GetRequiredComponentState(int componentId)

test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs

@@ -256,10 +256,10 @@ namespace Microsoft.AspNetCore.Blazor.Test
             builder.OpenComponentElement<TestComponent>(11);     //  1:     <testcomponent
             builder.AddAttribute(12, "child1attribute1", "A");   //  2:       child1attribute1="A"
             builder.AddAttribute(13, "child1attribute2", "B");   //  3:       child1attribute2="B">
-            builder.CloseElement();                              //         </testcomponent>
+            builder.CloseComponent();                            //         </testcomponent>
             builder.OpenComponentElement<TestComponent>(14);     //  4:     <testcomponent
             builder.AddAttribute(15, "child2attribute", "C");    //  5:       child2attribute="C">
-            builder.CloseElement();                              //         </testcomponent>
+            builder.CloseComponent();                            //         </testcomponent>
             builder.CloseElement();                              //     </parent>
 
             // Assert

test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffComputerTest.cs

@@ -586,9 +586,9 @@ namespace Microsoft.AspNetCore.Blazor.Test
             newTree.AddText(10, "text1");                       //  0: text1
             newTree.OpenElement(11, "container");               //  1: <container>
             newTree.OpenComponentElement<FakeComponent>(12);    //  2:   <FakeComponent>
-            newTree.CloseElement();                             //       </FakeComponent>
+            newTree.CloseComponent();                           //       </FakeComponent>
             newTree.OpenComponentElement<FakeComponent2>(13);   //  3:   <FakeComponent2>
-            newTree.CloseElement();                             //       </FakeComponent2>
+            newTree.CloseComponent();                           //       </FakeComponent2>
             newTree.CloseElement();                             //     </container>
 
             // Act
@@ -645,7 +645,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             newTree.AddAttribute(1, nameof(FakeComponent.IntProperty), 123);
             newTree.AddAttribute(2, nameof(FakeComponent.StringProperty), "some string");
             newTree.AddAttribute(3, nameof(FakeComponent.ObjectProperty), testObject);
-            newTree.CloseElement();
+            newTree.CloseComponent();
 
             // Act
             var renderBatch = GetRenderedBatch();
@@ -669,7 +669,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             var testObject = new object();
             newTree.OpenComponentElement<FakeComponent>(0);
             newTree.AddAttribute(1, "SomeUnknownProperty", 123);
-            newTree.CloseElement();
+            newTree.CloseComponent();
 
             // Act/Assert
             var ex = Assert.Throws<InvalidOperationException>(() =>
@@ -686,7 +686,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             var testObject = new object();
             newTree.OpenComponentElement<FakeComponent>(0);
             newTree.AddAttribute(1, nameof(FakeComponent.ReadonlyProperty), 123);
-            newTree.CloseElement();
+            newTree.CloseComponent();
 
             // Act/Assert
             var ex = Assert.Throws<InvalidOperationException>(() =>
@@ -704,16 +704,16 @@ namespace Microsoft.AspNetCore.Blazor.Test
             oldTree.AddText(10, "text1");                       //  0: text1
             oldTree.OpenElement(11, "container");               //  1: <container>
             oldTree.OpenComponentElement<FakeComponent>(12);    //  2:   <FakeComponent>
-            oldTree.CloseElement();                             //       </FakeComponent>
+            oldTree.CloseComponent();                           //       </FakeComponent>
             oldTree.OpenComponentElement<FakeComponent2>(13);   //  3:   <FakeComponent2>
-            oldTree.CloseElement();                             //       </FakeComponent2>
+            oldTree.CloseComponent();                           //       </FakeComponent2>
-            oldTree.CloseElement();                             //     </container
+            oldTree.CloseElement();                             //     </container>
             newTree.AddText(10, "text1");                       //  0: text1
             newTree.OpenElement(11, "container");               //  1: <container>
             newTree.OpenComponentElement<FakeComponent>(12);    //  2:   <FakeComponent>
-            newTree.CloseElement();                             //       </FakeComponent>
+            newTree.CloseComponent();                           //       </FakeComponent>
             newTree.OpenComponentElement<FakeComponent2>(13);   //  3:   <FakeComponent2>
-            newTree.CloseElement();                             //       </FakeComponent2>
+            newTree.CloseComponent();                           //       </FakeComponent2>
             newTree.CloseElement();                             //     </container
 
             diff.ApplyNewRenderTreeVersion(new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames());
@@ -741,11 +741,11 @@ namespace Microsoft.AspNetCore.Blazor.Test
             oldTree.OpenComponentElement<FakeComponent>(12);
             oldTree.AddAttribute(13, nameof(FakeComponent.StringProperty), "String will change");
             oldTree.AddAttribute(14, nameof(FakeComponent.ObjectProperty), objectWillNotChange);
-            oldTree.CloseElement();
+            oldTree.CloseComponent();
             newTree.OpenComponentElement<FakeComponent>(12);
             newTree.AddAttribute(13, nameof(FakeComponent.StringProperty), "String did change");
             newTree.AddAttribute(14, nameof(FakeComponent.ObjectProperty), objectWillNotChange);
-            newTree.CloseElement();
+            newTree.CloseComponent();
 
             diff.ApplyNewRenderTreeVersion(new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames());
             var originalComponentInstance = (FakeComponent)oldTree.GetFrames().Array[0].Component;
@@ -767,7 +767,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
         {
             // Arrange
             newTree.OpenComponentElement<HandlePropertiesChangedComponent>(0);
-            newTree.CloseElement();
+            newTree.CloseComponent();
 
             // Act
             var batch = GetRenderedBatch();
@@ -786,13 +786,13 @@ namespace Microsoft.AspNetCore.Blazor.Test
             var newTree2 = new RenderTreeBuilder(renderer);
             oldTree.OpenComponentElement<HandlePropertiesChangedComponent>(0);
             oldTree.AddAttribute(1, nameof(HandlePropertiesChangedComponent.IntProperty), 123);
-            oldTree.CloseElement();
+            oldTree.CloseComponent();
             newTree1.OpenComponentElement<HandlePropertiesChangedComponent>(0);
             newTree1.AddAttribute(1, nameof(HandlePropertiesChangedComponent.IntProperty), 123);
-            newTree1.CloseElement();
+            newTree1.CloseComponent();
             newTree2.OpenComponentElement<HandlePropertiesChangedComponent>(0);
             newTree2.AddAttribute(1, nameof(HandlePropertiesChangedComponent.IntProperty), 456);
-            newTree2.CloseElement();
+            newTree2.CloseComponent();
 
             // Act/Assert 0: Initial render
             var batch0 = GetRenderedBatch(new RenderTreeBuilder(renderer), oldTree);
@@ -820,13 +820,13 @@ namespace Microsoft.AspNetCore.Blazor.Test
         {
             // Arrange
             oldTree.OpenComponentElement<DisposableComponent>(10);       // <DisposableComponent>
-            oldTree.CloseElement();                                      // </DisposableComponent>
+            oldTree.CloseComponent();                                    // </DisposableComponent>
             oldTree.OpenComponentElement<NonDisposableComponent>(20);    // <NonDisposableComponent>
-            oldTree.CloseElement();                                      // </NonDisposableComponent>
+            oldTree.CloseComponent();                                    // </NonDisposableComponent>
             oldTree.OpenComponentElement<DisposableComponent>(30);       // <DisposableComponent>
-            oldTree.CloseElement();                                      // </DisposableComponent>
+            oldTree.CloseComponent();                                    // </DisposableComponent>
             newTree.OpenComponentElement<DisposableComponent>(30);       // <DisposableComponent>
-            newTree.CloseElement();                                      // </DisposableComponent>
+            newTree.CloseComponent();                                    // </DisposableComponent>
 
             diff.ApplyNewRenderTreeVersion(new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames());
             var disposableComponent1 = (DisposableComponent)oldTree.GetFrames().Array[0].Component;

test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs

@@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
                 builder.AddText(0, "Hello");
                 builder.OpenComponentElement<MessageComponent>(1);
                 builder.AddAttribute(2, nameof(MessageComponent.Message), "Nested component output");
-                builder.CloseElement();
+                builder.CloseComponent();
             });
 
             // Act/Assert
@@ -92,7 +92,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             var parentComponent = new TestComponent(builder =>
             {
                 builder.OpenComponentElement<MessageComponent>(0);
-                builder.CloseElement();
+                builder.CloseComponent();
             });
             var parentComponentId = renderer.AssignComponentId(parentComponent);
             renderer.RenderNewBatch(parentComponentId);
@@ -151,7 +151,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             var parentComponent = new TestComponent(builder =>
             {
                 builder.OpenComponentElement<EventComponent>(0);
-                builder.CloseElement();
+                builder.CloseComponent();
             });
             var parentComponentId = renderer.AssignComponentId(parentComponent);
             renderer.RenderNewBatch(parentComponentId);
@@ -307,7 +307,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             {
                 builder.AddText(0, message);
                 builder.OpenComponentElement<MessageComponent>(1);
-                builder.CloseElement();
+                builder.CloseComponent();
             });
 
             var rootComponentId = renderer.AssignComponentId(component);
@@ -340,7 +340,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
                 builder.AddAttribute(2, nameof(FakeComponent.IntProperty), firstRender ? 123 : 256);
                 builder.AddAttribute(3, nameof(FakeComponent.ObjectProperty), objectThatWillNotChange);
                 builder.AddAttribute(4, nameof(FakeComponent.StringProperty), firstRender ? "String that will change" : "String that did change");
-                builder.CloseElement();
+                builder.CloseComponent();
             });
 
             var rootComponentId = renderer.AssignComponentId(component);
@@ -380,7 +380,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             {
                 builder.OpenComponentElement<MessageComponent>(1);
                 builder.AddAttribute(2, nameof(MessageComponent.Message), firstRender ? "first" : "second");
-                builder.CloseElement();
+                builder.CloseComponent();
             });
 
             var rootComponentId = renderer.AssignComponentId(component);
@@ -414,12 +414,12 @@ namespace Microsoft.AspNetCore.Blazor.Test
                 if (firstRender)
                 {
                     builder.OpenComponentElement<FakeComponent>(100);
-                    builder.CloseElement();
+                    builder.CloseComponent();
                     builder.OpenComponentElement<FakeComponent>(150);
-                    builder.CloseElement();
+                    builder.CloseComponent();
                 }
                 builder.OpenComponentElement<FakeComponent>(200);
-                builder.CloseElement();
+                builder.CloseComponent();
                 builder.CloseElement();
             });