Add Selfcontained checks and misc fixes for race condition in AppNoti… (

#2267)

* Add Selfcontained checks and misc fixes for race condition in AppNotificationManager::Register()

* Address PR concerns

dev/AppNotifications/AppNotificationManager.cpp

@@ -20,6 +20,7 @@
 #include <string_view>
 #include <winrt/Windows.Foundation.Collections.h>
 #include <TerminalVelocityFeatures-AppNotifications.h>
+#include <WindowsAppRuntime.SelfContained.h>
 
 using namespace std::literals;
 
@@ -95,6 +96,18 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
 
         try
         {
+            {
+                auto lock{ m_lock.lock_exclusive() };
+                THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_OPERATION_IN_PROGRESS), m_registering, "Registration is in progress!");
+                m_registering = true;
+            }
+
+            auto registeringScopeExit{ wil::scope_exit([&]()
+            {
+                auto lock { m_lock.lock_exclusive() };
+                m_registering = false;
+            }) };
+
             winrt::guid storedComActivatorGuid{ GUID_NULL };
             if (!PushNotificationHelpers::IsPackagedAppScenario())
             {
@@ -105,8 +118,11 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
                     storedComActivatorGuid = RegisterComActivatorGuidAndAssets();
                 }
 
-                auto notificationPlatform{ PushNotificationHelpers::GetNotificationPlatform() };
-                THROW_IF_FAILED(notificationPlatform->AddToastRegistrationMapping(m_processName.c_str(), m_appId.c_str()));
+                if (!WindowsAppRuntime::SelfContained::IsSelfContained())
+                {
+                    auto notificationPlatform{ PushNotificationHelpers::GetNotificationPlatform() };
+                    THROW_IF_FAILED(notificationPlatform->AddToastRegistrationMapping(m_processName.c_str(), m_appId.c_str()));
+                }
             }
 
             winrt::guid registeredClsid{ GUID_NULL };
@@ -139,6 +155,12 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
         }
     }
 
+    // This assumes that the caller has taken an exclusive lock
+    void AppNotificationManager::UnregisterHelper()
+    {
+        m_notificationComActivatorRegistration.reset();
+    }
+
     void AppNotificationManager::Unregister()
     {
         HRESULT hr{ S_OK };
@@ -150,8 +172,15 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
         try
         {
             auto lock{ m_lock.lock_exclusive() };
+            THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_OPERATION_IN_PROGRESS), m_registering, "Register or Unregister currently in progress!");
+            m_registering = true;
+            auto scope_exit = wil::scope_exit(
+                [&] {
+                    m_registering = false;
+                });
+
             THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), !m_notificationComActivatorRegistration, "Not Registered for App Notifications!");
-            m_notificationComActivatorRegistration.reset();
+            UnregisterHelper();
         }
         catch (...)
         {
@@ -170,10 +199,21 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
 
         try
         {
-            Unregister();
+            {
+                auto lock{ m_lock.lock_exclusive() };
+                UnregisterHelper();
+                THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_OPERATION_IN_PROGRESS), m_registering, "Register or Unregister currently in progress!");
+                m_registering = true;
+            }
+
+            auto scope_exit = wil::scope_exit(
+                [&] {
+                    auto lock{ m_lock.lock_exclusive() };
+                    m_registering = false;
+                });
 
             // Remove any Registrations from the Long Running Process that are necessary for Cloud toasts
-            if (!PushNotificationHelpers::IsPackagedAppScenario())
+            if (!PushNotificationHelpers::IsPackagedAppScenario() && !WindowsAppRuntime::SelfContained::IsSelfContained())
             {
                 auto notificationPlatform{ PushNotificationHelpers::GetNotificationPlatform() };
                 THROW_IF_FAILED(notificationPlatform->RemoveToastRegistrationMapping(m_processName.c_str()));

dev/AppNotifications/AppNotificationManager.h

@@ -44,6 +44,9 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
         // INotificationManagerDeserializer
         winrt::Windows::Foundation::IInspectable Deserialize(winrt::Windows::Foundation::Uri const& uri);
     private:
+
+        void UnregisterHelper();
+
         wil::unique_com_class_object_cookie m_notificationComActivatorRegistration;
         wil::srwlock m_lock;
         winrt::event<NotificationActivationEventHandler> m_notificationHandlers;
@@ -52,6 +55,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
         wil::unique_event m_waitHandleForArgs;
         winrt::Microsoft::Windows::AppNotifications::AppNotificationActivatedEventArgs m_activatedEventArgs{ nullptr };
         std::wstring m_appId;
+        bool m_registering{ false };
     };
 
     struct AppNotificationManagerFactory : winrt::implements<AppNotificationManagerFactory, IClassFactory>

