From 137d922256100e5a8465cfeb1225432b150ba09f Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Thu, 21 May 2026 07:24:20 +0200 Subject: [PATCH] Dock layout persistence drops session-only document state User-reported: opening multiple document tabs and restarting brings them all back broken except the first one. Assisted-by: Claude:claude-opus-4-7:Claude Code --- ILSpy.Tests/Docking/LayoutPersistenceTests.cs | 71 +++++++++++++++++++ ILSpy/Docking/ILSpyDockFactory.cs | 14 ++++ ILSpy/Docking/ILSpyDockJson.cs | 32 ++++++++- 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/ILSpy.Tests/Docking/LayoutPersistenceTests.cs b/ILSpy.Tests/Docking/LayoutPersistenceTests.cs index 82134a8ff..2aef713e4 100644 --- a/ILSpy.Tests/Docking/LayoutPersistenceTests.cs +++ b/ILSpy.Tests/Docking/LayoutPersistenceTests.cs @@ -168,6 +168,77 @@ public class LayoutPersistenceTests yield return f; } + [AvaloniaTest] + public async Task Documents_Beyond_MainTab_Are_Not_Persisted_Across_Save_And_Load() + { + // The user-reported bug: opening multiple document tabs in a session and restarting + // brings them all back "broken except the first one". Reason: every + // ContentTabPage in the DocumentDock gets serialised, but their Content is a + // runtime-only TabPageModel (live decompiler/metadata/compare state, Tasks, + // CancellationTokenSources, IEntity refs) that can't survive a process restart — + // Activator reconstructs the ContentTabPage shell with Content=null, and the + // chrome shows a blank tab. + // + // Correct behaviour: persist the *structural* layout (tool docks, splitter ratios, + // pinned panes) but treat document children as session-only. On load the + // DocumentDock comes back with a single fresh MainTab; user's tree selection + // re-projects through ShowSelectedNode as on first launch. + + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1); + + var dockWorkspace = AppComposition.Current.GetExport(); + + // Open two extra documents on top of the persistent MainTab. Content type is + // irrelevant for this test — the bug is structural, about ContentTabPage shells + // being persisted at all. + dockWorkspace.OpenNewTab(new object()); + dockWorkspace.OpenNewTab(new object()); + var sourceDocs = dockWorkspace.Documents!.VisibleDockables! + .OfType().Count(); + sourceDocs.Should().Be(3, "test must open 3 tabs (MainTab + 2 extras) before saving"); + + var path = Path.Combine(Path.GetTempPath(), $"ILSpy.Layout.test.{System.Guid.NewGuid():N}.json"); + + try + { + ILSpyDockFactory.SaveLayout(path, dockWorkspace.Layout); + + // JSON shape: the saved file must not embed any ContentTabPage entries at all. + // Stripping at the IDocumentDock.VisibleDockables level is the only way to + // guarantee no broken-tab shells survive a restart. + var json = File.ReadAllText(path); + json.Should().NotContain("ContentTabPage", + "document children must be excluded from persistence so broken tab shells " + + "can't come back after a restart"); + + // Round-trip with a fresh factory. The loaded DocumentDock must contain exactly + // one ContentTabPage (the fresh MainTab repopulated by the load path); none of + // the extras may resurrect. + var registry = AppComposition.Current.GetExport(); + var factory = new ILSpyDockFactory(registry); + var loaded = factory.LoadLayout(path); + ((object?)loaded).Should().NotBeNull(); + + factory.Documents.Should().NotBeNull( + "a DocumentDock must still exist post-load so navigation has a target"); + factory.MainTab.Should().NotBeNull( + "a fresh MainTab must be in place post-load so tree selections still decompile"); + var loadedDocs = factory.Documents!.VisibleDockables! + .OfType().Count(); + loadedDocs.Should().Be(1, + "only the freshly-created MainTab must exist; persisted document children " + + "must not survive the round-trip"); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + [AvaloniaTest] public void LoadLayout_Returns_Null_When_The_File_Does_Not_Exist() { diff --git a/ILSpy/Docking/ILSpyDockFactory.cs b/ILSpy/Docking/ILSpyDockFactory.cs index 0162fc97d..042874ed4 100644 --- a/ILSpy/Docking/ILSpyDockFactory.cs +++ b/ILSpy/Docking/ILSpyDockFactory.cs @@ -186,6 +186,20 @@ namespace ILSpy.Docking MainTab = Documents?.VisibleDockables? .OfType() .FirstOrDefault(); + + // Documents aren't persisted (ILSpyDockJson strips IDocumentDock child slots), + // so the loaded Documents.VisibleDockables comes back null/empty. Repopulate + // with a fresh MainTab here so the user lands in the same "(no selection)" + // state as on first launch — DockWorkspace.ShowSelectedNode then re-projects + // the tree selection through this fresh tab as normal. + if (Documents is { } docs && MainTab is null) + { + MainTab = new ContentTabPage { Title = "(no selection)" }; + docs.VisibleDockables ??= CreateList(MainTab); + if (docs.VisibleDockables.Count == 0) + docs.VisibleDockables.Add(MainTab); + docs.ActiveDockable = MainTab; + } } // Matches a deserialised stand-in against the matching member of its dock's diff --git a/ILSpy/Docking/ILSpyDockJson.cs b/ILSpy/Docking/ILSpyDockJson.cs index 26c1b04e3..188983cb4 100644 --- a/ILSpy/Docking/ILSpyDockJson.cs +++ b/ILSpy/Docking/ILSpyDockJson.cs @@ -178,12 +178,20 @@ namespace ILSpy.Docking // "Id": "..." } per dockable, enough for the factory to look up the singleton // on Load via CreateObject below. bool isSingletonDockable = IsCompositionResolvableDockable(typeInfo.Type); + bool isDocumentDock = typeof(IDocumentDock).IsAssignableFrom(typeInfo.Type); + // On IRootDock we strip the per-dock-active-pointer trio too because they + // transitively reference a ContentTabPage (the focused/active document on the + // previous session). Keeping IRootDock.VisibleDockables intact preserves the + // proportional layout tree — only the focus pointers are dropped. + bool isRootDock = typeof(IRootDock).IsAssignableFrom(typeInfo.Type); for (int i = typeInfo.Properties.Count - 1; i >= 0; i--) { var prop = typeInfo.Properties[i]; bool drop = IsCycleProneType(prop.PropertyType) || IsBackReferenceProperty(typeInfo.Type, prop.Name) - || (isSingletonDockable && prop.Name is not "Id" and not "Title"); + || (isSingletonDockable && prop.Name is not "Id" and not "Title") + || (isDocumentDock && IsDocumentChildSlot(prop.Name)) + || (isRootDock && IsActivePointerSlot(prop.Name)); if (drop) typeInfo.Properties.RemoveAt(i); } @@ -242,6 +250,28 @@ namespace ILSpy.Docking + "AppComposition has no export for this type and Activator.CreateInstance failed too."); } + // Document children (decompiler text, metadata grid, compare diff, options page) + // are session-only: their viewmodels hold live IEntity references, decompiler Tasks, + // CancellationTokenSources, and AssemblyLoaded contexts that can't survive a + // process restart. Persisting the ContentTabPage shells through Activator brings + // them back with Content=null, which renders blank tabs — the user-reported + // "documents are broken except the first tab" symptom. Strip every IDocumentDock + // child slot at save-time so only the structural document slot survives; the + // factory repopulates a fresh MainTab on load. + static bool IsDocumentChildSlot(string propertyName) + => propertyName is "VisibleDockables" + or "ActiveDockable" + or "FocusedDockable" + or "DefaultDockable"; + + // The three "currently selected child" pointers on a dock. Used by IRootDock-only + // stripping — the strict-stronger IsDocumentChildSlot above also covers + // VisibleDockables, which we must keep on IRootDock. + static bool IsActivePointerSlot(string propertyName) + => propertyName is "ActiveDockable" + or "FocusedDockable" + or "DefaultDockable"; + static bool IsCycleProneType(Type type) { // Task / Task / ValueTask / ValueTask: TaskCompletionSource's internal