PRE-MERGE #20064 [Unpackaged] Fix taskbar glomming due to AUMID

Author: carlos-zamora

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

@@ -1882,6 +1882,7 @@ WCIA
 WCIW
 wcs
 WCSHELPER
+wcsicmp
 wcsrev
 wcswidth
 wddm

src/cascadia/WindowsTerminal/WindowEmperor.cpp

@@ -11,6 +11,8 @@
 #include <wil/token_helpers.h>
 #include <winrt/TerminalApp.h>
 #include <sddl.h>
+#include <propkey.h>
+#include <propvarutil.h>
 
 #include "AppHost.h"
 #include "resource.h"
@@ -313,6 +315,107 @@ AppHost* WindowEmperor::_mostRecentWindow() const noexcept
     return mostRecent;
 }
 
+// GH#20053: The shell resolves taskbar grouping identity as: per-window AUMID >
+// per-process AUMID > auto-derived from exe path. Before we started setting a
+// process AUMID, both the pinned .lnk and the process used auto-derived
+// identity, so they matched. Now that we set an explicit AUMID, a pinned .lnk
+// that predates the AUMID change has no AUMID and still uses auto-derived
+// identity, causing a mismatch and a duplicate taskbar button.
+//
+// To fix this, we check if a pinned taskbar shortcut (.lnk) points to our exe.
+// If it already carries our AUMID (or no pin exists), we set the process AUMID
+// normally. If a pin exists WITHOUT our AUMID, we skip setting the process
+// AUMID for THIS launch (both sides use auto-derived identity, so they match)
+// and defer stamping the shortcut to process exit. On the next launch, the pin
+// has our AUMID, so we set the process AUMID to match, and both agree.
+//
+// NOTE: On the first launch after pinning, the process AUMID is not set. If
+// toast notifications are needed in the future, use
+// ToastNotificationManager::CreateToastNotifier(aumid) with the AUMID string
+// directly. That API does not depend on SetCurrentProcessExplicitAppUserModelID.
+// A Start Menu shortcut with the AUMID (separate from the taskbar pin) is also
+// required for toast routing; see
+// https://learn.microsoft.com/windows/apps/develop/notifications/app-notifications/send-local-toast-other-apps
+void WindowEmperor::_setupAumid(const std::wstring& aumid)
+{
+    const auto ourExePath = wil::GetModuleFileNameW<std::wstring>(nullptr);
+
+    bool needsDeferredStamping = false;
+    std::wstring pinnedLnkPath;
+
+    const auto taskbarGlob = wil::ExpandEnvironmentStringsW<std::wstring>(
+        LR"(%APPDATA%\Microsoft\Internet Explorer\Quick Launch\User Pinned\TaskBar\*.lnk)");
+
+    WIN32_FIND_DATAW findData{};
+    const wil::unique_hfind findHandle{ FindFirstFileW(taskbarGlob.c_str(), &findData) };
+    if (findHandle)
+    {
+        const auto lastSlash = taskbarGlob.rfind(L'\\');
+        const auto taskbarDir = taskbarGlob.substr(0, lastSlash + 1);
+
+        do
+        {
+            const auto lnkPath = taskbarDir + findData.cFileName;
+
+            wil::com_ptr<IShellLinkW> shellLink;
+            if (FAILED(CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&shellLink))))
+            {
+                continue;
+            }
+
+            const auto persistFile = shellLink.try_query<IPersistFile>();
+            if (!persistFile || FAILED(persistFile->Load(lnkPath.c_str(), STGM_READ)))
+            {
+                continue;
+            }
+
+            wchar_t targetPath[MAX_PATH]{};
+            if (FAILED(shellLink->GetPath(targetPath, MAX_PATH, nullptr, SLGP_RAWPATH)))
+            {
+                continue;
+            }
+
+            if (_wcsicmp(targetPath, ourExePath.c_str()) != 0)
+            {
+                continue;
+            }
+
+            // Found a pin pointing to us. Assume it needs stamping unless
+            // we confirm it already has our AUMID.
+            pinnedLnkPath = lnkPath;
+            needsDeferredStamping = true;
+
+            if (const auto propertyStore = shellLink.try_query<IPropertyStore>())
+            {
+                wil::unique_prop_variant pv;
+                if (SUCCEEDED(propertyStore->GetValue(PKEY_AppUserModel_ID, &pv)) &&
+                    pv.vt == VT_LPWSTR && pv.pwszVal &&
+                    aumid == pv.pwszVal)
+                {
+                    needsDeferredStamping = false;
+                }
+            }
+
+            break;
+        } while (FindNextFileW(findHandle.get(), &findData));
+    }
+
+    if (needsDeferredStamping)
+    {
+        // The pin exists but doesn't have our AUMID yet. Don't set the process
+        // AUMID or stamp the shortcut now. Writing the shortcut causes the
+        // shell to re-read it immediately, changing the pin's cached identity
+        // mid-launch and creating a mismatch in the opposite direction. Instead,
+        // stamp it at shutdown when the taskbar association no longer matters.
+        _pendingAumidLnkPath = std::move(pinnedLnkPath);
+        _pendingAumid = aumid;
+    }
+    else
+    {
+        LOG_IF_FAILED(SetCurrentProcessExplicitAppUserModelID(aumid.c_str()));
+    }
+}
+
 void WindowEmperor::HandleCommandlineArgs(int nCmdShow)
 {
     // When running without package identity, set an explicit AppUserModelID so
@@ -373,7 +476,7 @@ void WindowEmperor::HandleCommandlineArgs(int nCmdShow)
 #else
             fmt::format_to(std::back_inserter(unpackagedAumid), FMT_COMPILE(L".{:08x}"), hash);
 #endif
-            LOG_IF_FAILED(SetCurrentProcessExplicitAppUserModelID(unpackagedAumid.c_str()));
+            _setupAumid(unpackagedAumid);
         }
     }
 
