From dbac3a1fa3755737be0d761e6e44e86b58e2865e Mon Sep 17 00:00:00 2001
From: Leonard Hecker <lhecker@microsoft.com>
Date: Wed, 8 May 2024 22:52:52 +0200
Subject: [PATCH] Fix session being persisted even when disabled (#17211)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This fixes 2 bugs:
* `PersistState` being called when the window is closed
  (as opposed to closing the tab). The settings check was missing.
* Session cleanup running depending on whether the feature is
  currently enabled as opposed to whether it was enabled on launch.

Closes #17206
Closes #17207

## Validation Steps Performed
* Create a bunch of leftover buffer_*.txt files by running
  the current Dev version off of main
* Build this branch, then open and close a window
* All buffer_*.txt are gone and state.json is cleaned up ✅
---
 src/cascadia/TerminalApp/TerminalWindow.cpp    |  2 +-
 src/cascadia/TerminalApp/TerminalWindow.h      |  2 +-
 src/cascadia/TerminalApp/TerminalWindow.idl    |  2 +-
 src/cascadia/WindowsTerminal/AppHost.cpp       |  9 ++++++---
 src/cascadia/WindowsTerminal/WindowEmperor.cpp | 10 +++++++++-
 src/cascadia/WindowsTerminal/WindowEmperor.h   |  1 +
 6 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp
index 749fd2e842..c04a34affc 100644
--- a/src/cascadia/TerminalApp/TerminalWindow.cpp
+++ b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -263,7 +263,7 @@ namespace winrt::TerminalApp::implementation
         AppLogic::Current()->NotifyRootInitialized();
     }
 
-    void TerminalWindow::Quit()
+    void TerminalWindow::PersistState()
     {
         if (_root)
         {
diff --git a/src/cascadia/TerminalApp/TerminalWindow.h b/src/cascadia/TerminalApp/TerminalWindow.h
index b10c29a8d7..40e94a3920 100644
--- a/src/cascadia/TerminalApp/TerminalWindow.h
+++ b/src/cascadia/TerminalApp/TerminalWindow.h
@@ -73,7 +73,7 @@ namespace winrt::TerminalApp::implementation
 
         void Create();
 
-        void Quit();
+        void PersistState();
 
         winrt::fire_and_forget UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args);
 
diff --git a/src/cascadia/TerminalApp/TerminalWindow.idl b/src/cascadia/TerminalApp/TerminalWindow.idl
index 75c47fce8b..2cf4b31729 100644
--- a/src/cascadia/TerminalApp/TerminalWindow.idl
+++ b/src/cascadia/TerminalApp/TerminalWindow.idl
@@ -61,7 +61,7 @@ namespace TerminalApp
         Boolean ShouldImmediatelyHandoffToElevated();
         void HandoffToElevated();
 
-        void Quit();
+        void PersistState();
 
         Windows.UI.Xaml.UIElement GetRoot();
 
diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
index 94585f4391..63d8de231d 100644
--- a/src/cascadia/WindowsTerminal/AppHost.cpp
+++ b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -1184,13 +1184,16 @@ winrt::fire_and_forget AppHost::_QuitRequested(const winrt::Windows::Foundation:
     co_await wil::resume_foreground(_windowLogic.GetRoot().Dispatcher());
 
     const auto strongThis = weakThis.lock();
-    // GH #16235: If we don't have a window logic, we're already refrigerating, and won't have our _window either.
-    if (!strongThis || _windowLogic == nullptr)
+    if (!strongThis)
     {
         co_return;
     }
 
-    _windowLogic.Quit();
+    if (_appLogic && _windowLogic && _appLogic.ShouldUsePersistedLayout())
+    {
+        _windowLogic.PersistState();
+    }
+
     PostQuitMessage(0);
 }
 
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
index a8f0961e42..1e78e721fd 100644
--- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp
+++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
@@ -348,6 +348,14 @@ void WindowEmperor::_becomeMonarch()
 
     _revokers.WindowCreated = _manager.WindowCreated(winrt::auto_revoke, { this, &WindowEmperor::_numberOfWindowsChanged });
     _revokers.WindowClosed = _manager.WindowClosed(winrt::auto_revoke, { this, &WindowEmperor::_numberOfWindowsChanged });
+
+    // If a previous session of Windows Terminal stored buffer_*.txt files, then we need to clean all those up on exit
+    // that aren't needed anymore, even if the user disabled the ShouldUsePersistedLayout() setting in the meantime.
+    {
+        const auto state = ApplicationState::SharedInstance();
+        const auto layouts = state.PersistedWindowLayouts();
+        _requiresPersistenceCleanupOnExit = layouts && layouts.Size() > 0;
+    }
 }
 
 // sender and args are always nullptr
@@ -486,7 +494,7 @@ void WindowEmperor::_finalizeSessionPersistence() const
     // Ensure to write the state.json before we TerminateProcess()
     state.Flush();
 
-    if (!_app.Logic().ShouldUsePersistedLayout())
+    if (!_requiresPersistenceCleanupOnExit)
     {
         return;
     }
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.h b/src/cascadia/WindowsTerminal/WindowEmperor.h
index a35032e6d5..d4186cbbb0 100644
--- a/src/cascadia/WindowsTerminal/WindowEmperor.h
+++ b/src/cascadia/WindowsTerminal/WindowEmperor.h
@@ -51,6 +51,7 @@ private:
 
     std::unique_ptr<NotificationIcon> _notificationIcon;
 
+    bool _requiresPersistenceCleanupOnExit = false;
     bool _quitting{ false };
 
     void _windowStartedHandlerPostXAML(const std::shared_ptr<WindowThread>& sender);