Eliminate IComponent.BuildRenderTree to guarantee that components are only rendered by themselves

samples/HostedInAspNet.Client/Program.cs

@@ -27,24 +27,5 @@ namespace HostedInAspNet.Client
         public void SetParameters(ParameterCollection parameters)
         {
         }
-
-        public void BuildRenderTree(RenderTreeBuilder builder)
-        {
-            builder.OpenElement(0, "h1");
-            builder.AddText(1, "Hello from RenderTree");
-            builder.CloseElement();
-
-            builder.OpenElement(2, "ul");
-
-            builder.OpenElement(3, "li");
-            builder.AddText(4, "First item");
-            builder.CloseElement();
-
-            builder.OpenElement(5, "li");
-            builder.AddText(6, "Second item");
-            builder.CloseElement();
-
-            builder.CloseElement();
-        }
     }
 }

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

@@ -42,9 +42,9 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation.Engine
             // from here. We inject the parameter later in RazorCompiler.
             var primaryMethod = documentNode.FindPrimaryMethod();
             primaryMethod.ReturnType = "void";
-            primaryMethod.MethodName = nameof(BlazorComponent.BuildRenderTree);
+            primaryMethod.MethodName = BlazorComponent.BuildRenderTreeMethodName;
             primaryMethod.Modifiers.Clear();
-            primaryMethod.Modifiers.Add("public");
+            primaryMethod.Modifiers.Add("protected");
             primaryMethod.Modifiers.Add("override");
 
             var line = new CSharpCodeIntermediateNode();

src/Microsoft.AspNetCore.Blazor.Build/Core/RazorCompilation/RazorCompiler.cs

@@ -105,7 +105,7 @@ namespace Microsoft.AspNetCore.Blazor.Build.Core.RazorCompilation
 
                 // Add parameters to the primary method via string manipulation because
                 // DefaultDocumentWriter's VisitMethodDeclaration can't emit parameters
-                var primaryMethodSource = $"public override void {nameof(BlazorComponent.BuildRenderTree)}";
+                var primaryMethodSource = $"protected override void {BlazorComponent.BuildRenderTreeMethodName}";
                 generatedCode = generatedCode.Replace(
                     $"{primaryMethodSource}()",
                     $"{primaryMethodSource}({typeof(RenderTreeBuilder).FullName} builder)");

src/Microsoft.AspNetCore.Blazor/Components/BlazorComponent.cs

@@ -13,12 +13,23 @@ namespace Microsoft.AspNetCore.Blazor.Components
     /// </summary>
     public abstract class BlazorComponent : IComponent, IHandleEvent
     {
+        public const string BuildRenderTreeMethodName = nameof(BuildRenderTree);
+
+        private readonly Action<RenderTreeBuilder> _renderAction;
         private RenderHandle _renderHandle;
         private bool _hasNeverRendered = true;
         private bool _hasPendingQueuedRender;
 
-        /// <inheritdoc />
+        public BlazorComponent()
-        public virtual void BuildRenderTree(RenderTreeBuilder builder)
+        {
+            _renderAction = BuildRenderTree;
+        }
+
+        /// <summary>
+        /// Renders the component to the supplied <see cref="RenderTreeBuilder"/>.
+        /// </summary>
+        /// <param name="builder">A <see cref="RenderTreeBuilder"/> that will receive the render output.</param>
+        protected virtual void BuildRenderTree(RenderTreeBuilder builder)
         {
             // Developers can either override this method in derived classes, or can use Razor
             // syntax to define a derived class and have the compiler generate the method.
@@ -48,7 +59,7 @@ namespace Microsoft.AspNetCore.Blazor.Components
             if (_hasNeverRendered || ShouldRender())
             {
                 _hasPendingQueuedRender = true;
-                _renderHandle.Render();
+                _renderHandle.Render(_renderAction);
             }
         }
 

src/Microsoft.AspNetCore.Blazor/Components/IComponent.cs

@@ -1,8 +1,6 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using Microsoft.AspNetCore.Blazor.RenderTree;
-
 namespace Microsoft.AspNetCore.Blazor.Components
 {
     /// <summary>
@@ -21,11 +19,5 @@ namespace Microsoft.AspNetCore.Blazor.Components
         /// </summary>
         /// <param name="parameters">The parameters.</param>
         void SetParameters(ParameterCollection parameters);
-
-        /// <summary>
-        /// Builds a <see cref="RenderTree"/> representing the current state of the component.
-        /// </summary>
-        /// <param name="builder">A <see cref="RenderTreeBuilder"/> to which the rendered frames should be appended.</param>
-        void BuildRenderTree(RenderTreeBuilder builder);
     }
 }

