Code Quality: Improved SetAs actions

src/Files.App.CsWin32/Windows.Win32.ComPtr.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Runtime.CompilerServices;
 using Windows.Win32;
+using Windows.Win32.Foundation;
 using Windows.Win32.System.Com;
 
 namespace Windows.Win32
@@ -39,11 +40,19 @@ public ComPtr(T* ptr)
 		}
 
 		[MethodImpl(MethodImplOptions.AggressiveInlining)]
-		public readonly ComPtr<U> As<U>(Guid riid) where U : unmanaged
+		public readonly ComPtr<U> As<U>() where U : unmanaged
 		{
-			ComPtr<U> pNewPtr = default;
-			((IUnknown*)_ptr)->QueryInterface(&riid, (void**)pNewPtr.GetAddressOf());
-			return pNewPtr;
+			ComPtr<U> ptr = default;
+			Guid iid = typeof(U).GUID;
+			((IUnknown*)_ptr)->QueryInterface(&iid, (void**)ptr.GetAddressOf());
+			return ptr;
+		}
+
+		[MethodImpl(MethodImplOptions.AggressiveInlining)]
+		public readonly HRESULT CoCreateInstance<U>(CLSCTX dwClsContext = CLSCTX.CLSCTX_LOCAL_SERVER)
+		{
+			Guid iid = typeof(T).GUID, clsid = typeof(U).GUID;
+			return PInvoke.CoCreateInstance(&clsid, null, dwClsContext, &iid, (void**)this.GetAddressOf());
 		}
 
 		[MethodImpl(MethodImplOptions.AggressiveInlining)]

src/Files.App/Actions/Content/Background/BaseSetAsAction.cs

