folkslinux
/
windows-terminal
mirror of https://github.com/microsoft/terminal.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Avoid focus loops in ConPTY

This commit is contained in:
Leonard Hecker 2024-08-30 01:18:05 +02:00
parent 17a55da0f9
commit a3c12bf3da
13 changed files with 131 additions and 120 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.github/actions/spelling/allow/apis.txt vendored
View File

@ -96,6 +96,7 @@ IGraphics
IImage
IInheritable
IMap
imm
IMonarch
IObject
iosfwd

23
src/cascadia/TerminalControl/ControlCore.cpp
View File

@ -201,6 +201,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation
}
});
// If you rapidly show/hide Windows Terminal, something about GotFocus()/LostFocus() gets broken.
// We'll then receive easily 10+ such calls from WinUI the next time the application is shown.
shared->focusChanged = std::make_unique<til::debounced_func_trailing<bool>>(
std::chrono::milliseconds{ 25 },
[weakThis = get_weak()](const bool focused) {
if (const auto core{ weakThis.get() })
{
core->_focusChanged(focused);
}
});
// Scrollbar updates are also expensive (XAML), so we'll throttle them as well.
shared->updateScrollBar = std::make_shared<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>>(
_dispatcher,
@ -2480,13 +2491,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation
// - This is related to work done for GH#2988.
void ControlCore::GotFocus()
{
_focusChanged(true);
const auto shared = _shared.lock_shared();
if (shared->focusChanged)
{
(*shared->focusChanged)(true);
}
}
// See GotFocus.
void ControlCore::LostFocus()
{
_focusChanged(false);
const auto shared = _shared.lock_shared();
if (shared->focusChanged)
{
(*shared->focusChanged)(false);
}
}
void ControlCore::_focusChanged(bool focused)

1
src/cascadia/TerminalControl/ControlCore.h
View File

@ -307,6 +307,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
struct SharedState
{
std::unique_ptr<til::debounced_func_trailing<>> outputIdle;
std::unique_ptr<til::debounced_func_trailing<bool>> focusChanged;
std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> updateScrollBar;
};

46
src/host/PtySignalInputThread.cpp
View File

@ -74,8 +74,6 @@ void PtySignalInputThread::ConnectConsole() noexcept
{
_DoShowHide(*_initialShowHide);
}
// We should have successfully used the _earlyReparent message in CreatePseudoWindow.
}
// Method Description:
@ -87,8 +85,7 @@ void PtySignalInputThread::ConnectConsole() noexcept
// - Refer to GH#13066 for details.
void PtySignalInputThread::CreatePseudoWindow()
{
HWND owner = _earlyReparent.has_value() ? reinterpret_cast<HWND>((*_earlyReparent).handle) : HWND_DESKTOP;
ServiceLocator::LocatePseudoWindow(owner);
ServiceLocator::LocatePseudoWindow();
}
// Method Description:
@ -220,7 +217,7 @@ void PtySignalInputThread::_DoShowHide(const ShowHideData& data)
return;
}
_api.ShowWindow(data.show);
ServiceLocator::SetPseudoWindowVisibility(data.show);
}
// Method Description:
@ -237,44 +234,7 @@ void PtySignalInputThread::_DoSetWindowParent(const SetParentData& data)
LockConsole();
auto Unlock = wil::scope_exit([&] { UnlockConsole(); });
// If the client app hasn't yet connected, stash the new owner.
// We'll later (PtySignalInputThread::ConnectConsole) use the value
// to set up the owner of the conpty window.
if (!_consoleConnected)
{
_earlyReparent = data;
return;
}
const auto owner{ reinterpret_cast<HWND>(data.handle) };
// This will initialize s_interactivityFactory for us. It will also
// conveniently return 0 when we're on OneCore.
//
// If the window hasn't been created yet, by some other call to
// LocatePseudoWindow, then this will also initialize the owner of the
// window.
if (const auto pseudoHwnd{ ServiceLocator::LocatePseudoWindow(owner) })
{
// SetWindowLongPtrW may call back into the message handler and wait for it to finish,
// similar to SendMessageW(). If the conhost message handler is already processing and
// waiting to acquire the console lock, which we're currently holding, we'd deadlock.
// --> Release the lock now.
Unlock.reset();
// DO NOT USE SetParent HERE!
//
// Calling SetParent on a window that is WS_VISIBLE will cause the OS to
// hide the window, make it a _child_ window, then call SW_SHOW on the
// window to re-show it. SW_SHOW, however, will cause the OS to also set
// that window as the _foreground_ window, which would result in the
// pty's hwnd stealing the foreground away from the owning terminal
// window. That's bad.
//
// SetWindowLongPtr seems to do the job of changing who the window owner
// is, without all the other side effects of reparenting the window.
// See #13066
::SetWindowLongPtrW(pseudoHwnd, GWLP_HWNDPARENT, reinterpret_cast<LONG_PTR>(owner));
}
ServiceLocator::SetPseudoWindowOwner(reinterpret_cast<HWND>(data.handle));
}
// Method Description:

