Catch GetProcessById ArgumentException (#559)

Now catches `ArgumentException` raised by `Process.GetProcessById`. As a result, `GetProcessNameAndPath` is optionally `null`. Also added `CustomAssert.DoesNotPropertyChange`.

.vscode/tasks.json

@@ -5,7 +5,7 @@
       "label": "build",
       "command": "dotnet",
       "type": "process",
-      "args": ["build", "Whim.sln"],
+      "args": ["build", "Whim.sln", "-p:Platform=x64"],
       "problemMatcher": "$msCompile"
     },
     {

src/Whim.Bar.Tests/FocusedWindow/FocusedWindowWidget.test.xaml.cs

@@ -52,7 +52,7 @@ public void GetProcessName(IWindow window)
 		window.ProcessName.Returns("code");
 
 		// When
-		string processName = FocusedWindowWidget.GetProcessName(window);
+		string? processName = FocusedWindowWidget.GetProcessName(window);
 
 		// Then
 		Assert.Equal("code", processName);

src/Whim.Bar/FocusedWindow/FocusedWindowWidget.xaml.cs

@@ -57,5 +57,5 @@ public static string GetShortTitle(IWindow window)
 	/// </summary>
 	/// <param name="window"></param>
 	/// <returns></returns>
-	public static string GetProcessName(IWindow window) => window.ProcessName;
+	public static string? GetProcessName(IWindow window) => window.ProcessName;
 }

src/Whim.LayoutPreview.Tests/LayoutPreviewPluginTests.cs

@@ -1,5 +1,6 @@
 using AutoFixture;
 using NSubstitute;
+using System.Diagnostics.CodeAnalysis;
 using System.Text.Json;
 using Whim.TestUtils;
 using Xunit;
@@ -59,6 +60,7 @@ public void PluginCommands(IContext ctx)
 	}
 
 	[Theory, AutoSubstituteData<LayoutPreviewPluginCustomization>]
+	[SuppressMessage("Usage", "NS5000:Received check.")]
 	public void PreInitialize(IContext ctx)
 	{
 		// Given

src/Whim.TestUtils/Assert.cs

@@ -1,4 +1,5 @@
 using System;
+using System.ComponentModel;
 using Xunit.Sdk;
 
 namespace Whim.TestUtils;
@@ -53,4 +54,38 @@ void handler(object? sender, T e)
 			throw new DoesNotRaiseException(typeof(T));
 		}
 	}
+
+	/// <summary>
+	/// Asserts that <see cref="INotifyPropertyChanged.PropertyChanged"/> is not raised.
+	/// </summary>
+	/// <param name="attach">The method to attach the event handler.</param>
+	/// <param name="detach">The method to detach the event handler.</param>
+	/// <param name="action">The action to perform.</param>
+	/// <exception cref="DoesNotRaiseException">Thrown when the event is raised.</exception>
+	public static void DoesNotPropertyChange(
+		Action<PropertyChangedEventHandler> attach,
+		Action<PropertyChangedEventHandler> detach,
+		Action action
+	)
+	{
+		bool raised = false;
+		void handler(object? sender, PropertyChangedEventArgs e)
+		{
+			raised = true;
+		}
+		attach(handler);
+		try
+		{
+			action();
+		}
+		finally
+		{
+			detach(handler);
+		}
+
+		if (raised)
+		{
+			throw new DoesNotRaiseException(typeof(PropertyChangedEventArgs));
+		}
+	}
 }

src/Whim.Tests/Router/FilterManagerTests.cs

@@ -9,84 +9,95 @@ public class FilterManagerTests
 	[Theory, AutoSubstituteData]
 	public void IgnoreWindowClass(IWindow window)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreWindowClass("Test");
 
 		window.WindowClass.Returns("Test");
 
+		// Then
 		Assert.True(filterManager.ShouldBeIgnored(window));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void IgnoreProcessName(IWindow window)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreProcessName("Test");
 
 		window.ProcessName.Returns("Test");
 
+		// Then
 		Assert.True(filterManager.ShouldBeIgnored(window));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void IgnoreTitle(IWindow window)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreTitle("Test");
 
 		window.Title.Returns("Test");
 
+		// Then
 		Assert.True(filterManager.ShouldBeIgnored(window));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void IgnoreTitleMatch(IWindow window)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreTitleMatch("Test");
 
 		window.Title.Returns("Test");
 
+		// Then
 		Assert.True(filterManager.ShouldBeIgnored(window));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void ClearKeepDefaults(IWindow window, IWindow searchUIWindow)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreWindowClass("Test");
 
 		window.WindowClass.Returns("Test");
