Let the viewer close when Windows is shutting down (#778)

OnFormClosing cancelled every close, whatever the CloseReason. For a close the
user asked for that is right: whether closing means hide or exit is
ViewerProgram's rule and it needs a tray check to decide, so the form defers and
CloseForReal brings the answer back.

For the close Windows sends when the session is ending it is not. WinForms
answers WM_QUERYENDSESSION with !e.Cancel, so a viewer that was open at shutdown
told Windows it was preventing one, and the user got the "DiffEngineViewer is
preventing shutdown" screen. With a tray running it was worse than a prompt: the
loop's answer to a close request is to hide the window, so the process stayed up
and went on blocking until the user chose "Shut down anyway".

Do not cancel when the reason is WindowsShutDown or TaskManagerClosing. Both
mean the process is going away regardless, so refusing buys nothing. Letting the
close through is safe: the loop already watches for a disposed form and returns,
running the same shutdown it would have run anyway.

Author: SimonCropp

src/DiffEngineViewer.Windows.Tests/GlobalUsings.cs

@@ -1,2 +1,2 @@
 global using System.Buffers.Binary;
-global using System.IO.Compression;
+global using System.IO.Compression;global using System.Reflection;

src/DiffEngineViewer.Windows.Tests/ViewerFormClosingTests.cs

@@ -0,0 +1,53 @@
+/// <summary>
+/// Who gets to refuse a close.
+/// <para>
+/// The form cancels a user close because whether closing means hide or exit is ViewerProgram's
+/// rule, not the form's. It used to cancel every close, including the one Windows sends when the
+/// session is ending — and WinForms answers WM_QUERYENDSESSION with !e.Cancel, so the viewer
+/// reported itself as preventing shutdown.
+/// </para>
+/// <para>
+/// Driven through OnFormClosing by reflection, because CloseReason is set by the message that
+/// started the close and there is no way to ask a form to close as though Windows had.
+/// </para>
+/// </summary>
+[NotInParallel]
+[TUnit.Core.Executors.STAThreadExecutor]
+public class ViewerFormClosingTests
+{
+    [Test]
+    [Arguments(CloseReason.UserClosing, true)]
+    [Arguments(CloseReason.None, true)]
+    [Arguments(CloseReason.WindowsShutDown, false)]
+    [Arguments(CloseReason.TaskManagerClosing, false)]
+    public async Task Cancels(CloseReason reason, bool expected)
+    {
+        using var form = new ViewerForm("title", 800, 600);
+        var args = new FormClosingEventArgs(reason, false);
+
+        Raise(form, args);
+
+        await Assert.That(args.Cancel).IsEqualTo(expected);
+    }
+
+    /// <summary>
+    /// And CloseForReal still wins, whatever the reason, since that is the loop answering its own
+    /// question.
+    /// </summary>
+    [Test]
+    public async Task CloseForRealIsNeverCancelled()
+    {
+        using var form = new ViewerForm("title", 800, 600);
+        form.CloseForReal();
+        var args = new FormClosingEventArgs(CloseReason.UserClosing, false);
+
+        Raise(form, args);
+
+        await Assert.That(args.Cancel).IsFalse();
+    }
+
+    static void Raise(ViewerForm form, FormClosingEventArgs args) =>
+        typeof(ViewerForm)
+            .GetMethod("OnFormClosing", BindingFlags.Instance | BindingFlags.NonPublic)!
+            .Invoke(form, [args]);
+}

src/DiffEngineViewer.Windows/ViewerForm.cs

@@ -312,9 +312,16 @@ public void CloseForReal()
 
     protected override void OnFormClosing(FormClosingEventArgs e)
     {
-        // Always cancelled, because whether closing means hide or exit is ViewerProgram's rule and
-        // it needs a tray check to decide. CloseForReal is how the answer comes back.
-        if (!closingForReal)
+        // Cancelled for a close the user asked for, because whether that means hide or exit is
+        // ViewerProgram's rule and it needs a tray check to decide. CloseForReal is how the answer
+        // comes back.
+        //
+        // Never for a close the session is ending: WinForms answers WM_QUERYENDSESSION with
+        // !e.Cancel, so refusing made Windows report the viewer as preventing shutdown, and with a
+        // tray running the loop only hid the window - leaving the process blocking until the user
+        // chose "Shut down anyway". Letting it through is safe because the loop watches for a
+        // disposed form and returns, which runs the same shutdown it would have run anyway.
+        if (!closingForReal && !EndsTheSession(e.CloseReason))
         {
             closeRequested = true;
             e.Cancel = true;
@@ -323,6 +330,13 @@ protected override void OnFormClosing(FormClosingEventArgs e)
         base.OnFormClosing(e);
     }
 
+    /// <summary>
+    /// The process is going away whatever this form says. Task Manager's End Task is here with
+    /// shutdown because refusing it buys the same nothing: the user has already decided.
+    /// </summary>
+    internal static bool EndsTheSession(CloseReason reason) =>
+        reason is CloseReason.WindowsShutDown or CloseReason.TaskManagerClosing;
+
     /// <summary>
     /// ProcessCmdKey rather than OnKeyDown, because Tab and Escape are consumed by focus
     /// navigation and the default button before a key handler would ever see them.