Skip to content

Commit

Permalink
Store event handler IDs as longs. Fixes #12058 (#12305)
Browse files Browse the repository at this point in the history
* Change event handler IDs to be longs

* Update unit tests

* Enable detailed errors for test app

* CR: Explicitly check if we exceed Number.MAX_SAFE_INTEGER

* Update ref assemblies
  • Loading branch information
SteveSandersonMS authored Jul 18, 2019
1 parent 010ffe6 commit 4e04b81
Show file tree
Hide file tree
Showing 27 changed files with 181 additions and 120 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public partial class WebAssemblyRenderer : Microsoft.AspNetCore.Components.Rende
public override Microsoft.AspNetCore.Components.Dispatcher Dispatcher { get { throw null; } }
public System.Threading.Tasks.Task AddComponentAsync(System.Type componentType, string domElementSelector) { throw null; }
public System.Threading.Tasks.Task AddComponentAsync<TComponent>(string domElementSelector) where TComponent : Microsoft.AspNetCore.Components.IComponent { throw null; }
public override System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo eventFieldInfo, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; }
public override System.Threading.Tasks.Task DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo eventFieldInfo, 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; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ protected override void HandleException(Exception exception)
}

/// <inheritdoc />
public override Task DispatchEventAsync(int eventHandlerId, EventFieldInfo eventFieldInfo, UIEventArgs eventArgs)
public override Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo eventFieldInfo, 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
Expand Down Expand Up @@ -183,12 +183,12 @@ private async Task ProcessNextDeferredEventAsync()

