Skip to content

Commit e792f20

Browse files
steipetesteipete
committed
fix: release recycled virtual chat rows
1 parent 149bcb2 commit e792f20

4 files changed

Lines changed: 199 additions & 14 deletions

File tree

‎src/OpenClawTray.FunctionalUI/FunctionalUI.cs‎

Lines changed: 92 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,7 @@ public sealed class FunctionalHostControl : ContentControl, IDisposable
816816
/// survives page navigation.
817817
/// </summary>
818818
public bool SuppressAutoDispose { get; set; }
819+
internal int CachedVirtualStackControlCount => _renderer.CachedVirtualStackControlCount;
819820

820821
public FunctionalHostControl()
821822
{
@@ -922,6 +923,9 @@ internal sealed class UiRenderer(Action requestRender)
922923
private readonly HashSet<string> _visitedVirtualStackPaths = new();
923924
private readonly Dictionary<string, string[]> _virtualStackOwnedPathPrefixes = new();
924925

926+
internal int CachedVirtualStackControlCount =>
927+
_controls.Keys.Count(IsOwnedByVirtualStack);
928+
925929
public UIElement Render(Element element, string path, List<Action> effects)
926930
{
927931
_visitedControlPaths.Clear();
@@ -1345,10 +1349,12 @@ private void UpdateRealizedVirtualStackItems(IEnumerable<VirtualStackItem> items
13451349
{
13461350
foreach (var item in items)
13471351
{
1348-
if (item.RealizedControl is null)
1352+
if (item.RealizedContainer is null)
13491353
continue;
13501354

1351-
item.RealizedControl = RenderElementCore(item.Element, item.Path, effects);
1355+
var child = RenderElementCore(item.Element, item.Path, effects);
1356+
SetChild(item.RealizedContainer, child);
1357+
PruneUnvisitedCachedSubtree(item.Path);
13521358
}
13531359
}
13541360

@@ -1361,6 +1367,14 @@ private static bool VirtualStackItemsMatch(VirtualStackItem[] currentItems, Virt
13611367
{
13621368
if (!string.Equals(currentItems[i].Path, nextItems[i].Path, StringComparison.Ordinal))
13631369
return false;
1370+
if (currentItems[i].Element.GetType() != nextItems[i].Element.GetType())
1371+
return false;
1372+
if (currentItems[i].Element is ComponentElement currentComponent
1373+
&& nextItems[i].Element is ComponentElement nextComponent
1374+
&& currentComponent.ComponentType != nextComponent.ComponentType)
1375+
{
1376+
return false;
1377+
}
13641378
}
13651379

13661380
return true;
@@ -1371,11 +1385,13 @@ private sealed class VirtualStackItem(int index, string path, Element element)
13711385
public int Index { get; set; } = index;
13721386
public string Path { get; } = path;
13731387
public Element Element { get; set; } = element;
1374-
public UIElement? RealizedControl { get; set; }
1388+
public Border? RealizedContainer { get; set; }
13751389
}
13761390

13771391
private sealed class VirtualStackItemTemplate(UiRenderer renderer, string path) : IElementFactory
13781392
{
1393+
private readonly Dictionary<UIElement, VirtualStackItem> _realizedItems = new();
1394+
13791395
public bool Matches(UiRenderer otherRenderer, string otherPath) =>
13801396
ReferenceEquals(renderer, otherRenderer) && string.Equals(path, otherPath, StringComparison.Ordinal);
13811397

@@ -1384,19 +1400,86 @@ public UIElement GetElement(ElementFactoryGetArgs args)
13841400
if (args.Data is not VirtualStackItem item)
13851401
return new Border();
13861402

1403+
var container = new Border { HorizontalAlignment = HorizontalAlignment.Stretch };
13871404
var effects = new List<Action>();
1388-
var control = renderer.RenderElementCore(item.Element, item.Path, effects);
1389-
item.RealizedControl = control;
1405+
var child = renderer.RenderElementCore(item.Element, item.Path, effects);
1406+
renderer.SetChild(container, child);
1407+
item.RealizedContainer = container;
1408+
_realizedItems[container] = item;
13901409
foreach (var effect in effects)
13911410
effect();
1392-
return control;
1411+
return container;
13931412
}
13941413

13951414
public void RecycleElement(ElementFactoryRecycleArgs args)
13961415
{
1397-
// The renderer cache owns element identity by path. ItemsRepeater
1398-
// detaches recycled rows; re-realization asks for the same path and
1399-
// gets a refreshed control without rebuilding the whole list.
1416+
if (args.Element is not { } control || !_realizedItems.Remove(control, out var item))
1417+
return;
1418+
1419+
if (ReferenceEquals(item.RealizedContainer, control))
1420+
item.RealizedContainer = null;
1421+
renderer.RemoveCachedSubtree(item.Path);
1422+
}
1423+
}
1424+
1425+
private void PruneUnvisitedCachedSubtree(string prefix)
1426+
{
1427+
foreach (var (key, component) in _components
1428+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix) && !_visitedComponentKeys.Contains(pair.Key))
1429+
.ToArray())
1430+
{
1431+
component.Context.RunEffectCleanups();
1432+
_components.Remove(key);
1433+
}
1434+
1435+
foreach (var (path, flyout) in _contentFlyouts
1436+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix) && !_visitedContentFlyoutPaths.Contains(pair.Key))
1437+
.ToArray())
1438+
{
1439+
flyout.Hide();
1440+
flyout.Content = null;
1441+
_contentFlyouts.Remove(path);
1442+
}
1443+
1444+
foreach (var (path, cachedControl) in _controls
1445+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix) && !_visitedControlPaths.Contains(pair.Key))
1446+
.OrderByDescending(pair => pair.Key.Length)
1447+
.ToArray())
1448+
{
1449+
_mountedPaths.Remove(path);
1450+
DetachChildren(cachedControl);
1451+
RemoveFromParent(cachedControl);
1452+
_controls.Remove(path);
1453+
}
1454+
}
1455+
1456+
private void RemoveCachedSubtree(string prefix)
1457+
{
1458+
foreach (var (key, component) in _components
1459+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix))
1460+
.ToArray())
1461+
{
1462+
component.Context.RunEffectCleanups();
1463+
_components.Remove(key);
1464+
}
1465+
1466+
foreach (var (path, flyout) in _contentFlyouts
1467+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix))
1468+
.ToArray())
1469+
{
1470+
flyout.Hide();
1471+
_contentFlyouts.Remove(path);
1472+
}
1473+
1474+
foreach (var (path, cachedControl) in _controls
1475+
.Where(pair => IsPathAtOrBelow(pair.Key, prefix))
1476+
.OrderByDescending(pair => pair.Key.Length)
1477+
.ToArray())
1478+
{
1479+
_mountedPaths.Remove(path);
1480+
DetachChildren(cachedControl);
1481+
RemoveFromParent(cachedControl);
1482+
_controls.Remove(path);
14001483
}
14011484
}
14021485

