From 7e47f6aab96b0f2d01309bc05cebe1c57b44a127 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Fri, 26 Aug 2022 06:16:29 -0500 Subject: [PATCH] Wire up passing LNK/EXE data from OpenCon to ITerminalHandoff (#13570) This PR by itself doesn't _really_ change much. Technically, now the Terminal will respect the Title of a `.lnk` when started for defterm, but we don't do anything else yet. Primarily, the goal of this PR is to just wire up startup info in OpenConsole to the connected Terminal. * This required a bit of changes in `srvinit.cpp:ConsoleEstablishHandoff` to replicate other bits of startup, where we crack open the connect message to get the relevant bits of info. * We pack that all into a `TERMINAL_STARTUP_INFO`, which we pass along to the registered terminal application. * `ConptyConnection` accepts the handoff, and gathers that information out of the `TERMINAL_STARTUP_INFO` * Some other updates to the scratch sln were made to make it build again (related, but unimportant). * This is a precursor to: * #13111 * #12154 * Closes #9458 * Tested manually * I work here --- .../SampleApp/SampleAppLib.vcxproj | 17 ++-- .../SampleApp/dll/SampleApp.vcxproj | 18 +++- .../WindowExe/SampleIslandWindow.cpp | 2 - .../WindowExe/WindowExe.vcxproj | 35 +++++--- .../CascadiaPackage/Package-Dev.appxmanifest | 3 +- .../CascadiaPackage/Package-Pre.appxmanifest | 3 +- .../CascadiaPackage/Package.appxmanifest | 3 +- src/cascadia/TerminalApp/TerminalPage.cpp | 1 + .../TerminalConnection/CTerminalHandoff.cpp | 4 +- .../TerminalConnection/CTerminalHandoff.h | 7 +- .../TerminalConnection/ConptyConnection.cpp | 18 +++- .../TerminalConnection/ConptyConnection.h | 16 +++- .../TerminalConnection/ConptyConnection.idl | 1 + src/host/proxy/ITerminalHandoff.idl | 44 ++++++++++ src/host/srvinit.cpp | 82 ++++++++++++++++++- .../inc/ISystemConfigurationProvider.hpp | 9 +- .../onecore/SystemConfigurationProvider.cpp | 3 +- .../onecore/SystemConfigurationProvider.hpp | 3 +- .../win32/SystemConfigurationProvider.cpp | 27 +++++- .../win32/SystemConfigurationProvider.hpp | 3 +- 20 files changed, 254 insertions(+), 45 deletions(-) diff --git a/scratch/ScratchIslandApp/SampleApp/SampleAppLib.vcxproj b/scratch/ScratchIslandApp/SampleApp/SampleAppLib.vcxproj index 2ed7347b1a..8a7e19e562 100644 --- a/scratch/ScratchIslandApp/SampleApp/SampleAppLib.vcxproj +++ b/scratch/ScratchIslandApp/SampleApp/SampleAppLib.vcxproj @@ -17,9 +17,15 @@ false nested + + + true + true + + - + @@ -147,14 +153,15 @@ - - + + + + This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - - + true + + true + true + + - @@ -79,15 +83,20 @@ + + + + + $(OpenConsoleCommonOutDir)\ConTypes.lib;WindowsApp.lib;shell32.lib;user32.lib;%(AdditionalDependencies) + + - This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - @@ -102,4 +111,7 @@ + + + diff --git a/scratch/ScratchIslandApp/WindowExe/SampleIslandWindow.cpp b/scratch/ScratchIslandApp/WindowExe/SampleIslandWindow.cpp index 1c5cf10d05..112b47fc17 100644 --- a/scratch/ScratchIslandApp/WindowExe/SampleIslandWindow.cpp +++ b/scratch/ScratchIslandApp/WindowExe/SampleIslandWindow.cpp @@ -104,8 +104,6 @@ void SampleIslandWindow::_HandleCreateWindow(const WPARAM, const LPARAM lParam) void SampleIslandWindow::Initialize() { - const bool initialized = (_interopWindowHandle != nullptr); - _source = DesktopWindowXamlSource{}; auto interop = _source.as(); diff --git a/scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj b/scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj index b1dec01d9b..6217d92e98 100644 --- a/scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj +++ b/scratch/ScratchIslandApp/WindowExe/WindowExe.vcxproj @@ -1,6 +1,5 @@ - {b4427499-9fde-4208-b456-5bc580637633} @@ -16,7 +15,15 @@ Windows + + true + true + true + true + + + @@ -138,16 +145,11 @@ - - This project references NuGet package(s) that are missing on this computer. Use NuGet Package Restore to download them. For more information, see http://go.microsoft.com/fwlink/?LinkID=322105. The missing file is {0}. - - - + + <_OpenConsoleExe Include="$(OpenConsoleCommonOutDir)\OpenConsole.exe" /> + + + $(ProjectName) + BuiltProjectOutputGroup + %(Filename)%(Extension) + + - - - - - + + + diff --git a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest index bc5ec67209..102539275c 100644 --- a/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package-Dev.appxmanifest @@ -99,7 +99,8 @@ - + + diff --git a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest index eb968f870c..934a105044 100644 --- a/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package-Pre.appxmanifest @@ -188,7 +188,8 @@ - + + diff --git a/src/cascadia/CascadiaPackage/Package.appxmanifest b/src/cascadia/CascadiaPackage/Package.appxmanifest index fb494d98af..82b1ce1b5d 100644 --- a/src/cascadia/CascadiaPackage/Package.appxmanifest +++ b/src/cascadia/CascadiaPackage/Package.appxmanifest @@ -188,7 +188,8 @@ - + + diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index cf6ff9c859..36937f4481 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -3270,6 +3270,7 @@ namespace winrt::TerminalApp::implementation { NewTerminalArgs newTerminalArgs; newTerminalArgs.Commandline(connection.Commandline()); + newTerminalArgs.TabTitle(connection.StartingTitle()); // 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 1f8d089a62..cf33c4002a 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 4e43c7b744..cdeb87f6af 100644 --- a/src/cascadia/TerminalConnection/CTerminalHandoff.h +++ b/src/cascadia/TerminalConnection/CTerminalHandoff.h @@ -26,10 +26,10 @@ 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> + CTerminalHandoff : public Microsoft::WRL::RuntimeClass, ITerminalHandoff2> { #pragma region ITerminalHandoff STDMETHODIMP EstablishPtyHandoff(HANDLE in, @@ -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 6fee6a797e..2f0b55ca9c 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,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation THROW_IF_FAILED(ConptyPackPseudoConsole(hServerProcess, hRef, hSig, &_hPC)); _piClient.hProcess = hClientProcess; + _startupInfo.title = winrt::hstring{ startupInfo.pszTitle, SysStringLen(startupInfo.pszTitle) }; + SysFreeString(startupInfo.pszTitle); + _startupInfo.iconPath = winrt::hstring{ startupInfo.pszIconPath, SysStringLen(startupInfo.pszIconPath) }; + SysFreeString(startupInfo.pszIconPath); + _startupInfo.iconIndex = startupInfo.iconIndex; + try { _commandline = _commandlineFromProcess(hClientProcess); @@ -288,6 +295,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return _commandline; } + winrt::hstring ConptyConnection::StartingTitle() const + { + return _startupInfo.title; + } + void ConptyConnection::Start() try { @@ -667,10 +679,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 d54ad7e03a..beaaac058f 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 StartingTitle() 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 82f51b70db..d284e2ce4a 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 StartingTitle { get; }; void ClearBuffer(); diff --git a/src/host/proxy/ITerminalHandoff.idl b/src/host/proxy/ITerminalHandoff.idl index d06799658a..37906770f0 100644 --- a/src/host/proxy/ITerminalHandoff.idl +++ b/src/host/proxy/ITerminalHandoff.idl @@ -4,11 +4,41 @@ import "oaidl.idl"; import "ocidl.idl"; + +typedef struct _TERMINAL_STARTUP_INFO +{ + // In STARTUPINFO + BSTR pszTitle; + + // Also wanted + BSTR pszIconPath; + LONG iconIndex; + + // The rest of STARTUPINFO + DWORD dwX; + DWORD dwY; + DWORD dwXSize; + DWORD dwYSize; + DWORD dwXCountChars; + DWORD dwYCountChars; + DWORD dwFillAttribute; + DWORD dwFlags; + WORD wShowWindow; +} TERMINAL_STARTUP_INFO; + +// LOAD BEARING! +// +// There is only ever one OpenConsoleProxy.dll loaded by COM for _ALL_ terminal +// instances, across Dev, Preview, Stable, whatever. So we have to keep all old +// versions of interfaces in the file here, even if the old version is no longer +// in use. + [ object, uuid(59D55CCE-FC8A-48B4-ACE8-0A9286C6557F) ] interface ITerminalHandoff : IUnknown { + // DEPRECATED! HRESULT EstablishPtyHandoff([in, system_handle(sh_pipe)] HANDLE in, [in, system_handle(sh_pipe)] HANDLE out, [in, system_handle(sh_pipe)] HANDLE signal, @@ -16,3 +46,17 @@ import "ocidl.idl"; [in, system_handle(sh_process)] HANDLE server, [in, system_handle(sh_process)] HANDLE client); }; + +[ + object, + uuid(AA6B364F-4A50-4176-9002-0AE755E7B5EF) +] interface ITerminalHandoff2 : IUnknown +{ + HRESULT EstablishPtyHandoff([in, system_handle(sh_pipe)] HANDLE in, + [in, system_handle(sh_pipe)] HANDLE out, + [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] TERMINAL_STARTUP_INFO startupInfo); +}; diff --git a/src/host/srvinit.cpp b/src/host/srvinit.cpp index 285809d700..6b12075cc6 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. @@ -198,6 +199,22 @@ static bool s_IsOnDesktop() // strong nudge in that direction. If an application _doesn't_ want VT // processing, it's free to disable this setting, even in conpty mode. settings.SetVirtTermLevel(1); + + // GH#9458 - In the case of a DefTerm handoff, the OriginalTitle might + // be stashed in the lnk. We want to crack that lnk open, so we can get + // that title from it, but we want to discard everything else. So build + // a dummy Settings object here, and read the link settings into it. + // `Title` will get filled with the title from the lnk, which we'll use + // below. + + Settings temp; + // We're not gonna copy over StartupFlags to the main gci settings, + // because we generally don't think those are valuable in ConPTY mode. + // However, we do need to apply them to the temp we've created, so that + // GetSettingsFromLink will actually look for the link settings (it will + // skip that if STARTF_TITLEISLINKNAME is not set). + temp.SetStartupFlags(pStartupSettings->GetStartupFlags()); + ServiceLocator::LocateSystemConfigurationProvider()->GetSettingsFromLink(&temp, Title, &TitleLength, CurDir, AppName, nullptr); } // 1. The settings we were passed contains STARTUPINFO structure settings to be applied last. @@ -494,7 +511,7 @@ try const auto serverProcess = GetCurrentProcess(); - ::Microsoft::WRL::ComPtr handoff; + ::Microsoft::WRL::ComPtr handoff; TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_PrepareToCreateDelegationTerminal", @@ -509,12 +526,71 @@ try TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE), TraceLoggingKeyword(TIL_KEYWORD_TRACE)); + // As a part of defterm handoff, we're gonna try to pull a lot of + // information out of the link and startup info, so we can let the terminal + // know these things as well. + // + // To let the terminal know these things, we have to look them up now, + // before we normally would. + // + // Typically, we'll just go into `ConsoleCreateIoThread` below, which will + // pull out the CONSOLE_API_CONNECTINFO from this connect message, and then + // get the link properties out of the title later. Below are elements of + // ConsoleAllocateConsole and SetUpConsole that get the bits of STARTUP_INFO + // we care about for defterm handoffs. + + // A placeholder that we'll read icon information into, instead of setting + // the globals icon state. + Microsoft::Console::Interactivity::IconInfo icon; + + // To be able to actually process this connect message into a + // CONSOLE_API_CONNECTINFO, we need to hook up the ConDrvDeviceComm to the + // message. Usually, we'd create the ConDrvDeviceComm later, in + // ConsoleServerInitialization, but we can set it up early here. + // ConsoleServerInitialization will safely no-op if it already finds one. + g.pDeviceComm = new ConDrvDeviceComm(Server); + // load bearing: if you don't set this, the ConsoleInitializeConnectInfo will fail. + connectMessage->_pDeviceComm = g.pDeviceComm; + CONSOLE_API_CONNECTINFO Cac; + RETURN_IF_NTSTATUS_FAILED(ConsoleInitializeConnectInfo(connectMessage, &Cac)); + + // BEGIN code from SetUpConsole + // Create a temporary Settings object to parse the settings into, rather + // than parsing them into the global settings object (gci). + Settings settings{}; + // We need to see if we were spawned from a link. If we were, we need to + // call back into the OS shell to try to get all the console information from the link. + // + // load bearing: if you don't pass the StartupFlags, then + // GetSettingsFromLink might not even bother attempting to check the lnk. + settings.SetStartupFlags(Cac.ConsoleInfo.GetStartupFlags()); + ServiceLocator::LocateSystemConfigurationProvider()->GetSettingsFromLink(&settings, + Cac.Title, + &Cac.TitleLength, + Cac.CurDir, + Cac.AppName, + &icon); + + // 1. The settings we were passed contains STARTUPINFO structure settings to be applied last. + settings.ApplyStartupInfo(&Cac.ConsoleInfo); + // END code from SetUpConsole + + // Take what we've collected, and bundle it up for handoff. + auto title = wil::make_bstr(Cac.Title); + auto iconPath = wil::make_bstr(icon.path.data()); + TERMINAL_STARTUP_INFO myStartupInfo{ + title.get(), + iconPath.get(), + icon.index + }; + RETURN_IF_FAILED(handoff->EstablishPtyHandoff(inPipeTheirSide.get(), outPipeTheirSide.get(), signalPipeTheirSide.get(), refHandle.get(), serverProcess, - clientProcess.get())); + clientProcess.get(), + myStartupInfo)); TraceLoggingWrite(g_hConhostV2EventTraceProvider, "SrvInit_DelegateToTerminalSucceeded", diff --git a/src/interactivity/inc/ISystemConfigurationProvider.hpp b/src/interactivity/inc/ISystemConfigurationProvider.hpp index 90431fa51b..3237d6e51c 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 72f06a5b93..21de54447a 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/onecore/SystemConfigurationProvider.hpp b/src/interactivity/onecore/SystemConfigurationProvider.hpp index 658c8e6954..054e8053af 100644 --- a/src/interactivity/onecore/SystemConfigurationProvider.hpp +++ b/src/interactivity/onecore/SystemConfigurationProvider.hpp @@ -37,7 +37,8 @@ namespace Microsoft::Console::Interactivity::OneCore _Inout_updates_bytes_(*pdwTitleLength) LPWSTR pwszTitle, _Inout_ PDWORD pdwTitleLength, _In_ PCWSTR pwszCurrDir, - _In_ PCWSTR pwszAppName) override; + _In_ PCWSTR pwszAppName, + _Inout_opt_ IconInfo* iconInfo) override; private: static constexpr UINT s_DefaultCaretBlinkTime = 530; // milliseconds diff --git a/src/interactivity/win32/SystemConfigurationProvider.cpp b/src/interactivity/win32/SystemConfigurationProvider.cpp index 3aeb9a8543..c4c74865c9 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 }; @@ -72,8 +73,15 @@ void SystemConfigurationProvider::GetSettingsFromLink( // Did we get started from a link? if (pLinkSettings->GetStartupFlags() & STARTF_TITLEISLINKNAME) { - if (SUCCEEDED(CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED))) + auto initializedCom = CoInitializeEx(nullptr, COINIT_APARTMENTTHREADED); + + // GH#9458: If it's RPC_E_CHANGED_MODE, that's okay, we're doing a + // defterm and have already started COM. We can continue on here. + if (SUCCEEDED(initializedCom) || initializedCom == RPC_E_CHANGED_MODE) { + // Don't CoUninitialize if we still need COM for defterm. + auto unInitCom = wil::scope_exit([initializedCom]() { if (SUCCEEDED(initializedCom)){CoUninitialize();} }); + const auto cch = *pdwTitleLength / sizeof(wchar_t); gci.SetLinkTitle(std::wstring(pwszTitle, cch)); @@ -156,7 +164,6 @@ void SystemConfigurationProvider::GetSettingsFromLink( // settings based on title. pLinkSettings->UnsetStartupFlag(STARTF_TITLEISLINKNAME); } - CoUninitialize(); } } @@ -191,7 +198,19 @@ void SystemConfigurationProvider::GetSettingsFromLink( if (wszIconLocation[0] != L'\0') { - LOG_IF_FAILED(Icon::Instance().LoadIconsFromPath(wszIconLocation, iIconIndex)); + // GH#9458, GH#13111 - when this is executed during defterm startup, + // we'll pass in an iconInfo pointer, which we should fill with the + // selected icon path and index, rather than loading the icon with our + // global Icon class. + 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 8c5d9bf65e..9f743cb00a 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;