Avoid having to go back to the UI thread in the navbar code

src/EditorFeatures/Core/NavigationBar/NavigationBarController.cs

@@ -42,8 +42,9 @@ internal partial class NavigationBarController : IDisposable
     private bool _disconnected = false;
 
     /// <summary>
-    /// The last full information we have presented. If we end up wanting to present the same thing again, we can
-    /// just skip doing that as the UI will already know about this.
+    /// The last full information we have presented. If we end up wanting to present the same thing again, we can just
+    /// skip doing that as the UI will already know about this.  This is only ever read or written from <see
+    /// cref="_selectItemQueue"/>.  So we don't need to worry about any synchronization over it.
     /// </summary>
     private (ImmutableArray<NavigationBarProjectItem> projectItems, NavigationBarProjectItem? selectedProjectItem, NavigationBarModel? model, NavigationBarSelectedTypeAndMember selectedInfo) _lastPresentedInfo;
 
@@ -66,10 +67,10 @@ internal partial class NavigationBarController : IDisposable
     private readonly AsyncBatchingWorkQueue<VoidResult, NavigationBarModel?> _computeModelQueue;
 
     /// <summary>
-    /// Queue to batch up work to do to determine the selected item.  Used so we can batch up a lot of events and
-    /// only compute the selected item once for every batch.
+    /// Queue to batch up work to do to determine the selected item.  Used so we can batch up a lot of events and only
+    /// compute the selected item once for every batch. The value passed in is the last recorded caret position.
     /// </summary>
-    private readonly AsyncBatchingWorkQueue _selectItemQueue;
+    private readonly AsyncBatchingWorkQueue<int> _selectItemQueue;
 
     /// <summary>
     /// Whether or not the navbar is paused.  We pause updates when documents become non-visible. See <see
@@ -99,7 +100,7 @@ public NavigationBarController(
             asyncListener,
             _cancellationTokenSource.Token);
 
