Further clean up Roslyn's package load

src/VisualStudio/Core/Def/EditorConfigSettings/SettingsEditorFactory.cs

@@ -23,21 +23,11 @@ namespace Microsoft.VisualStudio.LanguageServices.EditorConfigSettings;
 [Guid(SettingsEditorFactoryGuidString)]
 internal sealed class SettingsEditorFactory() : IVsEditorFactory, IVsEditorFactory4
 {
-    private static SettingsEditorFactory? s_instance;
-
     public static readonly Guid SettingsEditorFactoryGuid = new(SettingsEditorFactoryGuidString);
     public const string SettingsEditorFactoryGuidString = "68b46364-d378-42f2-9e72-37d86c5f4468";
     public const string Extension = ".editorconfig";
 
     private ServiceProvider? _vsServiceProvider;
-
-    public static SettingsEditorFactory GetInstance()
-    {
-        s_instance ??= new SettingsEditorFactory();
-
-        return s_instance;
-    }
-
     public int CreateEditorInstance(uint grfCreateDoc,
                                     string filePath,
                                     string pszPhysicalView,

src/VisualStudio/Core/Def/LanguageService/AbstractPackage.cs

@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using Microsoft.VisualStudio.ComponentModelHost;
 using Microsoft.VisualStudio.Shell;
+using Microsoft.VisualStudio.Shell.Interop;
 
 namespace Microsoft.VisualStudio.LanguageServices.Implementation.LanguageService;
 
@@ -51,26 +52,34 @@ private Task RegisterAndProcessTasksAsync(Action<PackageLoadTasks> registerTasks
 
     protected virtual void RegisterInitializeAsyncWork(PackageLoadTasks packageInitializationTasks)
     {
-        // This treatment of registering work on the bg/main threads is a bit unique as we want the component model initialized at the beginning
-        // of whichever context is invoked first. The current architecture doesn't execute any of the registered tasks concurrently,
-        // so that isn't a concern for calculating or setting _componentModel_doNotAccessDirectly multiple times.
+        // We register this task so our ComponentModel property is available during other parts of package initialization and OnAfterPackageLoaded work. The
+        // expectation at this point is no work scheduled to the UI thread needs the ComponentModel, so we only schedule it for the background thread.
         packageInitializationTasks.AddTask(isMainThreadTask: false, task: EnsureComponentModelAsync);
-        packageInitializationTasks.AddTask(isMainThreadTask: true, task: EnsureComponentModelAsync);
 
         async Task EnsureComponentModelAsync(PackageLoadTasks packageInitializationTasks, CancellationToken token)
         {
-            if (_componentModel_doNotAccessDirectly == null)
-            {
-                _componentModel_doNotAccessDirectly = (IComponentModel?)await GetServiceAsync(typeof(SComponentModel)).ConfigureAwait(false);
-                Assumes.Present(_componentModel_doNotAccessDirectly);
-            }
+            _componentModel_doNotAccessDirectly = (IComponentModel?)await GetServiceAsync(typeof(SComponentModel)).ConfigureAwait(false);
+            Assumes.Present(_componentModel_doNotAccessDirectly);
         }
     }
 
     protected virtual void RegisterOnAfterPackageLoadedAsyncWork(PackageLoadTasks afterPackageLoadedTasks)
     {
     }
 
+    /// <summary>
+    /// Registers an editor factory. This is the same as <see cref="Microsoft.VisualStudio.Shell.Package.RegisterEditorFactory(IVsEditorFactory)"/> except it fetches the service async.
+    /// </summary>
+    protected async Task RegisterEditorFactoryAsync(IVsEditorFactory editorFactory, CancellationToken cancellationToken)
+    {
+        // Call with ConfigureAwait(true): if we're off the UI thread we will stay that way, but a synchronous load of our package should continue to use the UI thread
+        // since the UI thread is otherwise blocked waiting for us. This method is called under JTF rules so that's fine.
+        var registerEditors = await GetServiceAsync<SVsRegisterEditors, IVsRegisterEditors>(throwOnFailure: true, cancellationToken).ConfigureAwait(true);
+        Assumes.Present(registerEditors);
+
+        ErrorHandler.ThrowOnFailure(registerEditors.RegisterEditor(editorFactory.GetType().GUID, editorFactory, out _));
+    }
+
     protected async Task LoadComponentsInUIContextOnceSolutionFullyLoadedAsync(CancellationToken cancellationToken)
     {
         // UIContexts can be "zombied" if UIContexts aren't supported because we're in a command line build or in other scenarios.

src/VisualStudio/Core/Def/LanguageService/AbstractPackage`2.cs

@@ -26,13 +26,14 @@ internal abstract partial class AbstractPackage<TPackage, TLanguageService> : Ab
 {
     private PackageInstallerService? _packageInstallerService;
     private VisualStudioSymbolSearchService? _symbolSearchService;
-    private IVsShell? _shell;
 
     /// <summary>
     /// Set to 1 if we've already preloaded project system components. Should be updated with <see cref="Interlocked.CompareExchange{T}(ref T, T, T)" />
     /// </summary>
     private int _projectSystemComponentsPreloaded;
 
+    private bool _objectBrowserLibraryManagerRegistered = false;
+
     protected AbstractPackage()
     {
     }
@@ -47,26 +48,14 @@ protected override void RegisterInitializeAsyncWork(PackageLoadTasks packageInit
 
     private async Task PackageInitializationMainThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
     {
-        // This code uses various main thread only services, so it must run completely on the main thread
-        // (thus the CA(true) usage throughout)
-        Contract.ThrowIfFalse(JoinableTaskFactory.Context.IsOnMainThread);
-
-        var shell = (IVsShell7?)await GetServiceAsync(typeof(SVsShell)).ConfigureAwait(true);
+        // We still need to ensure the RoslynPackage is loaded, since it's OnAfterPackageLoaded will hook up event handlers in RoslynPackage.LoadComponentsAsync.
+        // Once that method has been replaced, then this package load can be removed.
+        var shell = await GetServiceAsync<SVsShell, IVsShell7>(throwOnFailure: true, cancellationToken).ConfigureAwait(true);
         Assumes.Present(shell);
-
-        _shell = (IVsShell?)shell;
-        Assumes.Present(_shell);
-
-        foreach (var editorFactory in CreateEditorFactories())
-        {
-            RegisterEditorFactory(editorFactory);
-        }
-
-        // awaiting an IVsTask guarantees to return on the captured context
         await shell.LoadPackageAsync(Guids.RoslynPackageId);
     }
 
-    private Task PackageInitializationBackgroundThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
+    private async Task PackageInitializationBackgroundThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
     {
         AddService(typeof(TLanguageService), async (_, cancellationToken, _) =>
             {
@@ -103,7 +92,10 @@ private Task PackageInitializationBackgroundThreadAsync(PackageLoadTasks package
 
         RegisterMiscellaneousFilesWorkspaceInformation(miscellaneousFilesWorkspace);
 
-        return Task.CompletedTask;
+        foreach (var editorFactory in CreateEditorFactories())
+        {
+            await RegisterEditorFactoryAsync(editorFactory, cancellationToken).ConfigureAwait(true);
+        }
     }
 
     protected override void RegisterOnAfterPackageLoadedAsyncWork(PackageLoadTasks afterPackageLoadedTasks)
@@ -112,18 +104,18 @@ protected override void RegisterOnAfterPackageLoadedAsyncWork(PackageLoadTasks a
 
         afterPackageLoadedTasks.AddTask(
             isMainThreadTask: true,
-            task: (packageLoadedTasks, cancellationToken) =>
+            task: async (packageLoadedTasks, cancellationToken) =>
             {
-                if (_shell != null && !_shell.IsInCommandLineMode())
+                if (!await CommandLineMode.IsInCommandLineModeAsync(AsyncServiceProvider.GlobalProvider, cancellationToken).ConfigureAwait(true))
                 {
                     // not every derived package support object browser and for those languages
                     // this is a no op
                     RegisterObjectBrowserLibraryManager();
+
+                    _objectBrowserLibraryManagerRegistered = true;
                 }
 
                 LoadComponentsInUIContextOnceSolutionFullyLoadedAsync(cancellationToken).Forget();
-
-                return Task.CompletedTask;
             });
     }
 
@@ -160,7 +152,8 @@ protected override void Dispose(bool disposing)
         {
             // Per VS core team, Package.Dispose is called on the UI thread.
             Contract.ThrowIfFalse(JoinableTaskFactory.Context.IsOnMainThread);
-            if (_shell != null && !_shell.IsInCommandLineMode())
+
+            if (_objectBrowserLibraryManagerRegistered)
             {
                 UnregisterObjectBrowserLibraryManager();
             }

src/VisualStudio/Core/Def/RoslynPackage.cs

@@ -12,7 +12,6 @@
 using Microsoft.CodeAnalysis.Editor.Implementation.IntelliSense.AsyncCompletion;
 using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
 using Microsoft.CodeAnalysis.ErrorReporting;
-using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.Remote.ProjectSystem;
 using Microsoft.VisualStudio.LanguageServices.EditorConfigSettings;
 using Microsoft.VisualStudio.LanguageServices.ExternalAccess.UnitTesting;
@@ -69,41 +68,24 @@ protected override void RegisterInitializeAsyncWork(PackageLoadTasks packageInit
         base.RegisterInitializeAsyncWork(packageInitializationTasks);
 
         packageInitializationTasks.AddTask(isMainThreadTask: false, task: PackageInitializationBackgroundThreadAsync);
-        packageInitializationTasks.AddTask(isMainThreadTask: true, task: PackageInitializationMainThreadAsync);
 
         return;
 
-        Task PackageInitializationBackgroundThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
+        async Task PackageInitializationBackgroundThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
         {
-            return ProfferServiceBrokerServicesAsync(cancellationToken);
-        }
-
-        Task PackageInitializationMainThreadAsync(PackageLoadTasks packageInitializationTasks, CancellationToken cancellationToken)
-        {
-            var settingsEditorFactory = SettingsEditorFactory.GetInstance();
-            RegisterEditorFactory(settingsEditorFactory);
-
-            return Task.CompletedTask;
+            await RegisterEditorFactoryAsync(new SettingsEditorFactory(), cancellationToken).ConfigureAwait(true);
+            await ProfferServiceBrokerServicesAsync(cancellationToken).ConfigureAwait(true);
         }
     }
 
     protected override void RegisterOnAfterPackageLoadedAsyncWork(PackageLoadTasks afterPackageLoadedTasks)
     {
         base.RegisterOnAfterPackageLoadedAsyncWork(afterPackageLoadedTasks);
 
-        afterPackageLoadedTasks.AddTask(isMainThreadTask: false, task: OnAfterPackageLoadedBackgroundThreadAsync);
         afterPackageLoadedTasks.AddTask(isMainThreadTask: true, task: OnAfterPackageLoadedMainThreadAsync);
 
         return;
 
-        Task OnAfterPackageLoadedBackgroundThreadAsync(PackageLoadTasks afterPackageLoadedTasks, CancellationToken cancellationToken)
-        {
-            // Ensure the options persisters are loaded since we have to fetch options from the shell
-            _ = ComponentModel.GetService<IGlobalOptionService>();
-
-            return Task.CompletedTask;
-        }
-
         Task OnAfterPackageLoadedMainThreadAsync(PackageLoadTasks afterPackageLoadedTasks, CancellationToken cancellationToken)
         {
             // load some services that have to be loaded in UI thread

src/VisualStudio/Core/Def/Utilities/CommandLineMode.cs

@@ -0,0 +1,63 @@
+// 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.Runtime.InteropServices;
+using System.Threading;
+using System.Threading.Tasks;
+using Microsoft.VisualStudio.Shell;
+
+namespace Microsoft.VisualStudio.LanguageServices.Implementation.Utilities;
+
+internal static class CommandLineMode
+{
+    // tri-state: uninitialized (0), devenv is in command line mode (1), devenv is not in command line mode (-1)
+    private static volatile int s_isInCommandLineMode;
+
+    /// <summary>
+    /// Returns true if devenv is invoked in command line mode for build, e.g. devenv /rebuild MySolution.sln
+    /// </summary>
+    public static async Task<bool> IsInCommandLineModeAsync(IAsyncServiceProvider serviceProvider, CancellationToken cancellationToken)
+    {
+        if (s_isInCommandLineMode == 0)
+        {
+            var appId = await serviceProvider.GetServiceAsync<IVsAppId, IVsAppId>(cancellationToken).ConfigureAwait(true);
+
+            s_isInCommandLineMode =
+                ErrorHandler.Succeeded(appId.GetProperty(VSAPROPID_IsInCommandLineMode, out var result)) &&
+                (bool)result ? 1 : -1;
+        }
+
+        return s_isInCommandLineMode == 1;
+    }
+
+    // Copied from https://github.com/dotnet/project-system/blob/698c90fc016a24fd5b0b2b73df2c68299e04bd66/src/Microsoft.VisualStudio.ProjectSystem.Managed.VS/ProjectSystem/VS/Interop/IVsAppId.cs
+    [Guid("1EAA526A-0898-11d3-B868-00C04F79F802"), InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
+    internal interface IVsAppId
+    {
+        [PreserveSig]
+        int SetSite(Microsoft.VisualStudio.OLE.Interop.IServiceProvider pSP);
+
+        [PreserveSig]
+        int GetProperty(int propid, // VSAPROPID
+            [MarshalAs(UnmanagedType.Struct)] out object pvar);
+
+        [PreserveSig]
+        int SetProperty(int propid, //[in] VSAPROPID
+            [MarshalAs(UnmanagedType.Struct)] object var);
+
+        [PreserveSig]
+        int GetGuidProperty(int propid, // VSAPROPID
+            out Guid guid);
+
+        [PreserveSig]
+        int SetGuidProperty(int propid, // [in] VSAPROPID
+            ref Guid rguid);
+
+        [PreserveSig]
+        int Initialize();  // called after main initialization and before command executing and entering main loop
+    }
+
+    private const int VSAPROPID_IsInCommandLineMode = -8660;
+}