Skip to content

Commit

Permalink
Prevent crash when monitors change (#921)
Browse files Browse the repository at this point in the history 
Replace the stored `PrimaryMonitorHandle`, `ActiveMonitorHandle`, and LastWhimActiveMonitorHandle` when the monitors are changed. `HMONITOR`s can change as monitors change.
  • Loading branch information
@dalyIsaac
dalyIsaac authored Jun 28, 2024
1 parent 49a78b0 commit 2c68047
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 62 deletions.
52 changes: 38 additions & 14 deletions src/Whim.TestUtils/MonitorTestUtils.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,27 +10,51 @@ namespace Whim.TestUtils;

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)
Expand Down
146 changes: 101 additions & 45 deletions src/Whim.Tests/Store/MonitorSector/Transforms/MonitorsChangedTransformTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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 });

Expand Down Expand Up @@ -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());
Expand All @@ -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 });
Expand All @@ -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>]
Expand All @@ -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 });
Expand All @@ -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 });

Expand All @@ -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);
}
}
2 changes: 1 addition & 1 deletion src/Whim.Tests/Store/WorkspaceSector/Transforms/LayoutEngineCustomActionTransformTests.cs
View file
Original file line number Diff line number Diff line change
@@ -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
Expand Down
21 changes: 19 additions & 2 deletions src/Whim/Store/MonitorSector/Transforms/MonitorsChangedTransform.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2c68047

Please sign in to comment.