From 1fda7447703d6534ef3a2cab9fe1dba25d47bf16 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Mon, 5 Feb 2018 00:16:08 +0000 Subject: [PATCH] Make RenderTreeFrame properly readonly to allow more pass-by-ref cases --- .../Engine/BlazorIntermediateNodeWriter.cs | 7 +- .../RenderTree/RenderTreeBuilder.cs | 19 ++- .../RenderTree/RenderTreeDiffComputer.cs | 2 +- .../RenderTree/RenderTreeFrame.cs | 131 +++++++++--------- .../Rendering/Renderer.cs | 2 +- .../RenderTreeBuilderTest.cs | 4 +- .../RenderTreeDiffComputerTest.cs | 40 +++--- .../RendererTest.cs | 18 +-- 8 files changed, 122 insertions(+), 101 deletions(-) diff --git a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs b/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs index 0223715639..cc62d7e05f 100644 --- a/src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/Engine/BlazorIntermediateNodeWriter.cs +++ b/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; diff --git a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeBuilder.cs b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeBuilder.cs index 79ff25b5d1..e2200d23ad 100644 --- a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeBuilder.cs +++ b/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); + } + + /// + /// Marks a previously appended component frame as closed. Calls to this method + /// must be balanced with calls to . + /// + public void CloseComponent() + { + var indexOfEntryBeingClosed = _openElementIndices.Pop(); + ref var entry = ref _entries.Buffer[indexOfEntryBeingClosed]; + entry = entry.WithComponentDescendantsEndIndex(_entries.Count - 1); } /// @@ -57,7 +69,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree /// An integer that represents the position of the instruction in the source code. /// Content for the new text frame. public void AddText(int sequence, string textContent) - => Append(RenderTreeFrame.Text(sequence, textContent)); + => Append(RenderTreeFrame.Text(sequence, textContent ?? string.Empty)); /// /// Appends a frame representing text content. @@ -133,8 +145,7 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree } AssertCanAddAttribute(); - frame.SetSequence(sequence); - Append(frame); + Append(frame.WithAttributeSequence(sequence)); } /// diff --git a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffComputer.cs b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffComputer.cs index b04c16e2b0..cde220d59d 100644 --- a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffComputer.cs +++ b/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; diff --git a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeFrame.cs b/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeFrame.cs index 6fcdb62ca0..f869e12f00 100644 --- a/src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeFrame.cs +++ b/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. /// [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(4)] RenderTreeFrameType _frameType; + [FieldOffset(0)] readonly int _sequence; + [FieldOffset(4)] readonly RenderTreeFrameType _frameType; // RenderTreeFrameType.Element - [FieldOffset(8)] private int _elementDescendantsEndIndex; - [FieldOffset(16)] string _elementName; + [FieldOffset(8)] readonly int _elementDescendantsEndIndex; + [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(24)] private object _attributeValue; + [FieldOffset(16)] readonly string _attributeName; + [FieldOffset(24)] readonly object _attributeValue; // RenderTreeFrameType.Component - [FieldOffset(8)] private int _componentDescendantsEndIndex; - [FieldOffset(12)] private int _componentId; - [FieldOffset(16)] private Type _componentType; - [FieldOffset(24)] private IComponent _component; + [FieldOffset(8)] readonly int _componentDescendantsEndIndex; + [FieldOffset(12)] readonly int _componentId; + [FieldOffset(16)] readonly Type _componentType; + [FieldOffset(24)] readonly IComponent _component; /// /// Gets the sequence number of the frame. Sequence numbers indicate the relative source @@ -106,68 +106,73 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree /// public IComponent Component => _component; - internal static RenderTreeFrame Element(int sequence, string elementName) => new RenderTreeFrame - { - _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(int sequence) where T : IComponent => new RenderTreeFrame - { - _sequence = sequence, - _frameType = RenderTreeFrameType.Component, - _componentType = typeof(T) - }; - - internal void CloseElement(int descendantsEndIndex) + private RenderTreeFrame(int sequence, string elementName, int descendantsEndIndex) + : this() { + _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 - // need to construct the attribute frame in a context where they don't know the sequence - // number, so we assign it later + _frameType = RenderTreeFrameType.Text; _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(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); } } diff --git a/src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs b/src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs index 9fa9e43c29..7c2d3fa4d1 100644 --- a/src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs +++ b/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) diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs index d039cac7c8..38fb9192fa 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeBuilderTest.cs @@ -256,10 +256,10 @@ namespace Microsoft.AspNetCore.Blazor.Test builder.OpenComponentElement(11); // 1: - builder.CloseElement(); // + builder.CloseComponent(); // builder.OpenComponentElement(14); // 4: - builder.CloseElement(); // + builder.CloseComponent(); // builder.CloseElement(); // // Assert diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffComputerTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffComputerTest.cs index 64541ab5ab..dc4b6c3152 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffComputerTest.cs +++ b/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: newTree.OpenComponentElement(12); // 2: - newTree.CloseElement(); // + newTree.CloseComponent(); // newTree.OpenComponentElement(13); // 3: - newTree.CloseElement(); // + newTree.CloseComponent(); // newTree.CloseElement(); // // 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(0); newTree.AddAttribute(1, "SomeUnknownProperty", 123); - newTree.CloseElement(); + newTree.CloseComponent(); // Act/Assert var ex = Assert.Throws(() => @@ -686,7 +686,7 @@ namespace Microsoft.AspNetCore.Blazor.Test var testObject = new object(); newTree.OpenComponentElement(0); newTree.AddAttribute(1, nameof(FakeComponent.ReadonlyProperty), 123); - newTree.CloseElement(); + newTree.CloseComponent(); // Act/Assert var ex = Assert.Throws(() => @@ -704,16 +704,16 @@ namespace Microsoft.AspNetCore.Blazor.Test oldTree.AddText(10, "text1"); // 0: text1 oldTree.OpenElement(11, "container"); // 1: oldTree.OpenComponentElement(12); // 2: - oldTree.CloseElement(); // + oldTree.CloseComponent(); // oldTree.OpenComponentElement(13); // 3: - oldTree.CloseElement(); // - oldTree.CloseElement(); // + oldTree.CloseElement(); // newTree.AddText(10, "text1"); // 0: text1 newTree.OpenElement(11, "container"); // 1: newTree.OpenComponentElement(12); // 2: - newTree.CloseElement(); // + newTree.CloseComponent(); // newTree.OpenComponentElement(13); // 3: - newTree.CloseElement(); // + newTree.CloseComponent(); // newTree.CloseElement(); // (12); oldTree.AddAttribute(13, nameof(FakeComponent.StringProperty), "String will change"); oldTree.AddAttribute(14, nameof(FakeComponent.ObjectProperty), objectWillNotChange); - oldTree.CloseElement(); + oldTree.CloseComponent(); newTree.OpenComponentElement(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(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(0); oldTree.AddAttribute(1, nameof(HandlePropertiesChangedComponent.IntProperty), 123); - oldTree.CloseElement(); + oldTree.CloseComponent(); newTree1.OpenComponentElement(0); newTree1.AddAttribute(1, nameof(HandlePropertiesChangedComponent.IntProperty), 123); - newTree1.CloseElement(); + newTree1.CloseComponent(); newTree2.OpenComponentElement(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(10); // - oldTree.CloseElement(); // + oldTree.CloseComponent(); // oldTree.OpenComponentElement(20); // - oldTree.CloseElement(); // + oldTree.CloseComponent(); // oldTree.OpenComponentElement(30); // - oldTree.CloseElement(); // + oldTree.CloseComponent(); // newTree.OpenComponentElement(30); // - newTree.CloseElement(); // + newTree.CloseComponent(); // diff.ApplyNewRenderTreeVersion(new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetFrames(), oldTree.GetFrames()); var disposableComponent1 = (DisposableComponent)oldTree.GetFrames().Array[0].Component; diff --git a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs index a2966c3e63..4f97cf9306 100644 --- a/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs +++ b/test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Blazor.Test builder.AddText(0, "Hello"); builder.OpenComponentElement(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(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(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(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(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(100); - builder.CloseElement(); + builder.CloseComponent(); builder.OpenComponentElement(150); - builder.CloseElement(); + builder.CloseComponent(); } builder.OpenComponentElement(200); - builder.CloseElement(); + builder.CloseComponent(); builder.CloseElement(); });