src/Microsoft.AspNetCore.Blazor/Components/RenderHandle.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using Microsoft.AspNetCore.Blazor.Rendering;
+using Microsoft.AspNetCore.Blazor.RenderTree;
 using System;
 
 namespace Microsoft.AspNetCore.Blazor.Components
@@ -30,14 +31,14 @@ namespace Microsoft.AspNetCore.Blazor.Components
         /// <summary>
         /// Notifies the renderer that the component should be rendered.
         /// </summary>
-        public void Render()
+        public void Render(Action<RenderTreeBuilder> renderAction)
         {
             if (_renderer == null)
             {
                 throw new InvalidOperationException("The render handle is not yet assigned.");
             }
 
-            _renderer.ComponentRequestedRender(_componentId);
+            _renderer.AddToRenderQueue(new RenderQueueEntry(_componentId, renderAction));
         }
     }
 }

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

@@ -35,13 +35,13 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
             _renderTreeBuilderPrevious = new RenderTreeBuilder(renderer);
         }
 
-        public void RenderIntoBatch(RenderBatchBuilder batchBuilder)
+        public void RenderIntoBatch(RenderBatchBuilder batchBuilder, Action<RenderTreeBuilder> renderAction)
         {
             // Swap the old and new tree builders
             (_renderTreeBuilderCurrent, _renderTreeBuilderPrevious) = (_renderTreeBuilderPrevious, _renderTreeBuilderCurrent);
 
             _renderTreeBuilderCurrent.Clear();
-            _component.BuildRenderTree(_renderTreeBuilderCurrent);
+            renderAction(_renderTreeBuilderCurrent);
 
             var diff = RenderTreeDiffBuilder.ComputeDiff(
                 _renderer,

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

@@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
         public ArrayBuilder<RenderTreeFrame> ReferenceFramesBuffer { get; } = new ArrayBuilder<RenderTreeFrame>();
 
         // State of render pipeline
-        public Queue<int> ComponentRenderQueue { get; } = new Queue<int>();
+        public Queue<RenderQueueEntry> ComponentRenderQueue { get; } = new Queue<RenderQueueEntry>();
         public Queue<int> ComponentDisposalQueue { get; } = new Queue<int>();
 
         public void Clear()

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

@@ -0,0 +1,20 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Blazor.RenderTree;
+using System;
+
+namespace Microsoft.AspNetCore.Blazor.Rendering
+{
+    internal readonly struct RenderQueueEntry
+    {
+        public readonly int ComponentId;
+        public readonly Action<RenderTreeBuilder> RenderAction;
+
+        public RenderQueueEntry(int componentId, Action<RenderTreeBuilder> renderAction)
+        {
+            ComponentId = componentId;
+            RenderAction = renderAction ?? throw new ArgumentNullException(nameof(renderAction));
+        }
+    }
+}

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

@@ -5,7 +5,6 @@ using Microsoft.AspNetCore.Blazor.Components;
 using Microsoft.AspNetCore.Blazor.RenderTree;
 using System;
 using System.Collections.Generic;
-using System.Threading;
 
 namespace Microsoft.AspNetCore.Blazor.Rendering
 {
@@ -19,10 +18,8 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
         private readonly Dictionary<int, ComponentState> _componentStateById
             = new Dictionary<int, ComponentState>();
 
-        // Because rendering is currently synchronous and single-threaded, we can keep re-using the
+        private readonly RenderBatchBuilder _batchBuilder = new RenderBatchBuilder();
-        // same RenderBatchBuilder instance to avoid reallocating
+        private bool _isBatchInProgress;
-        private readonly RenderBatchBuilder _sharedRenderBatchBuilder = new RenderBatchBuilder();
-        private int _renderBatchLock = 0;
 
         private int _lastEventHandlerId = 0;
         private readonly Dictionary<int, UIEventHandler> _eventHandlersById
@@ -106,17 +103,13 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
             frame = frame.WithAttributeEventHandlerId(id);
         }
 
-        internal void ComponentRequestedRender(int componentId)
+        internal void AddToRenderQueue(RenderQueueEntry renderQueueEntry)
         {
-            // TODO: Clean up the locking around rendering. The Renderer doesn't really need
+            _batchBuilder.ComponentRenderQueue.Enqueue(renderQueueEntry);
-            // to be thread-safe, and the following code isn't actually thread-safe anyway.
+
-            if (_renderBatchLock == 0)
+            if (!_isBatchInProgress)
             {
-                RenderNewBatch(componentId);
+                ProcessRenderQueue();
-            }
-            else
-            {
-                _sharedRenderBatchBuilder.ComponentRenderQueue.Enqueue(componentId);
             }
         }
 
@@ -125,59 +118,42 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
                 ? componentState
                 : throw new ArgumentException($"The renderer does not have a component with ID {componentId}.");
 
-        private void RenderNewBatch(int componentId)
+        private void ProcessRenderQueue()
         {
-            // It's very important that components' rendering logic has no side-effects, and in particular
+            _isBatchInProgress = true;
-            // components must *not* trigger Render from inside their render logic, otherwise you could
-            // easily get hard-to-debug infinite loops.
-            // Since rendering is currently synchronous and single-threaded, we can enforce the above by
-            // checking here that no other rendering process is already underway. This also means we only
-            // need a single _renderBatchBuilder instance that can be reused throughout the lifetime of
-            // the Renderer instance, which also means we're not allocating on a typical render cycle.
-            // In the future, if rendering becomes async, we'll need a more sophisticated system of
-            // capturing successive diffs from each component and probably serializing them for the
-            // interop calls instead of using shared memory.
-
-            // Note that Monitor.TryEnter is not yet supported in Mono WASM, so using the following instead
-            var renderAlreadyRunning = Interlocked.CompareExchange(ref _renderBatchLock, 1, 0) == 1;
-            if (renderAlreadyRunning)
-            {
-                throw new InvalidOperationException("Cannot render while a render is already in progress. " +
-                    "Render logic must not have side-effects such as manually triggering other rendering.");
-            }
-
-            _sharedRenderBatchBuilder.ComponentRenderQueue.Enqueue(componentId);
 
             try
             {
                 // Process render queue until empty
-                while (_sharedRenderBatchBuilder.ComponentRenderQueue.Count > 0)
+                while (_batchBuilder.ComponentRenderQueue.Count > 0)
                 {
-                    var nextComponentIdToRender = _sharedRenderBatchBuilder.ComponentRenderQueue.Dequeue();
+                    var nextToRender = _batchBuilder.ComponentRenderQueue.Dequeue();
-                    RenderInExistingBatch(_sharedRenderBatchBuilder, nextComponentIdToRender);
+                    RenderInExistingBatch(nextToRender);
                 }
 
-                UpdateDisplay(_sharedRenderBatchBuilder.ToBatch());
+                UpdateDisplay(_batchBuilder.ToBatch());
             }
             finally
             {
-                RemoveEventHandlerIds(_sharedRenderBatchBuilder.DisposedEventHandlerIds.ToRange());
+                RemoveEventHandlerIds(_batchBuilder.DisposedEventHandlerIds.ToRange());
-                _sharedRenderBatchBuilder.Clear();
+                _batchBuilder.Clear();
-                Interlocked.Exchange(ref _renderBatchLock, 0);
+                _isBatchInProgress = false;
             }
         }
 
-        private void RenderInExistingBatch(RenderBatchBuilder batchBuilder, int componentId)
+        private void RenderInExistingBatch(RenderQueueEntry renderQueueEntry)
         {
-            GetRequiredComponentState(componentId).RenderIntoBatch(batchBuilder);
+            var componentId = renderQueueEntry.ComponentId;
+            GetRequiredComponentState(componentId)
+                .RenderIntoBatch(_batchBuilder, renderQueueEntry.RenderAction);
 
             // Process disposal queue now in case it causes further component renders to be enqueued
-            while (batchBuilder.ComponentDisposalQueue.Count > 0)
+            while (_batchBuilder.ComponentDisposalQueue.Count > 0)
             {
-                var disposeComponentId = batchBuilder.ComponentDisposalQueue.Dequeue();
+                var disposeComponentId = _batchBuilder.ComponentDisposalQueue.Dequeue();
-                GetRequiredComponentState(disposeComponentId).DisposeInBatch(batchBuilder);
+                GetRequiredComponentState(disposeComponentId).DisposeInBatch(_batchBuilder);
                 _componentStateById.Remove(disposeComponentId);
-                batchBuilder.DisposedComponentIds.Append(disposeComponentId);
+                _batchBuilder.DisposedComponentIds.Append(disposeComponentId);
             }
         }
 

test/Microsoft.AspNetCore.Blazor.Build.Test/RazorCompilerTest.cs

@@ -484,11 +484,6 @@ namespace Microsoft.AspNetCore.Blazor.Build.Test
             public void SetParameters(ParameterCollection parameters)
             {
             }
-
-            public void BuildRenderTree(RenderTreeBuilder builder)
-            {
-                builder.AddText(0, $"Hello from {nameof(TestComponent)}");
-            }
         }
     }
 }

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

