Address comments

microsoft · Jun 8, 2022 · 212455d · 212455d
1 parent 4918385
commit 212455d
Show file tree

Hide file tree

Showing 5 changed files with 32 additions and 23 deletions.
diff --git a/dev/AppNotifications/AppNotificationManager.cpp b/dev/AppNotifications/AppNotificationManager.cpp
@@ -170,7 +170,7 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation
 
             THROW_HR_IF(E_INVALIDARG, (displayName == winrt::hstring{}) || (iconUri == nullptr));
 
-            AppNotificationAssets assets{ GetAssets(displayName.c_str(), iconUri.RawUri().c_str()) };
+            AppNotificationAssets assets{ ValidateAssets(displayName.c_str(), iconUri.RawUri().c_str()) };
 
             {
                 auto lock{ m_lock.lock_exclusive() };

diff --git a/dev/AppNotifications/AppNotificationUtility.cpp b/dev/AppNotifications/AppNotificationUtility.cpp
@@ -286,13 +286,13 @@ AppNotificationAssets Microsoft::Windows::AppNotifications::Helpers::GetAssets()
     return assets;
 }
 
-AppNotificationAssets Microsoft::Windows::AppNotifications::Helpers::GetAssets(std::wstring displayName,std::wstring iconFilePath)
+AppNotificationAssets Microsoft::Windows::AppNotifications::Helpers::ValidateAssets(std::wstring displayName, std::filesystem::path iconFilePath)
 {
     winrt::check_bool(std::filesystem::exists(iconFilePath));
 
-    THROW_HR_IF_MSG(E_INVALIDARG, !IsIconFileExtensionSupported(std::filesystem::path{ iconFilePath }), "Icon format not supported");
+    THROW_HR_IF_MSG(E_INVALIDARG, !IsIconFileExtensionSupported(iconFilePath), "Icon format not supported");
 
-    return AppNotificationAssets{ std::move(displayName), std::move(iconFilePath) };
+    return AppNotificationAssets{ std::move(displayName), iconFilePath.wstring() };
 }
 
 void Microsoft::Windows::AppNotifications::Helpers::RegisterAssets(std::wstring const& appId, std::wstring const& clsid, AppNotificationAssets const& assets)

diff --git a/dev/AppNotifications/AppNotificationUtility.h b/dev/AppNotifications/AppNotificationUtility.h
@@ -66,5 +66,5 @@ namespace Microsoft::Windows::AppNotifications::Helpers
 
     Microsoft::Windows::AppNotifications::ShellLocalization::AppNotificationAssets GetAssets();
 
-    Microsoft::Windows::AppNotifications::ShellLocalization::AppNotificationAssets GetAssets(std::wstring displayName, std::wstring iconFilePath);
+    Microsoft::Windows::AppNotifications::ShellLocalization::AppNotificationAssets ValidateAssets(std::wstring displayName, std::filesystem::path iconFilePath);
 }
diff --git a/specs/AppNotifications/AppNotifications-spec.md b/specs/AppNotifications/AppNotifications-spec.md
@@ -139,6 +139,29 @@ int main()
 }
 ```
 
+## Registering for App Notifications using assets
+
+For Unpackaged applications, the developer can Register using a custom Display Name and Icon.
+WinAppSDK will register the application and display these assets when an App Notification is received.
+The developer should provide both the assets or not provide them at all. Icon provided by the developer
+should be a valid supported format. The API supports the formats - png, bmp, jpg. The icon
+should reside on the local machine only otherwise the API throws an exception. For Packaged applications,
+this API is not applicable and will throw an exception.
+
+```cpp
+int main()
+{
+    auto manager = winrt::AppNotificationManager::Default();
+    manager.Register(L"AppNotifications", winrt::Windows::Foundation::Uri {L"<Path to Icon>"});
+
+    // other app init and then message loop here
+
+    // Call Unregister() before exiting main so that subsequent invocations will launch a new process
+    manager.Unregister();
+    return 0;
+}
+```
+
 ## Displaying an App Notification
 
 To display a Notification, an app needs to define a payload in xml. In the example below, the
@@ -451,6 +474,9 @@ namespace Microsoft.Windows.AppNotifications
         // For Unpackaged apps, the caller process will be registered as the COM server. And assets like displayname and icon will be gleaned from Shell and registered as well.
         void Register();
 
+         // For Unpackaged apps only, the caller process will be registered as the COM server.
+        void Register(String displayName, Windows.Foundation.Uri iconUri);
+
         // Unregisters the COM Service so that a subsequent activation will launch a new process
         void Unregister();
 

diff --git a/test/TestApps/ToastNotificationsTestApp/main.cpp b/test/TestApps/ToastNotificationsTestApp/main.cpp
@@ -1329,9 +1329,6 @@ bool VerifyRegisterWithNullDisplayNameFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(winrt::hstring{}, winrt::Uri{ LR"(C:\Windows\System32\WindowsSecurityIcon.png)" });
-
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1348,8 +1345,6 @@ bool VerifyRegisterWithNullIconFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(L"AppNotificationApp", nullptr);
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1366,9 +1361,6 @@ bool VerifyRegisterWithNullDisplayNameAndNullIconFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(winrt::hstring{}, nullptr);
-
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1403,8 +1395,6 @@ bool VerifyRegisterWithDisplayNameAndInvalidIconPathFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(L"AppNotificationApp", winrt::Uri{ LR"(C:\InvalidPath\)" });
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1422,8 +1412,6 @@ bool VerifyRegisterWithEmptyDisplayNameFail_Unpackaged()
     {
         // hstring treats L"" as assigning nullptr
         winrt::AppNotificationManager::Default().Register(L"", winrt::Uri{ LR"(C:\Windows\System32\WindowsSecurityIcon.png)" });
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1440,8 +1428,6 @@ bool VerifyRegisterWithEmptyIconFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(L"AppNotificationApp", winrt::Uri{ L"" });
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1458,8 +1444,6 @@ bool VerifyRegisterWithEmptyDisplayNameAndEmptyIconFail_Unpackaged()
     try
     {
         winrt::AppNotificationManager::Default().Register(L"", winrt::Uri{ L"" });
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {
@@ -1475,9 +1459,8 @@ bool VerifyRegisterWithAssetsFail()
     winrt::AppNotificationManager::Default().UnregisterAll();
     try
     {
+        // API fails for Packaged Scenario
         winrt::AppNotificationManager::Default().Register(L"AppNotificationApp", winrt::Uri{ LR"(C:\InvalidPath\)" });
-
-        winrt::AppNotificationManager::Default().UnregisterAll();
     }
     catch (...)
     {