Changes per PR comments

Author: pranavkm

src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs

@@ -49,7 +49,7 @@ public void AddComponent<TComponent>(string domElementSelector)
         /// </summary>
         /// <param name="componentType">The type of the component.</param>
         /// <param name="domElementSelector">A CSS selector that uniquely identifies a DOM element.</param>
-        public void AddComponent(Type componentType, string domElementSelector)
+        public async void AddComponent(Type componentType, string domElementSelector)
         {
             var component = InstantiateComponent(componentType);
             var componentId = AssignRootComponentId(component);
@@ -65,7 +65,8 @@ public void AddComponent(Type componentType, string domElementSelector)
                 domElementSelector,
                 componentId);
 
-            RenderRootComponent(componentId);
+            // Dispatch the rendering. In the most common cases, this will finish synchronously
+            await RenderRootComponentAsync(componentId);
         }
 
         /// <inheritdoc />
@@ -94,7 +95,7 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch)
         }
 
         /// <inheritdoc />
-        protected override bool HandleException(Exception exception)
+        protected override void HandleException(Exception exception)
         {
             Console.Error.WriteLine($"Unhandled exception rendering component:");
             if (exception is AggregateException aggregateException)
@@ -108,8 +109,6 @@ protected override bool HandleException(Exception exception)
             {
                 Console.Error.WriteLine(exception);
             }
-
-            return true;
         }
     }
 }

src/Components/Blazor/Build/test/RazorIntegrationTestBase.cs