3
src/host/PtySignalInputThread.hpp
View File

@ -75,8 +75,5 @@ namespace Microsoft::Console
std::optional<ResizeWindowData> _earlyResize;
std::optional<ShowHideData> _initialShowHide;
ConhostInternalGetSet _api;
public:
std::optional<SetParentData> _earlyReparent;
};
}

2
src/host/exe/Host.EXE.vcxproj
View File

@ -86,7 +86,7 @@
</ClCompile>
<Link>
<AllowIsolation>true</AllowIsolation>
<AdditionalDependencies>WinMM.Lib;%(AdditionalDependencies)</AdditionalDependencies>
<AdditionalDependencies>winmm.lib;imm32.lib;%(AdditionalDependencies)</AdditionalDependencies>
</Link>
</ItemDefinitionGroup>
<!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->

11
src/host/outputStream.cpp
View File

@ -208,8 +208,14 @@ CursorType ConhostInternalGetSet::GetUserDefaultCursorStyle() const
// - <none>
void ConhostInternalGetSet::ShowWindow(bool showOrHide)
{
auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation();
const auto hwnd = gci.IsInVtIoMode() ? ServiceLocator::LocatePseudoWindow() : ServiceLocator::LocateConsoleWindow()->GetWindowHandle();
// ConPTY is supposed to be "transparent" to the VT application. Any VT it processes is given to the terminal.
// As such, it must not react to this "CSI 1 t" or "CSI 2 t" sequence. That's the job of the terminal.
// If the terminal encounters such a sequence, it can show/hide itself and let ConPTY know via its signal API.
const auto window = ServiceLocator::LocateConsoleWindow();
if (!window)
{
return;
}
// GH#13301 - When we send this ShowWindow message, if we send it to the
// conhost HWND, it's going to need to get processed by the window message
@ -217,6 +223,7 @@ void ConhostInternalGetSet::ShowWindow(bool showOrHide)
// However, ShowWindowAsync doesn't have this problem. It'll post the
// message to the window thread, then immediately return, so we don't have
// to worry about deadlocking.
const auto hwnd = window->GetWindowHandle();
::ShowWindowAsync(hwnd, showOrHide ? SW_SHOWNOACTIVATE : SW_MINIMIZE);
}

60
src/interactivity/base/InteractivityFactory.cpp
View File

