starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-07-09 02:26:45 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

[ainulindale] Prevent a race between the Emperor and the window logic when updating settings

Browse Source
This commit is contained in:
Mike Griese
2023-03-06 13:25:45 -06:00
parent 796a02e0b0
commit 35c7474abe
9 changed files with 45 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/cascadia/TerminalApp/AppLogic.cpp
View File

@@ -673,6 +673,18 @@ namespace winrt::TerminalApp::implementation
}
return _settings.GlobalSettings().IsolatedMode();
}
bool AppLogic::RequestsTrayIcon()
{
if (!_loadedInitialSettings)
{
// Load settings if we haven't already
ReloadSettings();
}
return _settings.GlobalSettings().AlwaysShowNotificationIcon() ||
_settings.GlobalSettings().MinimizeToNotificationArea();
;
}
TerminalApp::TerminalWindow AppLogic::CreateNewWindow()
{

1
src/cascadia/TerminalApp/AppLogic.h
View File

@@ -66,6 +66,7 @@ namespace winrt::TerminalApp::implementation
Microsoft::Terminal::Settings::Model::Theme Theme();
bool IsolatedMode();
bool RequestsTrayIcon();
TerminalApp::TerminalWindow CreateNewWindow();

3
src/cascadia/TerminalApp/AppLogic.idl
View File

@@ -43,8 +43,10 @@ namespace TerminalApp
void ReloadSettings();
// Selected settings to expose
Microsoft.Terminal.Settings.Model.Theme Theme { get; };
Boolean IsolatedMode { get; };
Boolean RequestsTrayIcon { get; };
FindTargetWindowResult FindTargetWindow(String[] args);
@@ -54,6 +56,7 @@ namespace TerminalApp
IMapView<Microsoft.Terminal.Control.KeyChord, Microsoft.Terminal.Settings.Model.Command> GlobalHotkeys();
event Windows.Foundation.TypedEventHandler<Object, SettingsLoadEventArgs> SettingsChanged;
}
}

40
src/cascadia/TerminalApp/TerminalPage.cpp
View File

@@ -102,37 +102,27 @@ namespace winrt::TerminalApp::implementation
return S_OK;
}
// This may be called on a background thread, or the main thread, but almost
// definitely not on OUR UI thread.
winrt::fire_and_forget TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
// INVARIANT: This needs to be called on OUR UI thread!
void TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
{
_settings = settings;
const auto weakThis{ get_weak() };
co_await wil::resume_foreground(Dispatcher());
// Back on our UI thread...
// Make sure to _UpdateCommandsForPalette before
// _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make
// sure the KeyChordText of Commands is updated, which needs to
// happen before the Settings UI is reloaded and tries to re-read
// those values.
_UpdateCommandsForPalette();
CommandPalette().SetActionMap(_settings.ActionMap());
if (const auto page{ weakThis.get() })
if (needRefreshUI)
{
// `this` is safe to use while `page` holds the strong ref
// Make sure to _UpdateCommandsForPalette before
// _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make
// sure the KeyChordText of Commands is updated, which needs to
// happen before the Settings UI is reloaded and tries to re-read
// those values.
_UpdateCommandsForPalette();
CommandPalette().SetActionMap(_settings.ActionMap());
if (needRefreshUI)
{
_RefreshUIForSettingsReload();
}
// Upon settings update we reload the system settings for scrolling as well.
// TODO: consider reloading this value periodically.
_systemRowsToScroll = _ReadSystemRowsToScroll();
_RefreshUIForSettingsReload();
}
// Upon settings update we reload the system settings for scrolling as well.
// TODO: consider reloading this value periodically.
_systemRowsToScroll = _ReadSystemRowsToScroll();
}
bool TerminalPage::IsElevated() const noexcept

2
src/cascadia/TerminalApp/TerminalPage.h
View File

@@ -88,7 +88,7 @@ namespace winrt::TerminalApp::implementation
// put it in our inheritance graph. https://github.com/microsoft/microsoft-ui-xaml/issues/3331
STDMETHODIMP Initialize(HWND hwnd);
winrt::fire_and_forget SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI);
void SetSettings(Microsoft::Terminal::Settings::Model::CascadiaSettings settings, bool needRefreshUI);
void Create();