readonly struct IncomingEventInfo
{
public readonly int EventHandlerId;
public readonly ulong EventHandlerId;
public readonly EventFieldInfo EventFieldInfo;
public readonly UIEventArgs EventArgs;
public readonly TaskCompletionSource<object> TaskCompletionSource;

public IncomingEventInfo(int eventHandlerId, EventFieldInfo eventFieldInfo, UIEventArgs eventArgs)
public IncomingEventInfo(ulong eventHandlerId, EventFieldInfo eventFieldInfo, UIEventArgs eventArgs)
{
EventHandlerId = eventHandlerId;
EventFieldInfo = eventFieldInfo;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -724,7 +724,7 @@ public readonly partial struct RenderBatch
{
private readonly object _dummy;
public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<int> DisposedComponentIDs { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<int> DisposedEventHandlerIDs { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<ulong> DisposedEventHandlerIDs { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame> ReferenceFrames { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.RenderTreeDiff> UpdatedComponents { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
}
Expand All @@ -735,7 +735,7 @@ public Renderer(System.IServiceProvider serviceProvider, Microsoft.Extensions.Lo
public event System.UnhandledExceptionEventHandler UnhandledSynchronizationException { add { } remove { } }
protected internal virtual void AddToRenderQueue(int componentId, Microsoft.AspNetCore.Components.RenderFragment renderFragment) { }
protected internal int AssignRootComponentId(Microsoft.AspNetCore.Components.IComponent component) { throw null; }
public virtual System.Threading.Tasks.Task DispatchEventAsync(int eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo fieldInfo, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; }
public virtual System.Threading.Tasks.Task DispatchEventAsync(ulong eventHandlerId, Microsoft.AspNetCore.Components.Rendering.EventFieldInfo fieldInfo, Microsoft.AspNetCore.Components.UIEventArgs eventArgs) { throw null; }
public void Dispose() { }
protected virtual void Dispose(bool disposing) { }
protected abstract void HandleException(System.Exception exception);
Expand Down
8 changes: 4 additions & 4 deletions src/Components/Components/src/RenderTree/RenderTreeFrame.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Components.RenderTree
/// <summary>
/// Represents an entry in a tree of user interface (UI) items.
/// </summary>
[StructLayout(LayoutKind.Explicit)]
[StructLayout(LayoutKind.Explicit, Pack = 4)]
public readonly struct RenderTreeFrame
{
// Note that the struct layout has to be valid in both 32-bit and 64-bit runtime platforms,
Expand Down Expand Up @@ -90,7 +90,7 @@ public readonly struct RenderTreeFrame
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Attribute"/>
/// gets the ID of the corresponding event handler, if any.
/// </summary>
[FieldOffset(8)] public readonly int AttributeEventHandlerId;
[FieldOffset(8)] public readonly ulong AttributeEventHandlerId;

/// <summary>
/// If the <see cref="FrameType"/> property equals <see cref="RenderTreeFrameType.Attribute"/>,
Expand Down Expand Up @@ -267,7 +267,7 @@ private RenderTreeFrame(int sequence, bool isMarkup, string textOrMarkup)
}

// Attribute constructor
private RenderTreeFrame(int sequence, string attributeName, object attributeValue, int attributeEventHandlerId, string attributeEventUpdatesAttributeName)
private RenderTreeFrame(int sequence, string attributeName, object attributeValue, ulong attributeEventHandlerId, string attributeEventUpdatesAttributeName)
: this()
{
FrameType = RenderTreeFrameType.Attribute;
Expand Down Expand Up @@ -337,7 +337,7 @@ internal RenderTreeFrame WithAttributeSequence(int sequence)
internal RenderTreeFrame WithComponent(ComponentState componentState)
=> new RenderTreeFrame(Sequence, componentSubtreeLength: ComponentSubtreeLength, ComponentType, componentState, ComponentKey);

internal RenderTreeFrame WithAttributeEventHandlerId(int eventHandlerId)
internal RenderTreeFrame WithAttributeEventHandlerId(ulong eventHandlerId)
=> new RenderTreeFrame(Sequence, attributeName: AttributeName, AttributeValue, eventHandlerId, AttributeEventUpdatesAttributeName);

internal RenderTreeFrame WithAttributeValue(object attributeValue)
Expand Down
6 changes: 3 additions & 3 deletions src/Components/Components/src/Rendering/RenderBatch.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright (c) .NET Foundation. All rights reserved.
// 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.Components.RenderTree;
Expand Down Expand Up @@ -30,13 +30,13 @@ public readonly struct RenderBatch
/// <summary>
/// Gets the IDs of the event handlers that were disposed.
/// </summary>
public ArrayRange<int> DisposedEventHandlerIDs { get; }
public ArrayRange<ulong> DisposedEventHandlerIDs { get; }

internal RenderBatch(
ArrayRange<RenderTreeDiff> updatedComponents,
ArrayRange<RenderTreeFrame> referenceFrames,
ArrayRange<int> disposedComponentIDs,
ArrayRange<int> disposedEventHandlerIDs)
ArrayRange<ulong> disposedEventHandlerIDs)
{
UpdatedComponents = updatedComponents;
ReferenceFrames = referenceFrames;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal class RenderBatchBuilder : IDisposable
// Primary result data
public ArrayBuilder<RenderTreeDiff> UpdatedComponentDiffs { get; } = new ArrayBuilder<RenderTreeDiff>();
public ArrayBuilder<int> DisposedComponentIds { get; } = new ArrayBuilder<int>();
public ArrayBuilder<int> DisposedEventHandlerIds { get; } = new ArrayBuilder<int>();
public ArrayBuilder<ulong> DisposedEventHandlerIds { get; } = new ArrayBuilder<ulong>();

// Buffers referenced by UpdatedComponentDiffs
public ArrayBuilder<RenderTreeEdit> EditsBuffer { get; } = new ArrayBuilder<RenderTreeEdit>(64);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ namespace Microsoft.AspNetCore.Components.Rendering
{
internal class RenderTreeUpdater
{
public static void UpdateToMatchClientState(RenderTreeBuilder renderTreeBuilder, int eventHandlerId, object newFieldValue)
public static void UpdateToMatchClientState(RenderTreeBuilder renderTreeBuilder, ulong eventHandlerId, object newFieldValue)
{
// We only allow the client to supply string or bool currently, since those are the only kinds of
// values we output on attributes that go to the client
Expand Down
6 changes: 3 additions & 3 deletions src/Components/Components/src/Rendering/Renderer.Log.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ internal static class Log
private static readonly Action<ILogger, int, Type, Exception> _disposingComponent =
LoggerMessage.Define<int, Type>(LogLevel.Debug, new EventId(4, "DisposingComponent"), "Disposing component {ComponentId} of type {ComponentType}");

private static readonly Action<ILogger, int, string, Exception> _handlingEvent =
LoggerMessage.Define<int, string>(LogLevel.Debug, new EventId(5, "HandlingEvent"), "Handling event {EventId} of type '{EventType}'");
private static readonly Action<ILogger, ulong, string, Exception> _handlingEvent =
LoggerMessage.Define<ulong, string>(LogLevel.Debug, new EventId(5, "HandlingEvent"), "Handling event {EventId} of type '{EventType}'");

public static void InitializingComponent(ILogger logger, ComponentState componentState, ComponentState parentComponentState)
{
Expand Down Expand Up @@ -56,7 +56,7 @@ internal static void DisposingComponent(ILogger<Renderer> logger, ComponentState
}
}

internal static void HandlingEvent(ILogger<Renderer> logger, int eventHandlerId, UIEventArgs eventArgs)
internal static void HandlingEvent(ILogger<Renderer> logger, ulong eventHandlerId, UIEventArgs eventArgs)
{
_handlingEvent(logger, eventHandlerId, eventArgs?.Type ?? "null", null);
}
Expand Down
18 changes: 9 additions & 9 deletions src/Components/Components/src/Rendering/Renderer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ public abstract partial class Renderer : IDisposable
private readonly IServiceProvider _serviceProvider;
private readonly Dictionary<int, ComponentState> _componentStateById = new Dictionary<int, ComponentState>();
private readonly RenderBatchBuilder _batchBuilder = new RenderBatchBuilder();
private readonly Dictionary<int, EventCallback> _eventBindings = new Dictionary<int, EventCallback>();
private readonly Dictionary<int, int> _eventHandlerIdReplacements = new Dictionary<int, int>();
private readonly Dictionary<ulong, EventCallback> _eventBindings = new Dictionary<ulong, EventCallback>();
private readonly Dictionary<ulong, ulong> _eventHandlerIdReplacements = new Dictionary<ulong, ulong>();
private readonly ILogger<Renderer> _logger;

private int _nextComponentId = 0; // TODO: change to 'long' when Mono .NET->JS interop supports it
private bool _isBatchInProgress;
private int _lastEventHandlerId = 0;
private ulong _lastEventHandlerId;
private List<Task> _pendingTasks;

/// <summary>
Expand Down Expand Up @@ -206,7 +206,7 @@ private ComponentState AttachAndInitComponent(IComponent component, int parentCo
/// A <see cref="Task"/> which will complete once all asynchronous processing related to the event
/// has completed.
/// </returns>
public virtual Task DispatchEventAsync(int eventHandlerId, EventFieldInfo fieldInfo, UIEventArgs eventArgs)
public virtual Task DispatchEventAsync(ulong eventHandlerId, EventFieldInfo fieldInfo, UIEventArgs eventArgs)
{
EnsureSynchronizationContext();

Expand Down Expand Up @@ -354,15 +354,15 @@ protected internal virtual void AddToRenderQueue(int componentId, RenderFragment
}
}

internal void TrackReplacedEventHandlerId(int oldEventHandlerId, int newEventHandlerId)
internal void TrackReplacedEventHandlerId(ulong oldEventHandlerId, ulong newEventHandlerId)
{
// Tracking the chain of old->new replacements allows us to interpret incoming EventFieldInfo
// values even if they refer to an event handler ID that's since been superseded. This is essential
// for tree patching to work in an async environment.
_eventHandlerIdReplacements.Add(oldEventHandlerId, newEventHandlerId);
}

private int FindLatestEventHandlerIdInChain(int eventHandlerId)
private ulong FindLatestEventHandlerIdInChain(ulong eventHandlerId)
{
while (_eventHandlerIdReplacements.TryGetValue(eventHandlerId, out var replacementEventHandlerId))
{
Expand Down Expand Up @@ -573,7 +573,7 @@ private void RenderInExistingBatch(RenderQueueEntry renderQueueEntry)
}
}

private void RemoveEventHandlerIds(ArrayRange<int> eventHandlerIds, Task afterTaskIgnoreErrors)
private void RemoveEventHandlerIds(ArrayRange<ulong> eventHandlerIds, Task afterTaskIgnoreErrors)
{
if (eventHandlerIds.Count == 0)
{
Expand All @@ -598,7 +598,7 @@ private void RemoveEventHandlerIds(ArrayRange<int> eventHandlerIds, Task afterTa

// Factor out the async part into a separate local method purely so, in the
// synchronous case, there's no state machine or task construction
async Task ContinueAfterTask(ArrayRange<int> eventHandlerIds, Task afterTaskIgnoreErrors)
async Task ContinueAfterTask(ArrayRange<ulong> eventHandlerIds, Task afterTaskIgnoreErrors)
{
// We need to delay the actual removal (e.g., until we've confirmed the client
// has processed the batch and hence can be sure not to reuse the handler IDs
Expand Down Expand Up @@ -637,7 +637,7 @@ private async Task GetErrorHandledTask(Task taskToHandle)
}
}

private void UpdateRenderTreeToMatchClientState(int eventHandlerId, EventFieldInfo fieldInfo)
private void UpdateRenderTreeToMatchClientState(ulong eventHandlerId, EventFieldInfo fieldInfo)
{
var componentState = GetOptionalComponentState(fieldInfo.ComponentId);
if (componentState != null)
Expand Down
6 changes: 3 additions & 3 deletions src/Components/Components/test/RenderTreeDiffBuilderTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -859,7 +859,7 @@ public void RecognizesAttributeEventHandlerValuesChanged()
Assert.Equal(0, entry.ReferenceFrameIndex);
});
AssertFrame.Attribute(referenceFrames[0], "onbar", addedHandler);
Assert.NotEqual(0, removedEventHandlerFrame.AttributeEventHandlerId);
Assert.NotEqual(default, removedEventHandlerFrame.AttributeEventHandlerId);
Assert.Equal(
new[] { removedEventHandlerFrame.AttributeEventHandlerId },
batchBuilder.DisposedEventHandlerIDs.AsEnumerable());
Expand Down Expand Up @@ -1592,7 +1592,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers()
Assert.Empty(result.Edits);
AssertFrame.Attribute(oldAttributeFrame, "ontest", retainedHandler);
AssertFrame.Attribute(newAttributeFrame, "ontest", retainedHandler);
Assert.NotEqual(0, oldAttributeFrame.AttributeEventHandlerId);
Assert.NotEqual(default, oldAttributeFrame.AttributeEventHandlerId);
Assert.Equal(oldAttributeFrame.AttributeEventHandlerId, newAttributeFrame.AttributeEventHandlerId);
Assert.Empty(batchBuilder.DisposedEventHandlerIDs.AsEnumerable());
}
Expand All @@ -1619,7 +1619,7 @@ public void PreservesEventHandlerIdsForRetainedEventHandlers_SlowPath()
Assert.Single(result.Edits);
AssertFrame.Attribute(oldAttributeFrame, "ontest", retainedHandler);
AssertFrame.Attribute(newAttributeFrame, "ontest", retainedHandler);
Assert.NotEqual(0, oldAttributeFrame.AttributeEventHandlerId);
Assert.NotEqual(default, oldAttributeFrame.AttributeEventHandlerId);
Assert.Equal(oldAttributeFrame.AttributeEventHandlerId, newAttributeFrame.AttributeEventHandlerId);
Assert.Empty(batchBuilder.DisposedEventHandlerIDs.AsEnumerable());
}
Expand Down
4 changes: 2 additions & 2 deletions src/Components/Components/test/RendererTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3313,7 +3313,7 @@ public void EventFieldInfoCanPatchTreeSoDiffDoesNotUpdateAttribute(string oldVal
Assert.Equal(RenderTreeEditType.SetAttribute, edit.Type);
var attributeFrame = batch2.ReferenceFrames[edit.ReferenceFrameIndex];
AssertFrame.Attribute(attributeFrame, "ontestevent", typeof(Action<UIChangeEventArgs>));
Assert.NotEqual(0, attributeFrame.AttributeEventHandlerId);
Assert.NotEqual(default, attributeFrame.AttributeEventHandlerId);
Assert.NotEqual(eventHandlerId, attributeFrame.AttributeEventHandlerId);
});
}
Expand Down Expand Up @@ -3365,7 +3365,7 @@ public void EventFieldInfoWorksWhenEventHandlerIdWasSuperseded()
Assert.Equal(RenderTreeEditType.SetAttribute, edit.Type);
var attributeFrame = latestBatch.ReferenceFrames[edit.ReferenceFrameIndex];
AssertFrame.Attribute(attributeFrame, "ontestevent", typeof(Action<UIChangeEventArgs>));
Assert.NotEqual(0, attributeFrame.AttributeEventHandlerId);
Assert.NotEqual(default, attributeFrame.AttributeEventHandlerId);
Assert.NotEqual(eventHandlerId, attributeFrame.AttributeEventHandlerId);
});
}
Expand Down
Loading

0 comments on commit 4e04b81

Please sign in to comment.