[2.0-stable] Cherry-pick: AppNotificationManager::Show and AppInstanc…

dev/AppLifecycle/AppInstance.cpp

@@ -20,6 +20,11 @@
 // 63876312: Prevent AppInstance::GetInstances from registering an invalid process handle.
 #define WINAPPSDK_CHANGEID_63876312 63876312
 
+// 61688595: Route AppInstance::GetInstances failures through a noexcept HRESULT worker so a
+// failure no longer unwinds through the deep C++/WinRT projection rethrow path that exhausted
+// the stack via WIL's FormatMessage-based exception logging chain.
+#define WINAPPSDK_CHANGEID_61688595 61688595
+
 using namespace winrt;
 using namespace winrt::Windows::Foundation;
 using namespace winrt::Windows::Foundation::Collections;
@@ -304,7 +309,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         return s_current.as<Microsoft::Windows::AppLifecycle::AppInstance>();
     }
 
-    IVector<Microsoft::Windows::AppLifecycle::AppInstance> AppInstance::GetInstances()
+    IVector<Microsoft::Windows::AppLifecycle::AppInstance> AppInstance::GetInstancesWorker()
     {
         // Force the singleton init.
         GetCurrent();
@@ -365,6 +370,30 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         return instances;
     }
 
+    HRESULT AppInstance::GetInstancesImpl(IVector<Microsoft::Windows::AppLifecycle::AppInstance>& instancesOut) noexcept try
+    {
+        instancesOut = GetInstancesWorker();
+        return S_OK;
+    }
+    CATCH_RETURN()
+
+    IVector<Microsoft::Windows::AppLifecycle::AppInstance> AppInstance::GetInstances()
+    {
+        if (!WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_61688595>())
+        {
+            // Pre-61688595 behavior: let the exception propagate straight out of the worker.
+            return GetInstancesWorker();
+        }
+
+        // Delegate to the noexcept GetInstancesImpl worker and rethrow any failure
+        // HRESULT cleanly via wil::ResultException. This avoids the deep WIL
+        // FormatMessage-based exception logging chain that caused stack overflow on
+        // the hot path (Bug 61688595).
+        IVector<Microsoft::Windows::AppLifecycle::AppInstance> instances{ nullptr };
+        THROW_IF_FAILED(GetInstancesImpl(instances));
+        return instances;
+    }
+
     AppLifecycle::AppInstance AppInstance::FindOrRegisterForKey(hstring const& key)
     {
         // Force the singleton init.

dev/AppLifecycle/AppInstance.h

@@ -55,6 +55,16 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation
         void EnqueueRedirectionRequestId(GUID id);
         GUID DequeueRedirectionRequestId();
 
+        // Throwing implementation of GetInstances. Called directly when contained change
+        // 61688595 is disabled, and via GetInstancesImpl when it is enabled.
+        static winrt::Windows::Foundation::Collections::IVector<Microsoft::Windows::AppLifecycle::AppInstance> GetInstancesWorker();
+
+        // noexcept HRESULT-returning worker for GetInstances. The public GetInstances
+        // boundary is intentionally thin so a failed HRESULT does not trigger the deep
+        // C++/WinRT projection rethrow path that exhausted the stack via WIL's
+        // FormatMessage-based exception logging (Bug 61688595).
+        static HRESULT GetInstancesImpl(winrt::Windows::Foundation::Collections::IVector<Microsoft::Windows::AppLifecycle::AppInstance>& instancesOut) noexcept;
+
         // Named object prefixes used to scope.
         std::wstring m_moduleName;
         std::wstring m_processName;

dev/AppNotifications/AppNotificationManager.cpp

@@ -27,6 +27,12 @@
 #include <ShellLocalization.h>
 #include <filesystem>
 #include <NotificationPlatformActivation.h>
+#include <FrameworkUdk/Containment.h>
+
+// 61688595: Route AppNotificationManager::Show failures through a noexcept HRESULT worker so a
+// failure no longer unwinds through the deep C++/WinRT projection rethrow path that exhausted
+// the stack via WIL's FormatMessage-based exception logging chain.
+#define WINAPPSDK_CHANGEID_61688595 61688595
 
 using namespace std::literals;
 
@@ -421,13 +427,8 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
     }
     CATCH_RETURN()
 