dev/AppNotifications/NotificationProperties.cpp

@@ -27,9 +27,7 @@ namespace Helpers
 NotificationProperties::NotificationProperties(winrt::AppNotification const& toastNotification)
 {
     // Extract payload and convert it from XML to a byte array
-    auto payload = toastNotification.Payload();
-
-    auto payloadAsSimpleString = Helpers::WideStringToUtf8String(payload.c_str());
+    auto payloadAsSimpleString = Helpers::WideStringToUtf8String(toastNotification.Payload());
 
     m_payload = wil::unique_cotaskmem_array_ptr<byte>(static_cast<byte*>(CoTaskMemAlloc(payloadAsSimpleString.size())), payloadAsSimpleString.size());
     THROW_IF_NULL_ALLOC(m_payload.get());

dev/PushNotifications/PushNotificationCreateChannelResult.h

@@ -8,6 +8,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 {
     struct PushNotificationCreateChannelResult : PushNotificationCreateChannelResultT<PushNotificationCreateChannelResult>
     {
+        PushNotificationCreateChannelResult() = default;
         PushNotificationCreateChannelResult(Microsoft::Windows::PushNotifications::PushNotificationChannel const& channel, hresult const& extendedError, Microsoft::Windows::PushNotifications::PushNotificationChannelStatus const& status);
         Microsoft::Windows::PushNotifications::PushNotificationChannel Channel();
         winrt::hresult ExtendedError();
@@ -19,9 +20,3 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
         const Microsoft::Windows::PushNotifications::PushNotificationChannelStatus m_status;
     };
 }
-namespace winrt::Microsoft::Windows::PushNotifications::factory_implementation
-{
-    struct PushNotificationCreateChannelResult : PushNotificationCreateChannelResultT<PushNotificationCreateChannelResult, implementation::PushNotificationCreateChannelResult>
-    {
-    };
-}

dev/PushNotifications/PushNotificationManager.cpp

@@ -127,6 +127,14 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
     }
     CATCH_RETURN()
 
+    bool PushNotificationManager::IsSupported()
+    {
+        // Only scenarios that use the Background Infrastructure component of the OS support Push in the SelfContained case
+        static bool isSupported{ !WindowsAppRuntime::SelfContained::IsSelfContained() || PushNotificationHelpers::IsPackagedAppScenario() };
+        return isSupported;
+
+    }
+
     winrt::Microsoft::Windows::PushNotifications::PushNotificationManager PushNotificationManager::Default()
     {
         THROW_HR_IF(E_NOTIMPL, !::Microsoft::Windows::PushNotifications::Feature_PushNotifications::IsEnabled());
@@ -158,29 +166,46 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     winrt::Windows::Foundation::IInspectable PushNotificationManager::Deserialize(winrt::Windows::Foundation::Uri const& uri)
     {
-        // Verify if the uri contains a background notification payload.
-        // Otherwise, we expect to process the notification in a background task.
-        for (auto const& pair : uri.QueryParsed())
+        winrt::Microsoft::Windows::PushNotifications::PushNotificationReceivedEventArgs eventArgs{ nullptr };
+
+        // All packaged processes are triggered through COM via Long Running Process or the Background Infra OS component
+        if (AppModel::Identity::IsPackagedProcess())
         {
-            if (pair.Name() == L"payload")
+            THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_TIMEOUT), !m_waitHandleForArgs.wait(c_receiveArgsTimeoutInMSec));
+            auto lock{ m_lock.lock_shared() };
+            THROW_HR_IF(E_UNEXPECTED, !m_backgroundTaskArgs);
+            eventArgs = m_backgroundTaskArgs;
+        }
+        else // The process was launched via ShellExecute and we need to parse the uri (Only unpackaged)
+        {
+            for (auto const& pair : uri.QueryParsed())
             {
-                // Convert escaped components to its normal content
-                // from the conversion done in the LRP (see NotificationListener.cpp)
-                std::wstring payloadAsEscapedWstring{ pair.Value() };
-                std::wstring payloadAsWstring{ winrt::Windows::Foundation::Uri::UnescapeComponent(payloadAsEscapedWstring) };
-                return winrt::make<winrt::Microsoft::Windows::PushNotifications::implementation::PushNotificationReceivedEventArgs>(payloadAsWstring);
+                if (pair.Name() == L"payload")
+                {
+                    // Convert escaped components to its normal content from the conversion done in the Long Running Process (see NotificationListener.cpp)
+                    auto payloadAsWstring = winrt::Windows::Foundation::Uri::UnescapeComponent(pair.Value());
+                    eventArgs = winrt::make<winrt::Microsoft::Windows::PushNotifications::implementation::PushNotificationReceivedEventArgs>(payloadAsWstring);
+                }
             }
+
+            THROW_HR_IF_NULL_MSG(E_UNEXPECTED, eventArgs, "Could not serialize payload from command line Uri!");
         }
 