-        _selectItemQueue = new AsyncBatchingWorkQueue(
+        _selectItemQueue = new AsyncBatchingWorkQueue<int>(
             DelayTimeSpan.Short,
             SelectItemAsync,
             asyncListener,
@@ -126,9 +127,10 @@ public NavigationBarController(
         {
             threadingContext.ThrowIfNotOnUIThread();
 
-            // any time visibility changes, resume tagging on all taggers.  Any non-visible taggers will pause
-            // themselves immediately afterwards.
-            Resume();
+            if (_visibilityTracker?.IsVisible(_subjectBuffer) is false)
+                Pause();
+            else
+                Resume();
         };
 
         // Register to hear about visibility changes so we can pause/resume this tagger.
@@ -138,6 +140,26 @@ public NavigationBarController(
 
         // Kick off initial work to populate the navbars
         StartModelUpdateAndSelectedItemUpdateTasks();
+
+        return;
+
+        void Pause()
+        {
+            _paused = true;
+            _eventSource.Pause();
+        }
+
+        void Resume()
+        {
+            // if we're not actually paused, no need to do anything.
+            if (_paused)
+            {
+                // Set us back to running, and kick off work to compute tags now that we're visible again.
+                _paused = false;
+                _eventSource.Resume();
+                StartModelUpdateAndSelectedItemUpdateTasks();
+            }
+        }
     }
 
     void IDisposable.Dispose()
@@ -161,26 +183,6 @@ void IDisposable.Dispose()
         _cancellationTokenSource.Cancel();
     }
 
-    private void Pause()
-    {
-        _threadingContext.ThrowIfNotOnUIThread();
-        _paused = true;
-        _eventSource.Pause();
-    }
-
-    private void Resume()
-    {
-        _threadingContext.ThrowIfNotOnUIThread();
-        // if we're not actually paused, no need to do anything.
-        if (_paused)
-        {
-            // Set us back to running, and kick off work to compute tags now that we're visible again.
-            _paused = false;
-            _eventSource.Resume();
-            StartModelUpdateAndSelectedItemUpdateTasks();
-        }
-    }
-
     public TestAccessor GetTestAccessor() => new TestAccessor(this);
 
     private void OnEventSourceChanged(object? sender, TaggerEventArgs e)
@@ -200,31 +202,46 @@ private void StartModelUpdateAndSelectedItemUpdateTasks()
     private void OnCaretMovedOrActiveViewChanged(object? sender, EventArgs e)
     {
         _threadingContext.ThrowIfNotOnUIThread();
-        StartSelectedItemUpdateTask();
+
+        var caretPoint = GetCaretPoint();
+        if (caretPoint == null)
+            return;
+
+        // Cancel any in flight work.  We're on the UI thread, so we know this is the latest position of the user, and that
+        // this should supersede any other selection work items.
+        _selectItemQueue.AddWork(caretPoint.Value, cancelExistingWork: true);
     }
 
-    private void GetProjectItems(out ImmutableArray<NavigationBarProjectItem> projectItems, out NavigationBarProjectItem? selectedProjectItem)
+    private int? GetCaretPoint()
     {
-        var documents = _subjectBuffer.CurrentSnapshot.GetRelatedDocumentsWithChanges();
-        if (!documents.Any())
-        {
-            projectItems = [];
-            selectedProjectItem = null;
-            return;
-        }
+        var currentView = _presenter.TryGetCurrentView();
+        return currentView?.GetCaretPoint(_subjectBuffer)?.Position;
+    }
 
-        projectItems = [.. documents.Select(d =>
-            new NavigationBarProjectItem(
+    private (ImmutableArray<NavigationBarProjectItem> projectItems, NavigationBarProjectItem? selectedProjectItem) GetProjectItems()
+    {
+        var textContainer = _subjectBuffer.AsTextContainer();
+
+        var documents = textContainer.GetRelatedDocuments();
+        if (documents.IsEmpty)
+            return ([], null);
+
+        var projectItems = documents
+            .Select(d => new NavigationBarProjectItem(
                 d.Project.Name,
                 d.Project.GetGlyph(),
                 workspace: d.Project.Solution.Workspace,
                 documentId: d.Id,
-                language: d.Project.Language)).OrderBy(projectItem => projectItem.Text)];
+                language: d.Project.Language))
+            .OrderBy(projectItem => projectItem.Text)
+            .ToImmutableArray();
 
-        var document = _subjectBuffer.AsTextContainer().GetOpenDocumentInCurrentContext();
-        selectedProjectItem = document != null
+        var document = textContainer.GetOpenDocumentInCurrentContext();
+        var selectedProjectItem = document != null
             ? projectItems.FirstOrDefault(p => p.Text == document.Project.Name) ?? projectItems.First()
             : projectItems.First();
+
+        return (projectItems, selectedProjectItem);
     }
 
     private void OnItemSelected(object? sender, NavigationBarItemSelectedEventArgs e)

src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs

@@ -13,6 +13,7 @@
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.CodeAnalysis.Workspaces;
+using Microsoft.VisualStudio.Text;
 using Microsoft.VisualStudio.Threading;
 using Roslyn.Utilities;
 
@@ -34,16 +35,19 @@ internal partial class NavigationBarController
             return null;
 
         var textSnapshot = _subjectBuffer.CurrentSnapshot;
+        var caretPoint = GetCaretPoint();
 
-        // Ensure we switch to the threadpool before calling GetDocumentWithFrozenPartialSemantics.  It ensures
-        // that any IO that performs is not potentially on the UI thread.
+        // Ensure we switch to the threadpool before calling GetDocumentWithFrozenPartialSemantics.  It ensures that any
+        // IO that performs is not potentially on the UI thread.
         await TaskScheduler.Default;
 
         var model = await ComputeModelAsync().ConfigureAwait(false);
 
-        // Now, enqueue work to select the right item in this new model.
-        if (model != null)
-            StartSelectedItemUpdateTask();
+        // Now, enqueue work to select the right item in this new model. Note: we don't want to cancel existing items in
+        // the queue as it may be the case that the user moved between us capturing the initial caret point and now, and
+        // we'd want the selection work we enqueued for that to take precedence over us.
+        if (model != null && caretPoint != null)
+            _selectItemQueue.AddWork(caretPoint.Value, cancelExistingWork: false);
 
         return model;
 
@@ -94,55 +98,27 @@ await _visibilityTracker.DelayWhileNonVisibleAsync(
         }
     }
 
-    /// <summary>
-    /// Starts a new task to compute what item should be selected.
-    /// </summary>
-    private void StartSelectedItemUpdateTask()
-    {
-        // Cancel any in flight work.  This way we don't update until a short lull after the last user event we received.
-        _selectItemQueue.AddWork(cancelExistingWork: true);
-    }
-
-    private async ValueTask SelectItemAsync(CancellationToken cancellationToken)
+    private async ValueTask SelectItemAsync(ImmutableSegmentedList<int> positions, CancellationToken cancellationToken)
     {
-        // Switch to the UI so we can determine where the user is and determine the state the last time we updated
-        // the UI.
-        await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken).NoThrowAwaitable();
-
-        // Cancellation exceptions are ignored in AsyncBatchingWorkQueue, so return without throwing if cancellation
-        // occurred while switching to the main thread.
-        if (cancellationToken.IsCancellationRequested)
-            return;
+        var lastCaretPosition = positions.Last();
 
-        await SelectItemWorkerAsync(cancellationToken).ConfigureAwait(true);
+        // Can grab this directly here as only this queue ever reads or writes to it.
+        var lastPresentedInfo = _lastPresentedInfo;
 
-        // Once we've computed and selected the latest navbar items, pause ourselves if we're no longer visible.
-        // That way we don't consume any machine resources that the user won't even notice.
-        if (_visibilityTracker?.IsVisible(_subjectBuffer) is false)
-            Pause();
-    }
+        // Make a task that waits indefinitely, or until the cancellation token is signaled.
+        var cancellationTriggeredTask = Task.Delay(-1, cancellationToken);
 
-    private async ValueTask SelectItemWorkerAsync(CancellationToken cancellationToken)
-    {
-        _threadingContext.ThrowIfNotOnUIThread();
+        // Get the task representing the computation of the model.
+        var modelTask = _computeModelQueue.WaitUntilCurrentBatchCompletesAsync();
 
-        var currentView = _presenter.TryGetCurrentView();
-        var caretPosition = currentView?.GetCaretPoint(_subjectBuffer);
-        if (!caretPosition.HasValue)
+        var completedTask = await Task.WhenAny(cancellationTriggeredTask, modelTask).ConfigureAwait(false);
+        if (completedTask == cancellationTriggeredTask)
             return;
 
-        var position = caretPosition.Value.Position;
-        var lastPresentedInfo = _lastPresentedInfo;
-
-        // Jump back to the BG to do any expensive work walking the entire model
-        await TaskScheduler.Default;
-
-        // Ensure the latest model is computed.
-        var model = await _computeModelQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(true);
-
-        var currentSelectedItem = ComputeSelectedTypeAndMember(model, position, cancellationToken);
+        var model = await modelTask.ConfigureAwait(false);
+        var currentSelectedItem = ComputeSelectedTypeAndMember(model, lastCaretPosition, cancellationToken);
 
-        GetProjectItems(out var projectItems, out var selectedProjectItem);
+        var (projectItems, selectedProjectItem) = GetProjectItems();
         if (Equals(model, lastPresentedInfo.model) &&
             Equals(currentSelectedItem, lastPresentedInfo.selectedInfo) &&
             Equals(selectedProjectItem, lastPresentedInfo.selectedProjectItem) &&

src/EditorFeatures/Core/Tagging/ITaggerEventSource.cs

@@ -30,14 +30,14 @@ internal interface ITaggerEventSource
     void Disconnect();
 
     /// <summary>
-    /// Pauses this event source and prevents it from firing the <see cref="Changed"/> event. Can be called many
-    /// times (but subsequence calls have no impact if already paused).  Must be called on the UI thread.
+    /// Pauses this event source and prevents it from firing the <see cref="Changed"/> event. Can be called many times
+    /// (but subsequent calls have no impact if already paused).  Must be called on the UI thread.
     /// </summary>
     void Pause();
 
     /// <summary>
     /// Resumes this event source and allows firing the <see cref="Changed"/> event. Can be called many times (but
-    /// subsequence calls have no impact if already resumed).  Must be called on the UI thread.
+    /// subsequent calls have no impact if already resumed).  Must be called on the UI thread.
     /// </summary>
     void Resume();