huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Dispose components on removal

This commit is contained in:
Steve Sanderson 2018-01-30 10:00:30 +00:00
parent 8839a6c558
commit 50a5baa872
10 changed files with 185 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/BrowserRenderer.ts
View File

@ -6,9 +6,6 @@ let raiseEventMethod: MethodHandle;
let renderComponentMethod: MethodHandle;
export class BrowserRenderer {
// TODO: To avoid leaking memory, automatically remove entries from this dict as soon
// as the corresponding DOM nodes are removed (or maybe when the associated component
// is disposed, assuming we can guarantee that always happens).
private childComponentLocations: { [componentId: number]: Element } = {};
constructor(private browserRendererId: number) {
@ -27,6 +24,10 @@ export class BrowserRenderer {
this.applyEdits(componentId, element, 0, edits, editsLength, referenceTree);
}
public disposeComponent(componentId: number) {
delete this.childComponentLocations[componentId];
}
applyEdits(componentId: number, parent: Element, childIndex: number, edits: System_Array<RenderTreeEditPointer>, editsLength: number, referenceTree: System_Array<RenderTreeNodePointer>) {
let currentDepth = 0;
let childIndexAtCurrentDepth = childIndex;

1
src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/RenderBatch.ts
View File

@ -7,6 +7,7 @@ import { RenderTreeEditPointer } from './RenderTreeEdit';
export const renderBatch = {
updatedComponents: (obj: RenderBatchPointer) => platform.readStructField<ArrayRangePointer<RenderTreeDiffPointer>>(obj, 0),
disposedComponentIds: (obj: RenderBatchPointer) => platform.readStructField<ArrayRangePointer<number>>(obj, arrayRangeStructLength),
};
const arrayRangeStructLength = 8;

11
src/Microsoft.AspNetCore.Blazor.Browser.JS/src/Rendering/Renderer.ts
View File

@ -32,7 +32,7 @@ export function renderBatch(browserRendererId: number, batch: RenderBatchPointer
const updatedComponents = renderBatchStruct.updatedComponents(batch);
const updatedComponentsLength = arrayRange.count(updatedComponents);
const updatedComponentsArray = arrayRange.array(updatedComponents);
for (var i = 0; i < updatedComponentsLength; i++) {
for (let i = 0; i < updatedComponentsLength; i++) {
const diff = platform.getArrayEntryPtr(updatedComponentsArray, i, renderTreeDiffStructLength);
const componentId = renderTreeDiff.componentId(diff);
@ -44,6 +44,15 @@ export function renderBatch(browserRendererId: number, batch: RenderBatchPointer
const tree = arrayRange.array(currentStateArrayRange);
browserRenderer.updateComponent(componentId, edits, editsLength, tree);
}
const disposedComponentIds = renderBatchStruct.disposedComponentIds(batch);
const disposedComponentIdsLength = arrayRange.count(disposedComponentIds);
const disposedComponentIdsArray = arrayRange.array(disposedComponentIds);
for (let i = 0; i < disposedComponentIdsLength; i++) {
const componentIdPtr = platform.getArrayEntryPtr(disposedComponentIdsArray, i, 4);
const componentId = platform.readInt32Field(componentIdPtr);
browserRenderer.disposeComponent(componentId);
}
}
function clearElement(element: Element) {

25
src/Microsoft.AspNetCore.Blazor/RenderTree/RenderTreeDiffComputer.cs
View File

@ -157,13 +157,19 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
else
{
ref var oldNode = ref oldTree[oldStartIndex];
if (oldNode.NodeType == RenderTreeNodeType.Attribute)
var oldNodeType = oldNode.NodeType;
if (oldNodeType == RenderTreeNodeType.Attribute)
{
Append(RenderTreeEdit.RemoveAttribute(siblingIndex, oldNode.AttributeName));
oldStartIndex++;
}
else
{
if (oldNodeType == RenderTreeNodeType.Element || oldNodeType == RenderTreeNodeType.Component)
{
DisposeChildComponents(batchBuilder, oldTree, oldStartIndex);
}
Append(RenderTreeEdit.RemoveNode(siblingIndex));
oldStartIndex = NextSiblingIndex(oldNode, oldStartIndex);
}
@ -381,6 +387,8 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
else
{
// Elements with different names are treated as completely unrelated
InstantiateChildComponents(batchBuilder, newTree, newNodeIndex);
DisposeChildComponents(batchBuilder, oldTree, oldNodeIndex);
Append(RenderTreeEdit.PrependNode(siblingIndex, newNodeIndex));
siblingIndex++;
Append(RenderTreeEdit.RemoveNode(siblingIndex));
@ -404,6 +412,8 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
else
{
// Child components of different types are treated as completely unrelated
InstantiateChildComponents(batchBuilder, newTree, newNodeIndex);
DisposeChildComponents(batchBuilder, oldTree, oldNodeIndex);
Append(RenderTreeEdit.PrependNode(siblingIndex, newNodeIndex));
siblingIndex++;
Append(RenderTreeEdit.RemoveNode(siblingIndex));
@ -502,5 +512,18 @@ namespace Microsoft.AspNetCore.Blazor.RenderTree
}
}
}
private void DisposeChildComponents(RenderBatchBuilder batchBuilder, RenderTreeNode[] nodes, int elementOrComponentIndex)
{
var endIndex = nodes[elementOrComponentIndex].ElementDescendantsEndIndex;
for (var i = elementOrComponentIndex; i <= endIndex; i++)
{
ref var node = ref nodes[i];
if (node.NodeType == RenderTreeNodeType.Component)
{
_renderer.DisposeInExistingBatch(batchBuilder, node.ComponentId);
}
}
}
}
}

12
src/Microsoft.AspNetCore.Blazor/Rendering/ComponentState.cs
View File

@ -80,5 +80,17 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
// developers don't need to call Render() on their components explicitly.
_renderer.RenderNewBatch(_componentId);
}
/// <summary>
/// Notifies the component that it is being disposed.
/// </summary>
public void NotifyDisposed()
{
// TODO: Handle components throwing during dispose. Shouldn't break the whole render batch.
if (_component is IDisposable disposable)
{
disposable.Dispose();
}
}
}
}

10
src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatch.cs
View File

@ -15,9 +15,17 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
/// </summary>
public ArrayRange<RenderTreeDiff> UpdatedComponents { get; }
internal RenderBatch(ArrayRange<RenderTreeDiff> updatedComponents)
/// <summary>
/// Gets the IDs of the components that were disposed.
/// </summary>
public ArrayRange<int> DisposedComponentIDs { get; }
internal RenderBatch(
ArrayRange<RenderTreeDiff> updatedComponents,
ArrayRange<int> disposedComponentIDs)
{
UpdatedComponents = updatedComponents;
DisposedComponentIDs = disposedComponentIDs;
}
}
}

