Prevent crash when monitors change

src/Whim.TestUtils/MonitorTestUtils.cs

@@ -10,27 +10,51 @@
 
 internal static class MonitorTestUtils
 {
-	public static void SetupMultipleMonitors(IInternalContext internalCtx, (RECT Rect, HMONITOR HMonitor)[] monitors)
+	public static void SetupMultipleMonitors(
+		IInternalContext internalCtx,
+		(RECT Rect, HMONITOR HMonitor)[] monitors,
+		HMONITOR? primaryMonitor = null
+	)
 	{
-		// Get the primary rect.
-		RECT[] potentialPrimaryMonitors = monitors.Select(m => m.Rect).Where(r => r.left == 0 && r.top == 0).ToArray();
-		if (potentialPrimaryMonitors.Length != 1)
+		// Set up the primary monitor.
+		if (primaryMonitor is HMONITOR primaryMonitorHandle)
 		{
-			throw new Exception("No primary monitor found");
+			internalCtx
+				.CoreNativeManager.MonitorFromPoint(new Point(0, 0), MONITOR_FROM_FLAGS.MONITOR_DEFAULTTOPRIMARY)
+				.Returns(primaryMonitorHandle);
 		}
-		RECT primaryRect = potentialPrimaryMonitors[0];
-
-		// Set up monitor creation.
-		foreach ((RECT rect, HMONITOR hMonitor) in monitors)
+		else
 		{
-			// Set up the primary monitor, if it's the primary monitor.
-			if (primaryRect.Equals(rect))
+			// Get the primary rect.
+			RECT[] potentialPrimaryMonitors = monitors
+				.Select(m => m.Rect)
+				.Where(r => r.left == 0 && r.top == 0)
+				.ToArray();
+
+			if (potentialPrimaryMonitors.Length != 1)
 			{
-				internalCtx
-					.CoreNativeManager.MonitorFromPoint(new Point(0, 0), MONITOR_FROM_FLAGS.MONITOR_DEFAULTTOPRIMARY)
-					.Returns(hMonitor);
+				throw new Exception("No primary monitor found");
 			}
 
+			RECT primaryRect = potentialPrimaryMonitors[0];
+
+			foreach ((RECT rect, HMONITOR hMonitor) in monitors)
+			{
+				if (primaryRect.Equals(rect))
+				{
+					internalCtx
+						.CoreNativeManager.MonitorFromPoint(
+							new Point(0, 0),
+							MONITOR_FROM_FLAGS.MONITOR_DEFAULTTOPRIMARY
+						)
+						.Returns(hMonitor);
+				}
+			}
+		}
+
+		// Set up monitor creation.
+		foreach ((RECT rect, HMONITOR hMonitor) in monitors)
+		{
 			// Set up the fetching of metadata in a multiple monitor setup.
 			internalCtx
 				.CoreNativeManager.GetMonitorInfoEx(hMonitor)

src/Whim.Tests/Store/MonitorSector/Transforms/MonitorsChangedTransformTests.cs

@@ -59,7 +59,13 @@ private static Assert.RaisedEvent<MonitorsChangedEventArgs> DispatchTransformEve
 	private static void Setup_TryEnqueue(IInternalContext internalCtx) =>
 		internalCtx.CoreNativeManager.IsStaThread().Returns(_ => true, _ => false);
 
-	private static IWorkspace[] SetupWorkspaces_AlreadyAdded(IContext ctx, MutableRootSector rootSector)
+	/// <summary>
+	/// Populate the sector with workspaces.
+	/// </summary>
+	/// <param name="ctx"></param>
+	/// <param name="rootSector"></param>
+	/// <returns></returns>
+	private static IWorkspace[] PopulateWorkspaces(IContext ctx, MutableRootSector rootSector)
 	{
 		Workspace workspace1 = CreateWorkspace(ctx);
 		Workspace workspace2 = CreateWorkspace(ctx);
@@ -69,7 +75,13 @@ private static IWorkspace[] SetupWorkspaces_AlreadyAdded(IContext ctx, MutableRo
 		return new[] { workspace1, workspace2, workspace3 };
 	}
 
-	private static IWorkspace[] SetupWorkspaces_AddWorkspaces(IContext ctx, MutableRootSector rootSector)
+	/// <summary>
+	/// Setup the adding of workspaces to the context.
+	/// </summary>
+	/// <param name="ctx"></param>
+	/// <param name="rootSector"></param>
+	/// <returns></returns>
+	private static IWorkspace[] SetupAddWorkspaces(IContext ctx, MutableRootSector rootSector)
 	{
 		Workspace workspace1 = CreateWorkspace(ctx);
 		Workspace workspace2 = CreateWorkspace(ctx);
@@ -96,6 +108,30 @@ private static IWorkspace[] SetupWorkspaces_AddWorkspaces(IContext ctx, MutableR
 		return new[] { workspace1, workspace2, workspace3 };
 	}
 
+	private static void AssertContainsTransform(IContext ctx, Guid workspaceId, int times = 1)
+	{
+		CustomAssert.Contains(
+			ctx.GetTransforms(),
+			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaceId),
+			times
+		);
+	}
+
+	private static void AssertDoesNotContainTransform(IContext ctx, Guid workspaceId)
+	{
+		Assert.DoesNotContain(
+			ctx.GetTransforms(),
+			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaceId)
+		);
+	}
+
+	private static void AssertPrimaryMonitor(MutableRootSector rootSector, HMONITOR handle)
+	{
+		Assert.Equal(handle, rootSector.MonitorSector.PrimaryMonitorHandle);
+		Assert.Equal(handle, rootSector.MonitorSector.ActiveMonitorHandle);
+		Assert.Equal(handle, rootSector.MonitorSector.LastWhimActiveMonitorHandle);
+	}
+
 	[Theory, AutoSubstituteData<StoreCustomization>]
 	internal void WorkspaceSectorNotInitialized(
 		IContext ctx,
@@ -104,7 +140,7 @@ MutableRootSector rootSector
 	)
 	{
 		// Given we've populated monitors
-		SetupWorkspaces_AddWorkspaces(ctx, rootSector);
+		SetupAddWorkspaces(ctx, rootSector);
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup, LeftBottomMonitorSetup });
 
 		rootSector.WorkspaceSector.HasInitialized = false;
@@ -122,7 +158,7 @@ internal void MonitorsRemoved(IContext ctx, IInternalContext internalCtx, Mutabl
 	{
 		// Given we've populated monitors
 		Setup_TryEnqueue(internalCtx);
-		IWorkspace[] workspaces = SetupWorkspaces_AlreadyAdded(ctx, rootSector);
+		IWorkspace[] workspaces = PopulateWorkspaces(ctx, rootSector);
 
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup, LeftBottomMonitorSetup });
 
@@ -150,29 +186,19 @@ internal void MonitorsRemoved(IContext ctx, IInternalContext internalCtx, Mutabl
 		Assert.Equal(workspaces[2].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
 		Assert.DoesNotContain(LeftBottomMonitorSetup.Handle, rootSector.MapSector.MonitorWorkspaceMap.Keys);
 
-		Assert.Contains(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[0].Id)
-		);
+		AssertContainsTransform(ctx, workspaces[0].Id);
+		AssertContainsTransform(ctx, workspaces[1].Id, 2);
+		AssertContainsTransform(ctx, workspaces[2].Id);
 
-		CustomAssert.Contains(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[1].Id),
-			2
-		);
-
-		Assert.Contains(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[2].Id)
-		);
+		AssertPrimaryMonitor(rootSector, LeftTopMonitorSetup.Handle);
 	}
 
 	[Theory, AutoSubstituteData<StoreCustomization>]
 	internal void MonitorsAdded(IContext ctx, IInternalContext internalCtx, MutableRootSector rootSector)
 	{
 		// Given we've populated monitors
 		Setup_TryEnqueue(internalCtx);
-		IWorkspace[] workspaces = SetupWorkspaces_AlreadyAdded(ctx, rootSector);
+		IWorkspace[] workspaces = PopulateWorkspaces(ctx, rootSector);
 
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup });
 		ctx.Store.Dispatch(new MonitorsChangedTransform());