@@ -553,6 +656,29 @@ void WindowEmperor::HandleCommandlineArgs(int nCmdShow)
         Shell_NotifyIconW(NIM_DELETE, &_notificationIcon);
     }
 
+    // GH#20053: Deferred shortcut stamping. See _setupAumid() for context.
+    if (!_pendingAumidLnkPath.empty())
+    {
+        wil::com_ptr<IShellLinkW> shellLink;
+        if (SUCCEEDED(CoCreateInstance(CLSID_ShellLink, nullptr, CLSCTX_INPROC_SERVER, IID_PPV_ARGS(&shellLink))))
+        {
+            if (const auto persistFile = shellLink.try_query<IPersistFile>();
+                persistFile && SUCCEEDED(persistFile->Load(_pendingAumidLnkPath.c_str(), STGM_READWRITE)))
+            {
+                if (const auto propertyStore = shellLink.try_query<IPropertyStore>())
+                {
+                    wil::unique_prop_variant pv;
+                    if (SUCCEEDED(InitPropVariantFromString(_pendingAumid.c_str(), &pv)) &&
+                        SUCCEEDED(propertyStore->SetValue(PKEY_AppUserModel_ID, pv)) &&
+                        SUCCEEDED(propertyStore->Commit()))
+                    {
+                        persistFile->Save(_pendingAumidLnkPath.c_str(), TRUE);
+                    }
+                }
+            }
+        }
+    }
+
     // There's a mysterious crash in XAML on Windows 10 if you just let _app get destroyed (GH#15410).
     // We also need to ensure that all UI threads exit before WindowEmperor leaves the scope on the main thread (MSFT:46744208).
     // Both problems can be solved and the shutdown accelerated by using TerminateProcess.

src/cascadia/WindowsTerminal/WindowEmperor.h

@@ -68,6 +68,7 @@ class WindowEmperor
     void _persistState(const winrt::Microsoft::Terminal::Settings::Model::ApplicationState& state) const;
     void _finalizeSessionPersistence() const;
     void _checkWindowsForNotificationIcon();
+    void _setupAumid(const std::wstring& aumid);
 
     wil::unique_hwnd _window;
     winrt::TerminalApp::App _app{ nullptr };
@@ -83,6 +84,8 @@ class WindowEmperor
     std::optional<bool> _currentSystemThemeIsDark;
     int32_t _windowCount = 0;
     int32_t _messageBoxCount = 0;
+    std::wstring _pendingAumidLnkPath;
+    std::wstring _pendingAumid;
 
 #if 0 // #ifdef NDEBUG
     static constexpr void _assertIsMainThread() noexcept