Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump mac editor and implement CocoaTextBufferVisibilityTracker #65568

Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/Versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<VisualStudioEditorPackagesVersion>17.5.42-preview</VisualStudioEditorPackagesVersion>
<!-- This should generally be set to $(VisualStudioEditorPackagesVersion),
but sometimes EditorFeatures.Cocoa specifically requires a newer editor build. -->
<VisualStudioMacEditorPackagesVersion>17.3.133-preview</VisualStudioMacEditorPackagesVersion>
<VisualStudioMacEditorPackagesVersion>17.5.152-preview-g356e9d7141</VisualStudioMacEditorPackagesVersion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@genlu @allisonchou what branch should this go into? main-vs-deps?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to bump this to a standard daily build version, yes (won't have a commit hash suffix)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is a package from a PR build. Let's change this to 17.5.157-preview (or 17.5.170-preview if we want latest).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually VisualStudioEditorPackagesVersion should be bumped and this property's value should be changed to $(VisualStudioEditorPackagesVersion). But that can happen in another PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this (hopefully?) only affects Mac we shouldn't need to worry about vs-deps. If we did bump VisualStudioEditorPackagesVersion then we might

<ILAsmPackageVersion>5.0.0-alpha1.19409.1</ILAsmPackageVersion>
<ILDAsmPackageVersion>5.0.0-preview.1.20112.8</ILDAsmPackageVersion>
<MicrosoftVisualStudioLanguageServerClientPackagesVersion>17.3.3101</MicrosoftVisualStudioLanguageServerClientPackagesVersion>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.ComponentModel.Composition;
using Microsoft.CodeAnalysis.Editor;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.Text.Editor;

namespace Microsoft.CodeAnalysis.Workspaces
{
[Export(typeof(ITextBufferVisibilityTracker))]
internal sealed class CocoaTextBufferVisibilityTracker
: AbstractTextBufferVisibilityTracker<ICocoaTextView, EventHandler>
{
[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public CocoaTextBufferVisibilityTracker(
ITextBufferAssociatedViewService associatedViewService,
IThreadingContext threadingContext)
: base(associatedViewService, threadingContext)
{
}

protected override bool IsVisible(ICocoaTextView view)
=> view.IsVisible;

protected override EventHandler GetVisiblityChangeCallback(VisibleTrackerData visibleTrackerData)
=> (sender, args) => visibleTrackerData.TriggerCallbacks();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, heh, I didn't think of wrapping in the derived impl.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes!


protected override void AddVisibilityChangedCallback(ICocoaTextView view, EventHandler visibilityChangedCallback)
=> view.IsVisibleChanged += visibilityChangedCallback;

protected override void RemoveVisibilityChangedCallback(ICocoaTextView view, EventHandler visibilityChangedCallback)
=> view.IsVisibleChanged -= visibilityChangedCallback;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,223 +3,39 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Collections.ObjectModel;
using System.ComponentModel.Composition;
using System.Linq;
using System.Windows;
using Microsoft.CodeAnalysis.Editor;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
using Microsoft.CodeAnalysis.Host.Mef;
using Microsoft.VisualStudio.Text;
using Microsoft.VisualStudio.Text.Editor;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.Workspaces
{
[Export(typeof(ITextBufferVisibilityTracker))]
internal sealed class WpfTextBufferVisibilityTracker : ITextBufferVisibilityTracker
internal sealed class WpfTextBufferVisibilityTracker
: AbstractTextBufferVisibilityTracker<IWpfTextView, DependencyPropertyChangedEventHandler>
{
private readonly ITextBufferAssociatedViewService _associatedViewService;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this moved to abstract base class.

private readonly IThreadingContext _threadingContext;

private readonly Dictionary<ITextBuffer, VisibleTrackerData> _subjectBufferToCallbacks = new();

[ImportingConstructor]
[Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
public WpfTextBufferVisibilityTracker(
ITextBufferAssociatedViewService associatedViewService,
IThreadingContext threadingContext)
: base(associatedViewService, threadingContext)
{
_associatedViewService = associatedViewService;
_threadingContext = threadingContext;

associatedViewService.SubjectBuffersConnected += AssociatedViewService_SubjectBuffersConnected;
associatedViewService.SubjectBuffersDisconnected += AssociatedViewService_SubjectBuffersDisconnected;
}

private void AssociatedViewService_SubjectBuffersConnected(object sender, SubjectBuffersConnectedEventArgs e)
{
_threadingContext.ThrowIfNotOnUIThread();
UpdateAllAssociatedViews(e.SubjectBuffers);
}

private void AssociatedViewService_SubjectBuffersDisconnected(object sender, SubjectBuffersConnectedEventArgs e)
{
_threadingContext.ThrowIfNotOnUIThread();
UpdateAllAssociatedViews(e.SubjectBuffers);
}

private void UpdateAllAssociatedViews(ReadOnlyCollection<ITextBuffer> subjectBuffers)
{
// Whenever views get attached/detached from buffers, make sure we're hooked up to the appropriate events
// for them.
foreach (var buffer in subjectBuffers)
{
if (_subjectBufferToCallbacks.TryGetValue(buffer, out var data))
data.UpdateAssociatedViews();
}
}

public bool IsVisible(ITextBuffer subjectBuffer)
{
_threadingContext.ThrowIfNotOnUIThread();
protected override bool IsVisible(IWpfTextView view)
=> view.VisualElement.IsVisible;

var views = _associatedViewService.GetAssociatedTextViews(subjectBuffer).ToImmutableArrayOrEmpty();
protected override DependencyPropertyChangedEventHandler GetVisiblityChangeCallback(VisibleTrackerData visibleTrackerData)
=> (sender, args) => visibleTrackerData.TriggerCallbacks();

// If we don't have any views at all, then assume the buffer is visible.
if (views.Length == 0)
return true;

// if any of the views were *not* wpf text views, assume the buffer is visible. We don't know how to
// determine the visibility of this buffer. While unlikely to happen, this is possible with VS's
// extensibility model, which allows for a plugin to host an ITextBuffer in their own impl of an ITextView.
// For those cases, just assume these buffers are visible.
if (views.Any(static v => v is not IWpfTextView))
return true;

return views.OfType<IWpfTextView>().Any(v => v.VisualElement.IsVisible);
}

public void RegisterForVisibilityChanges(ITextBuffer subjectBuffer, Action callback)
{
_threadingContext.ThrowIfNotOnUIThread();
if (!_subjectBufferToCallbacks.TryGetValue(subjectBuffer, out var data))
{
data = new VisibleTrackerData(this, subjectBuffer);
_subjectBufferToCallbacks.Add(subjectBuffer, data);
}

data.AddCallback(callback);
}

public void UnregisterForVisibilityChanges(ITextBuffer subjectBuffer, Action callback)
{
_threadingContext.ThrowIfNotOnUIThread();

// Both of these methods must succeed. Otherwise we're somehow unregistering something we don't know about.
Contract.ThrowIfFalse(_subjectBufferToCallbacks.TryGetValue(subjectBuffer, out var data));
Contract.ThrowIfFalse(data.Callbacks.Contains(callback));
data.RemoveCallback(callback);

// If we have nothing that wants to listen to information about this buffer anymore, then disconnect it
// from all events and remove our map.
if (data.Callbacks.Count == 0)
{
data.Dispose();
_subjectBufferToCallbacks.Remove(subjectBuffer);
}
}
protected override void AddVisibilityChangedCallback(IWpfTextView view, DependencyPropertyChangedEventHandler visibilityChangedCallback)
=> view.VisualElement.IsVisibleChanged += visibilityChangedCallback;

public TestAccessor GetTestAccessor()
=> new TestAccessor(this);
protected override void RemoveVisibilityChangedCallback(IWpfTextView view, DependencyPropertyChangedEventHandler visibilityChangedCallback)
=> view.VisualElement.IsVisibleChanged -= visibilityChangedCallback;

public readonly struct TestAccessor
{
private readonly WpfTextBufferVisibilityTracker _visibilityTracker;

public TestAccessor(WpfTextBufferVisibilityTracker visibilityTracker)
{
_visibilityTracker = visibilityTracker;
}

public void TriggerCallbacks(ITextBuffer subjectBuffer)
{
var data = _visibilityTracker._subjectBufferToCallbacks[subjectBuffer];
data.TriggerCallbacks();
}
}

private sealed class VisibleTrackerData : IDisposable
{
public readonly HashSet<ITextView> TextViews = new();

private readonly WpfTextBufferVisibilityTracker _tracker;
private readonly ITextBuffer _subjectBuffer;

/// <summary>
/// The callbacks that want to be notified when our <see cref="TextViews"/> change visibility. Stored as an
/// <see cref="ImmutableHashSet{T}"/> so we can enumerate it safely without it changing underneath us.
/// </summary>
public ImmutableHashSet<Action> Callbacks { get; private set; } = ImmutableHashSet<Action>.Empty;

public VisibleTrackerData(
WpfTextBufferVisibilityTracker tracker,
ITextBuffer subjectBuffer)
{
_tracker = tracker;
_subjectBuffer = subjectBuffer;
UpdateAssociatedViews();
}

public void Dispose()
{
_tracker._threadingContext.ThrowIfNotOnUIThread();

// Shouldn't be disposing of this if we still have clients that want to hear about visibility changes.
Contract.ThrowIfTrue(Callbacks.Count > 0);

// Clear out all our textviews. This will disconnect us from any events we have registered with them.
UpdateTextViews(Array.Empty<ITextView>());

Contract.ThrowIfTrue(TextViews.Count > 0);
}

public void AddCallback(Action callback)
{
_tracker._threadingContext.ThrowIfNotOnUIThread();
this.Callbacks = this.Callbacks.Add(callback);
}

public void RemoveCallback(Action callback)
{
_tracker._threadingContext.ThrowIfNotOnUIThread();
this.Callbacks = this.Callbacks.Remove(callback);
}

public void UpdateAssociatedViews()
{
_tracker._threadingContext.ThrowIfNotOnUIThread();

// Update us to whatever the currently associated text views are for this buffer.
UpdateTextViews(_tracker._associatedViewService.GetAssociatedTextViews(_subjectBuffer));
}

private void UpdateTextViews(IEnumerable<ITextView> associatedTextViews)
{
var removedViews = TextViews.Except(associatedTextViews);
var addedViews = associatedTextViews.Except(TextViews);

// Disconnect from hearing about visibility changes for any views we're no longer associated with.
foreach (var removedView in removedViews)
{
if (removedView is IWpfTextView removedWpfView)
removedWpfView.VisualElement.IsVisibleChanged -= VisualElement_IsVisibleChanged;
}

// Connect to hearing about visbility changes for any views we are associated with.
foreach (var addedView in addedViews)
{
if (addedView is IWpfTextView addedWpfView)
addedWpfView.VisualElement.IsVisibleChanged += VisualElement_IsVisibleChanged;
}

TextViews.Clear();
TextViews.AddRange(associatedTextViews);
}

private void VisualElement_IsVisibleChanged(object sender, System.Windows.DependencyPropertyChangedEventArgs e)
{
TriggerCallbacks();
}

internal void TriggerCallbacks()
{
_tracker._threadingContext.ThrowIfNotOnUIThread();
foreach (var callback in Callbacks)
callback();
}
}
}
}
Loading