From 19ef8cc5e11cacf58f4b2a10259c72a6e64274d3 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 19 May 2023 11:27:50 -0500 Subject: [PATCH 01/17] Manually close the ContentDialog in teardown As discussed. Closes #15364. Prevents one crash on Windows 10. Opens the door to may more horrors. Co-authored by: @j4james --- src/cascadia/WindowsTerminal/AppHost.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 54cdc9d24fa..ccc52ead0cd 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -421,6 +421,12 @@ void AppHost::Close() _revokers = {}; _showHideWindowThrottler.reset(); _window->Close(); + + if (_windowLogic) + { + _windowLogic.DismissDialog(); + _windowLogic = nullptr; + } } // Method Description: From 3241ef0efd5bbaaf4195810e8f270e013d8a0b83 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 19 May 2023 17:09:53 -0500 Subject: [PATCH 02/17] yea sure this works I guess --- src/cascadia/WindowsTerminal/AppHost.cpp | 97 +++++++++++++++---- src/cascadia/WindowsTerminal/AppHost.h | 26 ++++- src/cascadia/WindowsTerminal/IslandWindow.cpp | 62 +++++++----- src/cascadia/WindowsTerminal/IslandWindow.h | 6 +- .../WindowsTerminal/NonClientIslandWindow.cpp | 10 +- .../WindowsTerminal/NonClientIslandWindow.h | 2 +- .../WindowsTerminal/WindowEmperor.cpp | 50 ++++++++-- src/cascadia/WindowsTerminal/WindowEmperor.h | 2 + src/cascadia/WindowsTerminal/WindowThread.cpp | 91 +++++++++++++++-- src/cascadia/WindowsTerminal/WindowThread.h | 11 +++ 10 files changed, 295 insertions(+), 62 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index ccc52ead0cd..2134a32b775 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -31,7 +31,8 @@ static constexpr short KeyPressed{ gsl::narrow_cast(0x8000) }; AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, const Remoting::WindowManager& manager, - const Remoting::Peasant& peasant) noexcept : + const Remoting::Peasant& peasant, + std::unique_ptr window) noexcept : _appLogic{ logic }, _windowLogic{ nullptr }, // don't make one, we're going to take a ref on app's _windowManager{ manager }, @@ -44,13 +45,21 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, // _HandleCommandlineArgs will create a _windowLogic _useNonClientArea = _windowLogic.GetShowTabsInTitlebar(); - if (_useNonClientArea) + + if (window) { - _window = std::make_unique(_windowLogic.GetRequestedTheme()); + _window = std::move(window); } else { - _window = std::make_unique(); + if (_useNonClientArea) + { + _window = std::make_unique(_windowLogic.GetRequestedTheme()); + } + else + { + _window = std::make_unique(); + } } // Update our own internal state tracking if we're in quake mode or not. @@ -65,16 +74,38 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, std::placeholders::_2); _window->SetCreateCallback(pfn); - _window->MouseScrolled({ this, &AppHost::_WindowMouseWheeled }); - _window->WindowActivated({ this, &AppHost::_WindowActivated }); - _window->WindowMoved({ this, &AppHost::_WindowMoved }); + _windowCallbacks.MouseScrolled = _window->MouseScrolled({ this, &AppHost::_WindowMouseWheeled }); + _windowCallbacks.WindowActivated = _window->WindowActivated({ this, &AppHost::_WindowActivated }); + _windowCallbacks.WindowMoved = _window->WindowMoved({ this, &AppHost::_WindowMoved }); + _windowCallbacks.ShouldExitFullscreen = _window->ShouldExitFullscreen({ &_windowLogic, &winrt::TerminalApp::TerminalWindow::RequestExitFullscreen }); - _window->ShouldExitFullscreen({ &_windowLogic, &winrt::TerminalApp::TerminalWindow::RequestExitFullscreen }); + // TODO! These call APIs that are re-entrant on the window message loop. + // If you call them in the ctor, then if we're being instantiated for a reheated thread, we'll deadlock. + // _window->SetAlwaysOnTop(_windowLogic.GetInitialAlwaysOnTop()); + // _window->SetAutoHideWindow(_windowLogic.AutoHideWindow()); - _window->SetAlwaysOnTop(_windowLogic.GetInitialAlwaysOnTop()); - _window->SetAutoHideWindow(_windowLogic.AutoHideWindow()); - - _window->MakeWindow(); + if (_window->GetHandle() == nullptr) + { + _window->MakeWindow(); + } + else + { + // _window->Microwave(); + + //auto dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); + //dispatcher.TryEnqueue([this]() { + // // TODO! obv these bounds are wrong + // _HandleCreateWindow(_window->GetHandle(), til::rect{ 0, 0, 256, 256 }); + //}); + + CREATESTRUCTW dumb{ + .cy = 256, + .cx = 256, + .y = 0, + .x = 0, + }; + PostMessageW(_window->GetHandle(), WM_CREATE, 0, (LPARAM)&dumb); + } } bool AppHost::OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down) @@ -297,7 +328,7 @@ void AppHost::Initialize() // Register the 'X' button of the window for a warning experience of multiple // tabs opened, this is consistent with Alt+F4 closing - _window->WindowCloseButtonClicked([this]() { + _windowCallbacks.WindowCloseButtonClicked = _window->WindowCloseButtonClicked([this]() { _CloseRequested(nullptr, nullptr); }); // If the user requests a close in another way handle the same as if the 'X' @@ -306,11 +337,11 @@ void AppHost::Initialize() // Add an event handler to plumb clicks in the titlebar area down to the // application layer. - _window->DragRegionClicked([this]() { _windowLogic.TitlebarClicked(); }); + _windowCallbacks.DragRegionClicked = _window->DragRegionClicked([this]() { _windowLogic.TitlebarClicked(); }); - _window->WindowVisibilityChanged([this](bool showOrHide) { _windowLogic.WindowVisibilityChanged(showOrHide); }); + _windowCallbacks.WindowVisibilityChanged = _window->WindowVisibilityChanged([this](bool showOrHide) { _windowLogic.WindowVisibilityChanged(showOrHide); }); - _window->UpdateSettingsRequested({ this, &AppHost::_requestUpdateSettings }); + _windowCallbacks.UpdateSettingsRequested = _window->UpdateSettingsRequested({ this, &AppHost::_requestUpdateSettings }); _revokers.Initialized = _windowLogic.Initialized(winrt::auto_revoke, { this, &AppHost::_WindowInitializedHandler }); _revokers.RequestedThemeChanged = _windowLogic.RequestedThemeChanged(winrt::auto_revoke, { this, &AppHost::_UpdateTheme }); @@ -321,14 +352,14 @@ void AppHost::Initialize() _revokers.SystemMenuChangeRequested = _windowLogic.SystemMenuChangeRequested(winrt::auto_revoke, { this, &AppHost::_SystemMenuChangeRequested }); _revokers.ChangeMaximizeRequested = _windowLogic.ChangeMaximizeRequested(winrt::auto_revoke, { this, &AppHost::_ChangeMaximizeRequested }); - _window->MaximizeChanged([this](bool newMaximize) { + _windowCallbacks.MaximizeChanged = _window->MaximizeChanged([this](bool newMaximize) { if (_windowLogic) { _windowLogic.Maximized(newMaximize); } }); - _window->AutomaticShutdownRequested([this]() { + _windowCallbacks.AutomaticShutdownRequested = _window->AutomaticShutdownRequested([this]() { // Raised when the OS is beginning an update of the app. We will quit, // to save our state, before the OS manually kills us. Remoting::WindowManager::RequestQuitAll(_peasant); @@ -429,6 +460,36 @@ void AppHost::Close() } } +[[nodiscard]] std::unique_ptr AppHost::Refrigerate() +{ + // After calling _window->Close() we should avoid creating more WinUI related actions. + // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. + _revokers = {}; + _showHideWindowThrottler.reset(); + + // DO NOT CLOSE THE WINDOW + _window->Refrigerate(); + + _window->MouseScrolled(_windowCallbacks.MouseScrolled); + _window->WindowActivated(_windowCallbacks.WindowActivated); + _window->WindowMoved(_windowCallbacks.WindowMoved); + _window->ShouldExitFullscreen(_windowCallbacks.ShouldExitFullscreen); + _window->WindowCloseButtonClicked(_windowCallbacks.WindowCloseButtonClicked); + _window->DragRegionClicked(_windowCallbacks.DragRegionClicked); + _window->WindowVisibilityChanged(_windowCallbacks.WindowVisibilityChanged); + _window->UpdateSettingsRequested(_windowCallbacks.UpdateSettingsRequested); + _window->MaximizeChanged(_windowCallbacks.MaximizeChanged); + _window->AutomaticShutdownRequested(_windowCallbacks.AutomaticShutdownRequested); + + if (_windowLogic) + { + _windowLogic.DismissDialog(); + _windowLogic = nullptr; + } + + return std::move(_window); +} + // Method Description: // - Called every time when the active tab's title changes. We'll also fire off // a window message so we can update the window's title on the main thread, diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index 5cd639fd5ed..c3bd2c4287d 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -12,12 +12,16 @@ class AppHost AppHost(const winrt::TerminalApp::AppLogic& logic, winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, const winrt::Microsoft::Terminal::Remoting::WindowManager& manager, - const winrt::Microsoft::Terminal::Remoting::Peasant& peasant) noexcept; + const winrt::Microsoft::Terminal::Remoting::Peasant& peasant, + std::unique_ptr window = nullptr) noexcept; void AppTitleChanged(const winrt::Windows::Foundation::IInspectable& sender, winrt::hstring newTitle); void LastTabClosed(const winrt::Windows::Foundation::IInspectable& sender, const winrt::TerminalApp::LastTabClosedEventArgs& args); void Initialize(); void Close(); + + [[nodiscard]] std::unique_ptr Refrigerate(); + bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down); void SetTaskbarProgress(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& args); @@ -194,4 +198,24 @@ class AppHost winrt::Microsoft::Terminal::Remoting::WindowManager::QuitAllRequested_revoker QuitAllRequested; winrt::Microsoft::Terminal::Remoting::Peasant::SendContentRequested_revoker SendContentRequested; } _revokers{}; + + // our IslandWindow is not a WinRT type. It can't make auto_revokers like + // the above. We also need to make sure to unregister ourself from the + // window when we refrigerate the window thread so that the window can later + // be re-used. + struct WindowRevokers + { + winrt::event_token MouseScrolled; + winrt::event_token WindowActivated; + winrt::event_token WindowMoved; + winrt::event_token ShouldExitFullscreen; + winrt::event_token WindowCloseButtonClicked; + winrt::event_token DragRegionClicked; + winrt::event_token WindowVisibilityChanged; + winrt::event_token UpdateSettingsRequested; + winrt::event_token MaximizeChanged; + winrt::event_token AutomaticShutdownRequested; + // LOAD BEARING!! + //If you add events here, make sure they're revokec in AppHost::Refrigerate + } _windowCallbacks{}; }; diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 36b2243f5ed..8a2215b1df5 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -42,6 +42,8 @@ IslandWindow::~IslandWindow() void IslandWindow::Close() { + // auto a{ _source }; + // winrt::detach_abi(_source); if (_source) { _source.Close(); @@ -49,6 +51,15 @@ void IslandWindow::Close() } } +void IslandWindow::Refrigerate() noexcept +{ + _pfnCreateCallback = nullptr; + _pfnSnapDimensionCallback = nullptr; + + _rootGrid.Children().Clear(); + ShowWindow(_window.get(), SW_HIDE); +} + HWND IslandWindow::GetInteropHandle() const { return _interopWindowHandle; @@ -295,38 +306,45 @@ LRESULT IslandWindow::_OnMoving(const WPARAM /*wParam*/, const LPARAM lParam) return false; } -void IslandWindow::Initialize() +// return true if this was a "cold" initialize, that didn't start XAML before. +bool IslandWindow::Initialize() { - _source = DesktopWindowXamlSource{}; + if (!_source) + { + _source = DesktopWindowXamlSource{}; - auto interop = _source.as(); - winrt::check_hresult(interop->AttachToWindow(_window.get())); + auto interop = _source.as(); + winrt::check_hresult(interop->AttachToWindow(_window.get())); - // stash the child interop handle so we can resize it when the main hwnd is resized - interop->get_WindowHandle(&_interopWindowHandle); + // stash the child interop handle so we can resize it when the main hwnd is resized + interop->get_WindowHandle(&_interopWindowHandle); - // Immediately hide our XAML island hwnd. On earlier versions of Windows, - // this HWND could sometimes appear as an actual window in the taskbar - // without this! - ShowWindow(_interopWindowHandle, SW_HIDE); + // Immediately hide our XAML island hwnd. On earlier versions of Windows, + // this HWND could sometimes appear as an actual window in the taskbar + // without this! + ShowWindow(_interopWindowHandle, SW_HIDE); - _rootGrid = winrt::Windows::UI::Xaml::Controls::Grid(); - _source.Content(_rootGrid); + _rootGrid = winrt::Windows::UI::Xaml::Controls::Grid(); + _source.Content(_rootGrid); - // initialize the taskbar object - if (auto taskbar = wil::CoCreateInstanceNoThrow(CLSID_TaskbarList)) - { - if (SUCCEEDED(taskbar->HrInit())) + // initialize the taskbar object + if (auto taskbar = wil::CoCreateInstanceNoThrow(CLSID_TaskbarList)) { - _taskbar = std::move(taskbar); + if (SUCCEEDED(taskbar->HrInit())) + { + _taskbar = std::move(taskbar); + } } - } - _systemMenuNextItemId = IDM_SYSTEM_MENU_BEGIN; + _systemMenuNextItemId = IDM_SYSTEM_MENU_BEGIN; + + // Enable vintage opacity by removing the XAML emergency backstop, GH#603. + // We don't really care if this failed or not. + TerminalTrySetTransparentBackground(true); - // Enable vintage opacity by removing the XAML emergency backstop, GH#603. - // We don't really care if this failed or not. - TerminalTrySetTransparentBackground(true); + return true; + } + return false; } void IslandWindow::OnSize(const UINT width, const UINT height) diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index ca34bc125bf..deff15d0629 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -3,6 +3,7 @@ #include "pch.h" #include "BaseWindow.h" +#include void SetWindowLongWHelper(const HWND hWnd, const int nIndex, const LONG dwNewLong) noexcept; @@ -21,6 +22,9 @@ class IslandWindow : virtual void MakeWindow() noexcept; virtual void Close(); + + void Refrigerate() noexcept; + virtual void OnSize(const UINT width, const UINT height); HWND GetInteropHandle() const; @@ -37,7 +41,7 @@ class IslandWindow : virtual til::rect GetNonClientFrame(const UINT dpi) const noexcept; virtual til::size GetTotalNonClientExclusiveSize(const UINT dpi) const noexcept; - virtual void Initialize(); + virtual bool Initialize(); void SetCreateCallback(std::function pfn) noexcept; diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index bb71bb1f69d..c299393645c 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -335,9 +335,9 @@ void NonClientIslandWindow::OnAppInitialized() IslandWindow::OnAppInitialized(); } -void NonClientIslandWindow::Initialize() +bool NonClientIslandWindow::Initialize() { - IslandWindow::Initialize(); + const bool coldInit = IslandWindow::Initialize(); _UpdateFrameMargins(); @@ -367,6 +367,8 @@ void NonClientIslandWindow::Initialize() // then make sure to update its visual state to reflect if we're in the // maximized state on launch. _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); + + return coldInit; } // Method Description: @@ -519,8 +521,8 @@ void NonClientIslandWindow::_OnMaximizeChange() noexcept const auto isIconified = WI_IsFlagSet(windowStyle, WS_ICONIC); const auto state = _isMaximized ? winrt::TerminalApp::WindowVisualState::WindowVisualStateMaximized : - isIconified ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified : - winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal; + isIconified ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified : + winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal; try { diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h index 09e02022486..544d27857ff 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h @@ -40,7 +40,7 @@ class NonClientIslandWindow : public IslandWindow virtual til::rect GetNonClientFrame(UINT dpi) const noexcept override; virtual til::size GetTotalNonClientExclusiveSize(UINT dpi) const noexcept override; - void Initialize() override; + bool Initialize() override; void OnAppInitialized() override; void SetContent(winrt::Windows::UI::Xaml::UIElement content) override; diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 77701a354ce..b3e2fc0a988 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -129,7 +129,28 @@ void WindowEmperor::WaitForWindows() void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& args) { Remoting::Peasant peasant{ _manager.CreatePeasant(args) }; - auto window{ std::make_shared(_app.Logic(), args, _manager, peasant) }; + std::shared_ptr window{ nullptr }; + + { + auto fridge{ _oldThreads.lock() }; + if (fridge->size() > 0) + { + window = fridge->back(); + fridge->pop_back(); + } + } + if (window) + { + _windowThreadInstances.fetch_add(1, std::memory_order_relaxed); + + window->Microwave(args, peasant); + // This will unblock the event we're waiting on in KeepWarm, and the + // window thread (started below) will continue through it's loop + return; + } + + window = std::make_shared(_app.Logic(), args, _manager, peasant); + std::weak_ptr weakThis{ weak_from_this() }; // Increment our count of window instances _now_, immediately. We're @@ -150,7 +171,7 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& std::thread t([weakThis, window]() { try { - const auto decrementWindowCount = wil::scope_exit([&]() { + auto decrementWindowCount = wil::scope_exit([&]() { if (auto self{ weakThis.lock() }) { self->_decrementWindowCount(); @@ -169,14 +190,27 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& { self->_windowStartedHandlerPostXAML(window); } + while (window->KeepWarm()) + { + window->RunMessagePump(); - window->RunMessagePump(); + // Manually trigger the cleanup callback. This will ensure that we + // remove the window from our list of windows, before we release the + // AppHost (and subsequently, the host's Logic() member that we use + // elsewhere). + removeWindow.reset(); - // Manually trigger the cleanup callback. This will ensure that we - // remove the window from our list of windows, before we release the - // AppHost (and subsequently, the host's Logic() member that we use - // elsewhere). - removeWindow.reset(); + window->Refrigerate(); + decrementWindowCount.reset(); + + if (auto self{ weakThis.lock() }) + { + auto fridge{ self->_oldThreads.lock() }; + fridge->push_back(window); + } + + // TODO! we never re-establish the decrementWindowCount handler for this thread. + } // Now that we no longer care about this thread's window, let it // release it's app host and flush the rest of the XAML queue. diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.h b/src/cascadia/WindowsTerminal/WindowEmperor.h index 96688ee08c2..47e8b356195 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.h +++ b/src/cascadia/WindowsTerminal/WindowEmperor.h @@ -43,6 +43,8 @@ class WindowEmperor : public std::enable_shared_from_this til::shared_mutex>> _windows; std::atomic _windowThreadInstances; + til::shared_mutex>> _oldThreads; + std::optional> _getWindowLayoutThrottler; winrt::event_token _WindowCreatedToken; diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index 7ddef728b0b..2287f317dfb 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -18,14 +18,23 @@ WindowThread::WindowThread(winrt::TerminalApp::AppLogic logic, void WindowThread::CreateHost() { + const bool coldStart = _warmWindow == nullptr; // Start the AppHost HERE, on the actual thread we want XAML to run on - _host = std::make_unique<::AppHost>(_appLogic, - _args, - _manager, - _peasant); - _host->UpdateSettingsRequested([this]() { _UpdateSettingsRequestedHandlers(); }); - - winrt::init_apartment(winrt::apartment_type::single_threaded); + _host = !coldStart ? std::make_unique<::AppHost>(_appLogic, + _args, + _manager, + _peasant, + std::move(_warmWindow)) : + std::make_unique<::AppHost>(_appLogic, + _args, + _manager, + _peasant); + _UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { _UpdateSettingsRequestedHandlers(); }); + + if (coldStart) + { + winrt::init_apartment(winrt::apartment_type::single_threaded); + } // Initialize the xaml content. This must be called AFTER the // WindowsXamlManager is initialized. @@ -42,6 +51,7 @@ int WindowThread::RunMessagePump() void WindowThread::RundownForExit() { + _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); _host->Close(); // !! LOAD BEARING !! @@ -60,6 +70,73 @@ void WindowThread::RundownForExit() } } } +bool WindowThread::KeepWarm() +{ + if (_host != nullptr) + { + return true; + } + // If we're refrigerated, then wait on the microwave signal, which will be + // raised when we get microwaved by another thread to reactivate us. . + + if (_warmWindow != nullptr) + { + // DO THE THING + std::unique_lock lock(_microwave); + _microwaveGoButton.wait(lock); + const bool reheated = _host != nullptr; + if (reheated) + { + // Re-initialize the host here, on the window thread + _host->Initialize(); + } + return reheated; + } + else + { + return false; + } +} + +void WindowThread::Refrigerate() +{ + _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); + + // keep a reference to the HWND and DWXS alive. + _warmWindow = std::move(_host->Refrigerate()); + + // rundown remaining messages before dtoring the app host + { + MSG msg = {}; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) + { + ::DispatchMessageW(&msg); + } + } + _host = nullptr; +} + +void WindowThread::Microwave( + winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, + winrt::Microsoft::Terminal::Remoting::Peasant peasant) +{ + _peasant = std::move(peasant); + _args = std::move(args); + // CreateHost(); + { + _host = std::make_unique<::AppHost>(_appLogic, + _args, + _manager, + _peasant, + std::move(_warmWindow)); + } + + // raise the signal + { + // std::unique_lock lock(_microwave); + _microwaveGoButton.notify_one(); + } +} winrt::TerminalApp::TerminalWindow WindowThread::Logic() { diff --git a/src/cascadia/WindowsTerminal/WindowThread.h b/src/cascadia/WindowsTerminal/WindowThread.h index 8699a791092..6bf4be5dbe6 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.h +++ b/src/cascadia/WindowsTerminal/WindowThread.h @@ -17,6 +17,12 @@ class WindowThread : public std::enable_shared_from_this int RunMessagePump(); void RundownForExit(); + bool KeepWarm(); + void Refrigerate(); + void Microwave( + winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, + winrt::Microsoft::Terminal::Remoting::Peasant peasant); + uint64_t PeasantID(); WINRT_CALLBACK(UpdateSettingsRequested, winrt::delegate); @@ -29,6 +35,11 @@ class WindowThread : public std::enable_shared_from_this winrt::Microsoft::Terminal::Remoting::WindowManager _manager{ nullptr }; std::unique_ptr<::AppHost> _host{ nullptr }; + winrt::event_token _UpdateSettingsRequestedToken; + + std::unique_ptr<::IslandWindow> _warmWindow{ nullptr }; + std::mutex _microwave; + std::condition_variable _microwaveGoButton; int _messagePump(); }; From f3d6f6a56fac6c37ef51af43924aefe12f0740f8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 23 May 2023 15:09:17 -0500 Subject: [PATCH 03/17] try to get the initial sizing right --- src/cascadia/WindowsTerminal/AppHost.cpp | 14 +++++++------- src/cascadia/WindowsTerminal/IslandWindow.cpp | 15 +++++++++++++++ src/cascadia/WindowsTerminal/IslandWindow.h | 2 +- .../WindowsTerminal/NonClientIslandWindow.cpp | 19 ++++++++++++++++--- .../WindowsTerminal/NonClientIslandWindow.h | 12 ++++++++++++ 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 2134a32b775..4b4c3e99418 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -98,13 +98,13 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, // _HandleCreateWindow(_window->GetHandle(), til::rect{ 0, 0, 256, 256 }); //}); - CREATESTRUCTW dumb{ - .cy = 256, - .cx = 256, - .y = 0, - .x = 0, - }; - PostMessageW(_window->GetHandle(), WM_CREATE, 0, (LPARAM)&dumb); + //CREATESTRUCTW dumb{ + // .cy = 256, + // .cx = 256, + // .y = 0, + // .x = 0, + //}; + //PostMessageW(_window->GetHandle(), WM_CREATE, 0, (LPARAM)&dumb); } } diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 8a2215b1df5..4e7f2c07ee0 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -344,6 +344,21 @@ bool IslandWindow::Initialize() return true; } + else + { + // This was a "warm" initialize - we've already got an HWND, but it's most certainly at the wrong place. + // Manually ask how we want to be created? + + if (_pfnCreateCallback) + { + til::rect rc{ GetWindowRect() }; + _pfnCreateCallback(_window.get(), rc); + } + UpdateWindow(_window.get()); + ForceResize(); + const auto size = GetPhysicalSize(); + this->IslandWindow::OnSize(size.width, size.height); + } return false; } diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index deff15d0629..f8e57db17e1 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -23,7 +23,7 @@ class IslandWindow : virtual void MakeWindow() noexcept; virtual void Close(); - void Refrigerate() noexcept; + virtual void Refrigerate() noexcept; virtual void OnSize(const UINT width, const UINT height); HWND GetInteropHandle() const; diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index c299393645c..6c9839a1d7c 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -335,6 +335,14 @@ void NonClientIslandWindow::OnAppInitialized() IslandWindow::OnAppInitialized(); } +void NonClientIslandWindow::Refrigerate() noexcept +{ + IslandWindow::Refrigerate(); + _dragBar.SizeChanged(_callbacks.dragBar_SizeChanged); + _rootGrid.SizeChanged(_callbacks.rootGrid_SizeChanged); + _titlebar.Loaded(_callbacks.titlebar_Loaded); +} + bool NonClientIslandWindow::Initialize() { const bool coldInit = IslandWindow::Initialize(); @@ -349,6 +357,7 @@ bool NonClientIslandWindow::Initialize() Controls::RowDefinition contentRow{}; titlebarRow.Height(GridLengthHelper::Auto()); + _rootGrid.RowDefinitions().Clear(); _rootGrid.RowDefinitions().Append(titlebarRow); _rootGrid.RowDefinitions().Append(contentRow); @@ -356,8 +365,12 @@ bool NonClientIslandWindow::Initialize() _titlebar = winrt::TerminalApp::TitlebarControl{ reinterpret_cast(GetHandle()) }; _dragBar = _titlebar.DragBar(); - _dragBar.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); - _rootGrid.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); + _callbacks.dragBar_SizeChanged = _dragBar.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); + + // if (coldInit) + { + _callbacks.rootGrid_SizeChanged = _rootGrid.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); + } _rootGrid.Children().Append(_titlebar); @@ -366,7 +379,7 @@ bool NonClientIslandWindow::Initialize() // GH#3440 - When the titlebar is loaded (officially added to our UI tree), // then make sure to update its visual state to reflect if we're in the // maximized state on launch. - _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); + _callbacks.titlebar_Loaded = _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); return coldInit; } diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h index 544d27857ff..250e895d90e 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h @@ -31,6 +31,8 @@ class NonClientIslandWindow : public IslandWindow NonClientIslandWindow(const winrt::Windows::UI::Xaml::ElementTheme& requestedTheme) noexcept; + void Refrigerate() noexcept override; + virtual void Close() override; void MakeWindow() noexcept override; virtual void OnSize(const UINT width, const UINT height) override; @@ -95,4 +97,14 @@ class NonClientIslandWindow : public IslandWindow void _UpdateFrameMargins() const noexcept; void _UpdateMaximizedState(); void _UpdateIslandPosition(const UINT windowWidth, const UINT windowHeight); + + struct Revokers + { + // winrt::Windows::UI::Xaml::Controls::Border::SizeChanged_revoker dragBar_SizeChanged; + // winrt::Windows::UI::Xaml::Controls::Grid::SizeChanged_revoker rootGrid_SizeChanged; + // winrt::TerminalApp::TitlebarControl::SizeChanged_revoker titlebar_Loaded; + winrt::event_token dragBar_SizeChanged; + winrt::event_token rootGrid_SizeChanged; + winrt::event_token titlebar_Loaded; + } _callbacks{}; }; From b0e80be4f318c0de8ab69c0db9d8cf592ddbd34f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 23 May 2023 15:58:09 -0500 Subject: [PATCH 04/17] hey look it worked --- src/cascadia/WindowsTerminal/IslandWindow.cpp | 8 ++++++-- src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 4e7f2c07ee0..f8d2415c50d 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -356,8 +356,12 @@ bool IslandWindow::Initialize() } UpdateWindow(_window.get()); ForceResize(); - const auto size = GetPhysicalSize(); - this->IslandWindow::OnSize(size.width, size.height); + + // DONT do this + // + // The NCIW doesn't set the Width/Height members of the _rootGrid. If you set them here (via IslandWindow::OnSize), we'll never change them for a subsequent resize + // const auto size = GetPhysicalSize(); + // this->IslandWindow::OnSize(size.width, size.height); } return false; } diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index 6c9839a1d7c..f3a65ebe47a 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -381,6 +381,8 @@ bool NonClientIslandWindow::Initialize() // maximized state on launch. _callbacks.titlebar_Loaded = _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); + _ResizeDragBarWindow(); + return coldInit; } From 0baf84cf285d01e65673240243e280c91fd4098f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 23 May 2023 16:36:19 -0500 Subject: [PATCH 05/17] some more minor code cleanup --- src/cascadia/WindowsTerminal/AppHost.cpp | 69 ++++++------ src/cascadia/WindowsTerminal/AppHost.h | 4 +- src/cascadia/WindowsTerminal/IslandWindow.cpp | 100 ++++++++++-------- src/cascadia/WindowsTerminal/IslandWindow.h | 3 + src/cascadia/WindowsTerminal/WindowThread.cpp | 1 + 5 files changed, 96 insertions(+), 81 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 4b4c3e99418..e8ad54a247e 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -46,7 +46,8 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, // _HandleCommandlineArgs will create a _windowLogic _useNonClientArea = _windowLogic.GetShowTabsInTitlebar(); - if (window) + const bool isWarmStart = window != nullptr; + if (isWarmStart) { _window = std::move(window); } @@ -79,33 +80,10 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, _windowCallbacks.WindowMoved = _window->WindowMoved({ this, &AppHost::_WindowMoved }); _windowCallbacks.ShouldExitFullscreen = _window->ShouldExitFullscreen({ &_windowLogic, &winrt::TerminalApp::TerminalWindow::RequestExitFullscreen }); - // TODO! These call APIs that are re-entrant on the window message loop. - // If you call them in the ctor, then if we're being instantiated for a reheated thread, we'll deadlock. - // _window->SetAlwaysOnTop(_windowLogic.GetInitialAlwaysOnTop()); - // _window->SetAutoHideWindow(_windowLogic.AutoHideWindow()); - - if (_window->GetHandle() == nullptr) + if (!isWarmStart) { _window->MakeWindow(); } - else - { - // _window->Microwave(); - - //auto dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); - //dispatcher.TryEnqueue([this]() { - // // TODO! obv these bounds are wrong - // _HandleCreateWindow(_window->GetHandle(), til::rect{ 0, 0, 256, 256 }); - //}); - - //CREATESTRUCTW dumb{ - // .cy = 256, - // .cx = 256, - // .y = 0, - // .x = 0, - //}; - //PostMessageW(_window->GetHandle(), WM_CREATE, 0, (LPARAM)&dumb); - } } bool AppHost::OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down) @@ -313,6 +291,14 @@ void AppHost::Initialize() _windowLogic.SetTitleBarContent({ this, &AppHost::_UpdateTitleBarContent }); } + // These call APIs that are re-entrant on the window message loop. If + // you call them in the ctor, we might deadlock. The ctor for AppHost isn't + // always called on the window thread - for reheated windows, it could be + // called on a random COM thread. + + _window->SetAlwaysOnTop(_windowLogic.GetInitialAlwaysOnTop()); + _window->SetAutoHideWindow(_windowLogic.AutoHideWindow()); + // MORE EVENT HANDLERS HERE! // MAKE SURE THEY ARE ALL: // * winrt::auto_revoke @@ -322,9 +308,10 @@ void AppHost::Initialize() // tearing down, after we've nulled out the window, during the dtor. That // can cause unexpected AV's everywhere. // - // _window callbacks don't need to be treated this way, because: - // * IslandWindow isn't a WinRT type (so it doesn't have neat revokers like this) - // * This particular bug scenario applies when we've already freed the window. + // _window callbacks are a little special: + // * IslandWindow isn't a WinRT type (so it doesn't have neat revokers like + // this), so instead they go in their own special helper struct. + // * they all need to be manually revoked in . // Register the 'X' button of the window for a warning experience of multiple // tabs opened, this is consistent with Alt+F4 closing @@ -451,6 +438,9 @@ void AppHost::Close() // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. _revokers = {}; _showHideWindowThrottler.reset(); + + _revokeWindowCallbacks(); + _window->Close(); if (_windowLogic) @@ -460,16 +450,8 @@ void AppHost::Close() } } -[[nodiscard]] std::unique_ptr AppHost::Refrigerate() +void AppHost::_revokeWindowCallbacks() { - // After calling _window->Close() we should avoid creating more WinUI related actions. - // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. - _revokers = {}; - _showHideWindowThrottler.reset(); - - // DO NOT CLOSE THE WINDOW - _window->Refrigerate(); - _window->MouseScrolled(_windowCallbacks.MouseScrolled); _window->WindowActivated(_windowCallbacks.WindowActivated); _window->WindowMoved(_windowCallbacks.WindowMoved); @@ -480,6 +462,19 @@ void AppHost::Close() _window->UpdateSettingsRequested(_windowCallbacks.UpdateSettingsRequested); _window->MaximizeChanged(_windowCallbacks.MaximizeChanged); _window->AutomaticShutdownRequested(_windowCallbacks.AutomaticShutdownRequested); +} + +[[nodiscard]] std::unique_ptr AppHost::Refrigerate() +{ + // After calling _window->Close() we should avoid creating more WinUI related actions. + // I suspect WinUI wouldn't like that very much. As such unregister all event handlers first. + _revokers = {}; + _showHideWindowThrottler.reset(); + + // DO NOT CLOSE THE WINDOW + _window->Refrigerate(); + + _revokeWindowCallbacks(); if (_windowLogic) { diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index c3bd2c4287d..36753e5b76b 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -59,6 +59,8 @@ class AppHost void _preInit(); + void _revokeWindowCallbacks(); + void _HandleCommandlineArgs(const winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs& args); void _HandleSessionRestore(const bool startedForContent); @@ -216,6 +218,6 @@ class AppHost winrt::event_token MaximizeChanged; winrt::event_token AutomaticShutdownRequested; // LOAD BEARING!! - //If you add events here, make sure they're revokec in AppHost::Refrigerate + //If you add events here, make sure they're revokec in AppHost::_revokeWindowCallbacks } _windowCallbacks{}; }; diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index f8d2415c50d..51c323d7a55 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -42,8 +42,6 @@ IslandWindow::~IslandWindow() void IslandWindow::Close() { - // auto a{ _source }; - // winrt::detach_abi(_source); if (_source) { _source.Close(); @@ -311,63 +309,79 @@ bool IslandWindow::Initialize() { if (!_source) { - _source = DesktopWindowXamlSource{}; + _coldInitialize(); + return true; + } + else + { + // This was a "warm" initialize - we've already got an HWND, but we need + // to move it to the new correct place, new size, and reset any leftover + // runtime state. + _warmInitialize(); + } + return false; +} - auto interop = _source.as(); - winrt::check_hresult(interop->AttachToWindow(_window.get())); +// Method Description: +// - Start this window for the first time. This will instantiate our XAML +// island, set up our root grid, and initialize some other members that only +// need to be initialized once. +// - This should only be called once. +void IslandWindow::_coldInitialize() +{ + _source = DesktopWindowXamlSource{}; + + auto interop = _source.as(); + winrt::check_hresult(interop->AttachToWindow(_window.get())); - // stash the child interop handle so we can resize it when the main hwnd is resized - interop->get_WindowHandle(&_interopWindowHandle); + // stash the child interop handle so we can resize it when the main hwnd is resized + interop->get_WindowHandle(&_interopWindowHandle); - // Immediately hide our XAML island hwnd. On earlier versions of Windows, - // this HWND could sometimes appear as an actual window in the taskbar - // without this! - ShowWindow(_interopWindowHandle, SW_HIDE); + // Immediately hide our XAML island hwnd. On earlier versions of Windows, + // this HWND could sometimes appear as an actual window in the taskbar + // without this! + ShowWindow(_interopWindowHandle, SW_HIDE); - _rootGrid = winrt::Windows::UI::Xaml::Controls::Grid(); - _source.Content(_rootGrid); + _rootGrid = winrt::Windows::UI::Xaml::Controls::Grid(); + _source.Content(_rootGrid); - // initialize the taskbar object - if (auto taskbar = wil::CoCreateInstanceNoThrow(CLSID_TaskbarList)) + // initialize the taskbar object + if (auto taskbar = wil::CoCreateInstanceNoThrow(CLSID_TaskbarList)) + { + if (SUCCEEDED(taskbar->HrInit())) { - if (SUCCEEDED(taskbar->HrInit())) - { - _taskbar = std::move(taskbar); - } + _taskbar = std::move(taskbar); } + } - _systemMenuNextItemId = IDM_SYSTEM_MENU_BEGIN; + _systemMenuNextItemId = IDM_SYSTEM_MENU_BEGIN; - // Enable vintage opacity by removing the XAML emergency backstop, GH#603. - // We don't really care if this failed or not. - TerminalTrySetTransparentBackground(true); + // Enable vintage opacity by removing the XAML emergency backstop, GH#603. + // We don't really care if this failed or not. + TerminalTrySetTransparentBackground(true); +} +void IslandWindow::_warmInitialize() +{ + // Manually ask how we want to be created? - return true; - } - else + if (_pfnCreateCallback) { - // This was a "warm" initialize - we've already got an HWND, but it's most certainly at the wrong place. - // Manually ask how we want to be created? - - if (_pfnCreateCallback) - { - til::rect rc{ GetWindowRect() }; - _pfnCreateCallback(_window.get(), rc); - } - UpdateWindow(_window.get()); - ForceResize(); - - // DONT do this - // - // The NCIW doesn't set the Width/Height members of the _rootGrid. If you set them here (via IslandWindow::OnSize), we'll never change them for a subsequent resize - // const auto size = GetPhysicalSize(); - // this->IslandWindow::OnSize(size.width, size.height); + til::rect rc{ GetWindowRect() }; + _pfnCreateCallback(_window.get(), rc); } - return false; + UpdateWindow(_window.get()); + ForceResize(); + + // Don't call IslandWindow::OnSize - that will set the Width/Height members + // of the _rootGrid. However, NCIW doesn't use those! If you set them, here, + // the contents of the window will never resize. } void IslandWindow::OnSize(const UINT width, const UINT height) { + // NOTE: This _isn't_ called by NonClientIslandWindow::OnSize. The NCIW has + // very different logic for positioning the DWXS inside it's HWND. + // update the interop window size SetWindowPos(_interopWindowHandle, nullptr, 0, 0, width, height, SWP_SHOWWINDOW | SWP_NOACTIVATE); diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index f8e57db17e1..3b2525715b7 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -117,6 +117,9 @@ class IslandWindow : RECT _rcWorkBeforeFullscreen{}; UINT _dpiBeforeFullscreen{ 96 }; + void _coldInitialize(); + void _warmInititialize(); + virtual void _SetIsBorderless(const bool borderlessEnabled); virtual void _SetIsFullscreen(const bool fullscreenEnabled); void _RestoreFullscreenPosition(const RECT& rcWork); diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index 2287f317dfb..f58c12d467a 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -87,6 +87,7 @@ bool WindowThread::KeepWarm() const bool reheated = _host != nullptr; if (reheated) { + _UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { _UpdateSettingsRequestedHandlers(); }); // Re-initialize the host here, on the window thread _host->Initialize(); } From b6478ce8f62994c6c0d6177d100581d5a8dcc95f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 23 May 2023 16:56:35 -0500 Subject: [PATCH 06/17] I think we can safely move this down --- src/cascadia/WindowsTerminal/IslandWindow.h | 2 +- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 15 +++++++-------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/cascadia/WindowsTerminal/IslandWindow.h b/src/cascadia/WindowsTerminal/IslandWindow.h index 3b2525715b7..541bb995ccd 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.h +++ b/src/cascadia/WindowsTerminal/IslandWindow.h @@ -118,7 +118,7 @@ class IslandWindow : UINT _dpiBeforeFullscreen{ 96 }; void _coldInitialize(); - void _warmInititialize(); + void _warmInitialize(); virtual void _SetIsBorderless(const bool borderlessEnabled); virtual void _SetIsFullscreen(const bool fullscreenEnabled); diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index b3e2fc0a988..a268dfd5992 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -171,12 +171,6 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& std::thread t([weakThis, window]() { try { - auto decrementWindowCount = wil::scope_exit([&]() { - if (auto self{ weakThis.lock() }) - { - self->_decrementWindowCount(); - } - }); auto removeWindow = wil::scope_exit([&]() { if (auto self{ weakThis.lock() }) { @@ -192,6 +186,13 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& } while (window->KeepWarm()) { + auto decrementWindowCount = wil::scope_exit([&]() { + if (auto self{ weakThis.lock() }) + { + self->_decrementWindowCount(); + } + }); + window->RunMessagePump(); // Manually trigger the cleanup callback. This will ensure that we @@ -208,8 +209,6 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& auto fridge{ self->_oldThreads.lock() }; fridge->push_back(window); } - - // TODO! we never re-establish the decrementWindowCount handler for this thread. } // Now that we no longer care about this thread's window, let it From fe08ad264d222cda48faaf7bb5d18029a92d5b43 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 23 May 2023 17:09:30 -0500 Subject: [PATCH 07/17] notes --- src/cascadia/WindowsTerminal/WindowThread.cpp | 41 +++++++++++-------- src/cascadia/WindowsTerminal/WindowThread.h | 2 +- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index f58c12d467a..98ca98b27e2 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -70,20 +70,34 @@ void WindowThread::RundownForExit() } } } + +// Method Description: +// - Check if we should keep this window alive, to try it's message loop again. +// If we were refrigerated for later, then this will block the thread on the +// _microwaveBuzzer. We'll sit there like that till the emperor decides if +// they want to re-use this window thread for a new window. +// Return Value: +// - true IFF we should enter this thread's message loop +// INVARAINT: This must be called on our "ui thread", our window thread. bool WindowThread::KeepWarm() { if (_host != nullptr) { + // We're currently hot return true; } + // If we're refrigerated, then wait on the microwave signal, which will be - // raised when we get microwaved by another thread to reactivate us. . + // raised when we get microwaved by another thread to reactivate us. if (_warmWindow != nullptr) { - // DO THE THING std::unique_lock lock(_microwave); - _microwaveGoButton.wait(lock); + _microwaveBuzzer.wait(lock); + + // TODO! There probably needs to be a way for us to be closed and have + // that set the signal too (so that we drop out of this and return false + // and cleanup our window.) const bool reheated = _host != nullptr; if (reheated) { @@ -123,20 +137,15 @@ void WindowThread::Microwave( { _peasant = std::move(peasant); _args = std::move(args); - // CreateHost(); - { - _host = std::make_unique<::AppHost>(_appLogic, - _args, - _manager, - _peasant, - std::move(_warmWindow)); - } - // raise the signal - { - // std::unique_lock lock(_microwave); - _microwaveGoButton.notify_one(); - } + _host = std::make_unique<::AppHost>(_appLogic, + _args, + _manager, + _peasant, + std::move(_warmWindow)); + + // raise the signal to unblock KeepWarm and start the window message loop again. + _microwaveBuzzer.notify_one(); } winrt::TerminalApp::TerminalWindow WindowThread::Logic() diff --git a/src/cascadia/WindowsTerminal/WindowThread.h b/src/cascadia/WindowsTerminal/WindowThread.h index 6bf4be5dbe6..4d135f849fb 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.h +++ b/src/cascadia/WindowsTerminal/WindowThread.h @@ -39,7 +39,7 @@ class WindowThread : public std::enable_shared_from_this std::unique_ptr<::IslandWindow> _warmWindow{ nullptr }; std::mutex _microwave; - std::condition_variable _microwaveGoButton; + std::condition_variable _microwaveBuzzer; int _messagePump(); }; From 7a85d663485974847e5301c0aa2e538f43d2d743 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 24 May 2023 16:33:44 -0500 Subject: [PATCH 08/17] egads this fixes #15410 --- .../WindowsTerminal/WindowEmperor.cpp | 32 ++++++++----------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index a268dfd5992..46f129f312b 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -37,6 +37,20 @@ WindowEmperor::WindowEmperor() noexcept : }); _dispatcher = winrt::Windows::System::DispatcherQueue::GetForCurrentThread(); + + // BODGY + // + // There's a mysterious crash in XAML on Windows 10 if you just let the App + // get dtor'd. By all accounts, it doesn't make sense. To mitigate this, we + // need to intentionally leak a reference to our App. Crazily, if you just + // let the app get cleaned up with the rest of the process when the process + // exits, then it doesn't crash. But if you let it get explicitly dtor'd, it + // absolutely will crash on exit. + // + // GH#15410 has more details. + + auto a{ _app }; + ::winrt::detach_abi(a); } WindowEmperor::~WindowEmperor() @@ -329,24 +343,6 @@ void WindowEmperor::_becomeMonarch() // We want at least some delay to prevent the first save from overwriting _getWindowLayoutThrottler.emplace(std::move(std::chrono::seconds(10)), std::move([this]() { _saveWindowLayoutsRepeat(); })); _getWindowLayoutThrottler.value()(); - - // BODGY - // - // We've got a weird crash that happens terribly inconsistently, but pretty - // readily on migrie's laptop, only in Debug mode. Apparently, there's some - // weird ref-counting magic that goes on during teardown, and our - // Application doesn't get closed quite right, which can cause us to crash - // into the debugger. This of course, only happens on exit, and happens - // somewhere in the XamlHost.dll code. - // - // Crazily, if we _manually leak the Application_ here, then the crash - // doesn't happen. This doesn't matter, because we really want the - // Application to live for _the entire lifetime of the process_, so the only - // time when this object would actually need to get cleaned up is _during - // exit_. So we can safely leak this Application object, and have it just - // get cleaned up normally when our process exits. - auto a{ _app }; - ::winrt::detach_abi(a); } // sender and args are always nullptr From 242dfe58997817406a43b883d85a6e63cb101e3f Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 24 May 2023 16:34:09 -0500 Subject: [PATCH 09/17] Good enough to start --- .../WindowsTerminal/WindowEmperor.cpp | 86 ++++++++++++++----- src/cascadia/WindowsTerminal/WindowThread.cpp | 81 ++++++++++------- src/cascadia/WindowsTerminal/WindowThread.h | 2 + 3 files changed, 117 insertions(+), 52 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 46f129f312b..86c5dc37bdb 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -59,6 +59,25 @@ WindowEmperor::~WindowEmperor() _app = nullptr; } +static bool IsWindows11() +{ + static const bool isWindows11 = []() { + OSVERSIONINFOEXW osver{}; + osver.dwOSVersionInfoSize = sizeof(osver); + osver.dwBuildNumber = 22000; + + DWORDLONG dwlConditionMask = 0; + VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); + + if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) + { + return true; + } + return false; + }(); + return isWindows11; +} + void _buildArgsFromCommandline(std::vector& args) { if (auto commandline{ GetCommandLineW() }) @@ -111,7 +130,8 @@ bool WindowEmperor::HandleCommandlineArgs() const auto result = _manager.ProposeCommandline(eventArgs, isolatedMode); - if (result.ShouldCreateWindow()) + const bool makeWindow = result.ShouldCreateWindow(); + if (makeWindow) { _createNewWindowThread(Remoting::WindowRequestedArgs{ result, eventArgs }); @@ -127,7 +147,7 @@ bool WindowEmperor::HandleCommandlineArgs() } } - return result.ShouldCreateWindow(); + return makeWindow; } void WindowEmperor::WaitForWindows() @@ -185,12 +205,6 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& std::thread t([weakThis, window]() { try { - auto removeWindow = wil::scope_exit([&]() { - if (auto self{ weakThis.lock() }) - { - self->_removeWindow(window->PeasantID()); - } - }); window->CreateHost(); @@ -200,6 +214,23 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& } while (window->KeepWarm()) { + // Now that the window is ready to go, we can add it to our list of windows, + // because we know it will be well behaved. + // + // Be sure to only modify the list of windows under lock. + + if (auto self{ weakThis.lock() }) + { + auto lockedWindows{ self->_windows.lock() }; + lockedWindows->push_back(window); + } + auto removeWindow = wil::scope_exit([&]() { + if (auto self{ weakThis.lock() }) + { + self->_removeWindow(window->PeasantID()); + } + }); + auto decrementWindowCount = wil::scope_exit([&]() { if (auto self{ weakThis.lock() }) { @@ -215,13 +246,24 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& // elsewhere). removeWindow.reset(); - window->Refrigerate(); - decrementWindowCount.reset(); + // On Windows 11, we DONT want to refrigerate the window. There, we can just + // close it like normal. + // TODO! - if (auto self{ weakThis.lock() }) + if (IsWindows11()) { - auto fridge{ self->_oldThreads.lock() }; - fridge->push_back(window); + decrementWindowCount.reset(); + } + else + { + window->Refrigerate(); + decrementWindowCount.reset(); + + if (auto self{ weakThis.lock() }) + { + auto fridge{ self->_oldThreads.lock() }; + fridge->push_back(window); + } } } @@ -257,15 +299,6 @@ void WindowEmperor::_windowStartedHandlerPostXAML(const std::shared_ptrpush_back(sender); - } } void WindowEmperor::_removeWindow(uint64_t senderID) @@ -553,6 +586,15 @@ LRESULT WindowEmperor::_messageHandler(UINT const message, WPARAM const wParam, winrt::fire_and_forget WindowEmperor::_close() { + { + auto fridge{ _oldThreads.lock() }; + for (auto& window : *fridge) + { + window->ThrowAway(); + } + fridge->clear(); + } + // Important! Switch back to the main thread for the emperor. That way, the // quit will go to the emperor's message pump. co_await wil::resume_foreground(_dispatcher); diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index 98ca98b27e2..2993778742c 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -18,23 +18,20 @@ WindowThread::WindowThread(winrt::TerminalApp::AppLogic logic, void WindowThread::CreateHost() { - const bool coldStart = _warmWindow == nullptr; + // Calling this while refrigerated won't work. + // * We can't re-initialize our winrt apartment. + // * AppHost::Initialize has to be done on the "UI" thread. + assert(_warmWindow == nullptr); + // Start the AppHost HERE, on the actual thread we want XAML to run on - _host = !coldStart ? std::make_unique<::AppHost>(_appLogic, - _args, - _manager, - _peasant, - std::move(_warmWindow)) : - std::make_unique<::AppHost>(_appLogic, - _args, - _manager, - _peasant); + _host = std::make_unique<::AppHost>(_appLogic, + _args, + _manager, + _peasant); + _UpdateSettingsRequestedToken = _host->UpdateSettingsRequested([this]() { _UpdateSettingsRequestedHandlers(); }); - if (coldStart) - { - winrt::init_apartment(winrt::apartment_type::single_threaded); - } + winrt::init_apartment(winrt::apartment_type::single_threaded); // Initialize the xaml content. This must be called AFTER the // WindowsXamlManager is initialized. @@ -49,10 +46,29 @@ int WindowThread::RunMessagePump() return exitCode; } +void WindowThread::_pumpRemainingXamlMessages() +{ + MSG msg = {}; + while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) + { + ::DispatchMessageW(&msg); + } +} + void WindowThread::RundownForExit() { - _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); - _host->Close(); + if (_host) + { + _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); + _host->Close(); + } + if (_warmWindow) + { + // If we have a _warmWindow, we're a refrigerated thread without a + // AppHost in control of the window. Manually close the window + // ourselves, to free the DWXS. + _warmWindow->Close(); + } // !! LOAD BEARING !! // @@ -62,13 +78,13 @@ void WindowThread::RundownForExit() // exiting. So do that now. If you don't, then the last tab to close // will never actually destruct the last tab / TermControl / ControlCore // / renderer. - { - MSG msg = {}; - while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) - { - ::DispatchMessageW(&msg); - } - } + _pumpRemainingXamlMessages(); +} + +void WindowThread::ThrowAway() +{ + // raise the signal to unblock KeepWarm. We won't have a host, so we'll drop out of the message loop to eventually RundownForExit. + _microwaveBuzzer.notify_one(); } // Method Description: @@ -113,6 +129,12 @@ bool WindowThread::KeepWarm() } } +// Method Description: +// - "Refrigerate" this thread for later reuse. This will refrigerate the window +// itself, and tear down our current app host. We'll save our window for +// later. We'll also pump out the existing message from XAML, before +// returning. After we return, the emperor will add us to the list of threads +// that can be re-used. void WindowThread::Refrigerate() { _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); @@ -121,16 +143,15 @@ void WindowThread::Refrigerate() _warmWindow = std::move(_host->Refrigerate()); // rundown remaining messages before dtoring the app host - { - MSG msg = {}; - while (PeekMessageW(&msg, nullptr, 0, 0, PM_REMOVE)) - { - ::DispatchMessageW(&msg); - } - } + _pumpRemainingXamlMessages(); _host = nullptr; } +// Method Description: +// - "Reheat" this thread for reuse. We'll build a new AppHost, and pass in the +// existing window to it. We'll then trigger the _microwaveBuzzer, so KeepWarm +// (which is on the UI thread) will get unblocked, and we can initialize this +// window. void WindowThread::Microwave( winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, winrt::Microsoft::Terminal::Remoting::Peasant peasant) diff --git a/src/cascadia/WindowsTerminal/WindowThread.h b/src/cascadia/WindowsTerminal/WindowThread.h index 4d135f849fb..0e8bc5429d1 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.h +++ b/src/cascadia/WindowsTerminal/WindowThread.h @@ -22,6 +22,7 @@ class WindowThread : public std::enable_shared_from_this void Microwave( winrt::Microsoft::Terminal::Remoting::WindowRequestedArgs args, winrt::Microsoft::Terminal::Remoting::Peasant peasant); + void ThrowAway(); uint64_t PeasantID(); @@ -42,4 +43,5 @@ class WindowThread : public std::enable_shared_from_this std::condition_variable _microwaveBuzzer; int _messagePump(); + void _pumpRemainingXamlMessages(); }; From a3bfa2133733ad5f1fdc7a2d54bb2ceb2f3e3b70 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 May 2023 06:48:14 -0500 Subject: [PATCH 10/17] spel and format --- src/cascadia/WindowsTerminal/AppHost.h | 2 +- src/cascadia/WindowsTerminal/IslandWindow.cpp | 7 ++++--- src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp | 4 ++-- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 1 - src/cascadia/WindowsTerminal/WindowThread.cpp | 10 +++++----- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h index 36753e5b76b..003e0fd89ad 100644 --- a/src/cascadia/WindowsTerminal/AppHost.h +++ b/src/cascadia/WindowsTerminal/AppHost.h @@ -218,6 +218,6 @@ class AppHost winrt::event_token MaximizeChanged; winrt::event_token AutomaticShutdownRequested; // LOAD BEARING!! - //If you add events here, make sure they're revokec in AppHost::_revokeWindowCallbacks + //If you add events here, make sure they're revoked in AppHost::_revokeWindowCallbacks } _windowCallbacks{}; }; diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 6c2379deca5..9a29db90a9c 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -423,14 +423,15 @@ void IslandWindow::_warmInitialize() ForceResize(); // Don't call IslandWindow::OnSize - that will set the Width/Height members - // of the _rootGrid. However, NCIW doesn't use those! If you set them, here, + // of the _rootGrid. However, NonClientIslandWindow doesn't use those! If you set them, here, // the contents of the window will never resize. } void IslandWindow::OnSize(const UINT width, const UINT height) { - // NOTE: This _isn't_ called by NonClientIslandWindow::OnSize. The NCIW has - // very different logic for positioning the DWXS inside it's HWND. + // NOTE: This _isn't_ called by NonClientIslandWindow::OnSize. The + // NonClientIslandWindow has very different logic for positioning the + // DesktopWindowXamlSource inside it's HWND. // update the interop window size SetWindowPos(_interopWindowHandle, nullptr, 0, 0, width, height, SWP_SHOWWINDOW | SWP_NOACTIVATE); diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index f3a65ebe47a..0d738a5c0b5 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -536,8 +536,8 @@ void NonClientIslandWindow::_OnMaximizeChange() noexcept const auto isIconified = WI_IsFlagSet(windowStyle, WS_ICONIC); const auto state = _isMaximized ? winrt::TerminalApp::WindowVisualState::WindowVisualStateMaximized : - isIconified ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified : - winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal; + isIconified ? winrt::TerminalApp::WindowVisualState::WindowVisualStateIconified : + winrt::TerminalApp::WindowVisualState::WindowVisualStateNormal; try { diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 1fe21a5e526..6e9390563f8 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -205,7 +205,6 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& std::thread t([weakThis, window]() { try { - window->CreateHost(); if (auto self{ weakThis.lock() }) diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index 2993778742c..5855c8a7b38 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -66,7 +66,7 @@ void WindowThread::RundownForExit() { // If we have a _warmWindow, we're a refrigerated thread without a // AppHost in control of the window. Manually close the window - // ourselves, to free the DWXS. + // ourselves, to free the DesktopWindowXamlSource. _warmWindow->Close(); } @@ -94,7 +94,7 @@ void WindowThread::ThrowAway() // they want to re-use this window thread for a new window. // Return Value: // - true IFF we should enter this thread's message loop -// INVARAINT: This must be called on our "ui thread", our window thread. +// INVARIANT: This must be called on our "ui thread", our window thread. bool WindowThread::KeepWarm() { if (_host != nullptr) @@ -104,7 +104,7 @@ bool WindowThread::KeepWarm() } // If we're refrigerated, then wait on the microwave signal, which will be - // raised when we get microwaved by another thread to reactivate us. + // raised when we get re-heated by another thread to reactivate us. if (_warmWindow != nullptr) { @@ -139,10 +139,10 @@ void WindowThread::Refrigerate() { _host->UpdateSettingsRequested(_UpdateSettingsRequestedToken); - // keep a reference to the HWND and DWXS alive. + // keep a reference to the HWND and DesktopWindowXamlSource alive. _warmWindow = std::move(_host->Refrigerate()); - // rundown remaining messages before dtoring the app host + // rundown remaining messages before destructing the app host _pumpRemainingXamlMessages(); _host = nullptr; } From b0994cdff6073d2c43041baf0acde95dee333af8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 May 2023 10:44:08 -0500 Subject: [PATCH 11/17] fix actually most of the Windows 11 issues --- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 6e9390563f8..f498c32bbd7 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -245,13 +245,13 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& // elsewhere). removeWindow.reset(); - // On Windows 11, we DONT want to refrigerate the window. There, we can just - // close it like normal. - // TODO! - + // On Windows 11, we DONT want to refrigerate the window. There, + // we can just close it like normal. Break out of the loop, so + // we don't try to put this window in the fridge. if (IsWindows11()) { decrementWindowCount.reset(); + break; } else { From ffc5ebd0075815ed8ffeb8d4153ef34e7f05db41 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 May 2023 10:59:08 -0500 Subject: [PATCH 12/17] comments --- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 5 +++++ src/cascadia/WindowsTerminal/WindowThread.cpp | 13 +++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index f498c32bbd7..3710b693a75 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -583,6 +583,11 @@ LRESULT WindowEmperor::_messageHandler(UINT const message, WPARAM const wParam, return DefWindowProc(_window.get(), message, wParam, lParam); } +// Close the Terminal application. This will exit the main thread for the +// emperor itself. We should probably only ever be called when we have no +// windows left, and we don't want to keep running anymore. This will discard +// all our refrigerated windows. If we try to use XAML on Windows 10 after this, +// we'll undoubtably crash. winrt::fire_and_forget WindowEmperor::_close() { { diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp index 5855c8a7b38..25fb94d5b07 100644 --- a/src/cascadia/WindowsTerminal/WindowThread.cpp +++ b/src/cascadia/WindowsTerminal/WindowThread.cpp @@ -83,7 +83,12 @@ void WindowThread::RundownForExit() void WindowThread::ThrowAway() { - // raise the signal to unblock KeepWarm. We won't have a host, so we'll drop out of the message loop to eventually RundownForExit. + // raise the signal to unblock KeepWarm. We won't have a host, so we'll drop + // out of the message loop to eventually RundownForExit. + // + // This should only be called when the app is fully quitting. After this is + // called on any thread, on win10, we won't be able to call into XAML + // anymore. _microwaveBuzzer.notify_one(); } @@ -111,9 +116,9 @@ bool WindowThread::KeepWarm() std::unique_lock lock(_microwave); _microwaveBuzzer.wait(lock); - // TODO! There probably needs to be a way for us to be closed and have - // that set the signal too (so that we drop out of this and return false - // and cleanup our window.) + // If ThrowAway() was called, then the buzzer will be signalled without + // setting a new _host. In that case, the app is quitting, for real. We + // just want to exit with false. const bool reheated = _host != nullptr; if (reheated) { From 44ef400139f35147c0472d9424fe400cfb18c5fc Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 May 2023 13:56:00 -0500 Subject: [PATCH 13/17] more notes --- src/cascadia/WindowsTerminal/AppHost.cpp | 6 ++ src/cascadia/WindowsTerminal/BaseWindow.h | 6 +- src/cascadia/WindowsTerminal/IslandWindow.cpp | 61 ++++++------------- .../WindowsTerminal/NonClientIslandWindow.cpp | 10 +-- .../WindowsTerminal/NonClientIslandWindow.h | 5 +- .../WindowsTerminal/WindowEmperor.cpp | 12 +++- 6 files changed, 47 insertions(+), 53 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index e8ad54a247e..19f3a0a0bcc 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -464,6 +464,12 @@ void AppHost::_revokeWindowCallbacks() _window->AutomaticShutdownRequested(_windowCallbacks.AutomaticShutdownRequested); } +// revoke our callbacks, discard our XAML content (TerminalWindow & +// TerminalPage), and hand back our IslandWindow. This does _not_ close the XAML +// island for this thread. We should not be re-used after this, and aour caller +// can destruct us like they normaly would during a close. The returned +// IslandWindow will retain ownership of the DesktopWindowXamlSource, for later +// reuse. [[nodiscard]] std::unique_ptr AppHost::Refrigerate() { // After calling _window->Close() we should avoid creating more WinUI related actions. diff --git a/src/cascadia/WindowsTerminal/BaseWindow.h b/src/cascadia/WindowsTerminal/BaseWindow.h index 75afc8e3b88..36c4eabb80e 100644 --- a/src/cascadia/WindowsTerminal/BaseWindow.h +++ b/src/cascadia/WindowsTerminal/BaseWindow.h @@ -210,13 +210,17 @@ class BaseWindow bool _minimized = false; + void _setupUserData() + { + SetWindowLongPtr(_window.get(), GWLP_USERDATA, reinterpret_cast(this)); + } // Method Description: // - This method is called when the window receives the WM_NCCREATE message. // Return Value: // - The value returned from the window proc. [[nodiscard]] virtual LRESULT OnNcCreate(WPARAM wParam, LPARAM lParam) noexcept { - SetWindowLongPtr(_window.get(), GWLP_USERDATA, reinterpret_cast(this)); + _setupUserData(); EnableNonClientDpiScaling(_window.get()); _currentDpi = GetDpiForWindow(_window.get()); diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 9a29db90a9c..38f38383345 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -42,46 +42,6 @@ IslandWindow::~IslandWindow() void IslandWindow::Close() { - static const bool isWindows11 = []() { - OSVERSIONINFOEXW osver{}; - osver.dwOSVersionInfoSize = sizeof(osver); - osver.dwBuildNumber = 22000; - - DWORDLONG dwlConditionMask = 0; - VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); - - if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) - { - return true; - } - return false; - }(); - - if (!isWindows11) - { - // BODGY - // ____ ____ _____ _______ __ - // | _ \ / __ \| __ \ / ____\ \ / / - // | |_) | | | | | | | | __ \ \_/ / - // | _ <| | | | | | | | |_ | \ / - // | |_) | |__| | |__| | |__| | | | - // |____/ \____/|_____/ \_____| |_| - // - // There's a bug in Windows 10 where closing a DesktopWindowXamlSource - // on any thread will free an internal static resource that's used by - // XAML for the entire process. This would result in closing window - // essentially causing the entire app to crash. - // - // To avoid this, leak the XAML island. We only need to leak this on - // Windows 10, since the bug is fixed in Windows 11. - // - // See GH #15384, MSFT:32109540 - auto a{ _source }; - winrt::detach_abi(_source); - - // - } - // GH#15454: Unset the user data for the window. This will prevent future // callbacks that come onto our window message loop from being sent to the // IslandWindow (or other derived class's) implementation. @@ -99,8 +59,20 @@ void IslandWindow::Close() } } +// Clear out any state that might be associated with this app instance, so that +// we can later re-use this HWND for another instance. +// +// This doesn't actually close out our HWND or DesktopWindowXamlSource, but it +// will remove all our content, and SW_HIDE the window, so it isn't accessible. void IslandWindow::Refrigerate() noexcept { + // Similar to in Close - unset our HWND's user data. We'll re-set this when + // we get re-heated, so that while we're refrigerated, we won't have + // unexpected callbacks into us while we don't have content. + // + // This pointer will get re-set in _warmInitialize + SetWindowLongPtr(_window.get(), GWLP_USERDATA, 0); + _pfnCreateCallback = nullptr; _pfnSnapDimensionCallback = nullptr; @@ -412,19 +384,22 @@ void IslandWindow::_coldInitialize() } void IslandWindow::_warmInitialize() { - // Manually ask how we want to be created? + // re-add the pointer to us to our HWND's user data, so that we can start + // getting window proc callbacks again. + _setupUserData(); + // Manually ask how we want to be created. if (_pfnCreateCallback) { til::rect rc{ GetWindowRect() }; _pfnCreateCallback(_window.get(), rc); } - UpdateWindow(_window.get()); - ForceResize(); // Don't call IslandWindow::OnSize - that will set the Width/Height members // of the _rootGrid. However, NonClientIslandWindow doesn't use those! If you set them, here, // the contents of the window will never resize. + UpdateWindow(_window.get()); + ForceResize(); } void IslandWindow::OnSize(const UINT width, const UINT height) diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index 0d738a5c0b5..adac312998b 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -366,11 +366,7 @@ bool NonClientIslandWindow::Initialize() _dragBar = _titlebar.DragBar(); _callbacks.dragBar_SizeChanged = _dragBar.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); - - // if (coldInit) - { - _callbacks.rootGrid_SizeChanged = _rootGrid.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); - } + _callbacks.rootGrid_SizeChanged = _rootGrid.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); _rootGrid.Children().Append(_titlebar); @@ -381,6 +377,10 @@ bool NonClientIslandWindow::Initialize() // maximized state on launch. _callbacks.titlebar_Loaded = _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); + // LOAD BEARING: call _ResizeDragBarWindow to update the position of our + // XAML island to reflect our current bounds. In the case of a "warm init" + // (i.e. re-using an existing window), we need to manually update the + // island's position to fill the new window bounds. _ResizeDragBarWindow(); return coldInit; diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h index 250e895d90e..2c18e9d3b7e 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h @@ -100,11 +100,10 @@ class NonClientIslandWindow : public IslandWindow struct Revokers { - // winrt::Windows::UI::Xaml::Controls::Border::SizeChanged_revoker dragBar_SizeChanged; - // winrt::Windows::UI::Xaml::Controls::Grid::SizeChanged_revoker rootGrid_SizeChanged; - // winrt::TerminalApp::TitlebarControl::SizeChanged_revoker titlebar_Loaded; winrt::event_token dragBar_SizeChanged; winrt::event_token rootGrid_SizeChanged; winrt::event_token titlebar_Loaded; + // LOAD BEARING!! + //If you add events here, make sure they're revoked in NonClientIslandWindow::Refrigerate } _callbacks{}; }; diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 3710b693a75..633fbb2c9c7 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -165,16 +165,23 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& Remoting::Peasant peasant{ _manager.CreatePeasant(args) }; std::shared_ptr window{ nullptr }; - { + // FIRST: Attempt to reheat an existing window that we refrigerated for + // later. If we have an existing unused window, then we don't need to create + // a new WindowThread & HWND for this request. + { // Add a scope to minimize lock duration. auto fridge{ _oldThreads.lock() }; if (fridge->size() > 0) { + // Look at that, a refrigerated thread ready to be used. Let's use that! window = fridge->back(); fridge->pop_back(); } } + + // Did we find one? if (window) { + // Cool! Let's increment the number of active windows, and re-heat it. _windowThreadInstances.fetch_add(1, std::memory_order_relaxed); window->Microwave(args, peasant); @@ -183,6 +190,9 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& return; } + // At this point, there weren't any pending refrigerated threads we could + // just use. That's fine. Let's just go create a new one. + window = std::make_shared(_app.Logic(), args, _manager, peasant); std::weak_ptr weakThis{ weak_from_this() }; From 33546b7c7ffca7d5f6193d03a07843c2a37c24c6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 30 May 2023 14:03:52 -0500 Subject: [PATCH 14/17] spell --- src/cascadia/WindowsTerminal/AppHost.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index 19f3a0a0bcc..9f6eb90e174 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -291,7 +291,7 @@ void AppHost::Initialize() _windowLogic.SetTitleBarContent({ this, &AppHost::_UpdateTitleBarContent }); } - // These call APIs that are re-entrant on the window message loop. If + // These call APIs that are reentrant on the window message loop. If // you call them in the ctor, we might deadlock. The ctor for AppHost isn't // always called on the window thread - for reheated windows, it could be // called on a random COM thread. @@ -466,8 +466,8 @@ void AppHost::_revokeWindowCallbacks() // revoke our callbacks, discard our XAML content (TerminalWindow & // TerminalPage), and hand back our IslandWindow. This does _not_ close the XAML -// island for this thread. We should not be re-used after this, and aour caller -// can destruct us like they normaly would during a close. The returned +// island for this thread. We should not be re-used after this, and our caller +// can destruct us like they normally would during a close. The returned // IslandWindow will retain ownership of the DesktopWindowXamlSource, for later // reuse. [[nodiscard]] std::unique_ptr AppHost::Refrigerate() From b03c525a58b1f8524af3effe9cb4c3fb838331db Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 12 Jul 2023 13:47:48 -0500 Subject: [PATCH 15/17] Update src/cascadia/WindowsTerminal/WindowEmperor.cpp Co-authored-by: Leonard Hecker --- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 633fbb2c9c7..8966577f418 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -170,10 +170,10 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& // a new WindowThread & HWND for this request. { // Add a scope to minimize lock duration. auto fridge{ _oldThreads.lock() }; - if (fridge->size() > 0) + if (!fridge->empty()) { // Look at that, a refrigerated thread ready to be used. Let's use that! - window = fridge->back(); + window = std::move(fridge->back()); fridge->pop_back(); } } From 47b28321460338e116e425bdfe476e6ce4430497 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 17 Jul 2023 09:55:21 -0500 Subject: [PATCH 16/17] some of the more minor notes --- src/cascadia/WindowsTerminal/AppHost.cpp | 31 +++++-------------- src/cascadia/WindowsTerminal/IslandWindow.cpp | 10 ++++-- .../WindowsTerminal/NonClientIslandWindow.cpp | 18 +++++++---- .../WindowsTerminal/NonClientIslandWindow.h | 8 ++--- .../WindowsTerminal/WindowEmperor.cpp | 21 +------------ src/types/inc/utils.hpp | 2 ++ src/types/utils.cpp | 19 ++++++++++++ 7 files changed, 53 insertions(+), 56 deletions(-) diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp index be61b978de5..34b69947e54 100644 --- a/src/cascadia/WindowsTerminal/AppHost.cpp +++ b/src/cascadia/WindowsTerminal/AppHost.cpp @@ -84,10 +84,7 @@ AppHost::AppHost(const winrt::TerminalApp::AppLogic& logic, _windowCallbacks.WindowMoved = _window->WindowMoved({ this, &AppHost::_WindowMoved }); _windowCallbacks.ShouldExitFullscreen = _window->ShouldExitFullscreen({ &_windowLogic, &winrt::TerminalApp::TerminalWindow::RequestExitFullscreen }); - if (!isWarmStart) - { - _window->MakeWindow(); - } + _window->MakeWindow(); } bool AppHost::OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down) @@ -315,7 +312,7 @@ void AppHost::Initialize() // _window callbacks are a little special: // * IslandWindow isn't a WinRT type (so it doesn't have neat revokers like // this), so instead they go in their own special helper struct. - // * they all need to be manually revoked in . + // * they all need to be manually revoked in _revokeWindowCallbacks. // Register the 'X' button of the window for a warning experience of multiple // tabs opened, this is consistent with Alt+F4 closing @@ -460,6 +457,9 @@ void AppHost::Close() void AppHost::_revokeWindowCallbacks() { + // You'll recall, IslandWindow isn't a WinRT type so it can't have auto-revokers. + // + // Instead, we need to manually remove our callbacks we registered on the window object. _window->MouseScrolled(_windowCallbacks.MouseScrolled); _window->WindowActivated(_windowCallbacks.WindowActivated); _window->WindowMoved(_windowCallbacks.WindowMoved); @@ -485,11 +485,11 @@ void AppHost::_revokeWindowCallbacks() _revokers = {}; _showHideWindowThrottler.reset(); + _revokeWindowCallbacks(); + // DO NOT CLOSE THE WINDOW _window->Refrigerate(); - _revokeWindowCallbacks(); - if (_windowLogic) { _windowLogic.DismissDialog(); @@ -1111,22 +1111,7 @@ static bool _isActuallyDarkTheme(const auto requestedTheme) // Windows 10, so that we don't even get that spew void _frameColorHelper(const HWND h, const COLORREF color) { - static const bool isWindows11 = []() { - OSVERSIONINFOEXW osver{}; - osver.dwOSVersionInfoSize = sizeof(osver); - osver.dwBuildNumber = 22000; - - DWORDLONG dwlConditionMask = 0; - VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); - - if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) - { - return true; - } - return false; - }(); - - if (isWindows11) + if (Utils::IsWindows11()) { LOG_IF_FAILED(DwmSetWindowAttribute(h, DWMWA_BORDER_COLOR, &color, sizeof(color))); } diff --git a/src/cascadia/WindowsTerminal/IslandWindow.cpp b/src/cascadia/WindowsTerminal/IslandWindow.cpp index 32cbeffd2fa..87ec8d5dbce 100644 --- a/src/cascadia/WindowsTerminal/IslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/IslandWindow.cpp @@ -93,6 +93,12 @@ HWND IslandWindow::GetInteropHandle() const // - void IslandWindow::MakeWindow() noexcept { + if (_window) + { + // no-op if we already have a window. + return; + } + WNDCLASS wc{}; wc.hCursor = LoadCursor(nullptr, IDC_ARROW); wc.hInstance = reinterpret_cast(&__ImageBase); @@ -340,8 +346,8 @@ bool IslandWindow::Initialize() // to move it to the new correct place, new size, and reset any leftover // runtime state. _warmInitialize(); + return false; } - return false; } // Method Description: @@ -406,7 +412,7 @@ void IslandWindow::OnSize(const UINT width, const UINT height) { // NOTE: This _isn't_ called by NonClientIslandWindow::OnSize. The // NonClientIslandWindow has very different logic for positioning the - // DesktopWindowXamlSource inside it's HWND. + // DesktopWindowXamlSource inside its HWND. // update the interop window size SetWindowPos(_interopWindowHandle, nullptr, 0, 0, width, height, SWP_SHOWWINDOW | SWP_NOACTIVATE); diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp index adac312998b..ee538bfa71a 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.cpp @@ -57,6 +57,12 @@ static constexpr const wchar_t* dragBarClassName{ L"DRAG_BAR_WINDOW_CLASS" }; void NonClientIslandWindow::MakeWindow() noexcept { + if (_window) + { + // no-op if we already have a window. + return; + } + IslandWindow::MakeWindow(); static auto dragBarWindowClass{ []() { @@ -338,9 +344,9 @@ void NonClientIslandWindow::OnAppInitialized() void NonClientIslandWindow::Refrigerate() noexcept { IslandWindow::Refrigerate(); - _dragBar.SizeChanged(_callbacks.dragBar_SizeChanged); - _rootGrid.SizeChanged(_callbacks.rootGrid_SizeChanged); - _titlebar.Loaded(_callbacks.titlebar_Loaded); + + // Revoke all our XAML callbacks. + _callbacks = {}; } bool NonClientIslandWindow::Initialize() @@ -365,8 +371,8 @@ bool NonClientIslandWindow::Initialize() _titlebar = winrt::TerminalApp::TitlebarControl{ reinterpret_cast(GetHandle()) }; _dragBar = _titlebar.DragBar(); - _callbacks.dragBar_SizeChanged = _dragBar.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); - _callbacks.rootGrid_SizeChanged = _rootGrid.SizeChanged({ this, &NonClientIslandWindow::_OnDragBarSizeChanged }); + _callbacks.dragBarSizeChanged = _dragBar.SizeChanged(winrt::auto_revoke, { this, &NonClientIslandWindow::_OnDragBarSizeChanged }); + _callbacks.rootGridSizeChanged = _rootGrid.SizeChanged(winrt::auto_revoke, { this, &NonClientIslandWindow::_OnDragBarSizeChanged }); _rootGrid.Children().Append(_titlebar); @@ -375,7 +381,7 @@ bool NonClientIslandWindow::Initialize() // GH#3440 - When the titlebar is loaded (officially added to our UI tree), // then make sure to update its visual state to reflect if we're in the // maximized state on launch. - _callbacks.titlebar_Loaded = _titlebar.Loaded([this](auto&&, auto&&) { _OnMaximizeChange(); }); + _callbacks.titlebarLoaded = _titlebar.Loaded(winrt::auto_revoke, [this](auto&&, auto&&) { _OnMaximizeChange(); }); // LOAD BEARING: call _ResizeDragBarWindow to update the position of our // XAML island to reflect our current bounds. In the case of a "warm init" diff --git a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h index 2c18e9d3b7e..d173ad68c2d 100644 --- a/src/cascadia/WindowsTerminal/NonClientIslandWindow.h +++ b/src/cascadia/WindowsTerminal/NonClientIslandWindow.h @@ -100,10 +100,8 @@ class NonClientIslandWindow : public IslandWindow struct Revokers { - winrt::event_token dragBar_SizeChanged; - winrt::event_token rootGrid_SizeChanged; - winrt::event_token titlebar_Loaded; - // LOAD BEARING!! - //If you add events here, make sure they're revoked in NonClientIslandWindow::Refrigerate + winrt::Windows::UI::Xaml::Controls::Border::SizeChanged_revoker dragBarSizeChanged; + winrt::Windows::UI::Xaml::Controls::Grid::SizeChanged_revoker rootGridSizeChanged; + winrt::TerminalApp::TitlebarControl::Loaded_revoker titlebarLoaded; } _callbacks{}; }; diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 8966577f418..2e0c97b41c3 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -59,25 +59,6 @@ WindowEmperor::~WindowEmperor() _app = nullptr; } -static bool IsWindows11() -{ - static const bool isWindows11 = []() { - OSVERSIONINFOEXW osver{}; - osver.dwOSVersionInfoSize = sizeof(osver); - osver.dwBuildNumber = 22000; - - DWORDLONG dwlConditionMask = 0; - VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); - - if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) - { - return true; - } - return false; - }(); - return isWindows11; -} - void _buildArgsFromCommandline(std::vector& args) { if (auto commandline{ GetCommandLineW() }) @@ -258,7 +239,7 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs& // On Windows 11, we DONT want to refrigerate the window. There, // we can just close it like normal. Break out of the loop, so // we don't try to put this window in the fridge. - if (IsWindows11()) + if (Utils::IsWindows11()) { decrementWindowCount.reset(); break; diff --git a/src/types/inc/utils.hpp b/src/types/inc/utils.hpp index b8ce375f1ea..2aac2ef526e 100644 --- a/src/types/inc/utils.hpp +++ b/src/types/inc/utils.hpp @@ -115,4 +115,6 @@ namespace Microsoft::Console::Utils // Same deal, but in TerminalPage::_evaluatePathForCwd std::wstring EvaluateStartingDirectory(std::wstring_view cwd, std::wstring_view startingDirectory); + bool IsWindows11(); + } diff --git a/src/types/utils.cpp b/src/types/utils.cpp index 5207d87a91b..75a884883a9 100644 --- a/src/types/utils.cpp +++ b/src/types/utils.cpp @@ -852,3 +852,22 @@ std::wstring Utils::EvaluateStartingDirectory( } return resultPath; } + +bool Utils::IsWindows11() +{ + static const bool isWindows11 = []() { + OSVERSIONINFOEXW osver{}; + osver.dwOSVersionInfoSize = sizeof(osver); + osver.dwBuildNumber = 22000; + + DWORDLONG dwlConditionMask = 0; + VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL); + + if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE) + { + return true; + } + return false; + }(); + return isWindows11; +} From 7650ef71b7efc1859c4d1db5a7e75a1b0e7b6ef8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Wed, 19 Jul 2023 10:16:38 -0500 Subject: [PATCH 17/17] Last audit mode nits --- src/cascadia/WindowsTerminal/WindowEmperor.cpp | 2 +- src/types/inc/utils.hpp | 2 +- src/types/utils.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp index 2e0c97b41c3..23aba3d683e 100644 --- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp +++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp @@ -578,7 +578,7 @@ LRESULT WindowEmperor::_messageHandler(UINT const message, WPARAM const wParam, // emperor itself. We should probably only ever be called when we have no // windows left, and we don't want to keep running anymore. This will discard // all our refrigerated windows. If we try to use XAML on Windows 10 after this, -// we'll undoubtably crash. +// we'll undoubtedly crash. winrt::fire_and_forget WindowEmperor::_close() { { diff --git a/src/types/inc/utils.hpp b/src/types/inc/utils.hpp index 2aac2ef526e..381e07b243c 100644 --- a/src/types/inc/utils.hpp +++ b/src/types/inc/utils.hpp @@ -115,6 +115,6 @@ namespace Microsoft::Console::Utils // Same deal, but in TerminalPage::_evaluatePathForCwd std::wstring EvaluateStartingDirectory(std::wstring_view cwd, std::wstring_view startingDirectory); - bool IsWindows11(); + bool IsWindows11() noexcept; } diff --git a/src/types/utils.cpp b/src/types/utils.cpp index 75a884883a9..91fd21e2633 100644 --- a/src/types/utils.cpp +++ b/src/types/utils.cpp @@ -853,9 +853,9 @@ std::wstring Utils::EvaluateStartingDirectory( return resultPath; } -bool Utils::IsWindows11() +bool Utils::IsWindows11() noexcept { - static const bool isWindows11 = []() { + static const bool isWindows11 = []() noexcept { OSVERSIONINFOEXW osver{}; osver.dwOSVersionInfoSize = sizeof(osver); osver.dwBuildNumber = 22000;