@@ -443,6 +443,11 @@ public TestRenderer() : base(new TestServiceProvider(), CreateDefaultDispatcher(
             public void AttachComponent(IComponent component)
                 => AssignRootComponentId(component);
 
+            protected override void HandleException(Exception exception)
+            {
+                throw new NotImplementedException();
+            }
+
             protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
             {
                 LatestBatchReferenceFrames = renderBatch.ReferenceFrames.ToArray();

src/Components/Browser.JS/src/package-lock.json

@@ -91,6 +91,11 @@ public FakeRenderer()
             {
             }
 
+            protected override void HandleException(Exception exception)
+            {
+                throw new NotImplementedException();
+            }
+
             protected override Task UpdateDisplayAsync(in RenderBatch renderBatch)
                 => Task.CompletedTask;
         }

src/Components/Components/perf/RenderTreeDiffBuilderBenchmark.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Components.RenderTree;
 
@@ -19,7 +20,6 @@ public abstract class Renderer : IDisposable
         private readonly Dictionary<int, ComponentState> _componentStateById = new Dictionary<int, ComponentState>();
         private readonly RenderBatchBuilder _batchBuilder = new RenderBatchBuilder();
         private readonly Dictionary<int, EventHandlerInvoker> _eventBindings = new Dictionary<int, EventHandlerInvoker>();
-        private IDispatcher _dispatcher;
 
         private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it
         private bool _isBatchInProgress;
@@ -33,15 +33,15 @@ public event UnhandledExceptionEventHandler UnhandledSynchronizationException
         {
             add
             {
-                if (!(_dispatcher is RendererSynchronizationContext rendererSynchronizationContext))
+                if (!(Dispatcher is RendererSynchronizationContext rendererSynchronizationContext))
                 {
                     return;
                 }
                 rendererSynchronizationContext.UnhandledException += value;
             }
             remove
             {
-                if (!(_dispatcher is RendererSynchronizationContext rendererSynchronizationContext))
+                if (!(Dispatcher is RendererSynchronizationContext rendererSynchronizationContext))
                 {
                     return;
                 }
@@ -65,9 +65,14 @@ public Renderer(IServiceProvider serviceProvider)
         /// <param name="dispatcher">The <see cref="IDispatcher"/> to be for invoking user actions into the <see cref="Renderer"/> context.</param>
         public Renderer(IServiceProvider serviceProvider, IDispatcher dispatcher) : this(serviceProvider)
         {
-            _dispatcher = dispatcher;
+            Dispatcher = dispatcher;
         }
 
+        /// <summary>
+        /// Gets the <see cref="IDispatcher"/>.
+        /// </summary>
+        protected IDispatcher Dispatcher { get; }
+
         /// <summary>
         /// Creates an <see cref="IDispatcher"/> that can be used with one or more <see cref="Renderer"/>.
         /// </summary>
@@ -130,6 +135,11 @@ protected Task RenderRootComponentAsync(int componentId)
         /// </remarks>
         protected async Task RenderRootComponentAsync(int componentId, ParameterCollection initialParameters)
         {
+            if (Interlocked.CompareExchange(ref _pendingTasks, new List<Task>(), null) != null)
+            {
+                throw new InvalidOperationException("There is an ongoing rendering in progress.");
+            }
+
             // During the rendering process we keep a list of components performing work in _pendingTasks.
             // _renderer.AddToPendingTasks will be called by ComponentState.SetDirectParameters to add the
             // the Task produced by Component.SetParametersAsync to _pendingTasks in order to track the
@@ -176,8 +186,8 @@ private async Task ProcessAsynchronousWork()
 
                 try
                 {
-                await pendingWork;
-        }
+                    await pendingWork;
+                }
                 catch when (!pendingWork.IsCanceled)
                 {
                     // await will unwrap an AggregateException and throw exactly one inner exception.
@@ -251,13 +261,13 @@ public async Task DispatchEventAsync(int componentId, int eventHandlerId, UIEven
         public virtual Task Invoke(Action workItem)
         {
             // This is for example when we run on a system with a single thread, like WebAssembly.
-            if (_dispatcher == null)
+            if (Dispatcher == null)
             {
                 workItem();
                 return Task.CompletedTask;
             }
 
-            if (SynchronizationContext.Current == _dispatcher)
+            if (SynchronizationContext.Current == Dispatcher)
             {
                 // This is an optimization for when the dispatcher is also a syncronization context, like in the default case.
                 // No need to dispatch. Avoid deadlock by invoking directly.
@@ -266,7 +276,7 @@ public virtual Task Invoke(Action workItem)
             }
             else
             {
-                return _dispatcher.Invoke(workItem);
+                return Dispatcher.Invoke(workItem);
             }
         }
 
@@ -278,21 +288,21 @@ public virtual Task Invoke(Action workItem)
         public virtual Task InvokeAsync(Func<Task> workItem)
         {
             // This is for example when we run on a system with a single thread, like WebAssembly.
-            if (_dispatcher == null)
+            if (Dispatcher == null)
             {
                 workItem();
                 return Task.CompletedTask;
             }
 
-            if (SynchronizationContext.Current == _dispatcher)
+            if (SynchronizationContext.Current == Dispatcher)
             {
                 // This is an optimization for when the dispatcher is also a syncronization context, like in the default case.
                 // No need to dispatch. Avoid deadlock by invoking directly.
                 return workItem();
             }
             else
             {
-                return _dispatcher.InvokeAsync(workItem);
+                return Dispatcher.InvokeAsync(workItem);
             }
         }
 
@@ -388,7 +398,7 @@ private void EnsureSynchronizationContext()
             // Plus, any other logic that mutates state accessed during rendering also
             // needs not to run concurrently with rendering so should be dispatched to
             // the renderer's sync context.
-            if (_dispatcher is SynchronizationContext synchronizationContext && SynchronizationContext.Current != synchronizationContext)
+            if (Dispatcher is SynchronizationContext synchronizationContext && SynchronizationContext.Current != synchronizationContext)
             {
                 throw new InvalidOperationException(
                     "The current thread is not associated with the renderer's synchronization context. " +
@@ -442,24 +452,32 @@ private async void InvokeRenderCompletedCalls(ArrayRange<RenderTreeDiff> updated
                 var componentState = GetOptionalComponentState(array[i].ComponentId);
                 if (componentState != null)
                 {
-                // The component might be rendered and disposed in the same batch (if its parent
-                // was rendered later in the batch, and removed the child from the tree).
+                    // The component might be rendered and disposed in the same batch (if its parent
+                    // was rendered later in the batch, and removed the child from the tree).
                     var task = componentState.NotifyRenderCompletedAsync();
 
                     // We want to avoid allocations per rendering. Avoid allocating a state machine or an accumulator
                     // unless we absolutely have to.
-                    if (task.IsCompleted || task.IsCanceled)
+                    if (task.IsCompleted)
                     {
-                        // Nothing to do here.
-                        continue;
+                        if (task.Status == TaskStatus.RanToCompletion || task.Status == TaskStatus.Canceled)
+                        {
+                            // Nothing to do here.
+                            continue;
+                        }
+                        else if (task.Status == TaskStatus.Faulted)
+                        {
+                            HandleException(task.Exception);
+                            continue;
+                        }
                     }
 
                     // Either the Task is incomplete or is faulted.
                     // In either case, queue up the task and we can inspect it later.
                     batch = batch ?? new List<Task>();
                     batch.Add(task);
+                }
             }
-        }
 
             if (batch != null)
             {
@@ -538,7 +556,7 @@ protected virtual void Dispose(bool disposing)
                     }
                 }
             }
-            }
+        }
 
         /// <summary>
         /// Releases all resources currently used by this <see cref="Renderer"/> instance.