Browse Source

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
pull/3755/head
Siegfried Pammer 2 months ago
parent
commit
137d922256
  1. 71
      ILSpy.Tests/Docking/LayoutPersistenceTests.cs
  2. 14
      ILSpy/Docking/ILSpyDockFactory.cs
  3. 32
      ILSpy/Docking/ILSpyDockJson.cs

71
ILSpy.Tests/Docking/LayoutPersistenceTests.cs

@ -168,6 +168,77 @@ public class LayoutPersistenceTests @@ -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<MainWindow>();
window.Show();
var vm = (MainWindowViewModel)window.DataContext!;
await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1);
var dockWorkspace = AppComposition.Current.GetExport<DockWorkspace>();
// 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<ContentTabPage>().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<global::ILSpy.Commands.ToolPaneRegistry>();
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<ContentTabPage>().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()
{

14
ILSpy/Docking/ILSpyDockFactory.cs

@ -186,6 +186,20 @@ namespace ILSpy.Docking @@ -186,6 +186,20 @@ namespace ILSpy.Docking
MainTab = Documents?.VisibleDockables?
.OfType<ContentTabPage>()
.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<IDockable>(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

32
ILSpy/Docking/ILSpyDockJson.cs

@ -178,12 +178,20 @@ namespace ILSpy.Docking @@ -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 @@ -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<T> / ValueTask / ValueTask<T>: TaskCompletionSource's internal

Loading…
Cancel
Save