From 7be7b3995f480e581b22ba1899d1e49668c9af84 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Tue, 20 Oct 2020 12:02:23 -0400 Subject: [PATCH 1/4] optimize creation of jumplist entries --- src/cascadia/TerminalApp/Jumplist.cpp | 15 +++++++-------- src/cascadia/TerminalApp/Jumplist.h | 2 +- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index 53bfc8e7552..8cd8147f6d9 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/src/cascadia/TerminalApp/Jumplist.cpp @@ -118,8 +118,9 @@ static std::wstring_view _getExePath() // - settings - The settings object to update the jumplist with. // Return Value: // - -HRESULT Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept +winrt::fire_and_forget Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept { + co_await winrt::resume_background(); try { auto jumplistInstance = winrt::create_instance(CLSID_DestinationList, CLSCTX_ALL); @@ -131,10 +132,10 @@ HRESULT Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept // It's easier to clear the list and re-add everything. The settings aren't // updated often, and there likely isn't a huge amount of items to add. - RETURN_IF_FAILED(jumplistItems->Clear()); + THROW_IF_FAILED(jumplistItems->Clear()); // Update the list of profiles. - RETURN_IF_FAILED(_updateProfiles(jumplistItems.get(), settings.Profiles().GetView())); + THROW_IF_FAILED(_updateProfiles(jumplistItems.get(), settings.Profiles().GetView())); // TODO GH#1571: Add items from the future customizable new tab dropdown as well. // This could either replace the default profiles, or be added alongside them. @@ -142,13 +143,11 @@ HRESULT Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept // Add the items to the jumplist Task section. // The Tasks section is immutable by the user, unlike the destinations // section that can have its items pinned and removed. - RETURN_IF_FAILED(jumplistInstance->AddUserTasks(jumplistItems.get())); + THROW_IF_FAILED(jumplistInstance->AddUserTasks(jumplistItems.get())); - RETURN_IF_FAILED(jumplistInstance->CommitList()); - - return S_OK; + THROW_IF_FAILED(jumplistInstance->CommitList()); } - CATCH_RETURN(); + CATCH_LOG(); } // Method Description: diff --git a/src/cascadia/TerminalApp/Jumplist.h b/src/cascadia/TerminalApp/Jumplist.h index 3f9baa6cd3b..997c9778f82 100644 --- a/src/cascadia/TerminalApp/Jumplist.h +++ b/src/cascadia/TerminalApp/Jumplist.h @@ -18,7 +18,7 @@ struct IShellLinkW; class Jumplist { public: - static HRESULT UpdateJumplist(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings) noexcept; + static winrt::fire_and_forget UpdateJumplist(const winrt::Microsoft::Terminal::Settings::Model::CascadiaSettings& settings) noexcept; private: [[nodiscard]] static HRESULT _updateProfiles(IObjectCollection* jumplistItems, winrt::Windows::Foundation::Collections::IVectorView profiles) noexcept; From d1394f9dcfc9adce05628da57ffe55b78d823c32 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 21 Oct 2020 15:08:27 -0400 Subject: [PATCH 2/4] use copy of settings --- src/cascadia/TerminalApp/Jumplist.cpp | 6 +++++- src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index 8cd8147f6d9..fbb4562aff2 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/src/cascadia/TerminalApp/Jumplist.cpp @@ -120,7 +120,11 @@ static std::wstring_view _getExePath() // - winrt::fire_and_forget Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept { + // make sure to copy the settings _before_ the co_await + const auto settingsCopy = settings.Copy(); + co_await winrt::resume_background(); + try { auto jumplistInstance = winrt::create_instance(CLSID_DestinationList, CLSCTX_ALL); @@ -135,7 +139,7 @@ winrt::fire_and_forget Jumplist::UpdateJumplist(const CascadiaSettings& settings THROW_IF_FAILED(jumplistItems->Clear()); // Update the list of profiles. - THROW_IF_FAILED(_updateProfiles(jumplistItems.get(), settings.Profiles().GetView())); + THROW_IF_FAILED(_updateProfiles(jumplistItems.get(), settingsCopy.Profiles().GetView())); // TODO GH#1571: Add items from the future customizable new tab dropdown as well. // This could either replace the default profiles, or be added alongside them. diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 2cf9c7febbc..14dedc0a24f 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -244,7 +244,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Action = _Action; - copy->_Args = _Args.Copy(); + copy->_Args = _Args ? _Args.Copy() : IActionArgs{ nullptr }; return copy; } From 60dfa376407b421307bbbb3a5f3c3e392bffe81e Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Wed, 21 Oct 2020 15:56:16 -0400 Subject: [PATCH 3/4] capture not copy --- src/cascadia/TerminalApp/Jumplist.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalApp/Jumplist.cpp b/src/cascadia/TerminalApp/Jumplist.cpp index fbb4562aff2..b06f5ab2c62 100644 --- a/src/cascadia/TerminalApp/Jumplist.cpp +++ b/src/cascadia/TerminalApp/Jumplist.cpp @@ -120,8 +120,8 @@ static std::wstring_view _getExePath() // - winrt::fire_and_forget Jumplist::UpdateJumplist(const CascadiaSettings& settings) noexcept { - // make sure to copy the settings _before_ the co_await - const auto settingsCopy = settings.Copy(); + // make sure to capture the settings _before_ the co_await + const auto strongSettings = settings; co_await winrt::resume_background(); @@ -139,7 +139,7 @@ winrt::fire_and_forget Jumplist::UpdateJumplist(const CascadiaSettings& settings THROW_IF_FAILED(jumplistItems->Clear()); // Update the list of profiles. - THROW_IF_FAILED(_updateProfiles(jumplistItems.get(), settingsCopy.Profiles().GetView())); + THROW_IF_FAILED(_updateProfiles(jumplistItems.get(), strongSettings.Profiles().GetView())); // TODO GH#1571: Add items from the future customizable new tab dropdown as well. // This could either replace the default profiles, or be added alongside them. From 72d78be2c440ee002e341c9afbb91e074d686ee8 Mon Sep 17 00:00:00 2001 From: Pankaj Bhojwani Date: Thu, 22 Oct 2020 16:52:26 -0400 Subject: [PATCH 4/4] remove copy fix to make it its own PR --- src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp index 14dedc0a24f..2cf9c7febbc 100644 --- a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp @@ -244,7 +244,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation { auto copy{ winrt::make_self() }; copy->_Action = _Action; - copy->_Args = _Args ? _Args.Copy() : IActionArgs{ nullptr }; + copy->_Args = _Args.Copy(); return copy; }