Skip to content

Commit

Permalink
Don't apply custom Firefox processing on startup (#958)
Browse files Browse the repository at this point in the history
#957 introduced custom logic to handle Firefox starting while Whim was running. However, this logic is unnecessary if Firefox is started prior to Whim. 

This PR introduces `IWindowSector.StartupWindow` to store the windows which were open when Whim started. If the Firefox window is in this set, then the custom logic is not needed and all events are processed as normal.
  • Loading branch information
dalyIsaac authored Jul 30, 2024
1 parent 178650e commit b777779
Show file tree
Hide file tree
Showing 8 changed files with 161 additions and 13 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Linq;
using FluentAssertions;

namespace Whim.Tests;

Expand Down Expand Up @@ -254,5 +255,11 @@ internal void PopulateSavedWorkspaces(IContext ctx, IInternalContext internalCtx
Assert.Equal(browserWorkspace.Id, rootSector.MapSector.MonitorWorkspaceMap[BrowserMonitor]);
Assert.Equal(codeWorkspace.Id, rootSector.MapSector.MonitorWorkspaceMap[CodeMonitor]);
Assert.Equal(autoWorkspace.Id, rootSector.MapSector.MonitorWorkspaceMap[AutoMonitor]);

// - the startup windows are set
Assert.Equal(5, rootSector.WindowSector.StartupWindows.Count);
rootSector
.WindowSector.StartupWindows.Should()
.BeEquivalentTo(new[] { BrowserHandle, DiscordHandle, SpotifyHandle, BrokenHandle, VscodeHandle });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,21 @@ public void Create_Success(IContext ctx, IWindow window)
Assert.NotNull(processor);
}

[Theory, AutoSubstituteData<StoreCustomization>]
internal void ProcessEvent_IsStartupWindow(IContext ctx, IWindow window, MutableRootSector rootSector)
{
// Given a Firefox window which was open when Whim started
window.ProcessFileName.Returns("firefox.exe");
rootSector.WindowSector.StartupWindows = new[] { window.Handle }.ToImmutableHashSet();
IWindowProcessor processor = FirefoxWindowProcessor.Create(ctx, window)!;

// When `ProcessEvent` is called
WindowProcessorResult result = processor.ProcessEvent(PInvoke.EVENT_OBJECT_SHOW, 0, 0, 0, 0);

// Then the window should be marked as a startup window
Assert.Equal(WindowProcessorResult.Process, result);
}

[Theory]
[InlineAutoSubstituteData(PInvoke.EVENT_OBJECT_SHOW)]
[InlineAutoSubstituteData(PInvoke.EVENT_SYSTEM_FOREGROUND)]
Expand All @@ -40,27 +55,27 @@ public void Create_Success(IContext ctx, IWindow window)
[InlineAutoSubstituteData(PInvoke.EVENT_OBJECT_LOCATIONCHANGE)]
[InlineAutoSubstituteData(PInvoke.EVENT_SYSTEM_MINIMIZESTART)]
[InlineAutoSubstituteData(PInvoke.EVENT_SYSTEM_MINIMIZEEND)]
public void ShouldBeIgnored_UntilCloaked(uint eventType, IContext ctx, IWindow window)
public void ProcessEvent_UntilCloaked(uint eventType, IContext ctx, IWindow window)
{
// Given an event which isn't `EVENT_OBJECT_CLOAKED` or `EVENT_OBJECT_DESTROY`
window.ProcessFileName.Returns("firefox.exe");
IWindowProcessor processor = FirefoxWindowProcessor.Create(ctx, window)!;

// When the event is passed to `ShouldBeIgnored`
// When the event is passed to `ProcessEvent`
WindowProcessorResult result = processor.ProcessEvent(eventType, 0, 0, 0, 0);

// Then the event should be ignored
Assert.Equal(WindowProcessorResult.Ignore, result);
}

