Skip to content

DiffViewer: replace Application.Current null-check with IUiThreadMarshaller abstraction #30

@geevensingh

Description

@geevensingh

Problem

DiffPaneViewModel uses Application.Current is null as an implicit "am I running under tests?" marker in two places:

1. ApplyResult (DiffViewer/ViewModels/DiffPaneViewModel.cs:483-496):

private void ApplyResult(...)
{
    if (Application.Current is { Dispatcher: { } d } && !d.CheckAccess())
    {
        d.BeginInvoke(() => ApplyResult(...));
        return;
    }
    // ... apply on current thread
}

2. ScheduleOptionRefresh (DiffViewer/ViewModels/DiffPaneViewModel.cs:568-592):

private void ScheduleOptionRefresh()
{
    if (Application.Current is null)
    {
        // Test path - no Dispatcher, recompute synchronously.
        RefreshDiffFromCache();
        return;
    }
    // ... debounced via DispatcherTimer
}

Why this is brittle

  1. Tests do have a Dispatcher. RunOnUiSyncContextAsync in MainViewModelKeyboardShortcutTests.cs creates an STA thread + DispatcherSynchronizationContext + runs Dispatcher.Run(). But Application.Current stays null, so the test takes the "no dispatcher" branch — which means tests exercise a different code path than production. The auto-scroll race we just fixed was harder to debug because of exactly this: I assumed the test continuation would interleave like production, and it didn't.

  2. Application.Current is null is unrelated to "should I marshal". A WPF library used outside of App.xaml would also have null Application but a real Dispatcher. The check conflates two concerns.

  3. It's an implicit handshake between unrelated methods. New code added to the VM has to know the test-vs-prod marker convention or it'll break headless tests.

Suggested fix

Introduce an IUiThreadMarshaller (or reuse an existing dispatcher abstraction):

public interface IUiThreadMarshaller
{
    bool IsOnUiThread { get; }
    void Post(Action action);          // fire-and-forget, queue
    void Send(Action action);          // synchronous, marshal + wait
    IDisposable ScheduleDebounced(TimeSpan delay, Action action);  // for ScheduleOptionRefresh
}

Implementations:

  • DispatcherUiThreadMarshaller — wraps Application.Current.Dispatcher (or an injected Dispatcher). Used in production via CompositionRoot.
  • SyncContextUiThreadMarshaller — wraps SynchronizationContext.Current for tests under RunOnUiSyncContextAsync. Bonus: tests then exercise the same "marshal back to UI thread" path as production.

Inject into DiffPaneViewModel (and anywhere else that touches Application.Current.Dispatcher):

public DiffPaneViewModel(
    IRepositoryService repository,
    IDiffService? diffService = null,
    bool isCommitVsCommit = false,
    ISettingsService? settingsService = null,
    IUiThreadMarshaller? ui = null)
{
    _ui = ui ?? new DispatcherUiThreadMarshaller();
    ...
}

Then:

private void ApplyResult(...)
{
    if (!_ui.IsOnUiThread) { _ui.Post(() => ApplyResult(...)); return; }
    ...
}

private void ScheduleOptionRefresh()
{
    _debounce?.Dispose();
    _debounce = _ui.ScheduleDebounced(TimeSpan.FromMilliseconds(OptionDebounceMs), RefreshDiffFromCache);
}

Why now

The auto-scroll work just exposed how much rides on continuation timing under WPF Dispatcher. Making the UI marshalling explicit + injectable will make every future timing-sensitive feature easier to test and reason about.

Citations

  • DiffViewer/ViewModels/DiffPaneViewModel.cs:483-496ApplyResult dispatcher check
  • DiffViewer/ViewModels/DiffPaneViewModel.cs:568-592ScheduleOptionRefresh test/prod fork
  • DiffViewer.Tests/ViewModels/MainViewModelKeyboardShortcutTests.csRunOnUiSyncContextAsync (creates DispatcherSynchronizationContext + runs Dispatcher.Run(), but Application.Current is null)

Severity

Low (no current bug). Cleanup that pays back as more async / dispatcher-coordinated features land.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions