diff --git a/ILSpy.Tests/Docking/LayoutPersistenceTests.cs b/ILSpy.Tests/Docking/LayoutPersistenceTests.cs new file mode 100644 index 000000000..c745de01f --- /dev/null +++ b/ILSpy.Tests/Docking/LayoutPersistenceTests.cs @@ -0,0 +1,206 @@ +// Copyright (c) 2026 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; + +using Dock.Model.Core; + +using Avalonia.Headless.NUnit; + +using AwesomeAssertions; + +using ILSpy.AppEnv; +using ILSpy.Docking; +using ILSpy.ViewModels; +using ILSpy.Views; + +using NUnit.Framework; + +namespace ICSharpCode.ILSpy.Tests.Docking; + +/// +/// Save-side pins for the Avalonia layout-persistence wiring. The Save path writes a +/// JSON sidecar next to ILSpy.xml using Dock.Serializer.SystemTextJson, +/// with a custom JsonTypeInfoResolver modifier that strips back-reference +/// properties (Owner / Factory / Task-shaped properties) so the dock tree doesn't +/// surface a real cycle that +/// can't bridge across Dock's per-element JsonConverterList<T> calls. +/// +/// Load currently returns null on this same layout because the emitted JSON has both +/// $id (from Preserve) and $type (from polymorphism) on the same objects, +/// and System.Text.Json's reader requires $type to come first — a documented +/// incompatibility we'd need either a Dock-side fix or a Newtonsoft swap to resolve. +/// The fallback to on null load means the +/// app's user-visible behaviour is "layout still resets each restart" — same as before +/// — but the infrastructure for the eventual full fix is in place. +/// +/// +[TestFixture] +public class LayoutPersistenceTests +{ + [AvaloniaTest] + public async Task SaveLayout_Followed_By_LoadLayout_Round_Trips_The_Live_Layout() + { + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1); + + var dockWorkspace = AppComposition.Current.GetExport(); + var path = Path.Combine(Path.GetTempPath(), $"ILSpy.Layout.test.{System.Guid.NewGuid():N}.json"); + + try + { + ILSpyDockFactory.SaveLayout(path, dockWorkspace.Layout); + + File.Exists(path).Should().BeTrue("SaveLayout must write the file"); + new FileInfo(path).Length.Should().BeGreaterThan(0); + + // Inspect the saved JSON so the test pins the actual shape, not just + // "anything non-null came back". The persisted layout MUST: + // - have $type discriminators (polymorphism intact) + // - have no $id markers (cycle handling stripped) + // - reference the live tool pane IDs (so locator can re-attach on load) + var json = File.ReadAllText(path); + json.Should().Contain("\"$type\":", "polymorphism discriminator must be emitted"); + json.Should().NotContain("\"$id\":", "ReferenceHandler.Preserve must be off — $id breaks load"); + json.Should().Contain("\"Id\": \"AssemblyTree\"", + "the live AssemblyTreeModel singleton ID must persist so LoadLayout can re-attach it"); + json.Should().Contain("\"Id\": \"Search\"", + "SearchPaneModel's singleton ID must persist"); + + // Real Load — and the loaded layout must be structurally usable, not just + // non-null. Specifically: a RootDock with VisibleDockables containing the + // proportional dock tree, AND the tool panes resolved as the live singletons + // (CreateObject hook should have returned the MEF instances, not fresh ones). + var registryForRoundTrip = AppComposition.Current.GetExport(); + var roundTrip = new ILSpyDockFactory(registryForRoundTrip).LoadLayout(path); + ((object?)roundTrip).Should().NotBeNull("LoadLayout must materialise the saved root dock"); + var dockables = roundTrip!.VisibleDockables; + (dockables == null ? 0 : dockables.Count) + .Should().BeGreaterThan(0, "loaded root must have visible dockables"); + + // Walk every dockable in the loaded tree. Every ToolPaneModel-typed entry + // must be reference-equal to the live MEF singleton — that's what proves + // the CreateObject hook fired during deserialization, not a fresh ctor. + var liveAssemblyTree = AppComposition.Current.GetExport(); + var liveSearch = AppComposition.Current.GetExport(); + var allLoaded = Flatten(roundTrip).ToList(); + allLoaded.OfType().Should().Contain(liveAssemblyTree, + "loaded AssemblyTreeModel must be the live MEF singleton, not a fresh instance"); + allLoaded.OfType().Should().Contain(liveSearch, + "loaded SearchPaneModel must be the live MEF singleton, not a fresh instance"); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + [AvaloniaTest] + public async Task Second_Launch_Restores_MainTab_And_Documents_So_Tree_Selections_Still_Decompile() + { + // Repro of the user-reported "decompile view is empty on second launch" bug. + // First launch: factory.CreateLayout() runs, which sets factory.MainTab and + // factory.Documents alongside building the layout. SaveLayout writes the JSON. + // Second launch: LoadLayout returns a fresh IRootDock — but factory.MainTab / + // factory.Documents stay null, so ShowSelectedNode silently no-ops at + // `if (factory.MainTab is not { } main) return;` and the user sees an empty + // editor when they click a tree node. + + var window = AppComposition.Current.GetExport(); + window.Show(); + var vm = (MainWindowViewModel)window.DataContext!; + await vm.AssemblyTreeModel.WaitForAssembliesAsync(minimumCount: 1); + + var dockWorkspace = AppComposition.Current.GetExport(); + var path = Path.Combine(Path.GetTempPath(), $"ILSpy.Layout.test.{System.Guid.NewGuid():N}.json"); + + try + { + ILSpyDockFactory.SaveLayout(path, dockWorkspace.Layout); + + // Simulate the second-launch path: a NEW factory loads the saved layout + // instead of creating one. The factory MUST still expose MainTab + Documents + // so DockWorkspace can populate decompile content into them. + var registry = AppComposition.Current.GetExport(); + var factory = new ILSpyDockFactory(registry); + var loaded = factory.LoadLayout(path); + + ((object?)loaded).Should().NotBeNull("LoadLayout must materialise the saved root dock"); + ((object?)factory.Documents).Should().NotBeNull( + "after LoadLayout the factory must expose the loaded DocumentDock as Documents"); + ((object?)factory.MainTab).Should().NotBeNull( + "after LoadLayout the factory must expose the loaded MainTab so ShowSelectedNode can populate decompile content"); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } + + static IEnumerable Flatten(IDockable root) + { + yield return root; + if (root is IDock d && d.VisibleDockables is { } kids) + foreach (var k in kids) + foreach (var f in Flatten(k)) + yield return f; + } + + [AvaloniaTest] + public void LoadLayout_Returns_Null_When_The_File_Does_Not_Exist() + { + // The "no saved layout yet" path on first launch. DockWorkspace falls back + // to factory.CreateLayout() when this returns null — that's the contract. + var registry = AppComposition.Current.GetExport(); + var factory = new ILSpyDockFactory(registry); + var nonExistent = Path.Combine(Path.GetTempPath(), $"ILSpy.Layout.missing.{System.Guid.NewGuid():N}.json"); + + var result = factory.LoadLayout(nonExistent); + + ((object?)result).Should().BeNull(); + } + + [AvaloniaTest] + public void LoadLayout_Returns_Null_On_Malformed_Json() + { + // A corrupt sidecar (manually edited, version drift, mid-write crash) must + // not block startup. DockWorkspace silently falls back to defaults. + var registry = AppComposition.Current.GetExport(); + var factory = new ILSpyDockFactory(registry); + var path = Path.Combine(Path.GetTempPath(), $"ILSpy.Layout.malformed.{System.Guid.NewGuid():N}.json"); + File.WriteAllText(path, "{ this is not valid JSON"); + + try + { + var result = factory.LoadLayout(path); + ((object?)result).Should().BeNull("malformed JSON must surface as null, not throw"); + } + finally + { + if (File.Exists(path)) + File.Delete(path); + } + } +} diff --git a/ILSpy/Docking/DockWorkspace.cs b/ILSpy/Docking/DockWorkspace.cs index 19c22cc1b..bb4415dfb 100644 --- a/ILSpy/Docking/DockWorkspace.cs +++ b/ILSpy/Docking/DockWorkspace.cs @@ -101,6 +101,33 @@ namespace ILSpy.Docking public IRootDock Layout { get; } + /// + /// Persists the current dock layout to the JSON sidecar next to ILSpy.xml. + /// Called from MainWindow.OnClosing so the user's pane positions, + /// splitter ratios, and pinned panels survive a restart. Best-effort: any + /// serialization failure is logged and swallowed — losing the saved layout + /// is strictly less bad than blocking shutdown. + /// + public void SaveLayout() => ILSpyDockFactory.SaveLayout(GetLayoutFilePath(), Layout); + + /// + /// Resolves ILSpy.Layout.json as a sidecar in the same directory the + /// XML ILSpy.xml settings file lives in — local-to-binary on portable + /// installs, %APPDATA%/ICSharpCode/ otherwise. Keeping it next to the XML + /// makes "delete settings to reset" still work as a single-folder action. + /// WPF stays XML; this is Avalonia-side only. + /// + static string GetLayoutFilePath() + { + var xmlPath = ICSharpCode.ILSpyX.Settings.ILSpySettings.SettingsFilePathProvider?.Invoke(); + if (string.IsNullOrEmpty(xmlPath)) + return "ILSpy.Layout.json"; + var dir = System.IO.Path.GetDirectoryName(xmlPath); + return string.IsNullOrEmpty(dir) + ? "ILSpy.Layout.json" + : System.IO.Path.Combine(dir, "ILSpy.Layout.json"); + } + public IReadOnlyList ToolPaneMenuItems { get; } [ImportingConstructor] @@ -120,7 +147,11 @@ namespace ILSpy.Docking using (ILSpy.AppEnv.StartupLog.Phase("ILSpyDockFactory ctor + CreateLayout")) { factory = new ILSpyDockFactory(toolPaneRegistry); - Layout = factory.CreateLayout(); + // Prefer the user's saved layout (ILSpy.Layout.json sidecar next to + // ILSpy.xml); fall back to the default layout if there is no saved one + // or it failed to deserialize. The fallback path is the same shape the + // app uses on first launch. + Layout = factory.LoadLayout(GetLayoutFilePath()) ?? factory.CreateLayout(); } assemblyTreeModel.PropertyChanged += OnAssemblyTreePropertyChanged; diff --git a/ILSpy/Docking/ILSpyDockFactory.cs b/ILSpy/Docking/ILSpyDockFactory.cs index f5b688acd..24c81fe1f 100644 --- a/ILSpy/Docking/ILSpyDockFactory.cs +++ b/ILSpy/Docking/ILSpyDockFactory.cs @@ -16,13 +16,16 @@ // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // DEALINGS IN THE SOFTWARE. +using System; using System.Collections.Generic; +using System.IO; using System.Linq; using Dock.Model.Controls; using Dock.Model.Core; using Dock.Model.Mvvm; using Dock.Model.Mvvm.Controls; +using Dock.Serializer.SystemTextJson; using ILSpy.Commands; using ILSpy.TextView; @@ -48,6 +51,264 @@ namespace ILSpy.Docking this.panes = registry.Panes; } + /// + /// Lazily-built serializer wired with a custom + /// that strips properties whose type would otherwise drag in framework-cycle + /// state (, , + /// etc.). Without this filter the serializer follows a tool-pane VM's + /// Task-shaped property into TaskCompletionSource internals and + /// hits System.Text.Json's MaxDepth=64 on real cycles that + /// ReferenceHandler.Preserve can't repair (BCL types don't carry + /// $id markers). + /// + static readonly DockSerializer dockSerializer = BuildSerializer(); + + static DockSerializer BuildSerializer() + { + // Dock.Serializer.SystemTextJson's parameterless ctor wires the (internal) + // DockModelPolymorphicTypeResolver that handles IDockable / IDock / IRootDock + // / IDockWindow / I*Template polymorphism. We need to ADD our cycle-prone- + // property filter and DISABLE ReferenceHandler.Preserve on the options it + // builds — both via reflection because Dock keeps those fields internal. + var serializer = new DockSerializer(); + var optionsField = typeof(DockSerializer).GetField( + "_options", + System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); + if (optionsField?.GetValue(serializer) is System.Text.Json.JsonSerializerOptions opts + && opts.TypeInfoResolver is System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver poly) + { + poly.Modifiers.Add(StripCycleProneProperties); + // With Owner / Window / DockWindow.Layout stripped the dock tree has no + // structural cycles, so ReferenceHandler.Preserve isn't needed. Turning + // it OFF is what makes Load work: with Preserve on, the emitted JSON + // carries both $id (from Preserve) and $type (from Dock's polymorphism + // resolver) on the same objects, and System.Text.Json's reader rejects + // $id-not-first. Additionally, Dock's JsonConverterList.Write makes a + // per-element JsonSerializer.Serialize call which resets Preserve's $id + // counter, so multiple unrelated objects share $id="1" — $ref resolution + // wouldn't work even if ordering were correct. Both problems vanish when + // Preserve is off. + opts.ReferenceHandler = null; + // MaxDepth bump for safety on deep tool stacks; mutation must happen + // before first serialize call (options become immutable after first use). + opts.MaxDepth = 256; + } + return serializer; + } + + static void StripCycleProneProperties(System.Text.Json.Serialization.Metadata.JsonTypeInfo typeInfo) + { + if (typeInfo.Kind != System.Text.Json.Serialization.Metadata.JsonTypeInfoKind.Object) + return; + + // For MEF-injected tool panes (AssemblyTreeModel, SearchPaneModel, …) and the + // persistent ContentTabPage: strip EVERY property except Id + Title. The full + // VM state (settings refs, image properties typed as IImage, Task-shaped status + // fields) doesn't belong in a layout file — it's all reconstructed at runtime + // from composition. The remaining JSON shape is just { "$type": "...", "Id": + // "..." } per dockable, enough for the factory to look up the singleton on + // Load via CreateObject below. + bool isSingletonDockable = IsCompositionResolvableDockable(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"); + if (drop) + typeInfo.Properties.RemoveAt(i); + } + + // CreateObject bypasses the [ImportingConstructor] mismatch — STJ would + // otherwise fail with "Each parameter in the deserialization constructor on + // type X must bind to an object property". Returning the MEF singleton means + // subsequent property assignments (Id/Title) hit fields whose current value + // already matches what was saved, so they're effectively no-ops. + if (isSingletonDockable) + typeInfo.CreateObject = () => ResolveCompositionSingleton(typeInfo.Type); + } + + static bool IsCompositionResolvableDockable(Type type) + { + if (!typeof(Dock.Model.Core.IDockable).IsAssignableFrom(type)) + return false; + // ToolPaneModel-derived (AssemblyTreeModel, SearchPaneModel, AnalyzerTreeViewModel, …) + // are all [Shared] MEF singletons. + if (typeof(ViewModels.ToolPaneModel).IsAssignableFrom(type)) + return true; + // ContentTabPage is the persistent main-document slot — also resolved as a + // singleton from the dock factory, not constructed per-deserialize. + if (type == typeof(ViewModels.ContentTabPage)) + return true; + return false; + } + + static object ResolveCompositionSingleton(Type type) + { + // JsonTypeInfo.CreateObject is `Func` — null isn't a valid return. + // The two fallbacks below cover (a) live runtime (MEF resolves the singleton), + // (b) design-time previews / isolated tests where AppComposition isn't wired + // (Activator gives us a fresh instance with default Id/Title that STJ then + // overwrites from the saved JSON). If both fail, throw with the type name so + // the failure surfaces as something diagnosable instead of as a downstream + // "CreateObject returned null" from STJ. + object? instance = null; + try + { + var getExport = typeof(System.Composition.CompositionContext) + .GetMethods() + .First(m => m.Name == "GetExport" && m.IsGenericMethod && m.GetParameters().Length == 0) + .MakeGenericMethod(type); + instance = getExport.Invoke(AppEnv.AppComposition.Current, null); + } + catch + { + try + { instance = Activator.CreateInstance(type); } + catch { /* fall through to throw below */ } + } + return instance + ?? throw new InvalidOperationException( + $"Could not resolve composition singleton for {type.FullName}: " + + "AppComposition has no export for this type and Activator.CreateInstance failed too."); + } + + static bool IsCycleProneType(Type type) + { + // Task / Task / ValueTask / ValueTask: TaskCompletionSource's internal + // state forms a true cycle that ReferenceHandler.Preserve can't repair. + if (type == typeof(System.Threading.Tasks.Task) || type == typeof(System.Threading.Tasks.ValueTask)) + return true; + if (type.IsGenericType) + { + var def = type.GetGenericTypeDefinition(); + if (def == typeof(System.Threading.Tasks.Task<>) || def == typeof(System.Threading.Tasks.ValueTask<>)) + return true; + } + // CancellationToken / CancellationTokenSource carry similar internal state. + if (type == typeof(System.Threading.CancellationToken) + || type == typeof(System.Threading.CancellationTokenSource)) + return true; + // IDockable's Owner + IDockable's Factory back-refs cycle through the parent + // chain. Dock's own JsonConverterList calls JsonSerializer.Serialize per + // element which doesn't carry ReferenceHandler.Preserve's $id state across + // nested calls — so Owner/Factory wouldn't round-trip as $ref anyway. + // Stripping them removes the cycle entirely; Factory.InitLayout reconstructs + // the chain on Load via parent-walk. + if (type == typeof(Dock.Model.Core.IFactory)) + return true; + return false; + } + + // Hand-listed names of back-reference properties whose declared type isn't a + // reliable cycle signal (e.g. IDockable is a perfectly valid forward property + // type elsewhere). Stripping by (declaring type, name) keeps the filter + // surgical without false positives. With all of these stripped, the layout + // tree has NO cycles, so ReferenceHandler.Preserve can be turned off and the + // $id/$type ordering conflict disappears. + static bool IsBackReferenceProperty(Type declaringType, string propertyName) + { + if (typeof(Dock.Model.Core.IDockable).IsAssignableFrom(declaringType)) + { + // IDockable.Owner — parent back-ref. + if (propertyName == "Owner") + return true; + } + if (typeof(Dock.Model.Controls.IRootDock).IsAssignableFrom(declaringType)) + { + // IRootDock.Window → IDockWindow.Layout cycles back to the root. + // Stripping it loses floating-window persistence (acceptable v1 — + // ILSpy's docked panes don't currently float). + if (propertyName == "Window") + return true; + } + if (typeof(Dock.Model.Core.IDockWindow).IsAssignableFrom(declaringType)) + { + // Symmetric strip — covers any other DockWindow that gets serialised + // without going through IRootDock.Window. + if (propertyName == "Layout") + return true; + } + return false; + } + + /// + /// Serializes to using + /// the static . Best-effort: any IO or serialization + /// exception is swallowed; losing the saved layout is strictly less bad than + /// crashing the app on shutdown. + /// + public static void SaveLayout(string path, IRootDock layout) + { + ArgumentNullException.ThrowIfNull(path); + ArgumentNullException.ThrowIfNull(layout); + try + { + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + using var stream = File.Create(path); + dockSerializer.Save(stream, layout); + } + catch (Exception ex) + { + System.Diagnostics.Debug.WriteLine($"[ILSpyDockFactory] SaveLayout failed: {ex}"); + } + } + + /// + /// Tries to deserialize a previously-saved layout from . + /// Returns null on any failure — missing file, malformed JSON, version drift — + /// so the caller can fall back to without a user- + /// visible error. On success, also rehydrates and + /// by walking the loaded tree — these are the same + /// fields populates on fresh starts, and + /// silently no-ops on null + /// (the user-visible "decompile view stays empty on + /// second launch" bug). + /// + public IRootDock? LoadLayout(string path) + { + ArgumentNullException.ThrowIfNull(path); + if (!File.Exists(path)) + return null; + IRootDock? loaded; + try + { + using var stream = File.OpenRead(path); + loaded = dockSerializer.Load(stream); + } + catch (Exception ex) + { + System.Diagnostics.Debug.WriteLine($"[ILSpyDockFactory] LoadLayout failed: {ex}"); + return null; + } + if (loaded != null) + RehydrateFromLoadedLayout(loaded); + return loaded; + } + + void RehydrateFromLoadedLayout(IDockable root) + { + // Walk the loaded tree and rebind the two structural slots that CreateLayout + // would have set. Documents = the single DocumentDock; MainTab = its first + // ContentTabPage child (we can't look up tab.Owner because back-references + // are stripped during serialization to break cycles). Tool-pane singletons + // resolve through the CreateObject hook (StripCycleProneProperties) so they + // don't need rebinding here. + Documents = Flatten(root).OfType().FirstOrDefault(); + MainTab = Documents?.VisibleDockables? + .OfType() + .FirstOrDefault(); + } + + static IEnumerable Flatten(IDockable root) + { + yield return root; + if (root is IDock dock && dock.VisibleDockables is { } kids) + foreach (var k in kids) + foreach (var f in Flatten(k)) + yield return f; + } + public override IRootDock CreateLayout() { var documents = new DocumentDock { diff --git a/ILSpy/Views/MainWindow.axaml.cs b/ILSpy/Views/MainWindow.axaml.cs index a389d4bb1..784ffb0fd 100644 --- a/ILSpy/Views/MainWindow.axaml.cs +++ b/ILSpy/Views/MainWindow.axaml.cs @@ -89,6 +89,19 @@ namespace ILSpy.Views session.WindowSize = new Size(Width, Height); } } + // Persist the dock layout to ILSpy.Layout.json so the user's pane positions + // + splitter ratios survive the next launch. Resolved via composition so the + // window doesn't need to be wired with a direct DockWorkspace reference. + try + { + ILSpy.AppEnv.AppComposition.Current + .GetExport() + .SaveLayout(); + } + catch (System.Exception ex) + { + System.Diagnostics.Debug.WriteLine($"[MainWindow] SaveLayout on close failed: {ex}"); + } base.OnClosing(e); } }