From 307169b8a5a024f3fa232749efd51b0c49cbcf9f Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Thu, 15 Oct 2026 15:51:46 -0700 Subject: [PATCH 01/10] Replay ancestor Context when virtualized hosts realize rows Virtualized hosts (LazyVStack/ItemsRepeater, ItemsView, ListView/GridView, typed TreeView) build rows from WinUI layout callbacks that run outside the reconciler traversal, so rows read an empty or unrelated ContextScope: UseContext returned the default and child-mode UseNavigation threw. Each host now snapshots the provider stack when it is reconciled, and the realization callbacks replay that snapshot around the row mount. A host whose snapshot no longer matches the live providers declines the reference-stable skip so the snapshot is refreshed before the next realization. Fixes #1386 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 60 ++++- src/Reactor/Core/ElementFactory.cs | 10 +- .../Core/IItemsRepeaterFactorySource.cs | 10 + .../Core/Reconciler.KeyedItemsBinding.cs | 6 + src/Reactor/Core/Reconciler.Mount.cs | 3 + src/Reactor/Core/Reconciler.Update.cs | 4 + src/Reactor/Core/Reconciler.cs | 98 ++++++++ .../V1Protocol/Handlers/GridViewHandler.cs | 2 + .../V1Protocol/Handlers/ListViewHandler.cs | 2 + .../Core/V1Protocol/LazyStackLifecycle.cs | 2 + .../V1Protocol/Reconciler.TemplatedTree.cs | 4 + .../Core/V1Protocol/TemplatedListLifecycle.cs | 2 + .../Fixtures/VirtualizedRowContextFixtures.cs | 216 ++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 12 + 14 files changed, 429 insertions(+), 2 deletions(-) create mode 100644 tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 21ae17ad0..b2b4d5779 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -7,7 +7,7 @@ namespace Microsoft.UI.Reactor.Core; /// internal sealed class ContextScope { - private readonly List<(ContextBase Context, object? Value)> _stack = new(); + private List<(ContextBase Context, object? Value)> _stack = new(); private long _version; internal void Push(IReadOnlyDictionary values) @@ -51,4 +51,62 @@ internal T Read(Context context) internal bool HasActiveValues => _stack.Count > 0; internal long Version => _version; + + internal (ContextBase Context, object? Value)[] Snapshot() + => _stack.Count == 0 ? [] : _stack.ToArray(); + + /// + /// True when the live stack is a positional prefix of + /// (same contexts, equal values). The traversal pushes providers along the path, + /// so at any ancestor of the node that took the snapshot the live stack is a + /// prefix of it unless a provider at or above that ancestor changed. + /// + internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) + { + if (_stack.Count > snapshot.Length) return false; + for (int i = 0; i < _stack.Count; i++) + { + if (!ReferenceEquals(_stack[i].Context, snapshot[i].Context) + || !Equals(_stack[i].Value, snapshot[i].Value)) + return false; + } + return true; + } + + /// + /// Replaces the live stack with and returns the stack it + /// displaced, which the caller must hand back to . Replacing + /// (rather than pushing on top) keeps a re-entrant realization from observing the + /// providers of whatever subtree the traversal happened to be in. + /// + internal List<(ContextBase Context, object? Value)> Replace((ContextBase Context, object? Value)[] snapshot) + { + var previous = _stack; + _stack = new List<(ContextBase Context, object? Value)>(snapshot); + _version++; + return previous; + } + + internal void Restore(List<(ContextBase Context, object? Value)> previous) + { + _stack = previous; + _version++; + } +} + +/// +/// The provider stack as it stood when the reconciler last reconciled a virtualized +/// host (LazyVStack, ItemsView, ListView, TreeView, …). Those hosts realize rows from +/// WinUI callbacks — IElementFactory.GetElement, ContainerContentChanging — +/// that run during layout, outside the traversal that maintains , +/// so the row mount replays this snapshot instead of reading whatever the live stack +/// holds at that moment. +/// +internal sealed class ContextCapture +{ + internal (ContextBase Context, object? Value)[] Entries { get; private set; } = []; + + internal void Refresh(ContextScope scope) => Entries = scope.Snapshot(); + + internal bool IsStale(ContextScope scope) => !scope.IsPrefixOf(Entries); } diff --git a/src/Reactor/Core/ElementFactory.cs b/src/Reactor/Core/ElementFactory.cs index b3788ee7a..c5a3fc8b0 100644 --- a/src/Reactor/Core/ElementFactory.cs +++ b/src/Reactor/Core/ElementFactory.cs @@ -17,13 +17,19 @@ namespace Microsoft.UI.Reactor.Core; /// because the dictionary's int keys no longer matched the repeater's /// new positions. Keying by string makes the mapping reorder-stable. /// -public sealed partial class ElementFactory : IElementFactory +public sealed partial class ElementFactory : IElementFactory, IContextCapturingFactory { private IReadOnlyList _items; private Func _viewBuilder; private readonly Reconciler _reconciler; private readonly Action _requestRerender; private readonly ElementPool? _pool; + + // The host's provider-stack snapshot. GetElement runs from WinUI layout, + // outside the traversal that maintains the reconciler's context stack, so + // rows replay this instead (see ContextCapture). Null when the factory is + // driven directly (tests) — rows then read the live stack as before. + ContextCapture? IContextCapturingFactory.ContextCapture { get; set; } // Optional state used when ItemsSource is the OC path // (spec 042). Lets GetElement translate an ItemsRepeater realized // index → stable key for _mountedElements lookup. Null when running @@ -595,6 +601,8 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; + using var restoreContext = _reconciler.EnterCapturedContext(((IContextCapturingFactory)this).ContextCapture); + var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/IItemsRepeaterFactorySource.cs b/src/Reactor/Core/IItemsRepeaterFactorySource.cs index 8eb42fec4..b31d757c9 100644 --- a/src/Reactor/Core/IItemsRepeaterFactorySource.cs +++ b/src/Reactor/Core/IItemsRepeaterFactorySource.cs @@ -67,3 +67,13 @@ internal interface IItemsRepeaterFactorySource /// void RefreshRealizedItems(IElementFactory factory, WinUI.ItemsRepeater repeater); } + +/// +/// An that replays its host's provider stack while +/// realizing rows (see ). Hosts assign the capture via +/// whenever they install a factory. +/// +internal interface IContextCapturingFactory +{ + ContextCapture? ContextCapture { get; set; } +} diff --git a/src/Reactor/Core/Reconciler.KeyedItemsBinding.cs b/src/Reactor/Core/Reconciler.KeyedItemsBinding.cs index fad3e5644..7ac531d30 100644 --- a/src/Reactor/Core/Reconciler.KeyedItemsBinding.cs +++ b/src/Reactor/Core/Reconciler.KeyedItemsBinding.cs @@ -177,6 +177,7 @@ private void BindItemsRepeaterErasedKeyedItems( ir.ItemsSource = state.Source; var factory = factorySource.CreateFactory(this, requestRerender, _pool); factorySource.AttachListStateToFactory(factory, state); + AttachContextCapture(factory, ir); ir.ItemTemplate = factory; return; } @@ -200,6 +201,7 @@ private void BindItemsRepeaterErasedKeyedItems( ir.ItemsSource = fresh.Source; var factory = factorySource.CreateFactory(this, requestRerender, _pool); factorySource.AttachListStateToFactory(factory, fresh); + AttachContextCapture(factory, ir); ir.ItemTemplate = factory; } } @@ -263,6 +265,7 @@ private void BindItemsViewErasedKeyedItems( iv.ItemsSource = state.Source; var factory = itemsViewSource.CreateFactory(this, requestRerender, _pool); itemsViewSource.AttachListStateToFactory(factory, state); + AttachContextCapture(factory, iv); iv.ItemTemplate = factory; return; } @@ -282,6 +285,7 @@ private void BindItemsViewErasedKeyedItems( iv.ItemsSource = fresh.Source; var replacementFactory = itemsViewSource.CreateFactory(this, requestRerender, _pool); itemsViewSource.AttachListStateToFactory(replacementFactory, fresh); + AttachContextCapture(replacementFactory, iv); iv.ItemTemplate = replacementFactory; } @@ -324,6 +328,7 @@ private void BindListViewBaseErasedKeyedItems( { if (isMount) { + CaptureContextForHost(lvb); lvb.ItemTemplate = SharedContentControlTemplate.Value; lvb.ContainerContentChanging += (sender, args) => HandleTemplatedContainerContentChanging(sender, args, requestRerender); @@ -469,6 +474,7 @@ private void BindListViewBaseKeyedItems( // body is shared 1:1 with the legacy path so existing CCC // semantics (recycle teardown, attached-tag bookkeeping, // ambient-enter animation) come for free. + CaptureContextForHost(lvb); lvb.ItemTemplate = SharedContentControlTemplate.Value; lvb.ContainerContentChanging += (sender, args) => HandleTemplatedContainerContentChanging(sender, args, requestRerender); diff --git a/src/Reactor/Core/Reconciler.Mount.cs b/src/Reactor/Core/Reconciler.Mount.cs index 607cc93d1..865fc87f9 100644 --- a/src/Reactor/Core/Reconciler.Mount.cs +++ b/src/Reactor/Core/Reconciler.Mount.cs @@ -709,6 +709,9 @@ internal void HandleTemplatedContainerContentChanging(object sender, ContainerCo if (viewSource is not null && args.ItemIndex >= 0 && args.ItemIndex < viewSource.ItemCount && args.ItemContainer.ContentTemplateRoot is ContentControl cc) { + // Realization runs from layout, outside the traversal — replay the + // host's provider stack so the row sees its ancestors' context. + using var restoreContext = EnterCapturedContext((UIElement)sender!); var itemElement = viewSource.BuildItemView(args.ItemIndex); var ctrl = Mount(itemElement, requestRerender); cc.Content = ctrl; diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index bf07b789f..d30b1e2f0 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -176,6 +176,10 @@ public sealed partial class Reconciler ctxCount = ctxValues.Count; } + // Virtualized hosts realize rows outside this traversal; keep their provider + // snapshot current (no-op for every other control). + RefreshContextCapture(control); + UIElement? result; try { diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index d6ddb45c5..96b2d53e3 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -818,6 +818,11 @@ internal sealed class ReactorState // abstract base. public Internal.IItemViewSource? ItemViewSource; + // Provider stack snapshot for a virtualized host whose rows are realized + // from WinUI layout callbacks outside the reconciler traversal (see + // ContextCapture). Lives here rather than in a wrapper-keyed CWT so the + // realization callback finds it through any managed wrapper of the control. + public ContextCapture? ContextCapture; // Spec 047 §9.2 / §14 Phase 1 (1.7) — per-control event payload box. // Holds a strongly-typed payload struct (e.g. ToggleSwitchEventPayload) // discriminated by HandlerType. Populated for ported V1 controls and @@ -1202,6 +1207,9 @@ internal static void ClearCurrentEventHandlers(FrameworkElement fe) state.EchoSuppressScopeDepth = 0; state.PendingEchoMatch = null; state.PendingLabeledBy = null; + // A pooled host must not hand its provider values (and the objects they + // reference) to an unrelated renter. + state.ContextCapture = null; } } @@ -1248,6 +1256,7 @@ public static void DetachReactorState(FrameworkElement fe) state.ValidationRootBinding = null; state.ValidationRuleBinding = null; state.ValidationAttachedBinding = null; + state.ContextCapture = null; } /// @@ -1896,6 +1905,89 @@ internal IDisposable PushContextDisposable(Context context, T value) internal bool HasActiveContextValues => _contextScope.HasActiveValues; + // ── Context replay for virtualized realization ───────────────────── + // + // ItemsRepeater (IElementFactory.GetElement), ListViewBase + // (ContainerContentChanging) and the typed TreeView build row content from + // WinUI layout callbacks. Those run after the traversal that maintains + // _contextScope has unwound (first realization, scrolling) or re-entrantly in + // the middle of an unrelated subtree's update (a sibling's synchronous + // UpdateLayout), so the live stack is either empty or someone else's. Each + // such host captures the stack whenever it is reconciled, and the realization + // callbacks replay that capture around the row mount. + + // Gates the per-control capture lookups so apps without a virtualized host + // never pay the attached-state read. + private bool _hasContextCaptures; + + private static bool MayHostContextCapture(UIElement control) + => control is WinUI.ScrollViewer or WinUI.ItemsRepeater or WinUI.ItemsView + or WinUI.ListViewBase or WinUI.TreeView; + + /// + /// Registers as a virtualized host and snapshots the live + /// provider stack onto it. Call from the host's Mount, while the traversal is at + /// the host; the generic Update path refreshes the snapshot from then on. + /// + internal ContextCapture CaptureContextForHost(FrameworkElement host) + { + global::System.Diagnostics.Debug.Assert(MayHostContextCapture(host), + $"{host.GetType().Name} is not covered by MayHostContextCapture; its capture would never refresh."); + var state = GetOrCreateReactorState(host); + var capture = state.ContextCapture ??= new ContextCapture(); + capture.Refresh(_contextScope); + _hasContextCaptures = true; + return capture; + } + + private ContextCapture? TryGetContextCapture(UIElement control) + => _hasContextCaptures + && MayHostContextCapture(control) + && ((FrameworkElement)control).GetValue(ReactorAttached.StateProperty) is ReactorState { ContextCapture: { } capture } + ? capture + : null; + + /// Re-snapshots a host's provider stack; a no-op for any other control. + internal void RefreshContextCapture(UIElement control) + => TryGetContextCapture(control)?.Refresh(_contextScope); + + /// + /// Replaces the live provider stack with until the + /// returned scope is disposed. A null capture is a no-op. + /// + internal CapturedContextScope EnterCapturedContext(ContextCapture? capture) + => capture is null + ? default + : new CapturedContextScope(_contextScope, _contextScope.Replace(capture.Entries)); + + /// + internal CapturedContextScope EnterCapturedContext(UIElement host) + => EnterCapturedContext(TryGetContextCapture(host)); + + /// + /// Registers (see ) and + /// hands its capture to so GetElement replays it. + /// + internal void AttachContextCapture(IElementFactory factory, FrameworkElement host) + { + if (factory is IContextCapturingFactory capturing) + capturing.ContextCapture = CaptureContextForHost(host); + } + + internal readonly struct CapturedContextScope : IDisposable + { + private readonly ContextScope? _scope; + private readonly List<(ContextBase Context, object? Value)>? _previous; + + internal CapturedContextScope(ContextScope scope, List<(ContextBase Context, object? Value)> previous) + { + _scope = scope; + _previous = previous; + } + + public void Dispose() => _scope?.Restore(_previous!); + } + /// /// Spec 047 §14 Phase 1 (1.6) — push a stagger scope for child enter /// transitions. Returns an that pops the @@ -2753,6 +2845,12 @@ internal bool HasConsumedContextChangedInSubtree(UIElement control) && HasConsumedContextChanged(node)) return true; + // A virtualized host has no realized consumer for rows it has yet to realize, + // so its own snapshot stands in for them: if the providers above it changed, + // decline the skip so the Update path re-snapshots before the next realization. + if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope)) + return true; + bool found = false; ForEachReactorChildControl(control, child => { diff --git a/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs b/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs index 679b4970b..863457eed 100644 --- a/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs +++ b/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs @@ -36,6 +36,7 @@ public WinUI.GridView Mount(MountContext ctx, GridViewElement gv) if (gv.ItemContainerStyle is not null) gridView.ItemContainerStyle = gv.ItemContainerStyle; Reconciler.SetElementTag(gridView, gv); + reconciler.CaptureContextForHost(gridView); gridView.ItemTemplate = Reconciler.SharedContentControlTemplate.Value; @@ -58,6 +59,7 @@ public WinUI.GridView Mount(MountContext ctx, GridViewElement gv) if (items is not null && args.ItemIndex >= 0 && args.ItemIndex < items.Length && args.ItemContainer.ContentTemplateRoot is ContentControl cc) { + using var restoreContext = reconciler.EnterCapturedContext((UIElement)sender!); var ctrl = reconciler.Mount(items[args.ItemIndex], requestRerender); cc.Content = ctrl; // Issue #951 — keep .AutomationName(...) on an item view working diff --git a/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs b/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs index 39e63feaa..58b0390c8 100644 --- a/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs +++ b/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs @@ -37,6 +37,7 @@ public WinUI.ListView Mount(MountContext ctx, ListViewElement lv) if (lv.ItemContainerStyle is not null) listView.ItemContainerStyle = lv.ItemContainerStyle; Reconciler.SetElementTag(listView, lv); + reconciler.CaptureContextForHost(listView); // DataTemplate with a ContentControl shell — we populate its Content on demand listView.ItemTemplate = Reconciler.SharedContentControlTemplate.Value; @@ -60,6 +61,7 @@ public WinUI.ListView Mount(MountContext ctx, ListViewElement lv) if (items is not null && args.ItemIndex >= 0 && args.ItemIndex < items.Length && args.ItemContainer.ContentTemplateRoot is ContentControl cc) { + using var restoreContext = reconciler.EnterCapturedContext((UIElement)sender!); var ctrl = reconciler.Mount(items[args.ItemIndex], requestRerender); cc.Content = ctrl; // Issue #951 — keep .AutomationName(...) on an item view working diff --git a/src/Reactor/Core/V1Protocol/LazyStackLifecycle.cs b/src/Reactor/Core/V1Protocol/LazyStackLifecycle.cs index 655b8d517..79a9dc6f2 100644 --- a/src/Reactor/Core/V1Protocol/LazyStackLifecycle.cs +++ b/src/Reactor/Core/V1Protocol/LazyStackLifecycle.cs @@ -43,6 +43,7 @@ internal static UIElement Mount(Reconciler reconciler, LazyStackElementBase lazy Reconciler.ApplySetters(lazy.RepeaterSetters, repeater); var sv = reconciler._pool.TryRent(typeof(WinUI.ScrollViewer)) as WinUI.ScrollViewer ?? new WinUI.ScrollViewer(); + reconciler.AttachContextCapture(factory, sv); sv.Content = repeater; sv.HorizontalScrollBarVisibility = lazy.Orientation == Microsoft.UI.Xaml.Controls.Orientation.Horizontal ? WinUI.ScrollBarVisibility.Auto @@ -87,6 +88,7 @@ internal static UIElement Mount(Reconciler reconciler, LazyStackElementBase lazy repeater.ItemsSource = fresh.Source; var factory = n.CreateFactory(reconciler, requestRerender, reconciler._pool); n.AttachListStateToFactory(factory, fresh); + reconciler.AttachContextCapture(factory, sv); repeater.ItemTemplate = factory; } if (repeater.Layout is WinUI.StackLayout layout) diff --git a/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs b/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs index 4e84b2b1f..9ccd41893 100644 --- a/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs +++ b/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs @@ -44,6 +44,7 @@ internal WinUI.TreeView MountTemplatedTreeView(TemplatedTreeViewElementBase el, treeView.RootNodes.Add(BuildTemplatedTreeNode(el, root)); SetElementTag(treeView, el); + CaptureContextForHost(treeView); // Trampolines resolve the live element + the data item (node.Content) // on dispatch, so they're wired unconditionally and never need @@ -174,6 +175,9 @@ private void PopulateTypedTreeContainer( if (cc.Content is not null) return; // already hosted for this realization if (item is not WinUI.TreeViewNode node || node.Content is not { } data) return; + // Runs from ContainerContentChanging / Loaded / LayoutUpdated, outside the + // traversal — replay the tree's provider stack for the node view. + using var restoreContext = EnterCapturedContext(treeView); var view = el.BuildView(data); cc.Content = Mount(view, requestRerender); SetElementTag(cc, view); diff --git a/src/Reactor/Core/V1Protocol/TemplatedListLifecycle.cs b/src/Reactor/Core/V1Protocol/TemplatedListLifecycle.cs index 53c575fc7..b14592b3e 100644 --- a/src/Reactor/Core/V1Protocol/TemplatedListLifecycle.cs +++ b/src/Reactor/Core/V1Protocol/TemplatedListLifecycle.cs @@ -52,6 +52,7 @@ internal static WinUI.ListView MountListView(Reconciler reconciler, TemplatedLis if (header is not null) listView.Header = header; Reconciler.SetElementTag(listView, el); + reconciler.CaptureContextForHost(listView); listView.ItemTemplate = Reconciler.SharedContentControlTemplate.Value; listView.ContainerContentChanging += (sender, args) => @@ -133,6 +134,7 @@ internal static WinUI.GridView MountGridView(Reconciler reconciler, TemplatedLis if (header is not null) gridView.Header = header; Reconciler.SetElementTag(gridView, el); + reconciler.CaptureContextForHost(gridView); gridView.ItemTemplate = Reconciler.SharedContentControlTemplate.Value; gridView.ContainerContentChanging += (sender, args) => diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs new file mode 100644 index 000000000..0a9588e77 --- /dev/null +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs @@ -0,0 +1,216 @@ +using Microsoft.UI.Reactor; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.AppTests.Host.SelfTest; +using Microsoft.UI.Reactor.Navigation; +using Microsoft.UI.Xaml.Controls; +using static Microsoft.UI.Reactor.Factories; + +namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; + +/// +/// Virtualized hosts realize rows from WinUI layout callbacks +/// (IElementFactory.GetElement, ContainerContentChanging) that run +/// after the reconciler traversal has unwound, so a row that consumes context used +/// to read the provider's default (and UseNavigation<T>() threw) until +/// the parent happened to re-render (issue #1386). Each host now snapshots the provider stack when +/// reconciled and replays it around the row mount. +/// +internal static class VirtualizedRowContextFixtures +{ + private static readonly Context RowCtx = new("default"); + + private sealed record RowProps(int Index); + + private sealed class RowConsumer : Component + { + public override Element Render() + => TextBlock($"row{Props.Index}:{UseContext(RowCtx)}").Height(30); + } + + private sealed class EagerConsumer : Component + { + public override Element Render() => TextBlock($"eager:{UseContext(RowCtx)}"); + } + + private static Element Row(int i) => Component(new RowProps(i)); + + private static List RowTexts(Harness h, int minIndex = 0) + => h.FindAllControls(tb => tb.Text?.StartsWith("row", StringComparison.Ordinal) == true) + .Select(tb => tb.Text) + .Where(t => int.TryParse(t.AsSpan(3, t.IndexOf(':') - 3), out var i) && i >= minIndex) + .ToList(); + + private static bool AllEndWith(List texts, string suffix) + => texts.Count > 0 && texts.All(t => t.EndsWith(suffix, StringComparison.Ordinal)); + + private static string Sample(List texts) + => texts.Count == 0 ? "" : string.Join(",", texts.Take(5)); + + private static async Task ScrollListTo(Harness h, double offset) + { + var sv = h.FindControl(s => s.Content is ItemsRepeater) + ?? throw new InvalidOperationException("LazyVStack ScrollViewer not found"); + sv.ChangeView(null, offset, null, disableAnimation: true); + await Harness.Render(); + await Harness.Render(); + } + + internal sealed class LazyVStack_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 300).ToList(); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + return VStack( + Button("VCtxBump", () => setTick(tick + 1)), + TextBlock($"vctx-tick:{tick}"), + Component(), + LazyVStack(items, i => i.ToString(), (i, _) => Row(i)).Height(300)) + .Provide(RowCtx, "provided"); + }); + await Harness.Render(); + + // Positive control: the same consumer outside the virtualized host. + H.Check("VirtualizedCtx_Lazy_EagerControl", H.FindText("eager:provided") is not null); + + var initial = RowTexts(H); + H.Check("VirtualizedCtx_Lazy_InitialRows", AllEndWith(initial, ":provided"), Sample(initial)); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_Lazy_ScrolledRows", AllEndWith(scrolled, ":provided"), Sample(scrolled)); + + H.ClickButton("VCtxBump"); + await Harness.Render(); + H.Check("VirtualizedCtx_Lazy_Bumped", H.FindText("vctx-tick:1") is not null); + var afterBump = RowTexts(H); + H.Check("VirtualizedCtx_Lazy_RowsAfterRerender", AllEndWith(afterBump, ":provided"), Sample(afterBump)); + } + } + + // The list element is reference-stable and the realized rows do not consume the + // context, so nothing realized tells the reconciler the provider changed. Rows + // realized afterwards must still see the new value, not the mount-time snapshot. + internal sealed class LazyVStack_ProviderChangeWhileSkipped(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 300).ToList(); + var stableList = LazyVStack(items, i => i.ToString(), (i, _) => + i < 100 ? TextBlock($"plain{i}").Height(30) : Row(i)) + .Height(300); + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (value, setValue) = ctx.UseState("first"); + return VStack( + Button("VCtxChange", () => setValue("second")), + TextBlock($"vctx-value:{value}"), + stableList) + .Provide(RowCtx, value); + }); + await Harness.Render(); + H.Check("VirtualizedCtx_Skip_NoConsumerRealized", RowTexts(H).Count == 0, Sample(RowTexts(H))); + + H.ClickButton("VCtxChange"); + await Harness.Render(); + H.Check("VirtualizedCtx_Skip_ProviderChanged", H.FindText("vctx-value:second") is not null); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_Skip_RowsSeeNewValue", AllEndWith(scrolled, ":second"), Sample(scrolled)); + } + } + + private sealed record NavRowProps(int Index); + + private sealed class NavRow : Component + { + public override Element Render() + { + var nav = UseNavigation(); + return Button($"open{Props.Index}", () => nav.Navigate($"detail{Props.Index}")).Height(30); + } + } + + // The common list → detail shape: child-mode UseNavigation in a virtualized row + // used to throw "found no ancestor NavigationHost" on every realization. + internal sealed class LazyVStack_UseNavigationChildMode(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 100).ToList(); + var host = H.CreateHost(); + host.Mount(ctx => + { + var nav = ctx.UseNavigation("list"); + return NavigationHost(nav, route => route == "list" + ? LazyVStack(items, i => i.ToString(), (i, _) => + Component(new NavRowProps(i))).Height(300) + : TextBlock($"page:{route}")) with { Transition = NavigationTransition.None }; + }); + await Harness.Render(); + + var open0 = H.FindButton("open0"); + H.Check("VirtualizedCtx_Nav_RowRendered", open0 is not null, + H.FindTextContaining("UseNavigation")?.Text ?? ""); + if (open0 is null) return; + + H.ClickButton("open0"); + await Harness.Render(); + H.Check("VirtualizedCtx_Nav_RowNavigates", H.FindText("page:detail0") is not null); + } + } + + internal sealed class ListView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 50).ToList(); + var host = H.CreateHost(); + host.Mount(_ => VStack( + ListView(items, i => i.ToString(), (i, _) => Row(i)).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_ListView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); + } + } + + internal sealed class ItemsView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 50).ToList(); + var host = H.CreateHost(); + host.Mount(_ => VStack( + ItemsView(items, i => i.ToString(), (i, _) => ItemContainer(Row(i))).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_ItemsView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); + } + } + + internal sealed class TreeView_NodesSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var roots = Enumerable.Range(0, 5).ToList(); + var host = H.CreateHost(); + host.Mount(_ => VStack( + TreeView(roots, i => i.ToString(), _ => null, i => Row(i)).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_TreeView_Nodes", AllEndWith(rows, ":provided"), Sample(rows)); + } + } +} diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 689904857..680223710 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -617,6 +617,12 @@ internal static class SelfTestFixtureRegistry "Issue811_SplitViewContent_ContextConsumerRerenders", "Issue811_ViewboxNested_ContextConsumerRerenders", "Issue811_ActiveButUnchangedContext_StillSkips", + "VirtualizedCtx_LazyVStack_RowsSeeProvider", + "VirtualizedCtx_LazyVStack_ProviderChangeWhileSkipped", + "VirtualizedCtx_LazyVStack_UseNavigationChildMode", + "VirtualizedCtx_ListView_RowsSeeProvider", + "VirtualizedCtx_ItemsView_RowsSeeProvider", + "VirtualizedCtx_TreeView_NodesSeeProvider", "SelfTrigReuse_PositionalReusedInstance", "SelfTrigReuse_PositionalMemoizedWrapper", "SelfTrigReuse_PositionalNestedStableTree", @@ -2667,6 +2673,12 @@ internal static string[] StaleTierDeclarations() => "Issue811_SplitViewContent_ContextConsumerRerenders" => new Issue811ContextConsumerSkipFixtures.SplitViewContent_ContextConsumerRerenders(harness), "Issue811_ViewboxNested_ContextConsumerRerenders" => new Issue811ContextConsumerSkipFixtures.ViewboxNested_ContextConsumerRerenders(harness), "Issue811_ActiveButUnchangedContext_StillSkips" => new Issue811ContextConsumerSkipFixtures.ActiveButUnchangedContext_StillSkips(harness), + "VirtualizedCtx_LazyVStack_RowsSeeProvider" => new VirtualizedRowContextFixtures.LazyVStack_RowsSeeProvider(harness), + "VirtualizedCtx_LazyVStack_ProviderChangeWhileSkipped" => new VirtualizedRowContextFixtures.LazyVStack_ProviderChangeWhileSkipped(harness), + "VirtualizedCtx_LazyVStack_UseNavigationChildMode" => new VirtualizedRowContextFixtures.LazyVStack_UseNavigationChildMode(harness), + "VirtualizedCtx_ListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.ListView_RowsSeeProvider(harness), + "VirtualizedCtx_ItemsView_RowsSeeProvider" => new VirtualizedRowContextFixtures.ItemsView_RowsSeeProvider(harness), + "VirtualizedCtx_TreeView_NodesSeeProvider" => new VirtualizedRowContextFixtures.TreeView_NodesSeeProvider(harness), "SelfTrigReuse_PositionalReusedInstance" => new SelfTriggeredReusedChildFixtures.PositionalReusedInstance(harness), "SelfTrigReuse_PositionalMemoizedWrapper" => new SelfTriggeredReusedChildFixtures.PositionalMemoizedWrapper(harness), "SelfTrigReuse_PositionalNestedStableTree" => new SelfTriggeredReusedChildFixtures.PositionalNestedStableTree(harness), From 2cbc3c3b5701bef0d1ea65ba60e99057e678331e Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Thu, 15 Oct 2026 17:21:57 -0700 Subject: [PATCH 02/10] Address pr-review findings - Detect a provider removed above a skipped virtualized host: count non-skip Updates whose own ContextValues changed and treat any host snapshot (and the context skip gate) below them as stale. Also fixes a reference-stable consumer keeping a removed provider's value when the last provider is dropped. - Re-snapshot on the element skip arm so a host sharing its control with a decorator (Flyout, ...) keeps a .Provide() on the list element itself. - ElementFactory captures its own snapshot when no host capture is attached, so out-of-tree handlers built on it are covered. - Refresh reuses the snapshot array when nothing changed. - Unit tests for ContextScope/ContextCapture primitives; selftests for provider removal, decorator-wrapped list, typed GridView and untyped ListView/GridView. CHANGELOG entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- CHANGELOG.md | 15 ++ src/Reactor/Core/ContextScope.cs | 37 ++- src/Reactor/Core/ElementFactory.cs | 14 +- src/Reactor/Core/Reconciler.Update.cs | 9 + src/Reactor/Core/Reconciler.cs | 53 ++++- .../Fixtures/VirtualizedRowContextFixtures.cs | 164 +++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 14 ++ .../Reactor.Tests/ContextCaptureUnitTests.cs | 220 ++++++++++++++++++ 8 files changed, 518 insertions(+), 8 deletions(-) create mode 100644 tests/Reactor.Tests/ContextCaptureUnitTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index e5d336ad5..5732b7750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -291,6 +291,21 @@ Conventions for contributors: ### Fixed +- **Virtualized list rows now see their ancestors' `Context`** (issue #1386). Rows of + `LazyVStack` / `LazyHStack`, `ItemsRepeater`, `ItemsView`, `ListView` / `GridView` + (typed and untyped) and `TreeView` are built by WinUI during layout, after the + reconciler has finished walking the tree, so `UseContext` in a row returned the + context's default value, and hooks built on it misbehaved: child-mode + `UseNavigation()` threw "found no ancestor NavigationHost", `UseIntl()` ignored a + `LocaleProvider`, and `UseResource` ignored an enclosing `Pending`. A re-render of the + parent temporarily fixed the visible rows, but rows that scrolled in afterwards were + wrong again. Each list now saves its ancestors' provided values when it is reconciled + and restores them while building a row, so rows see the nearest `.Provide(...)` on first + display and while scrolling. Rows that previously read a default will now read the + provided value. Lists built by a custom handler on `ElementFactory` are covered too; + a custom handler that builds rows itself from `ContainerContentChanging` is not. + Removing the only `.Provide(...)` above a reference-stable consumer now also re-renders + that consumer with the default, which it previously missed. - **Replacing a host's root no longer leaves the old root running** (issue #1326). `ReactorHost.Mount(...)` and `ReactorHostControl.Mount(...)` now retire the outgoing root before installing the new one: its effect cleanups run, and a diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index b2b4d5779..8c8c362ac 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -55,11 +55,31 @@ internal T Read(Context context) internal (ContextBase Context, object? Value)[] Snapshot() => _stack.Count == 0 ? [] : _stack.ToArray(); + /// + /// True when the live stack holds exactly : same contexts, + /// same value instances. Lets a refresh keep the existing snapshot array instead of + /// allocating a new one on every reconcile of the host. + /// + internal bool Matches((ContextBase Context, object? Value)[] snapshot) + { + if (_stack.Count != snapshot.Length) return false; + for (int i = 0; i < _stack.Count; i++) + { + if (!ReferenceEquals(_stack[i].Context, snapshot[i].Context) + || !ReferenceEquals(_stack[i].Value, snapshot[i].Value)) + return false; + } + return true; + } + /// /// True when the live stack is a positional prefix of /// (same contexts, equal values). The traversal pushes providers along the path, /// so at any ancestor of the node that took the snapshot the live stack is a - /// prefix of it unless a provider at or above that ancestor changed. + /// prefix of it unless a provider at or above that ancestor changed value or was + /// added. A removed provider leaves a shorter stack that is still a prefix, + /// so callers must detect removal separately (see + /// Reconciler.InProviderChangeSubtree). /// internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) { @@ -106,7 +126,18 @@ internal sealed class ContextCapture { internal (ContextBase Context, object? Value)[] Entries { get; private set; } = []; - internal void Refresh(ContextScope scope) => Entries = scope.Snapshot(); + internal void Refresh(ContextScope scope) + { + if (!scope.Matches(Entries)) + Entries = scope.Snapshot(); + } - internal bool IsStale(ContextScope scope) => !scope.IsPrefixOf(Entries); + /// The live stack at an ancestor of (or at) the host. + /// + /// True while the traversal is below an element whose provided values changed this + /// pass. A removed provider is invisible to the prefix test, so any snapshot taken + /// beneath such an element has to be treated as stale. + /// + internal bool IsStale(ContextScope scope, bool inProviderChangeSubtree) + => inProviderChangeSubtree || !scope.IsPrefixOf(Entries); } diff --git a/src/Reactor/Core/ElementFactory.cs b/src/Reactor/Core/ElementFactory.cs index c5a3fc8b0..e0a3cd4ed 100644 --- a/src/Reactor/Core/ElementFactory.cs +++ b/src/Reactor/Core/ElementFactory.cs @@ -27,9 +27,14 @@ public sealed partial class ElementFactory : IElementFactory, IContextCapturi // The host's provider-stack snapshot. GetElement runs from WinUI layout, // outside the traversal that maintains the reconciler's context stack, so - // rows replay this instead (see ContextCapture). Null when the factory is - // driven directly (tests) — rows then read the live stack as before. + // rows replay this instead (see ContextCapture). Built-in hosts attach the + // capture registered on their control, which the reconciler also refreshes + // when the host is skipped. A factory installed by an out-of-tree handler has + // none attached and falls back to _ownCapture, which it takes itself at + // construction and in UpdateInPlace — both run inside the host's Mount/Update, + // where the live stack is the host's. ContextCapture? IContextCapturingFactory.ContextCapture { get; set; } + private readonly ContextCapture _ownCapture; // Optional state used when ItemsSource is the OC path // (spec 042). Lets GetElement translate an ItemsRepeater realized // index → stable key for _mountedElements lookup. Null when running @@ -294,6 +299,7 @@ public ElementFactory( _reconciler = reconciler; _requestRerender = requestRerender; _pool = pool; + _ownCapture = reconciler.CaptureLiveContext(); } /// @@ -307,6 +313,8 @@ internal void UpdateInPlace(IReadOnlyList items, Func viewBu { _items = items; _viewBuilder = viewBuilder; + if (((IContextCapturingFactory)this).ContextCapture is null) + _reconciler.RefreshLiveContext(_ownCapture); // A new viewBuilder closure may capture different external state (UseState // cells, Observable subscriptions, theme, etc.) than the one that produced the // cached entries. We can't see through @@ -601,7 +609,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterCapturedContext(((IContextCapturingFactory)this).ContextCapture); + using var restoreContext = _reconciler.EnterCapturedContext(((IContextCapturingFactory)this).ContextCapture ?? _ownCapture); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index d30b1e2f0..39a0e82b3 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -128,6 +128,10 @@ public sealed partial class Reconciler if ((HasGestureOrDragSlots(modifiers) || HasGestureOrDragSlots(oldModifiers)) && control is FrameworkElement gestFeSE) RefreshGestureDragStateOnSkip(gestFeSE, oldModifiers, modifiers); + // A virtualized host that shares its control with a decorator was refreshed + // by the decorator's Update at the wrong depth; re-snapshot here (no-op for + // every other control, and allocation-free when nothing changed). + RefreshContextCaptureOnSkip(control, newEl); // No validation refresh here, deliberately (issue #1262). A `.Validate()` // verdict is republished only by a component that re-rendered, and that // component's output is on the dirty ancestor path, which this arm declines @@ -175,6 +179,9 @@ public sealed partial class Reconciler _contextScope.Push(ctxValues); ctxCount = ctxValues.Count; } + bool providerChanged = !Element.ContextValuesEqual(oldEl.ContextValues, ctxValues); + if (providerChanged) + EnterProviderChange(); // Virtualized hosts realize rows outside this traversal; keep their provider // snapshot current (no-op for every other control). @@ -356,6 +363,8 @@ public sealed partial class Reconciler } finally { + if (providerChanged) + ExitProviderChange(); if (ctxCount > 0) _contextScope.Pop(ctxCount); _ambientRequestedTheme = prevAmbientTheme; diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index 96b2d53e3..590071a2d 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -1903,7 +1903,22 @@ internal IDisposable PushContextDisposable(Context context, T value) // without exposing the traversal stack itself. internal T ReadContext(Context context) => _contextScope.Read(context); - internal bool HasActiveContextValues => _contextScope.HasActiveValues; + internal bool HasActiveContextValues => _contextScope.HasActiveValues || _providerChangeDepth > 0; + + // Depth of non-skip Updates, on the current traversal path, whose element's own + // ContextValues differ from the previous render's (a provider added, removed or + // changed). Removing the LAST provider leaves the live stack empty, which used to + // short-circuit every context skip check (HasActiveContextValues) so a + // reference-stable consumer below kept the removed value; removing one of several + // leaves a shorter stack that a host's snapshot prefix test cannot tell apart from + // an ancestor's view. Counting the change lets both checks see it. + private int _providerChangeDepth; + + internal bool InProviderChangeSubtree => _providerChangeDepth > 0; + + internal void EnterProviderChange() => _providerChangeDepth++; + + internal void ExitProviderChange() => _providerChangeDepth--; // ── Context replay for virtualized realization ───────────────────── // @@ -1951,6 +1966,40 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) internal void RefreshContextCapture(UIElement control) => TryGetContextCapture(control)?.Refresh(_contextScope); + /// + /// Skip-arm counterpart of . A host whose + /// control is shared with a decorator (Flyout, MenuFlyout, …) is first refreshed by + /// the decorator's Update, before the host element's own ContextValues are on + /// the stack; when the host element then skips, this re-snapshots at its own depth so + /// a .Provide() on the list element itself is not lost. + /// + internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) + { + if (TryGetContextCapture(control) is not { } capture) return; + var own = newEl.ContextValues; + if (own is not { Count: > 0 }) + { + capture.Refresh(_contextScope); + return; + } + _contextScope.Push(own); + try { capture.Refresh(_contextScope); } + finally { _contextScope.Pop(own.Count); } + } + + /// + /// A standalone snapshot of the live provider stack, for an + /// whose host never registered one (an out-of-tree handler installing the factory). + /// + internal ContextCapture CaptureLiveContext() + { + var capture = new ContextCapture(); + capture.Refresh(_contextScope); + return capture; + } + + internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope); + /// /// Replaces the live provider stack with until the /// returned scope is disposed. A null capture is a no-op. @@ -2848,7 +2897,7 @@ internal bool HasConsumedContextChangedInSubtree(UIElement control) // A virtualized host has no realized consumer for rows it has yet to realize, // so its own snapshot stands in for them: if the providers above it changed, // decline the skip so the Update path re-snapshots before the next realization. - if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope)) + if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope, InProviderChangeSubtree)) return true; bool found = false; diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs index 0a9588e77..f46cff318 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs @@ -126,6 +126,123 @@ public override async Task RunAsync() } } + private static readonly Context OuterCtx = new("outer-default"); + + private static LazyVStackElement HalfConsumerList() + => LazyVStack(Enumerable.Range(0, 300).ToList(), i => i.ToString(), (i, _) => + i < 100 ? TextBlock($"plain{i}").Height(30) : Row(i)) + .Height(300); + + // A removed provider leaves a shorter live stack that is still a prefix of the + // skipped host's snapshot, so removal needs its own signal. + internal sealed class LazyVStack_OnlyProviderRemovedWhileSkipped(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var stableList = HalfConsumerList(); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (on, setOn) = ctx.UseState(true); + var body = VStack( + Button("VCtxDropOnly", () => setOn(false)), + TextBlock($"vctx-on:{on}"), + stableList); + return on ? body.Provide(RowCtx, "removed") : body; + }); + await Harness.Render(); + + H.ClickButton("VCtxDropOnly"); + await Harness.Render(); + H.Check("VirtualizedCtx_DropOnly_Toggled", H.FindText("vctx-on:False") is not null); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_DropOnly_RowsSeeDefault", AllEndWith(scrolled, ":default"), Sample(scrolled)); + } + } + + internal sealed class LazyVStack_InnerProviderRemovedWhileSkipped(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var stableList = HalfConsumerList(); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (on, setOn) = ctx.UseState(true); + var inner = Border(stableList); + return VStack( + Button("VCtxDropInner", () => setOn(false)), + TextBlock($"vctx-inner:{on}"), + on ? inner.Provide(RowCtx, "removed") : inner) + .Provide(OuterCtx, "stable"); + }); + await Harness.Render(); + + H.ClickButton("VCtxDropInner"); + await Harness.Render(); + H.Check("VirtualizedCtx_DropInner_Toggled", H.FindText("vctx-inner:False") is not null); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_DropInner_RowsSeeDefault", AllEndWith(scrolled, ":default"), Sample(scrolled)); + } + } + + // Same signal, non-virtualized: removing the LAST provider used to empty the stack, + // which short-circuited the reference-stable consumer check entirely. + internal sealed class StableConsumer_LastProviderRemoved(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var stableConsumer = Component(); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (on, setOn) = ctx.UseState(true); + var body = VStack(Button("VCtxDropEager", () => setOn(false)), stableConsumer); + return on ? body.Provide(RowCtx, "removed") : body; + }); + await Harness.Render(); + H.Check("VirtualizedCtx_DropEager_Before", H.FindText("eager:removed") is not null); + + H.ClickButton("VCtxDropEager"); + await Harness.Render(); + H.Check("VirtualizedCtx_DropEager_SeesDefault", H.FindText("eager:default") is not null, + H.FindTextContaining("eager:")?.Text ?? ""); + } + } + + // A decorator (Flyout) shares the host's control and refreshes its snapshot before + // the list element's own .Provide() is on the stack; the list then skips. + internal sealed class LazyVStack_OwnProviderUnderDecorator(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var stableList = HalfConsumerList().Provide(RowCtx, "own"); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + return VStack( + Button("VCtxDecoBump", () => setTick(tick + 1)), + TextBlock($"vctx-deco:{tick}"), + Flyout(stableList, TextBlock($"fly{tick}"))) + .Provide(RowCtx, "outer"); + }); + await Harness.Render(); + + H.ClickButton("VCtxDecoBump"); + await Harness.Render(); + H.Check("VirtualizedCtx_Deco_Bumped", H.FindText("vctx-deco:1") is not null); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_Deco_RowsSeeOwnProvider", AllEndWith(scrolled, ":own"), Sample(scrolled)); + } + } + private sealed record NavRowProps(int Index); private sealed class NavRow : Component @@ -182,6 +299,53 @@ public override async Task RunAsync() } } + internal sealed class GridView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var items = Enumerable.Range(0, 50).ToList(); + var host = H.CreateHost(); + host.Mount(_ => VStack( + GridView(items, i => i.ToString(), (i, _) => Row(i)).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_GridView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); + } + } + + // The untyped ListView/GridView handlers have their own ContainerContentChanging bodies. + internal sealed class UntypedListView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(_ => VStack( + ListView(Enumerable.Range(0, 20).Select(Row).ToArray()).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_UntypedListView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); + } + } + + internal sealed class UntypedGridView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(_ => VStack( + GridView(Enumerable.Range(0, 20).Select(Row).ToArray()).Height(300)) + .Provide(RowCtx, "provided")); + await Harness.Render(); + + var rows = RowTexts(H); + H.Check("VirtualizedCtx_UntypedGridView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); + } + } + internal sealed class ItemsView_RowsSeeProvider(Harness h) : SelfTestFixtureBase(h) { public override async Task RunAsync() diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 680223710..9c7078d1b 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -619,6 +619,13 @@ internal static class SelfTestFixtureRegistry "Issue811_ActiveButUnchangedContext_StillSkips", "VirtualizedCtx_LazyVStack_RowsSeeProvider", "VirtualizedCtx_LazyVStack_ProviderChangeWhileSkipped", + "VirtualizedCtx_LazyVStack_OnlyProviderRemovedWhileSkipped", + "VirtualizedCtx_LazyVStack_InnerProviderRemovedWhileSkipped", + "VirtualizedCtx_StableConsumer_LastProviderRemoved", + "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator", + "VirtualizedCtx_GridView_RowsSeeProvider", + "VirtualizedCtx_UntypedListView_RowsSeeProvider", + "VirtualizedCtx_UntypedGridView_RowsSeeProvider", "VirtualizedCtx_LazyVStack_UseNavigationChildMode", "VirtualizedCtx_ListView_RowsSeeProvider", "VirtualizedCtx_ItemsView_RowsSeeProvider", @@ -2675,6 +2682,13 @@ internal static string[] StaleTierDeclarations() => "Issue811_ActiveButUnchangedContext_StillSkips" => new Issue811ContextConsumerSkipFixtures.ActiveButUnchangedContext_StillSkips(harness), "VirtualizedCtx_LazyVStack_RowsSeeProvider" => new VirtualizedRowContextFixtures.LazyVStack_RowsSeeProvider(harness), "VirtualizedCtx_LazyVStack_ProviderChangeWhileSkipped" => new VirtualizedRowContextFixtures.LazyVStack_ProviderChangeWhileSkipped(harness), + "VirtualizedCtx_LazyVStack_OnlyProviderRemovedWhileSkipped" => new VirtualizedRowContextFixtures.LazyVStack_OnlyProviderRemovedWhileSkipped(harness), + "VirtualizedCtx_LazyVStack_InnerProviderRemovedWhileSkipped" => new VirtualizedRowContextFixtures.LazyVStack_InnerProviderRemovedWhileSkipped(harness), + "VirtualizedCtx_StableConsumer_LastProviderRemoved" => new VirtualizedRowContextFixtures.StableConsumer_LastProviderRemoved(harness), + "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderDecorator(harness), + "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), + "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), + "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), "VirtualizedCtx_LazyVStack_UseNavigationChildMode" => new VirtualizedRowContextFixtures.LazyVStack_UseNavigationChildMode(harness), "VirtualizedCtx_ListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.ListView_RowsSeeProvider(harness), "VirtualizedCtx_ItemsView_RowsSeeProvider" => new VirtualizedRowContextFixtures.ItemsView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs new file mode 100644 index 000000000..23250bd2d --- /dev/null +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -0,0 +1,220 @@ +using Microsoft.UI.Reactor.Core; +using Xunit; + +namespace Microsoft.UI.Reactor.Tests; + +/// +/// Issue #1386 — the snapshot/replay primitives virtualized hosts use to give rows +/// realized outside the reconciler traversal their ancestors' context values. +/// +public class ContextCaptureUnitTests +{ + private static readonly Context Theme = new("light"); + private static readonly Context Count = new(0); + + private sealed record Payload(string Name); + private static readonly Context Data = new(defaultValue: null); + + private static void Push(ContextScope scope, ContextBase ctx, object? value) + => scope.Push(new Dictionary { [ctx] = value }); + + // ── IsPrefixOf ─────────────────────────────────────────────────── + + [Fact] + public void IsPrefixOf_EmptyLiveStack_IsPrefixOfAnything() + { + var snapshotSource = new ContextScope(); + Push(snapshotSource, Theme, "dark"); + Assert.True(new ContextScope().IsPrefixOf(snapshotSource.Snapshot())); + } + + [Fact] + public void IsPrefixOf_EqualStack_IsPrefix() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + Push(scope, Count, 3); + Assert.True(scope.IsPrefixOf(scope.Snapshot())); + } + + [Fact] + public void IsPrefixOf_LiveLongerThanSnapshot_IsNotPrefix() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var snapshot = scope.Snapshot(); + Push(scope, Count, 3); + Assert.False(scope.IsPrefixOf(snapshot)); + } + + [Fact] + public void IsPrefixOf_SameContextDifferentValue_IsNotPrefix() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var snapshot = scope.Snapshot(); + scope.Pop(1); + Push(scope, Theme, "high-contrast"); + Assert.False(scope.IsPrefixOf(snapshot)); + } + + [Fact] + public void IsPrefixOf_EqualButDistinctValue_IsPrefix() + { + // Matches the consumer check (HasConsumedContextChanged uses Equals). + var scope = new ContextScope(); + Push(scope, Data, new Payload("a")); + var snapshot = scope.Snapshot(); + scope.Pop(1); + Push(scope, Data, new Payload("a")); + Assert.True(scope.IsPrefixOf(snapshot)); + } + + [Fact] + public void IsPrefixOf_DifferentContextInSameSlot_IsNotPrefix() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var snapshot = scope.Snapshot(); + scope.Pop(1); + Push(scope, Count, 1); + Assert.False(scope.IsPrefixOf(snapshot)); + } + + // ── Matches ────────────────────────────────────────────────────── + + [Fact] + public void Matches_RequiresSameLengthAndSameInstances() + { + var scope = new ContextScope(); + var payload = new Payload("a"); + Push(scope, Data, payload); + var snapshot = scope.Snapshot(); + Assert.True(scope.Matches(snapshot)); + + scope.Pop(1); + Push(scope, Data, new Payload("a")); + Assert.False(scope.Matches(snapshot)); // equal but a different instance + + scope.Pop(1); + Assert.False(scope.Matches(snapshot)); // shorter + } + + // ── Replace / Restore ──────────────────────────────────────────── + + [Fact] + public void Replace_HidesLiveValues_AndRestoreBringsThemBack() + { + var captured = new ContextScope(); + Push(captured, Theme, "captured"); + + var scope = new ContextScope(); + Push(scope, Theme, "live"); + Push(scope, Count, 7); + + var previous = scope.Replace(captured.Snapshot()); + Assert.Equal("captured", scope.Read(Theme)); + Assert.Equal(0, scope.Read(Count)); // live-only provider is not visible + + scope.Restore(previous); + Assert.Equal("live", scope.Read(Theme)); + Assert.Equal(7, scope.Read(Count)); + } + + [Fact] + public void Replace_PushesInsideTheReplacedScope_DoNotLeakIntoRestoredStack() + { + var scope = new ContextScope(); + Push(scope, Theme, "live"); + + var previous = scope.Replace([]); + Push(scope, Count, 5); + Assert.Equal(5, scope.Read(Count)); + scope.Pop(1); + + scope.Restore(previous); + Assert.Equal("live", scope.Read(Theme)); + Assert.Equal(0, scope.Read(Count)); + } + + [Fact] + public void Replace_Nested_RestoresInLifoOrder() + { + var a = new ContextScope(); Push(a, Theme, "a"); + var b = new ContextScope(); Push(b, Theme, "b"); + + var scope = new ContextScope(); + Push(scope, Theme, "live"); + + var outer = scope.Replace(a.Snapshot()); + var inner = scope.Replace(b.Snapshot()); + Assert.Equal("b", scope.Read(Theme)); + scope.Restore(inner); + Assert.Equal("a", scope.Read(Theme)); + scope.Restore(outer); + Assert.Equal("live", scope.Read(Theme)); + } + + [Fact] + public void ReplaceAndRestore_AdvanceVersion() + { + var scope = new ContextScope(); + long v0 = scope.Version; + var previous = scope.Replace([]); + long v1 = scope.Version; + scope.Restore(previous); + Assert.True(v1 > v0); + Assert.True(scope.Version > v1); + } + + // ── ContextCapture ─────────────────────────────────────────────── + + [Fact] + public void Capture_IsStale_TracksValueChangesAndAdditions() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var capture = new ContextCapture(); + capture.Refresh(scope); + Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); + + Push(scope, Count, 1); // provider added above the host + Assert.True(capture.IsStale(scope, inProviderChangeSubtree: false)); + + capture.Refresh(scope); + Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); + } + + [Fact] + public void Capture_IsStale_RemovedProviderNeedsTheProviderChangeSignal() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + Push(scope, Count, 1); + var capture = new ContextCapture(); + capture.Refresh(scope); + + scope.Pop(1); // provider removed: the shorter stack is still a prefix + Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); + Assert.True(capture.IsStale(scope, inProviderChangeSubtree: true)); + } + + [Fact] + public void Capture_Refresh_KeepsArrayWhenNothingChanged() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var capture = new ContextCapture(); + capture.Refresh(scope); + var first = capture.Entries; + + capture.Refresh(scope); + Assert.Same(first, capture.Entries); + + scope.Pop(1); + Push(scope, Theme, "light"); + capture.Refresh(scope); + Assert.NotSame(first, capture.Entries); + Assert.Equal("light", capture.Entries[0].Value); + } +} From f482652eda33bc2abd5029d8e3d1f814d88d6c40 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Thu, 15 Oct 2026 18:08:03 -0700 Subject: [PATCH 03/10] Address review round 1 - Refresh a virtualized host's context snapshot only from the element that registered it, so a reference-stable decorator (Flyout, ...) sharing the host's control cannot overwrite it at its own shallower depth. - ElementFactory drops its fallback snapshot once a host capture is attached, so it no longer pins mount-time provider values. - Regenerate docs/guide/architecture-overview.md (GetElement snippet). - Selftest for a skipped whole decorator; list fixtures wait for realization (TreeView realizes over more than one layout wave). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- docs/guide/architecture-overview.md | 2 + src/Reactor/Core/ContextScope.cs | 10 +++++ src/Reactor/Core/ElementFactory.cs | 22 +++++++--- src/Reactor/Core/Reconciler.Mount.cs | 5 +++ src/Reactor/Core/Reconciler.Update.cs | 15 ++++--- src/Reactor/Core/Reconciler.cs | 33 +++++++++----- .../Fixtures/VirtualizedRowContextFixtures.cs | 43 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + 8 files changed, 112 insertions(+), 20 deletions(-) diff --git a/docs/guide/architecture-overview.md b/docs/guide/architecture-overview.md index d6395c5cc..f626392d5 100644 --- a/docs/guide/architecture-overview.md +++ b/docs/guide/architecture-overview.md @@ -155,6 +155,8 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; + using var restoreContext = _reconciler.EnterCapturedContext(_hostCapture ?? _ownCapture); + var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 8c8c362ac..21420b7bd 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -126,6 +126,16 @@ internal sealed class ContextCapture { internal (ContextBase Context, object? Value)[] Entries { get; private set; } = []; + /// + /// Type of the element whose handler registered this capture, or null when it was + /// registered outside element dispatch (then any element sharing the control may + /// refresh it). Decorators and transparent wrappers that share the host's control + /// are a different type, so they do not overwrite the snapshot at the wrong depth. + /// + internal Type? OwnerType { get; init; } + + internal bool IsOwnedBy(Element element) => OwnerType is null || OwnerType == element.GetType(); + internal void Refresh(ContextScope scope) { if (!scope.Matches(Entries)) diff --git a/src/Reactor/Core/ElementFactory.cs b/src/Reactor/Core/ElementFactory.cs index e0a3cd4ed..b8e2d72f3 100644 --- a/src/Reactor/Core/ElementFactory.cs +++ b/src/Reactor/Core/ElementFactory.cs @@ -32,9 +32,21 @@ public sealed partial class ElementFactory : IElementFactory, IContextCapturi // when the host is skipped. A factory installed by an out-of-tree handler has // none attached and falls back to _ownCapture, which it takes itself at // construction and in UpdateInPlace — both run inside the host's Mount/Update, - // where the live stack is the host's. - ContextCapture? IContextCapturingFactory.ContextCapture { get; set; } - private readonly ContextCapture _ownCapture; + // where the live stack is the host's. Attaching a host capture drops the + // fallback so it cannot pin mount-time provider values for the list's lifetime. + private ContextCapture? _hostCapture; + private ContextCapture? _ownCapture; + + ContextCapture? IContextCapturingFactory.ContextCapture + { + get => _hostCapture; + set + { + _hostCapture = value; + if (value is not null) + _ownCapture = null; + } + } // Optional state used when ItemsSource is the OC path // (spec 042). Lets GetElement translate an ItemsRepeater realized // index → stable key for _mountedElements lookup. Null when running @@ -313,7 +325,7 @@ internal void UpdateInPlace(IReadOnlyList items, Func viewBu { _items = items; _viewBuilder = viewBuilder; - if (((IContextCapturingFactory)this).ContextCapture is null) + if (_ownCapture is not null) _reconciler.RefreshLiveContext(_ownCapture); // A new viewBuilder closure may capture different external state (UseState // cells, Observable subscriptions, theme, etc.) than the one that produced the @@ -609,7 +621,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterCapturedContext(((IContextCapturingFactory)this).ContextCapture ?? _ownCapture); + using var restoreContext = _reconciler.EnterCapturedContext(_hostCapture ?? _ownCapture); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/Reconciler.Mount.cs b/src/Reactor/Core/Reconciler.Mount.cs index 865fc87f9..4e1567e09 100644 --- a/src/Reactor/Core/Reconciler.Mount.cs +++ b/src/Reactor/Core/Reconciler.Mount.cs @@ -74,8 +74,12 @@ public sealed partial class Reconciler bool pushedStagger = element.StaggerConfig is not null; if (pushedStagger) PushStaggerScope(element.StaggerConfig!.Delay); + var prevReconcilingElement = _reconcilingElement; try { + // The innermost element being mounted owns any virtualized-host context capture + // its handler registers (see CaptureContextForHost). + _reconcilingElement = element; // Publish the effective theme for the subtree — INSIDE the try so the finally // restores it on every exit (including an exception during child mount below). // Children mounted inside MountXxx and this element's own ApplyResourceOverrides @@ -244,6 +248,7 @@ public sealed partial class Reconciler } finally { + _reconcilingElement = prevReconcilingElement; if (pushedStagger) PopStaggerScope(); if (ctxCount > 0) diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 39a0e82b3..6e3698991 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -128,9 +128,9 @@ public sealed partial class Reconciler if ((HasGestureOrDragSlots(modifiers) || HasGestureOrDragSlots(oldModifiers)) && control is FrameworkElement gestFeSE) RefreshGestureDragStateOnSkip(gestFeSE, oldModifiers, modifiers); - // A virtualized host that shares its control with a decorator was refreshed - // by the decorator's Update at the wrong depth; re-snapshot here (no-op for - // every other control, and allocation-free when nothing changed). + // A virtualized host's own skipped element re-snapshots here at its own depth + // (no-op for every other control or a decorator sharing it, and + // allocation-free when nothing changed). RefreshContextCaptureOnSkip(control, newEl); // No validation refresh here, deliberately (issue #1262). A `.Validate()` // verdict is republished only by a component that re-rendered, and that @@ -184,8 +184,12 @@ public sealed partial class Reconciler EnterProviderChange(); // Virtualized hosts realize rows outside this traversal; keep their provider - // snapshot current (no-op for every other control). - RefreshContextCapture(control); + // snapshot current (no-op for every other control, and for a decorator that + // shares the host's control — the host element's own Update refreshes it). + RefreshContextCapture(control, newEl); + + var prevReconcilingElement = _reconcilingElement; + _reconcilingElement = newEl; UIElement? result; try @@ -363,6 +367,7 @@ public sealed partial class Reconciler } finally { + _reconcilingElement = prevReconcilingElement; if (providerChanged) ExitProviderChange(); if (ctxCount > 0) diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index 590071a2d..25ea2d13e 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -1935,6 +1935,12 @@ internal IDisposable PushContextDisposable(Context context, T value) // never pay the attached-state read. private bool _hasContextCaptures; + // The innermost element currently in Mount/Update dispatch. A capture registered + // by a host handler records this element's type as its owner: a decorator + // (Flyout, MenuFlyout, …) or a transparent wrapper shares the host's control but + // sits at a shallower depth, so only the owner's own Update/skip may re-snapshot. + private Element? _reconcilingElement; + private static bool MayHostContextCapture(UIElement control) => control is WinUI.ScrollViewer or WinUI.ItemsRepeater or WinUI.ItemsView or WinUI.ListViewBase or WinUI.TreeView; @@ -1949,7 +1955,7 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) global::System.Diagnostics.Debug.Assert(MayHostContextCapture(host), $"{host.GetType().Name} is not covered by MayHostContextCapture; its capture would never refresh."); var state = GetOrCreateReactorState(host); - var capture = state.ContextCapture ??= new ContextCapture(); + var capture = state.ContextCapture ??= new ContextCapture { OwnerType = _reconcilingElement?.GetType() }; capture.Refresh(_contextScope); _hasContextCaptures = true; return capture; @@ -1962,20 +1968,27 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) ? capture : null; - /// Re-snapshots a host's provider stack; a no-op for any other control. - internal void RefreshContextCapture(UIElement control) - => TryGetContextCapture(control)?.Refresh(_contextScope); + /// + /// Re-snapshots a host's provider stack when is the element that + /// owns the capture; a no-op for any other control or element. + /// + internal void RefreshContextCapture(UIElement control, Element newEl) + { + if (TryGetContextCapture(control) is { } capture && capture.IsOwnedBy(newEl)) + capture.Refresh(_contextScope); + } /// - /// Skip-arm counterpart of . A host whose - /// control is shared with a decorator (Flyout, MenuFlyout, …) is first refreshed by - /// the decorator's Update, before the host element's own ContextValues are on - /// the stack; when the host element then skips, this re-snapshots at its own depth so - /// a .Provide() on the list element itself is not lost. + /// Skip-arm counterpart of . The + /// skip arm runs before the element's own ContextValues are pushed, so they are + /// pushed here for the snapshot. A decorator sharing the host's control is not the + /// owner, so a skipped decorator leaves the host's snapshot (taken at the host's own + /// depth) untouched; if a provider above it changed, the stale check declines that + /// skip and the decorator's Update reaches the host element. /// internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) { - if (TryGetContextCapture(control) is not { } capture) return; + if (TryGetContextCapture(control) is not { } capture || !capture.IsOwnedBy(newEl)) return; var own = newEl.ContextValues; if (own is not { Count: > 0 }) { diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs index f46cff318..73e6b416b 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs @@ -243,6 +243,37 @@ public override async Task RunAsync() } } + // The whole decorator is reference-stable and skips at the element level (Border's + // single child goes through Update's skip arm). Its skip must not re-snapshot the + // shared control at the decorator's depth, which would drop the list's own provider. + internal sealed class LazyVStack_OwnProviderUnderSkippedDecorator(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var stableFlyout = Flyout(HalfConsumerList().Provide(RowCtx, "own"), TextBlock("fly")); + var host = H.CreateHost(); + host.Mount(ctx => + { + var (tick, setTick) = ctx.UseState(0); + return VStack( + Button("VCtxDecoSkipBump", () => setTick(tick + 1)), + TextBlock($"vctx-decoskip:{tick}"), + // The Border changes each tick so it is updated, and hands the + // reference-stable Flyout to Update, whose element-level skip arm runs. + Border(stableFlyout).Margin(tick)); + }); + await Harness.Render(); + + H.ClickButton("VCtxDecoSkipBump"); + await Harness.Render(); + H.Check("VirtualizedCtx_DecoSkip_Bumped", H.FindText("vctx-decoskip:1") is not null); + + await ScrollListTo(H, 150 * 30); + var scrolled = RowTexts(H, minIndex: 100); + H.Check("VirtualizedCtx_DecoSkip_RowsSeeOwnProvider", AllEndWith(scrolled, ":own"), Sample(scrolled)); + } + } + private sealed record NavRowProps(int Index); private sealed class NavRow : Component @@ -293,6 +324,8 @@ public override async Task RunAsync() ListView(items, i => i.ToString(), (i, _) => Row(i)).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_ListView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); @@ -309,6 +342,8 @@ public override async Task RunAsync() GridView(items, i => i.ToString(), (i, _) => Row(i)).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_GridView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); @@ -325,6 +360,8 @@ public override async Task RunAsync() ListView(Enumerable.Range(0, 20).Select(Row).ToArray()).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_UntypedListView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); @@ -340,6 +377,8 @@ public override async Task RunAsync() GridView(Enumerable.Range(0, 20).Select(Row).ToArray()).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_UntypedGridView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); @@ -356,6 +395,8 @@ public override async Task RunAsync() ItemsView(items, i => i.ToString(), (i, _) => ItemContainer(Row(i))).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_ItemsView_Rows", AllEndWith(rows, ":provided"), Sample(rows)); @@ -372,6 +413,8 @@ public override async Task RunAsync() TreeView(roots, i => i.ToString(), _ => null, i => Row(i)).Height(300)) .Provide(RowCtx, "provided")); await Harness.Render(); + // Realization can take more than one layout wave (TreeView hosts on Loaded/LayoutUpdated). + await Harness.WaitFor(() => RowTexts(H).Count > 0); var rows = RowTexts(H); H.Check("VirtualizedCtx_TreeView_Nodes", AllEndWith(rows, ":provided"), Sample(rows)); diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 9c7078d1b..1d13e33d0 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -623,6 +623,7 @@ internal static class SelfTestFixtureRegistry "VirtualizedCtx_LazyVStack_InnerProviderRemovedWhileSkipped", "VirtualizedCtx_StableConsumer_LastProviderRemoved", "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator", + "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2686,6 +2687,7 @@ internal static string[] StaleTierDeclarations() => "VirtualizedCtx_LazyVStack_InnerProviderRemovedWhileSkipped" => new VirtualizedRowContextFixtures.LazyVStack_InnerProviderRemovedWhileSkipped(harness), "VirtualizedCtx_StableConsumer_LastProviderRemoved" => new VirtualizedRowContextFixtures.StableConsumer_LastProviderRemoved(harness), "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderDecorator(harness), + "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderSkippedDecorator(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), From e3a320f5e352154e0d880e46265861f8dc2da782 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Fri, 16 Oct 2026 16:11:27 -0700 Subject: [PATCH 04/10] Address review round 2 Keep context skip checks enabled while a captured snapshot is replayed, even when the snapshot is empty because the host's last provider was removed. A recycled row reused for the same item under that empty snapshot otherwise shallow-skips its reference-stable consumer and keeps the removed value. Gated on the scope having ever seen a provider, so provider-free apps pay nothing during realization. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 14 ++++++- .../Fixtures/VirtualizedRowContextFixtures.cs | 38 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 27 +++++++++++++ 4 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 21420b7bd..609f04210 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -14,6 +14,7 @@ internal void Push(IReadOnlyDictionary values) { foreach (var (ctx, val) in values) _stack.Add((ctx, val)); + if (values.Count > 0) _everHadValues = true; _version++; } @@ -48,7 +49,16 @@ internal T Read(Context context) return context.DefaultValueBoxed; } - internal bool HasActiveValues => _stack.Count > 0; + internal bool HasActiveValues => _stack.Count > 0 || (_replayDepth > 0 && _everHadValues); + + // A replayed snapshot (Replace) can be EMPTY because the host's last provider was + // removed, while the row being realized was built under that provider. With an empty + // stack HasActiveValues would disable every context skip check, so a reused row that + // is shallow-equal would keep the removed value. While a replay is active, keep the + // checks on — but only once this scope has ever seen a provider, so provider-free + // apps pay nothing during row realization. + private int _replayDepth; + private bool _everHadValues; internal long Version => _version; @@ -103,6 +113,7 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) { var previous = _stack; _stack = new List<(ContextBase Context, object? Value)>(snapshot); + _replayDepth++; _version++; return previous; } @@ -110,6 +121,7 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) internal void Restore(List<(ContextBase Context, object? Value)> previous) { _stack = previous; + _replayDepth--; _version++; } } diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs index 73e6b416b..c66e675a7 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/VirtualizedRowContextFixtures.cs @@ -2,6 +2,7 @@ using Microsoft.UI.Reactor.Core; using Microsoft.UI.Reactor.AppTests.Host.SelfTest; using Microsoft.UI.Reactor.Navigation; +using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; using static Microsoft.UI.Reactor.Factories; @@ -276,6 +277,43 @@ public override async Task RunAsync() private sealed record NavRowProps(int Index); + // A row parked by RecycleElement is not refreshed when the provider is removed; + // reusing it for the same row under the now-EMPTY snapshot must still re-check the + // context its consumer read, rather than shallow-skip and keep the removed value. + // Drives a standalone factory so every Get/Recycle is one the test asked for. + internal sealed class Factory_RecycledRowAfterLastProviderRemoved(Harness h) : SelfTestFixtureBase(h) + { + public override Task RunAsync() + { + var items = Enumerable.Range(0, 5).ToList(); + // A reference-stable consumer under a fresh Border: the Border updates, and its + // child takes the element-level skip unless a context check declines it. + var consumers = items.Select(Row).ToArray(); + Func builder = (i, _) => Border(consumers[i]); + var reconciler = new Reconciler(); + + ElementFactory factory; + using (reconciler.PushContextDisposable(RowCtx, "removed")) + factory = new ElementFactory(items, builder, reconciler, static () => { }, pool: null); + IElementFactory f = factory; + + static string Text(UIElement c) + => (c as Border)?.Child is Border wrapper && wrapper.Child is TextBlock tb ? tb.Text : $"<{c.GetType().Name}>"; + + var first = f.GetElement(new ElementFactoryGetArgs { Data = 0 }); + H.Check("VirtualizedCtx_Recycle_FirstSeesProvider", Text(first) == "row0:removed", Text(first)); + f.RecycleElement(new ElementFactoryRecycleArgs { Element = first }); + + // The host re-renders without the provider: its snapshot is now empty. + factory.UpdateInPlace(items, builder); + + var again = f.GetElement(new ElementFactoryGetArgs { Data = 0 }); + H.Check("VirtualizedCtx_Recycle_ContainerReused", ReferenceEquals(first, again)); + H.Check("VirtualizedCtx_Recycle_ReusedRowSeesDefault", Text(again) == "row0:default", Text(again)); + return Task.CompletedTask; + } + } + private sealed class NavRow : Component { public override Element Render() diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 1d13e33d0..398c55b28 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -624,6 +624,7 @@ internal static class SelfTestFixtureRegistry "VirtualizedCtx_StableConsumer_LastProviderRemoved", "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator", "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator", + "VirtualizedCtx_Factory_RecycledRowAfterLastProviderRemoved", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2688,6 +2689,7 @@ internal static string[] StaleTierDeclarations() => "VirtualizedCtx_StableConsumer_LastProviderRemoved" => new VirtualizedRowContextFixtures.StableConsumer_LastProviderRemoved(harness), "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderDecorator(harness), "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderSkippedDecorator(harness), + "VirtualizedCtx_Factory_RecycledRowAfterLastProviderRemoved" => new VirtualizedRowContextFixtures.Factory_RecycledRowAfterLastProviderRemoved(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index 23250bd2d..265446ab2 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -167,6 +167,33 @@ public void ReplaceAndRestore_AdvanceVersion() Assert.True(scope.Version > v1); } + [Fact] + public void HasActiveValues_StaysOnDuringEmptyReplay_OnceAProviderWasSeen() + { + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + scope.Pop(1); + Assert.False(scope.HasActiveValues); + + var outer = scope.Replace([]); + Assert.True(scope.HasActiveValues); + var inner = scope.Replace([]); + Assert.True(scope.HasActiveValues); + scope.Restore(inner); + Assert.True(scope.HasActiveValues); + scope.Restore(outer); + Assert.False(scope.HasActiveValues); + } + + [Fact] + public void HasActiveValues_StaysOffDuringEmptyReplay_WhenNoProviderWasEverSeen() + { + var scope = new ContextScope(); + var previous = scope.Replace([]); + Assert.False(scope.HasActiveValues); + scope.Restore(previous); + } + // ── ContextCapture ─────────────────────────────────────────────── [Fact] From 5d043c3c9157e9f061c98c4c5384011272cb4a2a Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 10:55:38 -0700 Subject: [PATCH 05/10] Isolate row realization from the in-flight update (review round 3) Rows realized by WinUI layout can run re-entrantly inside an unrelated Reactor update (any synchronous UpdateLayout). They used to join that update's bookkeeping. Realization now runs in a RealizationFrame that sets aside the ErrorBoundary rollback journal/roots and boundary depth, the provider-change depth, the reconciling element and stagger scope, replays the host's captured providers and ambient theme, and makes the owning ReactorHost active for window-scoped hooks. - A failing boundary child no longer tears down live rows in another list. - A row that throws gets its own in-tree fallback instead of tripping an unrelated boundary. - Rows realized outside a render resolve their own window host. Round 3 findings: - Host snapshots record a provider generation bumped on every provider change, so a nested host in a recycled row that comes back in a later pass still re-snapshots after a removed provider. - Retiring/pooling a host empties its capture so the aliasing ElementFactory no longer keeps old provider values alive. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- CHANGELOG.md | 16 + docs/guide/architecture-overview.md | 2 +- src/Reactor/Core/ContextScope.cs | 49 ++- src/Reactor/Core/ElementFactory.cs | 2 +- src/Reactor/Core/Reconciler.Mount.cs | 2 +- src/Reactor/Core/Reconciler.Update.cs | 4 +- src/Reactor/Core/Reconciler.cs | 159 ++++++--- .../V1Protocol/Handlers/GridViewHandler.cs | 2 +- .../V1Protocol/Handlers/ListViewHandler.cs | 2 +- .../V1Protocol/Reconciler.TemplatedTree.cs | 2 +- src/Reactor/Hosting/ReactorHost.cs | 1 + .../Fixtures/RealizationIsolationFixtures.cs | 322 ++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 10 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 79 ++++- 14 files changed, 578 insertions(+), 74 deletions(-) create mode 100644 tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 5732b7750..a2f4c61ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -306,6 +306,22 @@ Conventions for contributors: a custom handler that builds rows itself from `ContainerContentChanging` is not. Removing the only `.Provide(...)` above a reference-stable consumer now also re-renders that consumer with the default, which it previously missed. +- **Building a list row in the middle of another update no longer corrupts that update** + (issue #1386). WinUI can ask a virtualized list for rows synchronously while Reactor is + still applying an unrelated update, for example when an effect or a control's own + property setter calls `UpdateLayout()`. Such rows used to join that update's + bookkeeping: + - If the update was mounting an `ErrorBoundary` whose child then failed, the boundary's + rollback tore down those rows too: they stayed on screen with their effects cleaned + up, so they stopped updating. + - A row that threw was reported to that unrelated boundary, which then showed its + fallback. + - Rows built while scrolling saw no window host (or another window's), so window-scoped + hooks such as `UseWindow` could resolve the wrong window in multi-window apps. + + Row building now sets the surrounding update's state aside and restores the list's own + context, theme and window host, so a failing row shows its own error and a failing + boundary only rolls back its own subtree. - **Replacing a host's root no longer leaves the old root running** (issue #1326). `ReactorHost.Mount(...)` and `ReactorHostControl.Mount(...)` now retire the outgoing root before installing the new one: its effect cleanups run, and a diff --git a/docs/guide/architecture-overview.md b/docs/guide/architecture-overview.md index f626392d5..a70a479aa 100644 --- a/docs/guide/architecture-overview.md +++ b/docs/guide/architecture-overview.md @@ -155,7 +155,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterCapturedContext(_hostCapture ?? _ownCapture); + using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 609f04210..c188de473 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -1,3 +1,5 @@ +using Microsoft.UI.Xaml; + namespace Microsoft.UI.Reactor.Core; /// @@ -62,6 +64,18 @@ internal T Read(Context context) internal long Version => _version; + /// + /// Advances whenever an element's own provided values change between renders (a + /// provider added, removed or re-valued). A host snapshot records the generation it + /// was taken at; a mismatch means some provider changed since, and the snapshot is + /// re-taken the next time the reconciler reaches the host. Unlike a check scoped to + /// the pass that removed the provider, this survives until the host is reached — + /// e.g. a nested host inside a recycled row that comes back in a later pass. + /// + internal long ProviderGeneration { get; private set; } + + internal void BumpProviderGeneration() => ProviderGeneration++; + internal (ContextBase Context, object? Value)[] Snapshot() => _stack.Count == 0 ? [] : _stack.ToArray(); @@ -89,7 +103,7 @@ internal bool Matches((ContextBase Context, object? Value)[] snapshot) /// prefix of it unless a provider at or above that ancestor changed value or was /// added. A removed provider leaves a shorter stack that is still a prefix, /// so callers must detect removal separately (see - /// Reconciler.InProviderChangeSubtree). + /// ). /// internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) { @@ -148,18 +162,35 @@ internal sealed class ContextCapture internal bool IsOwnedBy(Element element) => OwnerType is null || OwnerType == element.GetType(); - internal void Refresh(ContextScope scope) + /// when was taken. + internal long Generation { get; private set; } = -1; + + /// + /// The ancestor-resolved RequestedTheme at the host, so ThemeRef resource + /// overrides on realized rows resolve against the same theme as the rest of the subtree. + /// + internal ElementTheme AmbientTheme { get; private set; } + + internal void Refresh(ContextScope scope, ElementTheme ambientTheme) { if (!scope.Matches(Entries)) Entries = scope.Snapshot(); + Generation = scope.ProviderGeneration; + AmbientTheme = ambientTheme; + } + + /// + /// Releases the snapshot's provider values. The capture object can be aliased (an + /// holds the same instance), so a host leaving Reactor's + /// ownership empties it rather than only dropping its own reference. + /// + internal void Clear() + { + Entries = []; + Generation = -1; } /// The live stack at an ancestor of (or at) the host. - /// - /// True while the traversal is below an element whose provided values changed this - /// pass. A removed provider is invisible to the prefix test, so any snapshot taken - /// beneath such an element has to be treated as stale. - /// - internal bool IsStale(ContextScope scope, bool inProviderChangeSubtree) - => inProviderChangeSubtree || !scope.IsPrefixOf(Entries); + internal bool IsStale(ContextScope scope) + => Generation != scope.ProviderGeneration || !scope.IsPrefixOf(Entries); } diff --git a/src/Reactor/Core/ElementFactory.cs b/src/Reactor/Core/ElementFactory.cs index b8e2d72f3..f4dbb12a0 100644 --- a/src/Reactor/Core/ElementFactory.cs +++ b/src/Reactor/Core/ElementFactory.cs @@ -621,7 +621,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterCapturedContext(_hostCapture ?? _ownCapture); + using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/Reconciler.Mount.cs b/src/Reactor/Core/Reconciler.Mount.cs index 4e1567e09..64eb2684c 100644 --- a/src/Reactor/Core/Reconciler.Mount.cs +++ b/src/Reactor/Core/Reconciler.Mount.cs @@ -716,7 +716,7 @@ internal void HandleTemplatedContainerContentChanging(object sender, ContainerCo { // Realization runs from layout, outside the traversal — replay the // host's provider stack so the row sees its ancestors' context. - using var restoreContext = EnterCapturedContext((UIElement)sender!); + using var realization = EnterRealization((UIElement)sender!); var itemElement = viewSource.BuildItemView(args.ItemIndex); var ctrl = Mount(itemElement, requestRerender); cc.Content = ctrl; diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 6e3698991..6044c4ddb 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -131,7 +131,7 @@ public sealed partial class Reconciler // A virtualized host's own skipped element re-snapshots here at its own depth // (no-op for every other control or a decorator sharing it, and // allocation-free when nothing changed). - RefreshContextCaptureOnSkip(control, newEl); + RefreshContextCaptureOnSkip(control, newEl, effectiveTheme); // No validation refresh here, deliberately (issue #1262). A `.Validate()` // verdict is republished only by a component that re-rendered, and that // component's output is on the dirty ancestor path, which this arm declines @@ -186,7 +186,7 @@ public sealed partial class Reconciler // Virtualized hosts realize rows outside this traversal; keep their provider // snapshot current (no-op for every other control, and for a decorator that // shares the host's control — the host element's own Update refreshes it). - RefreshContextCapture(control, newEl); + RefreshContextCapture(control, newEl, effectiveTheme); var prevReconcilingElement = _reconcilingElement; _reconcilingElement = newEl; diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index 25ea2d13e..cc37fd45b 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -1208,7 +1208,9 @@ internal static void ClearCurrentEventHandlers(FrameworkElement fe) state.PendingEchoMatch = null; state.PendingLabeledBy = null; // A pooled host must not hand its provider values (and the objects they - // reference) to an unrelated renter. + // reference) to an unrelated renter. Empty the capture before dropping it: + // the host's ElementFactory aliases the same instance. + state.ContextCapture?.Clear(); state.ContextCapture = null; } } @@ -1256,6 +1258,10 @@ public static void DetachReactorState(FrameworkElement fe) state.ValidationRootBinding = null; state.ValidationRuleBinding = null; state.ValidationAttachedBinding = null; + // A retired host's native subtree (and its ItemTemplate factory, which aliases + // this capture) can outlive Reactor ownership — e.g. a nested list inside a + // parked repeater row — so release the provider values, not just the reference. + state.ContextCapture?.Clear(); state.ContextCapture = null; } @@ -1908,18 +1914,27 @@ internal IDisposable PushContextDisposable(Context context, T value) // Depth of non-skip Updates, on the current traversal path, whose element's own // ContextValues differ from the previous render's (a provider added, removed or // changed). Removing the LAST provider leaves the live stack empty, which used to - // short-circuit every context skip check (HasActiveContextValues) so a - // reference-stable consumer below kept the removed value; removing one of several - // leaves a shorter stack that a host's snapshot prefix test cannot tell apart from - // an ancestor's view. Counting the change lets both checks see it. + // short-circuit every context skip check (HasActiveContextValues), so a + // reference-stable consumer below kept the removed value. Host snapshots use the + // longer-lived ContextScope.ProviderGeneration instead (bumped alongside). private int _providerChangeDepth; - internal bool InProviderChangeSubtree => _providerChangeDepth > 0; - - internal void EnterProviderChange() => _providerChangeDepth++; + internal void EnterProviderChange() + { + _providerChangeDepth++; + _contextScope.BumpProviderGeneration(); + } internal void ExitProviderChange() => _providerChangeDepth--; + /// + /// The host that owns this reconciler, or null for a standalone reconciler or a + /// ReactorHostControl. Window-scoped hooks resolve "the rendering host" through + /// , which is only pushed for the host's + /// own render pass; realization restores it from here. + /// + internal ReactorHost? OwnerHost { get; set; } + // ── Context replay for virtualized realization ───────────────────── // // ItemsRepeater (IElementFactory.GetElement), ListViewBase @@ -1929,7 +1944,7 @@ internal IDisposable PushContextDisposable(Context context, T value) // the middle of an unrelated subtree's update (a sibling's synchronous // UpdateLayout), so the live stack is either empty or someone else's. Each // such host captures the stack whenever it is reconciled, and the realization - // callbacks replay that capture around the row mount. + // callbacks replay that capture around the row mount (see EnterRealization). // Gates the per-control capture lookups so apps without a virtualized host // never pay the attached-state read. @@ -1956,7 +1971,7 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) $"{host.GetType().Name} is not covered by MayHostContextCapture; its capture would never refresh."); var state = GetOrCreateReactorState(host); var capture = state.ContextCapture ??= new ContextCapture { OwnerType = _reconcilingElement?.GetType() }; - capture.Refresh(_contextScope); + capture.Refresh(_contextScope, _ambientRequestedTheme); _hasContextCaptures = true; return capture; } @@ -1972,31 +1987,31 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) /// Re-snapshots a host's provider stack when is the element that /// owns the capture; a no-op for any other control or element. /// - internal void RefreshContextCapture(UIElement control, Element newEl) + internal void RefreshContextCapture(UIElement control, Element newEl, ElementTheme effectiveTheme) { if (TryGetContextCapture(control) is { } capture && capture.IsOwnedBy(newEl)) - capture.Refresh(_contextScope); + capture.Refresh(_contextScope, effectiveTheme); } /// - /// Skip-arm counterpart of . The - /// skip arm runs before the element's own ContextValues are pushed, so they are - /// pushed here for the snapshot. A decorator sharing the host's control is not the - /// owner, so a skipped decorator leaves the host's snapshot (taken at the host's own - /// depth) untouched; if a provider above it changed, the stale check declines that - /// skip and the decorator's Update reaches the host element. + /// Skip-arm counterpart of . The skip arm runs + /// before the element's own ContextValues are pushed, so they are pushed here + /// for the snapshot. A decorator sharing the host's control is not the owner, so a + /// skipped decorator leaves the host's snapshot (taken at the host's own depth) + /// untouched; if a provider above it changed, the stale check declines that skip and + /// the decorator's Update reaches the host element. /// - internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) + internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl, ElementTheme effectiveTheme) { if (TryGetContextCapture(control) is not { } capture || !capture.IsOwnedBy(newEl)) return; var own = newEl.ContextValues; if (own is not { Count: > 0 }) { - capture.Refresh(_contextScope); + capture.Refresh(_contextScope, effectiveTheme); return; } _contextScope.Push(own); - try { capture.Refresh(_contextScope); } + try { capture.Refresh(_contextScope, effectiveTheme); } finally { _contextScope.Pop(own.Count); } } @@ -2007,24 +2022,32 @@ internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) internal ContextCapture CaptureLiveContext() { var capture = new ContextCapture(); - capture.Refresh(_contextScope); + capture.Refresh(_contextScope, _ambientRequestedTheme); return capture; } - internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope); + internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope, _ambientRequestedTheme); /// - /// Replaces the live provider stack with until the - /// returned scope is disposed. A null capture is a no-op. + /// Opens a realization frame for building a row outside (or re-entrantly inside) the + /// traversal. Until the returned scope is disposed: + /// + /// the live provider stack is replaced with + /// and the ambient theme with the one captured at the host; + /// the in-flight pass's per-subtree bookkeeping is set aside, so the + /// row is not recorded in an ErrorBoundary's rollback lists (a failing boundary child + /// would otherwise tear down a live row in an unrelated list), a row that throws gets + /// its own in-tree fallback instead of escaping into WinUI layout, and stagger and + /// provider-change state from the surrounding subtree does not leak in; + /// the owning host is the active host, so window-scoped hooks in the + /// row resolve this window even outside the host's render pass. + /// + /// A null capture still isolates; it just leaves the provider stack and theme as they are. /// - internal CapturedContextScope EnterCapturedContext(ContextCapture? capture) - => capture is null - ? default - : new CapturedContextScope(_contextScope, _contextScope.Replace(capture.Entries)); + internal RealizationFrame EnterRealization(ContextCapture? capture) => new(this, capture); - /// - internal CapturedContextScope EnterCapturedContext(UIElement host) - => EnterCapturedContext(TryGetContextCapture(host)); + /// + internal RealizationFrame EnterRealization(UIElement host) => EnterRealization(TryGetContextCapture(host)); /// /// Registers (see ) and @@ -2036,18 +2059,70 @@ internal void AttachContextCapture(IElementFactory factory, FrameworkElement hos capturing.ContextCapture = CaptureContextForHost(host); } - internal readonly struct CapturedContextScope : IDisposable - { - private readonly ContextScope? _scope; - private readonly List<(ContextBase Context, object? Value)>? _previous; + internal readonly struct RealizationFrame : IDisposable + { + private readonly Reconciler _owner; + private readonly List<(ContextBase Context, object? Value)>? _previousStack; + private readonly ElementTheme _previousTheme; + private readonly List? _previousJournal; + private readonly List? _previousRoots; + private readonly int _previousBoundaryDepth; + private readonly int _previousProviderChangeDepth; + private readonly Element? _previousReconcilingElement; + private readonly StaggerScope? _previousStagger; + private readonly bool _swapHost; + private readonly ReactorHost? _previousHost; + + internal RealizationFrame(Reconciler owner, ContextCapture? capture) + { + _owner = owner; + _previousTheme = owner._ambientRequestedTheme; + _previousJournal = owner._boundaryMountJournal; + _previousRoots = owner._boundaryMountRoots; + _previousBoundaryDepth = owner._errorBoundaryDepth; + _previousProviderChangeDepth = owner._providerChangeDepth; + _previousReconcilingElement = owner._reconcilingElement; + _previousStagger = _staggerScope; + + owner._boundaryMountJournal = null; + owner._boundaryMountRoots = null; + owner._errorBoundaryDepth = 0; + owner._providerChangeDepth = 0; + owner._reconcilingElement = null; + _staggerScope = null; + + if (capture is not null) + { + _previousStack = owner._contextScope.Replace(capture.Entries); + owner._ambientRequestedTheme = capture.AmbientTheme; + } + else + { + _previousStack = null; + } - internal CapturedContextScope(ContextScope scope, List<(ContextBase Context, object? Value)> previous) - { - _scope = scope; - _previous = previous; + _swapHost = owner.OwnerHost is not null; + _previousHost = _swapHost ? ReactorApp.ActiveHostInternal : null; + if (_swapHost) + ReactorApp.ActiveHostInternal = owner.OwnerHost; } - public void Dispose() => _scope?.Restore(_previous!); + public void Dispose() + { + var owner = _owner; + if (owner is null) return; + if (_previousStack is not null) + owner._contextScope.Restore(_previousStack); + if (_swapHost) + ReactorApp.ActiveHostInternal = _previousHost; + owner._ambientRequestedTheme = _previousTheme; + owner._boundaryMountJournal = _previousJournal; + owner._boundaryMountRoots = _previousRoots; + owner._errorBoundaryDepth = _previousBoundaryDepth; + owner._providerChangeDepth = _previousProviderChangeDepth; + owner._reconcilingElement = _previousReconcilingElement; + _staggerScope = _previousStagger; + } } /// @@ -2910,7 +2985,7 @@ internal bool HasConsumedContextChangedInSubtree(UIElement control) // A virtualized host has no realized consumer for rows it has yet to realize, // so its own snapshot stands in for them: if the providers above it changed, // decline the skip so the Update path re-snapshots before the next realization. - if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope, InProviderChangeSubtree)) + if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope)) return true; bool found = false; diff --git a/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs b/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs index 863457eed..5e0441786 100644 --- a/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs +++ b/src/Reactor/Core/V1Protocol/Handlers/GridViewHandler.cs @@ -59,7 +59,7 @@ public WinUI.GridView Mount(MountContext ctx, GridViewElement gv) if (items is not null && args.ItemIndex >= 0 && args.ItemIndex < items.Length && args.ItemContainer.ContentTemplateRoot is ContentControl cc) { - using var restoreContext = reconciler.EnterCapturedContext((UIElement)sender!); + using var realization = reconciler.EnterRealization((UIElement)sender!); var ctrl = reconciler.Mount(items[args.ItemIndex], requestRerender); cc.Content = ctrl; // Issue #951 — keep .AutomationName(...) on an item view working diff --git a/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs b/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs index 58b0390c8..795237862 100644 --- a/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs +++ b/src/Reactor/Core/V1Protocol/Handlers/ListViewHandler.cs @@ -61,7 +61,7 @@ public WinUI.ListView Mount(MountContext ctx, ListViewElement lv) if (items is not null && args.ItemIndex >= 0 && args.ItemIndex < items.Length && args.ItemContainer.ContentTemplateRoot is ContentControl cc) { - using var restoreContext = reconciler.EnterCapturedContext((UIElement)sender!); + using var realization = reconciler.EnterRealization((UIElement)sender!); var ctrl = reconciler.Mount(items[args.ItemIndex], requestRerender); cc.Content = ctrl; // Issue #951 — keep .AutomationName(...) on an item view working diff --git a/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs b/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs index 9ccd41893..1b4a5272b 100644 --- a/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs +++ b/src/Reactor/Core/V1Protocol/Reconciler.TemplatedTree.cs @@ -177,7 +177,7 @@ private void PopulateTypedTreeContainer( // Runs from ContainerContentChanging / Loaded / LayoutUpdated, outside the // traversal — replay the tree's provider stack for the node view. - using var restoreContext = EnterCapturedContext(treeView); + using var realization = EnterRealization(treeView); var view = el.BuildView(data); cc.Content = Mount(view, requestRerender); SetElementTag(cc, view); diff --git a/src/Reactor/Hosting/ReactorHost.cs b/src/Reactor/Hosting/ReactorHost.cs index b5085ed69..366494274 100644 --- a/src/Reactor/Hosting/ReactorHost.cs +++ b/src/Reactor/Hosting/ReactorHost.cs @@ -229,6 +229,7 @@ public ReactorHost(Window window, ILogger? logger = null) _logger = logger ?? ReactorApp.AppLogger; _reconciler = new Reconciler(_logger); _reconciler.RenderErrorHandlerProvider = () => EffectiveRenderErrorHandler; + _reconciler.OwnerHost = this; _window = window; _backdropApplier = new BackdropApplier(window); _dispatcherQueue = DispatcherQueue.GetForCurrentThread(); diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs new file mode 100644 index 000000000..619891d34 --- /dev/null +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -0,0 +1,322 @@ +using Microsoft.UI.Reactor; +using Microsoft.UI.Reactor.Core; +using Microsoft.UI.Reactor.AppTests.Host.SelfTest; +using Microsoft.UI.Reactor.Hosting; +using Microsoft.UI.Xaml; +using Microsoft.UI.Xaml.Controls; +using Microsoft.UI.Xaml.Media; +using static Microsoft.UI.Reactor.Factories; + +namespace Microsoft.UI.Reactor.AppTests.Host.SelfTest.Fixtures; + +/// +/// Issue #1386 — a virtualized host builds rows from WinUI layout callbacks, which can +/// run re-entrantly in the middle of an unrelated Reactor update (any synchronous +/// UpdateLayout(): an app effect, a control's own property setter). Row building +/// must not join that update's bookkeeping: its ErrorBoundary rollback lists, its +/// boundary depth (error routing), its window host. And a host's provider snapshot must +/// go stale on a provider change even when the host is only reached in a later pass. +/// +internal static class RealizationIsolationFixtures +{ + private static readonly Context Ctx = new("default"); + + // ── Re-entrant realization during a failing ErrorBoundary mount ────────── + + private static int s_rowEffects; + private static int s_rowCleanups; + private static Action? s_forceLayout; + + private sealed record IndexProps(int Index); + + private sealed class EffectRow : Component + { + public override Element Render() + { + UseEffect(() => + { + s_rowEffects++; + return () => s_rowCleanups++; + }); + return TextBlock($"erow{Props.Index}").Height(30); + } + } + + private sealed class ThrowingRow : Component + { + public override Element Render() + => Props.Index >= 6 + ? throw new InvalidOperationException($"row-boom-{Props.Index}") + : TextBlock($"trow{Props.Index}").Height(30); + } + + private sealed class LayoutForcer : Component + { + public override Element Render() + { + UseEffect(() => s_forceLayout?.Invoke()); + return TextBlock("forcer"); + } + } + + private sealed class Thrower : Component + { + public override Element Render() => throw new InvalidOperationException("boundary child failed"); + } + + private static Action ForceLayoutOf(Harness h) => () => + { + var sv = h.FindControl(s => s.Content is ItemsRepeater); + (sv?.XamlRoot?.Content as UIElement)?.UpdateLayout(); + }; + + internal sealed class RowsRealizedDuringFailedBoundaryMount_StayAlive(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + s_rowEffects = s_rowCleanups = 0; + s_forceLayout = null; + var host = H.CreateHost(); + host.Mount(ctx => + { + var (phase, setPhase) = ctx.UseState(0); + IReadOnlyList items = Enumerable.Range(0, phase == 0 ? 2 : 20).ToList(); + return VStack( + Button("IsoBoundaryGo", () => setPhase(1)), + LazyVStack(items, i => i.ToString(), (i, _) => + Component(new IndexProps(i))).Height(300), + phase == 0 + ? TextBlock("iso-idle") + : ErrorBoundary( + VStack(Component(), Component()), + TextBlock("iso-fallback"))); + }); + await Harness.Render(); + + int before = s_rowEffects; + int realizedDuringPass = -1; + var force = ForceLayoutOf(H); + s_forceLayout = () => { force(); realizedDuringPass = s_rowEffects - before; }; + + H.ClickButton("IsoBoundaryGo"); + await Harness.Render(); + s_forceLayout = null; + + int visible = H.FindAllControls(tb => tb.Text?.StartsWith("erow", StringComparison.Ordinal) == true).Count; + var detail = $"realizedDuringPass={realizedDuringPass} effects={s_rowEffects} cleanups={s_rowCleanups} visible={visible}"; + H.Check("Iso_Boundary_FallbackShown", H.FindText("iso-fallback") is not null); + // Precondition: the scenario really realized rows inside the failing pass. + H.Check("Iso_Boundary_RowsRealizedReentrantly", realizedDuringPass > 0, detail); + H.Check("Iso_Boundary_LiveRowsNotTornDown", s_rowCleanups == 0 && visible > 0, detail); + } + } + + // A row that throws while realized re-entrantly under a mounting boundary is the + // row's own failure: it must not be routed to (and fail) that unrelated boundary. + internal sealed class ThrowingRowRealizedDuringBoundaryMount_DoesNotTripBoundary(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + s_forceLayout = null; + var host = H.CreateHost(); + host.Mount(ctx => + { + var (phase, setPhase) = ctx.UseState(0); + IReadOnlyList items = Enumerable.Range(0, phase == 0 ? 2 : 20).ToList(); + return VStack( + Button("IsoThrowGo", () => setPhase(1)), + LazyVStack(items, i => i.ToString(), (i, _) => + Component(new IndexProps(i))).Height(300), + phase == 0 + ? TextBlock("iso-throw-idle") + : ErrorBoundary( + VStack(Component(), TextBlock("iso-boundary-ok")), + TextBlock("iso-throw-fallback"))); + }); + await Harness.Render(); + s_forceLayout = ForceLayoutOf(H); + + H.ClickButton("IsoThrowGo"); + await Harness.Render(); + s_forceLayout = null; + + H.Check("Iso_Throw_BoundaryChildRendered", H.FindText("iso-boundary-ok") is not null); + H.Check("Iso_Throw_BoundaryNotTripped", H.FindText("iso-throw-fallback") is null); + H.Check("Iso_Throw_RowShowsItsOwnFallback", + H.FindTextContaining("row-boom-") is not null, + H.FindTextContaining("Render error")?.Text ?? ""); + } + } + + // ── Window host during realization ─────────────────────────────────────── + + private static readonly List s_seenHosts = new(); + + private sealed class HostProbeRow : Component + { + public override Element Render() + { + s_seenHosts.Add(ReactorApp.ActiveHostInternal); + return TextBlock($"hrow{Props.Index}").Height(30); + } + } + + // ActiveHostInternal is only pushed for a host's own render pass; between passes it + // is whichever host was constructed last, or null after any host is disposed. Rows + // realized while scrolling must still resolve their own host (window-scoped hooks). + internal sealed class RowsRealizedOutsideRender_SeeOwningHost(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + s_seenHosts.Clear(); + var host = H.CreateHost(); + host.Mount(_ => LazyVStack(Enumerable.Range(0, 300).ToList(), i => i.ToString(), (i, _) => + Component(new IndexProps(i))).Height(300)); + await Harness.Render(); + + var previous = ReactorApp.ActiveHostInternal; + try + { + // What ReactorHost.Dispose() of any other window's host leaves behind. + ReactorApp.ActiveHostInternal = null; + s_seenHosts.Clear(); + var sv = H.FindControl(s => s.Content is ItemsRepeater)!; + sv.ChangeView(null, 150 * 30, null, disableAnimation: true); + sv.UpdateLayout(); + (sv.XamlRoot?.Content as UIElement)?.UpdateLayout(); + + int total = s_seenHosts.Count; + int own = s_seenHosts.Count(x => ReferenceEquals(x, host)); + H.Check("Iso_Host_RowsRealized", total > 0, $"realized={total}"); + H.Check("Iso_Host_RowsSeeOwningHost", total > 0 && own == total, $"own={own}/{total}"); + H.Check("Iso_Host_RestoredAfterRealization", ReactorApp.ActiveHostInternal is null); + } + finally + { + ReactorApp.ActiveHostInternal = previous; + } + await Harness.Render(); + } + } + + // ── Nested host inside a parked row, provider removed meanwhile ────────── + + private sealed record NestedProps(int Outer, int Inner); + + private sealed class NestedConsumer : Component + { + public override Element Render() + => TextBlock($"n{Props.Outer}.{Props.Inner}:{UseContext(Ctx)}").Height(30); + } + + // Inner rows below 10 do not consume context, so when the outer row comes back + // nothing realized inside the nested host can reveal that the provider is gone — + // only the nested host's own snapshot can. + private static Element InnerList(int outer) + => LazyVStack(Enumerable.Range(0, 40).ToList(), j => j.ToString(), (j, _) => + j < 10 + ? TextBlock($"n{outer}.{j} plain").Height(30) + : Component(new NestedProps(outer, j))) + .Height(120); + + internal sealed class NestedHostInRecycledRow_SeesProviderRemoval(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var inner = Enumerable.Range(0, 200).Select(InnerList).ToArray(); + var outerList = LazyVStack(Enumerable.Range(0, 200).ToList(), i => i.ToString(), (i, _) => + Border(inner[i]).Height(130)) + .Height(300); + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (on, setOn) = ctx.UseState(true); + var body = VStack( + Button("IsoNestedDrop", () => setOn(false)), + TextBlock($"iso-nested-on:{on}"), + outerList); + return on ? body.Provide(Ctx, "removed") : body; + }); + await Harness.Render(); + + ScrollViewer Outer() => H.FindControl(s => s.Content is ItemsRepeater && s.Height == 300)!; + H.Check("Iso_Nested_Row0Realized", H.FindText("n0.0 plain") is not null); + + // Park outer row 0 (with its nested host) by scrolling far away. + Outer().ChangeView(null, 100 * 130, null, disableAnimation: true); + await Harness.Render(); + await Harness.Render(); + H.Check("Iso_Nested_Row0Parked", H.FindAllControls(t => t.Text == "n0.0 plain") + .All(t => !IsEffectivelyVisible(t))); + + H.ClickButton("IsoNestedDrop"); + await Harness.Render(); + H.Check("Iso_Nested_ProviderRemoved", H.FindText("iso-nested-on:False") is not null); + + // Bring row 0 back: the repeater reuses its parked container. + Outer().ChangeView(null, 0, null, disableAnimation: true); + await Harness.Render(); + await Harness.Render(); + + var innerSv = H.FindAllControls(t => t.Text == "n0.0 plain" && IsEffectivelyVisible(t)) + .Select(AncestorScrollViewer).FirstOrDefault(s => s is not null); + H.Check("Iso_Nested_Row0Back", innerSv is not null); + if (innerSv is null) return; + + // Realize consumer rows inside the nested host. + innerSv.ChangeView(null, 20 * 30, null, disableAnimation: true); + await Harness.Render(); + await Harness.Render(); + var consumers = H.FindAllControls(t => t.Text?.StartsWith("n0.", StringComparison.Ordinal) == true + && t.Text.Contains(':') && IsEffectivelyVisible(t)) + .Select(t => t.Text).ToList(); + H.Check("Iso_Nested_ConsumersSeeDefault", + consumers.Count > 0 && consumers.All(t => t.EndsWith(":default", StringComparison.Ordinal)), + consumers.Count == 0 ? "" : string.Join(",", consumers.Take(5))); + } + + private static bool IsEffectivelyVisible(DependencyObject d) + { + for (var cur = d; cur is not null; cur = VisualTreeHelper.GetParent(cur)) + if (cur is UIElement { Visibility: Visibility.Collapsed }) return false; + return true; + } + + private static ScrollViewer? AncestorScrollViewer(DependencyObject d) + { + for (var cur = VisualTreeHelper.GetParent(d); cur is not null; cur = VisualTreeHelper.GetParent(cur)) + if (cur is ScrollViewer { Content: Microsoft.UI.Xaml.Controls.ItemsRepeater } sv) return sv; + return null; + } + } + + // ── Retired host releases its provider values ───────────────────────────── + + private sealed record Payload(string Name); + private static readonly Context PayloadCtx = new(defaultValue: null); + + internal sealed class DetachedHost_ReleasesFactoryCapture(Harness h) : SelfTestFixtureBase(h) + { + public override async Task RunAsync() + { + var host = H.CreateHost(); + host.Mount(_ => VStack( + LazyVStack(Enumerable.Range(0, 20).ToList(), i => i.ToString(), (i, _) => + TextBlock($"d{i}").Height(30)).Height(200)) + .Provide(PayloadCtx, new Payload("retained"))); + await Harness.Render(); + + var sv = H.FindControl(s => s.Content is ItemsRepeater)!; + var factory = (sv.Content as ItemsRepeater)?.ItemTemplate as IContextCapturingFactory; + var capture = factory?.ContextCapture; + H.Check("Iso_Detach_FactoryHasCapture", capture is not null && capture.Entries.Length > 0); + if (capture is null) return; + + // What retiring a parked outer row does to every control in it. + Reconciler.DetachReactorState(sv); + H.Check("Iso_Detach_FactoryAliasReleased", capture.Entries.Length == 0, + $"entries={capture.Entries.Length}"); + } + } +} diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 398c55b28..acf29c3ef 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -625,6 +625,11 @@ internal static class SelfTestFixtureRegistry "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator", "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator", "VirtualizedCtx_Factory_RecycledRowAfterLastProviderRemoved", + "Iso_RowsRealizedDuringFailedBoundaryMount_StayAlive", + "Iso_ThrowingRowRealizedDuringBoundaryMount_DoesNotTripBoundary", + "Iso_RowsRealizedOutsideRender_SeeOwningHost", + "Iso_NestedHostInRecycledRow_SeesProviderRemoval", + "Iso_DetachedHost_ReleasesFactoryCapture", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2690,6 +2695,11 @@ internal static string[] StaleTierDeclarations() => "VirtualizedCtx_LazyVStack_OwnProviderUnderDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderDecorator(harness), "VirtualizedCtx_LazyVStack_OwnProviderUnderSkippedDecorator" => new VirtualizedRowContextFixtures.LazyVStack_OwnProviderUnderSkippedDecorator(harness), "VirtualizedCtx_Factory_RecycledRowAfterLastProviderRemoved" => new VirtualizedRowContextFixtures.Factory_RecycledRowAfterLastProviderRemoved(harness), + "Iso_RowsRealizedDuringFailedBoundaryMount_StayAlive" => new RealizationIsolationFixtures.RowsRealizedDuringFailedBoundaryMount_StayAlive(harness), + "Iso_ThrowingRowRealizedDuringBoundaryMount_DoesNotTripBoundary" => new RealizationIsolationFixtures.ThrowingRowRealizedDuringBoundaryMount_DoesNotTripBoundary(harness), + "Iso_RowsRealizedOutsideRender_SeeOwningHost" => new RealizationIsolationFixtures.RowsRealizedOutsideRender_SeeOwningHost(harness), + "Iso_NestedHostInRecycledRow_SeesProviderRemoval" => new RealizationIsolationFixtures.NestedHostInRecycledRow_SeesProviderRemoval(harness), + "Iso_DetachedHost_ReleasesFactoryCapture" => new RealizationIsolationFixtures.DetachedHost_ReleasesFactoryCapture(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index 265446ab2..34f130d28 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -1,3 +1,4 @@ +using Microsoft.UI.Xaml; using Microsoft.UI.Reactor.Core; using Xunit; @@ -196,34 +197,62 @@ public void HasActiveValues_StaysOffDuringEmptyReplay_WhenNoProviderWasEverSeen( // ── ContextCapture ─────────────────────────────────────────────── + private static ContextCapture Captured(ContextScope scope) + { + var capture = new ContextCapture(); + capture.Refresh(scope, ElementTheme.Default); + return capture; + } + [Fact] public void Capture_IsStale_TracksValueChangesAndAdditions() { var scope = new ContextScope(); Push(scope, Theme, "dark"); - var capture = new ContextCapture(); - capture.Refresh(scope); - Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); + var capture = Captured(scope); + Assert.False(capture.IsStale(scope)); Push(scope, Count, 1); // provider added above the host - Assert.True(capture.IsStale(scope, inProviderChangeSubtree: false)); + Assert.True(capture.IsStale(scope)); - capture.Refresh(scope); - Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); + capture.Refresh(scope, ElementTheme.Default); + Assert.False(capture.IsStale(scope)); } [Fact] - public void Capture_IsStale_RemovedProviderNeedsTheProviderChangeSignal() + public void Capture_IsStale_RemovedProviderNeedsTheGenerationSignal() { var scope = new ContextScope(); Push(scope, Theme, "dark"); Push(scope, Count, 1); - var capture = new ContextCapture(); - capture.Refresh(scope); + var capture = Captured(scope); scope.Pop(1); // provider removed: the shorter stack is still a prefix - Assert.False(capture.IsStale(scope, inProviderChangeSubtree: false)); - Assert.True(capture.IsStale(scope, inProviderChangeSubtree: true)); + Assert.False(capture.IsStale(scope)); + + scope.BumpProviderGeneration(); // what the reconciler does for a changed provider + Assert.True(capture.IsStale(scope)); + + capture.Refresh(scope, ElementTheme.Default); + Assert.False(capture.IsStale(scope)); + Assert.Single(capture.Entries); + } + + [Fact] + public void Capture_IsStale_GenerationOutlivesTheChangingPass() + { + // A host that was NOT reached in the pass that removed the provider (a nested + // host inside a parked row) must still be stale when it is reached later, under + // a stack that by then looks like an ancestor's. + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var nested = Captured(scope); + scope.Pop(1); + scope.BumpProviderGeneration(); + + var previous = scope.Replace([]); // later realization replays the outer snapshot + Assert.True(nested.IsStale(scope)); + scope.Restore(previous); } [Fact] @@ -231,17 +260,37 @@ public void Capture_Refresh_KeepsArrayWhenNothingChanged() { var scope = new ContextScope(); Push(scope, Theme, "dark"); - var capture = new ContextCapture(); - capture.Refresh(scope); + var capture = Captured(scope); var first = capture.Entries; - capture.Refresh(scope); + capture.Refresh(scope, ElementTheme.Default); Assert.Same(first, capture.Entries); scope.Pop(1); Push(scope, Theme, "light"); - capture.Refresh(scope); + capture.Refresh(scope, ElementTheme.Default); Assert.NotSame(first, capture.Entries); Assert.Equal("light", capture.Entries[0].Value); } + + [Fact] + public void Capture_RecordsAmbientTheme() + { + var scope = new ContextScope(); + var capture = new ContextCapture(); + capture.Refresh(scope, ElementTheme.Dark); + Assert.Equal(ElementTheme.Dark, capture.AmbientTheme); + } + + [Fact] + public void Capture_Clear_ReleasesValuesAndIsStale() + { + var scope = new ContextScope(); + Push(scope, Data, new Payload("big")); + var capture = Captured(scope); + + capture.Clear(); + Assert.Empty(capture.Entries); + Assert.True(capture.IsStale(scope)); + } } From ef99d20eb7ffd3e85bc58cc3ceac0cb8ce6e2530 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 11:26:17 -0700 Subject: [PATCH 06/10] Address review round 4 - Read the ambient theme for realized rows live from the host control's ActualTheme instead of capturing it at reconcile time. A RequestedTheme change on an ancestor while the host is skipped no longer leaves rows resolving ThemeRef overrides against the old theme (works with no Context providers at all). - Use an epsilon comparison in the nested-host fixture's selector. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- docs/guide/architecture-overview.md | 2 +- src/Reactor/Core/ContextScope.cs | 11 +-- src/Reactor/Core/ElementFactory.cs | 2 +- src/Reactor/Core/Reconciler.Update.cs | 4 +- src/Reactor/Core/Reconciler.cs | 59 +++++++++------ .../Fixtures/RealizationIsolationFixtures.cs | 73 ++++++++++++++++++- .../SelfTest/SelfTestFixtureRegistry.cs | 2 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 20 ++--- 8 files changed, 119 insertions(+), 54 deletions(-) diff --git a/docs/guide/architecture-overview.md b/docs/guide/architecture-overview.md index a70a479aa..47e814fb5 100644 --- a/docs/guide/architecture-overview.md +++ b/docs/guide/architecture-overview.md @@ -155,7 +155,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture); + using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture, args.Parent); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index c188de473..9eaf405a0 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -1,5 +1,3 @@ -using Microsoft.UI.Xaml; - namespace Microsoft.UI.Reactor.Core; /// @@ -165,18 +163,11 @@ internal sealed class ContextCapture /// when was taken. internal long Generation { get; private set; } = -1; - /// - /// The ancestor-resolved RequestedTheme at the host, so ThemeRef resource - /// overrides on realized rows resolve against the same theme as the rest of the subtree. - /// - internal ElementTheme AmbientTheme { get; private set; } - - internal void Refresh(ContextScope scope, ElementTheme ambientTheme) + internal void Refresh(ContextScope scope) { if (!scope.Matches(Entries)) Entries = scope.Snapshot(); Generation = scope.ProviderGeneration; - AmbientTheme = ambientTheme; } /// diff --git a/src/Reactor/Core/ElementFactory.cs b/src/Reactor/Core/ElementFactory.cs index f4dbb12a0..d07026abc 100644 --- a/src/Reactor/Core/ElementFactory.cs +++ b/src/Reactor/Core/ElementFactory.cs @@ -621,7 +621,7 @@ public UIElement GetElement(ElementFactoryGetArgs args) if (index < 0 || index >= _items.Count) return new TextBlock { Text = "" }; - using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture); + using var restoreContext = _reconciler.EnterRealization(_hostCapture ?? _ownCapture, args.Parent); var item = _items[index]; var element = BuildOrCache(key, item, index, keyed); diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 6044c4ddb..6e3698991 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -131,7 +131,7 @@ public sealed partial class Reconciler // A virtualized host's own skipped element re-snapshots here at its own depth // (no-op for every other control or a decorator sharing it, and // allocation-free when nothing changed). - RefreshContextCaptureOnSkip(control, newEl, effectiveTheme); + RefreshContextCaptureOnSkip(control, newEl); // No validation refresh here, deliberately (issue #1262). A `.Validate()` // verdict is republished only by a component that re-rendered, and that // component's output is on the dirty ancestor path, which this arm declines @@ -186,7 +186,7 @@ public sealed partial class Reconciler // Virtualized hosts realize rows outside this traversal; keep their provider // snapshot current (no-op for every other control, and for a decorator that // shares the host's control — the host element's own Update refreshes it). - RefreshContextCapture(control, newEl, effectiveTheme); + RefreshContextCapture(control, newEl); var prevReconcilingElement = _reconcilingElement; _reconcilingElement = newEl; diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index cc37fd45b..fd5512938 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -1971,7 +1971,7 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) $"{host.GetType().Name} is not covered by MayHostContextCapture; its capture would never refresh."); var state = GetOrCreateReactorState(host); var capture = state.ContextCapture ??= new ContextCapture { OwnerType = _reconcilingElement?.GetType() }; - capture.Refresh(_contextScope, _ambientRequestedTheme); + capture.Refresh(_contextScope); _hasContextCaptures = true; return capture; } @@ -1987,10 +1987,10 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) /// Re-snapshots a host's provider stack when is the element that /// owns the capture; a no-op for any other control or element. /// - internal void RefreshContextCapture(UIElement control, Element newEl, ElementTheme effectiveTheme) + internal void RefreshContextCapture(UIElement control, Element newEl) { if (TryGetContextCapture(control) is { } capture && capture.IsOwnedBy(newEl)) - capture.Refresh(_contextScope, effectiveTheme); + capture.Refresh(_contextScope); } /// @@ -2001,17 +2001,17 @@ internal void RefreshContextCapture(UIElement control, Element newEl, ElementThe /// untouched; if a provider above it changed, the stale check declines that skip and /// the decorator's Update reaches the host element. /// - internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl, ElementTheme effectiveTheme) + internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) { if (TryGetContextCapture(control) is not { } capture || !capture.IsOwnedBy(newEl)) return; var own = newEl.ContextValues; if (own is not { Count: > 0 }) { - capture.Refresh(_contextScope, effectiveTheme); + capture.Refresh(_contextScope); return; } _contextScope.Push(own); - try { capture.Refresh(_contextScope, effectiveTheme); } + try { capture.Refresh(_contextScope); } finally { _contextScope.Pop(own.Count); } } @@ -2022,18 +2022,19 @@ internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl, Elem internal ContextCapture CaptureLiveContext() { var capture = new ContextCapture(); - capture.Refresh(_contextScope, _ambientRequestedTheme); + capture.Refresh(_contextScope); return capture; } - internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope, _ambientRequestedTheme); + internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope); /// /// Opens a realization frame for building a row outside (or re-entrantly inside) the /// traversal. Until the returned scope is disposed: /// - /// the live provider stack is replaced with - /// and the ambient theme with the one captured at the host; + /// the live provider stack is replaced with , + /// and the ambient theme is read live off (see + /// ); /// the in-flight pass's per-subtree bookkeeping is set aside, so the /// row is not recorded in an ErrorBoundary's rollback lists (a failing boundary child /// would otherwise tear down a live row in an unrelated list), a row that throws gets @@ -2042,12 +2043,29 @@ internal ContextCapture CaptureLiveContext() /// the owning host is the active host, so window-scoped hooks in the /// row resolve this window even outside the host's render pass. /// - /// A null capture still isolates; it just leaves the provider stack and theme as they are. + /// A null capture still isolates; it just leaves the provider stack as it is. /// - internal RealizationFrame EnterRealization(ContextCapture? capture) => new(this, capture); + internal RealizationFrame EnterRealization(ContextCapture? capture, UIElement? host) + => new(this, capture, LiveAmbientTheme(host)); - /// - internal RealizationFrame EnterRealization(UIElement host) => EnterRealization(TryGetContextCapture(host)); + /// + internal RealizationFrame EnterRealization(UIElement host) => EnterRealization(TryGetContextCapture(host), host); + + /// + /// The theme a row realized into should resolve ThemeRef + /// resource overrides against. Read from the host itself rather than captured at + /// reconcile time: the host is in the tree, so its ActualTheme already reflects + /// every ancestor RequestedTheme — including one that changed while the host was + /// skipped. Returns when that matches the app theme, + /// preserving the reconciler's "no ancestor override" meaning. + /// + private static ElementTheme LiveAmbientTheme(UIElement? host) + { + if (host is not FrameworkElement fe) return ElementTheme.Default; + var actual = fe.ActualTheme; + var app = Application.Current?.RequestedTheme == ApplicationTheme.Dark ? ElementTheme.Dark : ElementTheme.Light; + return actual == app ? ElementTheme.Default : actual; + } /// /// Registers (see ) and @@ -2073,7 +2091,7 @@ internal void AttachContextCapture(IElementFactory factory, FrameworkElement hos private readonly bool _swapHost; private readonly ReactorHost? _previousHost; - internal RealizationFrame(Reconciler owner, ContextCapture? capture) + internal RealizationFrame(Reconciler owner, ContextCapture? capture, ElementTheme ambientTheme) { _owner = owner; _previousTheme = owner._ambientRequestedTheme; @@ -2089,17 +2107,10 @@ internal RealizationFrame(Reconciler owner, ContextCapture? capture) owner._errorBoundaryDepth = 0; owner._providerChangeDepth = 0; owner._reconcilingElement = null; + owner._ambientRequestedTheme = ambientTheme; _staggerScope = null; - if (capture is not null) - { - _previousStack = owner._contextScope.Replace(capture.Entries); - owner._ambientRequestedTheme = capture.AmbientTheme; - } - else - { - _previousStack = null; - } + _previousStack = capture is not null ? owner._contextScope.Replace(capture.Entries) : null; _swapHost = owner.OwnerHost is not null; _previousHost = _swapHost ? ReactorApp.ActiveHostInternal : null; diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs index 619891d34..63b0c2367 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -1,6 +1,7 @@ using Microsoft.UI.Reactor; using Microsoft.UI.Reactor.Core; using Microsoft.UI.Reactor.AppTests.Host.SelfTest; +using Microsoft.UI.Reactor.Elements; using Microsoft.UI.Reactor.Hosting; using Microsoft.UI.Xaml; using Microsoft.UI.Xaml.Controls; @@ -240,7 +241,7 @@ public override async Task RunAsync() }); await Harness.Render(); - ScrollViewer Outer() => H.FindControl(s => s.Content is ItemsRepeater && s.Height == 300)!; + ScrollViewer Outer() => H.FindControl(s => s.Content is ItemsRepeater && global::System.Math.Abs(s.Height - 300.0) < 0.001)!; H.Check("Iso_Nested_Row0Realized", H.FindText("n0.0 plain") is not null); // Park outer row 0 (with its nested host) by scrolling far away. @@ -319,4 +320,74 @@ public override async Task RunAsync() $"entries={capture.Entries.Length}"); } } + + // ── Ancestor theme changes while the host is skipped ───────────────────── + + // No Context providers anywhere: the provider checks are not even consulted. The + // list is reference-stable, so a theme toggle on its parent skips it, and rows + // realized afterwards must still resolve ThemeRef overrides against the NEW theme. + internal sealed class AncestorThemeToggledWhileSkipped_RowsResolveNewTheme(Harness h) : SelfTestFixtureBase(h) + { + private const string SrcKey = "IsoThemeSrcBrush"; + private const string TargetKey = "IsoThemeTargetBrush"; + + public override async Task RunAsync() + { + var light = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 0, 0, 220)); + var dark = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 220, 0, 0)); + var dict = new ResourceDictionary(); + dict.ThemeDictionaries["Light"] = new ResourceDictionary { [SrcKey] = light }; + dict.ThemeDictionaries["Dark"] = new ResourceDictionary { [SrcKey] = dark }; + Application.Current.Resources.MergedDictionaries.Add(dict); + ThemeRef.InvalidateResolutionCache(); + try + { + bool appIsDark = Application.Current.RequestedTheme == ApplicationTheme.Dark; + var first = appIsDark ? ElementTheme.Light : ElementTheme.Dark; + var second = appIsDark ? ElementTheme.Dark : ElementTheme.Light; + SolidColorBrush BrushFor(ElementTheme t) => t == ElementTheme.Dark ? dark : light; + + var stableList = LazyVStack(Enumerable.Range(0, 300).ToList(), i => i.ToString(), (i, _) => + TextBlock($"th{i}").Height(30).Resources(r => r.Set(TargetKey, Theme.Ref(SrcKey)))) + .Height(300); + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (flipped, setFlipped) = ctx.UseState(false); + return VStack( + Button("IsoThemeFlip", () => setFlipped(true)), + TextBlock($"iso-theme:{flipped}"), + stableList) + .RequestedTheme(flipped ? second : first); + }); + await Harness.Render(); + + var row0 = H.FindText("th0"); + H.Check("Iso_Theme_MountResolvesAncestor", + row0?.Resources.TryGetValue(TargetKey, out var b0) == true && ReferenceEquals(b0, BrushFor(first))); + + H.ClickButton("IsoThemeFlip"); + await Harness.Render(); + H.Check("Iso_Theme_Flipped", H.FindText("iso-theme:True") is not null); + + var sv = H.FindControl(s => s.Content is ItemsRepeater)!; + sv.ChangeView(null, 150 * 30, null, disableAnimation: true); + await Harness.Render(); + await Harness.Render(); + + var scrolled = H.FindAllControls(t => t.Text?.StartsWith("th1", StringComparison.Ordinal) == true + && int.TryParse(t.Text.AsSpan(2), out var n) && n >= 100) + .ToList(); + int right = scrolled.Count(t => t.Resources.TryGetValue(TargetKey, out var b) && ReferenceEquals(b, BrushFor(second))); + H.Check("Iso_Theme_ScrolledRowsResolveNewTheme", scrolled.Count > 0 && right == scrolled.Count, + $"right={right}/{scrolled.Count}"); + } + finally + { + Application.Current.Resources.MergedDictionaries.Remove(dict); + ThemeRef.InvalidateResolutionCache(); + } + } + } } diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index acf29c3ef..f32b11459 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -630,6 +630,7 @@ internal static class SelfTestFixtureRegistry "Iso_RowsRealizedOutsideRender_SeeOwningHost", "Iso_NestedHostInRecycledRow_SeesProviderRemoval", "Iso_DetachedHost_ReleasesFactoryCapture", + "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2700,6 +2701,7 @@ internal static string[] StaleTierDeclarations() => "Iso_RowsRealizedOutsideRender_SeeOwningHost" => new RealizationIsolationFixtures.RowsRealizedOutsideRender_SeeOwningHost(harness), "Iso_NestedHostInRecycledRow_SeesProviderRemoval" => new RealizationIsolationFixtures.NestedHostInRecycledRow_SeesProviderRemoval(harness), "Iso_DetachedHost_ReleasesFactoryCapture" => new RealizationIsolationFixtures.DetachedHost_ReleasesFactoryCapture(harness), + "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme" => new RealizationIsolationFixtures.AncestorThemeToggledWhileSkipped_RowsResolveNewTheme(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index 34f130d28..0401d4967 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -1,4 +1,3 @@ -using Microsoft.UI.Xaml; using Microsoft.UI.Reactor.Core; using Xunit; @@ -200,7 +199,7 @@ public void HasActiveValues_StaysOffDuringEmptyReplay_WhenNoProviderWasEverSeen( private static ContextCapture Captured(ContextScope scope) { var capture = new ContextCapture(); - capture.Refresh(scope, ElementTheme.Default); + capture.Refresh(scope); return capture; } @@ -215,7 +214,7 @@ public void Capture_IsStale_TracksValueChangesAndAdditions() Push(scope, Count, 1); // provider added above the host Assert.True(capture.IsStale(scope)); - capture.Refresh(scope, ElementTheme.Default); + capture.Refresh(scope); Assert.False(capture.IsStale(scope)); } @@ -233,7 +232,7 @@ public void Capture_IsStale_RemovedProviderNeedsTheGenerationSignal() scope.BumpProviderGeneration(); // what the reconciler does for a changed provider Assert.True(capture.IsStale(scope)); - capture.Refresh(scope, ElementTheme.Default); + capture.Refresh(scope); Assert.False(capture.IsStale(scope)); Assert.Single(capture.Entries); } @@ -263,25 +262,16 @@ public void Capture_Refresh_KeepsArrayWhenNothingChanged() var capture = Captured(scope); var first = capture.Entries; - capture.Refresh(scope, ElementTheme.Default); + capture.Refresh(scope); Assert.Same(first, capture.Entries); scope.Pop(1); Push(scope, Theme, "light"); - capture.Refresh(scope, ElementTheme.Default); + capture.Refresh(scope); Assert.NotSame(first, capture.Entries); Assert.Equal("light", capture.Entries[0].Value); } - [Fact] - public void Capture_RecordsAmbientTheme() - { - var scope = new ContextScope(); - var capture = new ContextCapture(); - capture.Refresh(scope, ElementTheme.Dark); - Assert.Equal(ElementTheme.Dark, capture.AmbientTheme); - } - [Fact] public void Capture_Clear_ReleasesValuesAndIsStale() { From 0cea32b0f4481855d88474be6458e074f2edeab1 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 11:55:55 -0700 Subject: [PATCH 07/10] Address review round 5 Use the pass's pending subtree theme for rows realized re-entrantly in the same reconcile pass that last reached their host. The native RequestedTheme modifier is applied only after children are dispatched, so the host's ActualTheme still shows the previous theme then. Hosts record the pass id and effective theme at mount, update and skip; outside that pass the live ActualTheme is still used, so a host skipped while an ancestor theme changes stays correct. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 18 ++++ src/Reactor/Core/Reconciler.Update.cs | 4 +- src/Reactor/Core/Reconciler.cs | 35 +++++++- .../Fixtures/RealizationIsolationFixtures.cs | 82 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + 5 files changed, 135 insertions(+), 6 deletions(-) diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 9eaf405a0..4807cebcb 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -1,3 +1,5 @@ +using Microsoft.UI.Xaml; + namespace Microsoft.UI.Reactor.Core; /// @@ -170,6 +172,22 @@ internal void Refresh(ContextScope scope) Generation = scope.ProviderGeneration; } + /// Reconcile pass in which the host was last reached (see ). + internal long PassId { get; private set; } = -1; + + /// + /// The subtree theme the host was reached with in pass . Only + /// meaningful while that pass is still in flight: afterwards the host's native + /// ActualTheme is authoritative and is read live instead. + /// + internal ElementTheme PassTheme { get; private set; } + + internal void NotePass(long passId, ElementTheme theme) + { + PassId = passId; + PassTheme = theme; + } + /// /// Releases the snapshot's provider values. The capture object can be aliased (an /// holds the same instance), so a host leaving Reactor's diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 6e3698991..6044c4ddb 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -131,7 +131,7 @@ public sealed partial class Reconciler // A virtualized host's own skipped element re-snapshots here at its own depth // (no-op for every other control or a decorator sharing it, and // allocation-free when nothing changed). - RefreshContextCaptureOnSkip(control, newEl); + RefreshContextCaptureOnSkip(control, newEl, effectiveTheme); // No validation refresh here, deliberately (issue #1262). A `.Validate()` // verdict is republished only by a component that re-rendered, and that // component's output is on the dirty ancestor path, which this arm declines @@ -186,7 +186,7 @@ public sealed partial class Reconciler // Virtualized hosts realize rows outside this traversal; keep their provider // snapshot current (no-op for every other control, and for a decorator that // shares the host's control — the host element's own Update refreshes it). - RefreshContextCapture(control, newEl); + RefreshContextCapture(control, newEl, effectiveTheme); var prevReconcilingElement = _reconcilingElement; _reconcilingElement = newEl; diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index fd5512938..f5611dd1a 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -320,10 +320,19 @@ private void RollBackDiscardedBoundaryMount(BoundaryMountRecord record, UIElemen private EntryFrame EnterFrame() { var scope = _entryDepth == 0 ? RenderErrorDispatch.EnterPropagationScope() : default; + if (_entryDepth == 0) + _passId++; _entryDepth++; return new EntryFrame(this, scope); } + // Identifies the outermost Reconcile/Mount pass in flight (advanced on entry at depth + // 0). A virtualized host records the pass and the subtree theme it was reached with, + // so a row realized re-entrantly later in the SAME pass can use that pending theme: + // the native RequestedTheme modifier is only applied after children are dispatched, + // so the host's ActualTheme still shows the previous theme at that point. + private long _passId; + private readonly struct EntryFrame(Reconciler owner, RenderErrorDispatch.PropagationScope scope) : IDisposable { public void Dispose() @@ -1972,6 +1981,7 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) var state = GetOrCreateReactorState(host); var capture = state.ContextCapture ??= new ContextCapture { OwnerType = _reconcilingElement?.GetType() }; capture.Refresh(_contextScope); + capture.NotePass(_passId, _ambientRequestedTheme); _hasContextCaptures = true; return capture; } @@ -1987,10 +1997,13 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) /// Re-snapshots a host's provider stack when is the element that /// owns the capture; a no-op for any other control or element. /// - internal void RefreshContextCapture(UIElement control, Element newEl) + internal void RefreshContextCapture(UIElement control, Element newEl, ElementTheme effectiveTheme) { if (TryGetContextCapture(control) is { } capture && capture.IsOwnedBy(newEl)) + { capture.Refresh(_contextScope); + capture.NotePass(_passId, effectiveTheme); + } } /// @@ -2001,9 +2014,10 @@ internal void RefreshContextCapture(UIElement control, Element newEl) /// untouched; if a provider above it changed, the stale check declines that skip and /// the decorator's Update reaches the host element. /// - internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl) + internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl, ElementTheme effectiveTheme) { if (TryGetContextCapture(control) is not { } capture || !capture.IsOwnedBy(newEl)) return; + capture.NotePass(_passId, effectiveTheme); var own = newEl.ContextValues; if (own is not { Count: > 0 }) { @@ -2023,10 +2037,15 @@ internal ContextCapture CaptureLiveContext() { var capture = new ContextCapture(); capture.Refresh(_contextScope); + capture.NotePass(_passId, _ambientRequestedTheme); return capture; } - internal void RefreshLiveContext(ContextCapture capture) => capture.Refresh(_contextScope); + internal void RefreshLiveContext(ContextCapture capture) + { + capture.Refresh(_contextScope); + capture.NotePass(_passId, _ambientRequestedTheme); + } /// /// Opens a realization frame for building a row outside (or re-entrantly inside) the @@ -2046,11 +2065,19 @@ internal ContextCapture CaptureLiveContext() /// A null capture still isolates; it just leaves the provider stack as it is. /// internal RealizationFrame EnterRealization(ContextCapture? capture, UIElement? host) - => new(this, capture, LiveAmbientTheme(host)); + => new(this, capture, RealizationTheme(capture, host)); /// internal RealizationFrame EnterRealization(UIElement host) => EnterRealization(TryGetContextCapture(host), host); + // Inside the pass that last reached the host, the theme that pass is applying to the + // host's subtree (its native RequestedTheme modifier lands only after children are + // dispatched); otherwise the host's live ActualTheme. + private ElementTheme RealizationTheme(ContextCapture? capture, UIElement? host) + => capture is not null && _entryDepth > 0 && capture.PassId == _passId + ? capture.PassTheme + : LiveAmbientTheme(host); + /// /// The theme a row realized into should resolve ThemeRef /// resource overrides against. Read from the host itself rather than captured at diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs index 63b0c2367..05e59f489 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -321,6 +321,88 @@ public override async Task RunAsync() } } + // ── Ancestor theme changes in the same pass that realizes rows ─────────── + + private sealed class ForcerWithDep : Component + { + public override Element Render() + { + UseEffect(() => s_forceLayout?.Invoke(), Props.Index); + return TextBlock("forcer-dep"); + } + } + + // The native RequestedTheme modifier on the ancestor lands only after its children + // are dispatched, so rows realized re-entrantly by a LATER sibling in that same pass + // must use the pass's pending theme, not the host's still-old ActualTheme. + internal sealed class RowsRealizedDuringThemeChangingPass_UsePendingTheme(Harness h) : SelfTestFixtureBase(h) + { + private const string SrcKey = "IsoPassThemeSrcBrush"; + private const string TargetKey = "IsoPassThemeTargetBrush"; + + public override async Task RunAsync() + { + var light = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 0, 0, 200)); + var dark = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 200, 0, 0)); + var dict = new ResourceDictionary(); + dict.ThemeDictionaries["Light"] = new ResourceDictionary { [SrcKey] = light }; + dict.ThemeDictionaries["Dark"] = new ResourceDictionary { [SrcKey] = dark }; + Application.Current.Resources.MergedDictionaries.Add(dict); + ThemeRef.InvalidateResolutionCache(); + s_forceLayout = null; + try + { + bool appIsDark = Application.Current.RequestedTheme == ApplicationTheme.Dark; + var first = appIsDark ? ElementTheme.Light : ElementTheme.Dark; + var second = appIsDark ? ElementTheme.Dark : ElementTheme.Light; + SolidColorBrush BrushFor(ElementTheme t) => t == ElementTheme.Dark ? dark : light; + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (phase, setPhase) = ctx.UseState(0); + IReadOnlyList items = Enumerable.Range(0, phase == 0 ? 2 : 20).ToList(); + return VStack( + Button("IsoPassThemeGo", () => setPhase(1)), + VStack( + LazyVStack(items, i => i.ToString(), (i, _) => + TextBlock($"pt{i}").Height(30).Resources(r => r.Set(TargetKey, Theme.Ref(SrcKey)))) + .Height(300), + Component(new IndexProps(phase))) + .RequestedTheme(phase == 0 ? first : second)); + }); + await Harness.Render(); + + var forceLayout = ForceLayoutOf(H); + var realizedInPass = new List(); + s_forceLayout = () => + { + forceLayout(); + realizedInPass.AddRange(H.FindAllControls(t => t.Text?.StartsWith("pt", StringComparison.Ordinal) == true + && int.TryParse(t.Text.AsSpan(2), out var n) && n >= 2) + .Select(t => t.Text)); + }; + + H.ClickButton("IsoPassThemeGo"); + await Harness.Render(); + s_forceLayout = null; + + var rows = H.FindAllControls(t => t.Text?.StartsWith("pt", StringComparison.Ordinal) == true + && int.TryParse(t.Text.AsSpan(2), out var n) && n >= 2) + .ToList(); + int right = rows.Count(t => t.Resources.TryGetValue(TargetKey, out var b) && ReferenceEquals(b, BrushFor(second))); + H.Check("Iso_PassTheme_RowsRealizedInPass", realizedInPass.Count > 0, $"inPass={realizedInPass.Count}"); + H.Check("Iso_PassTheme_RowsUseNewTheme", rows.Count > 0 && right == rows.Count, $"right={right}/{rows.Count}"); + } + finally + { + s_forceLayout = null; + Application.Current.Resources.MergedDictionaries.Remove(dict); + ThemeRef.InvalidateResolutionCache(); + } + } + } + // ── Ancestor theme changes while the host is skipped ───────────────────── // No Context providers anywhere: the provider checks are not even consulted. The diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index f32b11459..3390f91e7 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -631,6 +631,7 @@ internal static class SelfTestFixtureRegistry "Iso_NestedHostInRecycledRow_SeesProviderRemoval", "Iso_DetachedHost_ReleasesFactoryCapture", "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme", + "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2702,6 +2703,7 @@ internal static string[] StaleTierDeclarations() => "Iso_NestedHostInRecycledRow_SeesProviderRemoval" => new RealizationIsolationFixtures.NestedHostInRecycledRow_SeesProviderRemoval(harness), "Iso_DetachedHost_ReleasesFactoryCapture" => new RealizationIsolationFixtures.DetachedHost_ReleasesFactoryCapture(harness), "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme" => new RealizationIsolationFixtures.AncestorThemeToggledWhileSkipped_RowsResolveNewTheme(harness), + "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme" => new RealizationIsolationFixtures.RowsRealizedDuringThemeChangingPass_UsePendingTheme(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), From 7665a9e6053154ccbbe0aa79b4e157728dfa1ce1 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 12:52:08 -0700 Subject: [PATCH 08/10] Address review round 6 A host snapshot taken while an outdated snapshot is being replayed (a nested host mounted inside a row realized before the traversal refreshed the outer host) now records the replayed snapshot's provider generation instead of the current one, so it is not marked current while holding a removed provider. ContextScope tracks the generation of each replay; captures record and compare against SnapshotGeneration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 31 +++++++++++-- src/Reactor/Core/Reconciler.cs | 2 +- .../Fixtures/RealizationIsolationFixtures.cs | 46 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 21 +++++++++ 5 files changed, 98 insertions(+), 4 deletions(-) diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 4807cebcb..5e8590cf6 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -76,6 +76,19 @@ internal T Read(Context context) internal void BumpProviderGeneration() => ProviderGeneration++; + // Generations of the snapshots currently being replayed (innermost last). While a + // replay is active the live stack IS that snapshot, so a nested host captured from it + // must record the snapshot's generation — not the current one, which would mark + // possibly-outdated replayed values as current. + private readonly List _replayGenerations = new(); + + /// + /// The generation the live stack's contents correspond to: the replayed snapshot's + /// while a replay is active, otherwise . + /// + internal long SnapshotGeneration + => _replayGenerations.Count > 0 ? _replayGenerations[^1] : ProviderGeneration; + internal (ContextBase Context, object? Value)[] Snapshot() => _stack.Count == 0 ? [] : _stack.ToArray(); @@ -124,9 +137,20 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) /// providers of whatever subtree the traversal happened to be in. /// internal List<(ContextBase Context, object? Value)> Replace((ContextBase Context, object? Value)[] snapshot) + => Replace(snapshot, SnapshotGeneration); + + /// + /// Replaces the live stack with , recording the provider + /// generation it was taken at (see ), and returns the + /// displaced stack for . + /// + /// The provider entries to replay. + /// The was taken at. + internal List<(ContextBase Context, object? Value)> Replace((ContextBase Context, object? Value)[] snapshot, long generation) { var previous = _stack; _stack = new List<(ContextBase Context, object? Value)>(snapshot); + _replayGenerations.Add(generation); _replayDepth++; _version++; return previous; @@ -135,6 +159,7 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) internal void Restore(List<(ContextBase Context, object? Value)> previous) { _stack = previous; + _replayGenerations.RemoveAt(_replayGenerations.Count - 1); _replayDepth--; _version++; } @@ -162,14 +187,14 @@ internal sealed class ContextCapture internal bool IsOwnedBy(Element element) => OwnerType is null || OwnerType == element.GetType(); - /// when was taken. + /// when was taken. internal long Generation { get; private set; } = -1; internal void Refresh(ContextScope scope) { if (!scope.Matches(Entries)) Entries = scope.Snapshot(); - Generation = scope.ProviderGeneration; + Generation = scope.SnapshotGeneration; } /// Reconcile pass in which the host was last reached (see ). @@ -201,5 +226,5 @@ internal void Clear() /// The live stack at an ancestor of (or at) the host. internal bool IsStale(ContextScope scope) - => Generation != scope.ProviderGeneration || !scope.IsPrefixOf(Entries); + => Generation != scope.SnapshotGeneration || !scope.IsPrefixOf(Entries); } diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index f5611dd1a..29160cdd7 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -2137,7 +2137,7 @@ internal RealizationFrame(Reconciler owner, ContextCapture? capture, ElementThem owner._ambientRequestedTheme = ambientTheme; _staggerScope = null; - _previousStack = capture is not null ? owner._contextScope.Replace(capture.Entries) : null; + _previousStack = capture is not null ? owner._contextScope.Replace(capture.Entries, capture.Generation) : null; _swapHost = owner.OwnerHost is not null; _previousHost = _swapHost ? ReactorApp.ActiveHostInternal : null; diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs index 05e59f489..6197f41f3 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -321,6 +321,52 @@ public override async Task RunAsync() } } + // ── Nested host mounted while an outdated snapshot is replayed ──────────── + + // A provider is removed, and before the traversal refreshes the outer host's snapshot, + // a row is realized from the outdated snapshot and mounts a nested host. That nested + // host's snapshot holds the removed provider, so it must not record the current + // generation — otherwise a later skip check treats it as current and its later rows + // read the removed value. Drives a standalone outer factory so the ordering is exact. + internal sealed class NestedHostMountedDuringStaleReplay_IsNotCurrent(Harness h) : SelfTestFixtureBase(h) + { + public override Task RunAsync() + { + var reconciler = new Reconciler(); + Func builder = (i, _) => + LazyVStack(Enumerable.Range(0, 30).ToList(), j => j.ToString(), (j, _) => + TextBlock($"sr{i}.{j}").Height(30)).Height(120); + + ElementFactory outer; + using (reconciler.PushContextDisposable(Ctx, "removed")) + outer = new ElementFactory(Enumerable.Range(0, 5).ToList(), builder, reconciler, static () => { }, pool: null); + + // The pass that removes the provider: a non-skip Update whose own provided + // values changed. Inside it, an earlier sibling forces layout, so a row is + // realized from the outer snapshot before the traversal reaches the outer + // host; then the traversal reaches the (reference-stable) nested host. + reconciler.EnterProviderChange(); + try + { + var row = ((IElementFactory)outer).GetElement(new ElementFactoryGetArgs { Data = 0 }); + var nestedSv = row as ScrollViewer; + var nested = (nestedSv?.Content as ItemsRepeater)?.ItemTemplate as IContextCapturingFactory; + H.Check("Iso_StaleReplay_NestedHostMounted", nested?.ContextCapture is { Entries.Length: 1 }, + $"entries={nested?.ContextCapture?.Entries.Length}"); + if (nestedSv is null) return Task.CompletedTask; + + // The live stack is now empty (the removal happened): the nested host's + // snapshot still holds the removed provider, so it must decline the skip. + H.Check("Iso_StaleReplay_NestedHostDeclinesSkip", reconciler.ShouldDeclineSkip(nestedSv)); + } + finally + { + reconciler.ExitProviderChange(); + } + return Task.CompletedTask; + } + } + // ── Ancestor theme changes in the same pass that realizes rows ─────────── private sealed class ForcerWithDep : Component diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 3390f91e7..22ca0be54 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -632,6 +632,7 @@ internal static class SelfTestFixtureRegistry "Iso_DetachedHost_ReleasesFactoryCapture", "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme", "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme", + "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2704,6 +2705,7 @@ internal static string[] StaleTierDeclarations() => "Iso_DetachedHost_ReleasesFactoryCapture" => new RealizationIsolationFixtures.DetachedHost_ReleasesFactoryCapture(harness), "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme" => new RealizationIsolationFixtures.AncestorThemeToggledWhileSkipped_RowsResolveNewTheme(harness), "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme" => new RealizationIsolationFixtures.RowsRealizedDuringThemeChangingPass_UsePendingTheme(harness), + "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent" => new RealizationIsolationFixtures.NestedHostMountedDuringStaleReplay_IsNotCurrent(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index 0401d4967..e93e01e94 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -254,6 +254,27 @@ public void Capture_IsStale_GenerationOutlivesTheChangingPass() scope.Restore(previous); } + [Fact] + public void Capture_TakenDuringReplay_RecordsTheReplayedGeneration() + { + // Outer snapshot taken under a provider, then the provider is removed. + var scope = new ContextScope(); + Push(scope, Theme, "dark"); + var outer = Captured(scope); + scope.Pop(1); + scope.BumpProviderGeneration(); + + // A row realized from the (now outdated) outer snapshot mounts a nested host. + var previous = scope.Replace(outer.Entries, outer.Generation); + var nested = Captured(scope); + Assert.Equal(outer.Generation, nested.Generation); + Assert.False(nested.IsStale(scope)); // consistent with the replayed stack + scope.Restore(previous); + + // Back in the live traversal the nested snapshot must not look current. + Assert.True(nested.IsStale(scope)); + } + [Fact] public void Capture_Refresh_KeepsArrayWhenNothingChanged() { From 4f9856dbb8b3c0ff06a95ea9240c0cacf70bc46c Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 13:17:35 -0700 Subject: [PATCH 09/10] Address review round 7 A provider change inside a replayed subtree (a reused row dropping its own .Provide around a reference-stable nested host) now advances the replay's generation: to the new live generation if the replay was current, or to a unique never-current value if it was already stale. Nested snapshots taken under the replay before the change are stale, so the nested host declines the skip and re-snapshots. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 21 ++++++++-- .../Fixtures/RealizationIsolationFixtures.cs | 40 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 38 ++++++++++++++++++ 4 files changed, 98 insertions(+), 3 deletions(-) diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 5e8590cf6..6e4b2ad2f 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -74,7 +74,22 @@ internal T Read(Context context) /// internal long ProviderGeneration { get; private set; } - internal void BumpProviderGeneration() => ProviderGeneration++; + internal void BumpProviderGeneration() + { + var before = ProviderGeneration++; + if (_replayGenerations.Count == 0) return; + // A provider changed inside a replayed subtree (e.g. a reused row dropped its own + // .Provide): the replayed stack changed, so snapshots taken under it before the + // change are no longer current. A replay that was current moves to the new live + // generation; one that was already stale gets a value no live generation can + // ever equal, so nothing captured from it is treated as current afterwards. + int top = _replayGenerations.Count - 1; + _replayGenerations[top] = _replayGenerations[top] == before ? ProviderGeneration : _staleReplaySeq--; + } + + // Distinct negative generations for stale replays that saw a provider change. + // Live generations are non-negative and ContextCapture uses long.MinValue for "none". + private long _staleReplaySeq = -1; // Generations of the snapshots currently being replayed (innermost last). While a // replay is active the live stack IS that snapshot, so a nested host captured from it @@ -188,7 +203,7 @@ internal sealed class ContextCapture internal bool IsOwnedBy(Element element) => OwnerType is null || OwnerType == element.GetType(); /// when was taken. - internal long Generation { get; private set; } = -1; + internal long Generation { get; private set; } = long.MinValue; internal void Refresh(ContextScope scope) { @@ -221,7 +236,7 @@ internal void NotePass(long passId, ElementTheme theme) internal void Clear() { Entries = []; - Generation = -1; + Generation = long.MinValue; } /// The live stack at an ancestor of (or at) the host. diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs index 6197f41f3..1a28a52d1 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -367,6 +367,46 @@ public override Task RunAsync() } } + // ── Reused row drops its own provider around a stable nested host ───────── + + // A recycled row is reused for the same item, and the new row element no longer + // carries its own .Provide. The nested list is reference-stable, so the row's Update + // reaches it through the skip check while the outer snapshot is being replayed. The + // provider change inside the replay must make the nested snapshot stale. + internal sealed class ReusedRowDropsOwnProvider_NestedHostRefreshes(Harness h) : SelfTestFixtureBase(h) + { + public override Task RunAsync() + { + var reconciler = new Reconciler(); + var stableInner = LazyVStack(Enumerable.Range(0, 30).ToList(), j => j.ToString(), (j, _) => + TextBlock($"ru{j}").Height(30)).Height(120); + var items = Enumerable.Range(0, 5).ToList(); + Func withProvider = (_, _) => VStack(stableInner).Provide(Ctx, "row-local"); + Func withoutProvider = (_, _) => VStack(stableInner); + + var outer = new ElementFactory(items, withProvider, reconciler, static () => { }, pool: null); + IElementFactory f = outer; + + var first = f.GetElement(new ElementFactoryGetArgs { Data = 0 }); + ContextCapture? NestedCapture(UIElement row) + => ((row as StackPanel ?? (row as Panel))?.Children.OfType().FirstOrDefault()?.Content + as ItemsRepeater)?.ItemTemplate is IContextCapturingFactory c ? c.ContextCapture : null; + var nestedBefore = NestedCapture(first); + H.Check("Iso_RowLocal_NestedSeesRowProvider", nestedBefore is { Entries.Length: 1 }, + $"entries={nestedBefore?.Entries.Length} root={first.GetType().Name}"); + + f.RecycleElement(new ElementFactoryRecycleArgs { Element = first }); + outer.UpdateInPlace(items, withoutProvider); + var again = f.GetElement(new ElementFactoryGetArgs { Data = 0 }); + + H.Check("Iso_RowLocal_ContainerReused", ReferenceEquals(first, again)); + var nestedAfter = NestedCapture(again); + H.Check("Iso_RowLocal_NestedDroppedRemovedProvider", nestedAfter is { Entries.Length: 0 }, + $"entries={nestedAfter?.Entries.Length}"); + return Task.CompletedTask; + } + } + // ── Ancestor theme changes in the same pass that realizes rows ─────────── private sealed class ForcerWithDep : Component diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 22ca0be54..4704c645c 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -633,6 +633,7 @@ internal static class SelfTestFixtureRegistry "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme", "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme", "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent", + "Iso_ReusedRowDropsOwnProvider_NestedHostRefreshes", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2706,6 +2707,7 @@ internal static string[] StaleTierDeclarations() => "Iso_AncestorThemeToggledWhileSkipped_RowsResolveNewTheme" => new RealizationIsolationFixtures.AncestorThemeToggledWhileSkipped_RowsResolveNewTheme(harness), "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme" => new RealizationIsolationFixtures.RowsRealizedDuringThemeChangingPass_UsePendingTheme(harness), "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent" => new RealizationIsolationFixtures.NestedHostMountedDuringStaleReplay_IsNotCurrent(harness), + "Iso_ReusedRowDropsOwnProvider_NestedHostRefreshes" => new RealizationIsolationFixtures.ReusedRowDropsOwnProvider_NestedHostRefreshes(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index e93e01e94..7c501efd2 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -275,6 +275,44 @@ public void Capture_TakenDuringReplay_RecordsTheReplayedGeneration() Assert.True(nested.IsStale(scope)); } + [Fact] + public void ProviderChangeDuringReplay_InvalidatesSnapshotsTakenUnderIt() + { + var scope = new ContextScope(); + var outer = Captured(scope); // current, empty + + var previous = scope.Replace(outer.Entries, outer.Generation); + Push(scope, Theme, "row-local"); // a reused row's own provider, still on + var nested = Captured(scope); + Assert.False(nested.IsStale(scope)); + + scope.Pop(1); // the reused row drops its .Provide + scope.BumpProviderGeneration(); + Assert.True(nested.IsStale(scope)); // shorter stack is a prefix; generation catches it + var refreshed = Captured(scope); + scope.Restore(previous); + + // The replay was current, so a snapshot refreshed after the change is current live. + Assert.False(refreshed.IsStale(scope)); + } + + [Fact] + public void ProviderChangeDuringStaleReplay_NeverYieldsACurrentSnapshot() + { + var scope = new ContextScope(); + Push(scope, Theme, "removed"); + var outer = Captured(scope); + scope.Pop(1); + scope.BumpProviderGeneration(); // outer snapshot is now stale + + var previous = scope.Replace(outer.Entries, outer.Generation); + scope.BumpProviderGeneration(); // a provider change inside the replayed row + var nested = Captured(scope); + scope.Restore(previous); + + Assert.True(nested.IsStale(scope)); // still holds the removed provider + } + [Fact] public void Capture_Refresh_KeepsArrayWhenNothingChanged() { From 97948fe7af17b664d247c3c5ef77d68ec901f369 Mon Sep 17 00:00:00 2001 From: Alexandre Zollinger Chohfi Date: Mon, 19 Oct 2026 13:46:56 -0700 Subject: [PATCH 10/10] Address review round 8 - Every replay of an outdated host snapshot gets a fresh generation token that no live generation or other replay can equal, so snapshots taken from it are never treated as current - including across repeated replays of the same outdated snapshot (parked grandchild after a second reuse). - Track RequestedTheme changes like provider changes: a non-skip Update whose own RequestedTheme changed bumps a theme generation and keeps the context skip check active below it. A virtualized host whose recorded theme generation is behind declines the skip once, so its Update records the pending subtree theme before a later sibling forces layout in the same pass. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: d6963d93-8fc8-4956-8c83-88b394debeee --- src/Reactor/Core/ContextScope.cs | 17 ++++- src/Reactor/Core/Reconciler.Update.cs | 6 ++ src/Reactor/Core/Reconciler.cs | 35 +++++++-- .../Fixtures/RealizationIsolationFixtures.cs | 74 +++++++++++++++++++ .../SelfTest/SelfTestFixtureRegistry.cs | 2 + .../Reactor.Tests/ContextCaptureUnitTests.cs | 34 ++++++++- 6 files changed, 156 insertions(+), 12 deletions(-) diff --git a/src/Reactor/Core/ContextScope.cs b/src/Reactor/Core/ContextScope.cs index 6e4b2ad2f..480d5319f 100644 --- a/src/Reactor/Core/ContextScope.cs +++ b/src/Reactor/Core/ContextScope.cs @@ -157,7 +157,10 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) /// /// Replaces the live stack with , recording the provider /// generation it was taken at (see ), and returns the - /// displaced stack for . + /// displaced stack for . Replaying an outdated snapshot gets a + /// fresh token that no live generation and no other replay can ever equal, so a + /// snapshot taken from it is never treated as current — not after the replay, and not + /// by a later replay of the same outdated snapshot. /// /// The provider entries to replay. /// The was taken at. @@ -165,7 +168,7 @@ internal bool IsPrefixOf((ContextBase Context, object? Value)[] snapshot) { var previous = _stack; _stack = new List<(ContextBase Context, object? Value)>(snapshot); - _replayGenerations.Add(generation); + _replayGenerations.Add(generation == ProviderGeneration ? generation : _staleReplaySeq--); _replayDepth++; _version++; return previous; @@ -222,10 +225,18 @@ internal void Refresh(ContextScope scope) /// internal ElementTheme PassTheme { get; private set; } - internal void NotePass(long passId, ElementTheme theme) + /// + /// The reconciler's theme generation when the host was last reached. An ancestor's + /// RequestedTheme change advances it, so a skipped host is detected as needing + /// to re-record its subtree theme (see ). + /// + internal long ThemeGeneration { get; private set; } = long.MinValue; + + internal void NotePass(long passId, ElementTheme theme, long themeGeneration) { PassId = passId; PassTheme = theme; + ThemeGeneration = themeGeneration; } /// diff --git a/src/Reactor/Core/Reconciler.Update.cs b/src/Reactor/Core/Reconciler.Update.cs index 6044c4ddb..83f535f48 100644 --- a/src/Reactor/Core/Reconciler.Update.cs +++ b/src/Reactor/Core/Reconciler.Update.cs @@ -182,6 +182,10 @@ public sealed partial class Reconciler bool providerChanged = !Element.ContextValuesEqual(oldEl.ContextValues, ctxValues); if (providerChanged) EnterProviderChange(); + bool themeChanged = (oldModifiers?.RequestedTheme ?? ElementTheme.Default) + != (modifiers?.RequestedTheme ?? ElementTheme.Default); + if (themeChanged) + EnterThemeChange(); // Virtualized hosts realize rows outside this traversal; keep their provider // snapshot current (no-op for every other control, and for a decorator that @@ -368,6 +372,8 @@ public sealed partial class Reconciler finally { _reconcilingElement = prevReconcilingElement; + if (themeChanged) + ExitThemeChange(); if (providerChanged) ExitProviderChange(); if (ctxCount > 0) diff --git a/src/Reactor/Core/Reconciler.cs b/src/Reactor/Core/Reconciler.cs index 29160cdd7..4e7e62c0a 100644 --- a/src/Reactor/Core/Reconciler.cs +++ b/src/Reactor/Core/Reconciler.cs @@ -1918,7 +1918,7 @@ internal IDisposable PushContextDisposable(Context context, T value) // without exposing the traversal stack itself. internal T ReadContext(Context context) => _contextScope.Read(context); - internal bool HasActiveContextValues => _contextScope.HasActiveValues || _providerChangeDepth > 0; + internal bool HasActiveContextValues => _contextScope.HasActiveValues || _providerChangeDepth > 0 || _themeChangeDepth > 0; // Depth of non-skip Updates, on the current traversal path, whose element's own // ContextValues differ from the previous render's (a provider added, removed or @@ -1936,6 +1936,22 @@ internal void EnterProviderChange() internal void ExitProviderChange() => _providerChangeDepth--; + // Same idea for RequestedTheme: depth of non-skip Updates on the current path whose + // own RequestedTheme changed, plus a generation bumped alongside. A virtualized host + // records the generation when it is reached; a host skipped below a theme change + // would keep the previous pass's subtree theme, so the skip check declines it once + // and its Update re-records the pending theme for rows realized later in the pass. + private int _themeChangeDepth; + private long _themeGeneration; + + internal void EnterThemeChange() + { + _themeChangeDepth++; + _themeGeneration++; + } + + internal void ExitThemeChange() => _themeChangeDepth--; + /// /// The host that owns this reconciler, or null for a standalone reconciler or a /// ReactorHostControl. Window-scoped hooks resolve "the rendering host" through @@ -1981,7 +1997,7 @@ internal ContextCapture CaptureContextForHost(FrameworkElement host) var state = GetOrCreateReactorState(host); var capture = state.ContextCapture ??= new ContextCapture { OwnerType = _reconcilingElement?.GetType() }; capture.Refresh(_contextScope); - capture.NotePass(_passId, _ambientRequestedTheme); + capture.NotePass(_passId, _ambientRequestedTheme, _themeGeneration); _hasContextCaptures = true; return capture; } @@ -2002,7 +2018,7 @@ internal void RefreshContextCapture(UIElement control, Element newEl, ElementThe if (TryGetContextCapture(control) is { } capture && capture.IsOwnedBy(newEl)) { capture.Refresh(_contextScope); - capture.NotePass(_passId, effectiveTheme); + capture.NotePass(_passId, effectiveTheme, _themeGeneration); } } @@ -2017,7 +2033,7 @@ internal void RefreshContextCapture(UIElement control, Element newEl, ElementThe internal void RefreshContextCaptureOnSkip(UIElement control, Element newEl, ElementTheme effectiveTheme) { if (TryGetContextCapture(control) is not { } capture || !capture.IsOwnedBy(newEl)) return; - capture.NotePass(_passId, effectiveTheme); + capture.NotePass(_passId, effectiveTheme, _themeGeneration); var own = newEl.ContextValues; if (own is not { Count: > 0 }) { @@ -2037,14 +2053,14 @@ internal ContextCapture CaptureLiveContext() { var capture = new ContextCapture(); capture.Refresh(_contextScope); - capture.NotePass(_passId, _ambientRequestedTheme); + capture.NotePass(_passId, _ambientRequestedTheme, _themeGeneration); return capture; } internal void RefreshLiveContext(ContextCapture capture) { capture.Refresh(_contextScope); - capture.NotePass(_passId, _ambientRequestedTheme); + capture.NotePass(_passId, _ambientRequestedTheme, _themeGeneration); } /// @@ -2113,6 +2129,7 @@ internal void AttachContextCapture(IElementFactory factory, FrameworkElement hos private readonly List? _previousRoots; private readonly int _previousBoundaryDepth; private readonly int _previousProviderChangeDepth; + private readonly int _previousThemeChangeDepth; private readonly Element? _previousReconcilingElement; private readonly StaggerScope? _previousStagger; private readonly bool _swapHost; @@ -2126,6 +2143,7 @@ internal RealizationFrame(Reconciler owner, ContextCapture? capture, ElementThem _previousRoots = owner._boundaryMountRoots; _previousBoundaryDepth = owner._errorBoundaryDepth; _previousProviderChangeDepth = owner._providerChangeDepth; + _previousThemeChangeDepth = owner._themeChangeDepth; _previousReconcilingElement = owner._reconcilingElement; _previousStagger = _staggerScope; @@ -2133,6 +2151,7 @@ internal RealizationFrame(Reconciler owner, ContextCapture? capture, ElementThem owner._boundaryMountRoots = null; owner._errorBoundaryDepth = 0; owner._providerChangeDepth = 0; + owner._themeChangeDepth = 0; owner._reconcilingElement = null; owner._ambientRequestedTheme = ambientTheme; _staggerScope = null; @@ -2158,6 +2177,7 @@ public void Dispose() owner._boundaryMountRoots = _previousRoots; owner._errorBoundaryDepth = _previousBoundaryDepth; owner._providerChangeDepth = _previousProviderChangeDepth; + owner._themeChangeDepth = _previousThemeChangeDepth; owner._reconcilingElement = _previousReconcilingElement; _staggerScope = _previousStagger; } @@ -3023,7 +3043,8 @@ internal bool HasConsumedContextChangedInSubtree(UIElement control) // A virtualized host has no realized consumer for rows it has yet to realize, // so its own snapshot stands in for them: if the providers above it changed, // decline the skip so the Update path re-snapshots before the next realization. - if (TryGetContextCapture(control) is { } capture && capture.IsStale(_contextScope)) + if (TryGetContextCapture(control) is { } capture + && (capture.IsStale(_contextScope) || capture.ThemeGeneration != _themeGeneration)) return true; bool found = false; diff --git a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs index 1a28a52d1..4e2defc50 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/Fixtures/RealizationIsolationFixtures.cs @@ -489,6 +489,80 @@ public override async Task RunAsync() } } + // ── Stable list below an ancestor theme change, realized in the same pass ─ + + // The list element is reference-stable (child-level skip) and its items do not + // change; a later sibling's effect scrolls it and forces layout during the same + // update that flips the ancestor's RequestedTheme. Newly realized rows must resolve + // against the new theme, not the host's still-old ActualTheme. + internal sealed class StableListRealizedDuringThemeChangingPass_UsesPendingTheme(Harness h) : SelfTestFixtureBase(h) + { + private const string SrcKey = "IsoStablePassSrcBrush"; + private const string TargetKey = "IsoStablePassTargetBrush"; + + public override async Task RunAsync() + { + var light = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 0, 0, 180)); + var dark = new SolidColorBrush(global::Windows.UI.Color.FromArgb(255, 180, 0, 0)); + var dict = new ResourceDictionary(); + dict.ThemeDictionaries["Light"] = new ResourceDictionary { [SrcKey] = light }; + dict.ThemeDictionaries["Dark"] = new ResourceDictionary { [SrcKey] = dark }; + Application.Current.Resources.MergedDictionaries.Add(dict); + ThemeRef.InvalidateResolutionCache(); + s_forceLayout = null; + try + { + bool appIsDark = Application.Current.RequestedTheme == ApplicationTheme.Dark; + var first = appIsDark ? ElementTheme.Light : ElementTheme.Dark; + var second = appIsDark ? ElementTheme.Dark : ElementTheme.Light; + SolidColorBrush BrushFor(ElementTheme t) => t == ElementTheme.Dark ? dark : light; + + var stableList = LazyVStack(Enumerable.Range(0, 300).ToList(), i => i.ToString(), (i, _) => + TextBlock($"sp{i}").Height(30).Resources(r => r.Set(TargetKey, Theme.Ref(SrcKey)))) + .Height(300); + + var host = H.CreateHost(); + host.Mount(ctx => + { + var (phase, setPhase) = ctx.UseState(0); + return VStack( + Button("IsoStablePassGo", () => setPhase(1)), + VStack(stableList, Component(new IndexProps(phase))) + .RequestedTheme(phase == 0 ? first : second)); + }); + await Harness.Render(); + + int realizedInPass = 0; + s_forceLayout = () => + { + var sv = H.FindControl(s => s.Content is ItemsRepeater)!; + sv.ChangeView(null, 150 * 30, null, disableAnimation: true); + sv.UpdateLayout(); + (sv.XamlRoot?.Content as UIElement)?.UpdateLayout(); + realizedInPass = H.FindAllControls(t => t.Text?.StartsWith("sp1", StringComparison.Ordinal) == true + && int.TryParse(t.Text.AsSpan(2), out var n) && n >= 100).Count; + }; + + H.ClickButton("IsoStablePassGo"); + await Harness.Render(); + s_forceLayout = null; + + var rows = H.FindAllControls(t => t.Text?.StartsWith("sp1", StringComparison.Ordinal) == true + && int.TryParse(t.Text.AsSpan(2), out var n) && n >= 100) + .ToList(); + int right = rows.Count(t => t.Resources.TryGetValue(TargetKey, out var b) && ReferenceEquals(b, BrushFor(second))); + H.Check("Iso_StablePass_RowsRealizedInPass", realizedInPass > 0, $"inPass={realizedInPass}"); + H.Check("Iso_StablePass_RowsUseNewTheme", rows.Count > 0 && right == rows.Count, $"right={right}/{rows.Count}"); + } + finally + { + s_forceLayout = null; + Application.Current.Resources.MergedDictionaries.Remove(dict); + ThemeRef.InvalidateResolutionCache(); + } + } + } + // ── Ancestor theme changes while the host is skipped ───────────────────── // No Context providers anywhere: the provider checks are not even consulted. The diff --git a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs index 4704c645c..060cb3fae 100644 --- a/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs +++ b/tests/Reactor.AppTests.Host/SelfTest/SelfTestFixtureRegistry.cs @@ -634,6 +634,7 @@ internal static class SelfTestFixtureRegistry "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme", "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent", "Iso_ReusedRowDropsOwnProvider_NestedHostRefreshes", + "Iso_StableListRealizedDuringThemeChangingPass_UsesPendingTheme", "VirtualizedCtx_GridView_RowsSeeProvider", "VirtualizedCtx_UntypedListView_RowsSeeProvider", "VirtualizedCtx_UntypedGridView_RowsSeeProvider", @@ -2708,6 +2709,7 @@ internal static string[] StaleTierDeclarations() => "Iso_RowsRealizedDuringThemeChangingPass_UsePendingTheme" => new RealizationIsolationFixtures.RowsRealizedDuringThemeChangingPass_UsePendingTheme(harness), "Iso_NestedHostMountedDuringStaleReplay_IsNotCurrent" => new RealizationIsolationFixtures.NestedHostMountedDuringStaleReplay_IsNotCurrent(harness), "Iso_ReusedRowDropsOwnProvider_NestedHostRefreshes" => new RealizationIsolationFixtures.ReusedRowDropsOwnProvider_NestedHostRefreshes(harness), + "Iso_StableListRealizedDuringThemeChangingPass_UsesPendingTheme" => new RealizationIsolationFixtures.StableListRealizedDuringThemeChangingPass_UsesPendingTheme(harness), "VirtualizedCtx_GridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.GridView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedListView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedListView_RowsSeeProvider(harness), "VirtualizedCtx_UntypedGridView_RowsSeeProvider" => new VirtualizedRowContextFixtures.UntypedGridView_RowsSeeProvider(harness), diff --git a/tests/Reactor.Tests/ContextCaptureUnitTests.cs b/tests/Reactor.Tests/ContextCaptureUnitTests.cs index 7c501efd2..d5cb2b708 100644 --- a/tests/Reactor.Tests/ContextCaptureUnitTests.cs +++ b/tests/Reactor.Tests/ContextCaptureUnitTests.cs @@ -255,7 +255,7 @@ public void Capture_IsStale_GenerationOutlivesTheChangingPass() } [Fact] - public void Capture_TakenDuringReplay_RecordsTheReplayedGeneration() + public void Capture_TakenDuringStaleReplay_IsNeverCurrent() { // Outer snapshot taken under a provider, then the provider is removed. var scope = new ContextScope(); @@ -267,12 +267,42 @@ public void Capture_TakenDuringReplay_RecordsTheReplayedGeneration() // A row realized from the (now outdated) outer snapshot mounts a nested host. var previous = scope.Replace(outer.Entries, outer.Generation); var nested = Captured(scope); - Assert.Equal(outer.Generation, nested.Generation); + Assert.NotEqual(scope.ProviderGeneration, nested.Generation); Assert.False(nested.IsStale(scope)); // consistent with the replayed stack scope.Restore(previous); // Back in the live traversal the nested snapshot must not look current. Assert.True(nested.IsStale(scope)); + + // Nor in a later replay of the same outdated snapshot. + previous = scope.Replace(outer.Entries, outer.Generation); + Assert.True(nested.IsStale(scope)); + scope.Restore(previous); + } + + [Fact] + public void RepeatedReuse_ParkedGrandchildStaysStale() + { + // Copilot round 8: the outer factory keeps its generation-0 snapshot while a + // reused row drops its own .Provide; a parked grandchild list still holds the + // removed provider. A SECOND reuse must not re-stamp the nested list with gen 0. + var scope = new ContextScope(); + var outer = Captured(scope); // gen 0, empty, never refreshed + + var r1 = scope.Replace(outer.Entries, outer.Generation); + Push(scope, Theme, "row-local"); + var grandchild = Captured(scope); // [row-local] @ gen 0 + scope.Pop(1); + scope.BumpProviderGeneration(); // first reuse drops .Provide + var inner = Captured(scope); // empty @ live gen + scope.Restore(r1); + + var r2 = scope.Replace(outer.Entries, outer.Generation); // outer is now stale + inner.Refresh(scope); // inner refreshed under that replay + var r3 = scope.Replace(inner.Entries, inner.Generation); // inner realizes the grandchild + Assert.True(grandchild.IsStale(scope)); + scope.Restore(r3); + scope.Restore(r2); } [Fact]