From e168413e9fdf4313af208fee2d7f15afc1c781a6 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 10 Aug 2021 10:25:29 -0500 Subject: [PATCH 01/53] I think this merges `the-whole-thing` into this branch. The remote control doesn't render right, but I think that's because the actual HEAD of all this work is in `connection-factory` --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 146 +++++++++++++++++- scratch/ScratchIslandApp/SampleApp/MyPage.h | 3 + .../ScratchIslandApp/SampleApp/MyPage.xaml | 6 +- .../TerminalControl/ContentProcess.cpp | 60 +++++++ src/cascadia/TerminalControl/ContentProcess.h | 28 ++++ .../TerminalControl/ContentProcess.idl | 19 +++ src/cascadia/TerminalControl/TermControl.cpp | 24 ++- src/cascadia/TerminalControl/TermControl.h | 6 + src/cascadia/TerminalControl/TermControl.idl | 5 + .../TerminalControlLib.vcxproj | 7 + src/cascadia/WindowsTerminal/main.cpp | 101 ++++++++++++ 11 files changed, 399 insertions(+), 6 deletions(-) create mode 100644 src/cascadia/TerminalControl/ContentProcess.cpp create mode 100644 src/cascadia/TerminalControl/ContentProcess.h create mode 100644 src/cascadia/TerminalControl/ContentProcess.idl diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index fbea8e334f1..a7f5b7cc968 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -3,9 +3,11 @@ #include "pch.h" #include "MyPage.h" +#include "MySettings.h" #include #include "MyPage.g.cpp" -#include "MySettings.h" +#include "..\..\..\src\cascadia\UnitTests_Control\MockControlSettings.h" +#include "..\..\..\src\types\inc\utils.hpp" using namespace std::chrono_literals; using namespace winrt::Microsoft::Terminal; @@ -26,7 +28,8 @@ namespace winrt::SampleApp::implementation void MyPage::Create() { - auto settings = winrt::make_self(); + TerminalConnection::EchoConnection conn{}; + auto settings = winrt::make_self(); auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted in-proc...", winrt::hstring{}, @@ -44,6 +47,145 @@ namespace winrt::SampleApp::implementation Control::TermControl control{ *settings, *settings, conn }; InProcContent().Children().Append(control); + + // Once the control loads (and not before that), write some text for debugging: + control.Initialized([conn](auto&&, auto&&) { + conn.WriteInput(L"This TermControl is hosted in-proc..."); + }); + } + + static wil::unique_process_information _createHostClassProcess(const winrt::guid& g) + { + auto guidStr{ ::Microsoft::Console::Utils::GuidToString(g) }; + std::wstring commandline{ fmt::format(L"windowsterminal.exe --content {}", guidStr) }; + STARTUPINFO siOne{ 0 }; + siOne.cb = sizeof(STARTUPINFOW); + wil::unique_process_information piOne; + auto succeeded = CreateProcessW( + nullptr, + commandline.data(), + nullptr, // lpProcessAttributes + nullptr, // lpThreadAttributes + false, // bInheritHandles + CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags + nullptr, // lpEnvironment + nullptr, // startingDirectory + &siOne, // lpStartupInfo + &piOne // lpProcessInformation + ); + THROW_IF_WIN32_BOOL_FALSE(succeeded); + // if (!succeeded) + // { + // printf("Failed to create host process\n"); + // return; + // } + + // Ooof this is dumb, but we need a sleep here to make the server starts. + // That's _sub par_. Maybe we could use the host's stdout to have them emit + // a byte when they're set up? + Sleep(2000); + + // TODO MONDAY - It seems like it takes conhost too long to start up to + // host the ScratchWinRTServer that even a sleep 100 is too short. However, + // any longer, and XAML will just crash, because some frame took too long. + // So we _need_ to do the "have the server explicitly tell us it's ready" + // thing, and maybe also do it on a bg thread (and signal to the UI thread + // that it can attach now) + + return std::move(piOne); + } + + winrt::fire_and_forget MyPage::CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs) + { + auto guidString = GuidInput().Text(); + + // Capture calling context. + winrt::apartment_context ui_thread; + co_await winrt::resume_background(); + + auto canConvert = guidString.size() == 38 && guidString.front() == '{' && guidString.back() == '}'; + bool attached = false; + winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; + + if (canConvert) + { + GUID result{}; + if (SUCCEEDED(IIDFromString(guidString.c_str(), &result))) + { + contentGuid = result; + attached = true; + } + } + + if (!attached) + { + // 2. Spawn a Server.exe, with the guid on the commandline + auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; + } + + Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + + TerminalConnection::ITerminalConnection conn{ nullptr }; + Control::IControlSettings settings{ nullptr }; + + settings = *winrt::make_self(); + + if (!attached) + { + conn = TerminalConnection::EchoConnection{}; + // settings = *winrt::make_self(); + content.Initialize(settings, conn); + } + + // Switch back to the UI thread. + co_await ui_thread; + + Control::TermControl control{ contentGuid, settings, conn }; + + OutOfProcContent().Children().Append(control); + + if (!attached) + { + auto guidStr{ ::Microsoft::Console::Utils::GuidToString(contentGuid) }; + GuidInput().Text(guidStr); + } + } + + /*winrt::fire_and_forget MyPage::_attachToContent(winrt::guid contentGuid) + { + Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + + }*/ + + winrt::fire_and_forget MyPage::CreateOutOfProcTerminal() + { + // 1. Generate a GUID. + winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; + + // Capture calling context. + winrt::apartment_context ui_thread; + co_await winrt::resume_background(); + + // 2. Spawn a Server.exe, with the guid on the commandline + auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; + + Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + + TerminalConnection::EchoConnection conn{}; + auto settings = winrt::make_self(); + Control::IControlSettings s = *settings; + + if (s) + { + content.Initialize(s, conn); + + // Switch back to the UI thread. + co_await ui_thread; + + Control::TermControl control{ contentGuid, s, conn }; + + OutOfProcContent().Children().Append(control); + } } // Method Description: diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.h b/scratch/ScratchIslandApp/SampleApp/MyPage.h index c16c02bb39c..94a4f3ac77b 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.h +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.h @@ -16,6 +16,9 @@ namespace winrt::SampleApp::implementation void Create(); hstring Title(); + winrt::fire_and_forget CreateOutOfProcTerminal(); + + winrt::fire_and_forget CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); private: friend struct MyPageT; // for Xaml to bind events diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml index 0c132dea21b..f132e66a5e8 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml @@ -23,7 +23,9 @@ - @@ -53,8 +55,6 @@ VerticalAlignment="Stretch" Background="#0000ff" /> - - diff --git a/src/cascadia/TerminalControl/ContentProcess.cpp b/src/cascadia/TerminalControl/ContentProcess.cpp new file mode 100644 index 00000000000..6f3de9a6842 --- /dev/null +++ b/src/cascadia/TerminalControl/ContentProcess.cpp @@ -0,0 +1,60 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "pch.h" +#include "ContentProcess.h" +#include "ContentProcess.g.cpp" + +namespace winrt::Microsoft::Terminal::Control::implementation +{ + ContentProcess::ContentProcess() {} + + void ContentProcess::Initialize(Control::IControlSettings settings, TerminalConnection::ITerminalConnection connection) + { + _interactivity = winrt::make(settings, connection); + } + + Control::ControlInteractivity ContentProcess::GetInteractivity() + { + return _interactivity; + } + + uint64_t ContentProcess::RequestSwapChainHandle(const uint64_t pid) + { + auto ourPid = GetCurrentProcessId(); + HANDLE ourHandle = reinterpret_cast(_interactivity.Core().SwapChainHandle()); + if (pid == ourPid) + { + return reinterpret_cast(ourHandle); + } + + wil::unique_handle hWindowProcess{ OpenProcess(PROCESS_ALL_ACCESS, + FALSE, + static_cast(pid)) }; + if (hWindowProcess.get() == nullptr) + { + const auto gle = GetLastError(); + gle; + // TODO! tracelog an error here + return 0; + } + + HANDLE theirHandle{ nullptr }; + BOOL success = DuplicateHandle(GetCurrentProcess(), + ourHandle, + hWindowProcess.get(), + &theirHandle, + 0, + FALSE, + DUPLICATE_SAME_ACCESS); + if (!success) + { + const auto gle = GetLastError(); + gle; + // TODO! tracelog an error here + return 0; + } + return reinterpret_cast(theirHandle); + } + +} diff --git a/src/cascadia/TerminalControl/ContentProcess.h b/src/cascadia/TerminalControl/ContentProcess.h new file mode 100644 index 00000000000..609d0b7c30e --- /dev/null +++ b/src/cascadia/TerminalControl/ContentProcess.h @@ -0,0 +1,28 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#pragma once + +#include "ContentProcess.g.h" +#include "ControlInteractivity.h" + +namespace winrt::Microsoft::Terminal::Control::implementation +{ + struct ContentProcess : ContentProcessT + + { + ContentProcess(); + void Initialize(Control::IControlSettings settings, TerminalConnection::ITerminalConnection connection); + Control::ControlInteractivity GetInteractivity(); + + uint64_t RequestSwapChainHandle(const uint64_t pid); + + private: + Control::ControlInteractivity _interactivity{ nullptr }; + }; +} + +namespace winrt::Microsoft::Terminal::Control::factory_implementation +{ + BASIC_FACTORY(ContentProcess); +} diff --git a/src/cascadia/TerminalControl/ContentProcess.idl b/src/cascadia/TerminalControl/ContentProcess.idl new file mode 100644 index 00000000000..8aa945bd7ed --- /dev/null +++ b/src/cascadia/TerminalControl/ContentProcess.idl @@ -0,0 +1,19 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import "ControlInteractivity.idl"; + +namespace Microsoft.Terminal.Control +{ + runtimeclass ContentProcess { + + ContentProcess(); + + void Initialize(IControlSettings settings, + Microsoft.Terminal.TerminalConnection.ITerminalConnection connection); + + ControlInteractivity GetInteractivity(); + + UInt64 RequestSwapChainHandle(UInt64 pid); + }; +} diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 818a10c9b4b..647d4442609 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -50,6 +50,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation TermControl::TermControl(IControlSettings settings, Control::IControlAppearance unfocusedAppearance, TerminalConnection::ITerminalConnection connection) : + TermControl(winrt::guid{}, settings, unfocusedAppearance, connection) {} + + TermControl::TermControl(winrt::guid contentGuid, + IControlSettings settings, + Control::IControlAppearance unfocusedAppearance, + TerminalConnection::ITerminalConnection connection) : + _initializedTerminal{ false }, + _settings{ settings }, + _closing{ false }, _isInternalScrollBarUpdate{ false }, _autoScrollVelocity{ 0 }, _autoScrollingPointerPoint{ std::nullopt }, @@ -61,7 +70,20 @@ namespace winrt::Microsoft::Terminal::Control::implementation { InitializeComponent(); - _interactivity = winrt::make(settings, unfocusedAppearance, connection); + if (contentGuid != winrt::guid{}) + { + _contentProc = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + if (_contentProc != nullptr) + { + _interactivity = _contentProc.GetInteractivity(); + } + } + + if (_interactivity == nullptr) + { + _interactivity = winrt::make(settings, unfocusedAppearance, connection); + } + _core = _interactivity.Core(); // These events might all be triggered by the connection, but that diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 95aa82cdb92..c37be8f3a92 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -29,6 +29,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation Control::IControlAppearance unfocusedAppearance, TerminalConnection::ITerminalConnection connection); + TermControl(winrt::guid contentGuid, + IControlSettings settings, + Control::IControlAppearance unfocusedAppearance, + TerminalConnection::ITerminalConnection connection); + winrt::fire_and_forget UpdateControlSettings(Control::IControlSettings settings); winrt::fire_and_forget UpdateControlSettings(Control::IControlSettings settings, Control::IControlAppearance unfocusedAppearance); IControlSettings Settings() const; @@ -148,6 +153,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation Control::TermControlAutomationPeer _automationPeer{ nullptr }; Control::ControlInteractivity _interactivity{ nullptr }; Control::ControlCore _core{ nullptr }; + Control::ContentProcess _contentProc{ nullptr }; winrt::com_ptr _searchBox; diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index 307dd965530..175dc94325d 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -16,6 +16,11 @@ namespace Microsoft.Terminal.Control IMouseWheelListener, ICoreState { + TermControl(Guid contentGuid, + IControlSettings settings, + IControlAppearance unfocusedAppearance, + Microsoft.Terminal.TerminalConnection.ITerminalConnection connection); + TermControl(IControlSettings settings, IControlAppearance unfocusedAppearance, Microsoft.Terminal.TerminalConnection.ITerminalConnection connection); diff --git a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj index 250cb06cbe9..3da4e391d5b 100644 --- a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj +++ b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj @@ -34,6 +34,9 @@ + + ContentProcess.idl + ControlCore.idl @@ -71,6 +74,9 @@ Create + + ContentProcess.idl + ControlCore.idl @@ -107,6 +113,7 @@ + diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index fe2353fbf07..e126257c567 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -96,6 +96,96 @@ static bool _messageIsAltSpaceKeypress(const MSG& message) return message.message == WM_SYSKEYDOWN && message.wParam == VK_SPACE; } +static bool checkIfContentProcess(winrt::guid& contentProcessGuid) +{ + std::vector args; + + if (auto commandline{ GetCommandLineW() }) + { + int argc = 0; + + // Get the argv, and turn them into a hstring array to pass to the app. + wil::unique_any argv{ CommandLineToArgvW(commandline, &argc) }; + if (argv) + { + for (auto& elem : wil::make_range(argv.get(), argc)) + { + args.emplace_back(elem); + } + } + } + if (args.size() > 2 && args.at(1) == L"--content") + { + auto& guidString{ args.at(2) }; + auto canConvert = guidString.length() == 38 && guidString.front() == '{' && guidString.back() == '}'; + if (canConvert) + { + GUID result{}; + THROW_IF_FAILED(IIDFromString(guidString.c_str(), &result)); + contentProcessGuid = result; + return true; + } + } + return false; +} + +std::mutex m; +std::condition_variable cv; +bool dtored = false; +winrt::weak_ref g_weak{ nullptr }; + +struct HostClassFactory : implements +{ + HostClassFactory(winrt::guid g) : + _guid{ g } {}; + + HRESULT __stdcall CreateInstance(IUnknown* outer, GUID const& iid, void** result) noexcept final + { + *result = nullptr; + if (outer) + { + return CLASS_E_NOAGGREGATION; + } + + if (!g_weak) + { + winrt::Microsoft::Terminal::Control::ContentProcess strong{}; // = winrt::make(); + winrt::weak_ref weak{ strong }; + g_weak = weak; + return strong.as(iid, result); + } + else + { + auto strong = g_weak.get(); + return strong.as(iid, result); + } + } + + HRESULT __stdcall LockServer(BOOL) noexcept final + { + return S_OK; + } + +private: + winrt::guid _guid; +}; + +static void doContentProcessThing(const winrt::guid& contentProcessGuid) +{ + // !! LOAD BEARING !! - important to be a MTA + winrt::init_apartment(); + + DWORD registrationHostClass{}; + check_hresult(CoRegisterClassObject(contentProcessGuid, + make(contentProcessGuid).get(), + CLSCTX_LOCAL_SERVER, + REGCLS_MULTIPLEUSE, + ®istrationHostClass)); + + std::unique_lock lk(m); + cv.wait(lk, [] { return dtored; }); +} + int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) { TraceLoggingRegister(g_hWindowsTerminalProvider); @@ -119,6 +209,17 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) // should choose and install the correct one from the bundle. EnsureNativeArchitecture(); + winrt::guid contentProcessGuid{}; + if (checkIfContentProcess(contentProcessGuid)) + { + doContentProcessThing(contentProcessGuid); + // If we were told to not have a window, exit early. Make sure to use + // ExitProcess to die here. If you try just `return 0`, then + // the XAML app host will crash during teardown. ExitProcess avoids + // that. + ExitProcess(0); + } + // Make sure to call this so we get WM_POINTER messages. EnableMouseInPointer(true); From bb49d5086c589f5a9dcc921308d8fafe55702190 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 10 Aug 2021 16:17:27 -0500 Subject: [PATCH 02/53] I believe this merges the buisness of `connection-factory`, though there are many issues. --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 77 ++++++++++++------- scratch/ScratchIslandApp/SampleApp/MyPage.h | 2 +- .../TerminalControl/ContentProcess.cpp | 11 ++- src/cascadia/TerminalControl/ContentProcess.h | 3 +- .../TerminalControl/ContentProcess.idl | 4 +- 5 files changed, 63 insertions(+), 34 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index a7f5b7cc968..69f8a5b51f0 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -28,7 +28,6 @@ namespace winrt::SampleApp::implementation void MyPage::Create() { - TerminalConnection::EchoConnection conn{}; auto settings = winrt::make_self(); auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted in-proc...", @@ -125,22 +124,44 @@ namespace winrt::SampleApp::implementation Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); - TerminalConnection::ITerminalConnection conn{ nullptr }; + // TerminalConnection::ITerminalConnection conn{ nullptr }; + TerminalConnection::ConnectionInformation connectInfo{ nullptr }; Control::IControlSettings settings{ nullptr }; settings = *winrt::make_self(); + // When creating a terminal for the first time, pass it a connection + // info + // + // otherwise, when attaching to an existing one, just pass null, because + // we don't need the connection info. if (!attached) { - conn = TerminalConnection::EchoConnection{}; - // settings = *winrt::make_self(); - content.Initialize(settings, conn); + auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted out-of-proc...", + winrt::hstring{}, + L"", + nullptr, + 32, + 80, + winrt::guid()) }; + + // "Microsoft.Terminal.TerminalConnection.ConptyConnection" + winrt::hstring myClass{ winrt::name_of() }; + connectInfo = TerminalConnection::ConnectionInformation(myClass, connectionSettings); + + if (!content.Initialize(settings, connectInfo)) + { + co_return; + } + } + else + { } // Switch back to the UI thread. co_await ui_thread; - Control::TermControl control{ contentGuid, settings, conn }; + Control::TermControl control{ contentGuid, settings, nullptr }; OutOfProcContent().Children().Append(control); @@ -157,36 +178,36 @@ namespace winrt::SampleApp::implementation }*/ - winrt::fire_and_forget MyPage::CreateOutOfProcTerminal() - { - // 1. Generate a GUID. - winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; + //winrt::fire_and_forget MyPage::CreateOutOfProcTerminal() + //{ + // // 1. Generate a GUID. + // winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; - // Capture calling context. - winrt::apartment_context ui_thread; - co_await winrt::resume_background(); + // // Capture calling context. + // winrt::apartment_context ui_thread; + // co_await winrt::resume_background(); - // 2. Spawn a Server.exe, with the guid on the commandline - auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; + // // 2. Spawn a Server.exe, with the guid on the commandline + // auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; - Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + // Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); - TerminalConnection::EchoConnection conn{}; - auto settings = winrt::make_self(); - Control::IControlSettings s = *settings; + // TerminalConnection::EchoConnection conn{}; + // auto settings = winrt::make_self(); + // Control::IControlSettings s = *settings; - if (s) - { - content.Initialize(s, conn); + // if (s) + // { + // content.Initialize(s, conn); - // Switch back to the UI thread. - co_await ui_thread; + // // Switch back to the UI thread. + // co_await ui_thread; - Control::TermControl control{ contentGuid, s, conn }; + // Control::TermControl control{ contentGuid, s, conn }; - OutOfProcContent().Children().Append(control); - } - } + // OutOfProcContent().Children().Append(control); + // } + //} // Method Description: // - Gets the title of the currently focused terminal control. If there diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.h b/scratch/ScratchIslandApp/SampleApp/MyPage.h index 94a4f3ac77b..729513e487c 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.h +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.h @@ -16,7 +16,7 @@ namespace winrt::SampleApp::implementation void Create(); hstring Title(); - winrt::fire_and_forget CreateOutOfProcTerminal(); + // winrt::fire_and_forget CreateOutOfProcTerminal(); winrt::fire_and_forget CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); diff --git a/src/cascadia/TerminalControl/ContentProcess.cpp b/src/cascadia/TerminalControl/ContentProcess.cpp index 6f3de9a6842..063d47facfa 100644 --- a/src/cascadia/TerminalControl/ContentProcess.cpp +++ b/src/cascadia/TerminalControl/ContentProcess.cpp @@ -9,9 +9,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation { ContentProcess::ContentProcess() {} - void ContentProcess::Initialize(Control::IControlSettings settings, TerminalConnection::ITerminalConnection connection) + bool ContentProcess::Initialize(Control::IControlSettings settings, + TerminalConnection::ConnectionInformation connectionInfo) { - _interactivity = winrt::make(settings, connection); + auto conn{ TerminalConnection::ConnectionInformation::CreateConnection(connectionInfo) }; + if (conn == nullptr) + { + return false; + } + _interactivity = winrt::make(settings, conn); + return true; } Control::ControlInteractivity ContentProcess::GetInteractivity() diff --git a/src/cascadia/TerminalControl/ContentProcess.h b/src/cascadia/TerminalControl/ContentProcess.h index 609d0b7c30e..d3263bd9b4e 100644 --- a/src/cascadia/TerminalControl/ContentProcess.h +++ b/src/cascadia/TerminalControl/ContentProcess.h @@ -12,7 +12,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { ContentProcess(); - void Initialize(Control::IControlSettings settings, TerminalConnection::ITerminalConnection connection); + bool Initialize(Control::IControlSettings settings, + TerminalConnection::ConnectionInformation connectionInfo); Control::ControlInteractivity GetInteractivity(); uint64_t RequestSwapChainHandle(const uint64_t pid); diff --git a/src/cascadia/TerminalControl/ContentProcess.idl b/src/cascadia/TerminalControl/ContentProcess.idl index 8aa945bd7ed..81a81d3a85a 100644 --- a/src/cascadia/TerminalControl/ContentProcess.idl +++ b/src/cascadia/TerminalControl/ContentProcess.idl @@ -9,8 +9,8 @@ namespace Microsoft.Terminal.Control ContentProcess(); - void Initialize(IControlSettings settings, - Microsoft.Terminal.TerminalConnection.ITerminalConnection connection); + Boolean Initialize(IControlSettings settings, + Microsoft.Terminal.TerminalConnection.ConnectionInformation connectionInfo); ControlInteractivity GetInteractivity(); From 7731f5943ace5dd964a82b923ab40b3d5d084565 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 10 Aug 2021 16:25:44 -0500 Subject: [PATCH 03/53] Some comments because everything is hard --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 64 +++++-------------- 1 file changed, 16 insertions(+), 48 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 69f8a5b51f0..097bc6c668b 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -28,7 +28,7 @@ namespace winrt::SampleApp::implementation void MyPage::Create() { - auto settings = winrt::make_self(); + auto settings = winrt::make_self(); auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted in-proc...", winrt::hstring{}, @@ -103,7 +103,7 @@ namespace winrt::SampleApp::implementation co_await winrt::resume_background(); auto canConvert = guidString.size() == 38 && guidString.front() == '{' && guidString.back() == '}'; - bool attached = false; + bool tryingToAttach = false; winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; if (canConvert) @@ -112,30 +112,32 @@ namespace winrt::SampleApp::implementation if (SUCCEEDED(IIDFromString(guidString.c_str(), &result))) { contentGuid = result; - attached = true; + tryingToAttach = true; } } - if (!attached) + if (!tryingToAttach) { - // 2. Spawn a Server.exe, with the guid on the commandline + // Spawn a wt.exe, with the guid on the commandline auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; } + // THIS MUST TAKE PLACE AFTER _createHostClassProcess. + // * If we're creating a new OOP control, _createHostClassProcess will + // spawn the process that will actually host the ContentProcess + // object. + // * If we're attaching, then that process already exists. Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); - // TerminalConnection::ITerminalConnection conn{ nullptr }; TerminalConnection::ConnectionInformation connectInfo{ nullptr }; - Control::IControlSettings settings{ nullptr }; - - settings = *winrt::make_self(); + Control::IControlSettings settings{ *winrt::make_self() }; // When creating a terminal for the first time, pass it a connection // info // // otherwise, when attaching to an existing one, just pass null, because // we don't need the connection info. - if (!attached) + if (!tryingToAttach) { auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted out-of-proc...", winrt::hstring{}, @@ -156,59 +158,25 @@ namespace winrt::SampleApp::implementation } else { + // If we're attaching, we don't really need to do anything special. } // Switch back to the UI thread. co_await ui_thread; + // Create the XAML control that will be attached to the content process. + // We're not passing in a connection, because the contentGuid will be used instead. Control::TermControl control{ contentGuid, settings, nullptr }; OutOfProcContent().Children().Append(control); - if (!attached) + if (!tryingToAttach) { auto guidStr{ ::Microsoft::Console::Utils::GuidToString(contentGuid) }; GuidInput().Text(guidStr); } } - /*winrt::fire_and_forget MyPage::_attachToContent(winrt::guid contentGuid) - { - Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); - - }*/ - - //winrt::fire_and_forget MyPage::CreateOutOfProcTerminal() - //{ - // // 1. Generate a GUID. - // winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; - - // // Capture calling context. - // winrt::apartment_context ui_thread; - // co_await winrt::resume_background(); - - // // 2. Spawn a Server.exe, with the guid on the commandline - // auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; - - // Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); - - // TerminalConnection::EchoConnection conn{}; - // auto settings = winrt::make_self(); - // Control::IControlSettings s = *settings; - - // if (s) - // { - // content.Initialize(s, conn); - - // // Switch back to the UI thread. - // co_await ui_thread; - - // Control::TermControl control{ contentGuid, s, conn }; - - // OutOfProcContent().Children().Append(control); - // } - //} - // Method Description: // - Gets the title of the currently focused terminal control. If there // isn't a control selected for any reason, returns "Windows Terminal" From 996c71a93317581258b9991f799cb8aa84856f9a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 08:45:43 -0500 Subject: [PATCH 04/53] Add a signal that the content can use to tell the window it's ready --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 69 ++++++++++++------- scratch/ScratchIslandApp/SampleApp/MyPage.h | 4 +- .../ScratchIslandApp/SampleApp/MyPage.xaml | 7 +- src/cascadia/TerminalControl/ControlCore.h | 3 + src/cascadia/TerminalControl/TermControl.cpp | 11 ++- src/cascadia/WindowsTerminal/main.cpp | 22 ++++-- 6 files changed, 83 insertions(+), 33 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 097bc6c668b..354ba24f249 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -28,7 +28,7 @@ namespace winrt::SampleApp::implementation void MyPage::Create() { - auto settings = winrt::make_self(); + auto settings = winrt::make_self(); auto connectionSettings{ TerminalConnection::ConptyConnection::CreateSettings(L"cmd.exe /k echo This TermControl is hosted in-proc...", winrt::hstring{}, @@ -56,7 +56,22 @@ namespace winrt::SampleApp::implementation static wil::unique_process_information _createHostClassProcess(const winrt::guid& g) { auto guidStr{ ::Microsoft::Console::Utils::GuidToString(g) }; - std::wstring commandline{ fmt::format(L"windowsterminal.exe --content {}", guidStr) }; + + // Create an event that the content process will use to signal it is + // ready to go. We won't need the event after this function, so the + // unique_event will clean up our handle when we leave this scope. The + // ContentProcess is responsible for cleaning up its own handle. + wil::unique_event ev{ CreateEvent(nullptr, true, false, L"contentProcessStarted") }; + // Make sure to mark this handle as inheritable! Even with + // bInheritHandles=true, this is only inherited when it's explicitly + // allowed to be. + SetHandleInformation(ev.get(), HANDLE_FLAG_INHERIT, 1); + + // god bless, fmt::format will format a HANDLE like `0xa80` + std::wstring commandline{ + fmt::format(L"windowsterminal.exe --content {} --signal {}", guidStr, ev.get()) + }; + STARTUPINFO siOne{ 0 }; siOne.cb = sizeof(STARTUPINFOW); wil::unique_process_information piOne; @@ -65,7 +80,7 @@ namespace winrt::SampleApp::implementation commandline.data(), nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes - false, // bInheritHandles + true, // bInheritHandles CREATE_UNICODE_ENVIRONMENT, // dwCreationFlags nullptr, // lpEnvironment nullptr, // startingDirectory @@ -73,28 +88,23 @@ namespace winrt::SampleApp::implementation &piOne // lpProcessInformation ); THROW_IF_WIN32_BOOL_FALSE(succeeded); - // if (!succeeded) - // { - // printf("Failed to create host process\n"); - // return; - // } - - // Ooof this is dumb, but we need a sleep here to make the server starts. - // That's _sub par_. Maybe we could use the host's stdout to have them emit - // a byte when they're set up? - Sleep(2000); - - // TODO MONDAY - It seems like it takes conhost too long to start up to - // host the ScratchWinRTServer that even a sleep 100 is too short. However, - // any longer, and XAML will just crash, because some frame took too long. - // So we _need_ to do the "have the server explicitly tell us it's ready" - // thing, and maybe also do it on a bg thread (and signal to the UI thread - // that it can attach now) + + // Wait for the child process to signal that they're ready. + // TODO!: The _first_ time we run, we always fail to init the ContentProcess. What the heck? + WaitForSingleObject(ev.get(), INFINITE); return std::move(piOne); } - winrt::fire_and_forget MyPage::CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs) + void MyPage::_writeToLog(std::wstring_view str) + { + winrt::WUX::Controls::TextBlock block; + block.Text(str); + Log().Children().Append(block); + } + + winrt::fire_and_forget MyPage::CreateClicked(const IInspectable& sender, + const WUX::Input::TappedRoutedEventArgs& eventArgs) { auto guidString = GuidInput().Text(); @@ -102,7 +112,9 @@ namespace winrt::SampleApp::implementation winrt::apartment_context ui_thread; co_await winrt::resume_background(); - auto canConvert = guidString.size() == 38 && guidString.front() == '{' && guidString.back() == '}'; + auto canConvert = guidString.size() == 38 && + guidString.front() == '{' && + guidString.back() == '}'; bool tryingToAttach = false; winrt::guid contentGuid{ ::Microsoft::Console::Utils::CreateGuid() }; @@ -128,6 +140,13 @@ namespace winrt::SampleApp::implementation // object. // * If we're attaching, then that process already exists. Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + if (content == nullptr) + { + // Switch back to the UI thread. + co_await ui_thread; + _writeToLog(L"Failed to connect to the ContentProces object. It may not have been started fast enough."); + co_return; // be sure to co_return or we'll fall through to the part where we clear the log + } TerminalConnection::ConnectionInformation connectInfo{ nullptr }; Control::IControlSettings settings{ *winrt::make_self() }; @@ -153,7 +172,10 @@ namespace winrt::SampleApp::implementation if (!content.Initialize(settings, connectInfo)) { - co_return; + // Switch back to the UI thread. + co_await ui_thread; + _writeToLog(L"Failed to Initialize the ContentProces object."); + co_return; // be sure to co_return or we'll fall through to the part where we clear the log } } else @@ -168,6 +190,7 @@ namespace winrt::SampleApp::implementation // We're not passing in a connection, because the contentGuid will be used instead. Control::TermControl control{ contentGuid, settings, nullptr }; + Log().Children().Clear(); OutOfProcContent().Children().Append(control); if (!tryingToAttach) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.h b/scratch/ScratchIslandApp/SampleApp/MyPage.h index 729513e487c..74afee18f48 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.h +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.h @@ -14,14 +14,14 @@ namespace winrt::SampleApp::implementation MyPage(); void Create(); - hstring Title(); - // winrt::fire_and_forget CreateOutOfProcTerminal(); winrt::fire_and_forget CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); private: friend struct MyPageT; // for Xaml to bind events + + void _writeToLog(std::wstring_view str); }; } diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml index f132e66a5e8..83e44826b2d 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml @@ -53,7 +53,12 @@ Padding="16" HorizontalAlignment="Stretch" VerticalAlignment="Stretch" - Background="#0000ff" /> + Background="#0000ff"> + + + + diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index a8cd204f1ef..ccca7f66226 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -277,6 +277,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation inline bool _IsClosing() const noexcept { #ifndef NDEBUG + // TODO! This may not be strictly true if the core is running out of + // proc with XAML. I keep hitting this assertion every time it + // exits, so we might need a better solution. if (_dispatcher) { // _closing isn't atomic and may only be accessed from the main thread. diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 647d4442609..cd4d0619c22 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -652,7 +652,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (auto control{ weakThis.get() }) { - const HANDLE chainHandle = reinterpret_cast(control->_core.SwapChainHandle()); + // TODO! very good chance we leak this handle + const HANDLE chainHandle = reinterpret_cast(control->_contentProc ? + control->_contentProc.RequestSwapChainHandle(GetCurrentProcessId()) : + control->_core.SwapChainHandle()); _AttachDxgiSwapChainToXaml(chainHandle); } } @@ -740,7 +743,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation } _interactivity.Initialize(); - _AttachDxgiSwapChainToXaml(reinterpret_cast(_core.SwapChainHandle())); + // TODO! very good chance we leak this handle + const HANDLE chainHandle = reinterpret_cast(_contentProc ? + _contentProc.RequestSwapChainHandle(GetCurrentProcessId()) : + _core.SwapChainHandle()); + _AttachDxgiSwapChainToXaml(chainHandle); // Tell the DX Engine to notify us when the swap chain changes. We do // this after we initially set the swapchain so as to avoid unnecessary diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index e126257c567..99966517e0c 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -96,7 +96,7 @@ static bool _messageIsAltSpaceKeypress(const MSG& message) return message.message == WM_SYSKEYDOWN && message.wParam == VK_SPACE; } -static bool checkIfContentProcess(winrt::guid& contentProcessGuid) +static bool checkIfContentProcess(winrt::guid& contentProcessGuid, HANDLE& eventHandle) { std::vector args; @@ -114,7 +114,9 @@ static bool checkIfContentProcess(winrt::guid& contentProcessGuid) } } } - if (args.size() > 2 && args.at(1) == L"--content") + if (args.size() == 5 && + args.at(1) == L"--content" && + args.at(3) == L"--signal") { auto& guidString{ args.at(2) }; auto canConvert = guidString.length() == 38 && guidString.front() == '{' && guidString.back() == '}'; @@ -123,6 +125,10 @@ static bool checkIfContentProcess(winrt::guid& contentProcessGuid) GUID result{}; THROW_IF_FAILED(IIDFromString(guidString.c_str(), &result)); contentProcessGuid = result; + + eventHandle = reinterpret_cast(wcstoul(args.at(4).c_str(), + nullptr /*endptr*/, + 16 /*base*/)); return true; } } @@ -170,7 +176,7 @@ struct HostClassFactory : implements winrt::guid _guid; }; -static void doContentProcessThing(const winrt::guid& contentProcessGuid) +static void doContentProcessThing(const winrt::guid& contentProcessGuid, const HANDLE& eventHandle) { // !! LOAD BEARING !! - important to be a MTA winrt::init_apartment(); @@ -182,6 +188,11 @@ static void doContentProcessThing(const winrt::guid& contentProcessGuid) REGCLS_MULTIPLEUSE, ®istrationHostClass)); + // Signal the event handle that was passed to us that we're now set up and + // ready to go. + SetEvent(eventHandle); + CloseHandle(eventHandle); + std::unique_lock lk(m); cv.wait(lk, [] { return dtored; }); } @@ -210,9 +221,10 @@ int __stdcall wWinMain(HINSTANCE, HINSTANCE, LPWSTR, int) EnsureNativeArchitecture(); winrt::guid contentProcessGuid{}; - if (checkIfContentProcess(contentProcessGuid)) + HANDLE eventHandle{ INVALID_HANDLE_VALUE }; + if (checkIfContentProcess(contentProcessGuid, eventHandle)) { - doContentProcessThing(contentProcessGuid); + doContentProcessThing(contentProcessGuid, eventHandle); // If we were told to not have a window, exit early. Make sure to use // ExitProcess to die here. If you try just `return 0`, then // the XAML app host will crash during teardown. ExitProcess avoids From 2f23e1fc0c5369dfcbfd5a1a90e27e3418731c37 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 09:47:50 -0500 Subject: [PATCH 05/53] A close button, and more logging --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 40 ++++++++++++++----- scratch/ScratchIslandApp/SampleApp/MyPage.h | 5 ++- .../ScratchIslandApp/SampleApp/MyPage.xaml | 24 ++++++++--- src/cascadia/TerminalControl/TermControl.cpp | 10 ++++- 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 354ba24f249..561654e50d7 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -96,10 +96,13 @@ namespace winrt::SampleApp::implementation return std::move(piOne); } - void MyPage::_writeToLog(std::wstring_view str) + winrt::fire_and_forget MyPage::_writeToLog(std::wstring_view str) { + winrt::hstring copy{ str }; + // Switch back to the UI thread. + co_await resume_foreground(Dispatcher()); winrt::WUX::Controls::TextBlock block; - block.Text(str); + block.Text(copy); Log().Children().Append(block); } @@ -110,7 +113,6 @@ namespace winrt::SampleApp::implementation // Capture calling context. winrt::apartment_context ui_thread; - co_await winrt::resume_background(); auto canConvert = guidString.size() == 38 && guidString.front() == '{' && @@ -127,11 +129,13 @@ namespace winrt::SampleApp::implementation tryingToAttach = true; } } + _writeToLog(tryingToAttach ? L"Attaching to existing content process" : L"Creating new content process"); + co_await winrt::resume_background(); if (!tryingToAttach) { // Spawn a wt.exe, with the guid on the commandline - auto piContent{ std::move(_createHostClassProcess(contentGuid)) }; + piContentProcess = std::move(_createHostClassProcess(contentGuid)); } // THIS MUST TAKE PLACE AFTER _createHostClassProcess. @@ -139,11 +143,21 @@ namespace winrt::SampleApp::implementation // spawn the process that will actually host the ContentProcess // object. // * If we're attaching, then that process already exists. - Control::ContentProcess content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + Control::ContentProcess content; + try + { + content = create_instance(contentGuid, CLSCTX_LOCAL_SERVER); + } + catch (winrt::hresult_error hr) + { + _writeToLog(L"CreateInstance the ContentProces object"); + _writeToLog(fmt::format(L" HR ({}): {}", hr.code(), hr.message().c_str())); + co_return; // be sure to co_return or we'll fall through to the part where we clear the log + + } + if (content == nullptr) { - // Switch back to the UI thread. - co_await ui_thread; _writeToLog(L"Failed to connect to the ContentProces object. It may not have been started fast enough."); co_return; // be sure to co_return or we'll fall through to the part where we clear the log } @@ -172,8 +186,6 @@ namespace winrt::SampleApp::implementation if (!content.Initialize(settings, connectInfo)) { - // Switch back to the UI thread. - co_await ui_thread; _writeToLog(L"Failed to Initialize the ContentProces object."); co_return; // be sure to co_return or we'll fall through to the part where we clear the log } @@ -200,6 +212,16 @@ namespace winrt::SampleApp::implementation } } + void MyPage::CloseClicked(const IInspectable& /*sender*/, + const WUX::Input::TappedRoutedEventArgs& /*eventArgs*/) + { + OutOfProcContent().Children().Clear(); + if (piContentProcess.hProcess) + { + piContentProcess.reset(); + } + } + // Method Description: // - Gets the title of the currently focused terminal control. If there // isn't a control selected for any reason, returns "Windows Terminal" diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.h b/scratch/ScratchIslandApp/SampleApp/MyPage.h index 74afee18f48..d38c3391b33 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.h +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.h @@ -17,11 +17,14 @@ namespace winrt::SampleApp::implementation hstring Title(); winrt::fire_and_forget CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); + void CloseClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); private: friend struct MyPageT; // for Xaml to bind events - void _writeToLog(std::wstring_view str); + wil::unique_process_information piContentProcess; + + winrt::fire_and_forget _writeToLog(std::wstring_view str); }; } diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml index 83e44826b2d..1d916275ca5 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml @@ -28,6 +28,11 @@ Tapped="CreateClicked"> Create + @@ -48,16 +53,25 @@ VerticalAlignment="Stretch" Background="#ff0000" /> - + VerticalAlignment="Stretch"> + + + + + + diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index cd4d0619c22..7f5b8218cee 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -1863,8 +1863,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Disconnect the TSF input control so it doesn't receive EditContext events. TSFInputControl().Close(); _autoScrollTimer.Stop(); - - _core.Close(); + // try + // { + if (!_contentProc) + { + _core.Close(); + } + // } + // CATCH_LOG(); } } From c3a94454e0952f0532c85e492dc20b3f17af6eb7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 10:00:57 -0500 Subject: [PATCH 06/53] some short-circuits for these inits, to make them more stable --- .../ConnectionInformation.cpp | 56 ++++++++++++++++--- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalConnection/ConnectionInformation.cpp b/src/cascadia/TerminalConnection/ConnectionInformation.cpp index 5ae37fad691..0d406603b73 100644 --- a/src/cascadia/TerminalConnection/ConnectionInformation.cpp +++ b/src/cascadia/TerminalConnection/ConnectionInformation.cpp @@ -36,21 +36,59 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation ::IInspectable** raw = reinterpret_cast<::IInspectable**>(pointer); #pragma warning(pop) - // RoActivateInstance() will try to create an instance of the object, - // who's fully qualified name is the string in Name(). + TerminalConnection::ITerminalConnection connection{ nullptr }; + + // A couple short-circuits, for connections that _we_ implement. + // Sometimes, RoActivateInstance is weird and fails with errors like the + // following + // // - // The class has to be activatable. For the Terminal, this is easy - // enough - we're not hosting anything that's not already in our - // manifest, or living as a .dll & .winmd SxS. + /* + onecore\com\combase\inc\RegistryKey.hpp(527)\combase.dll!00007FFF75E1F855: + (caller: 00007FFF75D3BC29) LogHr(2) tid(83a8) 800700A1 The specified + path is invalid. + Msg:[StaticNtOpen failed with + path:\REGISTRY\A\{A41685A4-AD85-4C4C-BA5D-A849ADBF3C40}\ActivatableClassId + \REGISTRY\MACHINE\Software\Classes\ActivatableClasses] + ...\src\cascadia\TerminalConnection\ConnectionInformation.cpp(47)\TerminalConnection.dll!00007FFEC1381FC5: + (caller: 00007FFEC13810A5) LogHr(1) tid(83a8) 800700A1 The specified + path is invalid. + [...TerminalConnection::implementation::ConnectionInformation::CreateConnection(RoActivateInstance(name, + raw))] + */ // - // When we get to extensions (GH#4000), we may want to revisit. - if (LOG_IF_FAILED(RoActivateInstance(name, raw))) + // So to avoid those, we'll manually instantiate these + if (info.ClassName() == winrt::name_of()) + { + connection = TerminalConnection::ConptyConnection(); + } + else if (info.ClassName() == winrt::name_of()) + { + connection = TerminalConnection::AzureConnection(); + } + else if (info.ClassName() == winrt::name_of()) + { + connection = TerminalConnection::EchoConnection(); + } + else { - return nullptr; + // RoActivateInstance() will try to create an instance of the object, + // who's fully qualified name is the string in Name(). + // + // The class has to be activatable. For the Terminal, this is easy + // enough - we're not hosting anything that's not already in our + // manifest, or living as a .dll & .winmd SxS. + // + // When we get to extensions (GH#4000), we may want to revisit. + if (LOG_IF_FAILED(RoActivateInstance(name, raw))) + { + return nullptr; + } + connection = inspectable.try_as(); } // Now that thing we made, make sure it's actually a ITerminalConnection - if (const auto connection{ inspectable.try_as() }) + if (connection) { // Initialize it, and return it. connection.Initialize(info.Settings()); From 7e792b2b5e8eced4b4f611fbeb82126a147bbde2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 10:32:24 -0500 Subject: [PATCH 07/53] You know, there's 0% chance that this is the right pattern for this, but it _works_ --- .../TerminalControl/ContentProcess.cpp | 5 +++++ src/cascadia/TerminalControl/ContentProcess.h | 3 +++ .../TerminalControl/ContentProcess.idl | 4 ++++ src/cascadia/TerminalControl/ControlCore.h | 22 +++++++++---------- src/cascadia/WindowsTerminal/main.cpp | 5 +++++ 5 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/cascadia/TerminalControl/ContentProcess.cpp b/src/cascadia/TerminalControl/ContentProcess.cpp index 063d47facfa..eb7078afd8a 100644 --- a/src/cascadia/TerminalControl/ContentProcess.cpp +++ b/src/cascadia/TerminalControl/ContentProcess.cpp @@ -21,6 +21,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation return true; } + ContentProcess::~ContentProcess() + { + _DestructedHandlers(); + } + Control::ControlInteractivity ContentProcess::GetInteractivity() { return _interactivity; diff --git a/src/cascadia/TerminalControl/ContentProcess.h b/src/cascadia/TerminalControl/ContentProcess.h index d3263bd9b4e..465885ef66f 100644 --- a/src/cascadia/TerminalControl/ContentProcess.h +++ b/src/cascadia/TerminalControl/ContentProcess.h @@ -12,12 +12,15 @@ namespace winrt::Microsoft::Terminal::Control::implementation { ContentProcess(); + ~ContentProcess(); bool Initialize(Control::IControlSettings settings, TerminalConnection::ConnectionInformation connectionInfo); Control::ControlInteractivity GetInteractivity(); uint64_t RequestSwapChainHandle(const uint64_t pid); + WINRT_CALLBACK(Destructed, Control::DestructedArgs); + private: Control::ControlInteractivity _interactivity{ nullptr }; }; diff --git a/src/cascadia/TerminalControl/ContentProcess.idl b/src/cascadia/TerminalControl/ContentProcess.idl index 81a81d3a85a..998330cd667 100644 --- a/src/cascadia/TerminalControl/ContentProcess.idl +++ b/src/cascadia/TerminalControl/ContentProcess.idl @@ -5,6 +5,8 @@ import "ControlInteractivity.idl"; namespace Microsoft.Terminal.Control { + delegate void DestructedArgs(); + runtimeclass ContentProcess { ContentProcess(); @@ -15,5 +17,7 @@ namespace Microsoft.Terminal.Control ControlInteractivity GetInteractivity(); UInt64 RequestSwapChainHandle(UInt64 pid); + + event DestructedArgs Destructed; }; } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index ccca7f66226..cc3484dd1e7 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -277,17 +277,17 @@ namespace winrt::Microsoft::Terminal::Control::implementation inline bool _IsClosing() const noexcept { #ifndef NDEBUG - // TODO! This may not be strictly true if the core is running out of - // proc with XAML. I keep hitting this assertion every time it - // exits, so we might need a better solution. - if (_dispatcher) - { - // _closing isn't atomic and may only be accessed from the main thread. - // - // Though, the unit tests don't actually run in TAEF's main - // thread, so we don't care when we're running in tests. - assert(_inUnitTests || _dispatcher.HasThreadAccess()); - } + // // TODO! This may not be strictly true if the core is running out of + // // proc with XAML. I keep hitting this assertion every time it + // // exits, so we might need a better solution. + // if (_dispatcher) + // { + // // _closing isn't atomic and may only be accessed from the main thread. + // // + // // Though, the unit tests don't actually run in TAEF's main + // // thread, so we don't care when we're running in tests. + // assert(_inUnitTests || _dispatcher.HasThreadAccess()); + // } #endif return _closing; } diff --git a/src/cascadia/WindowsTerminal/main.cpp b/src/cascadia/WindowsTerminal/main.cpp index 99966517e0c..7a05e8b4163 100644 --- a/src/cascadia/WindowsTerminal/main.cpp +++ b/src/cascadia/WindowsTerminal/main.cpp @@ -156,6 +156,11 @@ struct HostClassFactory : implements if (!g_weak) { winrt::Microsoft::Terminal::Control::ContentProcess strong{}; // = winrt::make(); + strong.Destructed([]() { + std::unique_lock lk(m); + dtored = true; + cv.notify_one(); + }); winrt::weak_ref weak{ strong }; g_weak = weak; return strong.as(iid, result); From 5a07282d19fc4fcfb14d187697f9c47399bd0948 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 11:28:01 -0500 Subject: [PATCH 08/53] Add a kill button for manually killing the content --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 18 ++++++++++++++---- scratch/ScratchIslandApp/SampleApp/MyPage.h | 1 + scratch/ScratchIslandApp/SampleApp/MyPage.idl | 2 +- scratch/ScratchIslandApp/SampleApp/MyPage.xaml | 7 +++++++ 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 561654e50d7..395d407491f 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -90,7 +90,6 @@ namespace winrt::SampleApp::implementation THROW_IF_WIN32_BOOL_FALSE(succeeded); // Wait for the child process to signal that they're ready. - // TODO!: The _first_ time we run, we always fail to init the ContentProcess. What the heck? WaitForSingleObject(ev.get(), INFINITE); return std::move(piOne); @@ -153,9 +152,8 @@ namespace winrt::SampleApp::implementation _writeToLog(L"CreateInstance the ContentProces object"); _writeToLog(fmt::format(L" HR ({}): {}", hr.code(), hr.message().c_str())); co_return; // be sure to co_return or we'll fall through to the part where we clear the log - } - + if (content == nullptr) { _writeToLog(L"Failed to connect to the ContentProces object. It may not have been started fast enough."); @@ -213,15 +211,27 @@ namespace winrt::SampleApp::implementation } void MyPage::CloseClicked(const IInspectable& /*sender*/, - const WUX::Input::TappedRoutedEventArgs& /*eventArgs*/) + const WUX::Input::TappedRoutedEventArgs& /*eventArgs*/) { OutOfProcContent().Children().Clear(); + GuidInput().Text(L""); if (piContentProcess.hProcess) { piContentProcess.reset(); } } + void MyPage::KillClicked(const IInspectable& /*sender*/, + const WUX::Input::TappedRoutedEventArgs& /*eventArgs*/) + { + if (piContentProcess.hProcess) + { + TerminateProcess(piContentProcess.hProcess, (UINT)-1); + piContentProcess.reset(); + } + } + + // Method Description: // - Gets the title of the currently focused terminal control. If there // isn't a control selected for any reason, returns "Windows Terminal" diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.h b/scratch/ScratchIslandApp/SampleApp/MyPage.h index d38c3391b33..88afbbdc0e7 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.h +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.h @@ -18,6 +18,7 @@ namespace winrt::SampleApp::implementation winrt::fire_and_forget CreateClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); void CloseClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); + void KillClicked(const IInspectable& sender, const Windows::UI::Xaml::Input::TappedRoutedEventArgs& eventArgs); private: friend struct MyPageT; // for Xaml to bind events diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.idl b/scratch/ScratchIslandApp/SampleApp/MyPage.idl index d3d0645b58b..77bf9748c12 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.idl +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.idl @@ -3,7 +3,7 @@ namespace SampleApp { - [default_interface] runtimeclass MyPage : Windows.UI.Xaml.Controls.Page + [default_interface] runtimeclass MyPage : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged { MyPage(); } diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml index 1d916275ca5..dd38434c533 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.xaml +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.xaml @@ -30,9 +30,16 @@ + From 3d15b097b772262113f02fbbbfc6d81c1fbf5926 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Thu, 12 Aug 2021 12:55:03 -0500 Subject: [PATCH 09/53] This works to kill the content and have the app live --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 10 ++ scratch/ScratchIslandApp/SampleApp/MyPage.idl | 2 +- .../TerminalControl/ContentProcess.cpp | 8 +- src/cascadia/TerminalControl/ContentProcess.h | 2 + .../TerminalControl/ContentProcess.idl | 2 + src/cascadia/TerminalControl/TermControl.cpp | 144 ++++++++++++++++++ src/cascadia/TerminalControl/TermControl.h | 8 + 7 files changed, 174 insertions(+), 2 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 395d407491f..77a311a7aaa 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -199,6 +199,16 @@ namespace winrt::SampleApp::implementation // Create the XAML control that will be attached to the content process. // We're not passing in a connection, because the contentGuid will be used instead. Control::TermControl control{ contentGuid, settings, nullptr }; + control.RaiseNotice([this](auto&&, auto& args) { + _writeToLog(L"Content process died, probably."); + _writeToLog(args.Message()); + OutOfProcContent().Children().Clear(); + GuidInput().Text(L""); + if (piContentProcess.hProcess) + { + piContentProcess.reset(); + } + }); Log().Children().Clear(); OutOfProcContent().Children().Append(control); diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.idl b/scratch/ScratchIslandApp/SampleApp/MyPage.idl index 77bf9748c12..d3d0645b58b 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.idl +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.idl @@ -3,7 +3,7 @@ namespace SampleApp { - [default_interface] runtimeclass MyPage : Windows.UI.Xaml.Controls.Page, Windows.UI.Xaml.Data.INotifyPropertyChanged + [default_interface] runtimeclass MyPage : Windows.UI.Xaml.Controls.Page { MyPage(); } diff --git a/src/cascadia/TerminalControl/ContentProcess.cpp b/src/cascadia/TerminalControl/ContentProcess.cpp index eb7078afd8a..17446989203 100644 --- a/src/cascadia/TerminalControl/ContentProcess.cpp +++ b/src/cascadia/TerminalControl/ContentProcess.cpp @@ -7,7 +7,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation { - ContentProcess::ContentProcess() {} + ContentProcess::ContentProcess() : + _ourPID{ GetCurrentProcessId() } {} bool ContentProcess::Initialize(Control::IControlSettings settings, TerminalConnection::ConnectionInformation connectionInfo) @@ -31,6 +32,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation return _interactivity; } + uint64_t ContentProcess::GetPID() + { + return _ourPID; + } + uint64_t ContentProcess::RequestSwapChainHandle(const uint64_t pid) { auto ourPid = GetCurrentProcessId(); diff --git a/src/cascadia/TerminalControl/ContentProcess.h b/src/cascadia/TerminalControl/ContentProcess.h index 465885ef66f..e7ea32679a8 100644 --- a/src/cascadia/TerminalControl/ContentProcess.h +++ b/src/cascadia/TerminalControl/ContentProcess.h @@ -17,12 +17,14 @@ namespace winrt::Microsoft::Terminal::Control::implementation TerminalConnection::ConnectionInformation connectionInfo); Control::ControlInteractivity GetInteractivity(); + uint64_t GetPID(); uint64_t RequestSwapChainHandle(const uint64_t pid); WINRT_CALLBACK(Destructed, Control::DestructedArgs); private: Control::ControlInteractivity _interactivity{ nullptr }; + uint64_t _ourPID; }; } diff --git a/src/cascadia/TerminalControl/ContentProcess.idl b/src/cascadia/TerminalControl/ContentProcess.idl index 998330cd667..0f648fff7af 100644 --- a/src/cascadia/TerminalControl/ContentProcess.idl +++ b/src/cascadia/TerminalControl/ContentProcess.idl @@ -16,6 +16,8 @@ namespace Microsoft.Terminal.Control ControlInteractivity GetInteractivity(); + UInt64 GetPID(); + UInt64 RequestSwapChainHandle(UInt64 pid); event DestructedArgs Destructed; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 7f5b8218cee..887cbd29bc9 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -76,6 +76,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (_contentProc != nullptr) { _interactivity = _contentProc.GetInteractivity(); + _contentWaitInterrupt.create(); + _createContentWaitThread(); } } @@ -170,6 +172,148 @@ namespace winrt::Microsoft::Terminal::Control::implementation _ApplyUISettings(); } + bool s_waitOnContentProcess(uint64_t contentPid, HANDLE contentWaitInterrupt) + { + // This is the array of HANDLEs that we're going to wait on in + // WaitForMultipleObjects below. + // * waits[0] will be the handle to the monarch process. It gets + // signalled when the process exits / dies. + // * waits[1] is the handle to our _contentWaitInterrupt event. Another + // thread can use that to manually break this loop. We'll do that when + // we're getting torn down. + HANDLE waits[2]; + waits[1] = contentWaitInterrupt; + bool displayError = true; + // bool exitThreadRequested = false; + // while (!exitThreadRequested) + // { + // At any point in all this, the current monarch might die. If it + // does, we'll go straight to a new election, in the "jail" + // try/catch below. Worst case, eventually, we'll become the new + // monarch. + try + { + // This might fail to even ask the monarch for it's PID. + wil::unique_handle hContent{ OpenProcess(PROCESS_ALL_ACCESS, + FALSE, + static_cast(contentPid)) }; + + // If we fail to open the content, then they don't exist + // anymore! Go straight to an election. + if (hContent.get() == nullptr) + { + const auto gle = GetLastError(); + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_FailedToOpenContent", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingUInt64(gle, "lastError", "The result of GetLastError"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + // exitThreadRequested = true; + // continue; + } + + waits[0] = hContent.get(); + + switch (WaitForMultipleObjects(2, waits, FALSE, INFINITE)) + { + case WAIT_OBJECT_0 + 0: // waits[0] was signaled, the handle to the monarch process + + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_ContentDied", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // Connect to the new monarch, which might be us! + // If we become the monarch, then we'll return true and exit this thread. + // exitThreadRequested = true; + break; + + case WAIT_OBJECT_0 + 1: // waits[1] was signaled, our manual interrupt + + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_ContentWaitInterrupted", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // exitThreadRequested = true; + displayError = false; + break; + + case WAIT_TIMEOUT: + // This should be impossible. + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_ContentWaitTimeout", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // exitThreadRequested = true; + break; + + default: + { + // TODO! + // Returning any other value is invalid. Just die. + const auto gle = GetLastError(); + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_WaitFailed", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingUInt64(gle, "lastError", "The result of GetLastError"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // exitThreadRequested = true; + // ExitProcess(0); + } + } + } + catch (...) + { + // Theoretically, if window[1] dies when we're trying to get + // it's PID we'll get here. We can probably just exit here. + + TraceLoggingWrite(g_hTerminalControlProvider, + "TermControl_ExceptionInWaitThread", + // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // exitThreadRequested = true; + } + // } + + // if (displayError) + // { + // } + return displayError; + } + + void TermControl::_createContentWaitThread() + { + _contentWaitThread = std::thread([weakThis = get_weak(), contentPid = _contentProc.GetPID(), contentWaitInterrupt=_contentWaitInterrupt.get()] { + if (s_waitOnContentProcess(contentPid, contentWaitInterrupt)) + { + if (auto control{ weakThis.get() }) + { + control->_contentWaitThread.detach(); + control->_raiseContentDied(); + } + } + }); + } + + winrt::fire_and_forget TermControl::_raiseContentDied() + { + auto weakThis{ get_weak() }; + co_await winrt::resume_foreground(Dispatcher()); + + if (auto control{ weakThis.get() }) + { + // dialog the thing + auto noticeArgs = winrt::make(NoticeLevel::Error, L"The content of this terminal died unexpectedly."); + control->_RaiseNoticeHandlers(*control, noticeArgs); + } + } + // Method Description: // - Loads the search box from the xaml UI and focuses it. void TermControl::CreateSearchBoxControl() diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index c37be8f3a92..46f8ab57145 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -191,6 +191,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::Windows::UI::Xaml::Controls::SwapChainPanel::LayoutUpdated_revoker _layoutUpdatedRevoker; + wil::unique_event _contentWaitInterrupt; + std::thread _contentWaitThread; + void _createContentWaitThread(); + // void _waitOnContentProcess(); + inline bool _IsClosing() const noexcept { #ifndef NDEBUG @@ -281,7 +286,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::fire_and_forget _coreTransparencyChanged(IInspectable sender, Control::TransparencyChangedEventArgs args); void _coreRaisedNotice(const IInspectable& s, const Control::NoticeEventArgs& args); void _coreWarningBell(const IInspectable& sender, const IInspectable& args); + void _coreFoundMatch(const IInspectable& sender, const Control::FoundResultsArgs& args); + + winrt::fire_and_forget _raiseContentDied(); }; } From ea9d3cb5f7358fd9efac09038de9532eee2745c2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 10:17:41 -0500 Subject: [PATCH 10/53] a fix for a crash when closing --- src/cascadia/TerminalControl/TermControl.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 887cbd29bc9..3d6af093f48 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -736,6 +736,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation TermControl::~TermControl() { + _contentWaitInterrupt.SetEvent(); + _contentWaitThread.join(); Close(); } From 4a8f0e956241b4011c97622c773e1e783b9c5492 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 10:18:16 -0500 Subject: [PATCH 11/53] The sample app has a hard time loading TermControl resources so we're just going to disable this for now --- src/cascadia/TerminalControl/TermControlAutomationPeer.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp index b0203d24208..bdc2bc9cb28 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp @@ -261,7 +261,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation hstring TermControlAutomationPeer::GetLocalizedControlTypeCore() const { - return RS_(L"TerminalControl_ControlType"); + // return RS_(L"TerminalControl_ControlType"); + return L"foo"; } Windows::Foundation::IInspectable TermControlAutomationPeer::GetPatternCore(PatternInterface patternInterface) const From eeb01a139ddf6c8a80ecd14598b97de46562dfee Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 10:18:47 -0500 Subject: [PATCH 12/53] this was the part where I realized I dun goofed --- .../TerminalControl/InteractivityAutomationPeer.idl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl index 9eb79a6329c..772f8d5efb9 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl @@ -2,11 +2,18 @@ // Licensed under the MIT license. namespace Microsoft.Terminal.Control { - [default_interface] runtimeclass InteractivityAutomationPeer : + [default_interface] runtimeclass InteractivityAutomationPeer/* : Windows.UI.Xaml.Automation.Peers.AutomationPeer, - Windows.UI.Xaml.Automation.Provider.ITextProvider + Windows.UI.Xaml.Automation.Provider.ITextProvider*/ { + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetSelection(); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetVisibleRanges(); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromChild(Windows.UI.Xaml.Automation.Provider.IRawElementProviderSimple childElement); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromPoint(Windows.Foundation.Point screenLocation); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider DocumentRange(); + Windows.UI.Xaml.Automation.SupportedTextSelection SupportedTextSelection(); + void SetControlBounds(Windows.Foundation.Rect bounds); void SetControlPadding(Microsoft.Terminal.Core.Padding padding); void ParentProvider(Windows.UI.Xaml.Automation.Peers.AutomationPeer parentProvider); From dafb627278f868ca07bc2cc2c44968811920be5a Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 10:18:54 -0500 Subject: [PATCH 13/53] Revert "this was the part where I realized I dun goofed" This reverts commit 64533c838aa1f4a4f58682bba3fa8949acd5fe56. --- .../TerminalControl/InteractivityAutomationPeer.idl | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl index 772f8d5efb9..9eb79a6329c 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl @@ -2,18 +2,11 @@ // Licensed under the MIT license. namespace Microsoft.Terminal.Control { - [default_interface] runtimeclass InteractivityAutomationPeer/* : + [default_interface] runtimeclass InteractivityAutomationPeer : Windows.UI.Xaml.Automation.Peers.AutomationPeer, - Windows.UI.Xaml.Automation.Provider.ITextProvider*/ + Windows.UI.Xaml.Automation.Provider.ITextProvider { - Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetSelection(); - Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetVisibleRanges(); - Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromChild(Windows.UI.Xaml.Automation.Provider.IRawElementProviderSimple childElement); - Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromPoint(Windows.Foundation.Point screenLocation); - Windows.UI.Xaml.Automation.Provider.ITextRangeProvider DocumentRange(); - Windows.UI.Xaml.Automation.SupportedTextSelection SupportedTextSelection(); - void SetControlBounds(Windows.Foundation.Rect bounds); void SetControlPadding(Microsoft.Terminal.Core.Padding padding); void ParentProvider(Windows.UI.Xaml.Automation.Peers.AutomationPeer parentProvider); From 1a78557b61b5f5ddccd8465c616a7cffd22505cd Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 11:17:05 -0500 Subject: [PATCH 14/53] This too didn't work. Creating the XAML thing not on the XAML thing isn't going to work --- src/cascadia/TerminalControl/ControlCore.cpp | 5 +++++ src/cascadia/TerminalControl/ControlCore.h | 2 ++ .../TerminalControl/ControlInteractivity.cpp | 19 +++++++++++++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 820686d322b..1f28562b73d 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1695,4 +1695,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation // transparency, or our acrylic, or our image. return Opacity() < 1.0f || UseAcrylic() || !_settings->BackgroundImage().empty(); } + + winrt::Windows::System::DispatcherQueue ControlCore::Dispatcher() + { + return _dispatcher; + } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index cc3484dd1e7..28f4f583ced 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -168,6 +168,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation void AdjustOpacity(const double opacity, const bool relative); + winrt::Windows::System::DispatcherQueue Dispatcher(); + RUNTIME_SETTING(double, Opacity, _settings->Opacity()); RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic()); diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index de992e1b379..920683831b0 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -16,6 +16,7 @@ #include "ControlInteractivity.g.cpp" #include "TermControl.h" +#include using namespace ::Microsoft::Console::Types; using namespace ::Microsoft::Console::VirtualTerminal; @@ -626,11 +627,21 @@ namespace winrt::Microsoft::Terminal::Control::implementation Control::InteractivityAutomationPeer ControlInteractivity::OnCreateAutomationPeer() try { - const auto autoPeer = winrt::make_self(this); + til::latch latch{ 1 }; + Control::InteractivityAutomationPeer peer{ nullptr }; - _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get()); - _core->AttachUiaEngine(_uiaEngine.get()); - return *autoPeer; + _core->Dispatcher().TryEnqueue([this, &peer, &latch]() { + auto autoPeer = winrt::make_self(this); + + _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get()); + _core->AttachUiaEngine(_uiaEngine.get()); + + peer = *autoPeer; + latch.count_down(); + }); + + latch.wait(); + return peer; } catch (...) { From 35ca313b481ee2dbe07ce0035cb5a1cdaeedba34 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 11:17:16 -0500 Subject: [PATCH 15/53] Revert "This too didn't work. Creating the XAML thing not on the XAML thing isn't going to work" This reverts commit fd364db7275a5574e7c0793b82c4fb7e87234d44. --- src/cascadia/TerminalControl/ControlCore.cpp | 4 ---- src/cascadia/TerminalControl/ControlCore.h | 2 -- .../TerminalControl/ControlInteractivity.cpp | 19 ++++--------------- 3 files changed, 4 insertions(+), 21 deletions(-) diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 1f28562b73d..13be3ae5434 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1696,8 +1696,4 @@ namespace winrt::Microsoft::Terminal::Control::implementation return Opacity() < 1.0f || UseAcrylic() || !_settings->BackgroundImage().empty(); } - winrt::Windows::System::DispatcherQueue ControlCore::Dispatcher() - { - return _dispatcher; - } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 28f4f583ced..cc3484dd1e7 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -168,8 +168,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation void AdjustOpacity(const double opacity, const bool relative); - winrt::Windows::System::DispatcherQueue Dispatcher(); - RUNTIME_SETTING(double, Opacity, _settings->Opacity()); RUNTIME_SETTING(bool, UseAcrylic, _settings->UseAcrylic()); diff --git a/src/cascadia/TerminalControl/ControlInteractivity.cpp b/src/cascadia/TerminalControl/ControlInteractivity.cpp index 920683831b0..4f512ed663e 100644 --- a/src/cascadia/TerminalControl/ControlInteractivity.cpp +++ b/src/cascadia/TerminalControl/ControlInteractivity.cpp @@ -16,7 +16,6 @@ #include "ControlInteractivity.g.cpp" #include "TermControl.h" -#include using namespace ::Microsoft::Console::Types; using namespace ::Microsoft::Console::VirtualTerminal; @@ -627,21 +626,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation Control::InteractivityAutomationPeer ControlInteractivity::OnCreateAutomationPeer() try { - til::latch latch{ 1 }; - Control::InteractivityAutomationPeer peer{ nullptr }; + auto autoPeer = winrt::make_self(this); - _core->Dispatcher().TryEnqueue([this, &peer, &latch]() { - auto autoPeer = winrt::make_self(this); - - _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get()); - _core->AttachUiaEngine(_uiaEngine.get()); - - peer = *autoPeer; - latch.count_down(); - }); - - latch.wait(); - return peer; + _uiaEngine = std::make_unique<::Microsoft::Console::Render::UiaEngine>(autoPeer.get()); + _core->AttachUiaEngine(_uiaEngine.get()); + return *autoPeer; } catch (...) { From 8ceb9baf9a91885e7bf6caa2992b1b94c34ed665 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Aug 2021 13:04:55 -0500 Subject: [PATCH 16/53] This doesn't immediately crash, but it does crash when you start asking for the actual text ranges. That's not what you want. --- .../InteractivityAutomationPeer.cpp | 25 ++++++++++++++++--- .../InteractivityAutomationPeer.h | 4 +-- .../InteractivityAutomationPeer.idl | 13 +++++++--- src/cascadia/TerminalControl/TermControl.cpp | 3 ++- .../TermControlAutomationPeer.cpp | 6 +++++ .../TermControlAutomationPeer.h | 3 +++ .../TermControlAutomationPeer.idl | 1 + .../TerminalControlLib.vcxproj | 9 +++++-- .../TerminalControl/XamlUiaTextRange.h | 4 +-- .../TerminalControl/XamlUiaTextRange.idl | 10 ++++++++ 10 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 src/cascadia/TerminalControl/XamlUiaTextRange.idl diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp b/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp index 00621afcab5..01afd9948df 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.cpp @@ -48,6 +48,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation _parentProvider = parentProvider; } + void InteractivityAutomationPeer::SetParentProvider(Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple parentProvider) + { + _parentProvider = parentProvider; + } + // Method Description: // - Signals the ui automation client that the terminal's selection has // changed and should be updated @@ -119,21 +124,33 @@ namespace winrt::Microsoft::Terminal::Control::implementation // ScreenInfoUiaProvider doesn't actually use parameter, so just pass in nullptr THROW_IF_FAILED(_uiaProvider->RangeFromChild(/* IRawElementProviderSimple */ nullptr, &returnVal)); - return _CreateXamlUiaTextRange(returnVal); + + // const auto parentProvider = this->ProviderFromPeer(*this); + const auto parentProvider = _parentProvider; + const auto xutr = winrt::make_self(returnVal, parentProvider); + return xutr.as(); } XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::RangeFromPoint(Windows::Foundation::Point screenLocation) { UIA::ITextRangeProvider* returnVal; THROW_IF_FAILED(_uiaProvider->RangeFromPoint({ screenLocation.X, screenLocation.Y }, &returnVal)); - return _CreateXamlUiaTextRange(returnVal); + + // const auto parentProvider = this->ProviderFromPeer(*this); + const auto parentProvider = _parentProvider; + const auto xutr = winrt::make_self(returnVal, parentProvider); + return xutr.as(); } XamlAutomation::ITextRangeProvider InteractivityAutomationPeer::DocumentRange() { UIA::ITextRangeProvider* returnVal; THROW_IF_FAILED(_uiaProvider->get_DocumentRange(&returnVal)); - return _CreateXamlUiaTextRange(returnVal); + + // const auto parentProvider = this->ProviderFromPeer(*this); + const auto parentProvider = _parentProvider; + const auto xutr = winrt::make_self(returnVal, parentProvider); + return xutr.as(); } XamlAutomation::SupportedTextSelection InteractivityAutomationPeer::SupportedTextSelection() @@ -208,6 +225,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::vector vec; vec.reserve(count); + // auto parentProvider = this->ProviderFromPeer(*this); + const auto parentProvider = _parentProvider; for (int i = 0; i < count; i++) { if (auto xutr = _CreateXamlUiaTextRange(providers[i].detach())) diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.h b/src/cascadia/TerminalControl/InteractivityAutomationPeer.h index 9f0d96fead5..54b0d7e27a6 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.h +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.h @@ -43,7 +43,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void SetControlBounds(const Windows::Foundation::Rect bounds); void SetControlPadding(const Core::Padding padding); - void ParentProvider(Windows::UI::Xaml::Automation::Peers::AutomationPeer parentProvider); + void SetParentProvider(Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple parentProvider); #pragma region IUiaEventDispatcher void SignalSelectionChanged() override; @@ -81,7 +81,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation ::Microsoft::WRL::ComPtr<::Microsoft::Terminal::TermControlUiaProvider> _uiaProvider; winrt::Microsoft::Terminal::Control::implementation::ControlInteractivity* _interactivity; - weak_ref _parentProvider; + winrt::Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple _parentProvider{ nullptr }; til::rect _controlBounds{}; til::rect _controlPadding{}; diff --git a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl index 9eb79a6329c..e5b748827c1 100644 --- a/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl +++ b/src/cascadia/TerminalControl/InteractivityAutomationPeer.idl @@ -2,14 +2,21 @@ // Licensed under the MIT license. namespace Microsoft.Terminal.Control { - [default_interface] runtimeclass InteractivityAutomationPeer : + [default_interface] runtimeclass InteractivityAutomationPeer/* : Windows.UI.Xaml.Automation.Peers.AutomationPeer, - Windows.UI.Xaml.Automation.Provider.ITextProvider + Windows.UI.Xaml.Automation.Provider.ITextProvider*/ { + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetSelection(); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider[] GetVisibleRanges(); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromChild(Windows.UI.Xaml.Automation.Provider.IRawElementProviderSimple childElement); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider RangeFromPoint(Windows.Foundation.Point screenLocation); + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider DocumentRange(); + Windows.UI.Xaml.Automation.SupportedTextSelection SupportedTextSelection(); + void SetControlBounds(Windows.Foundation.Rect bounds); void SetControlPadding(Microsoft.Terminal.Core.Padding padding); - void ParentProvider(Windows.UI.Xaml.Automation.Peers.AutomationPeer parentProvider); + void SetParentProvider(Windows.UI.Xaml.Automation.Provider.IRawElementProviderSimple provider); event Windows.Foundation.TypedEventHandler SelectionChanged; event Windows.Foundation.TypedEventHandler TextChanged; diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 3d6af093f48..05fca6fc424 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -289,7 +289,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void TermControl::_createContentWaitThread() { - _contentWaitThread = std::thread([weakThis = get_weak(), contentPid = _contentProc.GetPID(), contentWaitInterrupt=_contentWaitInterrupt.get()] { + _contentWaitThread = std::thread([weakThis = get_weak(), contentPid = _contentProc.GetPID(), contentWaitInterrupt = _contentWaitInterrupt.get()] { if (s_waitOnContentProcess(contentPid, contentWaitInterrupt)) { if (auto control{ weakThis.get() }) @@ -764,6 +764,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation margins.Right, margins.Bottom }; _automationPeer = winrt::make(this, padding, interactivityAutoPeer); + interactivityAutoPeer.SetParentProvider(_automationPeer.GetParentProvider()); return _automationPeer; } } diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp index bdc2bc9cb28..11ae031b083 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.cpp @@ -117,6 +117,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation } } + XamlAutomation::IRawElementProviderSimple TermControlAutomationPeer::GetParentProvider() + { + const auto parentProvider = this->ProviderFromPeer(*this); + return parentProvider; + } + // Method Description: // - Signals the ui automation client that the terminal's selection has changed and should be updated // Arguments: diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.h b/src/cascadia/TerminalControl/TermControlAutomationPeer.h index 758864bc38f..9b44b93f1f8 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.h +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.h @@ -48,8 +48,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation void UpdateControlBounds(); void SetControlPadding(const Core::Padding padding); + void RecordKeyEvent(const WORD vkey); + Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple GetParentProvider(); + #pragma region FrameworkElementAutomationPeer hstring GetClassNameCore() const; Windows::UI::Xaml::Automation::Peers::AutomationControlType GetAutomationControlTypeCore() const; diff --git a/src/cascadia/TerminalControl/TermControlAutomationPeer.idl b/src/cascadia/TerminalControl/TermControlAutomationPeer.idl index 4ca83f0d9c1..01409ecea9f 100644 --- a/src/cascadia/TerminalControl/TermControlAutomationPeer.idl +++ b/src/cascadia/TerminalControl/TermControlAutomationPeer.idl @@ -12,5 +12,6 @@ namespace Microsoft.Terminal.Control void UpdateControlBounds(); void SetControlPadding(Microsoft.Terminal.Core.Padding padding); + Windows.UI.Xaml.Automation.Provider.IRawElementProviderSimple GetParentProvider(); } } diff --git a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj index 3da4e391d5b..208831af901 100644 --- a/src/cascadia/TerminalControl/TerminalControlLib.vcxproj +++ b/src/cascadia/TerminalControl/TerminalControlLib.vcxproj @@ -67,7 +67,9 @@ TSFInputControl.xaml - + + XamlUiaTextRange.idl + @@ -109,7 +111,9 @@ InteractivityAutomationPeer.idl - + + XamlUiaTextRange.idl + @@ -136,6 +140,7 @@ TSFInputControl.xaml + diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.h b/src/cascadia/TerminalControl/XamlUiaTextRange.h index 99242c59bca..a54049f7df6 100644 --- a/src/cascadia/TerminalControl/XamlUiaTextRange.h +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.h @@ -21,13 +21,13 @@ Author(s): #pragma once #include "TermControlAutomationPeer.h" +#include "XamlUiaTextRange.g.h" #include #include "../types/TermControlUiaTextRange.hpp" namespace winrt::Microsoft::Terminal::Control::implementation { - class XamlUiaTextRange : - public winrt::implements + class XamlUiaTextRange : public XamlUiaTextRangeT { public: XamlUiaTextRange(::ITextRangeProvider* uiaProvider, Windows::UI::Xaml::Automation::Provider::IRawElementProviderSimple parentProvider) : diff --git a/src/cascadia/TerminalControl/XamlUiaTextRange.idl b/src/cascadia/TerminalControl/XamlUiaTextRange.idl new file mode 100644 index 00000000000..d75bcae65d3 --- /dev/null +++ b/src/cascadia/TerminalControl/XamlUiaTextRange.idl @@ -0,0 +1,10 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +namespace Microsoft.Terminal.Control +{ + [default_interface] runtimeclass XamlUiaTextRange : + Windows.UI.Xaml.Automation.Provider.ITextRangeProvider + { + } +} From 768d4c0df3aaae542a97fb1d94c14a4343ca5d66 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 24 Aug 2021 10:40:30 -0500 Subject: [PATCH 17/53] some cleanup --- src/cascadia/TerminalControl/TermControl.cpp | 67 +++++++------------- src/cascadia/TerminalControl/TermControl.h | 2 +- 2 files changed, 24 insertions(+), 45 deletions(-) diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index 05fca6fc424..e1e336a15b9 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -172,11 +172,16 @@ namespace winrt::Microsoft::Terminal::Control::implementation _ApplyUISettings(); } + bool TermControl::_contentIsOutOfProc() const + { + return _contentProc != nullptr; + } + bool s_waitOnContentProcess(uint64_t contentPid, HANDLE contentWaitInterrupt) { // This is the array of HANDLEs that we're going to wait on in // WaitForMultipleObjects below. - // * waits[0] will be the handle to the monarch process. It gets + // * waits[0] will be the handle to the content process. It gets // signalled when the process exits / dies. // * waits[1] is the handle to our _contentWaitInterrupt event. Another // thread can use that to manually break this loop. We'll do that when @@ -184,60 +189,48 @@ namespace winrt::Microsoft::Terminal::Control::implementation HANDLE waits[2]; waits[1] = contentWaitInterrupt; bool displayError = true; - // bool exitThreadRequested = false; - // while (!exitThreadRequested) - // { - // At any point in all this, the current monarch might die. If it - // does, we'll go straight to a new election, in the "jail" - // try/catch below. Worst case, eventually, we'll become the new - // monarch. + + // At any point in all this, the content process might die. If it does, + // we want to raise an error message, to inform that this control is now + // dead. try { - // This might fail to even ask the monarch for it's PID. + // This might fail to even ask the content for it's PID. wil::unique_handle hContent{ OpenProcess(PROCESS_ALL_ACCESS, FALSE, static_cast(contentPid)) }; // If we fail to open the content, then they don't exist - // anymore! Go straight to an election. + // anymore! We'll need to immediately raise the notification that the content has died. if (hContent.get() == nullptr) { const auto gle = GetLastError(); TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_FailedToOpenContent", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingUInt64(gle, "lastError", "The result of GetLastError"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - - // exitThreadRequested = true; - // continue; + return displayError; } waits[0] = hContent.get(); switch (WaitForMultipleObjects(2, waits, FALSE, INFINITE)) { - case WAIT_OBJECT_0 + 0: // waits[0] was signaled, the handle to the monarch process + case WAIT_OBJECT_0 + 0: // waits[0] was signaled, the handle to the content process TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_ContentDied", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // Connect to the new monarch, which might be us! - // If we become the monarch, then we'll return true and exit this thread. - // exitThreadRequested = true; break; case WAIT_OBJECT_0 + 1: // waits[1] was signaled, our manual interrupt TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_ContentWaitInterrupted", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // exitThreadRequested = true; displayError = false; break; @@ -245,25 +238,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation // This should be impossible. TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_ContentWaitTimeout", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // exitThreadRequested = true; break; default: { - // TODO! // Returning any other value is invalid. Just die. const auto gle = GetLastError(); TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_WaitFailed", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingUInt64(gle, "lastError", "The result of GetLastError"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // exitThreadRequested = true; - // ExitProcess(0); } } } @@ -274,16 +261,9 @@ namespace winrt::Microsoft::Terminal::Control::implementation TraceLoggingWrite(g_hTerminalControlProvider, "TermControl_ExceptionInWaitThread", - // TraceLoggingUInt64(peasantID, "peasantID", "Our peasant ID"), TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); - // exitThreadRequested = true; } - // } - - // if (displayError) - // { - // } return displayError; } @@ -294,7 +274,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation { if (auto control{ weakThis.get() }) { - control->_contentWaitThread.detach(); + // control->_contentWaitThread.detach(); control->_raiseContentDied(); } } @@ -736,8 +716,11 @@ namespace winrt::Microsoft::Terminal::Control::implementation TermControl::~TermControl() { - _contentWaitInterrupt.SetEvent(); - _contentWaitThread.join(); + if (_contentIsOutOfProc()) + { + _contentWaitInterrupt.SetEvent(); + _contentWaitThread.join(); + } Close(); } @@ -800,7 +783,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (auto control{ weakThis.get() }) { // TODO! very good chance we leak this handle - const HANDLE chainHandle = reinterpret_cast(control->_contentProc ? + const HANDLE chainHandle = reinterpret_cast(control->_contentIsOutOfProc() ? control->_contentProc.RequestSwapChainHandle(GetCurrentProcessId()) : control->_core.SwapChainHandle()); _AttachDxgiSwapChainToXaml(chainHandle); @@ -891,7 +874,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation _interactivity.Initialize(); // TODO! very good chance we leak this handle - const HANDLE chainHandle = reinterpret_cast(_contentProc ? + const HANDLE chainHandle = reinterpret_cast(_contentIsOutOfProc() ? _contentProc.RequestSwapChainHandle(GetCurrentProcessId()) : _core.SwapChainHandle()); _AttachDxgiSwapChainToXaml(chainHandle); @@ -2010,14 +1993,10 @@ namespace winrt::Microsoft::Terminal::Control::implementation // Disconnect the TSF input control so it doesn't receive EditContext events. TSFInputControl().Close(); _autoScrollTimer.Stop(); - // try - // { - if (!_contentProc) + if (!_contentIsOutOfProc()) { _core.Close(); } - // } - // CATCH_LOG(); } } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 46f8ab57145..2811b18f252 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -194,7 +194,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation wil::unique_event _contentWaitInterrupt; std::thread _contentWaitThread; void _createContentWaitThread(); - // void _waitOnContentProcess(); + bool _contentIsOutOfProc() const; inline bool _IsClosing() const noexcept { From 651efe248b1b2ac187e2b6df62d58b625285584b Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 24 Aug 2021 12:20:36 -0500 Subject: [PATCH 18/53] add a dialog internally to the bounds of the control, not outside of the control --- scratch/ScratchIslandApp/SampleApp/MyPage.cpp | 14 +++++++- .../Resources/en-US/Resources.resw | 6 ++++ src/cascadia/TerminalControl/TermControl.cpp | 36 ++++++++++++++++--- src/cascadia/TerminalControl/TermControl.h | 4 ++- src/cascadia/TerminalControl/TermControl.xaml | 21 +++++++++++ 5 files changed, 74 insertions(+), 7 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp index 77a311a7aaa..68aabdb777a 100644 --- a/scratch/ScratchIslandApp/SampleApp/MyPage.cpp +++ b/scratch/ScratchIslandApp/SampleApp/MyPage.cpp @@ -209,6 +209,19 @@ namespace winrt::SampleApp::implementation piContentProcess.reset(); } }); + control.ConnectionStateChanged([this, control](auto&&, auto&) { + const auto newConnectionState = control.ConnectionState(); + if (newConnectionState == TerminalConnection::ConnectionState::Closed) + { + _writeToLog(L"Connection was closed"); + OutOfProcContent().Children().Clear(); + GuidInput().Text(L""); + if (piContentProcess.hProcess) + { + piContentProcess.reset(); + } + } + }); Log().Children().Clear(); OutOfProcContent().Children().Append(control); @@ -241,7 +254,6 @@ namespace winrt::SampleApp::implementation } } - // Method Description: // - Gets the title of the currently focused terminal control. If there // isn't a control selected for any reason, returns "Windows Terminal" diff --git a/src/cascadia/TerminalControl/Resources/en-US/Resources.resw b/src/cascadia/TerminalControl/Resources/en-US/Resources.resw index 4c17742c1ce..1dc0a3590cd 100644 --- a/src/cascadia/TerminalControl/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalControl/Resources/en-US/Resources.resw @@ -208,4 +208,10 @@ Please either install the missing font or choose another one. No results found Announced to a screen reader when the user searches for some text and there are no matches for that text in the terminal. + + The content of this terminal was closed unexpectedly. + + + Close + diff --git a/src/cascadia/TerminalControl/TermControl.cpp b/src/cascadia/TerminalControl/TermControl.cpp index e1e336a15b9..2e2b0e864c4 100644 --- a/src/cascadia/TerminalControl/TermControl.cpp +++ b/src/cascadia/TerminalControl/TermControl.cpp @@ -98,6 +98,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation // This event is specifically triggered by the renderer thread, a BG thread. Use a weak ref here. _core.RendererEnteredErrorState({ get_weak(), &TermControl::_RendererEnteredErrorState }); + _core.ConnectionStateChanged({ get_weak(), &TermControl::_coreConnectionStateChanged }); + // These callbacks can only really be triggered by UI interactions. So // they don't need weak refs - they can't be triggered unless we're // alive. @@ -272,9 +274,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation _contentWaitThread = std::thread([weakThis = get_weak(), contentPid = _contentProc.GetPID(), contentWaitInterrupt = _contentWaitInterrupt.get()] { if (s_waitOnContentProcess(contentPid, contentWaitInterrupt)) { + // When s_waitOnContentProcess returns, if it returned true, we + // should display a dialog in our bounds to indicate that we + // were closed unexpectedly. If we closed in an expected way, + // then s_waitOnContentProcess will return false. if (auto control{ weakThis.get() }) { - // control->_contentWaitThread.detach(); control->_raiseContentDied(); } } @@ -288,11 +293,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation if (auto control{ weakThis.get() }) { - // dialog the thing - auto noticeArgs = winrt::make(NoticeLevel::Error, L"The content of this terminal died unexpectedly."); - control->_RaiseNoticeHandlers(*control, noticeArgs); + if (auto loadedUiElement{ FindName(L"ContentDiedNotice") }) + { + if (auto uiElement{ loadedUiElement.try_as<::winrt::Windows::UI::Xaml::UIElement>() }) + { + uiElement.Visibility(Visibility::Visible); + } + } } } + // Method Description: + // - Handler for when the "Content Died" dialog's button is clicked. + void TermControl::_ContentDiedCloseButton_Click(IInspectable const& /*sender*/, IInspectable const& /*args*/) + { + // Alert whoever's hosting us that the connection was closed. + // When they come asking what the new connection state is, we'll reply with Closed + _ConnectionStateChangedHandlers(*this, nullptr); + } // Method Description: // - Loads the search box from the xaml UI and focuses it. @@ -768,7 +785,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation TerminalConnection::ConnectionState TermControl::ConnectionState() const { - return _core.ConnectionState(); + try + { + return _core.ConnectionState(); + } + CATCH_LOG(); + return TerminalConnection::ConnectionState::Closed; } winrt::fire_and_forget TermControl::RenderEngineSwapChainChanged(IInspectable /*sender*/, IInspectable /*args*/) @@ -2955,4 +2977,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation } } + void TermControl::_coreConnectionStateChanged(const IInspectable& /*sender*/, const IInspectable& /*args*/) + { + _ConnectionStateChangedHandlers(*this, nullptr); + } } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index 2811b18f252..a1f24b051de 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -82,6 +82,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation winrt::fire_and_forget _RendererEnteredErrorState(IInspectable sender, IInspectable args); void _RenderRetryButton_Click(IInspectable const& button, IInspectable const& args); + void _ContentDiedCloseButton_Click(IInspectable const& button, IInspectable const& args); winrt::fire_and_forget _RendererWarning(IInspectable sender, Control::RendererWarningArgs args); @@ -125,7 +126,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation PROJECTED_FORWARDED_TYPED_EVENT(TitleChanged, IInspectable, Control::TitleChangedEventArgs, _core, TitleChanged); PROJECTED_FORWARDED_TYPED_EVENT(TabColorChanged, IInspectable, IInspectable, _core, TabColorChanged); PROJECTED_FORWARDED_TYPED_EVENT(SetTaskbarProgress, IInspectable, IInspectable, _core, TaskbarProgressChanged); - PROJECTED_FORWARDED_TYPED_EVENT(ConnectionStateChanged, IInspectable, IInspectable, _core, ConnectionStateChanged); + TYPED_EVENT(ConnectionStateChanged, IInspectable, IInspectable); PROJECTED_FORWARDED_TYPED_EVENT(PasteFromClipboard, IInspectable, Control::PasteFromClipboardEventArgs, _interactivity, PasteFromClipboard); @@ -290,6 +291,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation void _coreFoundMatch(const IInspectable& sender, const Control::FoundResultsArgs& args); winrt::fire_and_forget _raiseContentDied(); + void _coreConnectionStateChanged(const IInspectable& sender, const IInspectable& args); }; } diff --git a/src/cascadia/TerminalControl/TermControl.xaml b/src/cascadia/TerminalControl/TermControl.xaml index 589f9d4c3f7..ddc30b05762 100644 --- a/src/cascadia/TerminalControl/TermControl.xaml +++ b/src/cascadia/TerminalControl/TermControl.xaml @@ -1272,6 +1272,27 @@ + + + + + - -