13
src/Microsoft.AspNetCore.Blazor/Rendering/RenderBatchBuilder.cs
View File

@ -1,6 +1,7 @@
// 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 System;
using Microsoft.AspNetCore.Blazor.RenderTree;
namespace Microsoft.AspNetCore.Blazor.Rendering
@ -8,6 +9,7 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
internal class RenderBatchBuilder
{
private ArrayBuilder<RenderTreeDiff> _updatedComponentDiffs = new ArrayBuilder<RenderTreeDiff>();
private ArrayBuilder<int> _disposedComponentIds = new ArrayBuilder<int>();
public int ReserveUpdatedComponentSlotId()
{
@ -20,10 +22,17 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
=> _updatedComponentDiffs.Overwrite(updatedComponentSlotId, diff);
public void Clear()
=> _updatedComponentDiffs.Clear();
{
_updatedComponentDiffs.Clear();
_disposedComponentIds.Clear();
}
public RenderBatch ToBatch()
=> new RenderBatch(
_updatedComponentDiffs.ToRange());
_updatedComponentDiffs.ToRange(),
_disposedComponentIds.ToRange());
public void AddDisposedComponent(int componentId)
=> _disposedComponentIds.Append(componentId);
}
}

6
src/Microsoft.AspNetCore.Blazor/Rendering/Renderer.cs
View File

@ -96,6 +96,12 @@ namespace Microsoft.AspNetCore.Blazor.Rendering
GetRequiredComponentState(componentId).Render(batchBuilder);
}
internal void DisposeInExistingBatch(RenderBatchBuilder batchBuilder, int componentId)
{
GetRequiredComponentState(componentId).NotifyDisposed();
batchBuilder.AddDisposedComponent(componentId);
}
/// <summary>
/// Notifies the specified component that an event has occurred.
/// </summary>

68
test/Microsoft.AspNetCore.Blazor.Test/RenderTreeDiffComputerTest.cs
View File

