Remove the need for TerminalPage to know the number of open windows

This removes all the weirdness around the way that TerminalPage needs to track the number of open windows. Instead of TerminalPage persisting empty state when the last tab closes, it lets the AppHost know that the last tab was closed due to _closing the tab_ (as opposed to closing the window / quitting). This gives AppHost an opportunity to persist empty state for that case, because _it_ knows how many windows there are. This could basically be its own PR. Probably worth xlinking this commit to #9800
microsoft · Mar 9, 2023 · 434abc2 · 434abc2
1 parent 6dead99
commit 434abc2
Show file tree

Hide file tree

Showing 8 changed files with 48 additions and 44 deletions.
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -523,13 +523,7 @@ namespace winrt::TerminalApp::implementation
             // if the user manually closed all tabs.
             // Do this only if we are the last window; the monarch will notice
             // we are missing and remove us that way otherwise.
-            if (!_maintainStateOnTabClose && _settings.GlobalSettings().ShouldUsePersistedLayout() && _numOpenWindows == 1)
-            {
-                auto state = ApplicationState::SharedInstance();
-                state.PersistedWindowLayouts(nullptr);
-            }
-
-            _LastTabClosedHandlers(*this, nullptr);
+            _LastTabClosedHandlers(*this, winrt::make<LastTabClosedEventArgs>(!_maintainStateOnTabClose));
         }
         else if (focusedTabIndex.has_value() && focusedTabIndex.value() == gsl::narrow_cast<uint32_t>(tabIndex))
         {

diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -5,6 +5,7 @@
 #include "pch.h"
 #include "TerminalPage.h"
 #include "TerminalPage.g.cpp"
+#include "LastTabClosedEventArgs.g.cpp"
 #include "RenameWindowRequestedArgs.g.cpp"
 
 #include <filesystem>
@@ -3808,14 +3809,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    void TerminalPage::SetNumberOfOpenWindows(const uint64_t num)
-    {
-        // This is used in TerminalPage::_RemoveTab, when we close a tab. If we
-        // close the last tab, and there's only one window open, then we will
-        // call to persist _no_ state.
-        _numOpenWindows = num;
-    }
-
     // Method Description:
     // - Called when an attempt to rename the window has failed. This will open
     //   the toast displaying a message to the user that the attempt to rename

diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
@@ -7,6 +7,7 @@
 #include "TerminalTab.h"
 #include "AppKeyBindings.h"
 #include "AppCommandlineArgs.h"
+#include "LastTabClosedEventArgs.g.h"
 #include "RenameWindowRequestedArgs.g.h"
 #include "Toast.h"
 
@@ -41,6 +42,15 @@ namespace winrt::TerminalApp::implementation
         ScrollDown = 1
     };
 