@ -294,7 +294,7 @@ using namespace Microsoft::Console::Interactivity;
// - owner: the HWND that should be the initial owner of the pseudo window.
// Return Value:
// - STATUS_SUCCESS on success, otherwise an appropriate error.
[[nodiscard]] NTSTATUS InteractivityFactory::CreatePseudoWindow(HWND& hwnd, const HWND owner)
[[nodiscard]] NTSTATUS InteractivityFactory::CreatePseudoWindow(HWND& hwnd)
{
hwnd = nullptr;
ApiLevel level;
@ -310,6 +310,11 @@ using namespace Microsoft::Console::Interactivity;
{
case ApiLevel::Win32:
{
// We don't need an "Default IME" window for ConPTY. That's the terminal's job.
// -1, aka DWORD_MAX, tells the function to disable it for the entire process.
// Must be called before creating any window.
ImmDisableIME(DWORD_MAX);
pseudoClass.cbSize = sizeof(WNDCLASSEXW);
pseudoClass.lpszClassName = PSEUDO_WINDOW_CLASS;
pseudoClass.lpfnWndProc = s_PseudoWindowProc;
@ -330,19 +335,15 @@ using namespace Microsoft::Console::Interactivity;
// will return the console handle again, not the owning
// terminal's handle. It's not entirely clear why, but WS_POPUP
// is absolutely vital for this to work correctly.
const auto windowStyle = WS_OVERLAPPEDWINDOW | WS_POPUP;
const auto exStyles = WS_EX_TOOLWINDOW | WS_EX_TRANSPARENT | WS_EX_LAYERED | WS_EX_NOACTIVATE;
// Attempt to create window.
hwnd = CreateWindowExW(exStyles,
hwnd = CreateWindowExW(WS_EX_TOOLWINDOW | WS_EX_NOACTIVATE,
reinterpret_cast<LPCWSTR>(windowClassAtom),
nullptr,
windowStyle,
WS_POPUP,
0,
0,
0,
0,
owner,
_owner.load(std::memory_order_relaxed),
nullptr,
nullptr,
this);
@ -375,6 +376,38 @@ using namespace Microsoft::Console::Interactivity;
return status;
}
void InteractivityFactory::SetOwner(HWND owner) noexcept
{
_owner.store(owner, std::memory_order_relaxed);
if (_pseudoConsoleWindowHwnd)
{
// DO NOT USE SetParent HERE!
//
// Calling SetParent on a window that is WS_VISIBLE will cause the OS to
// hide the window, make it a _child_ window, then call SW_SHOW on the
// window to re-show it. SW_SHOW, however, will cause the OS to also set
// that window as the _foreground_ window, which would result in the
// pty's hwnd stealing the foreground away from the owning terminal
// window. That's bad.
//
// SetWindowLongPtr seems to do the job of changing who the window owner
// is, without all the other side effects of reparenting the window.
// See #13066
::SetWindowLongPtrW(_pseudoConsoleWindowHwnd, GWLP_HWNDPARENT, reinterpret_cast<LONG_PTR>(owner));
}
}
void InteractivityFactory::SetVisibility(const bool isVisible) noexcept
{
if (_pseudoConsoleWindowHwnd && IsIconic(_pseudoConsoleWindowHwnd) != static_cast<BOOL>(isVisible))
{
_suppressVisibilityChange.store(true, std::memory_order_relaxed);
ShowWindow(_pseudoConsoleWindowHwnd, isVisible ? SW_SHOWNOACTIVATE : SW_MINIMIZE);
_suppressVisibilityChange.store(false, std::memory_order_relaxed);
}
}
// Method Description:
// - Static window procedure for pseudo console windows
// - Processes set-up on create to stow the "this" pointer to specific instantiations and routes
@ -446,7 +479,7 @@ using namespace Microsoft::Console::Interactivity;
{
_WritePseudoWindowCallback(false);
}
break;
return 0;
}
// case WM_WINDOWPOSCHANGING:
// As long as user32 didn't eat the `ShowWindow` call because the window state requested
@ -479,12 +512,12 @@ using namespace Microsoft::Console::Interactivity;
}
case WM_ACTIVATE:
{
if (const auto ownerHwnd{ ::GetAncestor(hWnd, GA_ROOTOWNER) })
if (const auto ownerHwnd = _owner.load(std::memory_order_relaxed))
{
SetFocus(ownerHwnd);
return 0;
}
break;
return 0;
}
}
// If we get this far, call the default window proc
@ -501,6 +534,11 @@ using namespace Microsoft::Console::Interactivity;
// - <none>
void InteractivityFactory::_WritePseudoWindowCallback(bool showOrHide)
{
if (_suppressVisibilityChange.load(std::memory_order_relaxed))
{
return;
}
// IMPORTANT!
//
// A hosting terminal window should only "restore" itself in response to

7
src/interactivity/base/InteractivityFactory.hpp
View File

@ -27,7 +27,10 @@ namespace Microsoft::Console::Interactivity
[[nodiscard]] NTSTATUS CreateAccessibilityNotifier(_Inout_ std::unique_ptr<IAccessibilityNotifier>& notifier);
[[nodiscard]] NTSTATUS CreateSystemConfigurationProvider(_Inout_ std::unique_ptr<ISystemConfigurationProvider>& provider);
[[nodiscard]] NTSTATUS CreatePseudoWindow(HWND& hwnd, const HWND owner);
[[nodiscard]] NTSTATUS CreatePseudoWindow(HWND& hwnd);
void SetOwner(HWND owner) noexcept;
void SetVisibility(const bool isVisible) noexcept;
// Wndproc
[[nodiscard]] static LRESULT CALLBACK s_PseudoWindowProc(_In_ HWND hwnd,
@ -43,6 +46,8 @@ namespace Microsoft::Console::Interactivity
void _WritePseudoWindowCallback(bool showOrHide);
HWND _pseudoConsoleWindowHwnd{ nullptr };
std::atomic<HWND> _owner{ HWND_DESKTOP };
std::atomic<bool> _suppressVisibilityChange{ false };
WRL::ComPtr<PseudoConsoleWindowAccessibilityProvider> _pPseudoConsoleUiaProvider{ nullptr };
};
}

