Use feature markers during terminal-by-default handoff (#13160)

If we want to make Windows Terminal the default terminal under Windows,
we'll have to make conhost "handoff" incoming connections by default.
But this poses a problem: How can the seldomly updated conhost know
whether the routinely updated Windows Terminal version is actually willing
to accept such handoffs by default (it might be unwilling due to bugs, etc.)?

This commit solves the issue by introducing:
* A marker interface (`IDefaultTerminalMarker`): If it exists,
  Windows Terminal indicates its willingness to accept the handoff.
* Turning the all-0 GUID from being synonymous for conhost,
  to being synonymous for "Let Windows decide". Without this we wouldn't
  be able to differentiate between users who consciously chose conhost
  as their default terminal, vs. users who want the standard behavior.

Testing fallback behavior:
* Install "Terminal" 1.13
* Delete the 2 keys below `HKCU\Console\%%Startup`
* Enable `Feature_AttemptHandoff` in `features.xml`
  Return `true` from `DefaultApp::CheckShouldTerminalBeDefault`
* Replace `conhost.exe` and `console.dll` with `sfpcopy` after building
* Launching `cmd.exe` launches as a conhost window ✅
  (because "Terminal" 1.13 lacks the marker interface)
* Open properties page in `conhost.exe`
  "Let Windows decide" is select by default ✅
* Changing the selection writes the new value ✅

Testing the new behavior:
* Delete the 2 keys below `HKCU\Console\%%Startup`
* Enable `Feature_AttemptHandoff` in `features.xml`
  Return `true` from `DefaultApp::CheckShouldTerminalBeDefault`
* Use `CLSID_WindowsTerminalConsoleDev` and `CLSID_WindowsTerminalTerminalDev`
  for the initialization of `TerminalDelegationPair`
* Replace `conhost.exe` and `console.dll` with `sfpcopy` after building
* Deploy the "Terminal Dev" package
* Launching `cmd.exe` launches "Terminal Dev" ✅
  (because "Terminal Dev" has the marker interface)
* Open the settings tab
  "Let Windows decide" is select by default ✅
* Changing the selection and saving writes the new value ✅

(cherry picked from commit 1b81c65)
Service-Card-Id: 82925080
Service-Version: Inbox

.github/actions/spelling/expect/expect.txt

@@ -1068,6 +1068,7 @@ ICore
 IData
 IDCANCEL
 IDD
+IDefault
 IDesktop
 IDevice
 IDictionary

src/host/exe/CConsoleHandoff.h

@@ -29,7 +29,7 @@ Author(s):
 using namespace Microsoft::WRL;
 
 struct __declspec(uuid(__CLSID_CConsoleHandoff))
-    CConsoleHandoff : public RuntimeClass<RuntimeClassFlags<ClassicCom>, IConsoleHandoff>
+    CConsoleHandoff : public RuntimeClass<RuntimeClassFlags<ClassicCom>, IConsoleHandoff, IDefaultTerminalMarker>
 {
 #pragma region IConsoleHandoff
     STDMETHODIMP EstablishHandoff(HANDLE server,

src/host/exe/exemain.cpp

@@ -265,8 +265,7 @@ int CALLBACK wWinMain(
     {
         // Only try to register as a handoff target if we are NOT a part of Windows.
 #if TIL_FEATURE_RECEIVEINCOMINGHANDOFF_ENABLED
-        bool defAppEnabled = false;
-        if (args.ShouldRunAsComServer() && SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(defAppEnabled)) && defAppEnabled)
+        if (args.ShouldRunAsComServer() && Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy())
         {
             try
             {

src/host/globals.h

@@ -22,8 +22,8 @@ Revision History:
 #include "ConsoleArguments.hpp"
 #include "ApiRoutines.h"
 
+#include "../propslib/DelegationConfig.hpp"
 #include "../renderer/base/Renderer.hpp"
-
 #include "../server/DeviceComm.h"
 #include "../server/ConDrvDeviceComm.h"
 
@@ -69,10 +69,10 @@ class Globals
 
     bool handoffTarget = false;
 
-    std::optional<CLSID> handoffConsoleClsid;
-    std::optional<CLSID> handoffTerminalClsid;
+    DelegationConfig::DelegationPair delegationPair;
     wil::unique_hfile handoffInboxConsoleHandle;
     wil::unique_threadpool_wait handoffInboxConsoleExitWait;
+    bool defaultTerminalMarkerCheckRequired = false;
 
 #ifdef UNIT_TESTING
     void EnableConptyModeForTests(std::unique_ptr<Microsoft::Console::Render::VtEngine> vtRenderEngine);

src/host/proxy/IConsoleHandoff.idl

@@ -31,3 +31,9 @@ typedef const CONSOLE_PORTABLE_ATTACH_MSG* PCCONSOLE_PORTABLE_ATTACH_MSG;
                              [out, system_handle(sh_process)] HANDLE* process);
 };
 
+[
+    object,
+    uuid(746E6BC0-AB05-4E38-AB14-71E86763141F)
+] interface IDefaultTerminalMarker : IUnknown
+{
+};

src/host/srvinit.cpp

@@ -26,7 +26,6 @@
 #include "../renderer/base/renderer.hpp"
 
 #include "../inc/conint.h"
-#include "../propslib/DelegationConfig.hpp"
 
 #include "tracing.hpp"
 
@@ -64,28 +63,28 @@ try
 
     // Check if this conhost is allowed to delegate its activities to another.
     // If so, look up the registered default console handler.
-    bool isEnabled = false;
-    if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(isEnabled)) && isEnabled)
+    if (Globals.delegationPair.IsUndecided() && Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy())
     {
-        IID delegationClsid;
-        if (SUCCEEDED(DelegationConfig::s_GetDefaultConsoleId(delegationClsid)))
-        {
-            Globals.handoffConsoleClsid = delegationClsid;
-            TraceLoggingWrite(g_hConhostV2EventTraceProvider,
-                              "SrvInit_FoundDelegationConsole",
-                              TraceLoggingGuid(Globals.handoffConsoleClsid.value(), "ConsoleClsid"),
-                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                              TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-        }
-        if (SUCCEEDED(DelegationConfig::s_GetDefaultTerminalId(delegationClsid)))
-        {
-            Globals.handoffTerminalClsid = delegationClsid;
-            TraceLoggingWrite(g_hConhostV2EventTraceProvider,
-                              "SrvInit_FoundDelegationTerminal",
-                              TraceLoggingGuid(Globals.handoffTerminalClsid.value(), "TerminalClsid"),
-                              TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                              TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-        }
+        Globals.delegationPair = DelegationConfig::s_GetDelegationPair();
+
+        TraceLoggingWrite(g_hConhostV2EventTraceProvider,
+                          "SrvInit_FoundDelegationConsole",
+                          TraceLoggingGuid(Globals.delegationPair.console, "ConsoleClsid"),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+        TraceLoggingWrite(g_hConhostV2EventTraceProvider,
+                          "SrvInit_FoundDelegationTerminal",
+                          TraceLoggingGuid(Globals.delegationPair.terminal, "TerminalClsid"),
+                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
+                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));
+    }
+    // If we looked up the registered defterm pair, and it was left as the default (missing or {0}),
+    // AND velocity is enabled for DxD, then we switch the delegation pair to Terminal and
+    // mark that we should check that class for the marker interface later.
+    if (Globals.delegationPair.IsDefault() && Microsoft::Console::Internal::DefaultApp::CheckShouldTerminalBeDefault())
+    {
+        Globals.delegationPair = DelegationConfig::TerminalDelegationPair;
+        Globals.defaultTerminalMarkerCheckRequired = true;
     }
 
     // Create the accessibility notifier early in the startup process.
@@ -427,28 +426,18 @@ try
     auto& g = ServiceLocator::LocateGlobals();
     g.handoffTarget = true;
 
-    IID delegationClsid;
-    if (SUCCEEDED(DelegationConfig::s_GetDefaultConsoleId(delegationClsid)))
-    {
-        g.handoffConsoleClsid = delegationClsid;
-    }
-    if (SUCCEEDED(DelegationConfig::s_GetDefaultTerminalId(delegationClsid)))
-    {
-        g.handoffTerminalClsid = delegationClsid;
-    }
-
-    if (!g.handoffTerminalClsid)
+    g.delegationPair = DelegationConfig::s_GetDelegationPair();
+    // We've been handed off to (we're OpenConsole, not conhost).
+    // If we get here and there's not a custom defterm set, then it must be because
+    // conhost defaulted to us for DxD. Set up Terminal as the thing to handoff too.
+    if (!g.delegationPair.IsCustom())
     {
-        TraceLoggingWrite(g_hConhostV2EventTraceProvider,
-                          "SrvInit_ReceiveHandoff_NoTerminal",
-                          TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
-                          TraceLoggingKeyword(TIL_KEYWORD_TRACE));
-        return E_NOT_SET;
+        g.delegationPair = DelegationConfig::TerminalDelegationPair;
     }
 
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "SrvInit_ReceiveHandoff",
-                      TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"),
+                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
                       TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                       TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 
@@ -509,14 +498,14 @@ try
 
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "SrvInit_PrepareToCreateDelegationTerminal",
-                      TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"),
+                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
                       TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                       TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 
-    RETURN_IF_FAILED(CoCreateInstance(g.handoffTerminalClsid.value(), nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff)));
+    RETURN_IF_FAILED(CoCreateInstance(g.delegationPair.terminal, nullptr, CLSCTX_LOCAL_SERVER, IID_PPV_ARGS(&handoff)));
     TraceLoggingWrite(g_hConhostV2EventTraceProvider,
                       "SrvInit_CreatedDelegationTerminal",
-                      TraceLoggingGuid(g.handoffTerminalClsid.value(), "TerminalClsid"),
+                      TraceLoggingGuid(g.delegationPair.terminal, "TerminalClsid"),
                       TraceLoggingLevel(WINEVENT_LEVEL_VERBOSE),
                       TraceLoggingKeyword(TIL_KEYWORD_TRACE));
 

src/inc/conint.h

@@ -47,7 +47,7 @@ namespace Microsoft::Console::Internal
 
     namespace DefaultApp
     {
-        [[nodiscard]] HRESULT CheckDefaultAppPolicy(bool& isEnabled) noexcept;
-        [[nodiscard]] HRESULT CheckShouldTerminalBeDefault(bool& isEnabled) noexcept;
+        [[nodiscard]] bool CheckDefaultAppPolicy() noexcept;
+        [[nodiscard]] bool CheckShouldTerminalBeDefault() noexcept;
     }
 }

src/internal/stubs.cpp

@@ -30,19 +30,17 @@ void EdpPolicy::AuditClipboard(const std::wstring_view /*destinationName*/) noex
     return S_FALSE;
 }
 
-[[nodiscard]] HRESULT DefaultApp::CheckDefaultAppPolicy(bool& isEnabled) noexcept
+[[nodiscard]] bool DefaultApp::CheckDefaultAppPolicy() noexcept
 {
     // True so propsheet will show configuration options but be sure that
     // the open one won't attempt handoff from double click of OpenConsole.exe
-    isEnabled = true;
-    return S_OK;
+    return true;
 }
 
-[[nodiscard]] HRESULT DefaultApp::CheckShouldTerminalBeDefault(bool& isEnabled) noexcept
+[[nodiscard]] bool DefaultApp::CheckShouldTerminalBeDefault() noexcept
 {
     // False since setting Terminal as the default app is an OS feature and probably
     // should not be done in the open source conhost. We can always decide to turn it
     // on in the future though.
-    isEnabled = false;
-    return S_OK;
+    return false;
 }

src/propsheet/TerminalPropsheetPage.cpp

@@ -97,26 +97,27 @@ void _PrepDefAppCombo(const HWND hDlg,
     const HWND hCombo = GetDlgItem(hDlg, dlgItem);
     ComboBox_ResetContent(hCombo);
 
+    DWORD i = 0;
     DWORD selectedIndex = 0;
-    for (DWORD i = 0; i < gsl::narrow<DWORD>(list.size()); ++i)
+    for (const auto& item : list)
     {
-        auto& item = list[i];
-
-        // An empty CLSID is a sentinel for the inbox console.
-        if (item.terminal.clsid == CLSID{ 0 })
-        {
-            const auto name = GetStringResource(IDS_TERMINAL_DEF_INBOX);
-            ComboBox_AddString(hCombo, name.c_str());
-        }
-        else
+        switch (item.pair.kind)
         {
-            ComboBox_AddString(hCombo, item.terminal.name.c_str());
+        case DelegationConfig::DelegationPairKind::Default:
+            ComboBox_AddString(hCombo, GetStringResource(IDS_TERMINAL_HANDOFF_DEFAULT).c_str());
+            break;
+        case DelegationConfig::DelegationPairKind::Conhost:
+            ComboBox_AddString(hCombo, GetStringResource(IDS_TERMINAL_HANDOFF_CONHOST).c_str());
+            break;
+        default:
+            ComboBox_AddString(hCombo, item.info.name.c_str());
         }
         ComboBox_SetItemData(hCombo, i, &item);
         if (selected == item)
         {
             selectedIndex = i;
         }
+        ++i;
     }
 
     ComboBox_SetCurSel(hCombo, selectedIndex);

src/propsheet/console.cpp

@@ -96,7 +96,7 @@ void SaveConsoleSettingsIfNeeded(const HWND hwnd)
             gpStateInfo->FaceName[0] = TEXT('\0');
         }
 
-        if (g_defAppEnabled)
+        if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy())
         {
             LOG_IF_FAILED(DelegationConfig::s_SetDefaultByPackage(g_selectedPackage));
         }
