From 874a15b89bf88ac071531bf41b4e6c94a09f80d2 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Tue, 19 Jul 2022 11:14:17 -0500 Subject: [PATCH] Good-news-everyone.gif This hooks up the defterm interface to pass more info along in the startup. It's notably not the actual link title, but it does work from a plumbing standpoint This is for #9458 --- src/cascadia/TerminalApp/TerminalPage.cpp | 1 + .../TerminalConnection/CTerminalHandoff.cpp | 4 +- .../TerminalConnection/CTerminalHandoff.h | 5 +- .../TerminalConnection/ConptyConnection.cpp | 33 +++++++- .../TerminalConnection/ConptyConnection.h | 16 +++- .../TerminalConnection/ConptyConnection.idl | 1 + src/host/proxy/ITerminalHandoff.idl | 11 ++- src/host/srvinit.cpp | 77 ++++++++++++++++++- .../inc/ISystemConfigurationProvider.hpp | 9 ++- .../onecore/SystemConfigurationProvider.cpp | 3 +- .../win32/SystemConfigurationProvider.cpp | 13 +++- .../win32/SystemConfigurationProvider.hpp | 3 +- 12 files changed, 159 insertions(+), 17 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index f4b2f262d62..fdd1699f8b7 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3191,6 +3191,7 @@ namespace winrt::TerminalApp::implementation { NewTerminalArgs newTerminalArgs; newTerminalArgs.Commandline(connection.Commandline()); + newTerminalArgs.TabTitle(connection.Title()); // GH #12370: We absolutely cannot allow a defterm connection to // auto-elevate. Defterm doesn't work for elevated scenarios in the // first place. If we try accepting the connection, the spawning an diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp index 1f8d089a626..cf33c4002a1 100644 --- a/src/cascadia/TerminalConnection/CTerminalHandoff.cpp +++ b/src/cascadia/TerminalConnection/CTerminalHandoff.cpp @@ -102,7 +102,7 @@ static HRESULT _duplicateHandle(const HANDLE in, HANDLE& out) noexcept // - E_NOT_VALID_STATE if a event handler is not registered before calling. `::DuplicateHandle` // error codes if we cannot manage to make our own copy of handles to retain. Or S_OK/error // from the registered handler event function. -HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) +HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) { try { @@ -132,7 +132,7 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE in, HANDLE out, HANDLE sign THROW_IF_FAILED(_duplicateHandle(client, client)); // Call registered handler from when we started listening. - THROW_IF_FAILED(localPfnHandoff(in, out, signal, ref, server, client)); + THROW_IF_FAILED(localPfnHandoff(in, out, signal, ref, server, client, startupInfo)); #pragma warning(suppress : 26477) TraceLoggingWrite( diff --git a/src/cascadia/TerminalConnection/CTerminalHandoff.h b/src/cascadia/TerminalConnection/CTerminalHandoff.h index 4e43c7b744c..4325e86c2f0 100644 --- a/src/cascadia/TerminalConnection/CTerminalHandoff.h +++ b/src/cascadia/TerminalConnection/CTerminalHandoff.h @@ -26,7 +26,7 @@ Author(s): #define __CLSID_CTerminalHandoff "051F34EE-C1FD-4B19-AF75-9BA54648434C" #endif -using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, HANDLE); +using NewHandoffFunction = HRESULT (*)(HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, HANDLE, TERMINAL_STARTUP_INFO); struct __declspec(uuid(__CLSID_CTerminalHandoff)) CTerminalHandoff : public Microsoft::WRL::RuntimeClass, ITerminalHandoff> @@ -37,7 +37,8 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff)) HANDLE signal, HANDLE ref, HANDLE server, - HANDLE client) override; + HANDLE client, + TERMINAL_STARTUP_INFO startupInfo) override; #pragma endregion diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 6fee6a797e0..6c5b459db4b 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -203,7 +203,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const HANDLE hOut, const HANDLE hRef, const HANDLE hServerProcess, - const HANDLE hClientProcess) : + const HANDLE hClientProcess, + TERMINAL_STARTUP_INFO startupInfo) : _initialRows{ 25 }, _initialCols{ 80 }, _guid{ Utils::CreateGuid() }, @@ -213,6 +214,27 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC)); _piClient.hProcess = hClientProcess; +#pragma warning(suppress : 26477 26485 26494 26482 26446) // We don't control TraceLoggingWrite + TraceLoggingWrite( + g_hTerminalConnectionProvider, + "ConptyConnection_CreatedForDefTerm", + TraceLoggingDescription("Event emitted when ConPTY connection is started for a defterm connection"), + TraceLoggingGuid(_guid, "SessionGuid", "The new connection's GUID"), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + _startupInfo.title = winrt::hstring{ startupInfo.pszTitle }; + _startupInfo.iconPath = winrt::hstring{ startupInfo.pszIconPath }; + _startupInfo.iconIndex = startupInfo.iconIndex; + +#pragma warning(suppress : 26477 26485 26494 26482 26446) // We don't control TraceLoggingWrite + TraceLoggingWrite( + g_hTerminalConnectionProvider, + "ConPty_GotStartupInfoConnected", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingWideString(_startupInfo.title.c_str(), "Title", "The title from the connection"), + TraceLoggingWideString(_startupInfo.iconPath.c_str(), "Icon", "The icon from the connection"), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // DebugBreak(); try { _commandline = _commandlineFromProcess(hClientProcess); @@ -288,6 +310,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return _commandline; } + winrt::hstring ConptyConnection::Title() const + { + return _startupInfo.title; + } + void ConptyConnection::Start() try { @@ -667,10 +694,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation winrt::event_token ConptyConnection::NewConnection(const NewConnectionHandler& handler) { return _newConnectionHandlers.add(handler); }; void ConptyConnection::NewConnection(const winrt::event_token& token) { _newConnectionHandlers.remove(token); }; - HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept + HRESULT ConptyConnection::NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept try { - _newConnectionHandlers(winrt::make(signal, in, out, ref, server, client)); + _newConnectionHandlers(winrt::make(signal, in, out, ref, server, client, startupInfo)); return S_OK; } diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h index d54ad7e03a3..11d135ff439 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.h +++ b/src/cascadia/TerminalConnection/ConptyConnection.h @@ -8,6 +8,8 @@ #include +#include "ITerminalHandoff.h" + namespace wil { // These belong in WIL upstream, so when we reingest the change that has them we'll get rid of ours. @@ -23,7 +25,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const HANDLE hOut, const HANDLE hRef, const HANDLE hServerProcess, - const HANDLE hClientProcess); + const HANDLE hClientProcess, + TERMINAL_STARTUP_INFO startupInfo); ConptyConnection() noexcept = default; void Initialize(const Windows::Foundation::Collections::ValueSet& settings); @@ -42,6 +45,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation winrt::guid Guid() const noexcept; winrt::hstring Commandline() const; + winrt::hstring Title() const; static void StartInboundListener(); static void StopInboundListener(); @@ -60,7 +64,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation WINRT_CALLBACK(TerminalOutput, TerminalOutputHandler); private: - static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client) noexcept; + static HRESULT NewHandoff(HANDLE in, HANDLE out, HANDLE signal, HANDLE ref, HANDLE server, HANDLE client, TERMINAL_STARTUP_INFO startupInfo) noexcept; static winrt::hstring _commandlineFromProcess(HANDLE process); HRESULT _LaunchAttachedClient() noexcept; @@ -93,6 +97,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation std::array _buffer{}; bool _passthroughMode{}; + struct StartupInfoFromDefTerm + { + winrt::hstring title{}; + winrt::hstring iconPath{}; + int32_t iconIndex{}; + + } _startupInfo{}; + DWORD _OutputThread(); }; } diff --git a/src/cascadia/TerminalConnection/ConptyConnection.idl b/src/cascadia/TerminalConnection/ConptyConnection.idl index 82f51b70db3..67e075d0a18 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.idl +++ b/src/cascadia/TerminalConnection/ConptyConnection.idl @@ -12,6 +12,7 @@ namespace Microsoft.Terminal.TerminalConnection ConptyConnection(); Guid Guid { get; }; String Commandline { get; }; + String Title { get; }; void ClearBuffer(); diff --git a/src/host/proxy/ITerminalHandoff.idl b/src/host/proxy/ITerminalHandoff.idl index d06799658ac..3625bff1565 100644 --- a/src/host/proxy/ITerminalHandoff.idl +++ b/src/host/proxy/ITerminalHandoff.idl @@ -4,6 +4,14 @@ import "oaidl.idl"; import "ocidl.idl"; + +typedef struct _TERMINAL_STARTUP_INFO +{ + /*[in, string] */LPCWSTR pszTitle; + /*[in, string] */LPCWSTR pszIconPath; + LONG iconIndex; +} TERMINAL_STARTUP_INFO; + [ object, uuid(59D55CCE-FC8A-48B4-ACE8-0A9286C6557F) @@ -14,5 +22,6 @@ import "ocidl.idl"; [in, system_handle(sh_pipe)] HANDLE signal, [in, system_handle(sh_file)] HANDLE ref, [in, system_handle(sh_process)] HANDLE server, - [in, system_handle(sh_process)] HANDLE client); + [in, system_handle(sh_process)] HANDLE client, + [in] TERMINAL_STARTUP_INFO startupInfo); }; diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index 285809d7002..5728fe568e1 100644 --- a/src/host/srvinit.cpp +++ b/src/host/srvinit.cpp @@ -18,6 +18,7 @@ #include "../server/Entrypoints.h" #include "../server/IoSorter.h" +#include "../interactivity/inc/ISystemConfigurationProvider.hpp" #include "../interactivity/inc/ServiceLocator.hpp" #include "../interactivity/base/ApiDetector.hpp" #include "../interactivity/base/RemoteConsoleControl.hpp" @@ -179,7 +180,7 @@ static bool s_IsOnDesktop() // We need to see if we were spawned from a link. If we were, we need to // call back into the shell to try to get all the console information from the link. - ServiceLocator::LocateSystemConfigurationProvider()->GetSettingsFromLink(&settings, Title, &TitleLength, CurDir, AppName); + ServiceLocator::LocateSystemConfigurationProvider()->GetSettingsFromLink(&settings, Title, &TitleLength, CurDir, AppName, nullptr); // If we weren't started from a link, this will already be set. // If LoadLinkInfo couldn't find anything, it will remove the flag so we can dig in the registry. @@ -509,12 +510,78 @@ try TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // DebugBreak(); + + g.pDeviceComm = new ConDrvDeviceComm(Server); + connectMessage->_pDeviceComm = g.pDeviceComm; + + // TODO! Here, add the additional info we pulled out of the StartupInfo + // { + // NEW: + Microsoft::Console::Interactivity::IconInfo icon; + // Old + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_BeforeConsoleInitializeConnectInfo", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + CONSOLE_API_CONNECTINFO Cac; + THROW_IF_NTSTATUS_FAILED(ConsoleInitializeConnectInfo(connectMessage, &Cac)); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_GotCac", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // BEGIN ConsoleAllocateConsole(PCONSOLE_API_CONNECTINFO p) + // CALL auto Status = SetUpConsole(&p->ConsoleInfo, p->TitleLength, p->Title, p->CurDir, p->AppName); + Settings* pStartupSettings = &Cac.ConsoleInfo; + DWORD TitleLength = Cac.TitleLength; + LPWSTR Title = Cac.Title; + LPCWSTR CurDir = Cac.CurDir; + LPCWSTR AppName = Cac.AppName; + // BEGIN SetUpConsole(_Inout_ Settings* pStartupSettings, _In_ DWORD TitleLength, _In_reads_bytes_(TitleLength) LPWSTR Title, _In_ LPCWSTR CurDir, _In_ LPCWSTR AppName) + // auto& settings = ServiceLocator::LocateGlobals().getConsoleInformation(); + Settings settings{}; + // We need to see if we were spawned from a link. If we were, we need to + // call back into the shell to try to get all the console information from the link. + ServiceLocator::LocateSystemConfigurationProvider()->GetSettingsFromLink(&settings, Title, &TitleLength, CurDir, AppName, &icon); + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_LoadedFromLink", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + // 1. The settings we were passed contains STARTUPINFO structure settings to be applied last. + settings.ApplyStartupInfo(pStartupSettings); + // } + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_AppliedStartupInfo", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + + TERMINAL_STARTUP_INFO myStartupInfo{ Title, icon.path.c_str(), icon.index }; + + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_CreatedStartupInfo", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingWideString(myStartupInfo.pszTitle, "Title", "The title for the connection"), + TraceLoggingWideString(myStartupInfo.pszIconPath, "Icon", "The icon for the connection"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(), outPipeTheirSide.get(), signalPipeTheirSide.get(), refHandle.get(), serverProcess, - clientProcess.get())); + clientProcess.get(), + myStartupInfo)); TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_DelegateToTerminalSucceeded", @@ -734,6 +801,12 @@ PWSTR TranslateConsoleTitle(_In_ PCWSTR pwszConsoleTitle, const BOOL fUnexpand, auto Status = NTSTATUS_FROM_HRESULT(Message->ReadMessageInput(0, &Data, sizeof(Data))); if (!NT_SUCCESS(Status)) { + TraceLoggingWrite(g_hConhostV2EventTraceProvider, + "SrvInit_ConsoleInitializeConnectInfo_ReadMessageInputFailed", + TraceLoggingDescription("TODO! debugging only"), + TraceLoggingUInt64(Status, "Status", "something happened"), + TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), + TraceLoggingKeyword(TIL_KEYWORD_TRACE)); return Status; } diff --git a/src/interactivity/inc/ISystemConfigurationProvider.hpp b/src/interactivity/inc/ISystemConfigurationProvider.hpp index 90431fa51b0..3237d6e51c8 100644 --- a/src/interactivity/inc/ISystemConfigurationProvider.hpp +++ b/src/interactivity/inc/ISystemConfigurationProvider.hpp @@ -19,6 +19,12 @@ class Settings; namespace Microsoft::Console::Interactivity { + struct IconInfo + { + std::wstring path; + int index = 0; + }; + class ISystemConfigurationProvider { public: @@ -36,6 +42,7 @@ namespace Microsoft::Console::Interactivity _Inout_updates_bytes_(*pdwTitleLength) LPWSTR pwszTitle, _Inout_ PDWORD pdwTitleLength, _In_ PCWSTR pwszCurrDir, - _In_ PCWSTR pwszAppName) = 0; + _In_ PCWSTR pwszAppName, + _Inout_opt_ IconInfo* iconInfo) = 0; }; } diff --git a/src/interactivity/onecore/SystemConfigurationProvider.cpp b/src/interactivity/onecore/SystemConfigurationProvider.cpp index 72f06a5b935..21de54447af 100644 --- a/src/interactivity/onecore/SystemConfigurationProvider.cpp +++ b/src/interactivity/onecore/SystemConfigurationProvider.cpp @@ -49,7 +49,8 @@ void SystemConfigurationProvider::GetSettingsFromLink( _Inout_updates_bytes_(*pdwTitleLength) LPWSTR /*pwszTitle*/, _Inout_ PDWORD /*pdwTitleLength*/, _In_ PCWSTR /*pwszCurrDir*/, - _In_ PCWSTR /*pwszAppName*/) + _In_ PCWSTR /*pwszAppName*/, + _Inout_opt_ IconInfo* /*iconInfo*/) { // While both OneCore console renderers use TrueType fonts, there is no // advanced font support on that platform. Namely, there is no way to pick diff --git a/src/interactivity/win32/SystemConfigurationProvider.cpp b/src/interactivity/win32/SystemConfigurationProvider.cpp index 3aeb9a85432..ab824b0f943 100644 --- a/src/interactivity/win32/SystemConfigurationProvider.cpp +++ b/src/interactivity/win32/SystemConfigurationProvider.cpp @@ -60,7 +60,8 @@ void SystemConfigurationProvider::GetSettingsFromLink( _Inout_updates_bytes_(*pdwTitleLength) LPWSTR pwszTitle, _Inout_ PDWORD pdwTitleLength, _In_ PCWSTR pwszCurrDir, - _In_ PCWSTR pwszAppName) + _In_ PCWSTR pwszAppName, + _Inout_opt_ IconInfo* iconInfo) { auto& gci = ServiceLocator::LocateGlobals().getConsoleInformation(); WCHAR wszLinkTarget[MAX_PATH] = { 0 }; @@ -191,7 +192,15 @@ void SystemConfigurationProvider::GetSettingsFromLink( if (wszIconLocation[0] != L'\0') { - LOG_IF_FAILED(Icon::Instance().LoadIconsFromPath(wszIconLocation, iIconIndex)); + if (iconInfo) + { + iconInfo->path = std::wstring{ wszIconLocation }; + iconInfo->index = iIconIndex; + } + else + { + LOG_IF_FAILED(Icon::Instance().LoadIconsFromPath(wszIconLocation, iIconIndex)); + } } if (!IsValidCodePage(pLinkSettings->GetCodePage())) diff --git a/src/interactivity/win32/SystemConfigurationProvider.hpp b/src/interactivity/win32/SystemConfigurationProvider.hpp index 8c5d9bf65e7..9f743cb00a2 100644 --- a/src/interactivity/win32/SystemConfigurationProvider.hpp +++ b/src/interactivity/win32/SystemConfigurationProvider.hpp @@ -37,7 +37,8 @@ namespace Microsoft::Console::Interactivity::Win32 _Inout_updates_bytes_(*pdwTitleLength) LPWSTR pwszTitle, _Inout_ PDWORD pdwTitleLength, _In_ PCWSTR pwszCurrDir, - _In_ PCWSTR pwszAppName); + _In_ PCWSTR pwszAppName, + _Inout_opt_ IconInfo* iconInfo); private: static const ULONG s_DefaultCursorWidth = 1;