Fix issue classifying while performing a layout

[CyrusNajmabadi]

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1870371

The core idea here came from an observation @AmadeusW had where he noticed a loop during layout. Specifically, during layout, a call was being made into roslyn to get tags, which caused us to then notify about the entire document changing.

This made me spelunk a bit into our classification taggers and how they operate. For background, our "classification tagger" is a "roslyn view tagger". This means that it is a tagger on an ITextView (not an ITextBuffer) and it operates by having 3 constituent taggers responsible for tagging a small portion of the file that's above the view, what's in view, and a small portion of the file below the view. What is in-view recomputes tags quickly, while what is out of view recomputes slowly. We classify what is out of view so that as you scroll those portions in, classification is already ready and you don't observe too much 'snapping' as unclassified text becomes classified (of course, you can still observe this if you change to an entirely different sectio of the file. but that is less common than just typing in one section and moving up and down it).

However, in order for the above to work, it's necessary for the view taggers to be able to determine "what is in view?". The helper it uses for this has several limitations on it due to restrictions in the editor layer itself. For example, we cannot get this information during a layout call itself.

Prior to this change, the view tagger would effectively say "if i cannot figure out the view, just classify the entire buffer". This was problematic as it was now trivial to get into a ping pong effect where we wanted to classify a portion of hte view, then the entire view, then back again. And each of these would cause us to create different tags (either addign a lot of tags or removing them) causing us to call back into the editor for the affected region (often the entire file).

The fix here is to detect this and gracefully bail out, doing no tagging (instead of tagging everything). If this happens, we just enqueue a new request to tag the views again at a later time when we're ideally no longer in a layout.

[AmadeusW]

Nice job! It would be great to follow the insertion and see performance impact

[olegtk]

The helper it uses for this has several limitations on it due to restrictions in the editor layer itself. For example, we cannot get this information during a layout call itself.

is there anything we improve on the editor layer here, @AmadeusW, @CyrusNajmabadi ?

[CyrusNajmabadi]

@olegtk @AmadeusW I think this is only part of the fix. Working on the rest tomorrow.

[CyrusNajmabadi]

@olegtk @AmadeusW Here are the set of things that cause us to bail out:

1. The text view is closed. This seems reasonable. If the view is closed, tagging is pointless :)
2. The view gives us back 'null' for .TextViewLines. Doc comments say this happens during initialization. Not sure if that's still correct, but it seems like it would make sense to not load any taggers until the point that a view had .TextViewLines ready.
3. There is no visible part of our subject buffer in the text view. This makes sense. If this is some sort of projection scenario, and what is on screen contains nothing of C#/VB, then don't classify.
4. The view is in Layout. All our code says is: "Much of the text view state is unsafe to access (and will throw)."

So '4' really is the thing that is problematic. Unfortunately, i don't actually know what this comment is referring to. If you want to see our code, it is here:

roslyn/src/EditorFeatures/Core/Shared/Extensions/ITextViewExtensions.cs

Line 328 in 5c14a31

public static SnapshotSpan? GetVisibleLinesSpan(this ITextView textView, ITextBuffer subjectBuffer, int extraLines = 0)

All we access is the following:

        // Determine the range of text that is visible in the view.  Then map this down to the
        // bufffer passed in.  From that, determine the start/end line for the buffer that is in
        // view.
        var visibleSpan = textView.TextViewLines.FormattedSpan;
        var visibleSpansInBuffer = textView.BufferGraph.MapDownToBuffer(visibleSpan, SpanTrackingMode.EdgeInclusive, subjectBuffer);
        if (visibleSpansInBuffer.Count == 0)
        {
            return null;
        }

        var visibleStart = visibleSpansInBuffer.First().Start;
        var visibleEnd = visibleSpansInBuffer.Last().End;

        var snapshot = subjectBuffer.CurrentSnapshot;
        var startLine = visibleStart.GetContainingLineNumber();
        var endLine = visibleEnd.GetContainingLineNumber();

        startLine = Math.Max(startLine - extraLines, 0);
        endLine = Math.Min(endLine + extraLines, snapshot.LineCount - 1);

        var start = snapshot.GetLineFromLineNumber(startLine).Start;
        var end = snapshot.GetLineFromLineNumber(endLine).EndIncludingLineBreak;

        var span = new SnapshotSpan(snapshot, Span.FromBounds(start, end));

So if we could make that stuff safe to access (maybe by returning the last correct stale result for it), then we could not bail out. Thoughts?