+    struct LastTabClosedEventArgs : LastTabClosedEventArgsT<LastTabClosedEventArgs>
+    {
+        WINRT_PROPERTY(bool, ClearPersistedState);
+
+    public:
+        LastTabClosedEventArgs(const bool& shouldClear) :
+            _ClearPersistedState{ shouldClear } {};
+    };
+
     struct RenameWindowRequestedArgs : RenameWindowRequestedArgsT<RenameWindowRequestedArgs>
     {
         WINRT_PROPERTY(winrt::hstring, ProposedName);
@@ -120,8 +130,6 @@ namespace winrt::TerminalApp::implementation
         winrt::hstring WindowIdForDisplay() const noexcept { return _WindowProperties.WindowIdForDisplay(); };
         winrt::hstring WindowNameForDisplay() const noexcept { return _WindowProperties.WindowNameForDisplay(); };
 
-        void SetNumberOfOpenWindows(const uint64_t value);
-
         bool IsElevated() const noexcept;
 
         void OpenSettingsUI();
@@ -188,7 +196,6 @@ namespace winrt::TerminalApp::implementation
         bool _isAlwaysOnTop{ false };
 
         std::optional<uint32_t> _loadFromPersistedLayoutIdx{};
-        uint64_t _numOpenWindows{ 0 };
 
         bool _maintainStateOnTabClose{ false };
         bool _rearranging{ false };

diff --git a/src/cascadia/TerminalApp/TerminalPage.idl b/src/cascadia/TerminalApp/TerminalPage.idl
@@ -5,7 +5,11 @@ import "IDirectKeyListener.idl";
 
 namespace TerminalApp
 {
-    delegate void LastTabClosedEventArgs();
+
+    [default_interface] runtimeclass LastTabClosedEventArgs
+    {
+        Boolean ClearPersistedState { get; };
+    };
 
     [default_interface] runtimeclass RenameWindowRequestedArgs
     {

diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -882,13 +882,13 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     // Return Value:
     // - <none>
-    void TerminalWindow::CloseWindow(LaunchPosition pos)
+    void TerminalWindow::CloseWindow(LaunchPosition pos, const bool isLastWindow)
     {
         if (_root)
         {
             // If persisted layout is enabled and we are the last window closing
             // we should save our state.
-            if (_settings.GlobalSettings().ShouldUsePersistedLayout() && _numOpenWindows == 1)
+            if (_settings.GlobalSettings().ShouldUsePersistedLayout() && isLastWindow)
             {
                 if (const auto layout = _root->GetWindowLayout())
                 {
@@ -902,6 +902,15 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    void TerminalWindow::ClearPersistedWindowState()
+    {
+        if (_settings.GlobalSettings().ShouldUsePersistedLayout())
+        {
+            auto state = ApplicationState::SharedInstance();
+            state.PersistedWindowLayouts(nullptr);
+        }
+    }
+
     winrt::TerminalApp::TaskbarState TerminalWindow::TaskbarState()
     {
         if (_root)
@@ -1114,15 +1123,6 @@ namespace winrt::TerminalApp::implementation
         return nullptr;
     }
 
-    void TerminalWindow::SetNumberOfOpenWindows(const uint64_t num)
-    {
-        _numOpenWindows = num;
-        if (_root)
-        {
-            _root->SetNumberOfOpenWindows(num);
-        }
-    }
-
     void TerminalWindow::IdentifyWindow()
     {
         if (_root)

diff --git a/src/cascadia/TerminalApp/TerminalWindow.h b/src/cascadia/TerminalApp/TerminalWindow.h
@@ -77,6 +77,7 @@ namespace winrt::TerminalApp::implementation
         void SetPersistedLayoutIdx(const uint32_t idx);
         void SetNumberOfOpenWindows(const uint64_t num);
         bool ShouldUsePersistedLayout() const;
+        void ClearPersistedWindowState();
 
         void RequestExitFullscreen();
 
@@ -95,7 +96,7 @@ namespace winrt::TerminalApp::implementation
         void TitlebarClicked();
         bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);
 
-        void CloseWindow(Microsoft::Terminal::Settings::Model::LaunchPosition position);
+        void CloseWindow(Microsoft::Terminal::Settings::Model::LaunchPosition position, const bool isLastWindow);
         void WindowVisibilityChanged(const bool showOrHide);
 
         winrt::TerminalApp::TaskbarState TaskbarState();
@@ -153,7 +154,6 @@ namespace winrt::TerminalApp::implementation
         winrt::hstring _WindowName{};
         uint64_t _WindowId{ 0 };
 
-        uint64_t _numOpenWindows{ 1 };
         std::optional<uint32_t> _loadFromPersistedLayoutIdx{};
 
         Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };

diff --git a/src/cascadia/TerminalApp/TerminalWindow.idl b/src/cascadia/TerminalApp/TerminalWindow.idl
@@ -77,7 +77,8 @@ namespace TerminalApp
 
         void IdentifyWindow();
         void SetPersistedLayoutIdx(UInt32 idx);
-        void SetNumberOfOpenWindows(UInt64 num);
+        void ClearPersistedWindowState();
+
         void RenameFailed();
         void RequestExitFullscreen();
 
@@ -91,7 +92,7 @@ namespace TerminalApp
         Boolean GetInitialAlwaysOnTop();
         Single CalcSnappedDimension(Boolean widthOrHeight, Single dimension);
         void TitlebarClicked();
-        void CloseWindow(Microsoft.Terminal.Settings.Model.LaunchPosition position);
+        void CloseWindow(Microsoft.Terminal.Settings.Model.LaunchPosition position, Boolean isLastWindow);
         void WindowVisibilityChanged(Boolean showOrHide);
 
         TaskbarState TaskbarState{ get; };

diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -316,7 +316,6 @@ void AppHost::_HandleCommandlineArgs()
                                                            ));
                 }
             }
-            _windowLogic.SetNumberOfOpenWindows(numPeasants);
         }
         _windowLogic.WindowName(peasant.WindowName());
         _windowLogic.WindowId(peasant.GetID());
@@ -505,7 +504,7 @@ void AppHost::AppTitleChanged(const winrt::Windows::Foundation::IInspectable& /*
 // - LastTabClosedEventArgs: unused
 // Return Value:
 // - <none>
-void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::TerminalApp::LastTabClosedEventArgs& /*args*/)
+void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*sender*/, const winrt::TerminalApp::LastTabClosedEventArgs& args)
 {
     if (_windowManager.IsMonarch() && _notificationIcon)
     {
@@ -524,11 +523,22 @@ void AppHost::LastTabClosed(const winrt::Windows::Foundation::IInspectable& /*se
     _windowManager.WindowCreated(_WindowCreatedToken);
     _windowManager.WindowClosed(_WindowClosedToken);
 
+    // If the user closes the last tab, in the last window, _by closing the tab_
+    // (not by closing the whole window), we need to manually persist an empty
+    // window state here. That will cause the terminal to re-open with the usual
+    // settings (not the persisted state)
+    if (args.ClearPersistedState() &&
+        _windowManager.GetNumberOfPeasants() == 1)
+    {
+        _windowLogic.ClearPersistedWindowState();
+    }
+
     // Remove ourself from the list of peasants so that we aren't included in
     // any future requests. This will also mean we block until any existing
     // event handler finishes.
     _windowManager.SignalClose();
 
+
     _window->Close();
 }
 
@@ -973,22 +983,16 @@ void AppHost::_BecomeMonarch(const winrt::Windows::Foundation::IInspectable& /*s
         _CreateNotificationIcon();
     }
 
-    // Set the number of open windows (so we know if we are the last window)
-    // and subscribe for updates if there are any changes to that number.
-    _windowLogic.SetNumberOfOpenWindows(_windowManager.GetNumberOfPeasants());
-
     _WindowCreatedToken = _windowManager.WindowCreated([this](auto&&, auto&&) {
         if (_getWindowLayoutThrottler) {
             _getWindowLayoutThrottler.value()();
-        }
-        _windowLogic.SetNumberOfOpenWindows(_windowManager.GetNumberOfPeasants()); });
+        } });
 
     _WindowClosedToken = _windowManager.WindowClosed([this](auto&&, auto&&) {
         if (_getWindowLayoutThrottler)
         {
             _getWindowLayoutThrottler.value()();
         }
-        _windowLogic.SetNumberOfOpenWindows(_windowManager.GetNumberOfPeasants());
     });
 
     // These events are coming from peasants that become or un-become quake windows.
@@ -1651,7 +1655,8 @@ void AppHost::_CloseRequested(const winrt::Windows::Foundation::IInspectable& /*
                               const winrt::Windows::Foundation::IInspectable& /*args*/)
 {
     const auto pos = _GetWindowLaunchPosition();
-    _windowLogic.CloseWindow(pos);
+    const bool isLastWindow = _windowManager.GetNumberOfPeasants() == 1;
+    _windowLogic.CloseWindow(pos, isLastWindow);
 }
 
 void AppHost::_PropertyChangedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/,