@ -316,16 +316,34 @@ namespace Microsoft.AspNetCore.Blazor.Test
newTree.OpenComponentElement<FakeComponent2>(123);
// Act
var result = GetSingleUpdatedComponent();
var renderBatch = GetRenderedBatch();
// Assert
Assert.Collection(result.Edits,
// Assert: Even though we didn't assign IDs to the components, this
// shows that FakeComponent was disposed
Assert.Collection(renderBatch.DisposedComponentIDs,
disposedComponentId => Assert.Equal(0, disposedComponentId));
// Assert: First updated component is the root with one child being
// prepended, and its earlier incarnation being removed
Assert.Equal(2, renderBatch.UpdatedComponents.Count);
var updatedComponent1 = renderBatch.UpdatedComponents.Array[0];
Assert.Collection(updatedComponent1.Edits,
entry =>
{
AssertEdit(entry, RenderTreeEditType.PrependNode, 0);
Assert.Equal(0, entry.NewTreeIndex);
Assert.IsType<FakeComponent2>(updatedComponent1.CurrentState.Array[0].Component);
},
entry => AssertEdit(entry, RenderTreeEditType.RemoveNode, 1));
// Assert: Second updated component is the new FakeComponent2
var updatedComponent2 = renderBatch.UpdatedComponents.Array[1];
Assert.Collection(updatedComponent2.Edits,
entry =>
{
AssertEdit(entry, RenderTreeEditType.PrependNode, 0);
Assert.Equal(0, entry.NewTreeIndex);
});
}
[Fact]
@ -744,6 +762,37 @@ namespace Microsoft.AspNetCore.Blazor.Test
Assert.Null(newComponentInstance.ObjectProperty); // To observe that the property wasn't even written, we nulled it out on the original
}
[Fact]
public void CallsDisposeOnlyOnRemovedChildComponents()
{
// Arrange
oldTree.OpenComponentElement<DisposableComponent>(10); // <DisposableComponent>
oldTree.CloseElement(); // </DisposableComponent>
oldTree.OpenComponentElement<NonDisposableComponent>(20); // <NonDisposableComponent>
oldTree.CloseElement(); // </NonDisposableComponent>
oldTree.OpenComponentElement<DisposableComponent>(30); // <DisposableComponent>
oldTree.CloseElement(); // </DisposableComponent>
newTree.OpenComponentElement<DisposableComponent>(30); // <DisposableComponent>
newTree.CloseElement(); // </DisposableComponent>
diff.ApplyNewRenderTreeVersion(new RenderBatchBuilder(), 0, new RenderTreeBuilder(renderer).GetNodes(), oldTree.GetNodes());
var disposableComponent1 = (DisposableComponent)oldTree.GetNodes().Array[0].Component;
var nonDisposableComponent = (NonDisposableComponent)oldTree.GetNodes().Array[1].Component;
var disposableComponent2 = (DisposableComponent)oldTree.GetNodes().Array[2].Component;
// Act
var renderedBatch = GetRenderedBatch();
// Assert: We track NonDisposableComponent was disposed even though it's not IDisposable
Assert.Equal(renderedBatch.DisposedComponentIDs, new[] { 0, 1 });
// Assert: We did call Dispose on the disposed DisposableComponent
Assert.Equal(1, disposableComponent1.DisposalCount);
// Assert: We didn't dispose the retained component
Assert.Equal(0, disposableComponent2.DisposalCount);
}
private RenderTreeDiff GetSingleUpdatedComponent()
{
var diffsInBatch = GetRenderedBatch().UpdatedComponents;
@ -786,6 +835,19 @@ namespace Microsoft.AspNetCore.Blazor.Test
}
}
private class DisposableComponent : IComponent, IDisposable
{
public int DisposalCount { get; private set; }
public void Dispose() => DisposalCount++;
public void BuildRenderTree(RenderTreeBuilder builder) { }
}
private class NonDisposableComponent : IComponent
{
public void BuildRenderTree(RenderTreeBuilder builder) { }
}
private static void AssertEdit(
RenderTreeEdit edit,
RenderTreeEditType type,

43
test/Microsoft.AspNetCore.Blazor.Test/RendererTest.cs
View File

@ -402,6 +402,45 @@ namespace Microsoft.AspNetCore.Blazor.Test
node => AssertNode.Text(node, "second"));
}
[Fact]
public void RenderBatchIncludesListOfDisposedComponents()
{
// Arrange
var renderer = new TestRenderer();
var firstRender = true;
var component = new TestComponent(builder =>
{
builder.OpenElement(7, "some element");
if (firstRender)
{
builder.OpenComponentElement<FakeComponent>(100);
builder.CloseElement();
builder.OpenComponentElement<FakeComponent>(150);
builder.CloseElement();
}
builder.OpenComponentElement<FakeComponent>(200);
builder.CloseElement();
builder.CloseElement();
});
var rootComponentId = renderer.AssignComponentId(component);
// Act/Assert 1: First render, capturing child component IDs
renderer.RenderNewBatch(rootComponentId);
var childComponentIds = renderer.Batches.Single().RenderTreesByComponentId[rootComponentId]
.Where(node => node.NodeType == RenderTreeNodeType.Component)
.Select(node => node.ComponentId)
.ToList();
Assert.Equal(childComponentIds, new[] { 1, 2, 3 });
// Act: Second render
firstRender = false;
renderer.RenderNewBatch(rootComponentId);
// Assert: Applicable children are included in disposal list
Assert.Equal(renderer.Batches[1].DisposedComponentIDs, new[] { 1, 2 });
}
private class NoOpRenderer : Renderer
{
public new int AssignComponentId(IComponent component)
@ -440,6 +479,8 @@ namespace Microsoft.AspNetCore.Blazor.Test
capturedBatch.RenderTreesByComponentId[renderTreeDiff.ComponentId] =
renderTreeDiff.CurrentState.ToArray();
}
capturedBatch.DisposedComponentIDs = renderBatch.DisposedComponentIDs.ToList();
}
}
@ -447,6 +488,8 @@ namespace Microsoft.AspNetCore.Blazor.Test
{
public IDictionary<int, RenderTreeNode[]> RenderTreesByComponentId { get; }
= new Dictionary<int, RenderTreeNode[]>();
public IList<int> DisposedComponentIDs { get; set; }
}
private class TestComponent : IComponent