[Theory, AutoSubstituteData]
public void ShouldBeIgnored_FirstCloaked(IContext ctx, IWindow window)
public void ProcessEvent_FirstCloaked(IContext ctx, IWindow window)
{
// Given the first `EVENT_OBJECT_CLOAKED` event
window.ProcessFileName.Returns("firefox.exe");
IWindowProcessor processor = FirefoxWindowProcessor.Create(ctx, window)!;

// When the event is passed to `ShouldBeIgnored`
// When the event is passed to `ProcessEvent`
WindowProcessorResult result = processor.ProcessEvent(PInvoke.EVENT_OBJECT_CLOAKED, 0, 0, 0, 0);

// Then the event should be ignored
Expand All @@ -75,7 +90,7 @@ public void ShouldNotBeIgnored_SecondCloaked(IContext ctx, IWindow window)
IWindowProcessor processor = FirefoxWindowProcessor.Create(ctx, window)!;
processor.ProcessEvent(PInvoke.EVENT_OBJECT_CLOAKED, 0, 0, 0, 0);

// When the event is passed to `ShouldBeIgnored`
// When the event is passed to `ProcessEvent`
WindowProcessorResult result = processor.ProcessEvent(PInvoke.EVENT_OBJECT_CLOAKED, 0, 0, 0, 0);

// Then the event should not be ignored
Expand All @@ -89,7 +104,7 @@ public void ShouldBeRemoved(IContext ctx, IWindow window)
window.ProcessFileName.Returns("firefox.exe");
IWindowProcessor processor = FirefoxWindowProcessor.Create(ctx, window)!;

// When the event is passed to `ShouldBeIgnored`
// When the event is passed to `ProcessEvent`
WindowProcessorResult result = processor.ProcessEvent(PInvoke.EVENT_OBJECT_DESTROY, 0, 0, 0, 0);

// Then the processor should be removed
Expand Down
101 changes: 101 additions & 0 deletions src/Whim.Tests/Store/WindowSector/WindowPickersTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
using System.Linq;

namespace Whim.Tests;

public class WindowPickersTests
{
[Theory, AutoSubstituteData]
public void PickAllWindows(IRootSector rootSector, IWindow window1, IWindow window2, IWindow window3)
{
// Given there are three windows
rootSector.WindowSector.Windows.Returns(
new Dictionary<HWND, IWindow>
{
[(HWND)1] = window1,
[(HWND)2] = window2,
[(HWND)3] = window3,
}.ToImmutableDictionary()
);

// When we get the windows
var result = Pickers.PickAllWindows()(rootSector);

// Then we get the windows
Assert.Equal(3, result.Count());
}

[Theory, AutoSubstituteData]
public void PickWindowByHandle_Failure(IRootSector rootSector, IWindow window1, IWindow window2, IWindow window3)
{
// Given there are three windows
rootSector.WindowSector.Windows.Returns(
new Dictionary<HWND, IWindow>
{
[(HWND)1] = window1,
[(HWND)2] = window2,
[(HWND)3] = window3,
}.ToImmutableDictionary()
);

HWND handle = (HWND)4;

// When we get the window
var result = Pickers.PickWindowByHandle(handle)(rootSector);

// Then we get an exception
Assert.False(result.IsSuccessful);
}

[Theory, AutoSubstituteData]
public void PickWindowByHandle_Success(IRootSector rootSector, IWindow window1, IWindow window2, IWindow window3)
{
// Given there are three windows
rootSector.WindowSector.Windows.Returns(
new Dictionary<HWND, IWindow>
{
[(HWND)1] = window1,
[(HWND)2] = window2,
[(HWND)3] = window3,
}.ToImmutableDictionary()
);

// When we get the window
var result = Pickers.PickWindowByHandle((HWND)1)(rootSector);

// Then we get the window
Assert.True(result.IsSuccessful);
Assert.Same(window1, result.Value);
}

[Theory, AutoSubstituteData]
public void PickIsStartupWindow_Failure(IRootSector rootSector)
{
// Given there are three startup windows
rootSector.WindowSector.StartupWindows.Returns(
new HashSet<HWND> { (HWND)1, (HWND)2, (HWND)3, }.ToImmutableHashSet()
);

HWND handle = (HWND)4;

// When we get whether the window is a startup window
var result = Pickers.PickIsStartupWindow(handle)(rootSector);

// Then we get false
Assert.False(result);
}

[Theory, AutoSubstituteData]
public void PickIsStartupWindow_Success(IRootSector rootSector)
{
// Given there are three startup windows
rootSector.WindowSector.StartupWindows.Returns(
new HashSet<HWND> { (HWND)1, (HWND)2, (HWND)3, }.ToImmutableHashSet()
);

// When we get whether the window is a startup window
var result = Pickers.PickIsStartupWindow((HWND)1)(rootSector);

// Then we get true
Assert.True(result);
}
}
5 changes: 5 additions & 0 deletions src/Whim/Store/WindowSector/IWindowSector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ public interface IWindowSector
/// </summary>
ImmutableDictionary<HWND, IWindow> Windows { get; }

/// <summary>
/// The windows which were open when Whim started.
/// </summary>
ImmutableHashSet<HWND> StartupWindows { get; }

/// <summary>
/// Whether a window is currently moving.
/// </summary>
Expand Down
22 changes: 15 additions & 7 deletions src/Whim/Store/WindowSector/Processors/FirefoxWindowProcessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,23 @@ namespace Whim;
/// </summary>
public class FirefoxWindowProcessor : IWindowProcessor
{
private readonly IContext _ctx;
private bool _hasSeenFirstCloaked;

/// <inheritdoc/>
public IWindow Window { get; }

private FirefoxWindowProcessor(IWindow window)
private FirefoxWindowProcessor(IContext ctx, IWindow window)
{
_ctx = ctx;
Window = window;
}

/// <summary>
/// Creates a new instance of the implementing class, if the given window matches the processor.
/// </summary>
public static IWindowProcessor? Create(IContext ctx, IWindow window) =>
window.ProcessFileName == "firefox.exe" ? new FirefoxWindowProcessor(window) : null;
window.ProcessFileName == "firefox.exe" ? new FirefoxWindowProcessor(ctx, window) : null;

/// <summary>
/// Indicates whether the event should be ignored by Whim.
Expand Down Expand Up @@ -51,6 +53,7 @@ private FirefoxWindowProcessor(IWindow window)
/// </list>
///
/// To deal with these issues, we ignore all events until the first <see cref="PInvoke.EVENT_OBJECT_CLOAKED"/> event is received.
/// If Firefox is already visible, we process all events.
/// </remarks>
/// <param name="eventType"></param>
/// <param name="idObject"></param>
Expand All @@ -66,6 +69,16 @@ public WindowProcessorResult ProcessEvent(
uint dwmsEventTime
)
{
if (eventType == PInvoke.EVENT_OBJECT_DESTROY)
{
return WindowProcessorResult.RemoveProcessor;
}

if (_ctx.Store.Pick(PickIsStartupWindow(Window.Handle)))
{
return WindowProcessorResult.Process;
}

if (eventType == PInvoke.EVENT_OBJECT_CLOAKED)
{
if (!_hasSeenFirstCloaked)
Expand All @@ -75,11 +88,6 @@ uint dwmsEventTime
}
}

if (eventType == PInvoke.EVENT_OBJECT_DESTROY)
{
return WindowProcessorResult.RemoveProcessor;
}

if (!_hasSeenFirstCloaked)
{
return WindowProcessorResult.Ignore;
Expand Down
8 changes: 8 additions & 0 deletions src/Whim/Store/WindowSector/WindowPickers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,12 @@ public static PurePicker<Result<IWindow>> PickWindowByHandle(HWND handle) =>
rootSector.WindowSector.Windows.TryGetValue(handle, out IWindow? w)
? Result.FromValue(w)
: Result.FromException<IWindow>(StoreExceptions.WindowNotFound(handle));

/// <summary>
/// Returns whether a window was open when Whim started.
/// </summary>
/// <param name="handle"></param>
/// <returns></returns>
public static PurePicker<bool> PickIsStartupWindow(HWND handle) =>
(rootSector) => rootSector.WindowSector.StartupWindows.Contains(handle);
}
2 changes: 2 additions & 0 deletions src/Whim/Store/WindowSector/WindowSector.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ internal class WindowSector : SectorBase, IWindowSector, IDisposable, IWindowSec

public ImmutableDictionary<HWND, IWindow> Windows { get; internal set; } = ImmutableDictionary<HWND, IWindow>.Empty;

public ImmutableHashSet<HWND> StartupWindows { get; internal set; } = ImmutableHashSet<HWND>.Empty;

public bool IsMovingWindow { get; internal set; }

public bool IsLeftMouseButtonDown { get; internal set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ MutableRootSector rootSector
WindowSector windowSector = rootSector.WindowSector;
MapSector mapSector = rootSector.MapSector;

windowSector.StartupWindows = internalCtx.CoreNativeManager.GetAllWindows().ToImmutableHashSet();

// Add the saved windows at their saved locations inside their saved workspaces.
// Other windows are routed to the monitor they're on.
List<HWND> processedWindows = new();
Expand Down

0 comments on commit b777779

Please sign in to comment.