Revert "ConPTY: Emit DSR CPR on resize (#19089)" (#19237)

This reverts commit c55aca508beadebc080b0c76b7322de80a294968
because it is unaware of the VT state and may inject the DSR CPR
while e.g. a DCS is going on.

Reopens #18725

src/host/PtySignalInputThread.cpp

@@ -184,9 +184,6 @@ void PtySignalInputThread::_DoResizeWindow(const ResizeWindowData& data)
     }
 
     _api.ResizeWindow(data.sx, data.sy);
-
-    auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
-    gci.GetVtIo()->RequestCursorPositionFromTerminal();
 }
 
 void PtySignalInputThread::_DoClearBuffer(const bool keepCursorRow) const

src/host/VtInputThread.cpp

@@ -21,13 +21,13 @@ using namespace Microsoft::Console::VirtualTerminal;
 // - hPipe - a handle to the file representing the read end of the VT pipe.
 // - inheritCursor - a bool indicating if the state machine should expect a
 //      cursor positioning sequence. See MSFT:15681311.
-VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, std::function<void()> capturedCPR) :
+VtInputThread::VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor) :
     _hFile{ std::move(hPipe) }
 {
     THROW_HR_IF(E_HANDLE, _hFile.get() == INVALID_HANDLE_VALUE);
 
     auto dispatch = std::make_unique<InteractDispatch>();
-    auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), std::move(capturedCPR));
+    auto engine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), inheritCursor);
     _pInputStateMachine = std::make_unique<StateMachine>(std::move(engine));
 }
 
@@ -185,7 +185,8 @@ void VtInputThread::_InputThread()
     return S_OK;
 }
 
-InputStateMachineEngine& VtInputThread::GetInputStateMachineEngine() const noexcept
+til::enumset<DeviceAttribute, uint64_t> VtInputThread::WaitUntilDA1(DWORD timeout) const noexcept
 {
-    return static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
+    const auto& engine = static_cast<InputStateMachineEngine&>(_pInputStateMachine->Engine());
+    return engine.WaitUntilDA1(timeout);
 }

src/host/VtInputThread.hpp

@@ -20,17 +20,16 @@ namespace Microsoft::Console
 {
     namespace VirtualTerminal
     {
-        class InputStateMachineEngine;
         enum class DeviceAttribute : uint64_t;
     }
 
     class VtInputThread
     {
     public:
-        VtInputThread(_In_ wil::unique_hfile hPipe, std::function<void()> capturedCPR = nullptr);
+        VtInputThread(_In_ wil::unique_hfile hPipe, const bool inheritCursor);
 
         [[nodiscard]] HRESULT Start();
-        VirtualTerminal::InputStateMachineEngine& GetInputStateMachineEngine() const noexcept;
+        til::enumset<VirtualTerminal::DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
 
     private:
         static DWORD WINAPI StaticVtInputThreadProc(_In_ LPVOID lpParameter);

src/host/VtIo.cpp

@@ -11,7 +11,6 @@
 #include "output.h" // CloseConsoleProcessState
 #include "../interactivity/inc/ServiceLocator.hpp"
 #include "../renderer/base/renderer.hpp"
-#include "../terminal/parser/InputStateMachineEngine.hpp"
 #include "../types/inc/CodepointWidthDetector.hpp"
 #include "../types/inc/utils.hpp"
 
@@ -156,9 +155,7 @@ bool VtIo::IsUsingVt() const
     {
         if (IsValidHandle(_hInput.get()))
         {
-            _pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput), [this]() {
+            _pVtInputThread = std::make_unique<VtInputThread>(std::move(_hInput), _lookingForCursorPosition);
-                _cursorPositionReportReceived();
-            });
         }
     }
     CATCH_RETURN();
@@ -180,7 +177,6 @@ bool VtIo::IsUsingVt() const
             // wait for the DA1 response below and effectively wait for both.
             if (_lookingForCursorPosition)
             {
-                _pVtInputThread->GetInputStateMachineEngine().CaptureNextCPR();
                 writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR)
             }
 