-        void AppNotificationManager::Show(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification)
+    void AppNotificationManager::ShowLegacy(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification)
     {
-        if (!IsSupported())
-        {
-            return;
-        }
-
         auto logTelemetry{ AppNotificationTelemetry::Show::Start(
             g_telemetryHelper,
             m_appId,
@@ -453,6 +454,56 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
         logTelemetry.Stop();
     }
 
+    HRESULT AppNotificationManager::ShowImpl(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) noexcept try
+    {
+        auto logTelemetry{ AppNotificationTelemetry::Show::Start(
+            g_telemetryHelper,
+            m_appId,
+            notification.Payload(),
+            notification.Tag(),
+            notification.Group(),
+            winrt::AppNotificationConferencingConfig::IsCallingPreviewSupported()) };
+
+        RETURN_HR_IF(WPN_E_NOTIFICATION_POSTED, notification.Id() != 0);
+
+        winrt::com_ptr<::ABI::Microsoft::Internal::ToastNotifications::INotificationProperties> notificationProperties = winrt::make_self<NotificationProperties>(notification);
+
+        winrt::com_ptr<::ABI::Microsoft::Internal::ToastNotifications::INotificationTransientProperties> notificationTransientProperties = winrt::make_self<NotificationTransientProperties>(notification);
+
+        DWORD notificationId = 0;
+        RETURN_IF_FAILED(ToastNotifications_PostToast(m_appId.c_str(), notificationProperties.get(), notificationTransientProperties.get(), &notificationId));
+
+        RETURN_HR_IF(E_UNEXPECTED, notificationId == 0);
+
+        implementation::AppNotification* notificationImpl = get_self<implementation::AppNotification>(notification);
+        notificationImpl->SetNotificationId(notificationId);
+
+        logTelemetry.Stop();
+        return S_OK;
+    }
+    CATCH_RETURN()
+
+    void AppNotificationManager::Show(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification)
+    {
+        if (!IsSupported())
+        {
+            return;
+        }
+
+        if (!WinAppSdk::Containment::IsChangeEnabled<WINAPPSDK_CHANGEID_61688595>())
+        {
+            // Pre-61688595 behavior: throw directly from the hot path.
+            ShowLegacy(notification);
+            return;
+        }
+
+        // Delegate to the noexcept ShowImpl worker and rethrow any failure HRESULT
+        // cleanly via wil::ResultException. This avoids the deep WIL
+        // FormatMessage-based exception logging chain that caused stack overflow on
+        // the hot path (Bug 61688595).
+        THROW_IF_FAILED(ShowImpl(notification));
+    }
+
     winrt::Windows::Foundation::IAsyncOperation<winrt::Microsoft::Windows::AppNotifications::AppNotificationProgressResult> AppNotificationManager::UpdateAsync(winrt::Microsoft::Windows::AppNotifications::AppNotificationProgressData const data, hstring const tag, hstring const group)
     {
         if (!IsSupported())

dev/AppNotifications/AppNotificationManager.h

@@ -58,6 +58,16 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
 
         void UnregisterHelper();
 
+        // Throwing implementation of Show. Called directly when contained change 61688595
+        // is disabled, and mirrored by ShowImpl when it is enabled.
+        void ShowLegacy(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification);
+
+        // noexcept HRESULT-returning worker for Show. The public Show boundary is
+        // intentionally thin so a failed HRESULT does not trigger the deep C++/WinRT
+        // projection rethrow path that exhausted the stack via WIL's FormatMessage-based
+        // exception logging (Bug 61688595).
+        HRESULT ShowImpl(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) noexcept;
+
         wil::unique_com_class_object_cookie m_notificationComActivatorRegistration;
         wil::srwlock m_lock;
         winrt::event<NotificationActivationEventHandler> m_notificationHandlers;

test/AppLifecycle/AppLifecycle.vcxproj

@@ -181,6 +181,7 @@
   <Target Name="CopyFiles" AfterTargets="AfterBuild">
     <Copy SkipUnchangedFiles="true" SourceFiles="AppLifecycle.testdef" DestinationFolder="$(OutDir)" />
     <Copy SkipUnchangedFiles="true" SourceFiles="$(OutDir)\..\WindowsAppRuntime_BootstrapDLL\Microsoft.WindowsAppRuntime.Bootstrap.dll" DestinationFolder="$(OutDir)" />
+    <Copy SkipUnchangedFiles="true" SourceFiles="$(OutDir)\..\WindowsAppRuntime_DLL\Microsoft.Internal.FrameworkUdk.dll" DestinationFolder="$(OutDir)" />
   </Target>
   <Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
     <PropertyGroup>

test/AppLifecycle/FunctionalTests.cpp

@@ -95,6 +95,31 @@ namespace Test::AppLifecycle
             VERIFY_IS_NOT_NULL(launchArgs);
         }
 
+        TEST_METHOD(GetInstancesIncludesCurrentInstance)
+        {
+            // Bug 61688595: GetInstances() is now a thin boundary over the noexcept
+            // GetInstancesImpl worker (when the contained change is enabled) or a direct
+            // call into GetInstancesWorker (when it is disabled). Either way the public
+            // API must still enumerate the current instance.
+            auto current{ AppInstance::GetCurrent() };
+            VERIFY_IS_NOT_NULL(current);
+
+            auto instances{ AppInstance::GetInstances() };
+            VERIFY_IS_NOT_NULL(instances);
+            VERIFY_IS_TRUE(instances.Size() > 0);
+
+            bool foundCurrent{ false };
+            for (auto const& instance : instances)
+            {
+                if (instance.ProcessId() == ::GetCurrentProcessId())
+                {
+                    VERIFY_IS_TRUE(instance.IsCurrent());
+                    foundCurrent = true;
+                }
+            }
+            VERIFY_IS_TRUE(foundCurrent);
+        }
+
         TEST_METHOD(GetActivatedEventArgsForFile_Win32)
         {
             // Create a named event for communicating with test app.

test/AppNotificationTests/BaseTestSuite.cpp

@@ -219,6 +219,20 @@ void BaseTestSuite::VerifyShowToast()
     VERIFY_IS_TRUE(VerifyToastsPosted(toastVector));
 }
 
+void BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted()
+{
+    RegisterWithAppNotificationManager();
+
+    AppNotification toast{ CreateToastNotification() };
+    AppNotificationManager::Default().Show(toast);
+    VERIFY_ARE_NOT_EQUAL(0u, toast.Id());
+
+    // Bug 61688595: Show() routes through the noexcept ShowImpl worker (or, when the
+    // contained change is disabled, the legacy throwing body). Either way, re-showing an
+    // already-posted notification must surface WPN_E_NOTIFICATION_POSTED to the caller.
+    VERIFY_THROWS_HR(AppNotificationManager::Default().Show(toast), WPN_E_NOTIFICATION_POSTED);
+}
+
 void BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup()
 {
     RegisterWithAppNotificationManager();

test/AppNotificationTests/BaseTestSuite.h

@@ -27,6 +27,7 @@ class BaseTestSuite
         void VerifyToastSuppressDisplay();
         void VerifyToastExpiresOnReboot();
         void VerifyShowToast();
+        void VerifyShowToastTwiceFailsWithNotificationPosted();
         void VerifyUpdateToastProgressDataUsingValidTagAndValidGroup();
         void VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup();
         void VerifyUpdateToastProgressDataUsingEmptyTagAndValidGroup();

test/AppNotificationTests/PackagedTests.cpp

@@ -78,6 +78,11 @@ void PackagedTests::VerifyShowToast()
     BaseTestSuite::VerifyShowToast();
 }
 
+void PackagedTests::VerifyShowToastTwiceFailsWithNotificationPosted()
+{
+    BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted();
+}
+
 void PackagedTests::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup()
 {
     BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup();

test/AppNotificationTests/PackagedTests.h

@@ -59,6 +59,7 @@ class PackagedTests : BaseTestSuite
     TEST_METHOD(VerifyToastExpiresOnReboot);
     TEST_METHOD(VerifyToastProgressDataSequence0Fail);
     TEST_METHOD(VerifyShowToast);
+    TEST_METHOD(VerifyShowToastTwiceFailsWithNotificationPosted);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndValidGroup);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingEmptyTagAndEmptyGroup);

test/AppNotificationTests/UnpackagedTests.cpp

@@ -78,6 +78,11 @@ void UnpackagedTests::VerifyShowToast()
     BaseTestSuite::VerifyShowToast();
 }
 
