Switch color loading to be async.#60205
Conversation
| if (_isInitialized) | ||
| return; | ||
|
|
||
| _ = _colorSchemeRegistryItems.GetValueAsync(CancellationToken.None); |
There was a problem hiding this comment.
Do we have a good sense what this fire-and-forget was doing before? As best I can tell the initialize path was going to block on this if/when we needed to change the theme right away in the call to UpdateColorScheme, and this isn't making things worse...but then what was this for?
src/VisualStudio/Core/Def/ColorSchemes/ColorSchemeApplier.ClassificationVerifier.cs
Outdated
Show resolved
Hide resolved
| await _threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); | ||
|
|
There was a problem hiding this comment.
So we have this code here:
Which implies we can get to local registry values without needing a UI thread switch. I'm not sure if VSRegistry.RegistryRoot() is just doing exactly what we do there or whether we can refactor that and reuse it?
There was a problem hiding this comment.
(this isn't wrong though, so better to keep the switch to UI thread if there is any uncertainty)
| await GetThemeIdAsync(_threadingContext.DisposalToken).ConfigureAwait(false), | ||
| _threadingContext.DisposalToken).ConfigureAwait(false)); |
There was a problem hiding this comment.
You could get rid of the .ConfigureAwait's here since we're in a definitely JTF safe situation. Our analyzers might not like it though. (Wonder if we should teach them to ignore that in JTF.Runs...)
|
|
||
| public bool IsSupportedTheme() | ||
| => IsSupportedTheme(GetThemeId()); | ||
| => _threadingContext.JoinableTaskFactory.Run(() => IsSupportedThemeAsync(_threadingContext.DisposalToken)); |
There was a problem hiding this comment.
Use of _threadingContext.DisposalToken seems a bit odd here, since it means whoever is calling IsSupportedTheme() might get it potentially cancelled out from underneath them. Can the caller pass through a token?
There was a problem hiding this comment.
that's a good point let me see.
Fixes https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1501481