+		searchUIWindow.ProcessName.Returns("SearchUI.exe");
 
+		// When the filter manager is cleared, and the defaults are cleared
 		filterManager.Clear(clearDefaults: true);
 
+		// Then neither window should be ignored
 		Assert.False(filterManager.ShouldBeIgnored(window));
-
-		searchUIWindow.ProcessName.Returns("SearchUI.exe");
-
 		Assert.False(filterManager.ShouldBeIgnored(searchUIWindow));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void ClearAll(IWindow window, IWindow searchUIWindow)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.IgnoreWindowClass("Test");
 
 		window.WindowClass.Returns("Test");
+		searchUIWindow.ProcessName.Returns("SearchUI.exe");
 
+		// When the filter manager is cleared, and the defaults are kept
 		filterManager.Clear(clearDefaults: false);
 
+		// Then the window should be ignored, but not the search UI window
 		Assert.False(filterManager.ShouldBeIgnored(window));
-
-		searchUIWindow.ProcessName.Returns("SearchUI.exe");
-
 		Assert.True(filterManager.ShouldBeIgnored(searchUIWindow));
 	}
 
 	[Theory, AutoSubstituteData]
 	public void CustomFilter(IWindow window)
 	{
+		// Given
 		FilterManager filterManager = new();
 		filterManager.Add(w => w.WindowClass == "Test");
 

src/Whim.Tests/Window/WindowManagerTests.cs

@@ -88,7 +88,7 @@ public static CaptureWinEventProc Create(
 )]
 public class WindowManagerTests
 {
-	private static readonly uint _processId = 1;
+	private const uint _processId = 1;
 
 	private static void AllowWindowCreation(IContext ctx, IInternalContext internalCtx, HWND hwnd)
 	{

src/Whim.Tests/Window/WindowTests.cs

@@ -143,7 +143,7 @@ internal void ProcessFileName(IContext ctx, IInternalContext internalCtx)
 		IWindow window = Window.CreateWindow(ctx, internalCtx, new HWND(123))!;
 
 		// When
-		string processFileName = window.ProcessFileName;
+		string? processFileName = window.ProcessFileName;
 
 		// Then
 		Assert.Equal("processFileName", processFileName);
@@ -161,10 +161,10 @@ internal void ProcessFileName_NA(IContext ctx, IInternalContext internalCtx)
 		IWindow window = Window.CreateWindow(ctx, internalCtx, new HWND(123))!;
 
 		// When
-		string processFileName = window.ProcessFileName;
+		string? processFileName = window.ProcessFileName;
 
 		// Then
-		Assert.Equal("--NA--", processFileName);
+		Assert.Null(processFileName);
 	}
 
 	[Theory, AutoSubstituteData<WindowCustomization>]
@@ -174,7 +174,7 @@ internal void ProcessName(IContext ctx, IInternalContext internalCtx)
 		IWindow window = Window.CreateWindow(ctx, internalCtx, new HWND(123))!;
 
 		// When
-		string processName = window.ProcessName;
+		string? processName = window.ProcessName;
 
 		// Then
 		Assert.Equal("processName", processName);

src/Whim.TreeLayout.Bar.Tests/TreeLayoutEngineWidgetViewModelTests.cs

@@ -207,18 +207,22 @@ IMonitor monitor
 		// Given
 		TreeLayoutEngineWidgetViewModel viewModel = new(ctx, plugin, monitor);
 
-		// When
-		ctx.WorkspaceManager.MonitorWorkspaceChanged += Raise.Event<EventHandler<MonitorWorkspaceChangedEventArgs>>(
-			new MonitorWorkspaceChangedEventArgs()
-			{
-				Monitor = Substitute.For<IMonitor>(),
-				CurrentWorkspace = Substitute.For<IWorkspace>(),
-				PreviousWorkspace = Substitute.For<IWorkspace>()
-			}
+		// Then should not have PropertyChanged event raised
+		CustomAssert.DoesNotPropertyChange(
+			h => viewModel.PropertyChanged += h,
+			h => viewModel.PropertyChanged -= h,
+			() =>
+				ctx.WorkspaceManager.MonitorWorkspaceChanged += Raise.Event<
+					EventHandler<MonitorWorkspaceChangedEventArgs>
+				>(
+					new MonitorWorkspaceChangedEventArgs()
+					{
+						Monitor = Substitute.For<IMonitor>(),
+						CurrentWorkspace = Substitute.For<IWorkspace>(),
+						PreviousWorkspace = Substitute.For<IWorkspace>()
+					}
+				)
 		);
-
-		// Then should not have called anything
-		ctx.WorkspaceManager.DidNotReceive().GetWorkspaceForMonitor(Arg.Any<IMonitor>());
 	}
 
 	[Theory, AutoSubstituteData<TreeLayoutEngineWidgetViewModelCustomization>]

src/Whim/Native/CoreNativeManager.cs

@@ -308,10 +308,18 @@ public HWND WindowMessageMonitorWindowHandle
 		}
 	}
 