‎src/OpenClawTray.FunctionalUI/OpenClawTray.FunctionalUI.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@
2323
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
2424
<_Parameter1>OpenClawTray.FunctionalUI.Tests</_Parameter1>
2525
</AssemblyAttribute>
26+
<AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleToAttribute">
27+
<_Parameter1>OpenClaw.Tray.UITests</_Parameter1>
28+
</AssemblyAttribute>
2629
</ItemGroup>
2730

2831
</Project>

‎tests/OpenClaw.Tray.Tests/ChatTimelineVirtualizationContractTests.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ public void FunctionalUiVirtualStack_IsPruneAwareAndStableAcrossRenderChurn()
4242
Assert.Contains("VirtualStackItemsMatch", functionalUi);
4343
Assert.Contains("UpdateRealizedVirtualStackItems", functionalUi);
4444
Assert.Contains("repeater.ItemTemplate is not VirtualStackItemTemplate", functionalUi);
45+
Assert.Contains("RemoveCachedSubtree(item.Path)", functionalUi);
46+
Assert.Contains("item.RealizedContainer = null", functionalUi);
47+
Assert.Contains("PruneUnvisitedCachedSubtree(item.Path)", functionalUi);
4548
Assert.Contains("PruneUnvisitedPaths();", functionalUi);
4649
}
4750