-        THROW_HR_IF(HRESULT_FROM_WIN32(ERROR_TIMEOUT), !m_waitHandleForArgs.wait(c_receiveArgsTimeoutInMSec));
-        THROW_HR_IF(E_UNEXPECTED, !m_backgroundTaskArgs);
-        return m_backgroundTaskArgs;
+        return eventArgs;
     }
 
     winrt::IAsyncOperationWithProgress<winrt::Microsoft::Windows::PushNotifications::PushNotificationCreateChannelResult, winrt::Microsoft::Windows::PushNotifications::PushNotificationCreateChannelStatus> PushNotificationManager::CreateChannelAsync(const winrt::guid remoteId)
     {
         THROW_HR_IF(E_NOTIMPL, !::Microsoft::Windows::PushNotifications::Feature_PushNotifications::IsEnabled());
 
+        if (!IsSupported())
+        {
+            co_return winrt::make<PushNotificationCreateChannelResult>(
+                nullptr,
+                E_FAIL,
+                PushNotificationChannelStatus::CompletedFailure);
+        }
+
         auto strong = get_strong();
 
         try
@@ -257,8 +282,8 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
                         // registeredClsid is set to GUID_NULL for unpackaged applications
                         THROW_IF_FAILED(notificationPlatform->RegisterLongRunningActivatorWithClsid(processName.c_str(), registeredClsid));
 
-                        std::wstring toastAppId{ RetrieveNotificationAppId() };
-                        THROW_IF_FAILED(notificationPlatform->AddToastRegistrationMapping(processName.c_str(), toastAppId.c_str()));
+                        std::wstring appId{ RetrieveNotificationAppId() };
+                        THROW_IF_FAILED(notificationPlatform->AddToastRegistrationMapping(processName.c_str(), appId.c_str()));
                     }
 
                     auto channel{ winrt::make<PushNotificationChannel>(channelInfo) };