@@ -8,7 +8,8 @@ namespace Files.App.Actions
 {
 	internal abstract class BaseSetAsAction : ObservableObject, IAction
 	{
-		protected readonly IContentPageContext context;
+		protected readonly IContentPageContext ContentPageContext = Ioc.Default.GetRequiredService<IContentPageContext>();
+		protected readonly IWindowsWallpaperService WindowsWallpaperService = Ioc.Default.GetRequiredService<IWindowsWallpaperService>();
 
 		public abstract string Label { get; }
 
@@ -17,16 +18,14 @@ internal abstract class BaseSetAsAction : ObservableObject, IAction
 		public abstract RichGlyph Glyph { get; }
 
 		public virtual bool IsExecutable =>
-			context.ShellPage is not null &&
-			context.PageType != ContentPageTypes.RecycleBin &&
-			context.PageType != ContentPageTypes.ZipFolder &&
-			(context.ShellPage?.SlimContentPage?.SelectedItemsPropertiesViewModel?.IsCompatibleToSetAsWindowsWallpaper ?? false);
+			ContentPageContext.ShellPage is not null &&
+			ContentPageContext.PageType != ContentPageTypes.RecycleBin &&
+			ContentPageContext.PageType != ContentPageTypes.ZipFolder &&
+			(ContentPageContext.ShellPage?.SlimContentPage?.SelectedItemsPropertiesViewModel?.IsCompatibleToSetAsWindowsWallpaper ?? false);
 
 		public BaseSetAsAction()
 		{
-			context = Ioc.Default.GetRequiredService<IContentPageContext>();
-
-			context.PropertyChanged += Context_PropertyChanged;
+			ContentPageContext.PropertyChanged += ContentPageContext_PropertyChanged;
 		}
 
 		public abstract Task ExecuteAsync(object? parameter = null);
@@ -46,7 +45,7 @@ protected async void ShowErrorDialog(string message)
 			await errorDialog.TryShowAsync();
 		}
 
-		private void Context_PropertyChanged(object? sender, PropertyChangedEventArgs e)
+		private void ContentPageContext_PropertyChanged(object? sender, PropertyChangedEventArgs e)
 		{
 			switch (e.PropertyName)
 			{
@@ -56,10 +55,10 @@ private void Context_PropertyChanged(object? sender, PropertyChangedEventArgs e)
 				case nameof(IContentPageContext.SelectedItem):
 				case nameof(IContentPageContext.SelectedItems):
 					{
-						if (context.ShellPage is not null && context.ShellPage.SlimContentPage is not null)
+						if (ContentPageContext.ShellPage is not null && ContentPageContext.ShellPage.SlimContentPage is not null)
 						{
-							var viewModel = context.ShellPage.SlimContentPage.SelectedItemsPropertiesViewModel;
-							var extensions = context.SelectedItems.Select(selectedItem => selectedItem.FileExtension).Distinct().ToList();
+							var viewModel = ContentPageContext.ShellPage.SlimContentPage.SelectedItemsPropertiesViewModel;
+							var extensions = ContentPageContext.SelectedItems.Select(selectedItem => selectedItem.FileExtension).Distinct().ToList();
 
 							viewModel.CheckAllFileExtensions(extensions);
 						}

src/Files.App/Actions/Content/Background/SetAsAppBackgroundAction.cs

@@ -5,7 +5,7 @@ namespace Files.App.Actions
 {
 	internal sealed class SetAsAppBackgroundAction : BaseSetAsAction
 	{
-		private IAppearanceSettingsService AppearanceSettingsService { get; } = Ioc.Default.GetRequiredService<IAppearanceSettingsService>();
+		private readonly IAppearanceSettingsService AppearanceSettingsService = Ioc.Default.GetRequiredService<IAppearanceSettingsService>();
 
 		public override string Label
 			=> "SetAsAppBackground".GetLocalizedResource();
@@ -18,12 +18,12 @@ public override RichGlyph Glyph
 
 		public override bool IsExecutable =>
 			base.IsExecutable &&
-			context.SelectedItem is not null;
+			ContentPageContext.SelectedItem is not null;
 
 		public override Task ExecuteAsync(object? parameter = null)
 		{
-			if (context.SelectedItem is not null)
-				AppearanceSettingsService.AppThemeBackgroundImageSource = context.SelectedItem.ItemPath;
+			if (IsExecutable && ContentPageContext.SelectedItem is ListedItem selectedItem)
+				AppearanceSettingsService.AppThemeBackgroundImageSource = selectedItem.ItemPath;
 
 			return Task.CompletedTask;
 		}

src/Files.App/Actions/Content/Background/SetAsLockscreenBackgroundAction.cs

@@ -1,6 +1,8 @@
 // Copyright (c) 2024 Files Community
 // Licensed under the MIT License. See the LICENSE.
 
+using Microsoft.Extensions.Logging;
+
 namespace Files.App.Actions
 {
 	internal sealed class SetAsLockscreenBackgroundAction : BaseSetAsAction
@@ -18,16 +20,16 @@ public override RichGlyph Glyph
 
 		public override bool IsExecutable =>
 			base.IsExecutable &&
-			context.SelectedItem is not null;
+			ContentPageContext.SelectedItem is not null;
 
 		public override Task ExecuteAsync(object? parameter = null)
 		{
-			if (context.SelectedItem is null)
+			if (!IsExecutable || ContentPageContext.SelectedItem is not ListedItem selectedItem)
 				return Task.CompletedTask;
 
 			try
 			{
-				return WindowsWallpaperService.SetLockScreenWallpaper(context.SelectedItem.ItemPath);
+				return WindowsWallpaperService.SetLockScreenWallpaper(selectedItem.ItemPath);
 			}
 			catch (Exception ex)
 			{

src/Files.App/Actions/Content/Background/SetAsSlideshowBackgroundAction.cs

@@ -1,6 +1,8 @@
 // Copyright (c) 2024 Files Community
 // Licensed under the MIT License. See the LICENSE.
 
+using Microsoft.Extensions.Logging;
+
 namespace Files.App.Actions
 {
 	internal sealed class SetAsSlideshowBackgroundAction : BaseSetAsAction
@@ -18,11 +20,12 @@ public override RichGlyph Glyph
 
 		public override bool IsExecutable =>
 			base.IsExecutable &&
-			context.SelectedItems.Count > 1;
+			ContentPageContext.SelectedItems.Count > 1;
 
 		public override Task ExecuteAsync(object? parameter = null)
 		{
-			var paths = context.SelectedItems.Select(item => item.ItemPath).ToArray();
+			if (!IsExecutable || ContentPageContext.SelectedItems.Select(item => item.ItemPath).ToArray() is not string[] paths || paths.Length is 0)
+				return Task.CompletedTask;
 
 			try
 			{

src/Files.App/Actions/Content/Background/SetAsWallpaperBackgroundAction.cs

@@ -1,12 +1,12 @@
 // Copyright (c) 2024 Files Community
 // Licensed under the MIT License. See the LICENSE.
 
+using Microsoft.Extensions.Logging;
+
 namespace Files.App.Actions
 {
 	internal sealed class SetAsWallpaperBackgroundAction : BaseSetAsAction
 	{
-		private readonly IWindowsWallpaperService WindowsWallpaperService = Ioc.Default.GetRequiredService<IWindowsWallpaperService>();
-
 		public override string Label
 			=> "SetAsBackground".GetLocalizedResource();
 
@@ -18,16 +18,16 @@ public override RichGlyph Glyph
 
 		public override bool IsExecutable =>
 			base.IsExecutable &&
-			context.SelectedItem is not null;
+			ContentPageContext.SelectedItem is not null;
 
 		public override Task ExecuteAsync(object? parameter = null)
 		{
-			if (context.SelectedItem is null)
+			if (!IsExecutable || ContentPageContext.SelectedItem is not ListedItem selectedItem)
 				return Task.CompletedTask;
 
 			try
 			{
-				WindowsWallpaperService.SetDesktopWallpaper(context.SelectedItem.ItemPath);
+				WindowsWallpaperService.SetDesktopWallpaper(selectedItem.ItemPath);
 			}
 			catch (Exception ex)
 			{

src/Files.App/Data/Contracts/IWindowsWallpaperService.cs

@@ -12,18 +12,21 @@ public interface IWindowsWallpaperService
 		/// Sets desktop wallpaper using the specified image path.
 		/// </summary>
 		/// <param name="szPath">The image path to assign as the desktop wallpaper.</param>
+		/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
 		void SetDesktopWallpaper(string szPath);
 
 		/// <summary>
 		/// Sets desktop slideshow using the specified image paths.
 		/// </summary>
 		/// <param name="aszPaths">The image paths to use to set as slideshow.</param>
+		/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
 		void SetDesktopSlideshow(string[] aszPaths);
 
 		/// <summary>
 		/// Gets lock screen wallpaper using the specified image path.
 		/// </summary>
 		/// <param name="szPath">The image path to use to set as lock screen wallpaper.</param>
+		/// <exception cref="System.Runtime.InteropServices.COMException">Thrown if any of COM methods failed to process successfully.</exception>
 		Task SetLockScreenWallpaper(string szPath);
 	}
 }

src/Files.App/Services/Windows/WindowsWallpaperService.cs

@@ -17,68 +17,54 @@ public sealed class WindowsWallpaperService : IWindowsWallpaperService
 		/// <inheritdoc/>
 		public unsafe void SetDesktopWallpaper(string szPath)
 		{
+			// Instantiate IDesktopWallpaper
 			using ComPtr<IDesktopWallpaper> pDesktopWallpaper = default;
-			var desktopWallpaperIid = typeof(IDesktopWallpaper).GUID;
-			var desktopWallpaperInstanceIid = typeof(DesktopWallpaper).GUID;
+			HRESULT hr = pDesktopWallpaper.CoCreateInstance<DesktopWallpaper>().ThrowOnFailure();
 
-			PInvoke.CoCreateInstance(
-				&desktopWallpaperInstanceIid,
-				null,
-				CLSCTX.CLSCTX_LOCAL_SERVER,
-				&desktopWallpaperIid,
-				(void**)pDesktopWallpaper.GetAddressOf())
-			.ThrowOnFailure();
-
-			pDesktopWallpaper.Get()->GetMonitorDevicePathCount(out var dwMonitorCount);
+			// Get total count of all available monitors
+			hr = pDesktopWallpaper.Get()->GetMonitorDevicePathCount(out var dwMonitorCount).ThrowOnFailure();
 
 			fixed (char* pszPath = szPath)
 			{
-				PWSTR pMonitorID = default;
+				PWSTR pMonitorId = default;
 
-				for (uint dwIndex = 0; dwIndex < dwMonitorCount; dwIndex++)
+				// Set the selected image file as wallpaper for all available monitors
+				for (uint dwIndex = 0u; dwIndex < dwMonitorCount; dwIndex++)
 				{
-					pDesktopWallpaper.Get()->GetMonitorDevicePathAt(dwIndex, &pMonitorID);
-					pDesktopWallpaper.Get()->SetWallpaper(pMonitorID, pszPath);
-					pMonitorID = default;
+					// Set the wallpaper
+					hr = pDesktopWallpaper.Get()->GetMonitorDevicePathAt(dwIndex, &pMonitorId).ThrowOnFailure();
+					hr = pDesktopWallpaper.Get()->SetWallpaper(pMonitorId, pszPath).ThrowOnFailure();
+
+					pMonitorId = default;
 				}
 			}
 		}
 
 		/// <inheritdoc/>
 		public unsafe void SetDesktopSlideshow(string[] aszPaths)
 		{
+			// Instantiate IDesktopWallpaper
 			using ComPtr<IDesktopWallpaper> pDesktopWallpaper = default;
-			var desktopWallpaperIid = typeof(IDesktopWallpaper).GUID;
-			var desktopWallpaperInstanceIid = typeof(DesktopWallpaper).GUID;
+			HRESULT hr = pDesktopWallpaper.CoCreateInstance<DesktopWallpaper>().ThrowOnFailure();
 
-			PInvoke.CoCreateInstance(
-				&desktopWallpaperInstanceIid,
-				null,
-				CLSCTX.CLSCTX_LOCAL_SERVER,
-				&desktopWallpaperIid,
-				(void**)pDesktopWallpaper.GetAddressOf())
-			.ThrowOnFailure();
+			uint dwCount = (uint)aszPaths.Length;
+			ITEMIDLIST** ppItemIdList = stackalloc ITEMIDLIST*[aszPaths.Length];
 
-			var dwCount = (uint)aszPaths.Length;
+			// Get an array of PIDL from the selected image files
+			for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
+				ppItemIdList[dwIndex] = PInvoke.ILCreateFromPath(aszPaths[dwIndex]);
 
-			fixed (ITEMIDLIST** idList = new ITEMIDLIST*[dwCount])
-			{
-				for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
-				{
-					var id = PInvoke.ILCreateFromPath(aszPaths[dwIndex]);
-					idList[dwIndex] = id;
-				}
-
-				// Get shell item array from images to use for slideshow
-				using ComPtr<IShellItemArray> pShellItemArray = default;
-				PInvoke.SHCreateShellItemArrayFromIDLists(dwCount, idList, pShellItemArray.GetAddressOf());
+			// Get an IShellItemArray from the array of the PIDL
+			using ComPtr<IShellItemArray> pShellItemArray = default;
+			hr = PInvoke.SHCreateShellItemArrayFromIDLists(dwCount, ppItemIdList, pShellItemArray.GetAddressOf()).ThrowOnFailure();
 
-				// Set slideshow
-				pDesktopWallpaper.Get()->SetSlideshow(pShellItemArray.Get());
-			}
+			// Release the allocated PIDL
+			for (uint dwIndex = 0u; dwIndex < dwCount; dwIndex++)
+				PInvoke.CoTaskMemFree((void*)ppItemIdList[dwIndex]);
 
-			// Set wallpaper to fill desktop.
-			pDesktopWallpaper.Get()->SetPosition(DESKTOP_WALLPAPER_POSITION.DWPOS_FILL);
+			// Set the slideshow and its position
+			hr = pDesktopWallpaper.Get()->SetSlideshow(pShellItemArray.Get()).ThrowOnFailure();
+			hr = pDesktopWallpaper.Get()->SetPosition(DESKTOP_WALLPAPER_POSITION.DWPOS_FILL).ThrowOnFailure();
 		}
 
 		/// <inheritdoc/>