14
src/cascadia/TerminalApp/TerminalWindow.cpp
View File

@@ -265,7 +265,7 @@ namespace winrt::TerminalApp::implementation
_root->SetStartupActions(_settingsStartupArgs);
}
_root->SetSettings(_settings, false);
_root->SetSettings(_settings, false); // We're on our UI thread right now, so this is safe
_root->Loaded({ this, &TerminalWindow::_OnLoaded });
_root->Initialized([this](auto&&, auto&&) {
// GH#288 - When we finish initialization, if the user wanted us
@@ -338,12 +338,6 @@ namespace winrt::TerminalApp::implementation
{
return _settings.GlobalSettings().AlwaysShowNotificationIcon();
}
bool TerminalWindow::RequestsTrayIcon()
{
return _settings.GlobalSettings().AlwaysShowNotificationIcon() ||
_settings.GlobalSettings().MinimizeToNotificationArea() ||
IsQuakeWindow();
}
bool TerminalWindow::GetShowTitleInTitlebar()
{
@@ -793,14 +787,16 @@ namespace winrt::TerminalApp::implementation
winrt::fire_and_forget TerminalWindow::UpdateSettings(winrt::TerminalApp::SettingsLoadEventArgs args)
{
_settings = args.NewSettings();
// Update the settings in TerminalPage
_root->SetSettings(_settings, true);
const auto weakThis{ get_weak() };
co_await wil::resume_foreground(_root->Dispatcher());
// Back on our UI thread...
if (auto logic{ weakThis.get() })
{
// Update the settings in TerminalPage
// We're on our UI thread right now, so this is safe
_root->SetSettings(_settings, true);
// Bubble the notification up to the AppHost, now that we've updated our _settings.
_SettingsChangedHandlers(*this, args);

1
src/cascadia/TerminalApp/TerminalWindow.h
View File

@@ -107,7 +107,6 @@ namespace winrt::TerminalApp::implementation
bool GetMinimizeToNotificationArea();
bool GetAlwaysShowNotificationIcon();
bool RequestsTrayIcon();
bool GetShowTitleInTitlebar();

1
src/cascadia/TerminalApp/TerminalWindow.idl
View File

@@ -102,7 +102,6 @@ namespace TerminalApp
Boolean GetMinimizeToNotificationArea();
Boolean GetAlwaysShowNotificationIcon();
Boolean RequestsTrayIcon();
Boolean GetShowTitleInTitlebar();
// These already have accessors as a part of IWindowProperties, but we

12
src/cascadia/WindowsTerminal/WindowEmperor.cpp
View File

@@ -124,7 +124,7 @@ void WindowEmperor::CreateNewWindowThread(Remoting::WindowRequestedArgs args, co
// Add a callback to the window's logic to let us know when the window's
// quake mode state changes. We'll use this to check if we need to add
// or remove the notification icon.
sender->Logic().IsQuakeWindowChanged([this](auto&&, auto&&) -> winrt::fire_and_forget {
sender->Logic().IsQuakeWindowChanged([this](auto&&, auto &&) -> winrt::fire_and_forget {
co_await wil::resume_foreground(this->_dispatcher);
this->_checkWindowsForNotificationIcon();
});
@@ -645,11 +645,15 @@ void WindowEmperor::_checkWindowsForNotificationIcon()
// re-summon any hidden windows, but right now we're not keeping track of
// who's hidden, so just summon them all. Tracking the work to do a "summon
// all minimized" in GH#10448
bool needsIcon = false;
//
// To avoid races between us thinking the settings updated, and the windows
// themselves getting the new settings, only ask the app logic for the
// RequestsTrayIcon setting value, and combine that with the result of each
// window (which won't change during a settings reload).
bool needsIcon = _app.Logic().RequestsTrayIcon();
for (const auto& _windowThread : _windows)
{
needsIcon |= _windowThread->Logic().RequestsTrayIcon();
needsIcon |= _windowThread->Logic().IsQuakeWindow();
}
if (needsIcon)