From c20e618b61efc1ae5569408e7dd797ddda0e1220 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 20 Mar 2018 21:58:04 +0000 Subject: [PATCH 01/25] Create MEF commands on background thread There is no need to do this on the Main thread. --- src/GitHub.InlineReviews/InlineReviewsPackage.cs | 9 ++++++--- src/GitHub.VisualStudio/GitHubPackage.cs | 12 +++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index 24f9c105a4..7d06563566 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -38,11 +38,14 @@ async Task InitializeMenus() var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; + var commands = new IVsCommandBase[] + { + exports.GetExportedValue(), + exports.GetExportedValue() + }; await JoinableTaskFactory.SwitchToMainThreadAsync(); - menuService.AddCommands( - exports.GetExportedValue(), - exports.GetExportedValue()); + menuService.AddCommands(commands); } } } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 233ce50840..72f80b8040 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -7,7 +7,6 @@ using System.Runtime.InteropServices; using GitHub.Api; using GitHub.Commands; -using GitHub.Helpers; using GitHub.Info; using GitHub.Exports; using GitHub.Logging; @@ -59,9 +58,8 @@ async Task InitializeMenus() var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; - - await JoinableTaskFactory.SwitchToMainThreadAsync(); - menuService.AddCommands( + var commands = new IVsCommandBase[] + { exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), @@ -69,7 +67,11 @@ async Task InitializeMenus() exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), - exports.GetExportedValue()); + exports.GetExportedValue() + }; + + await JoinableTaskFactory.SwitchToMainThreadAsync(); + menuService.AddCommands(commands); } async Task EnsurePackageLoaded(Guid packageGuid) From 05274979782263b92a887b8ec41b7f148ec174e5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 21 Mar 2018 10:20:54 +0000 Subject: [PATCH 02/25] Allow menuService.AddCommands to be called from a b/g thread --- .../InlineReviewsPackage.cs | 26 ++++++------------- src/GitHub.VisualStudio/GitHubPackage.cs | 21 +++++++-------- 2 files changed, 18 insertions(+), 29 deletions(-) diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index 7d06563566..d87584a44d 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -8,7 +8,6 @@ using GitHub.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; using Microsoft.VisualStudio.Shell; -using Microsoft.VisualStudio.Threading; using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews @@ -28,24 +27,15 @@ protected override async Task InitializeAsync( var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; - // Avoid delays when there is ongoing UI activity. - // See: https://github.com/github/VisualStudio/issues/1537 - await JoinableTaskFactory.RunAsync(VsTaskRunContext.UIThreadNormalPriority, InitializeMenus); - } - - async Task InitializeMenus() - { - var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); - var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); - var exports = componentModel.DefaultExportProvider; - var commands = new IVsCommandBase[] - { + menuService.AddCommands( exports.GetExportedValue(), - exports.GetExportedValue() - }; - - await JoinableTaskFactory.SwitchToMainThreadAsync(); - menuService.AddCommands(commands); + exports.GetExportedValue()); } + + // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). + // When called from a non-Main thread this would throw despite the fact these services don't exist. + // This override allows IMenuCommandService.AddCommands to be called form a background thread. + protected override object GetService(Type serviceType) + => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); } } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 72f80b8040..d28133ad8b 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -39,12 +39,15 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke LogVersionInformation(); await base.InitializeAsync(cancellationToken, progress); await GetServiceAsync(typeof(IUsageTracker)); - - // Avoid delays when there is ongoing UI activity. - // See: https://github.com/github/VisualStudio/issues/1537 - await JoinableTaskFactory.RunAsync(VsTaskRunContext.UIThreadNormalPriority, InitializeMenus); + await InitializeMenus(); } + // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). + // When called from a non-Main thread this would throw despite the fact these services don't exist. + // This override allows IMenuCommandService.AddCommands to be called form a background thread. + protected override object GetService(Type serviceType) + => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); + void LogVersionInformation() { var packageVersion = ApplicationInfo.GetPackageVersion(this); @@ -58,8 +61,8 @@ async Task InitializeMenus() var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; - var commands = new IVsCommandBase[] - { + + menuService.AddCommands( exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), @@ -67,11 +70,7 @@ async Task InitializeMenus() exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), - exports.GetExportedValue() - }; - - await JoinableTaskFactory.SwitchToMainThreadAsync(); - menuService.AddCommands(commands); + exports.GetExportedValue()); } async Task EnsurePackageLoaded(Guid packageGuid) From 266ec58ffd1aae3621524786f917d9ca8ae05fb9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 21 Mar 2018 12:35:18 +0000 Subject: [PATCH 03/25] Construct PullRequestStatusBarManager using MEF on b/g thread Call the IShowCurrentPullRequestCommand directly rather than using DTE. --- .../PullRequestStatusBarPackage.cs | 8 +-- .../Services/PullRequestStatusBarManager.cs | 67 +++++-------------- 2 files changed, 19 insertions(+), 56 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 30cdf34150..55eeb756e6 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,12 +1,12 @@ using System; using System.Threading; using System.Runtime.InteropServices; -using GitHub.Services; using GitHub.VisualStudio; using GitHub.InlineReviews.Services; using Microsoft.VisualStudio.Shell; using Task = System.Threading.Tasks.Task; using Microsoft.VisualStudio.Threading; +using Microsoft.VisualStudio.ComponentModelHost; namespace GitHub.InlineReviews { @@ -27,9 +27,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke async Task InitializeStatusBar() { - var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); - var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); - var barManager = new PullRequestStatusBarManager(usageTracker, serviceProvider); + var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); + var exports = componentModel.DefaultExportProvider; + var barManager = exports.GetExportedValue(); await JoinableTaskFactory.SwitchToMainThreadAsync(); barManager.StartShowingStatus(); diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index c013f1ce5b..84fd72346c 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -1,16 +1,14 @@ using System; using System.Windows; -using System.Windows.Input; using System.Windows.Controls; using System.Windows.Controls.Primitives; using System.ComponentModel.Composition; +using GitHub.Commands; using GitHub.InlineReviews.Views; using GitHub.InlineReviews.ViewModels; using GitHub.Services; -using GitHub.VisualStudio; using GitHub.Models; using GitHub.Logging; -using GitHub.Extensions; using Serilog; using ReactiveUI; @@ -19,21 +17,28 @@ namespace GitHub.InlineReviews.Services /// /// Manage the UI that shows the PR for the current branch. /// + [Export(typeof(PullRequestStatusBarManager))] public class PullRequestStatusBarManager { static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; readonly IUsageTracker usageTracker; - readonly IGitHubServiceProvider serviceProvider; + readonly IShowCurrentPullRequestCommand showCurrentPullRequestCommand; - IPullRequestSessionManager pullRequestSessionManager; + // More the moment this must be constructed on the main thread. + // TeamExplorerContext needs to retrieve DTE using GetService. + readonly Lazy pullRequestSessionManager; [ImportingConstructor] - public PullRequestStatusBarManager(IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) + public PullRequestStatusBarManager( + IUsageTracker usageTracker, + IShowCurrentPullRequestCommand showCurrentPullRequestCommand, + Lazy pullRequestSessionManager) { this.usageTracker = usageTracker; - this.serviceProvider = serviceProvider; + this.showCurrentPullRequestCommand = showCurrentPullRequestCommand; + this.pullRequestSessionManager = pullRequestSessionManager; } /// @@ -46,8 +51,7 @@ public void StartShowingStatus() { try { - pullRequestSessionManager = serviceProvider.GetService(); - pullRequestSessionManager.WhenAnyValue(x => x.CurrentSession) + pullRequestSessionManager.Value.WhenAnyValue(x => x.CurrentSession) .Subscribe(x => RefreshCurrentSession()); } catch (Exception e) @@ -58,16 +62,14 @@ public void StartShowingStatus() void RefreshCurrentSession() { - var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; + var pullRequest = pullRequestSessionManager.Value.CurrentSession?.PullRequest; var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(pullRequest) : null; ShowStatus(viewModel); } PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pullRequest) { - var dte = serviceProvider.TryGetService(); - var command = new RaisePullRequestCommand(dte, usageTracker); - var pullRequestStatusViewModel = new PullRequestStatusViewModel(command); + var pullRequestStatusViewModel = new PullRequestStatusViewModel(showCurrentPullRequestCommand); pullRequestStatusViewModel.Number = pullRequest.Number; pullRequestStatusViewModel.Title = pullRequest.Title; return pullRequestStatusViewModel; @@ -111,44 +113,5 @@ StatusBar FindSccStatusBar(Window mainWindow) var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl; return contentControl?.Content as StatusBar; } - - class RaisePullRequestCommand : ICommand - { - readonly string guid = Guids.guidGitHubCmdSetString; - readonly int id = PkgCmdIDList.showCurrentPullRequestCommand; - - readonly EnvDTE.DTE dte; - readonly IUsageTracker usageTracker; - - internal RaisePullRequestCommand(EnvDTE.DTE dte, IUsageTracker usageTracker) - { - this.dte = dte; - this.usageTracker = usageTracker; - } - - public bool CanExecute(object parameter) => true; - - public void Execute(object parameter) - { - try - { - object customIn = null; - object customOut = null; - dte?.Commands.Raise(guid, id, ref customIn, ref customOut); - } - catch (Exception e) - { - log.Error(e, "Couldn't raise {Guid}:{ID}", guid, id); - } - - usageTracker.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget(); - } - - public event EventHandler CanExecuteChanged - { - add { } - remove { } - } - } } } From 5730008aca33278b90f14e714380cc7c817bbb0b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 21 Mar 2018 13:03:44 +0000 Subject: [PATCH 04/25] Factor b/g loading menu code into AsyncMenuPackage --- .../InlineReviewsPackage.cs | 16 ++------ src/GitHub.Services.Vssdk/AsyncMenuPackage.cs | 41 +++++++++++++++++++ .../GitHub.Services.Vssdk.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 34 +++++---------- 4 files changed, 56 insertions(+), 36 deletions(-) create mode 100644 src/GitHub.Services.Vssdk/AsyncMenuPackage.cs diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index d87584a44d..b4741af9d8 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -1,9 +1,8 @@ using System; -using System.ComponentModel.Design; using System.Runtime.InteropServices; -using System.Threading; using GitHub.Commands; using GitHub.InlineReviews.Views; +using GitHub.Services.Vssdk; using GitHub.Services.Vssdk.Commands; using GitHub.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; @@ -17,13 +16,10 @@ namespace GitHub.InlineReviews [ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)] [ProvideMenuResource("Menus.ctmenu", 1)] [ProvideToolWindow(typeof(PullRequestCommentsPane), DocumentLikeTool = true)] - public class InlineReviewsPackage : AsyncPackage + public class InlineReviewsPackage : AsyncMenuPackage { - protected override async Task InitializeAsync( - CancellationToken cancellationToken, - IProgress progress) + protected override async Task InitializeMenusAsync(OleMenuCommandService menuService) { - var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; @@ -31,11 +27,5 @@ protected override async Task InitializeAsync( exports.GetExportedValue(), exports.GetExportedValue()); } - - // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). - // When called from a non-Main thread this would throw despite the fact these services don't exist. - // This override allows IMenuCommandService.AddCommands to be called form a background thread. - protected override object GetService(Type serviceType) - => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); } } diff --git a/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs b/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs new file mode 100644 index 0000000000..1b3230e255 --- /dev/null +++ b/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs @@ -0,0 +1,41 @@ +using System; +using System.Threading; +using System.ComponentModel.Design; +using Microsoft.VisualStudio.Shell; +using Microsoft.VisualStudio.Shell.Interop; +using Task = System.Threading.Tasks.Task; + +namespace GitHub.Services.Vssdk +{ + public abstract class AsyncMenuPackage : AsyncPackage + { + IVsUIShell vsUIShell; + + sealed protected async override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) + { + vsUIShell = await GetServiceAsync(typeof(SVsUIShell)) as IVsUIShell; + + var menuCommandService = (OleMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); + await InitializeMenusAsync(menuCommandService); + } + + protected abstract Task InitializeMenusAsync(OleMenuCommandService menuService); + + // The IDesignerHost, ISelectionService and IVsUIShell are requested by the MenuCommandService. + // This override allows IMenuCommandService.AddCommands to be called form a background thread. + protected override object GetService(Type serviceType) + { + if (serviceType == typeof(SVsUIShell)) + { + return vsUIShell; + } + + if (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) + { + return null; + } + + return base.GetService(serviceType); + } + } +} diff --git a/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj b/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj index 1bd5ab4904..59f3944c38 100644 --- a/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj +++ b/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj @@ -139,6 +139,7 @@ Properties\SolutionInfo.cs + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index d28133ad8b..6fee2b11cd 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -2,7 +2,6 @@ using System.Windows; using System.Threading; using System.Threading.Tasks; -using System.ComponentModel.Design; using System.ComponentModel.Composition; using System.Runtime.InteropServices; using GitHub.Api; @@ -14,6 +13,7 @@ using GitHub.Services.Vssdk.Commands; using GitHub.ViewModels.GitHubPane; using GitHub.VisualStudio.UI; +using GitHub.Services.Vssdk; using Microsoft.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; using Microsoft.VisualStudio.Shell; @@ -30,35 +30,15 @@ namespace GitHub.VisualStudio [ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)] [ProvideToolWindow(typeof(GitHubPane), Orientation = ToolWindowOrientation.Right, Style = VsDockStyle.Tabbed, Window = EnvDTE.Constants.vsWindowKindSolutionExplorer)] [ProvideOptionPage(typeof(OptionsPage), "GitHub for Visual Studio", "General", 0, 0, supportsAutomation: true)] - public class GitHubPackage : AsyncPackage + public class GitHubPackage : AsyncMenuPackage { static readonly ILogger log = LogManager.ForContext(); - protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) + protected async override Task InitializeMenusAsync(OleMenuCommandService menuService) { LogVersionInformation(); - await base.InitializeAsync(cancellationToken, progress); await GetServiceAsync(typeof(IUsageTracker)); - await InitializeMenus(); - } - - // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). - // When called from a non-Main thread this would throw despite the fact these services don't exist. - // This override allows IMenuCommandService.AddCommands to be called form a background thread. - protected override object GetService(Type serviceType) - => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); - - void LogVersionInformation() - { - var packageVersion = ApplicationInfo.GetPackageVersion(this); - var hostVersionInfo = ApplicationInfo.GetHostVersionInfo(); - log.Information("Initializing GitHub Extension v{PackageVersion} in {$FileDescription} ({$ProductVersion})", - packageVersion, hostVersionInfo.FileDescription, hostVersionInfo.ProductVersion); - } - async Task InitializeMenus() - { - var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; @@ -73,6 +53,14 @@ async Task InitializeMenus() exports.GetExportedValue()); } + void LogVersionInformation() + { + var packageVersion = ApplicationInfo.GetPackageVersion(this); + var hostVersionInfo = ApplicationInfo.GetHostVersionInfo(); + log.Information("Initializing GitHub Extension v{PackageVersion} in {$FileDescription} ({$ProductVersion})", + packageVersion, hostVersionInfo.FileDescription, hostVersionInfo.ProductVersion); + } + async Task EnsurePackageLoaded(Guid packageGuid) { var shell = await GetServiceAsync(typeof(SVsShell)) as IVsShell; From ec469547bdf9c18a95aa17754400b9694323e314 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 21 Mar 2018 14:44:12 +0000 Subject: [PATCH 05/25] Restore missing metrics --- .../Services/PullRequestStatusBarManager.cs | 45 ++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 84fd72346c..34188850b9 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -1,5 +1,6 @@ using System; using System.Windows; +using System.Windows.Input; using System.Windows.Controls; using System.Windows.Controls.Primitives; using System.ComponentModel.Composition; @@ -9,6 +10,7 @@ using GitHub.Services; using GitHub.Models; using GitHub.Logging; +using GitHub.Extensions; using Serilog; using ReactiveUI; @@ -69,12 +71,17 @@ void RefreshCurrentSession() PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pullRequest) { - var pullRequestStatusViewModel = new PullRequestStatusViewModel(showCurrentPullRequestCommand); + var trackingCommand = new UsageTrackingCommand(showCurrentPullRequestCommand, usageTracker); + var pullRequestStatusViewModel = new PullRequestStatusViewModel(trackingCommand); pullRequestStatusViewModel.Number = pullRequest.Number; pullRequestStatusViewModel.Title = pullRequest.Title; return pullRequestStatusViewModel; } + void IncrementNumberOfShowCurrentPullRequest() + { + } + void ShowStatus(PullRequestStatusViewModel pullRequestStatusViewModel = null) { var statusBar = FindSccStatusBar(Application.Current.MainWindow); @@ -113,5 +120,41 @@ StatusBar FindSccStatusBar(Window mainWindow) var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl; return contentControl?.Content as StatusBar; } + + class UsageTrackingCommand : ICommand + { + readonly ICommand command; + readonly IUsageTracker usageTracker; + + internal UsageTrackingCommand(ICommand command, IUsageTracker usageTracker) + { + this.command = command; + this.usageTracker = usageTracker; + } + + public event EventHandler CanExecuteChanged + { + add + { + command.CanExecuteChanged += value; + } + + remove + { + command.CanExecuteChanged -= value; + } + } + + public bool CanExecute(object parameter) + { + return command.CanExecute(parameter); + } + + public void Execute(object parameter) + { + command.Execute(parameter); + usageTracker.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget(); + } + } } } From 48cb0cccd4ee579e442f445707e3d9ff1b84d95f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 22 Mar 2018 10:58:13 +0000 Subject: [PATCH 06/25] Allow MEF to refresh its cache on a background thread --- src/GitHub.VisualStudio/GitHubPackage.cs | 3 +- src/GitHub.VisualStudio/UI/GitHubPane.cs | 43 +++++++++++++++++------- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 6fee2b11cd..69ceb25d1f 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -136,7 +136,8 @@ public async Task ShowGitHubPane() ErrorHandler.Failed(frame.Show()); } - var viewModel = (IGitHubPaneViewModel)((FrameworkElement)pane.Content).DataContext; + var gitHubPane = (GitHubPane)pane; + var viewModel = (IGitHubPaneViewModel)((FrameworkElement)gitHubPane.View).DataContext; await viewModel.InitializeAsync(pane); return viewModel; } diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index df89bc39b4..ae929e0db0 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -4,15 +4,19 @@ using System.Reactive.Linq; using System.Runtime.InteropServices; using System.Windows; +using System.Windows.Controls; +using GitHub.Helpers; using GitHub.Extensions; using GitHub.Factories; -using GitHub.Models; using GitHub.Services; using GitHub.ViewModels; using GitHub.ViewModels.GitHubPane; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; +using Microsoft.VisualStudio.ComponentModelHost; using ReactiveUI; +using Task = System.Threading.Tasks.Task; +using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider; namespace GitHub.VisualStudio.UI { @@ -34,16 +38,17 @@ public class GitHubPane : ToolWindowPane, IServiceProviderAware bool initialized = false; IDisposable viewSubscription; IGitHubPaneViewModel viewModel; + ContentControl contentControl; - FrameworkElement View + public FrameworkElement View { - get { return Content as FrameworkElement; } + get { return contentControl.Content as FrameworkElement; } set { viewSubscription?.Dispose(); viewSubscription = null; - Content = value; + contentControl.Content = value; viewSubscription = value.WhenAnyValue(x => x.DataContext) .SelectMany(x => @@ -61,6 +66,7 @@ FrameworkElement View public GitHubPane() : base(null) { Caption = "GitHub"; + Content = contentControl = new ContentControl(); BitmapImageMoniker = new Microsoft.VisualStudio.Imaging.Interop.ImageMoniker() { @@ -83,17 +89,28 @@ public void Initialize(IServiceProvider serviceProvider) { if (!initialized) { - var provider = VisualStudio.Services.GitHubServiceProvider; - var teServiceHolder = provider.GetService(); - teServiceHolder.ServiceProvider = serviceProvider; + InitializeAsync(serviceProvider).Forget(); + } + } - var factory = provider.GetService(); - viewModel = provider.ExportProvider.GetExportedValue(); - viewModel.InitializeAsync(this).Forget(); + async Task InitializeAsync(IServiceProvider serviceProvider) + { + // Allow MEF to refresh its cache on a background thread so it isn't counted against us. + var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); + await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); - View = factory.CreateView(); - View.DataContext = viewModel; - } + await ThreadingHelper.SwitchToMainThreadAsync(); + + var provider = VisualStudio.Services.GitHubServiceProvider; + var teServiceHolder = provider.GetService(); + teServiceHolder.ServiceProvider = serviceProvider; + + var factory = provider.GetService(); + viewModel = provider.ExportProvider.GetExportedValue(); + viewModel.InitializeAsync(this).Forget(); + + View = factory.CreateView(); + View.DataContext = viewModel; } [SuppressMessage("Microsoft.Design", "CA1061:DoNotHideBaseClassMethods", Justification = "WTF CA, I'm overriding!")] From ca0a2733862064ff07e5e889a0b388f4c12407eb Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Mar 2018 00:26:23 +0100 Subject: [PATCH 07/25] Use ContentPresenter instead of ContentControl This resolves the, "Logical tree depth exceeded while traversing the tree. This could indicate a cycle in the tree" exception. --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index ae929e0db0..5147afbaf9 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -5,7 +5,6 @@ using System.Runtime.InteropServices; using System.Windows; using System.Windows.Controls; -using GitHub.Helpers; using GitHub.Extensions; using GitHub.Factories; using GitHub.Services; @@ -38,17 +37,17 @@ public class GitHubPane : ToolWindowPane, IServiceProviderAware bool initialized = false; IDisposable viewSubscription; IGitHubPaneViewModel viewModel; - ContentControl contentControl; + ContentPresenter contentPresenter; public FrameworkElement View { - get { return contentControl.Content as FrameworkElement; } + get { return contentPresenter.Content as FrameworkElement; } set { viewSubscription?.Dispose(); viewSubscription = null; - contentControl.Content = value; + contentPresenter.Content = value; viewSubscription = value.WhenAnyValue(x => x.DataContext) .SelectMany(x => @@ -66,7 +65,7 @@ public FrameworkElement View public GitHubPane() : base(null) { Caption = "GitHub"; - Content = contentControl = new ContentControl(); + Content = contentPresenter = new ContentPresenter(); BitmapImageMoniker = new Microsoft.VisualStudio.Imaging.Interop.ImageMoniker() { @@ -95,12 +94,10 @@ public void Initialize(IServiceProvider serviceProvider) async Task InitializeAsync(IServiceProvider serviceProvider) { - // Allow MEF to refresh its cache on a background thread so it isn't counted against us. + // Allow MEF to initialize its cache asynchronously var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); - await ThreadingHelper.SwitchToMainThreadAsync(); - var provider = VisualStudio.Services.GitHubServiceProvider; var teServiceHolder = provider.GetService(); teServiceHolder.ServiceProvider = serviceProvider; From f66afaf8e99e9b3f7a7756e90295d73682f4ece4 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Mar 2018 00:32:57 +0100 Subject: [PATCH 08/25] Use package without MEF dependency for GitHubPane The package that hosts GitHubPane mustn't initialize MEF (even on a background thread). --- src/GitHub.Exports/Settings/Guids.cs | 1 + .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 3 +-- src/GitHub.VisualStudio/GitHubPanePackage.cs | 22 +++++++++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 src/GitHub.VisualStudio/GitHubPanePackage.cs diff --git a/src/GitHub.Exports/Settings/Guids.cs b/src/GitHub.Exports/Settings/Guids.cs index f84372a58b..dd2805aaea 100644 --- a/src/GitHub.Exports/Settings/Guids.cs +++ b/src/GitHub.Exports/Settings/Guids.cs @@ -13,6 +13,7 @@ public static class Guids public const string CodeContainerProviderId = "6CE146CB-EF57-4F2C-A93F-5BA685317660"; public const string InlineReviewsPackageId = "248325BE-4A2D-4111-B122-E7D59BF73A35"; public const string PullRequestStatusPackageId = "5121BEC6-1088-4553-8453-0DDC7C8E2238"; + public const string GitHubPanePackageId = "0A40459D-6B6D-4110-B6CE-EC83C0BC6A09"; public const string TeamExplorerWelcomeMessage = "C529627F-8AA6-4FDB-82EB-4BFB7DB753C3"; public const string LoginManagerId = "7BA2071A-790A-4F95-BE4A-0EEAA5928AAF"; diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 8f0a917e97..ce9380d2ce 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -315,6 +315,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 69ceb25d1f..e011258c7b 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -28,7 +28,6 @@ namespace GitHub.VisualStudio [Guid(Guids.guidGitHubPkgString)] [ProvideMenuResource("Menus.ctmenu", 1)] [ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)] - [ProvideToolWindow(typeof(GitHubPane), Orientation = ToolWindowOrientation.Right, Style = VsDockStyle.Tabbed, Window = EnvDTE.Constants.vsWindowKindSolutionExplorer)] [ProvideOptionPage(typeof(OptionsPage), "GitHub for Visual Studio", "General", 0, 0, supportsAutomation: true)] public class GitHubPackage : AsyncMenuPackage { @@ -137,7 +136,7 @@ public async Task ShowGitHubPane() } var gitHubPane = (GitHubPane)pane; - var viewModel = (IGitHubPaneViewModel)((FrameworkElement)gitHubPane.View).DataContext; + var viewModel = (IGitHubPaneViewModel)(gitHubPane.View).DataContext; await viewModel.InitializeAsync(pane); return viewModel; } diff --git a/src/GitHub.VisualStudio/GitHubPanePackage.cs b/src/GitHub.VisualStudio/GitHubPanePackage.cs new file mode 100644 index 0000000000..a92afcef08 --- /dev/null +++ b/src/GitHub.VisualStudio/GitHubPanePackage.cs @@ -0,0 +1,22 @@ +using System; +using System.Runtime.InteropServices; +using GitHub.VisualStudio.UI; +using Microsoft.VisualStudio.Shell; + +namespace GitHub.VisualStudio +{ + /// + /// This is the host package for the tool window. + /// + /// + /// This package mustn't use MEF. + /// See: https://github.com/github/VisualStudio/issues/1550 + /// + [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] + [Guid(Guids.GitHubPanePackageId)] + [ProvideToolWindow(typeof(GitHubPane), Orientation = ToolWindowOrientation.Right, + Style = VsDockStyle.Tabbed, Window = EnvDTE.Constants.vsWindowKindSolutionExplorer)] + public sealed class GitHubPanePackage : AsyncPackage + { + } +} \ No newline at end of file From 47d37538bd0f739afee6acc4fed0588b609a4ca5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Mar 2018 20:04:32 +0100 Subject: [PATCH 09/25] Revert "Factor b/g loading menu code into AsyncMenuPackage" This reverts commit 5730008aca33278b90f14e714380cc7c817bbb0b. --- .../InlineReviewsPackage.cs | 16 ++++++-- src/GitHub.Services.Vssdk/AsyncMenuPackage.cs | 41 ------------------- .../GitHub.Services.Vssdk.csproj | 1 - src/GitHub.VisualStudio/GitHubPackage.cs | 34 ++++++++++----- 4 files changed, 36 insertions(+), 56 deletions(-) delete mode 100644 src/GitHub.Services.Vssdk/AsyncMenuPackage.cs diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index b4741af9d8..d87584a44d 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -1,8 +1,9 @@ using System; +using System.ComponentModel.Design; using System.Runtime.InteropServices; +using System.Threading; using GitHub.Commands; using GitHub.InlineReviews.Views; -using GitHub.Services.Vssdk; using GitHub.Services.Vssdk.Commands; using GitHub.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; @@ -16,10 +17,13 @@ namespace GitHub.InlineReviews [ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)] [ProvideMenuResource("Menus.ctmenu", 1)] [ProvideToolWindow(typeof(PullRequestCommentsPane), DocumentLikeTool = true)] - public class InlineReviewsPackage : AsyncMenuPackage + public class InlineReviewsPackage : AsyncPackage { - protected override async Task InitializeMenusAsync(OleMenuCommandService menuService) + protected override async Task InitializeAsync( + CancellationToken cancellationToken, + IProgress progress) { + var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; @@ -27,5 +31,11 @@ protected override async Task InitializeMenusAsync(OleMenuCommandService menuSer exports.GetExportedValue(), exports.GetExportedValue()); } + + // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). + // When called from a non-Main thread this would throw despite the fact these services don't exist. + // This override allows IMenuCommandService.AddCommands to be called form a background thread. + protected override object GetService(Type serviceType) + => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); } } diff --git a/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs b/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs deleted file mode 100644 index 1b3230e255..0000000000 --- a/src/GitHub.Services.Vssdk/AsyncMenuPackage.cs +++ /dev/null @@ -1,41 +0,0 @@ -using System; -using System.Threading; -using System.ComponentModel.Design; -using Microsoft.VisualStudio.Shell; -using Microsoft.VisualStudio.Shell.Interop; -using Task = System.Threading.Tasks.Task; - -namespace GitHub.Services.Vssdk -{ - public abstract class AsyncMenuPackage : AsyncPackage - { - IVsUIShell vsUIShell; - - sealed protected async override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) - { - vsUIShell = await GetServiceAsync(typeof(SVsUIShell)) as IVsUIShell; - - var menuCommandService = (OleMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); - await InitializeMenusAsync(menuCommandService); - } - - protected abstract Task InitializeMenusAsync(OleMenuCommandService menuService); - - // The IDesignerHost, ISelectionService and IVsUIShell are requested by the MenuCommandService. - // This override allows IMenuCommandService.AddCommands to be called form a background thread. - protected override object GetService(Type serviceType) - { - if (serviceType == typeof(SVsUIShell)) - { - return vsUIShell; - } - - if (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) - { - return null; - } - - return base.GetService(serviceType); - } - } -} diff --git a/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj b/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj index 59f3944c38..1bd5ab4904 100644 --- a/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj +++ b/src/GitHub.Services.Vssdk/GitHub.Services.Vssdk.csproj @@ -139,7 +139,6 @@ Properties\SolutionInfo.cs - diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index e011258c7b..ba0635fbde 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -2,6 +2,7 @@ using System.Windows; using System.Threading; using System.Threading.Tasks; +using System.ComponentModel.Design; using System.ComponentModel.Composition; using System.Runtime.InteropServices; using GitHub.Api; @@ -13,7 +14,6 @@ using GitHub.Services.Vssdk.Commands; using GitHub.ViewModels.GitHubPane; using GitHub.VisualStudio.UI; -using GitHub.Services.Vssdk; using Microsoft.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; using Microsoft.VisualStudio.Shell; @@ -29,15 +29,35 @@ namespace GitHub.VisualStudio [ProvideMenuResource("Menus.ctmenu", 1)] [ProvideAutoLoad(Guids.UIContext_Git, PackageAutoLoadFlags.BackgroundLoad)] [ProvideOptionPage(typeof(OptionsPage), "GitHub for Visual Studio", "General", 0, 0, supportsAutomation: true)] - public class GitHubPackage : AsyncMenuPackage + public class GitHubPackage : AsyncPackage { static readonly ILogger log = LogManager.ForContext(); - protected async override Task InitializeMenusAsync(OleMenuCommandService menuService) + protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { LogVersionInformation(); + await base.InitializeAsync(cancellationToken, progress); await GetServiceAsync(typeof(IUsageTracker)); + await InitializeMenus(); + } + + // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). + // When called from a non-Main thread this would throw despite the fact these services don't exist. + // This override allows IMenuCommandService.AddCommands to be called form a background thread. + protected override object GetService(Type serviceType) + => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); + + void LogVersionInformation() + { + var packageVersion = ApplicationInfo.GetPackageVersion(this); + var hostVersionInfo = ApplicationInfo.GetHostVersionInfo(); + log.Information("Initializing GitHub Extension v{PackageVersion} in {$FileDescription} ({$ProductVersion})", + packageVersion, hostVersionInfo.FileDescription, hostVersionInfo.ProductVersion); + } + async Task InitializeMenus() + { + var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; @@ -52,14 +72,6 @@ protected async override Task InitializeMenusAsync(OleMenuCommandService menuSer exports.GetExportedValue()); } - void LogVersionInformation() - { - var packageVersion = ApplicationInfo.GetPackageVersion(this); - var hostVersionInfo = ApplicationInfo.GetHostVersionInfo(); - log.Information("Initializing GitHub Extension v{PackageVersion} in {$FileDescription} ({$ProductVersion})", - packageVersion, hostVersionInfo.FileDescription, hostVersionInfo.ProductVersion); - } - async Task EnsurePackageLoaded(Guid packageGuid) { var shell = await GetServiceAsync(typeof(SVsShell)) as IVsShell; From e9c8db95e98bc97683d40303fb9a20f52685308b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Mar 2018 20:08:03 +0100 Subject: [PATCH 10/25] Revert "Allow menuService.AddCommands to be called from a b/g thread" This reverts commit 05274979782263b92a887b8ec41b7f148ec174e5. --- .../InlineReviewsPackage.cs | 26 +++++++++++++------ src/GitHub.VisualStudio/GitHubPackage.cs | 21 ++++++++------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index d87584a44d..7d06563566 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -8,6 +8,7 @@ using GitHub.VisualStudio; using Microsoft.VisualStudio.ComponentModelHost; using Microsoft.VisualStudio.Shell; +using Microsoft.VisualStudio.Threading; using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews @@ -27,15 +28,24 @@ protected override async Task InitializeAsync( var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; - menuService.AddCommands( - exports.GetExportedValue(), - exports.GetExportedValue()); + // Avoid delays when there is ongoing UI activity. + // See: https://github.com/github/VisualStudio/issues/1537 + await JoinableTaskFactory.RunAsync(VsTaskRunContext.UIThreadNormalPriority, InitializeMenus); } - // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). - // When called from a non-Main thread this would throw despite the fact these services don't exist. - // This override allows IMenuCommandService.AddCommands to be called form a background thread. - protected override object GetService(Type serviceType) - => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); + async Task InitializeMenus() + { + var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); + var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); + var exports = componentModel.DefaultExportProvider; + var commands = new IVsCommandBase[] + { + exports.GetExportedValue(), + exports.GetExportedValue() + }; + + await JoinableTaskFactory.SwitchToMainThreadAsync(); + menuService.AddCommands(commands); + } } } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index ba0635fbde..f54eec6634 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -38,14 +38,11 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke LogVersionInformation(); await base.InitializeAsync(cancellationToken, progress); await GetServiceAsync(typeof(IUsageTracker)); - await InitializeMenus(); - } - // The IDesignerHost and ISelectionService services are requested by MenuCommandService.EnsureVerbs(). - // When called from a non-Main thread this would throw despite the fact these services don't exist. - // This override allows IMenuCommandService.AddCommands to be called form a background thread. - protected override object GetService(Type serviceType) - => (serviceType == typeof(ISelectionService) || serviceType == typeof(IDesignerHost)) ? null : base.GetService(serviceType); + // Avoid delays when there is ongoing UI activity. + // See: https://github.com/github/VisualStudio/issues/1537 + await JoinableTaskFactory.RunAsync(VsTaskRunContext.UIThreadNormalPriority, InitializeMenus); + } void LogVersionInformation() { @@ -60,8 +57,8 @@ async Task InitializeMenus() var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; - - menuService.AddCommands( + var commands = new IVsCommandBase[] + { exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), @@ -69,7 +66,11 @@ async Task InitializeMenus() exports.GetExportedValue(), exports.GetExportedValue(), exports.GetExportedValue(), - exports.GetExportedValue()); + exports.GetExportedValue() + }; + + await JoinableTaskFactory.SwitchToMainThreadAsync(); + menuService.AddCommands(commands); } async Task EnsurePackageLoaded(Guid packageGuid) From 2fc764b0b82d574c0ad1dbdeb78a8adaad36a90d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 29 Mar 2018 00:25:58 +0100 Subject: [PATCH 11/25] Add placeholder initializing and error views --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 5147afbaf9..4a52b137e4 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -88,13 +88,14 @@ public void Initialize(IServiceProvider serviceProvider) { if (!initialized) { - InitializeAsync(serviceProvider).Forget(); + InitializeAsync(serviceProvider).Catch(ShowError).Forget(); } } async Task InitializeAsync(IServiceProvider serviceProvider) { // Allow MEF to initialize its cache asynchronously + ShowInitializing(); var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); @@ -104,7 +105,7 @@ async Task InitializeAsync(IServiceProvider serviceProvider) var factory = provider.GetService(); viewModel = provider.ExportProvider.GetExportedValue(); - viewModel.InitializeAsync(this).Forget(); + viewModel.InitializeAsync(this).Catch(ShowError).Forget(); View = factory.CreateView(); View.DataContext = viewModel; @@ -145,6 +146,16 @@ public override void OnToolWindowCreated() UpdateSearchHost(pane?.IsSearchEnabled ?? false, pane?.SearchQuery); } + void ShowInitializing() + { + View = new Label { Content = "Initializing MEF" }; + } + + void ShowError(Exception e) + { + View = new TextBox { Text = e.ToString() }; + } + void UpdateSearchHost(bool enabled, string query) { if (SearchHost != null) From e6ee44dc13d0fbe3fe4dbf51f03ce9a503f21456 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 29 Mar 2018 19:35:23 +0100 Subject: [PATCH 12/25] Add GetViewModelAsync for GitHubPane We need an async way to obtain the IGitHubPaneViewModel. --- src/GitHub.VisualStudio/GitHubPackage.cs | 2 +- src/GitHub.VisualStudio/UI/GitHubPane.cs | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index f54eec6634..ea4bc530a0 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -149,7 +149,7 @@ public async Task ShowGitHubPane() } var gitHubPane = (GitHubPane)pane; - var viewModel = (IGitHubPaneViewModel)(gitHubPane.View).DataContext; + var viewModel = await gitHubPane.GetViewModelAsync(); await viewModel.InitializeAsync(pane); return viewModel; } diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 4a52b137e4..3ea5fd13ae 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -16,6 +16,7 @@ using ReactiveUI; using Task = System.Threading.Tasks.Task; using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider; +using System.Threading.Tasks; namespace GitHub.VisualStudio.UI { @@ -34,9 +35,12 @@ namespace GitHub.VisualStudio.UI public class GitHubPane : ToolWindowPane, IServiceProviderAware { public const string GitHubPaneGuid = "6b0fdc0a-f28e-47a0-8eed-cc296beff6d2"; + + readonly TaskCompletionSource viewModelSource = + new TaskCompletionSource(); + bool initialized = false; IDisposable viewSubscription; - IGitHubPaneViewModel viewModel; ContentPresenter contentPresenter; public FrameworkElement View @@ -92,6 +96,8 @@ public void Initialize(IServiceProvider serviceProvider) } } + public Task GetViewModelAsync() => viewModelSource.Task; + async Task InitializeAsync(IServiceProvider serviceProvider) { // Allow MEF to initialize its cache asynchronously @@ -104,11 +110,13 @@ async Task InitializeAsync(IServiceProvider serviceProvider) teServiceHolder.ServiceProvider = serviceProvider; var factory = provider.GetService(); - viewModel = provider.ExportProvider.GetExportedValue(); + var viewModel = provider.ExportProvider.GetExportedValue(); viewModel.InitializeAsync(this).Catch(ShowError).Forget(); View = factory.CreateView(); View.DataContext = viewModel; + + viewModelSource.SetResult(viewModel); } [SuppressMessage("Microsoft.Design", "CA1061:DoNotHideBaseClassMethods", Justification = "WTF CA, I'm overriding!")] From ed0cb39704983857fc5d98c6d8b2a1ec059dd900 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Thu, 29 Mar 2018 19:44:18 +0100 Subject: [PATCH 13/25] Remove redundant initialization code --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 3ea5fd13ae..46943de04c 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -32,14 +32,13 @@ namespace GitHub.VisualStudio.UI /// /// [Guid(GitHubPaneGuid)] - public class GitHubPane : ToolWindowPane, IServiceProviderAware + public class GitHubPane : ToolWindowPane { public const string GitHubPaneGuid = "6b0fdc0a-f28e-47a0-8eed-cc296beff6d2"; readonly TaskCompletionSource viewModelSource = new TaskCompletionSource(); - bool initialized = false; IDisposable viewSubscription; ContentPresenter contentPresenter; @@ -84,21 +83,12 @@ public GitHubPane() : base(null) protected override void Initialize() { - base.Initialize(); - Initialize(this); - } - - public void Initialize(IServiceProvider serviceProvider) - { - if (!initialized) - { - InitializeAsync(serviceProvider).Catch(ShowError).Forget(); - } + InitializeAsync().Catch(ShowError).Forget(); } public Task GetViewModelAsync() => viewModelSource.Task; - async Task InitializeAsync(IServiceProvider serviceProvider) + async Task InitializeAsync() { // Allow MEF to initialize its cache asynchronously ShowInitializing(); @@ -107,7 +97,7 @@ async Task InitializeAsync(IServiceProvider serviceProvider) var provider = VisualStudio.Services.GitHubServiceProvider; var teServiceHolder = provider.GetService(); - teServiceHolder.ServiceProvider = serviceProvider; + teServiceHolder.ServiceProvider = this; var factory = provider.GetService(); var viewModel = provider.ExportProvider.GetExportedValue(); From 5dcf605ad5800d827ce26ee7f1c104f6b0e14245 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 3 Apr 2018 20:31:43 +0100 Subject: [PATCH 14/25] Fixed typo --- .../Services/PullRequestStatusBarManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 34188850b9..ae9e8d8118 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -28,7 +28,7 @@ public class PullRequestStatusBarManager readonly IUsageTracker usageTracker; readonly IShowCurrentPullRequestCommand showCurrentPullRequestCommand; - // More the moment this must be constructed on the main thread. + // At the moment this must be constructed on the main thread. // TeamExplorerContext needs to retrieve DTE using GetService. readonly Lazy pullRequestSessionManager; From 29508bbbfef8a730a611c332ee6c62e607db4ef9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 14:35:50 +0100 Subject: [PATCH 15/25] Make error text box read only --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 46943de04c..dd28f761e4 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -151,7 +151,11 @@ void ShowInitializing() void ShowError(Exception e) { - View = new TextBox { Text = e.ToString() }; + View = new TextBox + { + Text = e.ToString(), + IsReadOnly = true, + }; } void UpdateSearchHost(bool enabled, string query) From 89ffd87dbb106c49a7cd6ff5eacb50cd85f86629 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 14:37:09 +0100 Subject: [PATCH 16/25] Show `Initializing...` while MEF is loading --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index dd28f761e4..a4eff12ac7 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -146,7 +146,12 @@ public override void OnToolWindowCreated() void ShowInitializing() { - View = new Label { Content = "Initializing MEF" }; + View = new TextBlock + { + Text = "Initializing...", + HorizontalAlignment = HorizontalAlignment.Center, + VerticalAlignment = VerticalAlignment.Center, + }; } void ShowError(Exception e) From e3158d2a2574b30985e0131771122cc60eed0b27 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 14:46:29 +0100 Subject: [PATCH 17/25] Make GitHubPane await initialization of IGitHubPaneViewModel GitHubPackage shouldn't be concerned about this. --- src/GitHub.VisualStudio/GitHubPackage.cs | 4 +--- src/GitHub.VisualStudio/UI/GitHubPane.cs | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index ea4bc530a0..33a3c390e8 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -149,9 +149,7 @@ public async Task ShowGitHubPane() } var gitHubPane = (GitHubPane)pane; - var viewModel = await gitHubPane.GetViewModelAsync(); - await viewModel.InitializeAsync(pane); - return viewModel; + return await gitHubPane.GetViewModelAsync(); } static ToolWindowPane ShowToolWindow(Guid windowGuid) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index a4eff12ac7..6de5aba41b 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -101,7 +101,7 @@ async Task InitializeAsync() var factory = provider.GetService(); var viewModel = provider.ExportProvider.GetExportedValue(); - viewModel.InitializeAsync(this).Catch(ShowError).Forget(); + await viewModel.InitializeAsync(this); View = factory.CreateView(); View.DataContext = viewModel; From 255b0079b02d0beddcef3a3a5692f0994e53198d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 14:48:04 +0100 Subject: [PATCH 18/25] Remove empty IncrementNumberOfShowCurrentPullRequest method --- .../Services/PullRequestStatusBarManager.cs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index ae9e8d8118..654a4882a9 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -78,10 +78,6 @@ PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pu return pullRequestStatusViewModel; } - void IncrementNumberOfShowCurrentPullRequest() - { - } - void ShowStatus(PullRequestStatusViewModel pullRequestStatusViewModel = null) { var statusBar = FindSccStatusBar(Application.Current.MainWindow); From 4746859165065505a51e3afcd6ecc7e4e7e1eb63 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 15:18:43 +0100 Subject: [PATCH 19/25] Cast IMenuCommandService on Main thread We should always cast STA COM services on Main thread. --- src/GitHub.InlineReviews/InlineReviewsPackage.cs | 2 +- src/GitHub.VisualStudio/GitHubPackage.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index 7d06563566..1bb4310743 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -35,7 +35,6 @@ protected override async Task InitializeAsync( async Task InitializeMenus() { - var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; var commands = new IVsCommandBase[] @@ -45,6 +44,7 @@ async Task InitializeMenus() }; await JoinableTaskFactory.SwitchToMainThreadAsync(); + var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); menuService.AddCommands(commands); } } diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 33a3c390e8..d008f5ac1c 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -54,7 +54,6 @@ void LogVersionInformation() async Task InitializeMenus() { - var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); var componentModel = (IComponentModel)(await GetServiceAsync(typeof(SComponentModel))); var exports = componentModel.DefaultExportProvider; var commands = new IVsCommandBase[] @@ -70,6 +69,7 @@ async Task InitializeMenus() }; await JoinableTaskFactory.SwitchToMainThreadAsync(); + var menuService = (IMenuCommandService)(await GetServiceAsync(typeof(IMenuCommandService))); menuService.AddCommands(commands); } From a9c5c16e78241fceda8bc1fbe6d03ad46ab81067 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 16:18:48 +0100 Subject: [PATCH 20/25] Use JoinableTask rather than TaskCompletionSource This avoids the risk deadlock if called within the context of a JTF.Run delegate. https://github.com/Microsoft/vs-threading/blob/master/doc/threading_rules.md#rule-3-if-ever-awaiting-work-that-was-started-earlier-that-work-must-be-joined https://blogs.msdn.microsoft.com/andrewarnottms/2014/05/07/asynchronous-and-multithreaded-programming-within-vs-using-the-joinabletaskfactory/ --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 49 +++++++++++++----------- 1 file changed, 27 insertions(+), 22 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 6de5aba41b..9d4618542a 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -1,22 +1,20 @@ using System; +using System.Threading.Tasks; using System.ComponentModel.Design; using System.Diagnostics.CodeAnalysis; using System.Reactive.Linq; using System.Runtime.InteropServices; using System.Windows; using System.Windows.Controls; -using GitHub.Extensions; using GitHub.Factories; using GitHub.Services; -using GitHub.ViewModels; using GitHub.ViewModels.GitHubPane; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; using Microsoft.VisualStudio.ComponentModelHost; +using Microsoft.VisualStudio.Threading; using ReactiveUI; -using Task = System.Threading.Tasks.Task; using IAsyncServiceProvider = Microsoft.VisualStudio.Shell.IAsyncServiceProvider; -using System.Threading.Tasks; namespace GitHub.VisualStudio.UI { @@ -36,8 +34,7 @@ public class GitHubPane : ToolWindowPane { public const string GitHubPaneGuid = "6b0fdc0a-f28e-47a0-8eed-cc296beff6d2"; - readonly TaskCompletionSource viewModelSource = - new TaskCompletionSource(); + JoinableTask viewModelTask; IDisposable viewSubscription; ContentPresenter contentPresenter; @@ -83,30 +80,38 @@ public GitHubPane() : base(null) protected override void Initialize() { - InitializeAsync().Catch(ShowError).Forget(); + viewModelTask = ThreadHelper.JoinableTaskFactory.RunAsync(InitializeAsync); } - public Task GetViewModelAsync() => viewModelSource.Task; + public Task GetViewModelAsync() => viewModelTask.JoinAsync(); - async Task InitializeAsync() + async Task InitializeAsync() { - // Allow MEF to initialize its cache asynchronously - ShowInitializing(); - var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); - await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); + try + { + // Allow MEF to initialize its cache asynchronously + ShowInitializing(); + var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); + await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); - var provider = VisualStudio.Services.GitHubServiceProvider; - var teServiceHolder = provider.GetService(); - teServiceHolder.ServiceProvider = this; + var provider = VisualStudio.Services.GitHubServiceProvider; + var teServiceHolder = provider.GetService(); + teServiceHolder.ServiceProvider = this; - var factory = provider.GetService(); - var viewModel = provider.ExportProvider.GetExportedValue(); - await viewModel.InitializeAsync(this); + var factory = provider.GetService(); + var viewModel = provider.ExportProvider.GetExportedValue(); + await viewModel.InitializeAsync(this); - View = factory.CreateView(); - View.DataContext = viewModel; + View = factory.CreateView(); + View.DataContext = viewModel; - viewModelSource.SetResult(viewModel); + return viewModel; + } + catch (Exception e) + { + ShowError(e); + throw; + } } [SuppressMessage("Microsoft.Design", "CA1061:DoNotHideBaseClassMethods", Justification = "WTF CA, I'm overriding!")] From 050411969d32614237346c4fec152f6a0d303eac Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 4 Apr 2018 16:24:24 +0100 Subject: [PATCH 21/25] This page is intentionally left blank --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 9d4618542a..2aa47b47fe 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -151,12 +151,7 @@ public override void OnToolWindowCreated() void ShowInitializing() { - View = new TextBlock - { - Text = "Initializing...", - HorizontalAlignment = HorizontalAlignment.Center, - VerticalAlignment = VerticalAlignment.Center, - }; + // This page is intentionally left blank. } void ShowError(Exception e) From cb266083f4d88a0e09392dd9160633bc818fc3b5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 6 Apr 2018 08:49:26 +0100 Subject: [PATCH 22/25] Use JoinableTaskFactory from parent AsyncPackage --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index 2aa47b47fe..a25db8c10e 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -80,7 +80,10 @@ public GitHubPane() : base(null) protected override void Initialize() { - viewModelTask = ThreadHelper.JoinableTaskFactory.RunAsync(InitializeAsync); + // Using JoinableTaskFactory from parent AsyncPackage. That way if VS shuts down before this + // work is done, we won't risk crashing due to arbitrary work going on in background threads. + var asyncPackage = (AsyncPackage)Package; + viewModelTask = asyncPackage.JoinableTaskFactory.RunAsync(InitializeAsync); } public Task GetViewModelAsync() => viewModelTask.JoinAsync(); From 75111131fac09561b7ed014a6e082a290377bcb7 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 6 Apr 2018 10:11:22 +0100 Subject: [PATCH 23/25] Retrieve IGitHubServiceProvider asynchronously Rather than retrieving SComponentModel asynchronously, we can simply retrieve IGitHubServiceProvider (which has the dependency on SComponentModel). --- src/GitHub.VisualStudio/UI/GitHubPane.cs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/GitHubPane.cs b/src/GitHub.VisualStudio/UI/GitHubPane.cs index a25db8c10e..7b44755d79 100644 --- a/src/GitHub.VisualStudio/UI/GitHubPane.cs +++ b/src/GitHub.VisualStudio/UI/GitHubPane.cs @@ -83,21 +83,20 @@ protected override void Initialize() // Using JoinableTaskFactory from parent AsyncPackage. That way if VS shuts down before this // work is done, we won't risk crashing due to arbitrary work going on in background threads. var asyncPackage = (AsyncPackage)Package; - viewModelTask = asyncPackage.JoinableTaskFactory.RunAsync(InitializeAsync); + viewModelTask = asyncPackage.JoinableTaskFactory.RunAsync(() => InitializeAsync(asyncPackage)); } public Task GetViewModelAsync() => viewModelTask.JoinAsync(); - async Task InitializeAsync() + async Task InitializeAsync(AsyncPackage asyncPackage) { try { - // Allow MEF to initialize its cache asynchronously ShowInitializing(); - var asyncServiceProvider = (IAsyncServiceProvider)GetService(typeof(SAsyncServiceProvider)); - await asyncServiceProvider.GetServiceAsync(typeof(SComponentModel)); - var provider = VisualStudio.Services.GitHubServiceProvider; + // Allow MEF to initialize its cache asynchronously + var provider = (IGitHubServiceProvider)await asyncPackage.GetServiceAsync(typeof(IGitHubServiceProvider)); + var teServiceHolder = provider.GetService(); teServiceHolder.ServiceProvider = this; From 0983ef0818641b4552ceb59fc8054d35df783c4e Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 9 Apr 2018 09:46:08 +0100 Subject: [PATCH 24/25] Moved NumberOfShowCurrentPullRequest tracking into command --- .../Services/PullRequestStatusBarManager.cs | 41 +------------------ .../Commands/ShowCurrentPullRequestCommand.cs | 7 +++- 2 files changed, 7 insertions(+), 41 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 654a4882a9..3db79ae6e8 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -1,6 +1,5 @@ using System; using System.Windows; -using System.Windows.Input; using System.Windows.Controls; using System.Windows.Controls.Primitives; using System.ComponentModel.Composition; @@ -10,7 +9,6 @@ using GitHub.Services; using GitHub.Models; using GitHub.Logging; -using GitHub.Extensions; using Serilog; using ReactiveUI; @@ -71,8 +69,7 @@ void RefreshCurrentSession() PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pullRequest) { - var trackingCommand = new UsageTrackingCommand(showCurrentPullRequestCommand, usageTracker); - var pullRequestStatusViewModel = new PullRequestStatusViewModel(trackingCommand); + var pullRequestStatusViewModel = new PullRequestStatusViewModel(showCurrentPullRequestCommand); pullRequestStatusViewModel.Number = pullRequest.Number; pullRequestStatusViewModel.Title = pullRequest.Title; return pullRequestStatusViewModel; @@ -116,41 +113,5 @@ StatusBar FindSccStatusBar(Window mainWindow) var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl; return contentControl?.Content as StatusBar; } - - class UsageTrackingCommand : ICommand - { - readonly ICommand command; - readonly IUsageTracker usageTracker; - - internal UsageTrackingCommand(ICommand command, IUsageTracker usageTracker) - { - this.command = command; - this.usageTracker = usageTracker; - } - - public event EventHandler CanExecuteChanged - { - add - { - command.CanExecuteChanged += value; - } - - remove - { - command.CanExecuteChanged -= value; - } - } - - public bool CanExecute(object parameter) - { - return command.CanExecute(parameter); - } - - public void Execute(object parameter) - { - command.Execute(parameter); - usageTracker.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget(); - } - } } } diff --git a/src/GitHub.VisualStudio/Commands/ShowCurrentPullRequestCommand.cs b/src/GitHub.VisualStudio/Commands/ShowCurrentPullRequestCommand.cs index 749db44dfc..48c9f9ae19 100644 --- a/src/GitHub.VisualStudio/Commands/ShowCurrentPullRequestCommand.cs +++ b/src/GitHub.VisualStudio/Commands/ShowCurrentPullRequestCommand.cs @@ -4,6 +4,7 @@ using GitHub.Commands; using GitHub.Logging; using GitHub.Services; +using GitHub.Extensions; using GitHub.Services.Vssdk.Commands; using Serilog; @@ -20,12 +21,14 @@ public class ShowCurrentPullRequestCommand : VsCommand, IShowCurrentPullRequestC { static readonly ILogger log = LogManager.ForContext(); readonly IGitHubServiceProvider serviceProvider; + readonly Lazy usageTracker; [ImportingConstructor] - protected ShowCurrentPullRequestCommand(IGitHubServiceProvider serviceProvider) + protected ShowCurrentPullRequestCommand(IGitHubServiceProvider serviceProvider, Lazy usageTracker) : base(CommandSet, CommandId) { this.serviceProvider = serviceProvider; + this.usageTracker = usageTracker; } /// @@ -58,6 +61,8 @@ public override async Task Execute() var manager = serviceProvider.TryGetService(); var host = await manager.ShowGitHubPane(); await host.ShowPullRequest(session.RepositoryOwner, host.LocalRepository.Name, pullRequest.Number); + + usageTracker.Value.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget(); } catch (Exception ex) { From 39d77aa9161a6d0deff5504fffb9af4e343b5e0a Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 9 Apr 2018 14:39:20 +0100 Subject: [PATCH 25/25] Keep CA happy --- .../Services/PullRequestStatusBarManager.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 3db79ae6e8..cd2c204538 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -23,7 +23,6 @@ public class PullRequestStatusBarManager static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; - readonly IUsageTracker usageTracker; readonly IShowCurrentPullRequestCommand showCurrentPullRequestCommand; // At the moment this must be constructed on the main thread. @@ -32,11 +31,9 @@ public class PullRequestStatusBarManager [ImportingConstructor] public PullRequestStatusBarManager( - IUsageTracker usageTracker, IShowCurrentPullRequestCommand showCurrentPullRequestCommand, Lazy pullRequestSessionManager) { - this.usageTracker = usageTracker; this.showCurrentPullRequestCommand = showCurrentPullRequestCommand; this.pullRequestSessionManager = pullRequestSessionManager; }