Make sure foreground access works for DefTerm (#13247)

See also: #12799, the origin of much of this.

This change evolved over multiple phases.

When we create a defterm connection in `TerminalPage::_OnNewConnection`,
we don't have the hosting HWND yet, so the tab gets created without one.
We'll later get called with the owner, in `Initialize`.

To remedy this, we need to:
* In `Initialize`, make sure to update any existing controls with the
  new owner.
* In `ControlCore`, actually propogate the new owner down to the
  connection

DefTerm launches don't actually request focus mode, so the Terminal
never sends them focus events. We need those focus events so that the
console can request foreground rights.

To remedy this, we need to:
* pass `--win32input` to the commandline used to initialize OpenConsole
  in ConPTY mode. We request focus events at the same time we request
  win32-input-mode.
* I also added `--resizeQuirk`, because _by all accounts that should be
  there_. Resizing in defterm windows should be _wacky_ without it, and
  I'm a little surprised we haven't seen any bugs due to this yet.

`ConsoleSetForeground` expects a `HANDLE` to the process we want to give
foreground rights to. The problem is, the wire format we used _also_
decided that a HANDLE value was a good idea. It's not. If we pass the
literal value of the HANDLE to the process from OpenConsole to conhost,
so conhost can call that API, the value that conhost uses there will
most likely be an invalid handle. The HANDLE's value is its value in
_OpenConsole_, not in conhost.

To remedy this, we need to:
* Just not forward `ConsoleSetForeground`. Turns out, we _can_ just call
  that in OpenConsole safely. There's no validation. So just instantiate
  a static version of the Win32 version of ConsoleControl, just to use
  for SetForeground. (thanks Dustin)

* [x] Tested manually - Win+R `powershell`, `notepad` spawns on top.

Closes #13211

(cherry picked from commit 2a7dd8b)
Service-Card-Id: 83026847
Service-Version: 1.14

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

@@ -264,11 +264,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);
                 }
             }
 
@@ -1764,4 +1764,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

@@ -176,10 +176,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());
@@ -249,6 +247,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

@@ -536,7 +536,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

@@ -94,17 +94,19 @@ T HostSignalInputThread::_ReceiveTypedPacket()
         }
         case HostSignals::SetForeground:
         {
-            auto msg = _ReceiveTypedPacket<HostSignalSetForegroundData>();
+            _ReceiveTypedPacket<HostSignalSetForegroundData>();
 
-            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;
         }
         case HostSignals::EndTask:
         {
             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

@@ -57,19 +57,17 @@ template<typename T>
 
 [[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;
 

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

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