diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 46889b1db99..ea1bb14a8fb 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -69,6 +69,29 @@ namespace winrt::TerminalApp::implementation // - see GH#2988 HRESULT TerminalPage::Initialize(HWND hwnd) { + if (!_hostingHwnd.has_value()) + { + // GH#13211 - if we haven't yet set the owning hwnd, reparent all the controls now. + for (const auto& tab : _tabs) + { + if (auto terminalTab{ _GetTerminalTabImpl(tab) }) + { + terminalTab->GetRootPane()->WalkTree([&](auto&& pane) { + if (const auto& term{ pane->GetTerminalControl() }) + { + term.OwningHwnd(reinterpret_cast(hwnd)); + } + }); + } + // We don't need to worry about resetting the owning hwnd for the + // SUI here. GH#13211 only repros for a defterm connection, where + // the tab is spawned before the window is created. It's not + // possible to make a SUI tab like that, before the window is + // created. The SUI could be spawned as a part of a window restore, + // but that would still work fine. The window would be created + // before restoring previous tabs in that scenario. + } + } _hostingHwnd = hwnd; return S_OK; } diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index ec470a31dd6..5635f4603d9 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -289,11 +289,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation const auto height = vp.Height(); _connection.Resize(height, width); - if (_OwningHwnd != 0) + if (_owningHwnd != 0) { if (auto conpty{ _connection.try_as() }) { - conpty.ReparentWindow(_OwningHwnd); + conpty.ReparentWindow(_owningHwnd); } } @@ -1881,6 +1881,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation return Opacity() < 1.0f || UseAcrylic() || !_settings->BackgroundImage().empty() || _settings->UseBackgroundImageForWindow(); } + uint64_t ControlCore::OwningHwnd() + { + return _owningHwnd; + } + + void ControlCore::OwningHwnd(uint64_t owner) + { + if (owner != _owningHwnd && _connection) + { + if (auto conpty{ _connection.try_as() }) + { + conpty.ReparentWindow(owner); + } + } + _owningHwnd = owner; + } + Windows::Foundation::Collections::IVector ControlCore::ScrollMarks() const { auto internalMarks{ _terminal->GetScrollMarks() }; diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 5f20493db17..0dcfde33359 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -186,10 +186,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void WindowVisibilityChanged(const bool showOrHide); - // TODO:GH#1256 - When a tab can be torn out or otherwise reparented to - // another window, this value will need a custom setter, so that we can - // also update the connection. - WINRT_PROPERTY(uint64_t, OwningHwnd, 0); + uint64_t OwningHwnd(); + void OwningHwnd(uint64_t owner); RUNTIME_SETTING(double, Opacity, _settings->Opacity()); RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic()); @@ -259,6 +257,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation double _panelHeight{ 0 }; double _compositionScale{ 0 }; + uint64_t _owningHwnd{ 0 }; + winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr }; std::shared_ptr> _tsfTryRedrawCanvas; std::shared_ptr> _updatePatternLocations; diff --git a/src/host/ConsoleArguments.cpp b/src/host/ConsoleArguments.cpp index f89bf249732..2541cc8ab82 100644 --- a/src/host/ConsoleArguments.cpp +++ b/src/host/ConsoleArguments.cpp @@ -25,6 +25,12 @@ const std::wstring_view ConsoleArguments::FEATURE_ARG = L"--feature"; const std::wstring_view ConsoleArguments::FEATURE_PTY_ARG = L"pty"; const std::wstring_view ConsoleArguments::COM_SERVER_ARG = L"-Embedding"; const std::wstring_view ConsoleArguments::PASSTHROUGH_ARG = L"--passthrough"; +// NOTE: Thinking about adding more commandline args that control conpty, for +// the Terminal? Make sure you add them to the commandline in +// ConsoleEstablishHandoff. We use that to initialize the ConsoleArguments for a +// defterm handoff s.t. they behave like a conpty connection that was started by +// the terminal. If you forget them there, the conpty won't obey them, only for +// defterm. std::wstring EscapeArgument(std::wstring_view ac) { diff --git a/src/host/input.cpp b/src/host/input.cpp index a689a9454c7..e49dee1a8be 100644 --- a/src/host/input.cpp +++ b/src/host/input.cpp @@ -326,7 +326,7 @@ void ProcessCtrlEvents() if (NT_SUCCESS(Status)) { Status = ServiceLocator::LocateConsoleControl() - ->EndTask((HANDLE)rgProcessHandleList[i].dwProcessID, + ->EndTask(rgProcessHandleList[i].dwProcessID, EventType, CtrlFlags); if (rgProcessHandleList[i].hProcess == nullptr) diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index d84f503e29f..285809d7002 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -525,7 +525,12 @@ try outPipeTheirSide.reset(); signalPipeTheirSide.reset(); - const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --signal {:#x}"), (int64_t)signalPipeOurSide.release()); + // GH#13211 - Make sure we request win32input mode and that the terminal + // obeys the resizing quirk. Otherwise, defterm connections to the Terminal + // are going to have weird resizing, and aren't going to send full fidelity + // input messages. + const auto commandLine = fmt::format(FMT_COMPILE(L" --headless --resizeQuirk --win32input --signal {:#x}"), + (int64_t)signalPipeOurSide.release()); ConsoleArguments consoleArgs(commandLine, inPipeOurSide.release(), outPipeOurSide.release()); RETURN_IF_FAILED(consoleArgs.ParseCommandline()); diff --git a/src/inc/HostSignals.hpp b/src/inc/HostSignals.hpp index 05e1d0f3f72..ec29e540646 100644 --- a/src/inc/HostSignals.hpp +++ b/src/inc/HostSignals.hpp @@ -13,21 +13,21 @@ namespace Microsoft::Console struct HostSignalNotifyAppData { uint32_t sizeInBytes; - uint32_t processId; + uint32_t processId; // THIS IS A PID }; struct HostSignalSetForegroundData { uint32_t sizeInBytes; - uint32_t processId; + uint32_t processId; // THIS IS A HANDLE, NOT A PID bool isForeground; }; struct HostSignalEndTaskData { uint32_t sizeInBytes; - uint32_t processId; + uint32_t processId; // THIS IS A PID uint32_t eventType; uint32_t ctrlFlags; }; -}; \ No newline at end of file +}; diff --git a/src/interactivity/base/HostSignalInputThread.cpp b/src/interactivity/base/HostSignalInputThread.cpp index 397131b08e4..bed01bbd013 100644 --- a/src/interactivity/base/HostSignalInputThread.cpp +++ b/src/interactivity/base/HostSignalInputThread.cpp @@ -94,9 +94,11 @@ T HostSignalInputThread::_ReceiveTypedPacket() } case HostSignals::SetForeground: { - auto msg = _ReceiveTypedPacket(); + _ReceiveTypedPacket(); - LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->SetForeground(ULongToHandle(msg.processId), msg.isForeground)); + // GH#13211 - This honestly shouldn't be called by OpenConsole + // anymore, but it's possible that a much older version of + // OpenConsole is still calling this. Just do nothing. break; } @@ -104,7 +106,7 @@ T HostSignalInputThread::_ReceiveTypedPacket() { auto msg = _ReceiveTypedPacket(); - LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->EndTask(ULongToHandle(msg.processId), msg.eventType, msg.ctrlFlags)); + LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->EndTask(msg.processId, msg.eventType, msg.ctrlFlags)); break; } diff --git a/src/interactivity/base/RemoteConsoleControl.cpp b/src/interactivity/base/RemoteConsoleControl.cpp index 21b8155a79f..7cd03e36317 100644 --- a/src/interactivity/base/RemoteConsoleControl.cpp +++ b/src/interactivity/base/RemoteConsoleControl.cpp @@ -57,19 +57,17 @@ template [[nodiscard]] NTSTATUS RemoteConsoleControl::SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground) { - HostSignalSetForegroundData data{}; - data.sizeInBytes = sizeof(data); - data.processId = HandleToULong(hProcess); - data.isForeground = fForeground; - - return _SendTypedPacket(_pipe.get(), HostSignals::SetForeground, data); + // GH#13211 - Apparently this API doesn't need to be forwarded to conhost at + // all. Instead, just perform the ConsoleControl operation here, in proc. + // This lets us avoid all sorts of strange handle duplicating weirdness. + return _control.SetForeground(hProcess, fForeground); } -[[nodiscard]] NTSTATUS RemoteConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) +[[nodiscard]] NTSTATUS RemoteConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) { HostSignalEndTaskData data{}; data.sizeInBytes = sizeof(data); - data.processId = HandleToULong(hProcessId); + data.processId = dwProcessId; data.eventType = dwEventType; data.ctrlFlags = ulCtrlFlags; diff --git a/src/interactivity/base/RemoteConsoleControl.hpp b/src/interactivity/base/RemoteConsoleControl.hpp index 52de8db1eb3..b922b9beaa6 100644 --- a/src/interactivity/base/RemoteConsoleControl.hpp +++ b/src/interactivity/base/RemoteConsoleControl.hpp @@ -14,6 +14,7 @@ Author(s): #pragma once #include "../inc/IConsoleControl.hpp" +#include "../win32/ConsoleControl.hpp" namespace Microsoft::Console::Interactivity { @@ -25,9 +26,11 @@ namespace Microsoft::Console::Interactivity // IConsoleControl Members [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId); [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground); - [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags); + [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags); private: wil::unique_handle _pipe; + + Win32::ConsoleControl _control; }; } diff --git a/src/interactivity/inc/IConsoleControl.hpp b/src/interactivity/inc/IConsoleControl.hpp index 02d83e1b97e..7129cff68ff 100644 --- a/src/interactivity/inc/IConsoleControl.hpp +++ b/src/interactivity/inc/IConsoleControl.hpp @@ -23,6 +23,6 @@ namespace Microsoft::Console::Interactivity virtual ~IConsoleControl() = default; [[nodiscard]] virtual NTSTATUS NotifyConsoleApplication(DWORD dwProcessId) = 0; [[nodiscard]] virtual NTSTATUS SetForeground(HANDLE hProcess, BOOL fForeground) = 0; - [[nodiscard]] virtual NTSTATUS EndTask(HANDLE hProcessId, DWORD dwEventType, ULONG ulCtrlFlags) = 0; + [[nodiscard]] virtual NTSTATUS EndTask(DWORD dwProcessId, DWORD dwEventType, ULONG ulCtrlFlags) = 0; }; } diff --git a/src/interactivity/onecore/ConsoleControl.cpp b/src/interactivity/onecore/ConsoleControl.cpp index 662a2b6b1e9..248d02b3ac8 100644 --- a/src/interactivity/onecore/ConsoleControl.cpp +++ b/src/interactivity/onecore/ConsoleControl.cpp @@ -22,13 +22,13 @@ using namespace Microsoft::Console::Interactivity::OneCore; return STATUS_SUCCESS; } -[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) +[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) { USER_API_MSG m{}; const auto a = &m.u.EndTask; RtlZeroMemory(a, sizeof(*a)); - a->ProcessId = hProcessId; + a->ProcessId = ULongToHandle(dwProcessId); // This is actually a PID, even though the struct expects a HANDLE. a->ConsoleEventCode = dwEventType; a->ConsoleFlags = ulCtrlFlags; diff --git a/src/interactivity/onecore/ConsoleControl.hpp b/src/interactivity/onecore/ConsoleControl.hpp index 9b406a475f4..4771c6c2220 100644 --- a/src/interactivity/onecore/ConsoleControl.hpp +++ b/src/interactivity/onecore/ConsoleControl.hpp @@ -26,6 +26,6 @@ namespace Microsoft::Console::Interactivity::OneCore // IConsoleControl Members [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId) noexcept override; [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground) noexcept override; - [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) override; + [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) override; }; } diff --git a/src/interactivity/win32/ConsoleControl.cpp b/src/interactivity/win32/ConsoleControl.cpp index 4cea4e3857e..78e66110df2 100644 --- a/src/interactivity/win32/ConsoleControl.cpp +++ b/src/interactivity/win32/ConsoleControl.cpp @@ -37,12 +37,12 @@ using namespace Microsoft::Console::Interactivity::Win32; sizeof(Flags)); } -[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) +[[nodiscard]] NTSTATUS ConsoleControl::EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags) { auto pConsoleWindow = ServiceLocator::LocateConsoleWindow(); CONSOLEENDTASK ConsoleEndTaskParams; - ConsoleEndTaskParams.ProcessId = hProcessId; + ConsoleEndTaskParams.ProcessId = ULongToHandle(dwProcessId); // This is actually a PID, even though the struct expects a HANDLE. ConsoleEndTaskParams.ConsoleEventCode = dwEventType; ConsoleEndTaskParams.ConsoleFlags = ulCtrlFlags; ConsoleEndTaskParams.hwnd = pConsoleWindow == nullptr ? nullptr : pConsoleWindow->GetWindowHandle(); diff --git a/src/interactivity/win32/ConsoleControl.hpp b/src/interactivity/win32/ConsoleControl.hpp index 9a8ab927fc0..fdd490a528a 100644 --- a/src/interactivity/win32/ConsoleControl.hpp +++ b/src/interactivity/win32/ConsoleControl.hpp @@ -47,7 +47,7 @@ namespace Microsoft::Console::Interactivity::Win32 // IConsoleControl Members [[nodiscard]] NTSTATUS NotifyConsoleApplication(_In_ DWORD dwProcessId); [[nodiscard]] NTSTATUS SetForeground(_In_ HANDLE hProcess, _In_ BOOL fForeground); - [[nodiscard]] NTSTATUS EndTask(_In_ HANDLE hProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags); + [[nodiscard]] NTSTATUS EndTask(_In_ DWORD dwProcessId, _In_ DWORD dwEventType, _In_ ULONG ulCtrlFlags); // Public Members [[nodiscard]] NTSTATUS Control(_In_ ConsoleControl::ControlType ConsoleCommand, diff --git a/src/server/ProcessList.cpp b/src/server/ProcessList.cpp index f852db7728e..ab9c529a76d 100644 --- a/src/server/ProcessList.cpp +++ b/src/server/ProcessList.cpp @@ -305,7 +305,7 @@ void ConsoleProcessList::ModifyConsoleProcessFocus(const bool fForeground) { const auto pProcessHandle = *it; - if (pProcessHandle->_hProcess != nullptr) + if (pProcessHandle->_hProcess) { _ModifyProcessForegroundRights(pProcessHandle->_hProcess.get(), fForeground); }