From bbec5d58346d24e26fa9e0f57050a52a386258d3 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 8 Apr 2021 11:57:56 +0100 Subject: [PATCH 1/5] POC solution - checking CI passes before refining and adding E2E test --- .../src/Rendering/WebAssemblyRenderer.cs | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs index a6711af411d1..f879ac2f9ff2 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs @@ -22,7 +22,7 @@ internal class WebAssemblyRenderer : Renderer private readonly int _webAssemblyRendererId; private bool isDispatchingEvent; - private Queue deferredIncomingEvents = new Queue(); + private List deferredIncomingEvents = new List(); /// /// Constructs an instance of . @@ -103,7 +103,24 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch) _webAssemblyRendererId, batch); - return Task.CompletedTask; + if (deferredIncomingEvents.Count == 0) + { + // In the vast majority of cases, since the call to update the UI is synchronous, + // we just return a pre-completed task from here. + return Task.CompletedTask; + } + else + { + // However, in the rare case where JS sent us any event notifications that we had to + // defer until later, we behave as if the renderbatch isn't acknowledged until we have at + // least dispatched those event calls. This is to make the WebAssembly behavior more + // consistent with the Server behavior, which receives batch acknowledgements asynchronously + // and they are queued up with any other calls from JS such as event calls. If we didn't + // do this, then the order of execution could be inconsistent with Server, and in fact + // leads to a specific bug: https://github.com/dotnet/aspnetcore/issues/26838 + var lastInQueue = deferredIncomingEvents[deferredIncomingEvents.Count - 1]; + return lastInQueue.EventDispatcherCompletedSource.Task; + } } /// @@ -143,8 +160,8 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev if (isDispatchingEvent) { var info = new IncomingEventInfo(eventHandlerId, eventFieldInfo, eventArgs); - deferredIncomingEvents.Enqueue(info); - return info.TaskCompletionSource.Task; + deferredIncomingEvents.Add(info); + return info.EventHandlerCompletedSource.Task; } else { @@ -170,17 +187,20 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev private async Task ProcessNextDeferredEventAsync() { - var info = deferredIncomingEvents.Dequeue(); - var taskCompletionSource = info.TaskCompletionSource; + var info = deferredIncomingEvents[0]; + deferredIncomingEvents.RemoveAt(0); try { - await DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs); - taskCompletionSource.SetResult(); + var eventHandlerTask = DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs); + info.EventDispatcherCompletedSource.SetResult(); + await eventHandlerTask; + info.EventHandlerCompletedSource.SetResult(); } catch (Exception ex) { - taskCompletionSource.SetException(ex); + info.EventDispatcherCompletedSource.TrySetResult(); // The fact that it was dispatched is all this cares about, not the result + info.EventHandlerCompletedSource.SetException(ex); } } @@ -189,14 +209,16 @@ readonly struct IncomingEventInfo public readonly ulong EventHandlerId; public readonly EventFieldInfo? EventFieldInfo; public readonly EventArgs EventArgs; - public readonly TaskCompletionSource TaskCompletionSource; + public readonly TaskCompletionSource EventDispatcherCompletedSource; + public readonly TaskCompletionSource EventHandlerCompletedSource; public IncomingEventInfo(ulong eventHandlerId, EventFieldInfo? eventFieldInfo, EventArgs eventArgs) { EventHandlerId = eventHandlerId; EventFieldInfo = eventFieldInfo; EventArgs = eventArgs; - TaskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + EventDispatcherCompletedSource = new TaskCompletionSource(); + EventHandlerCompletedSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); } } From abe5a23a7695b523fd9fd2b6ff42f2214a872b68 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 8 Apr 2021 14:23:48 +0100 Subject: [PATCH 2/5] Cleaner implementation --- .../src/Rendering/WebAssemblyRenderer.cs | 59 +++++++++++++------ 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs index f879ac2f9ff2..3eadd15c0a06 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs @@ -22,7 +22,7 @@ internal class WebAssemblyRenderer : Renderer private readonly int _webAssemblyRendererId; private bool isDispatchingEvent; - private List deferredIncomingEvents = new List(); + private QueueWithLast deferredIncomingEvents = new(); /// /// Constructs an instance of . @@ -118,8 +118,7 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch) // and they are queued up with any other calls from JS such as event calls. If we didn't // do this, then the order of execution could be inconsistent with Server, and in fact // leads to a specific bug: https://github.com/dotnet/aspnetcore/issues/26838 - var lastInQueue = deferredIncomingEvents[deferredIncomingEvents.Count - 1]; - return lastInQueue.EventDispatcherCompletedSource.Task; + return deferredIncomingEvents.Last.DispatcherCompletionSource.Task; } } @@ -160,8 +159,8 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev if (isDispatchingEvent) { var info = new IncomingEventInfo(eventHandlerId, eventFieldInfo, eventArgs); - deferredIncomingEvents.Add(info); - return info.EventHandlerCompletedSource.Task; + deferredIncomingEvents.Enqueue(info); + return info.HandlerCompletionSource.Task; } else { @@ -187,20 +186,21 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev private async Task ProcessNextDeferredEventAsync() { - var info = deferredIncomingEvents[0]; - deferredIncomingEvents.RemoveAt(0); + var info = deferredIncomingEvents.Dequeue(); try { - var eventHandlerTask = DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs); - info.EventDispatcherCompletedSource.SetResult(); - await eventHandlerTask; - info.EventHandlerCompletedSource.SetResult(); + var handlerTask = DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs); + info.DispatcherCompletionSource.SetResult(); + await handlerTask; + info.HandlerCompletionSource.SetResult(); } catch (Exception ex) { - info.EventDispatcherCompletedSource.TrySetResult(); // The fact that it was dispatched is all this cares about, not the result - info.EventHandlerCompletedSource.SetException(ex); + // Even if the handler threw synchronously, we at least started processing, so always complete successfully + info.DispatcherCompletionSource.TrySetResult(); + + info.HandlerCompletionSource.SetException(ex); } } @@ -209,16 +209,16 @@ readonly struct IncomingEventInfo public readonly ulong EventHandlerId; public readonly EventFieldInfo? EventFieldInfo; public readonly EventArgs EventArgs; - public readonly TaskCompletionSource EventDispatcherCompletedSource; - public readonly TaskCompletionSource EventHandlerCompletedSource; + public readonly TaskCompletionSource DispatcherCompletionSource; // Notifies when we start processing the event + public readonly TaskCompletionSource HandlerCompletionSource; // Notifies when we finish processing the event public IncomingEventInfo(ulong eventHandlerId, EventFieldInfo? eventFieldInfo, EventArgs eventArgs) { EventHandlerId = eventHandlerId; EventFieldInfo = eventFieldInfo; EventArgs = eventArgs; - EventDispatcherCompletedSource = new TaskCompletionSource(); - EventHandlerCompletedSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + DispatcherCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + HandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); } } @@ -247,5 +247,30 @@ public static void UnhandledExceptionRenderingComponent(ILogger logger, Exceptio exception); } } + + private class QueueWithLast + { + private readonly Queue _items = new(); + + public int Count => _items.Count; + + public T? Last { get; private set; } + + public T Dequeue() + { + if (_items.Count == 1) + { + Last = default; + } + + return _items.Dequeue(); + } + + public void Enqueue(T item) + { + Last = item; + _items.Enqueue(item); + } + } } } From 8552271c3c062697f9c6d88eef7784254e8e0c74 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 8 Apr 2021 14:26:09 +0100 Subject: [PATCH 3/5] Clearer naming --- .../src/Rendering/WebAssemblyRenderer.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs index 3eadd15c0a06..deab87db24ff 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs @@ -118,7 +118,7 @@ protected override Task UpdateDisplayAsync(in RenderBatch batch) // and they are queued up with any other calls from JS such as event calls. If we didn't // do this, then the order of execution could be inconsistent with Server, and in fact // leads to a specific bug: https://github.com/dotnet/aspnetcore/issues/26838 - return deferredIncomingEvents.Last.DispatcherCompletionSource.Task; + return deferredIncomingEvents.Last.StartHandlerCompletionSource.Task; } } @@ -160,7 +160,7 @@ public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo? ev { var info = new IncomingEventInfo(eventHandlerId, eventFieldInfo, eventArgs); deferredIncomingEvents.Enqueue(info); - return info.HandlerCompletionSource.Task; + return info.FinishHandlerCompletionSource.Task; } else { @@ -191,16 +191,16 @@ private async Task ProcessNextDeferredEventAsync() try { var handlerTask = DispatchEventAsync(info.EventHandlerId, info.EventFieldInfo, info.EventArgs); - info.DispatcherCompletionSource.SetResult(); + info.StartHandlerCompletionSource.SetResult(); await handlerTask; - info.HandlerCompletionSource.SetResult(); + info.FinishHandlerCompletionSource.SetResult(); } catch (Exception ex) { // Even if the handler threw synchronously, we at least started processing, so always complete successfully - info.DispatcherCompletionSource.TrySetResult(); + info.StartHandlerCompletionSource.TrySetResult(); - info.HandlerCompletionSource.SetException(ex); + info.FinishHandlerCompletionSource.SetException(ex); } } @@ -209,16 +209,16 @@ readonly struct IncomingEventInfo public readonly ulong EventHandlerId; public readonly EventFieldInfo? EventFieldInfo; public readonly EventArgs EventArgs; - public readonly TaskCompletionSource DispatcherCompletionSource; // Notifies when we start processing the event - public readonly TaskCompletionSource HandlerCompletionSource; // Notifies when we finish processing the event + public readonly TaskCompletionSource StartHandlerCompletionSource; + public readonly TaskCompletionSource FinishHandlerCompletionSource; public IncomingEventInfo(ulong eventHandlerId, EventFieldInfo? eventFieldInfo, EventArgs eventArgs) { EventHandlerId = eventHandlerId; EventFieldInfo = eventFieldInfo; EventArgs = eventArgs; - DispatcherCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - HandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + StartHandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + FinishHandlerCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); } } From e1ea760ee9e772a22b5f9d9e9fc04a47110bd3fa Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 8 Apr 2021 14:30:40 +0100 Subject: [PATCH 4/5] Tiny cleanup --- .../WebAssembly/src/Rendering/WebAssemblyRenderer.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs index deab87db24ff..b84feae77b02 100644 --- a/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/WebAssembly/WebAssembly/src/Rendering/WebAssemblyRenderer.cs @@ -20,9 +20,9 @@ internal class WebAssemblyRenderer : Renderer { private readonly ILogger _logger; private readonly int _webAssemblyRendererId; + private readonly QueueWithLast deferredIncomingEvents = new(); private bool isDispatchingEvent; - private QueueWithLast deferredIncomingEvents = new(); /// /// Constructs an instance of . From 22ab648bf9db7b7249971326a4aa416327a33f67 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 8 Apr 2021 14:45:26 +0100 Subject: [PATCH 5/5] Add E2E test --- .../test/E2ETest/Tests/EventTest.cs | 11 +++++++ .../BasicTestApp/FocusEventComponent.razor | 31 +++++++++++++++---- 2 files changed, 36 insertions(+), 6 deletions(-) diff --git a/src/Components/test/E2ETest/Tests/EventTest.cs b/src/Components/test/E2ETest/Tests/EventTest.cs index 9cbaea5c6fb1..9b2e68aeb3c3 100644 --- a/src/Components/test/E2ETest/Tests/EventTest.cs +++ b/src/Components/test/E2ETest/Tests/EventTest.cs @@ -50,6 +50,17 @@ public void FocusEvents_CanTrigger() Browser.Equal("onfocus,onfocusin,onblur,onfocusout,", () => output.Text); } + [Fact] + public void FocusEvents_CanReceiveBlurCausedByElementRemoval() + { + // Represents https://github.com/dotnet/aspnetcore/issues/26838 + + Browser.MountTestComponent(); + + Browser.FindElement(By.Id("button-that-disappears")).Click(); + Browser.Equal("True", () => Browser.FindElement(By.Id("button-received-focus-out")).Text); + } + [Fact] public void MouseOverAndMouseOut_CanTrigger() { diff --git a/src/Components/test/testassets/BasicTestApp/FocusEventComponent.razor b/src/Components/test/testassets/BasicTestApp/FocusEventComponent.razor index 3af7b37e754e..3628a6ed7980 100644 --- a/src/Components/test/testassets/BasicTestApp/FocusEventComponent.razor +++ b/src/Components/test/testassets/BasicTestApp/FocusEventComponent.razor @@ -3,7 +3,7 @@

Focus and activation

- Input: + Input:

Output: @message @@ -12,40 +12,59 @@

+

+ A button that disappears when clicked: + @if (showButtonThatDisappearsWhenClicked) + { + + } + + Received focus out: @buttonReceivedFocusOut +

+

Another input (to distract you)

@code { - + bool showButtonThatDisappearsWhenClicked = true; + bool buttonReceivedFocusOut; string message; void OnFocus(FocusEventArgs e) { message += "onfocus,"; - StateHasChanged(); } void OnBlur(FocusEventArgs e) { message += "onblur,"; - StateHasChanged(); } void OnFocusIn(FocusEventArgs e) { message += "onfocusin,"; - StateHasChanged(); } void OnFocusOut(FocusEventArgs e) { message += "onfocusout,"; - StateHasChanged(); } void Clear() { message = string.Empty; } + + void MakeButtonDisappear() + { + showButtonThatDisappearsWhenClicked = false; + } + + void DisappearingButtonFocusOut() + { + buttonReceivedFocusOut = true; + } }