@@ -10,8 +10,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
     {
         private RenderHandle _renderHandle;
 
-        public abstract void BuildRenderTree(RenderTreeBuilder builder);
-
         public void Init(RenderHandle renderHandle)
         {
             _renderHandle = renderHandle;
@@ -24,6 +22,8 @@ namespace Microsoft.AspNetCore.Blazor.Test
         }
 
         public void TriggerRender()
-            => _renderHandle.Render();
+            => _renderHandle.Render(BuildRenderTree);
+
+        protected abstract void BuildRenderTree(RenderTreeBuilder builder);
     }
 }

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

@@ -109,9 +109,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
 
             public void SetParameters(ParameterCollection parameters)
                 => throw new NotImplementedException();
-
-            public void BuildRenderTree(RenderTreeBuilder builder)
-                => throw new NotImplementedException();
         }
     }
 }

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

@@ -295,9 +295,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
 
             public void SetParameters(ParameterCollection parameters)
                 => throw new NotImplementedException();
-
-            public void BuildRenderTree(RenderTreeBuilder builder)
-                => throw new NotImplementedException();
         }
 
         private class TestRenderer : Renderer

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

@@ -775,7 +775,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             {
                 parameters.AssignToProperties(this);
             }
-            public void BuildRenderTree(RenderTreeBuilder builder) { }
         }
 
         private class FakeComponent2 : IComponent