@@ -552,7 +552,7 @@ BOOL PopulatePropSheetPageArray(_Out_writes_(cPsps) PROPSHEETPAGE* pPsp, const s
         {
             pTerminalPage->dwSize = sizeof(PROPSHEETPAGE);
             pTerminalPage->hInstance = ghInstance;
-            if (g_defAppEnabled)
+            if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy())
             {
                 pTerminalPage->pszTemplate = MAKEINTRESOURCE(DID_TERMINAL_WITH_DEFTERM);
             }
@@ -629,7 +629,7 @@ INT_PTR ConsolePropertySheet(__in HWND hWnd, __in PCONSOLE_STATE_INFO pStateInfo
     // Find the available default console/terminal packages
     //
 
-    if (SUCCEEDED(Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy(g_defAppEnabled)) && g_defAppEnabled)
+    if (Microsoft::Console::Internal::DefaultApp::CheckDefaultAppPolicy())
     {
         LOG_IF_FAILED(DelegationConfig::s_GetAvailablePackages(g_availablePackages, g_selectedPackage));
     }

src/propsheet/console.h

@@ -58,7 +58,8 @@ Revision History:
 // unused 16
 #define IDS_TOOLTIP_OPACITY          17
 #define IDS_TOOLTIP_INTERCEPT_COPY_PASTE    18
-#define IDS_TERMINAL_DEF_INBOX       19
+#define IDS_TERMINAL_HANDOFF_DEFAULT 19
+#define IDS_TERMINAL_HANDOFF_CONHOST 20
 // clang-format on
 
 void MakeAltRasterFont(

src/propsheet/console.rc

@@ -721,7 +721,7 @@ BEGIN
     AUTOCHECKBOX    "Disable Scroll-Forward", IDD_DISABLE_SCROLLFORWARD, T_SCROLL_X+P_1, T_SCROLL_Y+P_2, T_SCROLL_W-P_4-P_4, 10
 
     GROUPBOX        "Default Terminal Application", -1, T_DEFAPP_X, T_DEFAPP_Y, T_DEFAPP_W, T_DEFAPP_H
-    COMBOBOX        IDD_TERMINAL_COMBO_DEFTERM, T_DEFTERM_X+P_0, T_DEFTERM_Y, T_DEFTERM_W, 10, CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP 
+    COMBOBOX        IDD_TERMINAL_COMBO_DEFTERM, T_DEFTERM_X+P_0, T_DEFTERM_Y, T_DEFTERM_W, 10, CBS_DROPDOWNLIST | WS_VSCROLL | WS_TABSTOP
 
     CONTROL         "Find out more about <A HREF=""https://go.microsoft.com/fwlink/?linkid=2028595"">experimental terminal settings</A>",
                     IDD_HELP_TERMINAL, "SysLink", WS_TABSTOP, 10, 225, 200, 10
@@ -749,7 +749,8 @@ BEGIN
     IDS_TOOLTIP_EDIT_KEYS,       "Enable enhanced keyboard editing on command line."
     IDS_TOOLTIP_OPACITY,         "Adjust the transparency of the console window."
     IDS_TOOLTIP_INTERCEPT_COPY_PASTE,   "Use Ctrl+Shift+C/V as copy/paste shortcuts, regardless of input mode"
-    IDS_TERMINAL_DEF_INBOX,      "Windows Console Host"
+    IDS_TERMINAL_HANDOFF_DEFAULT, "Let Windows decide"
+    IDS_TERMINAL_HANDOFF_CONHOST, "Windows Console Host"
 END
 
 

src/propsheet/globals.cpp

@@ -60,6 +60,5 @@ COLORREF g_fakeCursorColor = RGB(242, 242, 242); // Default bright white
 HWND g_hTerminalDlg = static_cast<HWND>(INVALID_HANDLE_VALUE);
 HWND g_hOptionsDlg = static_cast<HWND>(INVALID_HANDLE_VALUE);
 
-bool g_defAppEnabled = false;
 std::vector<DelegationConfig::DelegationPackage> g_availablePackages;
 DelegationConfig::DelegationPackage g_selectedPackage;

src/propsheet/globals.h

@@ -56,6 +56,5 @@ extern COLORREF g_fakeCursorColor;
 extern HWND g_hTerminalDlg;
 extern HWND g_hOptionsDlg;
 
-extern bool g_defAppEnabled;
 extern std::vector<DelegationConfig::DelegationPackage> g_availablePackages;
 extern DelegationConfig::DelegationPackage g_selectedPackage;