36
src/interactivity/base/ServiceLocator.cpp
View File

@ -322,7 +322,7 @@ Globals& ServiceLocator::LocateGlobals()
// owner of the pseudo window.
// Return Value:
// - a reference to the pseudoconsole window.
HWND ServiceLocator::LocatePseudoWindow(const HWND owner)
HWND ServiceLocator::LocatePseudoWindow()
{
auto status = STATUS_SUCCESS;
if (!s_pseudoWindowInitialized)
@ -335,7 +335,7 @@ HWND ServiceLocator::LocatePseudoWindow(const HWND owner)
if (SUCCEEDED_NTSTATUS(status))
{
HWND hwnd;
status = s_interactivityFactory->CreatePseudoWindow(hwnd, owner);
status = s_interactivityFactory->CreatePseudoWindow(hwnd);
s_pseudoWindow.reset(hwnd);
}
@ -345,6 +345,38 @@ HWND ServiceLocator::LocatePseudoWindow(const HWND owner)
return s_pseudoWindow.get();
}
void ServiceLocator::SetPseudoWindowOwner(HWND owner)
{
auto status = STATUS_SUCCESS;
if (!s_interactivityFactory)
{
status = ServiceLocator::LoadInteractivityFactory();
}
if (s_interactivityFactory)
{
static_cast<InteractivityFactory*>(s_interactivityFactory.get())->SetOwner(owner);
}
LOG_IF_NTSTATUS_FAILED(status);
}
void ServiceLocator::SetPseudoWindowVisibility(bool showOrHide)
{
auto status = STATUS_SUCCESS;
if (!s_interactivityFactory)
{
status = ServiceLocator::LoadInteractivityFactory();
}
if (s_interactivityFactory)
{
static_cast<InteractivityFactory*>(s_interactivityFactory.get())->SetVisibility(showOrHide);
}
LOG_IF_NTSTATUS_FAILED(status);
}
#pragma endregion
#pragma region Private Methods

2
src/interactivity/inc/IInteractivityFactory.hpp
View File