‎tests/OpenClaw.Tray.UITests/ChatTimelineVirtualizationProofTests.cs‎

Lines changed: 101 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using OpenClaw.Chat;
88
using OpenClawTray.Chat;
99
using OpenClawTray.FunctionalUI;
10+
using OpenClawTray.FunctionalUI.Core;
1011
using OpenClawTray.FunctionalUI.Hosting;
1112
using Windows.Graphics;
1213
using static OpenClawTray.FunctionalUI.Factories;
@@ -30,6 +31,7 @@ public async Task LargeNativeChatTimeline_VirtualizesRowsAndFollowsNewMessages()
3031

3132
var props = BuildProps(InitialRows, scrollToBottomToken: 0);
3233
FunctionalHostControl? host = null;
34+
var initialCachedVirtualControls = 0;
3335

3436
await _ui.RunOnUIAsync(() =>
3537
{
@@ -57,24 +59,44 @@ await _ui.RunOnUIAsync(() =>
5759
Assert.Equal(Orientation.Vertical, layout.Orientation);
5860
Assert.Equal(2, layout.Spacing);
5961
Assert.Equal(InitialRows, CountItems(repeater.ItemsSource));
62+
var realizedRows = Enumerable.Range(0, InitialRows)
63+
.Count(index => repeater.TryGetElement(index) is not null);
64+
Assert.InRange(realizedRows, 1, InitialRows - 1);
65+
initialCachedVirtualControls = host!.CachedVirtualStackControlCount;
66+
Assert.True(initialCachedVirtualControls > 0);
6067

6168
var scrollViewer = FindLogical<ScrollViewer>(host!).Single();
6269
_ui.Container.UpdateLayout();
6370
Assert.True(scrollViewer.ScrollableHeight > 0, "large chat timeline should overflow and become scrollable");
64-
65-
scrollViewer.ChangeView(null, scrollViewer.ScrollableHeight, null, disableAnimation: true);
66-
_ui.Container.UpdateLayout();
67-
Assert.True(scrollViewer.VerticalOffset > 0, "large chat timeline should scroll away from the top");
6871
});
6972

70-
await DrainRenderQueueAsync();
73+
foreach (var fraction in new[] { 0.25, 0.5, 0.75, 1.0 })
74+
{
75+
await _ui.RunOnUIAsync(() =>
76+
{
77+
var scrollViewer = FindLogical<ScrollViewer>(host!).Single();
78+
scrollViewer.ChangeView(
79+
null,
80+
scrollViewer.ScrollableHeight * fraction,
81+
null,
82+
disableAnimation: true);
83+
_ui.Container.UpdateLayout();
84+
});
85+
await DrainRenderQueueAsync();
86+
}
7187

7288
object? stableItemsSource = null;
7389
object? stableItemTemplate = null;
7490
props = BuildProps(InitialRows, scrollToBottomToken: 0, textRevision: 1);
7591
await _ui.RunOnUIAsync(() =>
7692
{
7793
var repeater = FindLogical<ItemsRepeater>(host!).Single();
94+
Assert.Null(repeater.TryGetElement(0));
95+
Assert.NotNull(repeater.TryGetElement(InitialRows - 1));
96+
Assert.InRange(
97+
host!.CachedVirtualStackControlCount,
98+
1,
99+
initialCachedVirtualControls * 2);
78100
stableItemsSource = repeater.ItemsSource;
79101
stableItemTemplate = repeater.ItemTemplate;
80102

@@ -140,6 +162,57 @@ await _ui.RunOnUIAsync(() =>
140162
await _ui.RunOnUIAsync(() => host!.Dispose());
141163
}
142164

165+
[Fact]
166+
public async Task RealizedComponentRow_ReplacesRootAndPrunesRemovedEffects()
167+
{
168+
await _ui.ResetContainerAsync();
169+
DisposableVirtualRow.CleanupCount = 0;
170+
171+
FunctionalHostControl? host = null;
172+
ItemsRepeater? repeater = null;
173+
UIElement? stableContainer = null;
174+
var expandedCacheCount = 0;
175+
var cleanupCountBeforeCollapse = 0;
176+
177+
await _ui.RunOnUIAsync(() =>
178+
{
179+
host = new FunctionalHostControl { Width = 600, Height = 400, SuppressAutoDispose = true };
180+
_ui.Container.Children.Add(host);
181+
host.Mount(_ => VirtualVStack(
182+
0,
183+
Component<SwappingVirtualRow, SwappingVirtualRowProps>(new SwappingVirtualRowProps(true))));
184+
});
185+
await DrainRenderQueueAsync();
186+
187+
await _ui.RunOnUIAsync(() =>
188+
{
189+
repeater = FindLogical<ItemsRepeater>(host!).Single();
190+
stableContainer = repeater.TryGetElement(0);
191+
Assert.IsType<Border>(stableContainer);
192+
Assert.Contains(FindDescendants<TextBlock>(host!), text => text.Text == "expanded row");
193+
Assert.Contains(FindDescendants<TextBlock>(host!), text => text.Text == "disposable child");
194+
expandedCacheCount = host!.CachedVirtualStackControlCount;
195+
Assert.True(expandedCacheCount > 1);
196+
cleanupCountBeforeCollapse = DisposableVirtualRow.CleanupCount;
197+
198+
host.Mount(_ => VirtualVStack(
199+
0,
200+
Component<SwappingVirtualRow, SwappingVirtualRowProps>(new SwappingVirtualRowProps(false))));
201+
});
202+
await DrainRenderQueueAsync();
203+
204+
await _ui.RunOnUIAsync(() =>
205+
{
206+
Assert.Same(stableContainer, repeater!.TryGetElement(0));
207+
Assert.Contains(FindDescendants<TextBlock>(host!), text => text.Text == "collapsed row");
208+
Assert.DoesNotContain(FindDescendants<TextBlock>(host!), text => text.Text == "disposable child");
209+
Assert.Equal(cleanupCountBeforeCollapse + 1, DisposableVirtualRow.CleanupCount);
210+
Assert.InRange(host!.CachedVirtualStackControlCount, 1, expandedCacheCount - 1);
211+
});
212+
213+
await _ui.RunOnUIAsync(() => host!.Dispose());
214+
}
215+
143216
private async Task DrainRenderQueueAsync()
144217
{
145218
await _ui.RunOnUIAsync(() => { });
@@ -191,4 +264,27 @@ private static int CountItems(object? itemsSource) =>
191264
itemsSource is System.Collections.IEnumerable enumerable
192265
? enumerable.Cast<object>().Count()
193266
: 0;
267+
268+
private sealed record SwappingVirtualRowProps(bool Expanded);
269+
270+
private sealed class SwappingVirtualRow : Component<SwappingVirtualRowProps>
271+
{
272+
public override Element Render() => Props.Expanded
273+
? VStack(
274+
0,
275+
TextBlock("expanded row"),
276+
Component<DisposableVirtualRow>())
277+
: TextBlock("collapsed row");
278+
}
279+
280+
private sealed class DisposableVirtualRow : Component
281+
{
282+
internal static int CleanupCount { get; set; }
283+
284+
public override Element Render()
285+
{
286+
UseEffect((Func<Action>)(() => () => CleanupCount++), Array.Empty<object>());
287+
return TextBlock("disposable child");
288+
}
289+
}
194290
}

0 commit comments

Comments
 (0)