From 942dcb271fab5a4aa9b5701b9e8ec024e965dddb Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 14:28:39 +0100 Subject: [PATCH 1/8] Revert "In client-side Blazor, prevent recursive event handler invocations" This reverts commit 9c4d46c0e92d6efaaa145fa268bd2ed9718d35ec. --- .../src/RendererRegistryEventDispatcher.cs | 78 +------------------ 1 file changed, 3 insertions(+), 75 deletions(-) diff --git a/src/Components/Browser/src/RendererRegistryEventDispatcher.cs b/src/Components/Browser/src/RendererRegistryEventDispatcher.cs index 1ddc01f33590..469a0dcae022 100644 --- a/src/Components/Browser/src/RendererRegistryEventDispatcher.cs +++ b/src/Components/Browser/src/RendererRegistryEventDispatcher.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.JSInterop; @@ -14,10 +13,6 @@ namespace Microsoft.AspNetCore.Components.Browser /// public static class RendererRegistryEventDispatcher { - private static bool isDispatchingEvent; - private static Queue deferredIncomingEvents - = new Queue(); - /// /// For framework use only. /// @@ -25,62 +20,9 @@ private static Queue deferredIncomingEvents public static Task DispatchEvent( BrowserEventDescriptor eventDescriptor, string eventArgsJson) { - // Be sure we only run one event handler at once. Although they couldn't run - // simultaneously anyway (there's only one thread), they could run nested on - // the stack if somehow one event handler triggers another event synchronously. - // We need event handlers not to overlap because (a) that's consistent with - // server-side Blazor which uses a sync context, and (b) the rendering logic - // relies completely on the idea that within a given scope it's only building - // or processing one batch at a time. - // - // The only currently known case where this makes a difference is in the E2E - // tests in ReorderingFocusComponent, where we hit what seems like a Chrome bug - // where mutating the DOM cause an element's "change" to fire while its "input" - // handler is still running (i.e., nested on the stack) -- this doesn't happen - // in Firefox. Possibly a future version of Chrome may fix this, but even then, - // it's conceivable that DOM mutation events could trigger this too. - - if (isDispatchingEvent) - { - var info = new IncomingEventInfo(eventDescriptor, eventArgsJson); - deferredIncomingEvents.Enqueue(info); - return info.TaskCompletionSource.Task; - } - else - { - isDispatchingEvent = true; - try - { - var eventArgs = ParseEventArgsJson(eventDescriptor.EventArgsType, eventArgsJson); - var renderer = RendererRegistry.Current.Find(eventDescriptor.BrowserRendererId); - return renderer.DispatchEventAsync(eventDescriptor.EventHandlerId, eventArgs); - } - finally - { - isDispatchingEvent = false; - if (deferredIncomingEvents.Count > 0) - { - ProcessNextDeferredEvent(); - } - } - } - } - - private static void ProcessNextDeferredEvent() - { - var info = deferredIncomingEvents.Dequeue(); - var task = DispatchEvent(info.EventDescriptor, info.EventArgsJson); - task.ContinueWith(_ => - { - if (task.Exception != null) - { - info.TaskCompletionSource.SetException(task.Exception); - } - else - { - info.TaskCompletionSource.SetResult(null); - } - }); + var eventArgs = ParseEventArgsJson(eventDescriptor.EventArgsType, eventArgsJson); + var renderer = RendererRegistry.Current.Find(eventDescriptor.BrowserRendererId); + return renderer.DispatchEventAsync(eventDescriptor.EventHandlerId, eventArgs); } private static UIEventArgs ParseEventArgsJson(string eventArgsType, string eventArgsJson) @@ -136,19 +78,5 @@ public class BrowserEventDescriptor /// public string EventArgsType { get; set; } } - - readonly struct IncomingEventInfo - { - public readonly BrowserEventDescriptor EventDescriptor; - public readonly string EventArgsJson; - public readonly TaskCompletionSource TaskCompletionSource; - - public IncomingEventInfo(BrowserEventDescriptor eventDescriptor, string eventArgsJson) - { - EventDescriptor = eventDescriptor; - EventArgsJson = eventArgsJson; - TaskCompletionSource = new TaskCompletionSource(); - } - } } } From 9766e26474ac567b0fae8bebd9224fdbe97f15d4 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 15:20:52 +0100 Subject: [PATCH 2/8] Move event-deferral logic into WebAssembly-specific code --- .../src/Rendering/WebAssemblyRenderer.cs | 78 +++++++++++++++++++ .../Components/src/Rendering/Renderer.cs | 2 +- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs b/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs index 3e05f10d5faa..8fc099df2a1f 100644 --- a/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Threading.Tasks; using Microsoft.AspNetCore.Blazor.Services; using Microsoft.AspNetCore.Components; @@ -18,6 +19,9 @@ public class WebAssemblyRenderer : Renderer { private readonly int _webAssemblyRendererId; + private bool isDispatchingEvent; + private Queue deferredIncomingEvents = new Queue(); + /// /// Constructs an instance of . /// @@ -110,5 +114,79 @@ protected override void HandleException(Exception exception) Console.Error.WriteLine(exception); } } + + /// + public override Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArgs) + { + // Be sure we only run one event handler at once. Although they couldn't run + // simultaneously anyway (there's only one thread), they could run nested on + // the stack if somehow one event handler triggers another event synchronously. + // We need event handlers not to overlap because (a) that's consistent with + // server-side Blazor which uses a sync context, and (b) the rendering logic + // relies completely on the idea that within a given scope it's only building + // or processing one batch at a time. + // + // The only currently known case where this makes a difference is in the E2E + // tests in ReorderingFocusComponent, where we hit what seems like a Chrome bug + // where mutating the DOM cause an element's "change" to fire while its "input" + // handler is still running (i.e., nested on the stack) -- this doesn't happen + // in Firefox. Possibly a future version of Chrome may fix this, but even then, + // it's conceivable that DOM mutation events could trigger this too. + + if (isDispatchingEvent) + { + var info = new IncomingEventInfo(eventHandlerId, eventArgs); + deferredIncomingEvents.Enqueue(info); + return info.TaskCompletionSource.Task; + } + else + { + try + { + isDispatchingEvent = true; + return base.DispatchEventAsync(eventHandlerId, eventArgs); + } + finally + { + isDispatchingEvent = false; + + if (deferredIncomingEvents.Count > 0) + { + ProcessNextDeferredEvent(); + } + } + } + } + + private void ProcessNextDeferredEvent() + { + var info = deferredIncomingEvents.Dequeue(); + var task = DispatchEventAsync(info.EventHandlerId, info.EventArgs); + task.ContinueWith(_ => + { + if (task.Exception != null) + { + info.TaskCompletionSource.SetException(task.Exception); + } + else + { + info.TaskCompletionSource.SetResult(null); + } + }); + } + + readonly struct IncomingEventInfo + { + public readonly int EventHandlerId; + public readonly UIEventArgs EventArgs; + public readonly TaskCompletionSource TaskCompletionSource; + + public IncomingEventInfo(int eventHandlerId, UIEventArgs eventArgs) + { + EventHandlerId = eventHandlerId; + EventArgs = eventArgs; + TaskCompletionSource = new TaskCompletionSource(); + } + } } } diff --git a/src/Components/Components/src/Rendering/Renderer.cs b/src/Components/Components/src/Rendering/Renderer.cs index 8a51fc287bf6..88b9f71e09d1 100644 --- a/src/Components/Components/src/Rendering/Renderer.cs +++ b/src/Components/Components/src/Rendering/Renderer.cs @@ -209,7 +209,7 @@ private ComponentState AttachAndInitComponent(IComponent component, int parentCo /// A which will complete once all asynchronous processing related to the event /// has completed. /// - public Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArgs) + public virtual Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArgs) { EnsureSynchronizationContext(); From 098b80cbb103f8ac3dd54d09414dd8894f4b8d7c Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 14:36:02 +0100 Subject: [PATCH 3/8] Add missing Server-side E2E test subclasses --- .../ServerExecutionTests/TestSubclasses.cs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/Components/test/E2ETest/ServerExecutionTests/TestSubclasses.cs b/src/Components/test/E2ETest/ServerExecutionTests/TestSubclasses.cs index 1195b3ccdb18..9265718d5492 100644 --- a/src/Components/test/E2ETest/ServerExecutionTests/TestSubclasses.cs +++ b/src/Components/test/E2ETest/ServerExecutionTests/TestSubclasses.cs @@ -57,4 +57,28 @@ public ServerCascadingValueTest(BrowserFixture browserFixture, ToggleExecutionMo { } } + + public class ServerEventCallbackTest : EventCallbackTest + { + public ServerEventCallbackTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture serverFixture, ITestOutputHelper output) + : base(browserFixture, serverFixture.WithServerExecution(), output) + { + } + } + + public class ServerFormsTest : FormsTest + { + public ServerFormsTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture serverFixture, ITestOutputHelper output) + : base(browserFixture, serverFixture.WithServerExecution(), output) + { + } + } + + public class ServerKeyTest : KeyTest + { + public ServerKeyTest(BrowserFixture browserFixture, ToggleExecutionModeServerFixture serverFixture, ITestOutputHelper output) + : base(browserFixture, serverFixture.WithServerExecution(), output) + { + } + } } From 53f15a2d4f8ef22d609fe5cf4d4eabb9abe42fbd Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 15:02:44 +0100 Subject: [PATCH 4/8] Fix ServerFormTest cases --- .../Components/src/Forms/InputComponents/InputNumber.cs | 4 ++-- src/Components/test/E2ETest/Tests/FormsTest.cs | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Components/Components/src/Forms/InputComponents/InputNumber.cs b/src/Components/Components/src/Forms/InputComponents/InputNumber.cs index 8433454a2ed5..72f0fea0f781 100644 --- a/src/Components/Components/src/Forms/InputComponents/InputNumber.cs +++ b/src/Components/Components/src/Forms/InputComponents/InputNumber.cs @@ -119,7 +119,7 @@ static bool TryParseLong(string value, out T result) static bool TryParseFloat(string value, out T result) { var success = float.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var parsedValue); - if (success) + if (success && !float.IsInfinity(parsedValue)) { result = (T)(object)parsedValue; return true; @@ -134,7 +134,7 @@ static bool TryParseFloat(string value, out T result) static bool TryParseDouble(string value, out T result) { var success = double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var parsedValue); - if (success) + if (success && !double.IsInfinity(parsedValue)) { result = (T)(object)parsedValue; return true; diff --git a/src/Components/test/E2ETest/Tests/FormsTest.cs b/src/Components/test/E2ETest/Tests/FormsTest.cs index 9273f75a6c5e..f4c68a767cab 100644 --- a/src/Components/test/E2ETest/Tests/FormsTest.cs +++ b/src/Components/test/E2ETest/Tests/FormsTest.cs @@ -210,6 +210,7 @@ public void InputDateInteractsWithEditContext_NullableDateTimeOffset() Browser.Equal("modified valid", () => expiryDateInput.GetAttribute("class")); // Can become invalid + expiryDateInput.Clear(); expiryDateInput.SendKeys("111111111"); Browser.Equal("modified invalid", () => expiryDateInput.GetAttribute("class")); Browser.Equal(new[] { "The OptionalExpiryDate field must be a date." }, messagesAccessor); From c3405491fe775feb01ba4648905bba2ffdc5e2ed Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 15:11:06 +0100 Subject: [PATCH 5/8] Fix KeyTest on server --- src/Components/test/E2ETest/Tests/KeyTest.cs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/Components/test/E2ETest/Tests/KeyTest.cs b/src/Components/test/E2ETest/Tests/KeyTest.cs index 32f3ad5e3f69..b7a55196949e 100644 --- a/src/Components/test/E2ETest/Tests/KeyTest.cs +++ b/src/Components/test/E2ETest/Tests/KeyTest.cs @@ -211,7 +211,7 @@ public async Task CanRetainFocusWhileMovingTextBox() { var appElem = MountTestComponent(); Func textboxFinder = () => appElem.FindElement(By.CssSelector(".incomplete-items .item-1 input[type=text]")); - var textToType = "Hello this is a long string that should be typed"; + var textToType = "Hello there!"; var expectedTextTyped = ""; textboxFinder().Clear(); @@ -221,17 +221,16 @@ public async Task CanRetainFocusWhileMovingTextBox() textboxFinder().Click(); while (textToType.Length > 0) { - var nextBlockLength = Math.Min(5, textToType.Length); - var nextBlock = textToType.Substring(0, nextBlockLength); - textToType = textToType.Substring(nextBlockLength); - expectedTextTyped += nextBlock; + var nextChar = textToType.Substring(0, 1); + textToType = textToType.Substring(1); + expectedTextTyped += nextChar; // Send keys to whatever has focus - new Actions(Browser).SendKeys(nextBlock).Perform(); + new Actions(Browser).SendKeys(nextChar).Perform(); Browser.Equal(expectedTextTyped, () => textboxFinder().GetAttribute("value")); // We delay between typings to ensure the events aren't all collapsed into one. - await Task.Delay(100); + await Task.Delay(50); } // Verify that after all this, we can still move the edited item From 539184f615ef06339bd8c1dc38b8bd289bf7fcd0 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 16:34:46 +0100 Subject: [PATCH 6/8] Update ref source --- .../ref/Microsoft.AspNetCore.Components.netstandard2.0.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs index 95c356e520e7..b615df9df1a9 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs @@ -710,7 +710,7 @@ public event System.UnhandledExceptionEventHandler UnhandledSynchronizationExcep protected internal virtual void AddToRenderQueue(int componentId, Microsoft.AspNetCore.Components.RenderFragment renderFragment) { } protected internal int AssignRootComponentId(Microsoft.AspNetCore.Components.IComponent component) { throw null; } public static Microsoft.AspNetCore.Components.Rendering.IDispatcher CreateDefaultDispatcher() { throw null; } - public System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; } + public virtual System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; } public void Dispose() { } protected virtual void Dispose(bool disposing) { } protected abstract void HandleException(System.Exception exception); From 5c1668ecbc6e0da932bfb1157efc74fe36ea9192 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 16:39:54 +0100 Subject: [PATCH 7/8] CR: Replace ContinueWith with await --- .../src/Rendering/WebAssemblyRenderer.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs b/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs index 8fc099df2a1f..71a3a14741cd 100644 --- a/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs +++ b/src/Components/Blazor/Blazor/src/Rendering/WebAssemblyRenderer.cs @@ -158,21 +158,20 @@ public override Task DispatchEventAsync(int eventHandlerId, UIEventArgs eventArg } } - private void ProcessNextDeferredEvent() + private async void ProcessNextDeferredEvent() { var info = deferredIncomingEvents.Dequeue(); - var task = DispatchEventAsync(info.EventHandlerId, info.EventArgs); - task.ContinueWith(_ => + var taskCompletionSource = info.TaskCompletionSource; + + try { - if (task.Exception != null) - { - info.TaskCompletionSource.SetException(task.Exception); - } - else - { - info.TaskCompletionSource.SetResult(null); - } - }); + await DispatchEventAsync(info.EventHandlerId, info.EventArgs); + taskCompletionSource.SetResult(null); + } + catch (Exception ex) + { + taskCompletionSource.SetException(ex); + } } readonly struct IncomingEventInfo From d9ad192083c201799385e5e315caeaa61af62af2 Mon Sep 17 00:00:00 2001 From: Steve Sanderson Date: Thu, 9 May 2019 17:09:14 +0100 Subject: [PATCH 8/8] Update ref assembly more --- .../Blazor/ref/Microsoft.AspNetCore.Blazor.netstandard2.0.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Components/Blazor/Blazor/ref/Microsoft.AspNetCore.Blazor.netstandard2.0.cs b/src/Components/Blazor/Blazor/ref/Microsoft.AspNetCore.Blazor.netstandard2.0.cs index b0df2bf3d5ee..1cf4cca87966 100644 --- a/src/Components/Blazor/Blazor/ref/Microsoft.AspNetCore.Blazor.netstandard2.0.cs +++ b/src/Components/Blazor/Blazor/ref/Microsoft.AspNetCore.Blazor.netstandard2.0.cs @@ -61,6 +61,7 @@ public partial class WebAssemblyRenderer : Microsoft.AspNetCore.Components.Rende public WebAssemblyRenderer(System.IServiceProvider serviceProvider) : base (default(System.IServiceProvider)) { } public System.Threading.Tasks.Task AddComponentAsync(System.Type componentType, string domElementSelector) { throw null; } public System.Threading.Tasks.Task AddComponentAsync(string domElementSelector) where TComponent : Microsoft.AspNetCore.Components.IComponent { throw null; } + public override System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; } protected override void Dispose(bool disposing) { } protected override void HandleException(System.Exception exception) { } protected override System.Threading.Tasks.Task UpdateDisplayAsync(in Microsoft.AspNetCore.Components.Rendering.RenderBatch batch) { throw null; }