From 8cb2b9fbd3c6124e606f3368350823d954fc5360 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 14:55:52 +1100 Subject: [PATCH 01/31] Migrate KeybindHook to Task --- src/Whim/Keybind/KeybindHook.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Whim/Keybind/KeybindHook.cs b/src/Whim/Keybind/KeybindHook.cs index e27daa159..c4aad9fd8 100644 --- a/src/Whim/Keybind/KeybindHook.cs +++ b/src/Whim/Keybind/KeybindHook.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Windows.Win32; using Windows.Win32.Foundation; using Windows.Win32.UI.Input.KeyboardAndMouse; @@ -39,7 +40,7 @@ private LRESULT LowLevelKeyboardProcWrapper(int nCode, WPARAM wParam, LPARAM lPa { try { - return LowLevelKeyboardProc(nCode, wParam, lParam); + return Task.Run(() => LowLevelKeyboardProc(nCode, wParam, lParam)).Result; } catch (Exception e) { From 2e666a93c723f43922e82d3184e8b75aaf9cada2 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 16:42:26 +1100 Subject: [PATCH 02/31] Migrate entry points for core --- src/Whim/Native/MouseHook.cs | 3 ++- src/Whim/Window/WindowManager.cs | 5 ++++- src/Whim/WindowMessage/WindowMessageMonitor.cs | 3 ++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Whim/Native/MouseHook.cs b/src/Whim/Native/MouseHook.cs index 9a9a5c7f3..acb781335 100644 --- a/src/Whim/Native/MouseHook.cs +++ b/src/Whim/Native/MouseHook.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Windows.Win32; using Windows.Win32.Foundation; using Windows.Win32.UI.WindowsAndMessaging; @@ -40,7 +41,7 @@ private LRESULT LowLevelMouseProcWrapper(int nCode, WPARAM wParam, LPARAM lParam { try { - return LowLevelMouseProc(nCode, wParam, lParam); + return Task.Run(() => LowLevelMouseProc(nCode, wParam, lParam)).Result; } catch (Exception e) { diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index 9c663fce1..5045974bb 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -215,7 +215,10 @@ uint dwmsEventTime { try { - WinEventProc(hWinEventHook, eventType, hwnd, idObject, idChild, idEventThread, dwmsEventTime); + Task.Run( + () => WinEventProc(hWinEventHook, eventType, hwnd, idObject, idChild, idEventThread, dwmsEventTime) + ) + .Wait(); } catch (Exception e) { diff --git a/src/Whim/WindowMessage/WindowMessageMonitor.cs b/src/Whim/WindowMessage/WindowMessageMonitor.cs index c723ef795..0118b29cd 100644 --- a/src/Whim/WindowMessage/WindowMessageMonitor.cs +++ b/src/Whim/WindowMessage/WindowMessageMonitor.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Windows.Win32; using Windows.Win32.Foundation; using Windows.Win32.UI.Shell; @@ -57,7 +58,7 @@ nuint dwRefData { try { - return WindowProc(hWnd, uMsg, wParam, lParam, uIdSubclass, dwRefData); + return Task.Run(() => WindowProc(hWnd, uMsg, wParam, lParam, uIdSubclass, dwRefData)).Result; } catch (Exception e) { From f5b98451e162d19ff4169f2a3c40fdc8f22987f1 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 17:39:48 +1100 Subject: [PATCH 03/31] Remove DeferWindowPosManager and lock setting window position --- src/Whim/Context/IInternalContext.cs | 2 - src/Whim/Context/InternalContext.cs | 3 - src/Whim/Native/DeferWindowPosHandle.cs | 17 +-- src/Whim/Native/DeferWindowPosManager.cs | 105 ------------------ ...rWindowPosManager.cs => WindowPosState.cs} | 32 ------ src/Whim/Window/WindowManager.cs | 4 - 6 files changed, 10 insertions(+), 153 deletions(-) delete mode 100644 src/Whim/Native/DeferWindowPosManager.cs rename src/Whim/Native/{IDeferWindowPosManager.cs => WindowPosState.cs} (55%) diff --git a/src/Whim/Context/IInternalContext.cs b/src/Whim/Context/IInternalContext.cs index cfd85a34d..278ca44c2 100644 --- a/src/Whim/Context/IInternalContext.cs +++ b/src/Whim/Context/IInternalContext.cs @@ -25,8 +25,6 @@ internal interface IInternalContext : IDisposable IMouseHook MouseHook { get; } - IDeferWindowPosManager DeferWindowPosManager { get; } - IDeferWorkspacePosManager DeferWorkspacePosManager { get; } void PreInitialize(); diff --git a/src/Whim/Context/InternalContext.cs b/src/Whim/Context/InternalContext.cs index 6120c046e..345e0e312 100644 --- a/src/Whim/Context/InternalContext.cs +++ b/src/Whim/Context/InternalContext.cs @@ -22,8 +22,6 @@ internal class InternalContext : IInternalContext public IMouseHook MouseHook { get; } - public IDeferWindowPosManager DeferWindowPosManager { get; } - public IDeferWorkspacePosManager DeferWorkspacePosManager { get; } public IButlerEventHandlers ButlerEventHandlers => ((Butler)_context.Butler).EventHandlers; @@ -36,7 +34,6 @@ public InternalContext(IContext context) WindowMessageMonitor = new WindowMessageMonitor(context, this); KeybindHook = new KeybindHook(context, this); MouseHook = new MouseHook(context, this); - DeferWindowPosManager = new DeferWindowPosManager(context, this); DeferWorkspacePosManager = new DeferWorkspacePosManager(context, this); } diff --git a/src/Whim/Native/DeferWindowPosHandle.cs b/src/Whim/Native/DeferWindowPosHandle.cs index 402963102..6e7234816 100644 --- a/src/Whim/Native/DeferWindowPosHandle.cs +++ b/src/Whim/Native/DeferWindowPosHandle.cs @@ -17,6 +17,7 @@ namespace Whim; /// public sealed class DeferWindowPosHandle : IDisposable { + private static readonly object _lock = new(); private readonly IContext _context; private readonly IInternalContext _internalContext; @@ -116,6 +117,14 @@ private void AddToCorrectList(WindowPosState windowPosState) /// public void Dispose() + { + lock (_lock) + { + SetAllWindowPos(); + } + } + + private void SetAllWindowPos() { Logger.Debug("Disposing WindowDeferPosHandle"); @@ -125,12 +134,6 @@ public void Dispose() return; } - if (!_internalContext.DeferWindowPosManager.CanDoLayout()) - { - _internalContext.DeferWindowPosManager.DeferLayout(_windowStates, _minimizedWindowStates); - return; - } - // Check to see if any monitors have non-100% scaling. // If so, we need to set the window position twice. int numPasses = 1; @@ -175,7 +178,7 @@ public void Dispose() { for (int i = 0; i < numPasses; i++) { - Parallel.ForEach(allStates, _internalContext.DeferWindowPosManager.ParallelOptions, SetWindowPos); + Parallel.ForEach(allStates, SetWindowPos); } } diff --git a/src/Whim/Native/DeferWindowPosManager.cs b/src/Whim/Native/DeferWindowPosManager.cs deleted file mode 100644 index 377509432..000000000 --- a/src/Whim/Native/DeferWindowPosManager.cs +++ /dev/null @@ -1,105 +0,0 @@ -using System.Collections.Generic; -using System.Diagnostics; -using System.Threading.Tasks; - -namespace Whim; - -internal class DeferWindowPosManager : IDeferWindowPosManager -{ - private readonly IContext _context; - private readonly IInternalContext _internalContext; - - private readonly Dictionary _deferredWindowStates = new(); - - public ParallelOptions ParallelOptions { get; } = new(); - - public DeferWindowPosManager(IContext context, IInternalContext internalContext) - { - _context = context; - _internalContext = internalContext; - } - - public void DeferLayout(List windowStates, List minimizedWindowStates) - { - Logger.Debug("Deferring layout"); - foreach (WindowPosState windowState in windowStates) - { - _deferredWindowStates[windowState.WindowState.Window] = windowState; - } - foreach (WindowPosState windowState in minimizedWindowStates) - { - _deferredWindowStates[windowState.WindowState.Window] = windowState; - } - } - - public bool CanDoLayout() - { - StackTrace stackTrace = new(); - - // Iterate over each of the stack trace to see if reentrancy has occurred. - // If so, we cannot do layout. - int entryCount = 0; - for (int i = 0; i < stackTrace.FrameCount; i++) - { - StackFrame? frame = stackTrace.GetFrame(i); - if (frame?.GetMethod() == typeof(WindowManager).GetMethod(nameof(WindowManager.WinEventProcWrapper))) - { - entryCount++; - - if (entryCount > 1) - { - Logger.Debug("Cannot do layout due to reentrancy"); - return false; - } - } - } - - return true; - } - - public void RecoverLayout() - { - Logger.Debug("Attempting to recover layout"); - - if (!CanDoLayout()) - { - Logger.Debug("Cannot recover layout"); - return; - } - else if (_deferredWindowStates.Count == 0) - { - Logger.Debug("No windows to recover layout for"); - return; - } - - List deferredWorkspaces = new(); - List deferredWindowStates = new(); - foreach (WindowPosState windowState in _deferredWindowStates.Values) - { - IWorkspace? workspace = _context.Butler.Pantry.GetWorkspaceForWindow(windowState.WindowState.Window); - if (workspace != null && !deferredWorkspaces.Contains(workspace)) - { - deferredWorkspaces.Add(workspace); - } - else if (_context.FilterManager.ShouldBeIgnored(windowState.WindowState.Window)) - { - // We don't store the window in the window manager, but we do still set its position. - // We only set the positions of windows which aren't tracked by Whim - the other windows - // will be set out by their respective workspaces. - deferredWindowStates.Add(windowState); - } - } - - foreach (IWorkspace workspace in deferredWorkspaces) - { - workspace.DoLayout(); - } - - if (deferredWindowStates.Count != 0) - { - using DeferWindowPosHandle windowDeferPosHandle = new(_context, _internalContext, deferredWindowStates); - } - - _deferredWindowStates.Clear(); - } -} diff --git a/src/Whim/Native/IDeferWindowPosManager.cs b/src/Whim/Native/WindowPosState.cs similarity index 55% rename from src/Whim/Native/IDeferWindowPosManager.cs rename to src/Whim/Native/WindowPosState.cs index 38652f2c7..4f8400b05 100644 --- a/src/Whim/Native/IDeferWindowPosManager.cs +++ b/src/Whim/Native/WindowPosState.cs @@ -1,5 +1,3 @@ -using System.Collections.Generic; -using System.Threading.Tasks; using Windows.Win32.Foundation; using Windows.Win32.UI.WindowsAndMessaging; @@ -41,33 +39,3 @@ public WindowPosState(IWindowState windowState, HWND? hwndInsertAfter = null, SE Flags = flags; } } - -/// -/// Manager to defer the setting of window positions. -/// -internal interface IDeferWindowPosManager -{ - /// - /// Defers layout of the given windows until is called. - /// - /// - /// - void DeferLayout(List windowStates, List minimizedWindowStates); - - /// - /// Returns whether or not layout can be performed, based on whether Whim has been reentered. - /// - /// - bool CanDoLayout(); - - /// - /// Performs layout of all the windows that were deferred, if they have not been laid out in - /// after the last call to . - /// - void RecoverLayout(); - - /// - /// The to use when performing layout. - /// - ParallelOptions ParallelOptions { get; } -} diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index 5045974bb..9114379e6 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -271,8 +271,6 @@ uint _dwmsEventTime $"Window {hwnd} with event 0x{eventType:X4} was ignored, but still notifying listeners of focus" ); OnWindowFocused(window); - - _internalContext.DeferWindowPosManager.RecoverLayout(); return; } @@ -324,8 +322,6 @@ uint _dwmsEventTime Logger.Error($"Unhandled event 0x{eventType:X4}"); break; } - - _internalContext.DeferWindowPosManager.RecoverLayout(); } public IWindow? AddWindow(HWND hwnd) From a1b541003d5926773a3306828b1e7ad67a512575 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 18:01:43 +1100 Subject: [PATCH 04/31] Remove DeferWindowPosManager tests, and fix existing tests --- .../LayoutPreviewWindowTests.cs | 2 - src/Whim.TestUtils/Assert.cs | 1 - .../Native/DeferWindowPosHandleTests.cs | 31 +--- .../Native/DeferWindowPosManagerTests.cs | 144 ------------------ src/Whim/Context/IInternalContext.cs | 3 + src/Whim/Context/InternalContext.cs | 4 + 6 files changed, 8 insertions(+), 177 deletions(-) delete mode 100644 src/Whim.Tests/Native/DeferWindowPosManagerTests.cs diff --git a/src/Whim.LayoutPreview.Tests/LayoutPreviewWindowTests.cs b/src/Whim.LayoutPreview.Tests/LayoutPreviewWindowTests.cs index 6542349db..e5d8527fe 100644 --- a/src/Whim.LayoutPreview.Tests/LayoutPreviewWindowTests.cs +++ b/src/Whim.LayoutPreview.Tests/LayoutPreviewWindowTests.cs @@ -181,8 +181,6 @@ IMonitor monitor { // Given ctx.NativeManager.DeferWindowPos().Returns(new DeferWindowPosHandle(ctx, internalCtx)); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); - internalCtx.DeferWindowPosManager.ParallelOptions.Returns(new ParallelOptions { MaxDegreeOfParallelism = 1 }); // When LayoutPreviewWindow.Activate(ctx, layoutWindow, movingWindow, monitor); diff --git a/src/Whim.TestUtils/Assert.cs b/src/Whim.TestUtils/Assert.cs index 05d0fa6ad..ec075a280 100644 --- a/src/Whim.TestUtils/Assert.cs +++ b/src/Whim.TestUtils/Assert.cs @@ -116,6 +116,5 @@ internal static void NoInternalContextCalls(IInternalContext internalCtx) Assert.Empty(internalCtx.WindowManager.ReceivedCalls()); Assert.Empty(internalCtx.KeybindHook.ReceivedCalls()); Assert.Empty(internalCtx.MouseHook.ReceivedCalls()); - Assert.Empty(internalCtx.DeferWindowPosManager.ReceivedCalls()); } } diff --git a/src/Whim.Tests/Native/DeferWindowPosHandleTests.cs b/src/Whim.Tests/Native/DeferWindowPosHandleTests.cs index 988868a18..3d1bbc3fd 100644 --- a/src/Whim.Tests/Native/DeferWindowPosHandleTests.cs +++ b/src/Whim.Tests/Native/DeferWindowPosHandleTests.cs @@ -37,7 +37,7 @@ public void Customize(IFixture fixture) }); IInternalContext internalCtx = fixture.Freeze(); - internalCtx.DeferWindowPosManager.ParallelOptions.Returns(new ParallelOptions { MaxDegreeOfParallelism = 1 }); + internalCtx.ParallelOptions.Returns(new ParallelOptions { MaxDegreeOfParallelism = 1 }); } } @@ -84,30 +84,6 @@ internal void Dispose_NoWindows(IContext ctx, IInternalContext internalCtx) CustomAssert.NoInternalContextCalls(internalCtx); } - [Theory, AutoSubstituteData] - internal void Dispose_CannotDoLayout( - IContext ctx, - IInternalContext internalCtx, - WindowPosState windowPosState, - WindowPosState windowPosState2 - ) - { - // Given DeferWindowPosManager.CanDoLayout() returns false - windowPosState2.WindowState.WindowSize = WindowSize.Minimized; - using DeferWindowPosHandle handle = - new(ctx, internalCtx, new WindowPosState[] { windowPosState, windowPosState2 }); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(false); - - // When disposing - handle.Dispose(); - - // Then the layout is deferred - internalCtx.DeferWindowPosManager.DeferLayout( - Arg.Is>(x => x.Count == 1 && x[0] == windowPosState), - Arg.Is>(x => x.Count == 1 && x[0] == windowPosState2) - ); - } - [Theory] [InlineAutoSubstituteData(100, 1)] [InlineAutoSubstituteData(200, 2)] @@ -121,7 +97,6 @@ WindowPosState windowPosState { // Given a single window, and a monitor which has a scale factor != 100 using DeferWindowPosHandle handle = new(ctx, internalCtx); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); IMonitor monitor1 = Substitute.For(); monitor1.ScaleFactor.Returns(100); @@ -153,7 +128,6 @@ WindowPosState windowPosState2 // Given multiple windows, and a monitor which has a scale factor != 100 using DeferWindowPosHandle handle = new(ctx, internalCtx, new WindowPosState[] { windowPosState1, windowPosState2 }); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); IMonitor monitor1 = Substitute.For(); monitor1.ScaleFactor.Returns(100); @@ -177,7 +151,6 @@ internal void Dispose_NoWindowOffset(IContext ctx, IInternalContext internalCtx, ctx.NativeManager.GetWindowOffset(windowPosState.WindowState.Window.Handle).Returns((Rectangle?)null); using DeferWindowPosHandle handle = new(ctx, internalCtx, new WindowPosState[] { windowPosState }); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); // When disposing handle.Dispose(); @@ -214,7 +187,6 @@ WindowPosState windowPosState windowPosState.WindowState.WindowSize = windowSize; using DeferWindowPosHandle handle = new(ctx, internalCtx, new WindowPosState[] { windowPosState }); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); // When disposing handle.Dispose(); @@ -242,7 +214,6 @@ WindowPosState windowPosState { // Given no HWND is provided using DeferWindowPosHandle handle = new(ctx, internalCtx); - internalCtx.DeferWindowPosManager.CanDoLayout().Returns(true); handle.DeferWindowPos(windowPosState.WindowState, null, null); diff --git a/src/Whim.Tests/Native/DeferWindowPosManagerTests.cs b/src/Whim.Tests/Native/DeferWindowPosManagerTests.cs deleted file mode 100644 index 3f5d2c49d..000000000 --- a/src/Whim.Tests/Native/DeferWindowPosManagerTests.cs +++ /dev/null @@ -1,144 +0,0 @@ -using System.Threading.Tasks; -using AutoFixture; -using NSubstitute; -using Whim.TestUtils; -using Xunit; - -namespace Whim.Tests; - -public class DeferWindowPosManagerCustomization : ICustomization -{ - public void Customize(IFixture fixture) - { - fixture.Register( - () => - new WindowPosState( - new WindowState() - { - Rectangle = new Rectangle(), - Window = Substitute.For(), - WindowSize = WindowSize.Normal - } - ) - ); - - IInternalContext internalCtx = fixture.Freeze(); - internalCtx.DeferWindowPosManager.ParallelOptions.Returns(new ParallelOptions { MaxDegreeOfParallelism = 1 }); - } -} - -public class DeferWindowPosManagerTests -{ - private static DeferWindowPosManager CreateSut(IContext ctx, IInternalContext internalCtx) - { - DeferWindowPosManager manager = new(ctx, internalCtx); - internalCtx.DeferWindowPosManager.Returns(manager); - return manager; - } - - [Theory, AutoSubstituteData] - internal void RecoverLayout_NoDeferredWindows(IContext ctx, IInternalContext internalCtx) - { - // Given no windows are provided - DeferWindowPosManager manager = CreateSut(ctx, internalCtx); - // When RecoverLayout is called - manager.RecoverLayout(); - - // Then no calls to Butler.Pantry.GetWorkspaceForWindow are made - ctx.Butler.Pantry.DidNotReceive().GetWorkspaceForWindow(Arg.Any()); - } - - [Theory, AutoSubstituteData] - internal void RecoverLayout_DeferWorkspaceLayout( - IContext ctx, - IInternalContext internalCtx, - WindowPosState windowPosState, - WindowPosState minimizedWindowPosState, - IWorkspace workspace - ) - { - // Given a window is provided, and a workspace is found for it - minimizedWindowPosState.WindowState.WindowSize = WindowSize.Minimized; - DeferWindowPosManager manager = CreateSut(ctx, internalCtx); - manager.DeferLayout(new() { windowPosState }, new() { minimizedWindowPosState }); - - ctx.Butler.Pantry.GetWorkspaceForWindow(windowPosState.WindowState.Window).Returns(workspace); - - // When RecoverLayout is called - manager.RecoverLayout(); - - // Then DoLayout is called on the workspace - workspace.Received(1).DoLayout(); - ctx.MonitorManager.DidNotReceive().GetEnumerator(); - } - - [Theory, AutoSubstituteData] - internal void RecoverLayout_DeferWorkspaceLayout_WorkspaceAlreadyDeferred( - IContext ctx, - IInternalContext internalCtx, - WindowPosState windowPosState1, - WindowPosState windowPosState2, - WindowPosState minimizedWindowPosState, - IWorkspace workspace - ) - { - // Given two windows are provided for the same workspace - DeferWindowPosManager manager = CreateSut(ctx, internalCtx); - manager.DeferLayout(new() { windowPosState1, windowPosState2 }, new() { minimizedWindowPosState }); - - ctx.Butler.Pantry.GetWorkspaceForWindow(windowPosState1.WindowState.Window).Returns(workspace); - ctx.Butler.Pantry.GetWorkspaceForWindow(windowPosState2.WindowState.Window).Returns(workspace); - ctx.Butler.Pantry.GetWorkspaceForWindow(minimizedWindowPosState.WindowState.Window).Returns(workspace); - - // When RecoverLayout is called - manager.RecoverLayout(); - - // Then DoLayout is called on the workspace once - workspace.Received(1).DoLayout(); - ctx.MonitorManager.DidNotReceive().GetEnumerator(); - } - - [Theory, AutoSubstituteData] - internal void RecoverLayout_DeferWindowLayout( - IContext ctx, - IInternalContext internalCtx, - WindowPosState windowPosState - ) - { - // Given a window is provided, no workspace is found for the window, and the window is - // ignored by the FilterManager - DeferWindowPosManager manager = CreateSut(ctx, internalCtx); - manager.DeferLayout(new() { windowPosState }, new()); - - ctx.Butler.Pantry.GetWorkspaceForWindow(windowPosState.WindowState.Window).Returns((IWorkspace?)null); - ctx.FilterManager.ShouldBeIgnored(windowPosState.WindowState.Window).Returns(true); - - // When RecoverLayout is called - manager.RecoverLayout(); - - // Then the DeferWindowPosHandle is called - ctx.MonitorManager.Received(1).GetEnumerator(); - } - - [Theory, AutoSubstituteData] - internal void RecoverLayout_DoNotLayoutWindow( - IContext ctx, - IInternalContext internalCtx, - WindowPosState windowPosState - ) - { - // Given a window is provided, no workspace is found for the window, and the window is - // not ignored by the FilterManager - DeferWindowPosManager manager = CreateSut(ctx, internalCtx); - manager.DeferLayout(new() { windowPosState }, new()); - - ctx.Butler.Pantry.GetWorkspaceForWindow(windowPosState.WindowState.Window).Returns((IWorkspace?)null); - ctx.FilterManager.ShouldBeIgnored(windowPosState.WindowState.Window).Returns(false); - - // When RecoverLayout is called - manager.RecoverLayout(); - - // Then the DeferWindowPosHandle is not called - ctx.MonitorManager.DidNotReceive().GetEnumerator(); - } -} diff --git a/src/Whim/Context/IInternalContext.cs b/src/Whim/Context/IInternalContext.cs index 278ca44c2..32eb50773 100644 --- a/src/Whim/Context/IInternalContext.cs +++ b/src/Whim/Context/IInternalContext.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; namespace Whim; @@ -7,6 +8,8 @@ namespace Whim; /// internal interface IInternalContext : IDisposable { + ParallelOptions ParallelOptions { get; } + IInternalButler Butler { get; } IButlerEventHandlers ButlerEventHandlers { get; } diff --git a/src/Whim/Context/InternalContext.cs b/src/Whim/Context/InternalContext.cs index 345e0e312..9a344adf2 100644 --- a/src/Whim/Context/InternalContext.cs +++ b/src/Whim/Context/InternalContext.cs @@ -1,3 +1,5 @@ +using System.Threading.Tasks; + namespace Whim; internal class InternalContext : IInternalContext @@ -6,6 +8,8 @@ internal class InternalContext : IInternalContext private readonly IContext _context; + public ParallelOptions ParallelOptions { get; } = new(); + public ICoreSavedStateManager CoreSavedStateManager { get; } public ICoreNativeManager CoreNativeManager { get; } From 739420da749c71fb687b989271b8de559e71a18b Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 20:48:27 +1100 Subject: [PATCH 05/31] Run commands on a separate thread --- src/Whim/Commands/Command.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/Whim/Commands/Command.cs b/src/Whim/Commands/Command.cs index eec44e933..96a7d8bc9 100644 --- a/src/Whim/Commands/Command.cs +++ b/src/Whim/Commands/Command.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; namespace Whim; @@ -41,17 +42,18 @@ public Command(string identifier, string title, Action callback, Func? con } /// - public bool CanExecute() - { - return _condition?.Invoke() ?? true; - } + public bool CanExecute() => Task.Run(CanExecuteFn).Result; + + private bool CanExecuteFn() => _condition?.Invoke() ?? true; /// - public bool TryExecute() + public bool TryExecute() => Task.Run(TryExecuteFn).Result; + + private bool TryExecuteFn() { Logger.Debug($"Trying to execute command {Id}"); - if (CanExecute()) + if (_condition?.Invoke() ?? true) { _callback(); return true; From 822c17a1f3f16d343266ea7b4921fc37b6ba7de2 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 20:59:48 +1100 Subject: [PATCH 06/31] Lock CommandManager --- src/Whim/Commands/CommandManager.cs | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/Whim/Commands/CommandManager.cs b/src/Whim/Commands/CommandManager.cs index 281009551..f79dc8d33 100644 --- a/src/Whim/Commands/CommandManager.cs +++ b/src/Whim/Commands/CommandManager.cs @@ -6,6 +6,7 @@ namespace Whim; internal class CommandManager : ICommandManager { + private readonly object _lock = new(); private readonly Dictionary _commands = new(); public int Count => _commands.Count; @@ -16,6 +17,14 @@ internal class CommandManager : ICommandManager /// /// internal void AddPluginCommand(ICommand item) + { + lock (_lock) + { + AddPluginFn(item); + } + } + + private void AddPluginFn(ICommand item) { if (_commands.ContainsKey(item.Id)) { @@ -26,6 +35,14 @@ internal void AddPluginCommand(ICommand item) } public void Add(string identifier, string title, Action callback, Func? condition = null) + { + lock (_lock) + { + AddFn(identifier, title, callback, condition); + } + } + + private void AddFn(string identifier, string title, Action callback, Func? condition) { if (!identifier.StartsWith(ICommandManager.CustomCommandPrefix)) { @@ -37,12 +54,15 @@ public void Add(string identifier, string title, Action callback, Func? co public ICommand? TryGetCommand(string commandId) { - if (_commands.TryGetValue(commandId, out ICommand? command)) + lock (_lock) { - return command; - } + if (_commands.TryGetValue(commandId, out ICommand? command)) + { + return command; + } - return null; + return null; + } } public IEnumerator GetEnumerator() => _commands.Values.GetEnumerator(); From 0ee86f54940d1e9aeab9ebf4c15085b517afd890 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 21:46:36 +1100 Subject: [PATCH 07/31] Fix parallel tests --- src/Whim/Native/DeferWindowPosHandle.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Whim/Native/DeferWindowPosHandle.cs b/src/Whim/Native/DeferWindowPosHandle.cs index 6e7234816..615489da5 100644 --- a/src/Whim/Native/DeferWindowPosHandle.cs +++ b/src/Whim/Native/DeferWindowPosHandle.cs @@ -178,7 +178,7 @@ private void SetAllWindowPos() { for (int i = 0; i < numPasses; i++) { - Parallel.ForEach(allStates, SetWindowPos); + Parallel.ForEach(allStates, _internalContext.ParallelOptions, SetWindowPos); } } From a7ef8f841f22f8150c34fd95cd95ac8d493163ae Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 22:29:46 +1100 Subject: [PATCH 08/31] Add Lock struct to use instead of `lock statement` --- src/Whim/Commands/CommandManager.cs | 24 +++++++++--------------- src/Whim/Utils/Lock.cs | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 src/Whim/Utils/Lock.cs diff --git a/src/Whim/Commands/CommandManager.cs b/src/Whim/Commands/CommandManager.cs index f79dc8d33..12b6249e5 100644 --- a/src/Whim/Commands/CommandManager.cs +++ b/src/Whim/Commands/CommandManager.cs @@ -18,10 +18,8 @@ internal class CommandManager : ICommandManager /// internal void AddPluginCommand(ICommand item) { - lock (_lock) - { - AddPluginFn(item); - } + using Lock _ = new(_lock); + AddPluginFn(item); } private void AddPluginFn(ICommand item) @@ -36,10 +34,8 @@ private void AddPluginFn(ICommand item) public void Add(string identifier, string title, Action callback, Func? condition = null) { - lock (_lock) - { - AddFn(identifier, title, callback, condition); - } + using Lock _ = new(_lock); + AddFn(identifier, title, callback, condition); } private void AddFn(string identifier, string title, Action callback, Func? condition) @@ -54,15 +50,13 @@ private void AddFn(string identifier, string title, Action callback, Func? public ICommand? TryGetCommand(string commandId) { - lock (_lock) + using Lock _ = new(_lock); + if (_commands.TryGetValue(commandId, out ICommand? command)) { - if (_commands.TryGetValue(commandId, out ICommand? command)) - { - return command; - } - - return null; + return command; } + + return null; } public IEnumerator GetEnumerator() => _commands.Values.GetEnumerator(); diff --git a/src/Whim/Utils/Lock.cs b/src/Whim/Utils/Lock.cs new file mode 100644 index 000000000..55e73e7b6 --- /dev/null +++ b/src/Whim/Utils/Lock.cs @@ -0,0 +1,29 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.CompilerServices; + +namespace Whim; + +// Despite what the compiler says, `_lockWasTaken` is reassigned by `Monitor.Enter`. +[SuppressMessage("Style", "IDE0044:Make field readonly")] +internal struct Lock : IDisposable +{ + private readonly object _lockObj; + private bool _lockWasTaken = false; + + public Lock(object lockObj) + { + _lockObj = lockObj; + System.Threading.Monitor.Enter(_lockObj, ref _lockWasTaken); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public readonly void Dispose() + { + if (_lockWasTaken) + { + System.Threading.Monitor.Exit(_lockObj); + Console.WriteLine("Freed"); + } + } +} From c61be7fee7b375fa357a487c65ff1f3416270df7 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 22:31:21 +1100 Subject: [PATCH 09/31] Use `using Lock` --- src/Whim/Native/DeferWindowPosHandle.cs | 9 +-------- src/Whim/Native/NativeManager.cs | 18 ++++++++---------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/src/Whim/Native/DeferWindowPosHandle.cs b/src/Whim/Native/DeferWindowPosHandle.cs index 615489da5..f5792c50a 100644 --- a/src/Whim/Native/DeferWindowPosHandle.cs +++ b/src/Whim/Native/DeferWindowPosHandle.cs @@ -118,14 +118,7 @@ private void AddToCorrectList(WindowPosState windowPosState) /// public void Dispose() { - lock (_lock) - { - SetAllWindowPos(); - } - } - - private void SetAllWindowPos() - { + using Lock _ = new(_lock); Logger.Debug("Disposing WindowDeferPosHandle"); if (_windowStates.Count == 0 && _minimizedWindowStates.Count == 0) diff --git a/src/Whim/Native/NativeManager.cs b/src/Whim/Native/NativeManager.cs index b6bbcd430..8b624edb7 100644 --- a/src/Whim/Native/NativeManager.cs +++ b/src/Whim/Native/NativeManager.cs @@ -237,20 +237,18 @@ public Compositor Compositor { if (_compositor == null) { - lock (_compositorLock) - { + using Lock _ = new(_compositorLock); #pragma warning disable CA1508 // Avoid dead conditional code, because of the lock. - if (_compositor == null) + if (_compositor == null) + { + if (DispatcherQueue.GetForCurrentThread() == null) { - if (DispatcherQueue.GetForCurrentThread() == null) - { - InitializeCoreDispatcher(); - } - - _compositor = new Compositor(); + InitializeCoreDispatcher(); } -#pragma warning restore CA1508 // Avoid dead conditional code + + _compositor = new Compositor(); } +#pragma warning restore CA1508 // Avoid dead conditional code } return _compositor; From 14f75147dc25587d57f1cfaf08623a12e19386c6 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 16 Mar 2024 22:45:42 +1100 Subject: [PATCH 10/31] Lock ButlerPantry --- src/Whim/Butler/ButlerPantry.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Whim/Butler/ButlerPantry.cs b/src/Whim/Butler/ButlerPantry.cs index 40f70e65c..8e6f475db 100644 --- a/src/Whim/Butler/ButlerPantry.cs +++ b/src/Whim/Butler/ButlerPantry.cs @@ -6,6 +6,7 @@ namespace Whim; internal class ButlerPantry : IButlerPantry { + private readonly object _lock = new(); private readonly IContext _context; private readonly Dictionary _windowWorkspaceMap = new(); private readonly Dictionary _monitorWorkspaceMap = new(); @@ -17,6 +18,7 @@ public ButlerPantry(IContext context) public IWorkspace? GetAdjacentWorkspace(IWorkspace workspace, bool reverse = false, bool skipActive = false) { + using Lock _ = new(_lock); IWorkspace[] workspaces = _context.WorkspaceManager.ToArray(); int idx = Array.IndexOf(workspaces, workspace); @@ -41,12 +43,14 @@ public ButlerPantry(IContext context) public IEnumerable GetAllActiveWorkspaces() { + using Lock _ = new(_lock); Logger.Debug("Getting all active workspaces"); return _monitorWorkspaceMap.Values; } public IMonitor? GetMonitorForWindow(IWindow window) { + using Lock _ = new(_lock); Logger.Debug($"Getting monitor for window: {window}"); return _windowWorkspaceMap.TryGetValue(window, out IWorkspace? workspace) ? GetMonitorForWorkspace(workspace) @@ -55,6 +59,7 @@ public IEnumerable GetAllActiveWorkspaces() public IMonitor? GetMonitorForWorkspace(IWorkspace workspace) { + using Lock _ = new(_lock); Logger.Debug($"Getting monitor for workspace {workspace}"); // Linear search for the monitor that contains the workspace. @@ -73,30 +78,35 @@ public IEnumerable GetAllActiveWorkspaces() public IWorkspace? GetWorkspaceForMonitor(IMonitor monitor) { + using Lock _ = new(_lock); Logger.Debug($"Getting workspace for monitor {monitor}"); return _monitorWorkspaceMap.TryGetValue(monitor, out IWorkspace? workspace) ? workspace : null; } public IWorkspace? GetWorkspaceForWindow(IWindow window) { + using Lock _ = new(_lock); Logger.Debug($"Getting workspace for window {window}"); return _windowWorkspaceMap.TryGetValue(window, out IWorkspace? workspace) ? workspace : null; } public bool RemoveMonitor(IMonitor monitor) { + using Lock _ = new(_lock); Logger.Debug($"Removing monitor {monitor}"); return _monitorWorkspaceMap.Remove(monitor); } public bool RemoveWindow(IWindow window) { + using Lock _ = new(_lock); Logger.Debug($"Removing window {window}"); return _windowWorkspaceMap.Remove(window); } public void MergeWorkspaceWindows(IWorkspace workspaceToDelete, IWorkspace workspaceMergeTarget) { + using Lock _ = new(_lock); Logger.Debug($"Removing workspace {workspaceToDelete} and moving windows to {workspaceMergeTarget}"); // Remove the workspace from the monitor map. @@ -115,12 +125,14 @@ public void MergeWorkspaceWindows(IWorkspace workspaceToDelete, IWorkspace works public void SetWindowWorkspace(IWindow window, IWorkspace workspace) { + using Lock _ = new(_lock); Logger.Debug($"Setting window {window} to workspace {workspace}"); _windowWorkspaceMap[window] = workspace; } public void SetMonitorWorkspace(IMonitor monitor, IWorkspace workspace) { + using Lock _ = new(_lock); Logger.Debug($"Setting workspace {workspace} to monitor {monitor}"); _monitorWorkspaceMap[monitor] = workspace; } From 8796ca568c38f44d14d97425619861c479f50d6d Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Mon, 18 Mar 2024 19:35:47 +1100 Subject: [PATCH 11/31] FilterManager locks --- src/Whim/Filter/FilterManager.cs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/Whim/Filter/FilterManager.cs b/src/Whim/Filter/FilterManager.cs index 32633b57f..db8288e00 100644 --- a/src/Whim/Filter/FilterManager.cs +++ b/src/Whim/Filter/FilterManager.cs @@ -31,35 +31,42 @@ public void Clear() _filters.Clear(); } - public bool ShouldBeIgnored(IWindow window) => - _ignoreWindowClasses.Contains(window.WindowClass.ToLower()) - || ( - window.ProcessFileName is string processFileName - && _ignoreProcessFileNames.Contains(processFileName.ToLower()) - ) - || _ignoreTitles.Contains(window.Title.ToLower()) - || _filters.Any(f => f(window)); + public bool ShouldBeIgnored(IWindow window) + { + using Lock _ = new(); + return _ignoreWindowClasses.Contains(window.WindowClass.ToLower()) + || ( + window.ProcessFileName is string processFileName + && _ignoreProcessFileNames.Contains(processFileName.ToLower()) + ) + || _ignoreTitles.Contains(window.Title.ToLower()) + || _filters.Any(f => f(window)); + } public IFilterManager AddWindowClassFilter(string windowClass) { + using Lock _ = new(); _ignoreWindowClasses.Add(windowClass.ToLower()); return this; } public IFilterManager AddProcessFileNameFilter(string processFileName) { + using Lock _ = new(); _ignoreProcessFileNames.Add(processFileName.ToLower()); return this; } public IFilterManager AddTitleFilter(string title) { + using Lock _ = new(); _ignoreTitles.Add(title.ToLower()); return this; } public IFilterManager AddTitleMatchFilter(string title) { + using Lock _ = new(); Regex regex = new(title); _filters.Add(window => regex.IsMatch(window.Title)); return this; From 3e7ea171e2161d35f8e255bd32e94d1cd056be32 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Mon, 18 Mar 2024 20:18:33 +1100 Subject: [PATCH 12/31] KeybindManager locks --- src/Whim/Keybind/KeybindManager.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/Whim/Keybind/KeybindManager.cs b/src/Whim/Keybind/KeybindManager.cs index e582f3a45..ea59023c3 100644 --- a/src/Whim/Keybind/KeybindManager.cs +++ b/src/Whim/Keybind/KeybindManager.cs @@ -21,6 +21,7 @@ public bool UnifyKeyModifiers get => _uniqueKeyModifiers; set { + using Lock _ = new(); if (value && _uniqueKeyModifiers == false) { _uniqueKeyModifiers = true; @@ -45,6 +46,7 @@ private void UnifyKeybinds() public void SetKeybind(string commandId, IKeybind keybind) { + using Lock _ = new(); Logger.Debug($"Setting keybind '{keybind}' for command '{commandId}'"); keybind = UnifyKeyModifiers ? keybind.UnifyModifiers() : keybind; @@ -65,6 +67,7 @@ public void SetKeybind(string commandId, IKeybind keybind) public ICommand[] GetCommands(IKeybind keybind) { + using Lock _ = new(); Logger.Debug($"Getting commands for keybind '{keybind}'"); keybind = UnifyKeyModifiers ? keybind.UnifyModifiers() : keybind; @@ -94,13 +97,14 @@ public ICommand[] GetCommands(IKeybind keybind) public IKeybind? TryGetKeybind(string commandId) { + using Lock _ = new(); Logger.Debug($"Getting keybind for command '{commandId}'"); - return _commandsKeybindsMap.TryGetValue(commandId, out IKeybind? keybind) ? keybind : null; } public bool Remove(string commandId) { + using Lock _ = new(); Logger.Debug($"Removing keybind for command '{commandId}'"); IKeybind? keybind = TryGetKeybind(commandId); @@ -116,6 +120,7 @@ public bool Remove(string commandId) public void Clear() { + using Lock _ = new(); Logger.Debug("Removing all keybinds"); _commandsKeybindsMap.Clear(); _keybindsCommandsMap.Clear(); From 5c427d5f3c3e52c5ee3a880566786cb3e634cc1a Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Mon, 18 Mar 2024 20:24:23 +1100 Subject: [PATCH 13/31] PluginManager locks --- src/Whim/Plugin/PluginManager.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Whim/Plugin/PluginManager.cs b/src/Whim/Plugin/PluginManager.cs index 85244daa0..cca4fd92d 100644 --- a/src/Whim/Plugin/PluginManager.cs +++ b/src/Whim/Plugin/PluginManager.cs @@ -88,6 +88,7 @@ private void LoadSavedState() public T AddPlugin(T plugin) where T : IPlugin { + using Lock _ = new(); switch (plugin.Name) { case ICommandManager.CustomCommandPrefix: @@ -124,6 +125,7 @@ public T AddPlugin(T plugin) public bool Contains(string pluginName) { + using Lock _ = new(); return _plugins.Exists(plugin => plugin.Name == pluginName); } From 0bc76be5d09d16abbd988b8f00da91c615b599ab Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Mon, 18 Mar 2024 20:26:26 +1100 Subject: [PATCH 14/31] RouterManager locks --- src/Whim/Router/RouterManager.cs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/Whim/Router/RouterManager.cs b/src/Whim/Router/RouterManager.cs index b3bd97ad7..923ed7313 100644 --- a/src/Whim/Router/RouterManager.cs +++ b/src/Whim/Router/RouterManager.cs @@ -17,18 +17,21 @@ public RouterManager(IContext context) public void Add(Router router) { + using Lock _ = new(); Logger.Debug($"Adding router {router}"); _routers.Add(router); } public void Clear() { + using Lock _ = new(); Logger.Debug("Clearing routes"); _routers.Clear(); } public IRouterManager AddProcessFileNameRoute(string processFileName, string workspaceName) { + using Lock _ = new(); processFileName = processFileName.ToLower(); Logger.Debug($"Routing process file name {processFileName} to workspace {workspaceName}"); Add(window => @@ -44,6 +47,7 @@ public IRouterManager AddProcessFileNameRoute(string processFileName, string wor public IRouterManager AddProcessFileNameRoute(string processFileName, IWorkspace workspace) { + using Lock _ = new(); processFileName = processFileName.ToLower(); Logger.Debug($"Routing process file name: {processFileName} to workspace {workspace}"); Add(window => @@ -59,6 +63,7 @@ public IRouterManager AddProcessFileNameRoute(string processFileName, IWorkspace public IRouterManager AddTitleRoute(string title, string workspaceName) { + using Lock _ = new(); title = title.ToLower(); Logger.Debug($"Routing title: {title} to workspace {workspaceName}"); Add(window => @@ -74,6 +79,7 @@ public IRouterManager AddTitleRoute(string title, string workspaceName) public IRouterManager AddTitleRoute(string title, IWorkspace workspace) { + using Lock _ = new(); title = title.ToLower(); Logger.Debug($"Routing title: {title} to workspace {workspace}"); Add(window => @@ -89,6 +95,7 @@ public IRouterManager AddTitleRoute(string title, IWorkspace workspace) public IRouterManager AddTitleMatchRoute(string match, string workspaceName) { + using Lock _ = new(); Logger.Debug($"Routing title match: {match} to workspace {workspaceName}"); Regex regex = new(match); Add(window => @@ -104,6 +111,7 @@ public IRouterManager AddTitleMatchRoute(string match, string workspaceName) public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) { + using Lock _ = new(); Logger.Debug($"Routing title match: {match} to workspace {workspace}"); Regex regex = new(match); Add(window => @@ -119,6 +127,7 @@ public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) public IWorkspace? RouteWindow(IWindow window) { + using Lock _ = new(); Logger.Debug($"Routing window {window}"); foreach (Router router in _routers) @@ -136,6 +145,7 @@ public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) public IRouterManager AddWindowClassRoute(string windowClass, string workspaceName) { + using Lock _ = new(); windowClass = windowClass.ToLower(); Logger.Debug($"Routing window class: {windowClass} to workspace {workspaceName}"); Add(window => @@ -151,6 +161,7 @@ public IRouterManager AddWindowClassRoute(string windowClass, string workspaceNa public IRouterManager AddWindowClassRoute(string windowClass, IWorkspace workspace) { + using Lock _ = new(); windowClass = windowClass.ToLower(); Logger.Debug($"Routing window class: {windowClass} to workspace {workspace}"); Add(window => From fe50411de867998ac209cb6ea878b834194b9b37 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Mon, 18 Mar 2024 20:29:28 +1100 Subject: [PATCH 15/31] WorkspaceManager locks --- src/Whim/Workspace/WorkspaceManager.cs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/Whim/Workspace/WorkspaceManager.cs b/src/Whim/Workspace/WorkspaceManager.cs index ace0ac843..ebd26d8ac 100644 --- a/src/Whim/Workspace/WorkspaceManager.cs +++ b/src/Whim/Workspace/WorkspaceManager.cs @@ -20,6 +20,7 @@ internal record WorkspaceToCreate(string Name, IEnumerable? createLayoutEngines = null) { + using Lock _ = new(_lockObj); if (_initialized) { return CreateWorkspace(name, createLayoutEngines); @@ -94,16 +97,26 @@ public WorkspaceManager(IContext context, IInternalContext internalContext) public void AddProxyLayoutEngine(CreateProxyLayoutEngine proxyLayoutEngine) { + using Lock _ = new(_lockObj); Logger.Debug($"Adding proxy layout engine: {proxyLayoutEngine}"); _proxyLayoutEngines.Add(proxyLayoutEngine); } - public bool Contains(IWorkspace workspace) => _workspaces.Contains(workspace); + public bool Contains(IWorkspace workspace) + { + using Lock _ = new(_lockObj); + return _workspaces.Contains(workspace); + } - public IEnumerator GetEnumerator() => _workspaces.GetEnumerator(); + public IEnumerator GetEnumerator() + { + using Lock _ = new(_lockObj); + return _workspaces.GetEnumerator(); + } public void Initialize() { + using Lock _ = new(_lockObj); Logger.Debug("Initializing workspace manager"); _initialized = true; @@ -132,6 +145,7 @@ public void Initialize() IEnumerable? createLayoutEngines = null ) { + using Lock _ = new(_lockObj); CreateLeafLayoutEngine[] engineCreators = createLayoutEngines?.ToArray() ?? CreateLayoutEngines(); if (engineCreators.Length == 0) @@ -169,6 +183,7 @@ public void Initialize() public bool Remove(IWorkspace workspace) { + using Lock _ = new(_lockObj); Logger.Debug($"Removing workspace {workspace}"); if (_workspaces.Count - 1 < _context.MonitorManager.Length) @@ -194,6 +209,7 @@ public bool Remove(IWorkspace workspace) public bool Remove(string workspaceName) { + using Lock _ = new(_lockObj); Logger.Debug($"Trying to remove workspace {workspaceName}"); IWorkspace? workspace = _workspaces.Find(w => w.Name == workspaceName); @@ -208,6 +224,7 @@ public bool Remove(string workspaceName) public IWorkspace? TryGet(string workspaceName) { + using Lock _ = new(_lockObj); Logger.Debug($"Trying to get workspace {workspaceName}"); return _workspaces.Find(w => w.Name == workspaceName); } From 5e2e1c32d2a5469b91c7f5c1cf335767d015271b Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Tue, 19 Mar 2024 21:11:10 +1100 Subject: [PATCH 16/31] Add missing lock objects --- src/Whim/Butler/ButlerPantry.cs | 24 ++++++++++++------------ src/Whim/Commands/CommandManager.cs | 8 ++++---- src/Whim/Filter/FilterManager.cs | 11 ++++++----- src/Whim/Keybind/KeybindManager.cs | 13 +++++++------ src/Whim/Native/DeferWindowPosHandle.cs | 4 ++-- src/Whim/Plugin/PluginManager.cs | 5 +++-- src/Whim/Router/RouterManager.cs | 23 ++++++++++++----------- src/Whim/Utils/Lock.cs | 6 +++--- 8 files changed, 49 insertions(+), 45 deletions(-) diff --git a/src/Whim/Butler/ButlerPantry.cs b/src/Whim/Butler/ButlerPantry.cs index 8e6f475db..838836cb1 100644 --- a/src/Whim/Butler/ButlerPantry.cs +++ b/src/Whim/Butler/ButlerPantry.cs @@ -6,7 +6,7 @@ namespace Whim; internal class ButlerPantry : IButlerPantry { - private readonly object _lock = new(); + private readonly object _lockObj = new(); private readonly IContext _context; private readonly Dictionary _windowWorkspaceMap = new(); private readonly Dictionary _monitorWorkspaceMap = new(); @@ -18,7 +18,7 @@ public ButlerPantry(IContext context) public IWorkspace? GetAdjacentWorkspace(IWorkspace workspace, bool reverse = false, bool skipActive = false) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); IWorkspace[] workspaces = _context.WorkspaceManager.ToArray(); int idx = Array.IndexOf(workspaces, workspace); @@ -43,14 +43,14 @@ public ButlerPantry(IContext context) public IEnumerable GetAllActiveWorkspaces() { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug("Getting all active workspaces"); return _monitorWorkspaceMap.Values; } public IMonitor? GetMonitorForWindow(IWindow window) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Getting monitor for window: {window}"); return _windowWorkspaceMap.TryGetValue(window, out IWorkspace? workspace) ? GetMonitorForWorkspace(workspace) @@ -59,7 +59,7 @@ public IEnumerable GetAllActiveWorkspaces() public IMonitor? GetMonitorForWorkspace(IWorkspace workspace) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Getting monitor for workspace {workspace}"); // Linear search for the monitor that contains the workspace. @@ -78,35 +78,35 @@ public IEnumerable GetAllActiveWorkspaces() public IWorkspace? GetWorkspaceForMonitor(IMonitor monitor) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Getting workspace for monitor {monitor}"); return _monitorWorkspaceMap.TryGetValue(monitor, out IWorkspace? workspace) ? workspace : null; } public IWorkspace? GetWorkspaceForWindow(IWindow window) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Getting workspace for window {window}"); return _windowWorkspaceMap.TryGetValue(window, out IWorkspace? workspace) ? workspace : null; } public bool RemoveMonitor(IMonitor monitor) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Removing monitor {monitor}"); return _monitorWorkspaceMap.Remove(monitor); } public bool RemoveWindow(IWindow window) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Removing window {window}"); return _windowWorkspaceMap.Remove(window); } public void MergeWorkspaceWindows(IWorkspace workspaceToDelete, IWorkspace workspaceMergeTarget) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Removing workspace {workspaceToDelete} and moving windows to {workspaceMergeTarget}"); // Remove the workspace from the monitor map. @@ -125,14 +125,14 @@ public void MergeWorkspaceWindows(IWorkspace workspaceToDelete, IWorkspace works public void SetWindowWorkspace(IWindow window, IWorkspace workspace) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Setting window {window} to workspace {workspace}"); _windowWorkspaceMap[window] = workspace; } public void SetMonitorWorkspace(IMonitor monitor, IWorkspace workspace) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug($"Setting workspace {workspace} to monitor {monitor}"); _monitorWorkspaceMap[monitor] = workspace; } diff --git a/src/Whim/Commands/CommandManager.cs b/src/Whim/Commands/CommandManager.cs index 12b6249e5..4da71a9a2 100644 --- a/src/Whim/Commands/CommandManager.cs +++ b/src/Whim/Commands/CommandManager.cs @@ -6,7 +6,7 @@ namespace Whim; internal class CommandManager : ICommandManager { - private readonly object _lock = new(); + private readonly object _lockObj = new(); private readonly Dictionary _commands = new(); public int Count => _commands.Count; @@ -18,7 +18,7 @@ internal class CommandManager : ICommandManager /// internal void AddPluginCommand(ICommand item) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); AddPluginFn(item); } @@ -34,7 +34,7 @@ private void AddPluginFn(ICommand item) public void Add(string identifier, string title, Action callback, Func? condition = null) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); AddFn(identifier, title, callback, condition); } @@ -50,7 +50,7 @@ private void AddFn(string identifier, string title, Action callback, Func? public ICommand? TryGetCommand(string commandId) { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); if (_commands.TryGetValue(commandId, out ICommand? command)) { return command; diff --git a/src/Whim/Filter/FilterManager.cs b/src/Whim/Filter/FilterManager.cs index db8288e00..dbd2748dd 100644 --- a/src/Whim/Filter/FilterManager.cs +++ b/src/Whim/Filter/FilterManager.cs @@ -6,6 +6,7 @@ namespace Whim; internal class FilterManager : IFilterManager { + private readonly object _lockObj = new(); #region Filters for specific properties private readonly HashSet _ignoreWindowClasses = new(); private readonly HashSet _ignoreProcessFileNames = new(); @@ -33,7 +34,7 @@ public void Clear() public bool ShouldBeIgnored(IWindow window) { - using Lock _ = new(); + using Lock _ = new(_lockObj); return _ignoreWindowClasses.Contains(window.WindowClass.ToLower()) || ( window.ProcessFileName is string processFileName @@ -45,28 +46,28 @@ window.ProcessFileName is string processFileName public IFilterManager AddWindowClassFilter(string windowClass) { - using Lock _ = new(); + using Lock _ = new(_lockObj); _ignoreWindowClasses.Add(windowClass.ToLower()); return this; } public IFilterManager AddProcessFileNameFilter(string processFileName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); _ignoreProcessFileNames.Add(processFileName.ToLower()); return this; } public IFilterManager AddTitleFilter(string title) { - using Lock _ = new(); + using Lock _ = new(_lockObj); _ignoreTitles.Add(title.ToLower()); return this; } public IFilterManager AddTitleMatchFilter(string title) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Regex regex = new(title); _filters.Add(window => regex.IsMatch(window.Title)); return this; diff --git a/src/Whim/Keybind/KeybindManager.cs b/src/Whim/Keybind/KeybindManager.cs index ea59023c3..c43732160 100644 --- a/src/Whim/Keybind/KeybindManager.cs +++ b/src/Whim/Keybind/KeybindManager.cs @@ -6,6 +6,7 @@ namespace Whim; internal class KeybindManager : IKeybindManager { + private readonly object _lockObj = new(); private readonly IContext _context; private readonly Dictionary> _keybindsCommandsMap = new(); private readonly Dictionary _commandsKeybindsMap = new(); @@ -21,7 +22,7 @@ public bool UnifyKeyModifiers get => _uniqueKeyModifiers; set { - using Lock _ = new(); + using Lock _ = new(_lockObj); if (value && _uniqueKeyModifiers == false) { _uniqueKeyModifiers = true; @@ -46,7 +47,7 @@ private void UnifyKeybinds() public void SetKeybind(string commandId, IKeybind keybind) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Setting keybind '{keybind}' for command '{commandId}'"); keybind = UnifyKeyModifiers ? keybind.UnifyModifiers() : keybind; @@ -67,7 +68,7 @@ public void SetKeybind(string commandId, IKeybind keybind) public ICommand[] GetCommands(IKeybind keybind) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Getting commands for keybind '{keybind}'"); keybind = UnifyKeyModifiers ? keybind.UnifyModifiers() : keybind; @@ -97,14 +98,14 @@ public ICommand[] GetCommands(IKeybind keybind) public IKeybind? TryGetKeybind(string commandId) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Getting keybind for command '{commandId}'"); return _commandsKeybindsMap.TryGetValue(commandId, out IKeybind? keybind) ? keybind : null; } public bool Remove(string commandId) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Removing keybind for command '{commandId}'"); IKeybind? keybind = TryGetKeybind(commandId); @@ -120,7 +121,7 @@ public bool Remove(string commandId) public void Clear() { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug("Removing all keybinds"); _commandsKeybindsMap.Clear(); _keybindsCommandsMap.Clear(); diff --git a/src/Whim/Native/DeferWindowPosHandle.cs b/src/Whim/Native/DeferWindowPosHandle.cs index f5792c50a..16df6bf6a 100644 --- a/src/Whim/Native/DeferWindowPosHandle.cs +++ b/src/Whim/Native/DeferWindowPosHandle.cs @@ -17,7 +17,7 @@ namespace Whim; /// public sealed class DeferWindowPosHandle : IDisposable { - private static readonly object _lock = new(); + private static readonly object _lockObj = new(); private readonly IContext _context; private readonly IInternalContext _internalContext; @@ -118,7 +118,7 @@ private void AddToCorrectList(WindowPosState windowPosState) /// public void Dispose() { - using Lock _ = new(_lock); + using Lock _ = new(_lockObj); Logger.Debug("Disposing WindowDeferPosHandle"); if (_windowStates.Count == 0 && _minimizedWindowStates.Count == 0) diff --git a/src/Whim/Plugin/PluginManager.cs b/src/Whim/Plugin/PluginManager.cs index cca4fd92d..b229a768b 100644 --- a/src/Whim/Plugin/PluginManager.cs +++ b/src/Whim/Plugin/PluginManager.cs @@ -8,6 +8,7 @@ namespace Whim; internal partial class PluginManager : IPluginManager { + private readonly object _lockObj = new(); private readonly IContext _context; private readonly CommandManager _commandManager; private readonly string _savedStateFilePath; @@ -88,7 +89,7 @@ private void LoadSavedState() public T AddPlugin(T plugin) where T : IPlugin { - using Lock _ = new(); + using Lock _ = new(_lockObj); switch (plugin.Name) { case ICommandManager.CustomCommandPrefix: @@ -125,7 +126,7 @@ public T AddPlugin(T plugin) public bool Contains(string pluginName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); return _plugins.Exists(plugin => plugin.Name == pluginName); } diff --git a/src/Whim/Router/RouterManager.cs b/src/Whim/Router/RouterManager.cs index 923ed7313..44fabe16f 100644 --- a/src/Whim/Router/RouterManager.cs +++ b/src/Whim/Router/RouterManager.cs @@ -5,6 +5,7 @@ namespace Whim; internal class RouterManager : IRouterManager { + private readonly object _lockObj = new(); private readonly IContext _context; private readonly List _routers = new(); @@ -17,21 +18,21 @@ public RouterManager(IContext context) public void Add(Router router) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Adding router {router}"); _routers.Add(router); } public void Clear() { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug("Clearing routes"); _routers.Clear(); } public IRouterManager AddProcessFileNameRoute(string processFileName, string workspaceName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); processFileName = processFileName.ToLower(); Logger.Debug($"Routing process file name {processFileName} to workspace {workspaceName}"); Add(window => @@ -47,7 +48,7 @@ public IRouterManager AddProcessFileNameRoute(string processFileName, string wor public IRouterManager AddProcessFileNameRoute(string processFileName, IWorkspace workspace) { - using Lock _ = new(); + using Lock _ = new(_lockObj); processFileName = processFileName.ToLower(); Logger.Debug($"Routing process file name: {processFileName} to workspace {workspace}"); Add(window => @@ -63,7 +64,7 @@ public IRouterManager AddProcessFileNameRoute(string processFileName, IWorkspace public IRouterManager AddTitleRoute(string title, string workspaceName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); title = title.ToLower(); Logger.Debug($"Routing title: {title} to workspace {workspaceName}"); Add(window => @@ -79,7 +80,7 @@ public IRouterManager AddTitleRoute(string title, string workspaceName) public IRouterManager AddTitleRoute(string title, IWorkspace workspace) { - using Lock _ = new(); + using Lock _ = new(_lockObj); title = title.ToLower(); Logger.Debug($"Routing title: {title} to workspace {workspace}"); Add(window => @@ -95,7 +96,7 @@ public IRouterManager AddTitleRoute(string title, IWorkspace workspace) public IRouterManager AddTitleMatchRoute(string match, string workspaceName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Routing title match: {match} to workspace {workspaceName}"); Regex regex = new(match); Add(window => @@ -111,7 +112,7 @@ public IRouterManager AddTitleMatchRoute(string match, string workspaceName) public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Routing title match: {match} to workspace {workspace}"); Regex regex = new(match); Add(window => @@ -127,7 +128,7 @@ public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) public IWorkspace? RouteWindow(IWindow window) { - using Lock _ = new(); + using Lock _ = new(_lockObj); Logger.Debug($"Routing window {window}"); foreach (Router router in _routers) @@ -145,7 +146,7 @@ public IRouterManager AddTitleMatchRoute(string match, IWorkspace workspace) public IRouterManager AddWindowClassRoute(string windowClass, string workspaceName) { - using Lock _ = new(); + using Lock _ = new(_lockObj); windowClass = windowClass.ToLower(); Logger.Debug($"Routing window class: {windowClass} to workspace {workspaceName}"); Add(window => @@ -161,7 +162,7 @@ public IRouterManager AddWindowClassRoute(string windowClass, string workspaceNa public IRouterManager AddWindowClassRoute(string windowClass, IWorkspace workspace) { - using Lock _ = new(); + using Lock _ = new(_lockObj); windowClass = windowClass.ToLower(); Logger.Debug($"Routing window class: {windowClass} to workspace {workspace}"); Add(window => diff --git a/src/Whim/Utils/Lock.cs b/src/Whim/Utils/Lock.cs index 55e73e7b6..9f6129dd3 100644 --- a/src/Whim/Utils/Lock.cs +++ b/src/Whim/Utils/Lock.cs @@ -6,10 +6,10 @@ namespace Whim; // Despite what the compiler says, `_lockWasTaken` is reassigned by `Monitor.Enter`. [SuppressMessage("Style", "IDE0044:Make field readonly")] -internal struct Lock : IDisposable +internal class Lock : IDisposable { private readonly object _lockObj; - private bool _lockWasTaken = false; + private bool _lockWasTaken; public Lock(object lockObj) { @@ -18,7 +18,7 @@ public Lock(object lockObj) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public readonly void Dispose() + public void Dispose() { if (_lockWasTaken) { From 2dde577d7aa745de4dacdf4645c6530d89dfd22f Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Wed, 20 Mar 2024 21:28:41 +1100 Subject: [PATCH 17/31] WindowManager locks --- src/Whim/Window/WindowManager.cs | 64 +++++++++++++++++++------------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index 9114379e6..ee322497c 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -12,6 +12,7 @@ namespace Whim; internal class WindowManager : IWindowManager, IInternalWindowManager { + private readonly object _lockObj = new(); private readonly IContext _context; private readonly IInternalContext _internalContext; @@ -121,6 +122,7 @@ public void PostInitialize() public IWindow? CreateWindow(HWND hwnd) { + using Lock _ = new(_lockObj); Logger.Verbose($"Adding window {hwnd}"); if (_windows.TryGetValue(hwnd, out IWindow? window) && window != null) @@ -253,34 +255,39 @@ uint _dwmsEventTime return; } - // Try get the window - bool hasWindow = true; - if (!_windows.TryGetValue(hwnd, out IWindow? window) || window == null) - { - Logger.Verbose($"Window {hwnd} is not added, event type 0x{eventType:X4}"); - hasWindow = false; - window = AddWindow(hwnd); + bool windowWasCreated = false; + IWindow? window = null; - if ( - window == null - && (eventType == PInvoke.EVENT_SYSTEM_FOREGROUND || eventType == PInvoke.EVENT_OBJECT_UNCLOAKED) - ) + using (Lock _ = new(_lockObj)) + { + if (!_windows.TryGetValue(hwnd, out window)) { - // Even if the window was ignored, we need to fire OnWindowFocused. - Logger.Debug( - $"Window {hwnd} with event 0x{eventType:X4} was ignored, but still notifying listeners of focus" - ); - OnWindowFocused(window); - return; + Logger.Verbose($"Window {hwnd} is not added, event type 0x{eventType:X4}"); + windowWasCreated = true; + window = AddWindow(hwnd); } + } - if (window == null) - { - return; - } + if ( + windowWasCreated + && window == null + && (eventType == PInvoke.EVENT_SYSTEM_FOREGROUND || eventType == PInvoke.EVENT_OBJECT_UNCLOAKED) + ) + { + // Even if the window was ignored, we need to fire OnWindowFocused. + Logger.Debug( + $"Window {hwnd} with event 0x{eventType:X4} was ignored, but still notifying listeners of focus" + ); + OnWindowFocused(window); + return; + } + + if (window == null) + { + return; } - if (_internalContext.ButlerEventHandlers.AreMonitorsChanging && hasWindow) + if (_internalContext.ButlerEventHandlers.AreMonitorsChanging && windowWasCreated == false) { Logger.Debug($"Monitors are changing, ignoring event 0x{eventType:X4} for {window}"); return; @@ -362,7 +369,11 @@ uint _dwmsEventTime return null; } - _windows[hwnd] = window; + using (Lock _ = new(_lockObj)) + { + _windows[hwnd] = window; + } + OnWindowAdded(window); return window; @@ -409,8 +420,11 @@ public void OnWindowRemoved(IWindow window) { Logger.Debug($"Window removed: {window}"); - _windows.TryRemove(window.Handle, out _); - _handledLocationRestoringWindows.Remove(window); + using (Lock _lock = new(_lockObj)) + { + _windows.TryRemove(window.Handle, out _); + _handledLocationRestoringWindows.Remove(window); + } WindowEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowRemoved(args); From e53f31539fab1005d3e6d837cec021a483fe73a3 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Thu, 21 Mar 2024 20:29:11 +1100 Subject: [PATCH 18/31] MonitorManager locks --- src/Whim/Monitor/MonitorManager.cs | 59 ++++++++++++++++++++---------- 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/src/Whim/Monitor/MonitorManager.cs b/src/Whim/Monitor/MonitorManager.cs index 5876c9d8d..3d28f0280 100644 --- a/src/Whim/Monitor/MonitorManager.cs +++ b/src/Whim/Monitor/MonitorManager.cs @@ -14,6 +14,7 @@ namespace Whim; /// internal class MonitorManager : IInternalMonitorManager, IMonitorManager { + private readonly object _lockObj = new(); private readonly IContext _context; private readonly IInternalContext _internalContext; @@ -74,6 +75,7 @@ public void Initialize() public void OnWindowFocused(IWindow? window) { + using Lock _ = new(_lockObj); // If we know the window, use what the Butler knows instead of Windows. if (window is not null) { @@ -119,6 +121,7 @@ public void OnWindowFocused(IWindow? window) public void ActivateEmptyMonitor(IMonitor monitor) { + using Lock _ = new(_lockObj); Logger.Debug($"Activating empty monitor {monitor}"); if (!_monitors.Contains(monitor)) @@ -144,16 +147,46 @@ private void WindowMessageMonitor_SessionChanged(object? sender, WindowMessageMo private void WindowMessageMonitor_MonitorsChanged(object? sender, WindowMessageMonitorEventArgs e) { + OnMonitorsChanged( + e, + out List unchangedMonitors, + out List removedMonitors, + out List addedMonitors + ); + + // Notify listeners of the unchanged, removed, and added monitors. + MonitorsChangedEventArgs args = + new() + { + UnchangedMonitors = unchangedMonitors, + RemovedMonitors = removedMonitors, + AddedMonitors = addedMonitors + }; + + if (addedMonitors.Count != 0 || removedMonitors.Count != 0) + { + _internalContext.ButlerEventHandlers.OnMonitorsChanged(args); + } + MonitorsChanged?.Invoke(this, args); + } + + private void OnMonitorsChanged( + WindowMessageMonitorEventArgs e, + out List unchangedMonitors, + out List removedMonitors, + out List addedMonitors + ) + { + using Lock _ = new(_lockObj); Logger.Debug($"Monitors changed: {e.MessagePayload}"); // Get the new monitors. IMonitor[] previousMonitors = _monitors; _monitors = GetCurrentMonitors(); - List unchangedMonitors = new(); - List removedMonitors = new(); - List addedMonitors = new(); - + unchangedMonitors = new(); + removedMonitors = new(); + addedMonitors = new(); HashSet previousMonitorsSet = new(previousMonitors); HashSet currentMonitorsSet = new(_monitors); @@ -183,21 +216,6 @@ private void WindowMessageMonitor_MonitorsChanged(object? sender, WindowMessageM PrimaryMonitor = monitor; } } - - // Notify listeners of the unchanged, removed, and added monitors. - MonitorsChangedEventArgs args = - new() - { - UnchangedMonitors = unchangedMonitors, - RemovedMonitors = removedMonitors, - AddedMonitors = addedMonitors - }; - - if (addedMonitors.Count != 0 || removedMonitors.Count != 0) - { - _internalContext.ButlerEventHandlers.OnMonitorsChanged(args); - } - MonitorsChanged?.Invoke(this, args); } private void MouseHook_MouseLeftButtonUp(object? sender, MouseEventArgs e) @@ -267,6 +285,7 @@ private unsafe Monitor[] GetCurrentMonitors() public IMonitor GetMonitorAtPoint(IPoint point) { + using Lock _ = new(_lockObj); Logger.Debug($"Getting monitor at point {point}"); HMONITOR hmonitor = _internalContext.CoreNativeManager.MonitorFromPoint( point.ToSystemPoint(), @@ -285,6 +304,7 @@ public IMonitor GetMonitorAtPoint(IPoint point) public IMonitor GetPreviousMonitor(IMonitor monitor) { + using Lock _ = new(_lockObj); Logger.Debug($"Getting previous monitor for {monitor}"); int index = Array.IndexOf(_monitors, monitor); @@ -299,6 +319,7 @@ public IMonitor GetPreviousMonitor(IMonitor monitor) public IMonitor GetNextMonitor(IMonitor monitor) { + using Lock _ = new(_lockObj); Logger.Debug($"Getting next monitor for {monitor}"); int index = Array.IndexOf(_monitors, monitor); From 3d61a5714bf7f747538627f374bac5ebea3b80a6 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Thu, 21 Mar 2024 21:40:35 +1100 Subject: [PATCH 19/31] Remove event call from WindowManager lock --- src/Whim/Window/WindowManager.cs | 110 +++++++++++++++---------------- 1 file changed, 53 insertions(+), 57 deletions(-) diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index ee322497c..8381ed17f 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -255,36 +255,31 @@ uint _dwmsEventTime return; } + // Try get the window bool windowWasCreated = false; - IWindow? window = null; - - using (Lock _ = new(_lockObj)) + if (!_windows.TryGetValue(hwnd, out IWindow? window)) { - if (!_windows.TryGetValue(hwnd, out window)) + Logger.Verbose($"Window {hwnd} is not added, event type 0x{eventType:X4}"); + windowWasCreated = true; + window = AddWindow(hwnd); + + if ( + window == null + && (eventType == PInvoke.EVENT_SYSTEM_FOREGROUND || eventType == PInvoke.EVENT_OBJECT_UNCLOAKED) + ) { - Logger.Verbose($"Window {hwnd} is not added, event type 0x{eventType:X4}"); - windowWasCreated = true; - window = AddWindow(hwnd); + // Even if the window was ignored, we need to fire OnWindowFocused. + Logger.Debug( + $"Window {hwnd} with event 0x{eventType:X4} was ignored, but still notifying listeners of focus" + ); + OnWindowFocused(window); + return; } - } - - if ( - windowWasCreated - && window == null - && (eventType == PInvoke.EVENT_SYSTEM_FOREGROUND || eventType == PInvoke.EVENT_OBJECT_UNCLOAKED) - ) - { - // Even if the window was ignored, we need to fire OnWindowFocused. - Logger.Debug( - $"Window {hwnd} with event 0x{eventType:X4} was ignored, but still notifying listeners of focus" - ); - OnWindowFocused(window); - return; - } - if (window == null) - { - return; + if (window == null) + { + return; + } } if (_internalContext.ButlerEventHandlers.AreMonitorsChanging && windowWasCreated == false) @@ -333,44 +328,45 @@ uint _dwmsEventTime public IWindow? AddWindow(HWND hwnd) { - Logger.Debug($"Adding window {hwnd}"); - - if (_internalContext.CoreNativeManager.IsSplashScreen(hwnd)) + IWindow? window; + using (Lock _ = new(_lockObj)) { - Logger.Verbose($"Window {hwnd} is a splash screen, ignoring"); - return null; - } + Logger.Debug($"Adding window {hwnd}"); - if (_internalContext.CoreNativeManager.IsCloakedWindow(hwnd)) - { - Logger.Verbose($"Window {hwnd} is cloaked, ignoring"); - return null; - } + if (_internalContext.CoreNativeManager.IsSplashScreen(hwnd)) + { + Logger.Verbose($"Window {hwnd} is a splash screen, ignoring"); + return null; + } - if (!_internalContext.CoreNativeManager.IsStandardWindow(hwnd)) - { - Logger.Verbose($"Window {hwnd} is not a standard window, ignoring"); - return null; - } + if (_internalContext.CoreNativeManager.IsCloakedWindow(hwnd)) + { + Logger.Verbose($"Window {hwnd} is cloaked, ignoring"); + return null; + } - if (!_internalContext.CoreNativeManager.HasNoVisibleOwner(hwnd)) - { - Logger.Verbose($"Window {hwnd} has a visible owner, ignoring"); - return null; - } + if (!_internalContext.CoreNativeManager.IsStandardWindow(hwnd)) + { + Logger.Verbose($"Window {hwnd} is not a standard window, ignoring"); + return null; + } - IWindow? window = CreateWindow(hwnd); - if (window == null) - { - return null; - } - if (_context.FilterManager.ShouldBeIgnored(window)) - { - return null; - } + if (!_internalContext.CoreNativeManager.HasNoVisibleOwner(hwnd)) + { + Logger.Verbose($"Window {hwnd} has a visible owner, ignoring"); + return null; + } + + window = CreateWindow(hwnd); + if (window == null) + { + return null; + } + if (_context.FilterManager.ShouldBeIgnored(window)) + { + return null; + } - using (Lock _ = new(_lockObj)) - { _windows[hwnd] = window; } From f835b48de6b0cd658c136c14b98528c1f6819261 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 23 Mar 2024 16:21:55 +1100 Subject: [PATCH 20/31] Added new method to await a callback on the STA thread --- src/Whim/Native/INativeManager.cs | 10 +++++++++- src/Whim/Native/NativeManager.cs | 12 ++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Whim/Native/INativeManager.cs b/src/Whim/Native/INativeManager.cs index cef59885e..a0555381e 100644 --- a/src/Whim/Native/INativeManager.cs +++ b/src/Whim/Native/INativeManager.cs @@ -160,13 +160,21 @@ void SetWindowCorners( void RemoveWindowExTransparent(HWND hwnd); /// - /// Adds a task to the which will be executed on the thread associated + /// Adds a callback to the which will be executed on the thread associated /// with the . /// /// The task to execute. /// indicates that the task was added to the queue; , otherwise. + [Obsolete("Method is deprecated, use InvokeOnUIThread instead")] bool TryEnqueue(DispatcherQueueHandler callback); + /// + /// Adds a to the to run on the UI (STA) thread. + /// + /// The task to execute. + /// A task indicating the task's completion status. + Task InvokeOnUIThread(DispatcherQueueHandler callback); + /// /// Gets the version of Whim. /// diff --git a/src/Whim/Native/NativeManager.cs b/src/Whim/Native/NativeManager.cs index 8b624edb7..ee99c6c34 100644 --- a/src/Whim/Native/NativeManager.cs +++ b/src/Whim/Native/NativeManager.cs @@ -304,6 +304,18 @@ public void RemoveWindowExTransparent(HWND hwnd) public bool TryEnqueue(Microsoft.UI.Dispatching.DispatcherQueueHandler callback) => _dispatcherQueue.TryEnqueue(callback); + public Task InvokeOnUIThread(Microsoft.UI.Dispatching.DispatcherQueueHandler callback) + { + TaskCompletionSource tcs = new(); + bool isQueued = _dispatcherQueue.TryEnqueue(() => + { + callback(); + tcs.SetResult(true); + }); + + return tcs.Task; + } + public string GetWhimVersion() { #if DEBUG From 0cf8735f59e19771e0a378351e065edbb5bee2a8 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Thu, 28 Mar 2024 16:12:56 +1100 Subject: [PATCH 21/31] Added `ISubscriber` --- src/Whim/Context/ISubscriber.cs | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 src/Whim/Context/ISubscriber.cs diff --git a/src/Whim/Context/ISubscriber.cs b/src/Whim/Context/ISubscriber.cs new file mode 100644 index 000000000..bf0329c1e --- /dev/null +++ b/src/Whim/Context/ISubscriber.cs @@ -0,0 +1,7 @@ +internal interface ISubscriber +{ + /// + /// Subscribe to events. This must be called on the STA thread. + /// + void Subscribe(); +} From 9af38dc290bee1f2048fabf07e57c1b91abd2040 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 16:52:52 +1100 Subject: [PATCH 22/31] Implement `ISubscriber` for all relevant `Manager` classes --- src/Whim/Context/Context.cs | 5 ----- src/Whim/Context/IInternalContext.cs | 2 ++ src/Whim/Context/ISubscriber.cs | 6 ++++++ src/Whim/Context/InternalContext.cs | 11 +++++++++++ src/Whim/Monitor/IInternalMonitorManager.cs | 2 +- src/Whim/Monitor/IMonitorManager.cs | 5 ----- src/Whim/Monitor/MonitorManager.cs | 2 +- src/Whim/Notification/INotificationManager.cs | 5 ----- src/Whim/Notification/NotificationManager.cs | 4 ++-- src/Whim/Window/IInternalWindowManager.cs | 2 +- src/Whim/Window/IWindowManager.cs | 11 ----------- src/Whim/Window/WindowManager.cs | 11 ++++------- 12 files changed, 28 insertions(+), 38 deletions(-) diff --git a/src/Whim/Context/Context.cs b/src/Whim/Context/Context.cs index a8674a38f..e8e3d6a1a 100644 --- a/src/Whim/Context/Context.cs +++ b/src/Whim/Context/Context.cs @@ -93,14 +93,9 @@ public void Initialize() _internalContext.PreInitialize(); PluginManager.PreInitialize(); - NotificationManager.Initialize(); - MonitorManager.Initialize(); - WindowManager.Initialize(); WorkspaceManager.Initialize(); - Butler.Initialize(); - WindowManager.PostInitialize(); PluginManager.PostInitialize(); _internalContext.PostInitialize(); diff --git a/src/Whim/Context/IInternalContext.cs b/src/Whim/Context/IInternalContext.cs index 32eb50773..703d5920b 100644 --- a/src/Whim/Context/IInternalContext.cs +++ b/src/Whim/Context/IInternalContext.cs @@ -30,6 +30,8 @@ internal interface IInternalContext : IDisposable IDeferWorkspacePosManager DeferWorkspacePosManager { get; } + ISubscriber NotificationManager { get; } + void PreInitialize(); void PostInitialize(); diff --git a/src/Whim/Context/ISubscriber.cs b/src/Whim/Context/ISubscriber.cs index bf0329c1e..c43de769b 100644 --- a/src/Whim/Context/ISubscriber.cs +++ b/src/Whim/Context/ISubscriber.cs @@ -1,3 +1,9 @@ +namespace Whim; + +/// +/// A Whim object which subscribes to events. The method must be +/// called on the STA thread. +/// internal interface ISubscriber { /// diff --git a/src/Whim/Context/InternalContext.cs b/src/Whim/Context/InternalContext.cs index 9a344adf2..c393de97c 100644 --- a/src/Whim/Context/InternalContext.cs +++ b/src/Whim/Context/InternalContext.cs @@ -30,6 +30,8 @@ internal class InternalContext : IInternalContext public IButlerEventHandlers ButlerEventHandlers => ((Butler)_context.Butler).EventHandlers; + public ISubscriber NotificationManager => (ISubscriber)_context.NotificationManager; + public InternalContext(IContext context) { _context = context; @@ -52,6 +54,15 @@ public void PostInitialize() CoreSavedStateManager.PostInitialize(); KeybindHook.PostInitialize(); MouseHook.PostInitialize(); + + _context.NativeManager.InvokeOnUIThread(Subscribe); + } + + private void Subscribe() + { + MonitorManager.Subscribe(); + NotificationManager.Subscribe(); + WindowManager.Subscribe(); } protected virtual void Dispose(bool disposing) diff --git a/src/Whim/Monitor/IInternalMonitorManager.cs b/src/Whim/Monitor/IInternalMonitorManager.cs index 9a69b1f23..4e4e74c59 100644 --- a/src/Whim/Monitor/IInternalMonitorManager.cs +++ b/src/Whim/Monitor/IInternalMonitorManager.cs @@ -2,7 +2,7 @@ namespace Whim; -internal interface IInternalMonitorManager +internal interface IInternalMonitorManager : ISubscriber { /// /// The last which received an event sent by Windows which Whim did not ignore. diff --git a/src/Whim/Monitor/IMonitorManager.cs b/src/Whim/Monitor/IMonitorManager.cs index d1400c9ba..b72a07e8e 100644 --- a/src/Whim/Monitor/IMonitorManager.cs +++ b/src/Whim/Monitor/IMonitorManager.cs @@ -23,11 +23,6 @@ public interface IMonitorManager : IEnumerable, IDisposable /// IMonitor PrimaryMonitor { get; } - /// - /// Initialize the windows event hooks. - /// - void Initialize(); - /// /// Returns the at the given x and y coordinates. /// diff --git a/src/Whim/Monitor/MonitorManager.cs b/src/Whim/Monitor/MonitorManager.cs index 3d28f0280..7b6f6dc9c 100644 --- a/src/Whim/Monitor/MonitorManager.cs +++ b/src/Whim/Monitor/MonitorManager.cs @@ -64,7 +64,7 @@ public MonitorManager(IContext context, IInternalContext internalContext) LastWhimActiveMonitor = primaryMonitor; } - public void Initialize() + public void Subscribe() { _internalContext.WindowMessageMonitor.DisplayChanged += WindowMessageMonitor_MonitorsChanged; _internalContext.WindowMessageMonitor.WorkAreaChanged += WindowMessageMonitor_MonitorsChanged; diff --git a/src/Whim/Notification/INotificationManager.cs b/src/Whim/Notification/INotificationManager.cs index 0b3224265..3f3cf369a 100644 --- a/src/Whim/Notification/INotificationManager.cs +++ b/src/Whim/Notification/INotificationManager.cs @@ -13,11 +13,6 @@ public interface INotificationManager : IDisposable /// public const string NotificationIdKey = "WHIM_NOTIFICATION_ID"; - /// - /// Initialize the notification manager. - /// - void Initialize(); - /// /// Register a notification handler for a given notification id. /// diff --git a/src/Whim/Notification/NotificationManager.cs b/src/Whim/Notification/NotificationManager.cs index ed855ddc3..d65421c24 100644 --- a/src/Whim/Notification/NotificationManager.cs +++ b/src/Whim/Notification/NotificationManager.cs @@ -4,7 +4,7 @@ namespace Whim; -internal class NotificationManager : INotificationManager +internal class NotificationManager : INotificationManager, ISubscriber { private bool _disposedValue; private bool _initialized; @@ -17,7 +17,7 @@ public NotificationManager(IContext context) _context = context; } - public void Initialize() + public void Subscribe() { // To ensure all Notification handling happens in this process instance, register for // NotificationInvoked before calling Register(). Without this a new process will diff --git a/src/Whim/Window/IInternalWindowManager.cs b/src/Whim/Window/IInternalWindowManager.cs index 48eb3d2c1..04b018656 100644 --- a/src/Whim/Window/IInternalWindowManager.cs +++ b/src/Whim/Window/IInternalWindowManager.cs @@ -3,7 +3,7 @@ namespace Whim; -internal interface IInternalWindowManager +internal interface IInternalWindowManager : ISubscriber { /// /// Add the given as an inside this diff --git a/src/Whim/Window/IWindowManager.cs b/src/Whim/Window/IWindowManager.cs index ca5dd77cf..ff5c559fa 100644 --- a/src/Whim/Window/IWindowManager.cs +++ b/src/Whim/Window/IWindowManager.cs @@ -14,17 +14,6 @@ public interface IWindowManager : IEnumerable, IDisposable /// IFilterManager LocationRestoringFilterManager { get; } - /// - /// Initialize the windows event hooks. - /// - /// - void Initialize(); - - /// - /// Add the top-level windows. - /// - void PostInitialize(); - /// /// Creates a new window. If the window cannot be created, is returned. /// This will try reuse existing s if possible. diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index 8381ed17f..afad73037 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -65,9 +65,9 @@ public WindowManager(IContext context, IInternalContext internalContext) DefaultFilteredWindows.LoadLocationRestoringWindows(LocationRestoringFilterManager); } - public void Initialize() + public void Subscribe() { - Logger.Debug("Initializing window manager..."); + Logger.Debug("Start subscriptions"); // Each of the following hooks add just one or two event constants from https://docs.microsoft.com/en-us/windows/win32/winauto/event-constants _addedHooks[0] = _internalContext.CoreNativeManager.SetWinEventHook( @@ -110,14 +110,11 @@ public void Initialize() throw new InvalidOperationException($"Failed to add hook {i}"); } } - } - - public void PostInitialize() - { - Logger.Debug("Post-initializing window manager..."); _internalContext.MouseHook.MouseLeftButtonDown += MouseHook_MouseLeftButtonDown; _internalContext.MouseHook.MouseLeftButtonUp += MouseHook_MouseLeftButtonUp; + + Logger.Debug("Completed subscriptions"); } public IWindow? CreateWindow(HWND hwnd) From 217012e5942fbf7100261d96e0485009889f04ca Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 17:02:26 +1100 Subject: [PATCH 23/31] Fix tests which refer to Subscribe --- src/Whim.Tests/Monitor/MonitorManagerTests.cs | 38 +++---- src/Whim.Tests/Window/WindowManagerTests.cs | 104 +++++++++--------- 2 files changed, 71 insertions(+), 71 deletions(-) diff --git a/src/Whim.Tests/Monitor/MonitorManagerTests.cs b/src/Whim.Tests/Monitor/MonitorManagerTests.cs index 70ee35f4c..59850dc3a 100644 --- a/src/Whim.Tests/Monitor/MonitorManagerTests.cs +++ b/src/Whim.Tests/Monitor/MonitorManagerTests.cs @@ -148,13 +148,13 @@ internal void Create_NoPrimaryMonitorFound(IContext ctx, IInternalContext intern [Theory, AutoSubstituteData] [SuppressMessage("Usage", "NS5000:Received check.")] - internal void Initialize(IContext ctx, IInternalContext internalCtx) + internal void Subscribe(IContext ctx, IInternalContext internalCtx) { // Given MonitorManager monitorManager = new(ctx, internalCtx); // When - monitorManager.Initialize(); + monitorManager.Subscribe(); // Then internalCtx.WindowMessageMonitor.Received(1).DisplayChanged += Arg.Any< @@ -306,7 +306,7 @@ IInternalContext internalCtx // Given // Populate the monitor manager with the default two monitors MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // Set up the monitor manager to be given a new monitor RECT right = @@ -393,7 +393,7 @@ IInternalContext internalCtx MonitorManagerCustomization.UpdateGetCurrentMonitors(internalCtx, new[] { (primaryRect, (HMONITOR)1) }); MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // Set up the monitor manager to be given a new monitor RECT right = @@ -453,7 +453,7 @@ IInternalContext internalCtx // Given // Populate the monitor manager with the default two monitors MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // Set up the monitor manager to have only one monitor RECT left = @@ -509,7 +509,7 @@ internal void WindowMessageMonitor_DisplayChanged_NoChange(IContext ctx, IIntern // Given // Populate the monitor manager with the default two monitors MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // Set up the monitor manager to have the two monitors MonitorManagerCustomization.UpdateMultipleMonitors( @@ -564,7 +564,7 @@ internal void WindowMessageMonitor_WorkAreaChanged(IContext ctx, IInternalContex { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When var raisedEvent = Assert.Raises( @@ -585,7 +585,7 @@ internal void WindowMessageMonitor_DpiChanged(IContext ctx, IInternalContext int { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When var raisedEvent = Assert.Raises( @@ -607,7 +607,7 @@ internal void WindowMessageMonitor_SessionChanged(IContext ctx, IInternalContext { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When internalCtx.WindowMessageMonitor.SessionChanged += Raise.Event>( @@ -630,7 +630,7 @@ internal void GetMonitorAtPoint_Error_ReturnsFirstMonitor(IContext ctx, IInterna .Returns((HMONITOR)0); MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetMonitorAtPoint(point); @@ -650,7 +650,7 @@ internal void GetMonitorAtPoint_MultipleMonitors_ReturnsCorrectMonitor(IContext .Returns((HMONITOR)1); MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetMonitorAtPoint(point); @@ -664,7 +664,7 @@ internal void GetPreviousMonitor_Error_ReturnsFirstMonitor(IContext ctx, IIntern { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetPreviousMonitor(Substitute.For()); @@ -678,7 +678,7 @@ internal void GetPreviousMonitor_MultipleMonitors_ReturnsCorrectMonitor(IContext { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetPreviousMonitor(monitorManager.ElementAt(1)); @@ -692,7 +692,7 @@ internal void GetPreviousMonitor_MultipleMonitors_Mod(IContext ctx, IInternalCon { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetPreviousMonitor(monitorManager.ElementAt(0)); @@ -706,7 +706,7 @@ internal void GetNextMonitor_Error_ReturnsFirstMonitor(IContext ctx, IInternalCo { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetNextMonitor(Substitute.For()); @@ -720,7 +720,7 @@ internal void GetNextMonitor_MultipleMonitors_ReturnsCorrectMonitor(IContext ctx { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetNextMonitor(monitorManager.First()); @@ -734,7 +734,7 @@ internal void GetNextMonitor_MultipleMonitors_Mod(IContext ctx, IInternalContext { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When IMonitor monitor = monitorManager.GetNextMonitor(monitorManager.ElementAt(1)); @@ -749,7 +749,7 @@ internal void Dispose(IContext ctx, IInternalContext internalCtx) { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); // When monitorManager.Dispose(); @@ -787,7 +787,7 @@ internal void MouseHook_MouseLeftButtonUp(IContext ctx, IInternalContext interna { // Given MonitorManager monitorManager = new(ctx, internalCtx); - monitorManager.Initialize(); + monitorManager.Subscribe(); IMonitor monitor = monitorManager.ActiveMonitor; IMonitor? monitor2 = monitorManager.ElementAt(1); diff --git a/src/Whim.Tests/Window/WindowManagerTests.cs b/src/Whim.Tests/Window/WindowManagerTests.cs index 91f93b3ac..2e1e2030c 100644 --- a/src/Whim.Tests/Window/WindowManagerTests.cs +++ b/src/Whim.Tests/Window/WindowManagerTests.cs @@ -230,7 +230,7 @@ internal void OnWindowMinimizeStart(IContext ctx, IInternalContext internalCtx) CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx); AllowWindowCreation(ctx, internalCtx, hwnd); - windowManager.Initialize(); + windowManager.Subscribe(); // When var result = Assert.Raises( @@ -252,7 +252,7 @@ internal void OnWindowMinimizeEnd(IContext ctx, IInternalContext internalCtx) CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx); AllowWindowCreation(ctx, internalCtx, hwnd); - windowManager.Initialize(); + windowManager.Subscribe(); // When var result = Assert.Raises( @@ -265,7 +265,7 @@ internal void OnWindowMinimizeEnd(IContext ctx, IInternalContext internalCtx) Assert.Equal((int)_processId, result.Arguments.Window.ProcessId); } - private static void InitializeCoreNativeManagerMock(IInternalContext internalCtx) + private static void SubscribeCoreNativeManagerMock(IInternalContext internalCtx) { (uint, uint)[] events = new[] { @@ -286,15 +286,15 @@ private static void InitializeCoreNativeManagerMock(IInternalContext internalCtx } [Theory, AutoSubstituteData] - internal void Initialize(IContext ctx, IInternalContext internalCtx) + internal void Subscribe(IContext ctx, IInternalContext internalCtx) { // Given - InitializeCoreNativeManagerMock(internalCtx); + SubscribeCoreNativeManagerMock(internalCtx); WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then internalCtx @@ -303,10 +303,10 @@ internal void Initialize(IContext ctx, IInternalContext internalCtx) } [Theory, AutoSubstituteData] - internal void Initialize_Fail(IContext ctx, IInternalContext internalCtx) + internal void Subscribe_Fail(IContext ctx, IInternalContext internalCtx) { // Given - InitializeCoreNativeManagerMock(internalCtx); + SubscribeCoreNativeManagerMock(internalCtx); internalCtx .CoreNativeManager.SetWinEventHook( @@ -320,7 +320,7 @@ internal void Initialize_Fail(IContext ctx, IInternalContext internalCtx) // When // Then - Assert.Throws(windowManager.Initialize); + Assert.Throws(windowManager.Subscribe); } [InlineAutoSubstituteData(PInvoke.CHILDID_SELF + 1, 0, 0)] @@ -342,7 +342,7 @@ IInternalContext internalCtx WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then CustomAssert.DoesNotRaise( @@ -387,7 +387,7 @@ IInternalContext internalCtx WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_SHOW, hwnd, 0, 0, 0, 0); // Then @@ -409,7 +409,7 @@ internal void WinEventProc_AddWindow_CreateWindowNull(IContext ctx, IInternalCon WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); CustomAssert.DoesNotRaise( h => windowManager.WindowAdded += h, h => windowManager.WindowAdded -= h, @@ -433,7 +433,7 @@ internal void WinEventProc_AddWindow_IgnoreWindow(IContext ctx, IInternalContext WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); CustomAssert.DoesNotRaise( h => windowManager.WindowAdded += h, @@ -458,7 +458,7 @@ internal void WinEventProc_AddWindow_WindowIsMinimized(IContext ctx, IInternalCo WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowAdded += h, h => windowManager.WindowAdded -= h, @@ -485,7 +485,7 @@ internal void WinEventProc_MonitorsAreChanging_NewWindow(IContext ctx, IInternal internalCtx.ButlerEventHandlers.AreMonitorsChanging.Returns(true); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowAdded += h, h => windowManager.WindowAdded -= h, @@ -510,7 +510,7 @@ internal void WinEventProc_MonitorsAreNotChanging_OldWindow(IContext ctx, IInter internalCtx.ButlerEventHandlers.AreMonitorsChanging.Returns(false); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_FOREGROUND, hwnd, 0, 0, 0, 0); CustomAssert.DoesNotRaise( h => windowManager.WindowAdded += h, @@ -536,7 +536,7 @@ internal void WinEventProc_MonitorsAreChanging_OldWindow(IContext ctx, IInternal internalCtx.ButlerEventHandlers.AreMonitorsChanging.Returns(true); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_FOREGROUND, hwnd, 0, 0, 0, 0); CustomAssert.DoesNotRaise( h => windowManager.WindowAdded += h, @@ -562,7 +562,7 @@ internal void WinEventProc_OnWindowFocused(uint eventType, IContext ctx, IIntern WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowFocused += h, h => windowManager.WindowFocused -= h, @@ -588,7 +588,7 @@ internal void WinEventProc_OnWindowFocused_IgnoredWindow(uint eventType, IContex WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then Assert.Raises( @@ -614,7 +614,7 @@ internal void WinEventProc_OnWindowHidden_IgnoreWindow(IContext ctx, IInternalCo ctx.Butler.Pantry.GetMonitorForWindow(Arg.Any()).Returns((IMonitor?)null); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_HIDE, hwnd, 0, 0, 0, 0); // Then @@ -632,7 +632,7 @@ internal void WinEventProc_OnWindowHidden(IContext ctx, IInternalContext interna WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then Assert.Raises( @@ -655,7 +655,7 @@ internal void WinEventProc_OnWindowRemoved(uint eventType, IContext ctx, IIntern WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowRemoved += h, h => windowManager.WindowRemoved -= h, @@ -689,7 +689,7 @@ internal void WinEventProc_OnWindowMoveStart(IContext ctx, IInternalContext inte WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then var result = Assert.Raises( @@ -710,12 +710,12 @@ internal void WinEventProc_OnWindowMoveStart_GetCursorPos(IContext ctx, IInterna AllowWindowCreation(ctx, internalCtx, hwnd); WindowManager windowManager = new(ctx, internalCtx); - windowManager.PostInitialize(); + windowManager.Subscribe(); Trigger_MouseLeftButtonDown(internalCtx).Setup_GetCursorPos(internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); var result = Assert.Raises( h => windowManager.WindowMoveStart += h, h => windowManager.WindowMoveStart -= h, @@ -765,12 +765,12 @@ IPoint _pixelsDelta ctx.NativeManager.DwmGetWindowRectangle(Arg.Any()).Returns(newRect); WindowManager windowManager = new(ctx, internalCtx); - windowManager.PostInitialize(); + windowManager.Subscribe(); Trigger_MouseLeftButtonDown(internalCtx).Setup_GetCursorPos(internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); var result = Assert.Raises( h => windowManager.WindowMoveStart += h, h => windowManager.WindowMoveStart -= h, @@ -794,7 +794,7 @@ IInternalContext internalCtx CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx); AllowWindowCreation(ctx, internalCtx, hwnd); WindowManager windowManager = new(ctx, internalCtx) { WindowMovedDelay = 0 }; - windowManager.Initialize(); + windowManager.Subscribe(); return (capture, windowManager, hwnd); } @@ -946,7 +946,7 @@ internal void WinEventProc_OnWindowMoved_DoesNotRaise(IContext ctx, IInternalCon WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); // Then CustomAssert.DoesNotRaise( @@ -967,8 +967,8 @@ internal void WinEventProc_OnWindowMoved_DoesNotRaise_MouseIsUp(IContext ctx, II WindowManager windowManager = new(ctx, internalCtx); // When the window is moved - windowManager.Initialize(); - windowManager.PostInitialize(); + windowManager.Subscribe(); + windowManager.Subscribe(); Trigger_MouseLeftButtonDown(internalCtx).Trigger_MouseLeftButtonUp(internalCtx); @@ -990,12 +990,12 @@ internal void WinEventProc_OnWindowMoved_GetCursorPos(IContext ctx, IInternalCon internalCtx.CoreNativeManager.IsWindowMinimized(hwnd).Returns((BOOL)false); WindowManager windowManager = new(ctx, internalCtx); - windowManager.PostInitialize(); + windowManager.Subscribe(); Trigger_MouseLeftButtonDown(internalCtx).Setup_GetCursorPos(internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); var result = Assert.Raises( h => windowManager.WindowMoved += h, @@ -1036,7 +1036,7 @@ internal void WinEventProc_OnWindowMoved(IContext ctx, IInternalContext internal WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); var result = Assert.Raises( h => windowManager.WindowMoved += h, @@ -1061,7 +1061,7 @@ internal void WinEventProc_OnWindowMinimizeStart(IContext ctx, IInternalContext WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowMinimizeStart += h, h => windowManager.WindowMinimizeStart -= h, @@ -1082,7 +1082,7 @@ internal void WinEventProc_OnWindowMinimizeEnd(IContext ctx, IInternalContext in WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); Assert.Raises( h => windowManager.WindowMinimizeEnd += h, h => windowManager.WindowMinimizeEnd -= h, @@ -1104,7 +1104,7 @@ internal void WinEventProc_OnWindowMoveEnd_WindowNotMoving(IContext ctx, IIntern WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); // Then @@ -1124,7 +1124,7 @@ internal void WinEventProc_OnWindowMoveEnd_GetMovedEdges_NoWorkspace(IContext ct WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); @@ -1152,7 +1152,7 @@ IWorkspace workspace WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); var result = Assert.Raises( h => windowManager.WindowMoveEnd += h, h => windowManager.WindowMoveEnd -= h, @@ -1195,7 +1195,7 @@ IWorkspace workspace h => windowManager.WindowMoveEnd -= h, () => { - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); } @@ -1238,7 +1238,7 @@ IWorkspace workspace WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); @@ -1292,7 +1292,7 @@ IWorkspace workspace WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); @@ -1415,7 +1415,7 @@ IPoint pixelsDelta WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); // Then @@ -1475,7 +1475,7 @@ IWorkspace workspace WindowManager windowManager = new(ctx, internalCtx); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); // Then @@ -1498,7 +1498,7 @@ internal void WinEventProc_InvalidEvent(IContext ctx, IInternalContext internalC WindowManagerSubscriber subscriber = new(windowManager); // When - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, 0xBAADF00D, hwnd, 0, 0, 0, 0); // Then @@ -1519,7 +1519,7 @@ internal void WinEventProc_InvalidEvent(IContext ctx, IInternalContext internalC #region Dispose [Theory, AutoSubstituteData] - internal void Dispose_NotInitialized(IContext ctx, IInternalContext internalCtx) + internal void Dispose_NotSubscribed(IContext ctx, IInternalContext internalCtx) { // Given CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx); @@ -1539,7 +1539,7 @@ internal void Dispose_IsClosed(IContext ctx, IInternalContext internalCtx) CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx, false, true); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); capture.Handles.ForEach(h => h.HasDisposed = false); @@ -1557,7 +1557,7 @@ internal void Dispose_IsInvalid(IContext ctx, IInternalContext internalCtx) CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx, false, true); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); capture.Handles.ForEach(h => { @@ -1579,7 +1579,7 @@ internal void Dispose_Success(IContext ctx, IInternalContext internalCtx) CaptureWinEventProc capture = CaptureWinEventProc.Create(internalCtx); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); // When windowManager.Dispose(); @@ -1601,7 +1601,7 @@ internal void GetEnumerator(IContext ctx, IInternalContext internalCtx) AllowWindowCreation(ctx, internalCtx, hwnd); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_SHOW, hwnd, 0, 0, 0, 0); // When @@ -1620,7 +1620,7 @@ internal void IEnumerable_GetEnumerator(IContext ctx, IInternalContext internalC AllowWindowCreation(ctx, internalCtx, hwnd); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_SHOW, hwnd, 0, 0, 0, 0); // When @@ -1645,7 +1645,7 @@ internal void HandleException(IContext ctx, IInternalContext internalCtx) AllowWindowCreation(ctx, internalCtx, hwnd); WindowManager windowManager = new(ctx, internalCtx); - windowManager.Initialize(); + windowManager.Subscribe(); // When internalCtx From e1f98d6a72185ce5be8dd25fe526e73fc044538e Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 21:56:15 +1100 Subject: [PATCH 24/31] Added `ThreadSafeEvent`, and used it in `Butler` --- src/Whim/Butler/Butler.cs | 21 ++++- src/Whim/{Context => Utils}/ISubscriber.cs | 0 src/Whim/Utils/ThreadSafeEvent.cs | 94 ++++++++++++++++++++++ 3 files changed, 111 insertions(+), 4 deletions(-) rename src/Whim/{Context => Utils}/ISubscriber.cs (100%) create mode 100644 src/Whim/Utils/ThreadSafeEvent.cs diff --git a/src/Whim/Butler/Butler.cs b/src/Whim/Butler/Butler.cs index 8a488370a..212f9e92f 100644 --- a/src/Whim/Butler/Butler.cs +++ b/src/Whim/Butler/Butler.cs @@ -37,16 +37,29 @@ public Butler(IContext context, IInternalContext internalContext) _pantry = new ButlerPantry(_context); _chores = new ButlerChores(_context, _internalContext); EventHandlers = new ButlerEventHandlers(_context, _internalContext, _pantry, _chores); + + _windowRoutedEvent = new(context); + _monitorWorkspaceChangedEvent = new(context); } - public event EventHandler? WindowRouted; + private readonly ThreadSafeEvent _windowRoutedEvent; + public event EventHandler? WindowRouted + { + add => _windowRoutedEvent.Add(value); + remove => _windowRoutedEvent.Remove(value); + } - public event EventHandler? MonitorWorkspaceChanged; + private readonly ThreadSafeEvent _monitorWorkspaceChangedEvent; + public event EventHandler? MonitorWorkspaceChanged + { + add => _monitorWorkspaceChangedEvent.Add(value); + remove => _monitorWorkspaceChangedEvent.Remove(value); + } - public void TriggerWindowRouted(RouteEventArgs args) => WindowRouted?.Invoke(this, args); + public void TriggerWindowRouted(RouteEventArgs args) => _windowRoutedEvent.Invoke(this, args); public void TriggerMonitorWorkspaceChanged(MonitorWorkspaceChangedEventArgs args) => - MonitorWorkspaceChanged?.Invoke(this, args); + _monitorWorkspaceChangedEvent?.Invoke(this, args); public void Initialize() { diff --git a/src/Whim/Context/ISubscriber.cs b/src/Whim/Utils/ISubscriber.cs similarity index 100% rename from src/Whim/Context/ISubscriber.cs rename to src/Whim/Utils/ISubscriber.cs diff --git a/src/Whim/Utils/ThreadSafeEvent.cs b/src/Whim/Utils/ThreadSafeEvent.cs new file mode 100644 index 000000000..c704c96a7 --- /dev/null +++ b/src/Whim/Utils/ThreadSafeEvent.cs @@ -0,0 +1,94 @@ +using System; +using System.Collections.Generic; +using System.Threading; + +namespace Whim; + +/// +/// Invokes event handlers more safely than normal event handlers. If the UI thread subscribes to an +/// event, then when the event is invoked the delegate is called on the UI thread. +/// Otherwise, the event is invocation continues on the current thread. +/// +public class ThreadSafeEvent +{ + private readonly IContext _context; + private readonly List<(EventHandler Handler, bool IsSTA)> _handlers = new(); + + /// + /// Creates an , with easy access to the . + /// + /// + public ThreadSafeEvent(IContext context) + { + _context = context; + } + + /// + /// Adds the given . + /// + /// + public void Add(EventHandler? handler) + { + if (handler is null) + { + return; + } + + ApartmentState state = Thread.CurrentThread.GetApartmentState(); + _handlers.Add((handler, state == ApartmentState.STA)); + } + + /// + /// Removes the first instance of . + /// + /// + /// + /// true if was found, otherwise false. + /// + public bool Remove(EventHandler? handler) + { + if (handler is null) + { + return false; + } + + int idxToRemove = -1; + for (int idx = 0; idx < _handlers.Count; idx++) + { + if (_handlers[idx].Handler == handler) + { + idxToRemove = idx; + break; + } + } + + if (idxToRemove != -1) + { + _handlers.RemoveAt(idxToRemove); + return true; + } + + return false; + } + + /// + /// Invoke the event. If the subscriber is from the STA thread, invoke on the UI thread. Otherwise, + /// continue on the current thread. + /// + /// + /// + public void Invoke(object sender, T args) + { + foreach ((EventHandler handler, bool isSta) in _handlers) + { + if (isSta) + { + _context.NativeManager.InvokeOnUIThread(() => handler.Invoke(sender, args)); + } + else + { + handler.Invoke(sender, args); + } + } + } +} From 13ca7fcf591544bb62bfaf435ac8350859d2a79c Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 22:20:56 +1100 Subject: [PATCH 25/31] Added `ThreadSafeEvent` to the other core events --- src/Whim/Butler/Butler.cs | 2 +- src/Whim/Context/Context.cs | 22 ++++-- src/Whim/Monitor/MonitorManager.cs | 13 +++- src/Whim/Utils/ThreadSafeEvent.cs | 2 + src/Whim/Window/WindowManager.cs | 92 +++++++++++++++++++++----- src/Whim/Workspace/WorkspaceManager.cs | 62 +++++++++++++---- 6 files changed, 155 insertions(+), 38 deletions(-) diff --git a/src/Whim/Butler/Butler.cs b/src/Whim/Butler/Butler.cs index 212f9e92f..19b26cd9f 100644 --- a/src/Whim/Butler/Butler.cs +++ b/src/Whim/Butler/Butler.cs @@ -59,7 +59,7 @@ public event EventHandler? MonitorWorkspaceCha public void TriggerWindowRouted(RouteEventArgs args) => _windowRoutedEvent.Invoke(this, args); public void TriggerMonitorWorkspaceChanged(MonitorWorkspaceChangedEventArgs args) => - _monitorWorkspaceChangedEvent?.Invoke(this, args); + _monitorWorkspaceChangedEvent.Invoke(this, args); public void Initialize() { diff --git a/src/Whim/Context/Context.cs b/src/Whim/Context/Context.cs index e8e3d6a1a..d9bd6c169 100644 --- a/src/Whim/Context/Context.cs +++ b/src/Whim/Context/Context.cs @@ -31,14 +31,28 @@ internal class Context : IContext public IKeybindManager KeybindManager { get; } public INotificationManager NotificationManager { get; } - public event EventHandler? Exiting; - public event EventHandler? Exited; + private readonly ThreadSafeEvent _exitingEvent; + public event EventHandler? Exiting + { + add => _exitingEvent.Add(value); + remove => _exitingEvent.Remove(value); + } + + private readonly ThreadSafeEvent _exitedEvent; + public event EventHandler? Exited + { + add => _exitedEvent.Add(value); + remove => _exitedEvent.Remove(value); + } /// /// Create a new . /// public Context() { + _exitingEvent = new(this); + _exitedEvent = new(this); + string[] args = Environment.GetCommandLineArgs(); FileManager = new FileManager(args); @@ -123,7 +137,7 @@ public void Exit(ExitEventArgs? args = null) Logger.Debug("Exiting context..."); args ??= new ExitEventArgs() { Reason = ExitReason.User }; - Exiting?.Invoke(this, args); + _exitingEvent.Invoke(this, args); PluginManager.Dispose(); WorkspaceManager.Dispose(); @@ -135,6 +149,6 @@ public void Exit(ExitEventArgs? args = null) Logger.Debug("Mostly exited..."); Logger.Dispose(); - Exited?.Invoke(this, args); + _exitedEvent.Invoke(this, args); } } diff --git a/src/Whim/Monitor/MonitorManager.cs b/src/Whim/Monitor/MonitorManager.cs index 7b6f6dc9c..e59d7c3a0 100644 --- a/src/Whim/Monitor/MonitorManager.cs +++ b/src/Whim/Monitor/MonitorManager.cs @@ -38,7 +38,12 @@ internal class MonitorManager : IInternalMonitorManager, IMonitorManager IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); - public event EventHandler? MonitorsChanged; + private readonly ThreadSafeEvent _monitorsChangedEvent; + public event EventHandler? MonitorsChanged + { + add => _monitorsChangedEvent.Add(value); + remove => _monitorsChangedEvent.Remove(value); + } /// /// Creates a new instance of . @@ -53,6 +58,8 @@ public MonitorManager(IContext context, IInternalContext internalContext) _context = context; _internalContext = internalContext; + _monitorsChangedEvent = new(context); + // Get the monitors. _monitors = GetCurrentMonitors(); @@ -138,7 +145,7 @@ private void WindowMessageMonitor_SessionChanged(object? sender, WindowMessageMo // If we update monitors too quickly, the reported working area can sometimes be the // monitor's bounds, which is incorrect. So, we wait a bit before updating the monitors. // This gives Windows some to figure out the correct working area. - _context.NativeManager.TryEnqueue(async () => + Task.Run(async () => { await Task.Delay(5000).ConfigureAwait(true); WindowMessageMonitor_MonitorsChanged(sender, e); @@ -167,7 +174,7 @@ out List addedMonitors { _internalContext.ButlerEventHandlers.OnMonitorsChanged(args); } - MonitorsChanged?.Invoke(this, args); + _monitorsChangedEvent.Invoke(this, args); } private void OnMonitorsChanged( diff --git a/src/Whim/Utils/ThreadSafeEvent.cs b/src/Whim/Utils/ThreadSafeEvent.cs index c704c96a7..4c17cb99b 100644 --- a/src/Whim/Utils/ThreadSafeEvent.cs +++ b/src/Whim/Utils/ThreadSafeEvent.cs @@ -8,6 +8,8 @@ namespace Whim; /// Invokes event handlers more safely than normal event handlers. If the UI thread subscribes to an /// event, then when the event is invoked the delegate is called on the UI thread. /// Otherwise, the event is invocation continues on the current thread. +/// +/// For Whim's core, only public events in public classes need to implement this. /// public class ThreadSafeEvent { diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index afad73037..97817ba72 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -16,14 +16,61 @@ internal class WindowManager : IWindowManager, IInternalWindowManager private readonly IContext _context; private readonly IInternalContext _internalContext; - public event EventHandler? WindowAdded; - public event EventHandler? WindowFocused; - public event EventHandler? WindowRemoved; - public event EventHandler? WindowMoveStart; - public event EventHandler? WindowMoved; - public event EventHandler? WindowMoveEnd; - public event EventHandler? WindowMinimizeStart; - public event EventHandler? WindowMinimizeEnd; + private readonly ThreadSafeEvent _windowAddedEvent; + public event EventHandler? WindowAdded + { + add => _windowAddedEvent.Equals(value); + remove => _windowAddedEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowFocusedEvent; + public event EventHandler? WindowFocused + { + add => _windowFocusedEvent.Equals(value); + remove => _windowFocusedEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowRemovedEvent; + public event EventHandler? WindowRemoved + { + add => _windowRemovedEvent.Equals(value); + remove => _windowRemovedEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowMoveStartEvent; + public event EventHandler? WindowMoveStart + { + add => _windowMoveStartEvent.Equals(value); + remove => _windowMoveStartEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowMovedEvent; + public event EventHandler? WindowMoved + { + add => _windowMovedEvent.Equals(value); + remove => _windowMovedEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowMoveEndEvent; + public event EventHandler? WindowMoveEnd + { + add => _windowMoveEndEvent.Equals(value); + remove => _windowMoveEndEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowMinimizeStartEvent; + public event EventHandler? WindowMinimizeStart + { + add => _windowMinimizeStartEvent.Equals(value); + remove => _windowMinimizeStartEvent.Remove(value); + } + + private readonly ThreadSafeEvent _windowMinimizeEndEvent; + public event EventHandler? WindowMinimizeEnd + { + add => _windowMinimizeEndEvent.Equals(value); + remove => _windowMinimizeEndEvent.Remove(value); + } private readonly ConcurrentDictionary _windows = new(); public IReadOnlyDictionary HandleWindowMap => _windows; @@ -62,6 +109,15 @@ public WindowManager(IContext context, IInternalContext internalContext) _internalContext = internalContext; _hookDelegate = new WINEVENTPROC(WinEventProcWrapper); + _windowAddedEvent = new(context); + _windowFocusedEvent = new(context); + _windowRemovedEvent = new(context); + _windowMoveStartEvent = new(context); + _windowMovedEvent = new(context); + _windowMoveEndEvent = new(context); + _windowMinimizeStartEvent = new(context); + _windowMinimizeEndEvent = new(context); + DefaultFilteredWindows.LoadLocationRestoringWindows(LocationRestoringFilterManager); } @@ -376,7 +432,7 @@ public void OnWindowAdded(IWindow window) { WindowEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowAdded(args); - WindowAdded?.Invoke(this, args); + _windowAddedEvent.Invoke(this, args); } public void OnWindowFocused(IWindow? window) @@ -386,7 +442,7 @@ public void OnWindowFocused(IWindow? window) WindowFocusedEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowFocused(args); - WindowFocused?.Invoke(this, args); + _windowFocusedEvent.Invoke(this, args); } /// @@ -421,7 +477,7 @@ public void OnWindowRemoved(IWindow window) WindowEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowRemoved(args); - WindowRemoved?.Invoke(this, args); + _windowRemovedEvent.Invoke(this, args); } private void OnWindowMoveStart(IWindow window) @@ -435,7 +491,7 @@ private void OnWindowMoveStart(IWindow window) cursorPoint = point; } - WindowMoveStart?.Invoke( + _windowMoveStartEvent.Invoke( this, new WindowMovedEventArgs() { @@ -462,7 +518,7 @@ private void OnWindowMoveEnd(IWindow window) return; } - if (GetMovedEdges(window) is (Direction MovedEdges, IPoint MovedPoint) moved) + if (GetMovedEdges(window) is (Direction, IPoint) moved) { movedEdges = moved.MovedEdges; _context.Butler.MoveWindowEdgesInDirection(moved.MovedEdges, moved.MovedPoint, window); @@ -474,7 +530,7 @@ private void OnWindowMoveEnd(IWindow window) _isMovingWindow = false; - WindowMoveEnd?.Invoke( + _windowMoveEndEvent.Invoke( this, new WindowMovedEventArgs() { @@ -574,7 +630,7 @@ private void OnWindowMoved(IWindow window) { // The window's application tried to restore its position. // Wait, then restore the position. - _context.NativeManager.TryEnqueue(async () => + Task.Run(async () => { await Task.Delay(WindowMovedDelay).ConfigureAwait(true); if (_context.Butler.Pantry.GetWorkspaceForWindow(window) is IWorkspace workspace) @@ -597,7 +653,7 @@ private void OnWindowMoved(IWindow window) cursorPoint = point; } - WindowMoved?.Invoke( + _windowMovedEvent.Invoke( this, new WindowMovedEventArgs() { @@ -614,7 +670,7 @@ private void OnWindowMinimizeStart(IWindow window) WindowEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowMinimizeStart(args); - WindowMinimizeStart?.Invoke(this, args); + _windowMinimizeStartEvent.Invoke(this, args); } private void OnWindowMinimizeEnd(IWindow window) @@ -623,7 +679,7 @@ private void OnWindowMinimizeEnd(IWindow window) WindowEventArgs args = new() { Window = window }; _internalContext.ButlerEventHandlers.OnWindowMinimizeEnd(args); - WindowMinimizeEnd?.Invoke(this, args); + _windowMinimizeEndEvent.Invoke(this, args); } public IEnumerator GetEnumerator() => _windows.Values.GetEnumerator(); diff --git a/src/Whim/Workspace/WorkspaceManager.cs b/src/Whim/Workspace/WorkspaceManager.cs index ebd26d8ac..a0d2ebfaa 100644 --- a/src/Whim/Workspace/WorkspaceManager.cs +++ b/src/Whim/Workspace/WorkspaceManager.cs @@ -37,17 +37,47 @@ internal class WorkspaceManager : IWorkspaceManager /// private readonly List _workspacesToCreate = new(); - public event EventHandler? WorkspaceAdded; + private readonly ThreadSafeEvent _workspaceAddedEvent; + public event EventHandler? WorkspaceAdded + { + add => _workspaceAddedEvent.Add(value); + remove => _workspaceAddedEvent.Remove(value); + } - public event EventHandler? WorkspaceRemoved; + private readonly ThreadSafeEvent _workspaceRemovedEvent; + public event EventHandler? WorkspaceRemoved + { + add => _workspaceRemovedEvent.Add(value); + remove => _workspaceRemovedEvent.Remove(value); + } - public event EventHandler? ActiveLayoutEngineChanged; + private readonly ThreadSafeEvent _activeLayoutEngineChangedEvent; + public event EventHandler? ActiveLayoutEngineChanged + { + add => _activeLayoutEngineChangedEvent.Add(value); + remove => _activeLayoutEngineChangedEvent.Remove(value); + } - public event EventHandler? WorkspaceRenamed; + private readonly ThreadSafeEvent _workspaceRenamedEvent; + public event EventHandler? WorkspaceRenamed + { + add => _workspaceRenamedEvent.Add(value); + remove => _workspaceRenamedEvent.Remove(value); + } - public event EventHandler? WorkspaceLayoutStarted; + private readonly ThreadSafeEvent _workspaceLayoutStartedEvent; + public event EventHandler? WorkspaceLayoutStarted + { + add => _workspaceLayoutStartedEvent.Add(value); + remove => _workspaceLayoutStartedEvent.Remove(value); + } - public event EventHandler? WorkspaceLayoutCompleted; + private readonly ThreadSafeEvent _workspaceLayoutCompletedEvent; + public event EventHandler? WorkspaceLayoutCompleted + { + add => _workspaceLayoutCompletedEvent.Add(value); + remove => _workspaceLayoutCompletedEvent.Remove(value); + } public Func CreateLayoutEngines { get; set; } = () => new CreateLeafLayoutEngine[] { (id) => new ColumnLayoutEngine(id) }; @@ -73,13 +103,21 @@ public WorkspaceManager(IContext context, IInternalContext internalContext) { _context = context; _internalContext = internalContext; + + _workspaceAddedEvent = new(context); + _workspaceRemovedEvent = new(context); + _activeLayoutEngineChangedEvent = new(context); + _workspaceRenamedEvent = new(context); + _workspaceLayoutStartedEvent = new(context); + _workspaceLayoutCompletedEvent = new(context); + _triggers = new WorkspaceManagerTriggers() { ActiveLayoutEngineChanged = (ActiveLayoutEngineChangedEventArgs e) => - ActiveLayoutEngineChanged?.Invoke(this, e), - WorkspaceRenamed = (WorkspaceRenamedEventArgs e) => WorkspaceRenamed?.Invoke(this, e), - WorkspaceLayoutStarted = (WorkspaceEventArgs e) => WorkspaceLayoutStarted?.Invoke(this, e), - WorkspaceLayoutCompleted = (WorkspaceEventArgs e) => WorkspaceLayoutCompleted?.Invoke(this, e) + _activeLayoutEngineChangedEvent.Invoke(this, e), + WorkspaceRenamed = (WorkspaceRenamedEventArgs e) => _workspaceRenamedEvent.Invoke(this, e), + WorkspaceLayoutStarted = (WorkspaceEventArgs e) => _workspaceLayoutStartedEvent.Invoke(this, e), + WorkspaceLayoutCompleted = (WorkspaceEventArgs e) => _workspaceLayoutCompletedEvent.Invoke(this, e) }; } @@ -177,7 +215,7 @@ public void Initialize() Workspace workspace = new(_context, _internalContext, _triggers, name ?? $"Workspace {_workspaces.Count + 1}", layoutEngines); _workspaces.Add(workspace); - WorkspaceAdded?.Invoke(this, new WorkspaceEventArgs() { Workspace = workspace }); + _workspaceAddedEvent.Invoke(this, new WorkspaceEventArgs() { Workspace = workspace }); return workspace; } @@ -202,7 +240,7 @@ public bool Remove(IWorkspace workspace) _context.Butler.MergeWorkspaceWindows(workspace, _workspaces[^1]); _context.Butler.Activate(_workspaces[^1]); - WorkspaceRemoved?.Invoke(this, new WorkspaceEventArgs() { Workspace = workspace }); + _workspaceRemovedEvent.Invoke(this, new WorkspaceEventArgs() { Workspace = workspace }); return wasFound; } From 264ab2e6279319bd5f51b6c8d73284e544f80087 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 23:43:16 +1100 Subject: [PATCH 26/31] Switch to Task.Run from NativeManager.TryEnqueue --- src/Whim/Butler/ButlerEventHandlers.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Whim/Butler/ButlerEventHandlers.cs b/src/Whim/Butler/ButlerEventHandlers.cs index f0fc00b4a..6668cddfa 100644 --- a/src/Whim/Butler/ButlerEventHandlers.cs +++ b/src/Whim/Butler/ButlerEventHandlers.cs @@ -230,7 +230,7 @@ public void OnMonitorsChanged(MonitorsChangedEventArgs e) // windows around after a monitor change. // NOTE: ButlerEventHandlersTests has a test for this which only runs locally - it is // turned off in CI as it has proved flaky when running on GitHub Actions. - _context.NativeManager.TryEnqueue(async () => + Task.Run(async () => { await Task.Delay(MonitorsChangedDelay).ConfigureAwait(true); From bea6cea0c9c241892a6841513daa493356141397 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 23:43:28 +1100 Subject: [PATCH 27/31] Fix ThreadSafeEvent calls --- src/Whim/Window/WindowManager.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/Whim/Window/WindowManager.cs b/src/Whim/Window/WindowManager.cs index 97817ba72..06426e083 100644 --- a/src/Whim/Window/WindowManager.cs +++ b/src/Whim/Window/WindowManager.cs @@ -19,56 +19,56 @@ internal class WindowManager : IWindowManager, IInternalWindowManager private readonly ThreadSafeEvent _windowAddedEvent; public event EventHandler? WindowAdded { - add => _windowAddedEvent.Equals(value); + add => _windowAddedEvent.Add(value); remove => _windowAddedEvent.Remove(value); } private readonly ThreadSafeEvent _windowFocusedEvent; public event EventHandler? WindowFocused { - add => _windowFocusedEvent.Equals(value); + add => _windowFocusedEvent.Add(value); remove => _windowFocusedEvent.Remove(value); } private readonly ThreadSafeEvent _windowRemovedEvent; public event EventHandler? WindowRemoved { - add => _windowRemovedEvent.Equals(value); + add => _windowRemovedEvent.Add(value); remove => _windowRemovedEvent.Remove(value); } private readonly ThreadSafeEvent _windowMoveStartEvent; public event EventHandler? WindowMoveStart { - add => _windowMoveStartEvent.Equals(value); + add => _windowMoveStartEvent.Add(value); remove => _windowMoveStartEvent.Remove(value); } private readonly ThreadSafeEvent _windowMovedEvent; public event EventHandler? WindowMoved { - add => _windowMovedEvent.Equals(value); + add => _windowMovedEvent.Add(value); remove => _windowMovedEvent.Remove(value); } private readonly ThreadSafeEvent _windowMoveEndEvent; public event EventHandler? WindowMoveEnd { - add => _windowMoveEndEvent.Equals(value); + add => _windowMoveEndEvent.Add(value); remove => _windowMoveEndEvent.Remove(value); } private readonly ThreadSafeEvent _windowMinimizeStartEvent; public event EventHandler? WindowMinimizeStart { - add => _windowMinimizeStartEvent.Equals(value); + add => _windowMinimizeStartEvent.Add(value); remove => _windowMinimizeStartEvent.Remove(value); } private readonly ThreadSafeEvent _windowMinimizeEndEvent; public event EventHandler? WindowMinimizeEnd { - add => _windowMinimizeEndEvent.Equals(value); + add => _windowMinimizeEndEvent.Add(value); remove => _windowMinimizeEndEvent.Remove(value); } From f6edf166fe788a5f71358112f9ce2aa5c5735aee Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 23:43:36 +1100 Subject: [PATCH 28/31] Fix tests --- src/Whim.TestUtils/Assert.cs | 58 ++++++++++++++++--- .../Butler/ButlerEventHandlersTests.cs | 18 ++++-- src/Whim.Tests/Monitor/MonitorManagerTests.cs | 20 ++++--- src/Whim.Tests/Window/WindowManagerTests.cs | 14 +++++ 4 files changed, 86 insertions(+), 24 deletions(-) diff --git a/src/Whim.TestUtils/Assert.cs b/src/Whim.TestUtils/Assert.cs index ec075a280..ff53aad40 100644 --- a/src/Whim.TestUtils/Assert.cs +++ b/src/Whim.TestUtils/Assert.cs @@ -1,5 +1,6 @@ using System; using System.ComponentModel; +using System.Threading.Tasks; using NSubstitute; using Xunit; using Xunit.Sdk; @@ -7,18 +8,19 @@ namespace Whim.TestUtils; /// -/// Exception thrown when an event is raised when it should not have been. +/// Exception thrown when an event is not raises when it should have been (or vice versa). /// #pragma warning disable CA1032 // Implement standard exception constructors -public class DoesNotRaiseException : XunitException +public class ShouldRaiseException : XunitException #pragma warning restore CA1032 // Implement standard exception constructors { /// - /// Creates a new instance of the class. + /// Creates a new instance of the class. /// /// - public DoesNotRaiseException(Type type) - : base($"Expected event of type {type} to not be raised.") { } + /// Whether it was expected that the exception was raised. + public ShouldRaiseException(Type type, bool expected) + : base($"Expected event of type {type} to {(expected ? "not" : "")} be raised.") { } } /// @@ -33,7 +35,7 @@ public static class CustomAssert /// The method to attach the event handler. /// The method to detach the event handler. /// The action to perform. - /// Thrown when the event is raised. + /// Thrown when the event is raised. public static void DoesNotRaise(Action> attach, Action> detach, Action action) { bool raised = false; @@ -53,7 +55,45 @@ void handler(object? sender, T e) if (raised) { - throw new DoesNotRaiseException(typeof(T)); + throw new ShouldRaiseException(typeof(T), expected: false); + } + } + + /// + /// Asserts that an event is not raised. + /// + /// The type of event to check. + /// The method to attach the event handler. + /// The method to detach the event handler. + /// The action to perform. + /// The delay in milliseconds to wait. + /// Thrown when the event is raised. + public static async Task RaisesAsync( + Action> attach, + Action> detach, + Action action, + int delayMs + ) + { + bool raised = false; + void handler(object? sender, T e) + { + raised = true; + } + attach(handler); + try + { + action(); + await Task.Delay(delayMs).ConfigureAwait(true); + } + finally + { + detach(handler); + } + + if (!raised) + { + throw new ShouldRaiseException(typeof(T), expected: true); } } @@ -63,7 +103,7 @@ void handler(object? sender, T e) /// The method to attach the event handler. /// The method to detach the event handler. /// The action to perform. - /// Thrown when the event is raised. + /// Thrown when the event is raised. public static void DoesNotPropertyChange( Action attach, Action detach, @@ -87,7 +127,7 @@ void handler(object? sender, PropertyChangedEventArgs e) if (raised) { - throw new DoesNotRaiseException(typeof(PropertyChangedEventArgs)); + throw new ShouldRaiseException(typeof(PropertyChangedEventArgs), expected: false); } } diff --git a/src/Whim.Tests/Butler/ButlerEventHandlersTests.cs b/src/Whim.Tests/Butler/ButlerEventHandlersTests.cs index 0e35a643b..b5d786957 100644 --- a/src/Whim.Tests/Butler/ButlerEventHandlersTests.cs +++ b/src/Whim.Tests/Butler/ButlerEventHandlersTests.cs @@ -484,7 +484,7 @@ IWorkspace workspace #region OnMonitorsChanged [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_RemovedMonitor( + internal async Task OnMonitorsChanged_RemovedMonitorAsync( IContext ctx, IInternalContext internalCtx, IButlerPantry pantry, @@ -508,6 +508,7 @@ IWorkspace workspace AddedMonitors = Array.Empty() } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then the monitor is removed from the pantry pantry.Received().RemoveMonitor(monitors[0]); @@ -516,7 +517,7 @@ IWorkspace workspace } [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_RemovedMonitor_NoWorkspaceForMonitor( + internal async Task OnMonitorsChanged_RemovedMonitor_NoWorkspaceForMonitorAsync( IContext ctx, IInternalContext internalCtx, IButlerChores chores, @@ -539,6 +540,7 @@ IButlerPantry pantry AddedMonitors = Array.Empty() } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then nothing happens pantry.Received().RemoveMonitor(monitors[0]); @@ -564,7 +566,7 @@ private static IWorkspace[] CreateWorkspaces(IContext context, int count) } [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_AddedMonitor_UseSpareWorkspace( + internal async void OnMonitorsChanged_AddedMonitor_UseSpareWorkspace( IContext ctx, IInternalContext internalCtx, IButlerChores chores, @@ -591,6 +593,7 @@ IMonitor newMonitor AddedMonitors = new[] { newMonitor } } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then the monitor is added to the pantry pantry.Received().SetMonitorWorkspace(newMonitor, workspaces[2]); @@ -598,7 +601,7 @@ IMonitor newMonitor } [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_AddedMonitor_CreateWorkspace_Succeeds( + internal async Task OnMonitorsChanged_AddedMonitor_CreateWorkspace_SucceedsAsync( IContext ctx, IInternalContext internalCtx, IButlerChores chores, @@ -626,6 +629,7 @@ IWorkspace newWorkspace AddedMonitors = new[] { newMonitor } } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then the monitor is added to the pantry pantry.Received().SetMonitorWorkspace(newMonitor, newWorkspace); @@ -633,7 +637,7 @@ IWorkspace newWorkspace } [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_AddedMonitor_CreateWorkspace_Fails( + internal async void OnMonitorsChanged_AddedMonitor_CreateWorkspace_Fails( IContext ctx, IInternalContext internalCtx, IButlerChores chores, @@ -659,6 +663,7 @@ IButlerPantry pantry AddedMonitors = new[] { monitors[1] } } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then the monitor is not added to the pantry pantry.DidNotReceive().SetMonitorWorkspace(monitors[1], workspaces[0]); @@ -666,7 +671,7 @@ IButlerPantry pantry } [Theory, AutoSubstituteData] - internal void OnMonitorsChanged_RemovedAndAddedMonitor( + internal async Task OnMonitorsChanged_RemovedAndAddedMonitorAsync( IContext ctx, IInternalContext internalCtx, IButlerChores chores, @@ -697,6 +702,7 @@ IMonitor newMonitor AddedMonitors = new[] { newMonitor } } ); + await Task.Delay(sut.MonitorsChangedDelay + 100); // Then the monitor is removed from the pantry pantry.Received().RemoveMonitor(monitors[0]); diff --git a/src/Whim.Tests/Monitor/MonitorManagerTests.cs b/src/Whim.Tests/Monitor/MonitorManagerTests.cs index 59850dc3a..7858a508e 100644 --- a/src/Whim.Tests/Monitor/MonitorManagerTests.cs +++ b/src/Whim.Tests/Monitor/MonitorManagerTests.cs @@ -7,7 +7,6 @@ using System.Runtime.InteropServices; using AutoFixture; using FluentAssertions; -using Microsoft.UI.Dispatching; using NSubstitute; using NSubstitute.ClearExtensions; using NSubstitute.ReturnsExtensions; @@ -603,20 +602,23 @@ internal void WindowMessageMonitor_DpiChanged(IContext ctx, IInternalContext int } [Theory, AutoSubstituteData] - internal void WindowMessageMonitor_SessionChanged(IContext ctx, IInternalContext internalCtx) + internal async void WindowMessageMonitor_SessionChanged(IContext ctx, IInternalContext internalCtx) { // Given MonitorManager monitorManager = new(ctx, internalCtx); monitorManager.Subscribe(); - // When - internalCtx.WindowMessageMonitor.SessionChanged += Raise.Event>( - internalCtx.WindowMessageMonitor, - WindowMessageMonitorEventArgs + // When the WindowMessageMonitor raises the SessionChanged event, + // Then the MonitorManager will raise the MonitorsChanged event + await CustomAssert.RaisesAsync( + h => monitorManager.MonitorsChanged += h, + h => monitorManager.MonitorsChanged -= h, + () => + internalCtx.WindowMessageMonitor.SessionChanged += Raise.Event< + EventHandler + >(internalCtx.WindowMessageMonitor, WindowMessageMonitorEventArgs), + 5100 ); - - // Then - ctx.NativeManager.Received(1).TryEnqueue(Arg.Any()); } [Theory, AutoSubstituteData] diff --git a/src/Whim.Tests/Window/WindowManagerTests.cs b/src/Whim.Tests/Window/WindowManagerTests.cs index 2e1e2030c..ee330a2f3 100644 --- a/src/Whim.Tests/Window/WindowManagerTests.cs +++ b/src/Whim.Tests/Window/WindowManagerTests.cs @@ -172,6 +172,15 @@ private WindowManagerTests Setup_GetCursorPos(IInternalContext internalCtx) return this; } + private static void ClearReceivedCalls(IContext ctx, IInternalContext internalCtx, IWorkspace? workspace = null) + { + ctx.ClearReceivedCalls(); + ctx.NativeManager.ClearReceivedCalls(); + ctx.Butler.Pantry.ClearReceivedCalls(); + internalCtx.ClearReceivedCalls(); + workspace?.ClearReceivedCalls(); + } + [Theory, AutoSubstituteData] internal void OnWindowAdded(IContext ctx, IInternalContext internalCtx, IWindow window) { @@ -1126,6 +1135,7 @@ internal void WinEventProc_OnWindowMoveEnd_GetMovedEdges_NoWorkspace(IContext ct // When windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); + ClearReceivedCalls(ctx, internalCtx); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); // Then @@ -1159,6 +1169,7 @@ IWorkspace workspace () => { capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); + ClearReceivedCalls(ctx, internalCtx, workspace); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); } ); @@ -1197,6 +1208,7 @@ IWorkspace workspace { windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); + ClearReceivedCalls(ctx, internalCtx, workspace); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); } ); @@ -1240,6 +1252,7 @@ IWorkspace workspace // When windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); + ClearReceivedCalls(ctx, internalCtx); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); // Then @@ -1294,6 +1307,7 @@ IWorkspace workspace // When windowManager.Subscribe(); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZESTART, hwnd, 0, 0, 0, 0); + ClearReceivedCalls(ctx, internalCtx, workspace); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_SYSTEM_MOVESIZEEND, hwnd, 0, 0, 0, 0); // Then From 2a940cfbf193841bff50372df39c88d030f0f41d Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 23:52:18 +1100 Subject: [PATCH 29/31] Add a delay for a test --- src/Whim.Tests/Window/WindowManagerTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Whim.Tests/Window/WindowManagerTests.cs b/src/Whim.Tests/Window/WindowManagerTests.cs index ee330a2f3..443575479 100644 --- a/src/Whim.Tests/Window/WindowManagerTests.cs +++ b/src/Whim.Tests/Window/WindowManagerTests.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.ComponentModel; using System.Linq; +using System.Threading.Tasks; using AutoFixture; using FluentAssertions; using NSubstitute; @@ -927,7 +928,7 @@ IInternalContext internalCtx } [Theory, AutoSubstituteData] - internal void WinEventProc_OnWindowMoved_WindowGetsRemoved(IContext ctx, IInternalContext internalCtx) + internal async void WinEventProc_OnWindowMoved_WindowGetsRemoved(IContext ctx, IInternalContext internalCtx) { // Given the window has been been handled, but is removed (CaptureWinEventProc capture, WindowManager windowManager, HWND hwnd) = Setup_RectRestoring(ctx, internalCtx); @@ -935,10 +936,9 @@ internal void WinEventProc_OnWindowMoved_WindowGetsRemoved(IContext ctx, IIntern // When the window is moved and then removed capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_LOCATIONCHANGE, hwnd, 0, 0, 0, 0); - capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_DESTROY, hwnd, 0, 0, 0, 0); - capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_LOCATIONCHANGE, hwnd, 0, 0, 0, 0); + await Task.Delay(windowManager.WindowMovedDelay * 2 + 200); // Then the workspace is asked to do two layouts workspace.Received(2).DoLayout(); From 26695620f9ce72475d9526c699ee7a8db25fc2b1 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sat, 30 Mar 2024 23:55:47 +1100 Subject: [PATCH 30/31] Formatting --- src/Whim.Tests/Window/WindowManagerTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Whim.Tests/Window/WindowManagerTests.cs b/src/Whim.Tests/Window/WindowManagerTests.cs index 443575479..941e0fd3d 100644 --- a/src/Whim.Tests/Window/WindowManagerTests.cs +++ b/src/Whim.Tests/Window/WindowManagerTests.cs @@ -938,7 +938,7 @@ internal async void WinEventProc_OnWindowMoved_WindowGetsRemoved(IContext ctx, I capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_LOCATIONCHANGE, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_DESTROY, hwnd, 0, 0, 0, 0); capture.WinEventProc!.Invoke((HWINEVENTHOOK)0, PInvoke.EVENT_OBJECT_LOCATIONCHANGE, hwnd, 0, 0, 0, 0); - await Task.Delay(windowManager.WindowMovedDelay * 2 + 200); + await Task.Delay(windowManager.WindowMovedDelay * 2 + 200); // Then the workspace is asked to do two layouts workspace.Received(2).DoLayout(); From 185fea7f7a5dd31300ae773db479841418e93395 Mon Sep 17 00:00:00 2001 From: Isaac Daly Date: Sun, 31 Mar 2024 12:44:48 +1100 Subject: [PATCH 31/31] Move event subscription to STA thread --- src/Whim.Bar.Tests/BarPluginTests.cs | 4 ++-- src/Whim.Bar/BarPlugin.cs | 9 ++++++-- .../CommandPalettePlugin.cs | 3 +++ .../FloatingLayoutPluginTests.cs | 1 + .../FloatingLayoutPlugin.cs | 10 ++++++++- .../FocusIndicatorPlugin.cs | 10 ++++++--- src/Whim.Gaps/GapsPlugin.cs | 3 +++ .../LayoutPreviewPluginTests.cs | 21 +++++++++---------- src/Whim.LayoutPreview/LayoutPreviewPlugin.cs | 13 ++++++++---- src/Whim.SliceLayout/SliceLayoutPlugin.cs | 3 +++ .../TreeLayoutBarPlugin.cs | 3 +++ .../TreeLayoutCommandPalettePlugin.cs | 3 +++ src/Whim.TreeLayout/TreeLayoutPlugin.cs | 3 +++ src/Whim.Updater/UpdaterPlugin.cs | 5 ++++- src/Whim/Context/InternalContext.cs | 1 + src/Whim/Plugin/IPlugin.cs | 17 +++++++++++---- src/Whim/Plugin/IPluginManager.cs | 7 ++++++- src/Whim/Plugin/PluginManager.cs | 10 +++++++++ 18 files changed, 97 insertions(+), 29 deletions(-) diff --git a/src/Whim.Bar.Tests/BarPluginTests.cs b/src/Whim.Bar.Tests/BarPluginTests.cs index efa9b2256..c3da65c35 100644 --- a/src/Whim.Bar.Tests/BarPluginTests.cs +++ b/src/Whim.Bar.Tests/BarPluginTests.cs @@ -16,7 +16,7 @@ public void MonitorManager_MonitorsChanged_RemovedMonitors(IContext context, IMo NativeManagerUtils.SetupTryEnqueue(context); // When MonitorManager_MonitorsChanged is called with a removed monitor which is not in the map - barPlugin.PreInitialize(); + barPlugin.Subscribe(); context.MonitorManager.MonitorsChanged += Raise.EventWith( new MonitorsChangedEventArgs() { @@ -28,6 +28,6 @@ public void MonitorManager_MonitorsChanged_RemovedMonitors(IContext context, IMo // Then an exception is not thrown. barPlugin.Dispose(); - context.NativeManager.Received(1).TryEnqueue(Arg.Any()); + context.NativeManager.Received(1).InvokeOnUIThread(Arg.Any()); } } diff --git a/src/Whim.Bar/BarPlugin.cs b/src/Whim.Bar/BarPlugin.cs index fea9bd7f3..a260589aa 100644 --- a/src/Whim.Bar/BarPlugin.cs +++ b/src/Whim.Bar/BarPlugin.cs @@ -33,7 +33,6 @@ public BarPlugin(IContext context, BarConfig barConfig) /// public void PreInitialize() { - _context.MonitorManager.MonitorsChanged += MonitorManager_MonitorsChanged; _context.FilterManager.AddTitleMatchFilter("Whim Bar"); _context.WorkspaceManager.AddProxyLayoutEngine(layout => new BarLayoutEngine(_barConfig, layout)); } @@ -56,7 +55,7 @@ private void MonitorManager_MonitorsChanged(object? sender, MonitorsChangedEvent foreach (IMonitor monitor in e.RemovedMonitors) { _monitorBarMap.TryGetValue(monitor, out BarWindow? value); - _context.NativeManager.TryEnqueue(() => value?.Close()); + _context.NativeManager.InvokeOnUIThread(() => value?.Close()); _monitorBarMap.Remove(monitor); } @@ -87,6 +86,12 @@ private void ShowAll() } } + /// + public void Subscribe() + { + _context.MonitorManager.MonitorsChanged += MonitorManager_MonitorsChanged; + } + /// public void LoadState(JsonElement pluginSavedState) { } diff --git a/src/Whim.CommandPalette/CommandPalettePlugin.cs b/src/Whim.CommandPalette/CommandPalettePlugin.cs index 9e7729823..219e6c440 100644 --- a/src/Whim.CommandPalette/CommandPalettePlugin.cs +++ b/src/Whim.CommandPalette/CommandPalettePlugin.cs @@ -50,6 +50,9 @@ public void PostInitialize() _commandPaletteWindow = new CommandPaletteWindow(_context, this); } + /// + public void Subscribe() { } + /// /// Activate the command palette. /// diff --git a/src/Whim.FloatingLayout.Tests/FloatingLayoutPluginTests.cs b/src/Whim.FloatingLayout.Tests/FloatingLayoutPluginTests.cs index 68e17c05d..a533a91d2 100644 --- a/src/Whim.FloatingLayout.Tests/FloatingLayoutPluginTests.cs +++ b/src/Whim.FloatingLayout.Tests/FloatingLayoutPluginTests.cs @@ -136,6 +136,7 @@ IWorkspace activeWorkspace // When plugin.PreInitialize(); + plugin.PostInitialize(); plugin.MarkWindowAsFloating(window); context.WindowManager.WindowRemoved += Raise.EventWith(new WindowEventArgs() { Window = window }); diff --git a/src/Whim.FloatingLayout/FloatingLayoutPlugin.cs b/src/Whim.FloatingLayout/FloatingLayoutPlugin.cs index 1addf1f1f..e9c1b1f04 100644 --- a/src/Whim.FloatingLayout/FloatingLayoutPlugin.cs +++ b/src/Whim.FloatingLayout/FloatingLayoutPlugin.cs @@ -31,11 +31,19 @@ public FloatingLayoutPlugin(IContext context) public void PreInitialize() { _context.WorkspaceManager.AddProxyLayoutEngine(layout => new FloatingLayoutEngine(_context, this, layout)); + } + + /// + public void PostInitialize() + { _context.WindowManager.WindowRemoved += WindowManager_WindowRemoved; } /// - public void PostInitialize() { } + public void Subscribe() + { + _context.WindowManager.WindowRemoved += WindowManager_WindowRemoved; + } /// public IPluginCommands PluginCommands => new FloatingLayoutCommands(this); diff --git a/src/Whim.FocusIndicator/FocusIndicatorPlugin.cs b/src/Whim.FocusIndicator/FocusIndicatorPlugin.cs index 0c1d7f442..09e0cc205 100644 --- a/src/Whim.FocusIndicator/FocusIndicatorPlugin.cs +++ b/src/Whim.FocusIndicator/FocusIndicatorPlugin.cs @@ -37,9 +37,6 @@ public FocusIndicatorPlugin(IContext context, FocusIndicatorConfig focusIndicato public void PreInitialize() { _context.FilterManager.AddTitleMatchFilter(FocusIndicatorConfig.Title); - - _context.WindowManager.WindowMoveStart += WindowManager_WindowMoveStart; - _context.WindowManager.WindowFocused += WindowManager_WindowFocused; } /// @@ -51,6 +48,13 @@ public void PostInitialize() // Activate the window so it renders. _focusIndicatorWindow.Activate(); _focusIndicatorWindow.Hide(_context); + } + + /// + public void Subscribe() + { + _context.WindowManager.WindowMoveStart += WindowManager_WindowMoveStart; + _context.WindowManager.WindowFocused += WindowManager_WindowFocused; // Only subscribe to workspace changes once the indicator window has been created - we shouldn't // show a window which doesn't yet exist (it'll just crash Whim). diff --git a/src/Whim.Gaps/GapsPlugin.cs b/src/Whim.Gaps/GapsPlugin.cs index 00e943c6b..6f61e52cd 100644 --- a/src/Whim.Gaps/GapsPlugin.cs +++ b/src/Whim.Gaps/GapsPlugin.cs @@ -37,6 +37,9 @@ public void PreInitialize() /// public void PostInitialize() { } + /// + public void Subscribe() { } + /// /// Update the gap between the parent layout engine and the area where windows are placed by /// the . diff --git a/src/Whim.LayoutPreview.Tests/LayoutPreviewPluginTests.cs b/src/Whim.LayoutPreview.Tests/LayoutPreviewPluginTests.cs index 606885d9f..516eb1ae0 100644 --- a/src/Whim.LayoutPreview.Tests/LayoutPreviewPluginTests.cs +++ b/src/Whim.LayoutPreview.Tests/LayoutPreviewPluginTests.cs @@ -60,7 +60,6 @@ public void PluginCommands(IContext ctx) } [Theory, AutoSubstituteData] - [SuppressMessage("Usage", "NS5000:Received check.")] public void PreInitialize(IContext ctx) { // Given @@ -70,24 +69,24 @@ public void PreInitialize(IContext ctx) plugin.PreInitialize(); // Then - ctx.WindowManager.Received(1).WindowMoveStart += Arg.Any>(); - ctx.WindowManager.Received(1).WindowMoved += Arg.Any>(); - ctx.WindowManager.Received(1).WindowMoveEnd += Arg.Any>(); - ctx.WindowManager.Received(1).WindowRemoved += Arg.Any>(); ctx.FilterManager.Received(1).AddTitleMatchFilter(LayoutPreviewWindow.WindowTitle); } [Theory, AutoSubstituteData] - public void PostInitialize(IContext ctx) + [SuppressMessage("Usage", "NS5000:Received check.")] + public void Subscribe(IContext ctx) { // Given using LayoutPreviewPlugin plugin = new(ctx); // When - plugin.PostInitialize(); + plugin.Subscribe(); // Then - Assert.True(true); + ctx.WindowManager.Received(1).WindowMoveStart += Arg.Any>(); + ctx.WindowManager.Received(1).WindowMoved += Arg.Any>(); + ctx.WindowManager.Received(1).WindowMoveEnd += Arg.Any>(); + ctx.WindowManager.Received(1).WindowRemoved += Arg.Any>(); } [Theory, AutoSubstituteData] @@ -199,7 +198,7 @@ public void WindowMoved_Dragged_CannotFindWorkspace(IContext ctx, IWorkspace wor workspace.ActiveLayoutEngine.ClearReceivedCalls(); // When - plugin.PreInitialize(); + plugin.Subscribe(); ctx.WindowManager.WindowMoved += Raise.Event>(ctx.WindowManager, e); // Then @@ -225,7 +224,7 @@ public void WindowMoved_Dragged_Success(IContext ctx, IWindow window, IWorkspace workspace.ActiveLayoutEngine.ClearReceivedCalls(); // When - plugin.PreInitialize(); + plugin.Subscribe(); ctx.WindowManager.WindowMoved += Raise.Event>(ctx.WindowManager, e); // Then @@ -274,7 +273,7 @@ public void WindowManager_WindowRemoved_NotHidden(IContext ctx, IWindow movedWin WindowEventArgs removeArgs = new() { Window = removedWindow }; // When - plugin.PreInitialize(); + plugin.Subscribe(); ctx.WindowManager.WindowMoved += Raise.Event>(ctx.WindowManager, moveArgs); ctx.WindowManager.WindowRemoved += Raise.Event>(ctx.WindowManager, removeArgs); diff --git a/src/Whim.LayoutPreview/LayoutPreviewPlugin.cs b/src/Whim.LayoutPreview/LayoutPreviewPlugin.cs index 9481cdcba..2494c133a 100644 --- a/src/Whim.LayoutPreview/LayoutPreviewPlugin.cs +++ b/src/Whim.LayoutPreview/LayoutPreviewPlugin.cs @@ -36,18 +36,23 @@ public LayoutPreviewPlugin(IContext context) /// public void PreInitialize() + { + _context.FilterManager.AddTitleMatchFilter(LayoutPreviewWindow.WindowTitle); + } + + /// + public void PostInitialize() { } + + /// + public void Subscribe() { _context.WindowManager.WindowMoveStart += WindowManager_WindowMoveStart; _context.WindowManager.WindowMoved += WindowMoved; _context.WindowManager.WindowMoveEnd += WindowManager_WindowMoveEnd; _context.WindowManager.WindowRemoved += WindowManager_WindowRemoved; _context.WindowManager.WindowFocused += WindowManager_WindowFocused; - _context.FilterManager.AddTitleMatchFilter(LayoutPreviewWindow.WindowTitle); } - /// - public void PostInitialize() { } - /// public void LoadState(JsonElement state) { } diff --git a/src/Whim.SliceLayout/SliceLayoutPlugin.cs b/src/Whim.SliceLayout/SliceLayoutPlugin.cs index 0800cce29..f1ba8ddb1 100644 --- a/src/Whim.SliceLayout/SliceLayoutPlugin.cs +++ b/src/Whim.SliceLayout/SliceLayoutPlugin.cs @@ -45,6 +45,9 @@ public void PreInitialize() { } /// public void PostInitialize() { } + /// + public void Subscribe() { } + /// public void LoadState(JsonElement state) { } diff --git a/src/Whim.TreeLayout.Bar/TreeLayoutBarPlugin.cs b/src/Whim.TreeLayout.Bar/TreeLayoutBarPlugin.cs index 53a74bcdf..f1cc4d41a 100644 --- a/src/Whim.TreeLayout.Bar/TreeLayoutBarPlugin.cs +++ b/src/Whim.TreeLayout.Bar/TreeLayoutBarPlugin.cs @@ -33,6 +33,9 @@ public void PreInitialize() { } /// public void PostInitialize() { } + /// + public void Subscribe() { } + /// /// Create the tree layout engine bar component. /// diff --git a/src/Whim.TreeLayout.CommandPalette/TreeLayoutCommandPalettePlugin.cs b/src/Whim.TreeLayout.CommandPalette/TreeLayoutCommandPalettePlugin.cs index 4a5815cbc..0afcd50c2 100644 --- a/src/Whim.TreeLayout.CommandPalette/TreeLayoutCommandPalettePlugin.cs +++ b/src/Whim.TreeLayout.CommandPalette/TreeLayoutCommandPalettePlugin.cs @@ -44,6 +44,9 @@ public void PostInitialize() { } /// public void PreInitialize() { } + /// + public void Subscribe() { } + /// public void LoadState(JsonElement state) { } diff --git a/src/Whim.TreeLayout/TreeLayoutPlugin.cs b/src/Whim.TreeLayout/TreeLayoutPlugin.cs index 84dcafd6d..c23d787bd 100644 --- a/src/Whim.TreeLayout/TreeLayoutPlugin.cs +++ b/src/Whim.TreeLayout/TreeLayoutPlugin.cs @@ -37,6 +37,9 @@ public void PreInitialize() { } /// public void PostInitialize() { } + /// + public void Subscribe() { } + /// public Direction? GetAddWindowDirection(IMonitor monitor) { diff --git a/src/Whim.Updater/UpdaterPlugin.cs b/src/Whim.Updater/UpdaterPlugin.cs index 6e4ac8fb8..745b2be64 100644 --- a/src/Whim.Updater/UpdaterPlugin.cs +++ b/src/Whim.Updater/UpdaterPlugin.cs @@ -90,7 +90,7 @@ private void HandleOnShowWindowNotification(AppNotificationActivatedEventArgs ar { Logger.Debug("Showing update window"); - _context.NativeManager.TryEnqueue(async () => + _context.NativeManager.InvokeOnUIThread(async () => { _updaterWindow = new UpdaterWindow(plugin: this, null); await _updaterWindow.Activate(_notInstalledReleases).ConfigureAwait(true); @@ -113,6 +113,9 @@ public void PostInitialize() _timer.Start(); } + /// + public void Subscribe() { } + private async void Timer_Elapsed(object? sender, ElapsedEventArgs e) => await CheckForUpdates().ConfigureAwait(true); diff --git a/src/Whim/Context/InternalContext.cs b/src/Whim/Context/InternalContext.cs index c393de97c..bc1c5f109 100644 --- a/src/Whim/Context/InternalContext.cs +++ b/src/Whim/Context/InternalContext.cs @@ -63,6 +63,7 @@ private void Subscribe() MonitorManager.Subscribe(); NotificationManager.Subscribe(); WindowManager.Subscribe(); + _context.PluginManager.Subscribe(); } protected virtual void Dispose(bool disposing) diff --git a/src/Whim/Plugin/IPlugin.cs b/src/Whim/Plugin/IPlugin.cs index 78d5f7bc1..b5ad58eae 100644 --- a/src/Whim/Plugin/IPlugin.cs +++ b/src/Whim/Plugin/IPlugin.cs @@ -17,19 +17,28 @@ public interface IPlugin /// /// This method is to be called by the plugin manager. - /// Initializes the plugin before the has been initialized. - /// Put things like event listeners here or adding proxy layout engines + /// Initializes the plugin before the and + /// have completed initialization. + /// This should be used for things like adding proxy layout engines /// (see ). /// void PreInitialize(); /// /// This method is to be called by the plugin manager. - /// Initializes the plugin after the rest of the has been initialized. - /// Put things which rely on the rest of the context here. + /// Initializes the plugin after the and + /// have completed initialization. + /// Put things which rely on the rest of the context here. Event listeners which don't need + /// to run on the UI/STA thread can be placed here. /// void PostInitialize(); + /// + /// This method is to be called by the plugin manager. + /// Put event listeners which need to run on the UI/STA thread here. + /// + void Subscribe(); + /// /// The commands and keybinds for this plugin. These are registered during . /// diff --git a/src/Whim/Plugin/IPluginManager.cs b/src/Whim/Plugin/IPluginManager.cs index 2299f9b49..b2f63a70a 100644 --- a/src/Whim/Plugin/IPluginManager.cs +++ b/src/Whim/Plugin/IPluginManager.cs @@ -14,7 +14,7 @@ public interface IPluginManager : IDisposable IReadOnlyCollection LoadedPlugins { get; } /// - /// Calls all plugins' method. + /// Calls all plugins' methods. /// This runs before the rest of the context has been initialized. /// void PreInitialize(); @@ -40,6 +40,11 @@ public interface IPluginManager : IDisposable /// void PostInitialize(); + /// + /// Calls all plugins' methods. This runs on the UI thread. + /// + void Subscribe(); + /// /// Adds a plugin, registers its commands and keybinds from . /// diff --git a/src/Whim/Plugin/PluginManager.cs b/src/Whim/Plugin/PluginManager.cs index b229a768b..1dd187c8d 100644 --- a/src/Whim/Plugin/PluginManager.cs +++ b/src/Whim/Plugin/PluginManager.cs @@ -86,6 +86,16 @@ private void LoadSavedState() } } + public void Subscribe() + { + Logger.Debug("Subscribing each plugin..."); + + foreach (IPlugin plugin in _plugins) + { + plugin.Subscribe(); + } + } + public T AddPlugin(T plugin) where T : IPlugin {