Skip to content

Commit

Permalink
Fix session being persisted even when disabled (microsoft#17211)
Browse files Browse the repository at this point in the history
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 microsoft#17206
Closes microsoft#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 ✅
  • Loading branch information
lhecker authored May 8, 2024
1 parent 0b76c51 commit dbac3a1
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 7 deletions.
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ namespace winrt::TerminalApp::implementation
AppLogic::Current()->NotifyRootInitialized();
}

void TerminalWindow::Quit()
void TerminalWindow::PersistState()
{
if (_root)
{
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ namespace winrt::TerminalApp::implementation

void Create();

void Quit();
void PersistState();

winrt::fire_and_forget UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args);

Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/TerminalWindow.idl
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ namespace TerminalApp
Boolean ShouldImmediatelyHandoffToElevated();
void HandoffToElevated();

void Quit();
void PersistState();

Windows.UI.Xaml.UIElement GetRoot();

Expand Down
9 changes: 6 additions & 3 deletions src/cascadia/WindowsTerminal/AppHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
10 changes: 9 additions & 1 deletion src/cascadia/WindowsTerminal/WindowEmperor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/WindowsTerminal/WindowEmperor.h
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ class WindowEmperor : public std::enable_shared_from_this<WindowEmperor>

std::unique_ptr<NotificationIcon> _notificationIcon;

bool _requiresPersistenceCleanupOnExit = false;
bool _quitting{ false };

void _windowStartedHandlerPostXAML(const std::shared_ptr<WindowThread>& sender);
Expand Down

0 comments on commit dbac3a1

Please sign in to comment.