@@ -201,7 +197,7 @@ bool VtIo::IsUsingVt() const
             // Allow the input thread to momentarily gain the console lock.
             auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
             const auto suspension = gci.SuspendLock();
-            _deviceAttributes = _pVtInputThread->GetInputStateMachineEngine().WaitUntilDA1(3000);
+            _deviceAttributes = _pVtInputThread->WaitUntilDA1(3000);
         }
     }
 
@@ -252,35 +248,6 @@ void VtIo::Shutdown() noexcept
     CATCH_LOG();
 }
 
-void VtIo::RequestCursorPositionFromTerminal()
-{
-    if (_lookingForCursorPosition)
-    {
-        // By delaying sending another DSR CPR until we received a response to the previous one,
-        // we debounce our requests to the terminal. We don't want to flood it unnecessarily.
-        _scheduleAnotherCPR = true;
-        return;
-    }
-
-    _lookingForCursorPosition = true;
-    _pVtInputThread->GetInputStateMachineEngine().CaptureNextCPR();
-
-    Writer writer{ this };
-    writer.WriteUTF8("\x1b[6n"); // Cursor Position Report (DSR CPR)
-    writer.Submit();
-}
-
-void VtIo::_cursorPositionReportReceived()
-{
-    _lookingForCursorPosition = false;
-
-    if (_scheduleAnotherCPR)
-    {
-        _scheduleAnotherCPR = false;
-        RequestCursorPositionFromTerminal();
-    }
-}
-
 void VtIo::SetDeviceAttributes(const til::enumset<DeviceAttribute, uint64_t> attributes) noexcept
 {
     _deviceAttributes = attributes;

src/host/VtIo.hpp

@@ -61,7 +61,6 @@ namespace Microsoft::Console::VirtualTerminal
         [[nodiscard]] HRESULT StartIfNeeded();
         void Shutdown() noexcept;
 
-        void RequestCursorPositionFromTerminal();
         void SetDeviceAttributes(til::enumset<DeviceAttribute, uint64_t> attributes) noexcept;
         til::enumset<DeviceAttribute, uint64_t> GetDeviceAttributes() const noexcept;
         void SendCloseEvent();
@@ -78,7 +77,7 @@ namespace Microsoft::Console::VirtualTerminal
         };
 
         [[nodiscard]] HRESULT _Initialize(const HANDLE InHandle, const HANDLE OutHandle, _In_opt_ const HANDLE SignalHandle);
-        void _cursorPositionReportReceived();
+
         void _uncork();
         void _flushNow();
 
@@ -106,7 +105,6 @@ namespace Microsoft::Console::VirtualTerminal
 
         State _state = State::Uninitialized;
         bool _lookingForCursorPosition = false;
-        bool _scheduleAnotherCPR = false;
         bool _closeEventSent = false;
         int _corked = 0;
 

src/terminal/parser/InputStateMachineEngine.cpp

@@ -89,24 +89,14 @@ static bool operator==(const Ss3ToVkey& pair, const Ss3ActionCodes code) noexcep
     return pair.action == code;
 }
 
-InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, std::function<void()> capturedCPR) :
+InputStateMachineEngine::InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, const bool lookingForDSR) :
-    _pDispatch{ std::move(pDispatch) },
+    _pDispatch(std::move(pDispatch)),
-    _capturedCPR{ std::move(capturedCPR) },
+    _lookingForDSR(lookingForDSR),
-    _doubleClickTime{ std::chrono::milliseconds(GetDoubleClickTime()) }
+    _doubleClickTime(std::chrono::milliseconds(GetDoubleClickTime()))
 {
     THROW_HR_IF_NULL(E_INVALIDARG, _pDispatch.get());
 }
 
