Make sure foreground access works for DefTerm

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<uint64_t>(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;
     }

src/cascadia/TerminalControl/ControlCore.cpp

@@ -267,11 +267,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<TerminalConnection::ConptyConnection>() })
                 {
-                    conpty.ReparentWindow(_OwningHwnd);
+                    conpty.ReparentWindow(_owningHwnd);
                 }
             }
 
@@ -1854,4 +1854,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // transparency, or our acrylic, or our image.
         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<TerminalConnection::ConptyConnection>() })
+            {
+                conpty.ReparentWindow(owner);
+            }
+        }
+        _owningHwnd = owner;
+    }
 }

src/cascadia/TerminalControl/ControlCore.h

@@ -179,10 +179,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());
@@ -252,6 +250,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<ThrottledFuncTrailing<>> _tsfTryRedrawCanvas;
         std::shared_ptr<ThrottledFuncTrailing<>> _updatePatternLocations;

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)
 {

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)

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());

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;
     };
-};
+};

src/interactivity/base/HostSignalInputThread.cpp

@@ -96,6 +96,9 @@ T HostSignalInputThread::_ReceiveTypedPacket()
         {
             auto msg = _ReceiveTypedPacket<HostSignalSetForegroundData>();
 
+            // GH#13211 - The OpenConsole side should have already duplicated
+            // the handle to us, and processId should be the value in our
+            // process space.
             LOG_IF_NTSTATUS_FAILED(ServiceLocator::LocateConsoleControl()->SetForeground(ULongToHandle(msg.processId), msg.isForeground));
 
             break;
@@ -104,7 +107,7 @@ T HostSignalInputThread::_ReceiveTypedPacket()
         {
             auto msg = _ReceiveTypedPacket<HostSignalEndTaskData>();
 
-            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;
         }

src/interactivity/base/RemoteConsoleControl.cpp

@@ -65,11 +65,11 @@ template<typename T>
     return _SendTypedPacket(_pipe.get(), HostSignals::SetForeground, data);
 }
 
-[[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;
 

src/interactivity/base/RemoteConsoleControl.hpp

@@ -25,7 +25,7 @@ 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;

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;
     };
 }

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;
 

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;
     };
 }

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();

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,

src/server/ProcessHandle.cpp

@@ -8,6 +8,9 @@
 #include "../host/globals.h"
 #include "../host/telemetry.hpp"
 
+#include "../interactivity/inc/ServiceLocator.hpp"
+
+using namespace Microsoft::Console::Interactivity;
 // Routine Description:
 // - Constructs an instance of the ConsoleProcessHandle Class
 // - NOTE: Can throw if allocation fails or if there is a console policy we do not understand.
@@ -34,6 +37,44 @@ ConsoleProcessHandle::ConsoleProcessHandle(const DWORD dwProcessId,
     {
         Telemetry::Instance().LogProcessConnected(_hProcess.get());
     }
+
+    // GH#13211 - If we're running as the delegation console (someone handed off
+    // to us), then we need to make sure the original conhost has access to this
+    // process handle as well. Otherwise, the future calls to
+    // ConsoleControl(SetForeground,...) won't work, because the literal handle
+    // value doesn't exist in the original conhost process space.
+    // * g.handoffInboxConsoleHandle is only set when we've been delegated to
+    // * We can't just pass something like the PID, because the OS conhost
+    //   already expects a literal handle value via the HostSignalInputThread.
+    //   If we changed that in the OpenConsole version, then there'll surely be
+    //   the chance for a mispatch between the OS conhost and the delegated
+    //   console.
+    if (const auto& conhost{ ServiceLocator::LocateGlobals().handoffInboxConsoleHandle })
+    {
+        LOG_IF_WIN32_BOOL_FALSE(DuplicateHandle(GetCurrentProcess(),
+                                                _hProcess.get(),
+                                                conhost.get(),
+                                                _hProcessInConhost.put(),
+                                                0 /*dwDesiredAccess, ignored*/,
+                                                false,
+                                                DUPLICATE_SAME_ACCESS));
+    }
+}
+
+ConsoleProcessHandle::~ConsoleProcessHandle()
+{
+    // Close out the handle in the origin conhost.
+    const auto& conhost{ ServiceLocator::LocateGlobals().handoffInboxConsoleHandle };
+    if (_hProcessInConhost && conhost)
+    {
+        LOG_IF_WIN32_BOOL_FALSE(DuplicateHandle(conhost.get(),
+                                                _hProcessInConhost.get(),
+                                                nullptr /* hTargetProcessHandle */,
+                                                nullptr /* lpTargetHandle, ignored */,
+                                                0 /* dwDesiredAccess, ignored */,
+                                                false,
+                                                DUPLICATE_CLOSE_SOURCE));
+    }
 }
 
 // Routine Description:

src/server/ProcessHandle.h

@@ -50,7 +50,7 @@ class ConsoleProcessHandle
     ConsoleProcessHandle(const DWORD dwProcessId,
                          const DWORD dwThreadId,
                          const ULONG ulProcessGroupId);
-    ~ConsoleProcessHandle() = default;
+    ~ConsoleProcessHandle();
     ConsoleProcessHandle(const ConsoleProcessHandle&) = delete;
     ConsoleProcessHandle(ConsoleProcessHandle&&) = delete;
     ConsoleProcessHandle& operator=(const ConsoleProcessHandle&) & = delete;
@@ -59,6 +59,7 @@ class ConsoleProcessHandle
     ULONG _ulTerminateCount;
     ULONG const _ulProcessGroupId;
     wil::unique_handle const _hProcess;
+    wil::unique_handle _hProcessInConhost;
 
     mutable ULONG64 _processCreationTime;
 

src/server/ProcessList.cpp

@@ -305,9 +305,13 @@ void ConsoleProcessList::ModifyConsoleProcessFocus(const bool fForeground)
     {
         const auto pProcessHandle = *it;
 
-        if (pProcessHandle->_hProcess != nullptr)
+        // If we're a delegated-to console (OpenConsole), then make sure to pass
+        // in the value of the handle in the origin conhost, not our own.
+        if (const auto& handleToUse = pProcessHandle->_hProcessInConhost ?
+                                          pProcessHandle->_hProcessInConhost :
+                                          pProcessHandle->_hProcess)
         {
-            _ModifyProcessForegroundRights(pProcessHandle->_hProcess.get(), fForeground);
+            _ModifyProcessForegroundRights(handleToUse.get(), fForeground);
         }
 
         it = std::next(it);