-	public (string processName, string? processPath) GetProcessNameAndPath(int processId)
+	public (string ProcessName, string? ProcessPath)? GetProcessNameAndPath(int processId)
 	{
-		using Process process = Process.GetProcessById(processId);
-		return (process.ProcessName, process.MainModule?.FileName);
+		try
+		{
+			using Process process = Process.GetProcessById(processId);
+			return (process.ProcessName, process.MainModule?.FileName);
+		}
+		catch (ArgumentException ex)
+		{
+			Logger.Error($"Could not get process with id {processId}: {ex.Message}");
+			return null;
+		}
 	}
 
 	public nuint GetClassLongPtr(HWND hWnd, GET_CLASS_LONG_INDEX nIndex) => PInvoke.GetClassLongPtr(hWnd, nIndex);

src/Whim/Native/ICoreNativeManager.cs

@@ -487,7 +487,7 @@ uint dwThreadId
 	/// </summary>
 	/// <param name="processId">The process ID.</param>
 	/// <returns>The process name and process path.</returns>
-	(string processName, string? processPath) GetProcessNameAndPath(int processId);
+	(string ProcessName, string? ProcessPath)? GetProcessNameAndPath(int processId);
 
 	/// <summary>Retrieves the specified value from the WNDCLASSEX structure associated with the specified window.</summary>
 	/// <param name="hWnd">

src/Whim/Native/NativeManager.cs

@@ -1,5 +1,4 @@
 using System.Collections.Generic;
-using System.Diagnostics;
 using System.Runtime.InteropServices;
 using Windows.System;
 using Windows.UI.Composition;
@@ -218,9 +217,7 @@ public DeferWindowPosHandle DeferWindowPos(IEnumerable<WindowPosState> windowSta
 
 			if (childPid != pid)
 			{
-				// here we are
-				Process childProc = Process.GetProcessById((int)childPid);
-				return childProc.MainModule?.FileName;
+				return _internalContext.CoreNativeManager.GetProcessNameAndPath((int)childPid)?.ProcessPath;
 			}
 		}
 

src/Whim/Router/FilterManager.cs

@@ -43,7 +43,7 @@ public void Clear(bool clearDefaults = false)
 	public bool ShouldBeIgnored(IWindow window)
 	{
 		return _ignoreWindowClasses.Contains(window.WindowClass.ToLower())
-			|| _ignoreProcessNames.Contains(window.ProcessName.ToLower())
+			|| (window.ProcessName is string processName && _ignoreProcessNames.Contains(processName.ToLower()))
 			|| _ignoreTitles.Contains(window.Title.ToLower())
 			|| _filters.Any(f => f(window));
 	}

src/Whim/Router/RouterManager.cs

@@ -33,7 +33,7 @@ public IRouterManager AddProcessNameRoute(string processName, string workspaceNa
 		Logger.Debug($"Routing process name: {processName} to workspace {workspaceName}");
 		Add(window =>
 		{
-			if (window.ProcessName.ToLower() == processName)
+			if (window.ProcessName?.ToLower() == processName)
 			{
 				return _context.WorkspaceManager.TryGet(workspaceName);
 			}
@@ -48,7 +48,7 @@ public IRouterManager AddProcessNameRoute(string processName, IWorkspace workspa
 		Logger.Debug($"Routing process name: {processName} to workspace {workspace}");
 		Add(window =>
 		{
-			if (window.ProcessName.ToLower() == processName)
+			if (window.ProcessName?.ToLower() == processName)
 			{
 				return workspace;
 			}

src/Whim/Window/IWindow.cs

@@ -46,7 +46,7 @@ public interface IWindow
 	/// <summary>
 	/// The file name of the module.
 	/// </summary>
-	string ProcessFileName { get; }
+	string? ProcessFileName { get; }
 
 	/// <summary>
 	/// The fully qualified path that defines the location of the module.
@@ -56,7 +56,7 @@ public interface IWindow
 	/// <summary>
 	/// The name that the system uses to identify the process to the user.
 	/// </summary>
-	string ProcessName { get; }
+	string? ProcessName { get; }
 
 	/// <summary>
 	/// Indicates whether the window is focused.