@@ -198,27 +224,62 @@ internal void MonitorsAdded(IContext ctx, IInternalContext internalCtx, MutableR
 		Assert.Equal(workspaces[1].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
 		Assert.Equal(workspaces[2].Id, rootSector.MapSector.MonitorWorkspaceMap[LeftBottomMonitorSetup.Handle]);
 
-		Assert.Contains(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[0].Id)
-		);
+		AssertContainsTransform(ctx, workspaces[0].Id);
+		AssertContainsTransform(ctx, workspaces[1].Id);
+		AssertDoesNotContainTransform(ctx, workspaces[2].Id);
 
-		Assert.Contains(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[1].Id)
+		AssertPrimaryMonitor(rootSector, LeftTopMonitorSetup.Handle);
+	}
+
+	[Theory, AutoSubstituteData<StoreCustomization>]
+	internal void MonitorsAdded_PrimaryChanged(IContext ctx, IInternalContext internalCtx, MutableRootSector rootSector)
+	{
+		// Given we've populated monitors
+		Setup_TryEnqueue(internalCtx);
+		IWorkspace[] workspaces = PopulateWorkspaces(ctx, rootSector);
+
+		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup });
+		ctx.Store.Dispatch(new MonitorsChangedTransform());
+
+		// When a monitor is added, and the new monitor changes to become the primary monitor
+		Setup_TryEnqueue(internalCtx);
+		SetupMultipleMonitors(
+			internalCtx,
+			new[] { RightMonitorSetup, LeftTopMonitorSetup, LeftBottomMonitorSetup },
+			LeftBottomMonitorSetup.Handle
 		);
 
