Dispatch DocumentStructureChanged event on foreground thread.

- Had to add extra logic to track document structure changes so listeners could know if an event was on its way or not.
- Added and fixed some tests.

#1748
This commit is contained in:
N. Taylor Mullen 2017-11-01 16:43:22 -07:00
parent 7002dbf20a
commit a2972ebf1c
4 changed files with 184 additions and 89 deletions

View File

@ -11,7 +11,6 @@ using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Legacy;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.CodeAnalysis.Razor.Editor;
using Microsoft.CodeAnalysis.Razor.ProjectSystem;
using Microsoft.VisualStudio.Language.Intellisense;
using Microsoft.VisualStudio.Text;
using ITextBuffer = Microsoft.VisualStudio.Text.ITextBuffer;
@ -27,6 +26,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
internal TimeSpan IdleDelay = TimeSpan.FromSeconds(3);
internal Timer _idleTimer;
internal BackgroundParser _parser;
internal ChangeReference _latestChangeReference;
private readonly object IdleLock = new object();
private readonly ICompletionBroker _completionBroker;
@ -96,6 +96,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
public override ITextBuffer TextBuffer => _documentTracker.TextBuffer;
public override bool HasPendingChanges => _latestChangeReference != null;
// Used in unit tests to ensure we can be notified when idle starts.
internal ManualResetEventSlim NotifyForegroundIdleStart { get; set; }
@ -253,7 +255,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
// If partial parsing failed or there were outstanding parser tasks, start a full reparse
if ((result & PartialParseResultInternal.Rejected) == PartialParseResultInternal.Rejected)
{
_parser.QueueChange(change, snapshot);
QueueChange(change, snapshot);
}
if ((result & PartialParseResultInternal.Provisional) == PartialParseResultInternal.Provisional)
@ -292,7 +294,15 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
var snapshot = TextBuffer.CurrentSnapshot;
_parser.QueueChange(null, snapshot);
QueueChange(null, snapshot);
}
private void QueueChange(SourceChange change, ITextSnapshot snapshot)
{
_dispatcher.AssertForegroundThread();
_latestChangeReference = new ChangeReference(change, snapshot);
_parser.QueueChange(change, snapshot);
}
private void OnNotifyForegroundIdle()
@ -311,7 +321,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
}
private async void Timer_Tick(object state)
private void Timer_Tick(object state)
{
try
{
@ -322,12 +332,12 @@ namespace Microsoft.VisualStudio.Editor.Razor
StopIdleTimer();
// We need to get back to the UI thread to properly check if a completion is active.
await Task.Factory.StartNew(OnIdle, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
Task.Factory.StartNew(OnIdle, null, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
}
catch (Exception ex)
{
// This is something totally unexpected, let's just send it over to the workspace.
await Task.Factory.StartNew(() => _errorReporter.ReportError(ex), CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
Task.Factory.StartNew(() => _errorReporter.ReportError(ex), CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
}
}
@ -335,19 +345,29 @@ namespace Microsoft.VisualStudio.Editor.Razor
{
_dispatcher.AssertBackgroundThread();
if (DocumentStructureChanged != null)
{
if (args.Snapshot != TextBuffer.CurrentSnapshot)
{
// A different text change is being parsed.
return;
}
// Jump back to UI thread to notify structure changes.
Task.Factory.StartNew(OnDocumentStructureChanged, args, CancellationToken.None, TaskCreationOptions.None, _dispatcher.ForegroundScheduler);
}
_codeDocument = args.CodeDocument;
_snapshot = args.Snapshot;
_partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree());
DocumentStructureChanged(this, args);
// Internal for testing
internal void OnDocumentStructureChanged(object state)
{
_dispatcher.AssertForegroundThread();
var args = (DocumentStructureChangedEventArgs)state;
if (_latestChangeReference == null || // extra hardening
!_latestChangeReference.IsAssociatedWith(args))
{
// In the middle of parsing a newer change.
return;
}
_latestChangeReference = null;
_codeDocument = args.CodeDocument;
_snapshot = args.Snapshot;
_partialParser = new RazorSyntaxTreePartialParser(CodeDocument.GetSyntaxTree());
DocumentStructureChanged?.Invoke(this, args);
}
private void ConfigureTemplateEngine(IRazorEngineBuilder builder)
@ -399,5 +419,25 @@ namespace Microsoft.VisualStudio.Editor.Razor
return Array.Empty<TagHelperDescriptor>();
}
}
// Internal for testing
internal class ChangeReference
{
public ChangeReference(SourceChange change, ITextSnapshot snapshot)
{
Change = change;
Snapshot = snapshot;
}
public SourceChange Change { get; }
public ITextSnapshot Snapshot { get; }
public bool IsAssociatedWith(DocumentStructureChangedEventArgs other)
{
return Change == other.SourceChange &&
Snapshot == other.Snapshot;
}
}
}
}

