[ainulindale] This I'm confident fixes some of the x-thread errors, b…

…ut this doesn't fix the crash
microsoft · Feb 7, 2023 · 700aadc · 700aadc
1 parent 7bad8c9
commit 700aadc
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 7 deletions.
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -129,10 +129,12 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    void TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
+    winrt::fire_and_forget TerminalPage::SetSettings(CascadiaSettings settings, bool needRefreshUI)
     {
         _settings = settings;
 
+        co_await wil::resume_foreground(Dispatcher());
+
         // Make sure to _UpdateCommandsForPalette before
         // _RefreshUIForSettingsReload. _UpdateCommandsForPalette will make
         // sure the KeyChordText of Commands is updated, which needs to

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

diff --git a/src/cascadia/TerminalApp/TerminalWindow.cpp b/src/cascadia/TerminalApp/TerminalWindow.cpp
@@ -740,8 +740,15 @@ 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);
+
         co_await wil::resume_foreground(_root->Dispatcher());
 
+        // Bubble the notification up to the AppHost, now that we've updated our _settings.
+        _SettingsChangedHandlers(*this, args);
+
         if (FAILED(args.Result()))
         {
             const winrt::hstring titleKey = USES_RESOURCE(L"ReloadJsonParseErrorTitle");
@@ -756,9 +763,6 @@ namespace winrt::TerminalApp::implementation
         {
             _ShowLoadWarningsDialog(args.Warnings());
         }
-        _settings = args.NewSettings();
-        // Update the settings in TerminalPage
-        _root->SetSettings(_settings, true);
         _RefreshThemeRoutine();
     }
     void TerminalWindow::_OpenSettingsUI()

diff --git a/src/cascadia/TerminalApp/TerminalWindow.h b/src/cascadia/TerminalApp/TerminalWindow.h
@@ -197,6 +197,8 @@ namespace winrt::TerminalApp::implementation
 
         TYPED_EVENT(SystemMenuChangeRequested, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SystemMenuChangeArgs);
 
+        TYPED_EVENT(SettingsChanged, winrt::Windows::Foundation::IInspectable, winrt::TerminalApp::SettingsLoadEventArgs);
+
         FORWARDED_TYPED_EVENT(RequestMoveContent, Windows::Foundation::IInspectable, winrt::TerminalApp::RequestMoveContentArgs, _root, RequestMoveContent);
 
 #ifdef UNIT_TESTING

diff --git a/src/cascadia/TerminalApp/TerminalWindow.idl b/src/cascadia/TerminalApp/TerminalWindow.idl
@@ -136,6 +136,9 @@ namespace TerminalApp
         event Windows.Foundation.TypedEventHandler<Object, TerminalApp.SystemMenuChangeArgs> SystemMenuChangeRequested;
         event Windows.Foundation.TypedEventHandler<Object, Microsoft.Terminal.Control.ShowWindowArgs> ShowWindowChanged;
 
+        event Windows.Foundation.TypedEventHandler<Object, SettingsLoadEventArgs> SettingsChanged;
+
+
         event Windows.Foundation.TypedEventHandler<Object, RequestMoveContentArgs> RequestMoveContent;
         void AttachContent(String content, UInt32 tabIndex);
 

diff --git a/src/cascadia/WindowsTerminal/AppHost.cpp b/src/cascadia/WindowsTerminal/AppHost.cpp
@@ -453,7 +453,13 @@ void AppHost::Initialize()
     _revokers.SetTaskbarProgress = _windowLogic.SetTaskbarProgress(winrt::auto_revoke, { this, &AppHost::SetTaskbarProgress });
     _revokers.IdentifyWindowsRequested = _windowLogic.IdentifyWindowsRequested(winrt::auto_revoke, { this, &AppHost::_IdentifyWindowsRequested });
     _revokers.RenameWindowRequested = _windowLogic.RenameWindowRequested(winrt::auto_revoke, { this, &AppHost::_RenameWindowRequested });
-    _revokers.SettingsChanged = _appLogic.SettingsChanged(winrt::auto_revoke, { this, &AppHost::_HandleSettingsChanged });
+
+    // A note: make sure to listen to our _window_'s settings changed, not the
+    // applogic's. We want to make sure the event has gone through the window
+    // logic _before_ we handle it, so we can ask the window about it's newest
+    // properties.
+    _revokers.SettingsChanged = _windowLogic.SettingsChanged(winrt::auto_revoke, { this, &AppHost::_HandleSettingsChanged });
+
     _revokers.IsQuakeWindowChanged = _windowLogic.IsQuakeWindowChanged(winrt::auto_revoke, { this, &AppHost::_IsQuakeWindowChanged });
     _revokers.SummonWindowRequested = _windowLogic.SummonWindowRequested(winrt::auto_revoke, { this, &AppHost::_SummonWindowRequested });
     _revokers.OpenSystemMenu = _windowLogic.OpenSystemMenu(winrt::auto_revoke, { this, &AppHost::_OpenSystemMenu });

diff --git a/src/cascadia/WindowsTerminal/AppHost.h b/src/cascadia/WindowsTerminal/AppHost.h
@@ -180,7 +180,7 @@ class AppHost
         winrt::TerminalApp::TerminalWindow::QuitRequested_revoker QuitRequested;
         winrt::TerminalApp::TerminalWindow::ShowWindowChanged_revoker ShowWindowChanged;
         winrt::TerminalApp::TerminalWindow::PropertyChanged_revoker PropertyChanged;
-        winrt::TerminalApp::AppLogic::SettingsChanged_revoker SettingsChanged;
+        winrt::TerminalApp::TerminalWindow::SettingsChanged_revoker SettingsChanged;
         winrt::Microsoft::Terminal::Remoting::WindowManager::ShowNotificationIconRequested_revoker ShowNotificationIconRequested;
         winrt::Microsoft::Terminal::Remoting::WindowManager::HideNotificationIconRequested_revoker HideNotificationIconRequested;
         winrt::Microsoft::Terminal::Remoting::WindowManager::QuitAllRequested_revoker QuitAllRequested;