-		Assert.DoesNotContain(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[2].Id)
+		// Then the resulting event will have:
+		// - two monitors added (the new one and the previous primary as a non-primary)
+		// - one monitor removed (the previous primary as a non-primary)
+		var raisedEvent = DispatchTransformEvent(
+			ctx,
+			rootSector,
+			new[] { workspaces[0].Id, workspaces[1].Id, workspaces[2].Id }
 		);
+
+		Assert.Equal(2, raisedEvent.Arguments.AddedMonitors.Count());
+		Assert.Single(raisedEvent.Arguments.RemovedMonitors);
+		Assert.Single(raisedEvent.Arguments.UnchangedMonitors);
+
+		Assert.Equal(3, rootSector.MapSector.MonitorWorkspaceMap.Count);
+
+		Assert.Equal(workspaces[0].Id, rootSector.MapSector.MonitorWorkspaceMap[LeftTopMonitorSetup.Handle]);
+		Assert.Equal(workspaces[1].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
+		Assert.Equal(workspaces[2].Id, rootSector.MapSector.MonitorWorkspaceMap[LeftBottomMonitorSetup.Handle]);
+
+		AssertContainsTransform(ctx, workspaces[0].Id, 2);
+		AssertContainsTransform(ctx, workspaces[1].Id);
+		AssertDoesNotContainTransform(ctx, workspaces[2].Id);
+
+		AssertPrimaryMonitor(rootSector, LeftBottomMonitorSetup.Handle);
 	}
 
 	[Theory, AutoSubstituteData<StoreCustomization>]
 	internal void MonitorsAdded_Initialization(IContext ctx, IInternalContext internalCtx, MutableRootSector rootSector)
 	{
 		// Given we have no monitors
-		IWorkspace[] workspaces = SetupWorkspaces_AlreadyAdded(ctx, rootSector);
+		IWorkspace[] workspaces = PopulateWorkspaces(ctx, rootSector);
 
 		// When we add monitors
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup });
@@ -234,6 +295,8 @@ internal void MonitorsAdded_Initialization(IContext ctx, IInternalContext intern
 		Assert.Equal(workspaces[1].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
 
 		Assert.DoesNotContain(ctx.GetTransforms(), t => t is DeactivateWorkspaceTransform);
+
+		AssertPrimaryMonitor(rootSector, LeftTopMonitorSetup.Handle);
 	}
 
 	[Theory, AutoSubstituteData<StoreCustomization>]
@@ -244,7 +307,7 @@ MutableRootSector rootSector
 	)
 	{
 		// Given we have no monitors
-		IWorkspace[] workspaces = SetupWorkspaces_AddWorkspaces(ctx, rootSector);
+		IWorkspace[] workspaces = SetupAddWorkspaces(ctx, rootSector);
 
 		// When we add monitors
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup });
@@ -260,14 +323,16 @@ MutableRootSector rootSector
 		Assert.Equal(workspaces[1].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
 
 		Assert.DoesNotContain(ctx.GetTransforms(), t => t is DeactivateWorkspaceTransform);
+
+		AssertPrimaryMonitor(rootSector, LeftTopMonitorSetup.Handle);
 	}
 
 	[Theory, AutoSubstituteData<StoreCustomization>]
 	internal void MonitorsUnchanged(IContext ctx, IInternalContext internalCtx, MutableRootSector rootSector)
 	{
 		// Given there are no changes in the monitors.
 		Setup_TryEnqueue(internalCtx);
-		IWorkspace[] workspaces = SetupWorkspaces_AlreadyAdded(ctx, rootSector);
+		IWorkspace[] workspaces = PopulateWorkspaces(ctx, rootSector);
 
 		SetupMultipleMonitors(internalCtx, new[] { RightMonitorSetup, LeftTopMonitorSetup, LeftBottomMonitorSetup });
 
@@ -292,19 +357,10 @@ internal void MonitorsUnchanged(IContext ctx, IInternalContext internalCtx, Muta
 		Assert.Equal(workspaces[2].Id, rootSector.MapSector.MonitorWorkspaceMap[RightMonitorSetup.Handle]);
 		Assert.Equal(workspaces[1].Id, rootSector.MapSector.MonitorWorkspaceMap[LeftBottomMonitorSetup.Handle]);
 
-		Assert.DoesNotContain(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[0].Id)
-		);
-
-		Assert.DoesNotContain(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[1].Id)
-		);
+		AssertDoesNotContainTransform(ctx, workspaces[0].Id);
+		AssertDoesNotContainTransform(ctx, workspaces[1].Id);
+		AssertDoesNotContainTransform(ctx, workspaces[2].Id);
 