-IInteractDispatch& InputStateMachineEngine::GetDispatch() const noexcept
-{
-    return *_pDispatch.get();
-}
-
-void InputStateMachineEngine::CaptureNextCPR() noexcept
-{
-    _lookingForCPR = true;
-}
-
 til::enumset<DeviceAttribute, uint64_t> InputStateMachineEngine::WaitUntilDA1(DWORD timeout) const noexcept
 {
     uint64_t val = 0;
@@ -423,19 +413,13 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
         // The F3 case is special - it shares a code with the DeviceStatusResponse.
         // If we're looking for that response, then do that, and break out.
         // Else, fall though to the _GetCursorKeysModifierState handler.
-        if (_lookingForCPR)
+        if (_lookingForDSR)
         {
-            _lookingForCPR = false;
+            _pDispatch->MoveCursor(parameters.at(0), parameters.at(1));
-            _capturedCPR();
+            // Right now we're only looking for on initial cursor
-
+            //      position response. After that, only look for F3.
-            const auto y = parameters.at(0).value();
+            _lookingForDSR = false;
-            const auto x = parameters.at(1).value();
+            return true;
-
-            if (y > 0 && x > 0)
-            {
-                _pDispatch->MoveCursor(y, x);
-                return true;
-            }
         }
         // Heuristic: If the hosting terminal used the win32 input mode, chances are high
         // that this is a CPR requested by the terminal application as opposed to a F3 key.
@@ -507,6 +491,10 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
 
             _deviceAttributes.fetch_or(attributes.bits(), std::memory_order_relaxed);
             til::atomic_notify_all(_deviceAttributes);
+
+            // VtIo first sends a DSR CPR and then a DA1 request.
+            // If we encountered a DA1 response here, the DSR request is definitely done now.
+            _lookingForDSR = false;
             return true;
         }
         return false;

src/terminal/parser/InputStateMachineEngine.hpp

@@ -159,10 +159,8 @@ namespace Microsoft::Console::VirtualTerminal
     class InputStateMachineEngine : public IStateMachineEngine
     {
     public:
-        InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, std::function<void()> capturedCPR = nullptr);
+        InputStateMachineEngine(std::unique_ptr<IInteractDispatch> pDispatch, const bool lookingForDSR = false);
 
-        IInteractDispatch& GetDispatch() const noexcept;
-        void CaptureNextCPR() noexcept;
         til::enumset<DeviceAttribute, uint64_t> WaitUntilDA1(DWORD timeout) const noexcept;
 
         bool EncounteredWin32InputModeSequence() const noexcept override;
@@ -191,8 +189,7 @@ namespace Microsoft::Console::VirtualTerminal
     private:
         const std::unique_ptr<IInteractDispatch> _pDispatch;
         std::atomic<uint64_t> _deviceAttributes{ 0 };
-        bool _lookingForCPR = false;
+        bool _lookingForDSR = false;
-        std::function<void()> _capturedCPR;
         bool _encounteredWin32InputModeSequence = false;
         bool _expectingStringTerminator = false;
         DWORD _mouseButtonState = 0;

src/terminal/parser/ut_parser/InputEngineTest.cpp

@@ -2,26 +2,39 @@
 // Licensed under the MIT license.
 
 #include "precomp.h"
+#include "WexTestClass.h"
+#include "../../inc/consoletaeftemplates.hpp"
 
-#include <ascii.hpp>
+#include "stateMachine.hpp"
-#include <consoletaeftemplates.hpp>
+#include "InputStateMachineEngine.hpp"
-#include <InputStateMachineEngine.hpp>
+#include "ascii.hpp"
-#include <stateMachine.hpp>
+#include "../input/terminalInput.hpp"
-#include <unicode.hpp>
+#include "../../inc/unicode.hpp"
+#include "../../interactivity/inc/EventSynthesis.hpp"
+
+#include <vector>
+#include <functional>
+#include <sstream>
+#include <string>
+#include <algorithm>
 