@@ -787,11 +786,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public void SetParameters(ParameterCollection parameters)
             {
             }
-
-            public void BuildRenderTree(RenderTreeBuilder builder)
-            {
-                builder.AddText(100, $"Hello from {nameof(FakeComponent2)}");
-            }
         }
 
         private class DisposableComponent : IComponent, IDisposable
@@ -802,8 +796,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public void Init(RenderHandle renderHandle) { }
 
             public void SetParameters(ParameterCollection parameters) { }
-
-            public void BuildRenderTree(RenderTreeBuilder builder) { }
         }
 
         private class NonDisposableComponent : IComponent
@@ -811,8 +803,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public void Init(RenderHandle renderHandle) { }
 
             public void SetParameters(ParameterCollection parameters) { }
-
-            public void BuildRenderTree(RenderTreeBuilder builder) { }
         }
 
         private static void AssertEdit(

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

@@ -803,20 +803,17 @@ namespace Microsoft.AspNetCore.Blazor.Test
             }
 
             public void SetParameters(ParameterCollection parameters)
-                => _renderHandle.Render();
+                => TriggerRender();
-
-            public void BuildRenderTree(RenderTreeBuilder builder)
-                => _renderAction(builder);
 
             public void TriggerRender()
-                => _renderHandle.Render();
+                => _renderHandle.Render(_renderAction);
         }
 
         private class MessageComponent : AutoRenderComponent
         {
             public string Message { get; set; }
 
-            public override void BuildRenderTree(RenderTreeBuilder builder)
+            protected override void BuildRenderTree(RenderTreeBuilder builder)
             {
                 builder.AddText(0, Message);
             }
@@ -828,10 +825,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public string StringProperty { get; set; }
             public object ObjectProperty { get; set; }
 
-            public void BuildRenderTree(RenderTreeBuilder builder)
-            {
-            }
-
             public void Init(RenderHandle renderHandle)
             {
             }
@@ -845,7 +838,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public UIEventHandler Handler { get; set; }
             public bool SkipElement { get; set; }
 
-            public override void BuildRenderTree(RenderTreeBuilder builder)
+            protected override void BuildRenderTree(RenderTreeBuilder builder)
             {
                 builder.OpenElement(0, "grandparent");
                 if (!SkipElement)
@@ -871,7 +864,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public bool IncludeChild { get; set; }
             public IDictionary<string, object> ChildParameters { get; set; }
 
-            public override void BuildRenderTree(RenderTreeBuilder builder)
+            protected override void BuildRenderTree(RenderTreeBuilder builder)
             {
                 builder.AddText(0, "Parent here");
                 
@@ -896,7 +889,7 @@ namespace Microsoft.AspNetCore.Blazor.Test
             public TestComponent Parent { get; set; }
             private bool _isFirstTime = true;
 
-            public override void BuildRenderTree(RenderTreeBuilder builder)
+            protected override void BuildRenderTree(RenderTreeBuilder builder)
             {
                 if (_isFirstTime) // Don't want an infinite loop
                 {
@@ -913,9 +906,6 @@ namespace Microsoft.AspNetCore.Blazor.Test
             private readonly List<RenderHandle> _renderHandles
                 = new List<RenderHandle>();
 
-            public void BuildRenderTree(RenderTreeBuilder builder)
-                => builder.AddText(0, $"Hello from {nameof(MultiRendererComponent)}");
-
             public void Init(RenderHandle renderHandle)
                 => _renderHandles.Add(renderHandle);
 
@@ -927,7 +917,10 @@ namespace Microsoft.AspNetCore.Blazor.Test
             {
                 foreach (var renderHandle in _renderHandles)
                 {
-                    renderHandle.Render();
+                    renderHandle.Render(builder =>
+                    {
+                        builder.AddText(0, $"Hello from {nameof(MultiRendererComponent)}");
+                    });
                 }
             }
         }