-		Assert.DoesNotContain(
-			ctx.GetTransforms(),
-			t => (t as DeactivateWorkspaceTransform) == new DeactivateWorkspaceTransform(workspaces[2].Id)
-		);
+		AssertPrimaryMonitor(rootSector, LeftTopMonitorSetup.Handle);
 	}
 }

src/Whim.Tests/Store/WorkspaceSector/Transforms/LayoutEngineCustomActionTransformTests.cs

@@ -1,6 +1,6 @@
 using System.Diagnostics.CodeAnalysis;
 
-namespace Whim.TestUtils;
+namespace Whim.Tests;
 
 [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope")]
 public class LayoutEngineCustomActionTransformTests

src/Whim/Store/MonitorSector/Transforms/MonitorsChangedTransform.cs

@@ -18,8 +18,7 @@ MutableRootSector mutableRootSector
 
 		// Get the new monitors.
 		ImmutableArray<IMonitor> previousMonitors = sector.Monitors;
-
-		sector.Monitors = MonitorUtils.GetCurrentMonitors(internalCtx);
+		UpdateMonitorSector(ctx, internalCtx, mutableRootSector);
 
 		List<IMonitor> unchangedMonitors = new();
 		List<IMonitor> removedMonitors = new();
@@ -78,6 +77,24 @@ MutableRootSector mutableRootSector
 		return Unit.Result;
 	}
 
+	private static void UpdateMonitorSector(IContext ctx, IInternalContext internalCtx, MutableRootSector rootSector)
+	{
+		MonitorSector sector = rootSector.MonitorSector;
+
+		sector.Monitors = MonitorUtils.GetCurrentMonitors(internalCtx);
+
+		foreach (IMonitor m in sector.Monitors)
+		{
+			if (m.IsPrimary)
+			{
+				sector.PrimaryMonitorHandle = m.Handle;
+				sector.ActiveMonitorHandle = m.Handle;
+				sector.LastWhimActiveMonitorHandle = m.Handle;
+				break;
+			}
+		}
+	}
+
 	private static void UpdateMapSector(
 		IContext ctx,
 		MutableRootSector rootSector,