View File

@ -21,6 +21,8 @@ namespace Microsoft.VisualStudio.Editor.Razor
public abstract ITextBuffer TextBuffer { get; }
public abstract bool HasPendingChanges { get; }
public abstract void QueueReparse();
}
}

View File

@ -27,44 +27,30 @@ namespace Microsoft.VisualStudio.Editor.Razor
private const string TestProjectPath = "C:\\This\\Path\\Is\\Just\\For\\Project.csproj";
[ForegroundFact]
public void BufferChangeStartsFullReparseIfChangeOverlapsMultipleSpans()
public async Task BufferChangeStartsFullReparseIfChangeOverlapsMultipleSpans()
{
// Arrange
var original = new StringTextSnapshot("Foo @bar Baz");
var testBuffer = new TestTextBuffer(original);
var documentTracker = CreateDocumentTracker(testBuffer);
using (var parser = new DefaultVisualStudioRazorParser(
Dispatcher,
documentTracker,
CreateTemplateEngineFactory(),
new DefaultErrorReporter(),
new TestCompletionBroker()))
using (var manager = CreateParserManager(original))
{
parser.DocumentTracker_ContextChanged(null, null);
var changed = new StringTextSnapshot("Foo @bap Daz");
var edit = new TestEdit(7, 3, original, 3, changed, "p D");
var parseComplete = new ManualResetEventSlim();
var parseCount = 0;
parser.DocumentStructureChanged += (s, a) =>
{
Interlocked.Increment(ref parseCount);
parseComplete.Set();
};
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// Act - 1
testBuffer.ApplyEdit(edit);
DoWithTimeoutIfNotDebugging(parseComplete.Wait); // Wait for the parse to finish
await manager.ApplyEditAndWaitForParseAsync(edit);
// Assert - 1
Assert.Equal(1, parseCount);
parseComplete.Reset();
Assert.Equal(2, manager.ParseCount);
// Act - 2
testBuffer.ApplyEdit(edit);
await manager.ApplyEditAndWaitForParseAsync(edit);
// Assert - 2
DoWithTimeoutIfNotDebugging(parseComplete.Wait);
Assert.Equal(2, parseCount);
Assert.Equal(3, manager.ParseCount);
}
}
@ -78,7 +64,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
var factory = new SpanFactory();
var changed = new StringTextSnapshot("foo @await Html. baz");
var edit = new TestEdit(15, 0, original, 1, changed, ".");
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// Act
await manager.ApplyEditAndWaitForReparseAsync(edit);
@ -95,7 +81,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
[ForegroundFact]
public void ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlockAfterIdentifiers()
public async Task ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlockAfterIdentifiers()
{
var factory = new SpanFactory();
var changed = new StringTextSnapshot("@{" + Environment.NewLine
@ -130,7 +116,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
factory.EmptyHtml()));
};
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// This is the process of a dotless commit when doing "." insertions to commit intellisense changes.
ApplyAndVerifyPartialChange(edit, "DateTime.");
@ -154,7 +140,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
[ForegroundFact]
public void ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlock()
public async Task ImplicitExpressionAcceptsDotlessCommitInsertionsInStatementBlock()
{
var factory = new SpanFactory();
var changed = new StringTextSnapshot("@{" + Environment.NewLine
@ -189,7 +175,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
factory.EmptyHtml()));
};
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// This is the process of a dotless commit when doing "." insertions to commit intellisense changes.
ApplyAndVerifyPartialChange(edit, "DateT.");
@ -226,7 +212,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
factory.Markup(" baz")));
};
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// This is the process of a dotless commit when doing "." insertions to commit intellisense changes.
ApplyAndVerifyPartialChange(edit, "DateT.");
@ -272,7 +258,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
factory.Markup(" baz")));
};
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// This is the process of a dotless commit when doing "." insertions to commit intellisense changes.
ApplyAndVerifyPartialChange(edit, "DateTime.");
@ -324,7 +310,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
factory.Markup(" baz")));
};
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// This is the process of a dotless commit when doing "." insertions to commit intellisense changes.
@ -374,13 +360,13 @@ namespace Microsoft.VisualStudio.Editor.Razor
var charTyped = new TestEdit(14, 0, new StringTextSnapshot("foo @foo. @bar"), 1, new StringTextSnapshot("foo @foo. @barb"), "b");
using (var manager = CreateParserManager(dotTyped.OldSnapshot))
{
manager.InitializeWithDocument(dotTyped.OldSnapshot);
await manager.InitializeWithDocumentAsync(dotTyped.OldSnapshot);
// Apply the dot change
await manager.ApplyEditAndWaitForReparseAsync(dotTyped);
// Act (apply the identifier start char change)
manager.ApplyEditAndWaitForParse(charTyped);
await manager.ApplyEditAndWaitForParseAsync(charTyped);
// Assert
Assert.Equal(2, manager.ParseCount);
@ -403,7 +389,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
[ForegroundFact]
public void ImplicitExpressionAcceptsIdentifierTypedAfterDotIfLastChangeWasProvisionalAcceptanceOfDot()
public async Task ImplicitExpressionAcceptsIdentifierTypedAfterDotIfLastChangeWasProvisionalAcceptanceOfDot()
{
// Arrange
var factory = new SpanFactory();
@ -411,7 +397,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
var charTyped = new TestEdit(9, 0, new StringTextSnapshot("foo @foo. bar"), 1, new StringTextSnapshot("foo @foo.b bar"), "b");
using (var manager = CreateParserManager(dotTyped.OldSnapshot))
{
manager.InitializeWithDocument(dotTyped.OldSnapshot);
await manager.InitializeWithDocumentAsync(dotTyped.OldSnapshot);
// Apply the dot change
manager.ApplyEdit(dotTyped);
@ -434,87 +420,87 @@ namespace Microsoft.VisualStudio.Editor.Razor
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfIfKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfIfKeywordTyped()
{
RunTypeKeywordTest("if");
await RunTypeKeywordTestAsync("if");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfDoKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfDoKeywordTyped()
{
RunTypeKeywordTest("do");
await RunTypeKeywordTestAsync("do");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfTryKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfTryKeywordTyped()
{
RunTypeKeywordTest("try");
await RunTypeKeywordTestAsync("try");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfForKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfForKeywordTyped()
{
RunTypeKeywordTest("for");
await RunTypeKeywordTestAsync("for");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfForEachKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfForEachKeywordTyped()
{
RunTypeKeywordTest("foreach");
await RunTypeKeywordTestAsync("foreach");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfWhileKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfWhileKeywordTyped()
{
RunTypeKeywordTest("while");
await RunTypeKeywordTestAsync("while");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfSwitchKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfSwitchKeywordTyped()
{
RunTypeKeywordTest("switch");
await RunTypeKeywordTestAsync("switch");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfLockKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfLockKeywordTyped()
{
RunTypeKeywordTest("lock");
await RunTypeKeywordTestAsync("lock");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfUsingKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfUsingKeywordTyped()
{
RunTypeKeywordTest("using");
await RunTypeKeywordTestAsync("using");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfSectionKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfSectionKeywordTyped()
{
RunTypeKeywordTest("section");
await RunTypeKeywordTestAsync("section");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfInheritsKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfInheritsKeywordTyped()
{
RunTypeKeywordTest("inherits");
await RunTypeKeywordTestAsync("inherits");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfFunctionsKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfFunctionsKeywordTyped()
{
RunTypeKeywordTest("functions");
await RunTypeKeywordTestAsync("functions");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfNamespaceKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfNamespaceKeywordTyped()
{
RunTypeKeywordTest("namespace");
await RunTypeKeywordTestAsync("namespace");
}
[ForegroundFact]
public void ImplicitExpressionCorrectlyTriggersReparseIfClassKeywordTyped()
public async Task ImplicitExpressionCorrectlyTriggersReparseIfClassKeywordTyped()
{
RunTypeKeywordTest("class");
await RunTypeKeywordTestAsync("class");
}
private TestParserManager CreateParserManager(ITextSnapshot originalSnapshot)
@ -570,7 +556,7 @@ namespace Microsoft.VisualStudio.Editor.Razor
return templateEngineFactory;
}
private void RunTypeKeywordTest(string keyword)
private async Task RunTypeKeywordTestAsync(string keyword)
{
// Arrange
var before = "@" + keyword.Substring(0, keyword.Length - 1);
@ -581,10 +567,10 @@ namespace Microsoft.VisualStudio.Editor.Razor
var edit = new TestEdit(change, old, changed);
using (var manager = CreateParserManager(old))
{
manager.InitializeWithDocument(edit.OldSnapshot);
await manager.InitializeWithDocumentAsync(edit.OldSnapshot);
// Act
manager.ApplyEditAndWaitForParse(edit);
await manager.ApplyEditAndWaitForParseAsync(edit);
// Assert
Assert.Equal(2, manager.ParseCount);
@ -657,12 +643,12 @@ namespace Microsoft.VisualStudio.Editor.Razor
public RazorSyntaxTree CurrentSyntaxTree { get; private set; }
public void InitializeWithDocument(ITextSnapshot snapshot)
public async Task InitializeWithDocumentAsync(ITextSnapshot snapshot)
{
var old = new StringTextSnapshot(string.Empty);
var initialChange = new SourceChange(0, 0, snapshot.GetText());
var edit = new TestEdit(initialChange, old, snapshot);
ApplyEditAndWaitForParse(edit);
await ApplyEditAndWaitForParseAsync(edit);
}
public void ApplyEdit(TestEdit edit)
@ -670,10 +656,10 @@ namespace Microsoft.VisualStudio.Editor.Razor
_testBuffer.ApplyEdit(edit);
}
public void ApplyEditAndWaitForParse(TestEdit edit)
public async Task ApplyEditAndWaitForParseAsync(TestEdit edit)
{
ApplyEdit(edit);
WaitForParse();
await WaitForParseAsync();
}
public async Task ApplyEditAndWaitForReparseAsync(TestEdit edit)
@ -682,9 +668,14 @@ namespace Microsoft.VisualStudio.Editor.Razor
await WaitForReparseAsync();
}
public void WaitForParse()
public async Task WaitForParseAsync()
{
DoWithTimeoutIfNotDebugging(_parserComplete.Wait); // Wait for the parse to finish
// Get off of the foreground thread so we can wait for the document structure changed event to fire
await Task.Run(() =>
{
DoWithTimeoutIfNotDebugging(_parserComplete.Wait);
});
_parserComplete.Reset();
}
@ -703,7 +694,11 @@ namespace Microsoft.VisualStudio.Editor.Razor
Assert.Null(_parser._idleTimer);
DoWithTimeoutIfNotDebugging(_reparseComplete.Wait);
// Get off of the foreground thread so we can wait for the document structure changed event to fire for reparse
await Task.Run(() =>
{
DoWithTimeoutIfNotDebugging(_reparseComplete.Wait);
});
_reparseComplete.Reset();
_parser.BlockBackgroundIdleWork.Reset();

View File

@ -4,6 +4,7 @@
using System;
using System.Linq;
using System.Threading;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.CodeAnalysis.Razor;
using Microsoft.VisualStudio.Language.Intellisense;
using Microsoft.VisualStudio.Test;
@ -26,6 +27,63 @@ namespace Microsoft.VisualStudio.Editor.Razor
return documentTracker;
}
[ForegroundFact]
public void OnDocumentStructureChanged_IgnoresEditsThatAreOld()
{
// Arrange
using (var parser = new DefaultVisualStudioRazorParser(
Dispatcher,
CreateDocumentTracker(),
Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>()))
{
var called = false;
parser.DocumentStructureChanged += (sender, e) => called = true;
parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(null, new StringTextSnapshot(string.Empty));
var args = new DocumentStructureChangedEventArgs(
new SourceChange(0, 0, string.Empty),
new StringTextSnapshot(string.Empty),
TestRazorCodeDocument.CreateEmpty());
// Act
parser.OnDocumentStructureChanged(args);
// Assert
Assert.False(called);
}
}
[ForegroundFact]
public void OnDocumentStructureChanged_FiresForLatestTextBufferEdit()
{
// Arrange
var documentTracker = CreateDocumentTracker();
using (var parser = new DefaultVisualStudioRazorParser(
Dispatcher,
documentTracker,
Mock.Of<RazorTemplateEngineFactoryService>(),
new DefaultErrorReporter(),
Mock.Of<ICompletionBroker>()))
{
var called = false;
parser.DocumentStructureChanged += (sender, e) => called = true;
var latestChange = new SourceChange(0, 0, string.Empty);
var latestSnapshot = documentTracker.TextBuffer.CurrentSnapshot;
parser._latestChangeReference = new DefaultVisualStudioRazorParser.ChangeReference(latestChange, latestSnapshot);
var args = new DocumentStructureChangedEventArgs(
latestChange,
latestSnapshot,
TestRazorCodeDocument.CreateEmpty());
// Act
parser.OnDocumentStructureChanged(args);
// Assert
Assert.True(called);
}
}
[ForegroundFact]
public void StartIdleTimer_DoesNotRestartTimerWhenAlreadyRunning()
{