-#include "../../../interactivity/inc/EventSynthesis.hpp"
 #include "../../../interactivity/inc/VtApiRedirection.hpp"
-#include "../../input/terminalInput.hpp"
 
 using namespace WEX::Common;
 using namespace WEX::Logging;
 using namespace WEX::TestExecution;
 
-namespace Microsoft::Console::VirtualTerminal
+namespace Microsoft
 {
-    class InputEngineTest;
+    namespace Console
-    class TestInteractDispatch;
+    {
-}
+        namespace VirtualTerminal
+        {
+            class InputEngineTest;
+            class TestInteractDispatch;
+        };
+    };
+};
 using namespace Microsoft::Console::VirtualTerminal;
 
 bool IsShiftPressed(const DWORD modifierState)
@@ -400,6 +413,7 @@ void InputEngineTest::C0Test()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     Log::Comment(L"Sending 0x0-0x19 to parser to make sure they're translated correctly back to C-key");
@@ -499,6 +513,7 @@ void InputEngineTest::AlphanumericTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     Log::Comment(L"Sending every printable ASCII character");
@@ -548,6 +563,7 @@ void InputEngineTest::RoundTripTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     // Send Every VKEY through the TerminalInput module, then take the char's
@@ -610,6 +626,7 @@ void InputEngineTest::NonAsciiTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine.get());
     testState._stateMachine = _stateMachine.get();
     Log::Comment(L"Sending various non-ascii strings, and seeing what we get out");
 
@@ -662,9 +679,11 @@ void InputEngineTest::CursorPositioningTest()
     auto pfn = std::bind(&TestState::TestInputCallback, &testState, std::placeholders::_1);
 
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
-    auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), []() {});
+    VERIFY_IS_NOT_NULL(dispatch.get());
-    inputEngine->CaptureNextCPR();
+    auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch), true);
+    VERIFY_IS_NOT_NULL(inputEngine.get());
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     Log::Comment(NoThrowString().Format(
@@ -705,6 +724,7 @@ void InputEngineTest::CSICursorBackTabTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     INPUT_RECORD inputRec;
@@ -732,6 +752,7 @@ void InputEngineTest::EnhancedKeysTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     // The following vkeys should be handled as enhanced keys
@@ -781,6 +802,7 @@ void InputEngineTest::SS3CursorKeyTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     // clang-format off
@@ -824,6 +846,7 @@ void InputEngineTest::AltBackspaceTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     INPUT_RECORD inputRec;
@@ -851,6 +874,7 @@ void InputEngineTest::AltCtrlDTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     INPUT_RECORD inputRec;
@@ -899,6 +923,7 @@ void InputEngineTest::AltIntermediateTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfnInputStateMachineCallback, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(stateMachine);
     testState._stateMachine = stateMachine.get();
 
     // Write a Alt+/, Ctrl+e pair to the input engine, then take its output and
@@ -930,6 +955,7 @@ void InputEngineTest::AltBackspaceEnterTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     INPUT_RECORD inputRec;
@@ -1025,6 +1051,7 @@ void InputEngineTest::VerifySGRMouseData(const std::vector<std::tuple<SGR_PARAMS
     // Let's force it to a high value to make the double click tests pass.
     inputEngine->_doubleClickTime = std::chrono::milliseconds(1000);
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     SGR_PARAMS input;
@@ -1255,6 +1282,7 @@ void InputEngineTest::CtrlAltZCtrlAltXTest()
     auto dispatch = std::make_unique<TestInteractDispatch>(pfn, &testState);
     auto inputEngine = std::make_unique<InputStateMachineEngine>(std::move(dispatch));
     auto _stateMachine = std::make_unique<StateMachine>(std::move(inputEngine));
+    VERIFY_IS_NOT_NULL(_stateMachine);
     testState._stateMachine = _stateMachine.get();
 
     // This is a test for GH#4201. See that issue for more details.