+void UnpackagedTests::VerifyShowToastTwiceFailsWithNotificationPosted()
+{
+    BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted();
+}
+
 void UnpackagedTests::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup()
 {
     BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup();

test/AppNotificationTests/UnpackagedTests.h

@@ -57,6 +57,7 @@ class UnpackagedTests : BaseTestSuite
     TEST_METHOD(VerifyToastExpiresOnReboot);
     TEST_METHOD(VerifyToastProgressDataSequence0Fail);
     TEST_METHOD(VerifyShowToast);
+    TEST_METHOD(VerifyShowToastTwiceFailsWithNotificationPosted);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndValidGroup);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup);
     TEST_METHOD(VerifyUpdateToastProgressDataUsingEmptyTagAndEmptyGroup);

test/Compatibility/CompatibilityTests/CompatibilityTests.cpp

@@ -242,5 +242,32 @@ namespace Test::CompatibilityTests
             VERIFY_IS_FALSE((WinAppSdk::Containment::IsChangeEnabled<55555>()));    // explicit only
             VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled<99999>()));     // neither
         }
+
+        // Bug 61688595 gates the AppNotificationManager::Show and
+        // AppInstance::GetInstances noexcept-worker refactor. The change ID is declared
+        // locally at those callsites and is deliberately not part of the catalog, so it
+        // is enabled by default and only an app's explicit DisabledChanges list turns it
+        // off - which restores the pre-61688595 throwing hot paths.
+        static constexpr UINT32 c_changeId_NoexceptHotPaths{ 61688595 };
+
+        TEST_METHOD(VerifyNoexceptHotPathsChangeEnabledByDefault)
+        {
+            winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::RuntimeCompatibilityOptions options;
+            options.Apply();
+
+            VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled<c_changeId_NoexceptHotPaths>()));
+        }
+
+        TEST_METHOD(VerifyNoexceptHotPathsChangeCanBeDisabled)
+        {
+            winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::RuntimeCompatibilityOptions options;
+            options.DisabledChanges().Append((WAR::RuntimeCompatibilityChange)c_changeId_NoexceptHotPaths);
+            options.Apply();
+
+            VERIFY_IS_FALSE((WinAppSdk::Containment::IsChangeEnabled<c_changeId_NoexceptHotPaths>()));
+
+            // A neighbouring ID must be unaffected.
+            VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled<c_changeId_NoexceptHotPaths + 1>()));
+        }
     };
 }