@@ -331,6 +356,11 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     void PushNotificationManager::Register()
     {
+        if (!IsSupported())
+        {
+            return;
+        }
+
         winrt::hresult hr{ S_OK };
         try
         {
@@ -551,6 +581,11 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     void PushNotificationManager::Unregister()
     {
+        if (!IsSupported())
+        {
+            return;
+        }
+
         winrt::hresult hr{ S_OK };
         try
         {
@@ -621,6 +656,11 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     void PushNotificationManager::UnregisterAll()
     {
+        if (!IsSupported())
+        {
+            return;
+        }
+
         bool comActivatorRegistration{ false };
         bool singletonForegroundRegistration{ false };
         {
@@ -697,6 +737,11 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     winrt::event_token PushNotificationManager::PushReceived(TypedEventHandler<winrt::Microsoft::Windows::PushNotifications::PushNotificationManager, winrt::Microsoft::Windows::PushNotifications::PushNotificationReceivedEventArgs> handler)
     {
+        if (!IsSupported())
+        {
+            return winrt::event_token{};
+        }
+
         {
             auto lock{ m_lock.lock_shared() };
             THROW_HR_IF_MSG(HRESULT_FROM_WIN32(ERROR_NOT_FOUND), m_comActivatorRegistration || m_singletonLongRunningSinkRegistration, "Must register event handlers before calling Register().");
@@ -709,8 +754,11 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
     void PushNotificationManager::PushReceived(winrt::event_token const& token) noexcept
     {
-        auto lock{ m_lock.lock_exclusive() };
-        m_foregroundHandlers.remove(token);
+        if (IsSupported())
+        {
+            auto lock{ m_lock.lock_exclusive() };
+            m_foregroundHandlers.remove(token);
+        }
     }
 
     IFACEMETHODIMP PushNotificationManager::InvokeAll(_In_ ULONG length, _In_ byte* payload, _Out_ BOOL* foregroundHandled) noexcept try

dev/PushNotifications/PushNotificationManager.h

@@ -26,6 +26,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
     {
         PushNotificationManager();
         ~PushNotificationManager();
+        static bool IsSupported();
         static winrt::Microsoft::Windows::PushNotifications::PushNotificationManager Default();
         static winrt::Windows::Foundation::IInspectable PushDeserialize(winrt::Windows::Foundation::Uri const& uri);
         void Register();

dev/PushNotifications/PushNotificationReceivedEventArgs.cpp

@@ -48,7 +48,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
         THROW_HR_IF(E_NOTIMPL, !::Microsoft::Windows::PushNotifications::Feature_PushNotifications::IsEnabled());
     }
 
-    PushNotificationReceivedEventArgs::PushNotificationReceivedEventArgs(std::wstring const& payload) :
+    PushNotificationReceivedEventArgs::PushNotificationReceivedEventArgs(winrt::hstring const& payload) :
         m_rawNotificationPayload(BuildPayload(payload)),
         m_unpackagedAppScenario(true)
     {
@@ -65,7 +65,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
         return { payload, payload + (length * sizeof(uint8_t)) };
     }
 
-    std::vector<uint8_t> PushNotificationReceivedEventArgs::BuildPayload(std::wstring const& payload)
+    std::vector<uint8_t> PushNotificationReceivedEventArgs::BuildPayload(winrt::hstring const& payload)
     {
         std::string payloadToSimpleString{ ::winrt::Microsoft::Windows::PushNotifications::Helpers::WideStringToUtf8String(payload) };
         return { payloadToSimpleString.c_str(), payloadToSimpleString.c_str() + (payloadToSimpleString.length() * sizeof(uint8_t)) };

dev/PushNotifications/PushNotificationReceivedEventArgs.h

@@ -12,7 +12,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
         PushNotificationReceivedEventArgs(winrt::Windows::ApplicationModel::Background::IBackgroundTaskInstance const& backgroundTask);
         PushNotificationReceivedEventArgs(winrt::Windows::Networking::PushNotifications::PushNotificationReceivedEventArgs const& args);
 
-        PushNotificationReceivedEventArgs(std::wstring const& payload);
+        PushNotificationReceivedEventArgs(winrt::hstring const& payload);
 
         PushNotificationReceivedEventArgs(byte* const& payload, ULONG const& length);
 
@@ -26,7 +26,7 @@ namespace winrt::Microsoft::Windows::PushNotifications::implementation
 
         std::vector<uint8_t> BuildPayload(winrt::Windows::Storage::Streams::IBuffer const& buffer);
         std::vector<uint8_t> BuildPayload(byte* const& payload, ULONG const& length);
-        std::vector<uint8_t> BuildPayload(std::wstring const& payload);
+        std::vector<uint8_t> BuildPayload(winrt::hstring const& payload);
 
         std::vector<uint8_t> m_rawNotificationPayload;
 

dev/PushNotifications/PushNotificationUtility.h

@@ -21,13 +21,13 @@ namespace winrt::Microsoft::Windows::PushNotifications::Helpers
     inline constexpr std::wstring_view c_argumentCommandString = L"Arguments";
     inline constexpr std::wstring_view c_executableCommandString = L"Executable";
 
-    inline std::string WideStringToUtf8String(_In_ std::wstring const& utf16string)
+    inline std::string WideStringToUtf8String(_In_ winrt::hstring const& utf16string)
     {
         int size = WideCharToMultiByte(
             CP_UTF8,
             0,
-            utf16string.data(),
-            static_cast<int>(utf16string.length()),
+            utf16string.c_str(),
+            static_cast<int>(utf16string.size()),
             nullptr,
             0,
             nullptr,
@@ -41,8 +41,8 @@ namespace winrt::Microsoft::Windows::PushNotifications::Helpers
         size = WideCharToMultiByte(
             CP_UTF8,
             0,
-            utf16string.data(),
-            static_cast<int>(utf16string.length()),
+            utf16string.c_str(),
+            static_cast<int>(utf16string.size()),
             &utf8string[0],
             size,
             nullptr,

dev/PushNotifications/PushNotifications.idl

@@ -58,11 +58,6 @@ namespace Microsoft.Windows.PushNotifications
     [feature(Feature_PushNotifications)]
     runtimeclass PushNotificationCreateChannelResult
     {
-        PushNotificationCreateChannelResult(
-            PushNotificationChannel channel,
-            HRESULT extendedError,
-            PushNotificationChannelStatus status);
-
         // The Push channel associated with the Result. Valid only if status is CompletedSuccess.
         PushNotificationChannel Channel { get; };
 
@@ -76,6 +71,10 @@ namespace Microsoft.Windows.PushNotifications
     [feature(Feature_PushNotifications)]
     runtimeclass PushNotificationManager
     {
+        // Checks to see if the APIs are supported for the Desktop app
+        // Certain self-contained apps may not support Push Notification scenarios by design
+        static Boolean IsSupported();
+
         // Gets a Default instance of a PushNotificationManager
         static PushNotificationManager Default{ get; };