@ -40,6 +40,6 @@ namespace Microsoft::Console::Interactivity
[[nodiscard]] virtual NTSTATUS CreateAccessibilityNotifier(_Inout_ std::unique_ptr<IAccessibilityNotifier>& notifier) = 0;
[[nodiscard]] virtual NTSTATUS CreateSystemConfigurationProvider(_Inout_ std::unique_ptr<ISystemConfigurationProvider>& provider) = 0;
[[nodiscard]] virtual NTSTATUS CreatePseudoWindow(HWND& hwnd, const HWND owner) = 0;
[[nodiscard]] virtual NTSTATUS CreatePseudoWindow(HWND& hwnd) = 0;
};
}

4
src/interactivity/inc/ServiceLocator.hpp
View File

@ -84,7 +84,9 @@ namespace Microsoft::Console::Interactivity
static Globals& LocateGlobals();
static HWND LocatePseudoWindow(const HWND owner = nullptr /*HWND_DESKTOP = 0*/);
static HWND LocatePseudoWindow();
static void SetPseudoWindowOwner(HWND owner);
static void SetPseudoWindowVisibility(bool showOrHide);
ServiceLocator(const ServiceLocator&) = delete;
ServiceLocator& operator=(const ServiceLocator&) = delete;

55
src/terminal/adapter/InteractDispatch.cpp
View File

@ -162,59 +162,8 @@ void InteractDispatch::FocusChanged(const bool focused)
// InteractDispatch outside ConPTY mode, but just in case...
if (gci.IsInVtIoMode())
{
auto shouldActuallyFocus = false;
// From https://github.com/microsoft/terminal/pull/12799#issuecomment-1086289552
// Make sure that the process that's telling us it's focused, actually
// _is_ in the FG. We don't want to allow malicious.exe to say "yep I'm
// in the foreground, also, here's a popup" if it isn't actually in the
// FG.
if (focused)
{
if (const auto pseudoHwnd{ ServiceLocator::LocatePseudoWindow() })
{
// They want focus, we found a pseudo hwnd.
// BODGY
//
// This needs to be GA_ROOTOWNER here. Not GA_ROOT, GA_PARENT,
// or GetParent. The ConPTY hwnd is an owned, top-level, popup,
// non-parented window. It does not have a parent set. It does
// have an owner set. It is not a WS_CHILD window. This
// combination of things allows us to find the owning window
// with GA_ROOTOWNER. GA_ROOT will get us ourselves, and
// GA_PARENT will return the desktop HWND.
//
// See GH#13066
if (const auto ownerHwnd{ ::GetAncestor(pseudoHwnd, GA_ROOTOWNER) })
{
// We have an owner from a previous call to ReparentWindow
if (const auto currentFgWindow{ ::GetForegroundWindow() })
{
// There is a window in the foreground (it's possible there
// isn't one)
// Get the PID of the current FG window, and compare with our owner's PID.
DWORD currentFgPid{ 0 };
DWORD ownerPid{ 0 };
GetWindowThreadProcessId(currentFgWindow, &currentFgPid);
GetWindowThreadProcessId(ownerHwnd, &ownerPid);
if (ownerPid == currentFgPid)
{
// Huzzah, the app that owns us is actually the FG
// process. They're allowed to grand FG rights.
shouldActuallyFocus = true;
}
}
}
}
}
WI_UpdateFlag(gci.Flags, CONSOLE_HAS_FOCUS, shouldActuallyFocus);
gci.ProcessHandleList.ModifyConsoleProcessFocus(shouldActuallyFocus);
WI_UpdateFlag(gci.Flags, CONSOLE_HAS_FOCUS, focused);
gci.ProcessHandleList.ModifyConsoleProcessFocus(focused);
gci.pInputBuffer->WriteFocusEvent(focused);
}
// Does nothing outside of ConPTY. If there's a real HWND, then the HWND is solely in charge.