Call UpdateJumplist only if the settings changed (#13692)

This commit stores a hash of the `settings.json` file in `ApplicationState`
with which we can detect whether the settings contents actually changed.
Since I only use a small 64-bit hash as opposed to SHA2 for instance,
I'm taking the last write time of the file into account as well.
This allows us to skip calling `UpdateJumplist` at least the majority of app
launches which hopefully improves launch performance on devices with slower IO.

Part of #5907.

## Validation Steps Performed


* Delete some profiles (see above), save settings, tasks are gone ✅
  FYI For some (...) inexplicable reason, shell task lists are preserved forever
  even if msix applications are uninstalled, etc. So to test whether tasks are
  properly written on first app launch we have to delete some profiles/tasks
  first, otherwise we can't see whether they're actually written later.
* Now exit Windows Terminal, delete `settings.json` and relaunch
* All tasks are back ✅
* With a debugger, ensure that `CascadiaSettings::WriteSettingsToDisk`
  generates the same hash that `LoadAll` reads. ✅

src/cascadia/TerminalApp/AppLogic.cpp

@@ -297,6 +297,11 @@ namespace winrt::TerminalApp::implementation
             {
                 _root->SetFullscreen(true);
             }
+
+            // Both LoadSettings and ReloadSettings are supposed to call this function,
+            // but LoadSettings skips it, so that the UI starts up faster.
+            // Now that the UI is present we can do them with a less significant UX impact.
+            _ProcessLazySettingsChanges();
         });
         _root->Create();
 
@@ -886,8 +891,27 @@ namespace winrt::TerminalApp::implementation
 
         // Register for directory change notification.
         _RegisterSettingsChange();
+    }
+
+    // Call this function after loading your _settings.
+    // It handles any CPU intensive settings updates (like updating the Jumplist)
+    // which should thus only occur if the settings file actually changed.
+    void AppLogic::_ProcessLazySettingsChanges()
+    {
+        const auto hash = _settings.Hash();
+        const auto applicationState = ApplicationState::SharedInstance();
+        const auto cachedHash = applicationState.SettingsHash();
+
+        // The hash might be empty if LoadAll() failed and we're dealing with the defaults settings object.
+        // In that case we can just wait until the user fixed their settings or CascadiaSettings fixed
+        // itself and either will soon trigger a settings reload.
+        if (hash.empty() || hash == cachedHash)
+        {
+            return;
+        }
 
         Jumplist::UpdateJumplist(_settings);
+        applicationState.SettingsHash(hash);
     }
 
     // Method Description:
@@ -1058,8 +1082,7 @@ namespace winrt::TerminalApp::implementation
         _ApplyLanguageSettingChange();
         _RefreshThemeRoutine();
         _ApplyStartupTaskStateChange();
-
-        Jumplist::UpdateJumplist(_settings);
+        _ProcessLazySettingsChanges();
 
         _SettingsChangedHandlers(*this, nullptr);
     }

src/cascadia/TerminalApp/AppLogic.h

@@ -192,6 +192,7 @@ namespace winrt::TerminalApp::implementation
         void _OnLoaded(const IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs& eventArgs);
 
         [[nodiscard]] HRESULT _TryLoadSettings() noexcept;
+        void _ProcessLazySettingsChanges();
         void _RegisterSettingsChange();
         fire_and_forget _DispatchReloadSettings();
         void _ReloadSettings();

src/cascadia/TerminalSettingsModel/ApplicationState.h

@@ -35,6 +35,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
 // It provides X with the following arguments:
 //   (source, type, function name, JSON key, ...variadic construction arguments)
 #define MTSM_APPLICATION_STATE_FIELDS(X)                                                                                                                                  \
+    X(FileSource::Shared, winrt::hstring, SettingsHash, "settingsHash")                                                                                                   \
     X(FileSource::Shared, std::unordered_set<winrt::guid>, GeneratedProfiles, "generatedProfiles")                                                                        \
     X(FileSource::Local, Windows::Foundation::Collections::IVector<Model::WindowLayout>, PersistedWindowLayouts, "persistedWindowLayouts")                                \
     X(FileSource::Shared, Windows::Foundation::Collections::IVector<hstring>, RecentCommands, "recentCommands")                                                           \

src/cascadia/TerminalSettingsModel/ApplicationState.idl

@@ -33,13 +33,10 @@ namespace Microsoft.Terminal.Settings.Model
 
         Boolean IsStatePath(String filename);
 
-        Windows.Foundation.Collections.IVector<WindowLayout> PersistedWindowLayouts { get; set; };
-
-        Windows.Foundation.Collections.IVector<String> RecentCommands { get; set; };
-
-        Windows.Foundation.Collections.IVector<InfoBarMessage> DismissedMessages { get; set; };
-
-        Windows.Foundation.Collections.IVector<String> AllowedCommandlines { get; set; };
-
+        String SettingsHash;
+        Windows.Foundation.Collections.IVector<WindowLayout> PersistedWindowLayouts;
+        Windows.Foundation.Collections.IVector<String> RecentCommands;
+        Windows.Foundation.Collections.IVector<InfoBarMessage> DismissedMessages;
+        Windows.Foundation.Collections.IVector<String> AllowedCommandlines;
     }
 }

src/cascadia/TerminalSettingsModel/CascadiaSettings.cpp

@@ -43,6 +43,11 @@ winrt::com_ptr<Profile> Model::implementation::CreateChild(const winrt::com_ptr<
     return profile;
 }
 
+winrt::hstring CascadiaSettings::Hash() const noexcept
+{
+    return _hash;
+}
+
 Model::CascadiaSettings CascadiaSettings::Copy() const
 {
     const auto settings{ winrt::make_self<CascadiaSettings>() };

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -113,12 +113,13 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         explicit CascadiaSettings(SettingsLoader&& loader);
 
         // user settings
+        winrt::hstring Hash() const noexcept;
         Model::CascadiaSettings Copy() const;
         Model::GlobalAppSettings GlobalSettings() const;
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> AllProfiles() const noexcept;
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> ActiveProfiles() const noexcept;
         Model::ActionMap ActionMap() const noexcept;
-        void WriteSettingsToDisk() const;
+        void WriteSettingsToDisk();
         Json::Value ToJson() const;
         Model::Profile ProfileDefaults() const;
         Model::Profile CreateNewProfile();
@@ -146,6 +147,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     private:
         static const std::filesystem::path& _settingsPath();
         static const std::filesystem::path& _releaseSettingsPath();
+        static winrt::hstring _calculateHash(std::string_view settings, const FILETIME& lastWriteTime);
 
         winrt::com_ptr<implementation::Profile> _createNewProfile(const std::wstring_view& name) const;
         Model::Profile _getProfileForCommandLine(const winrt::hstring& commandLine) const;
@@ -162,6 +164,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void _validateThemeExists();
 
         // user settings
+        winrt::hstring _hash;
         winrt::com_ptr<implementation::GlobalAppSettings> _globals = winrt::make_self<implementation::GlobalAppSettings>();
         winrt::com_ptr<implementation::Profile> _baseLayerProfile = winrt::make_self<implementation::Profile>();
         winrt::Windows::Foundation::Collections::IObservableVector<Model::Profile> _allProfiles = winrt::single_threaded_observable_vector<Model::Profile>();

src/cascadia/TerminalSettingsModel/CascadiaSettings.idl

@@ -20,12 +20,14 @@ namespace Microsoft.Terminal.Settings.Model
         static String ApplicationVersion { get; };
 
         static void ExportFile(String path, String content);
-        
+
         CascadiaSettings(String userJSON, String inboxJSON);
 
         CascadiaSettings Copy();
         void WriteSettingsToDisk();
 
+        String Hash { get; };
+
         GlobalAppSettings GlobalSettings { get; };
 
         Profile ProfileDefaults { get; };

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

@@ -793,7 +793,8 @@ void SettingsLoader::_executeGenerator(const IDynamicProfileGenerator& generator
 Model::CascadiaSettings CascadiaSettings::LoadAll()
 try
 {
-    auto settingsString = ReadUTF8FileIfExists(_settingsPath()).value_or(std::string{});
+    FILETIME lastWriteTime{};
+    auto settingsString = ReadUTF8FileIfExists(_settingsPath(), false, &lastWriteTime).value_or(std::string{});
     auto firstTimeSetup = settingsString.empty();
 
     // If it's the firstTimeSetup and a preview build, then try to
@@ -874,6 +875,12 @@ try
             settings->_warnings.Append(SettingsLoadWarnings::FailedToWriteToSettings);
         }
     }
+    else
+    {
+        // lastWriteTime is only valid if mustWriteToDisk is false.
+        // Additionally WriteSettingsToDisk() updates the _hash for us already.
+        settings->_hash = _calculateHash(settingsString, lastWriteTime);
+    }
 
     return *settings;
 }
@@ -1015,6 +1022,15 @@ const std::filesystem::path& CascadiaSettings::_releaseSettingsPath()
     return path;
 }
 
+// Returns a has (approximately) uniquely identifying the settings.json contents on disk.
+winrt::hstring CascadiaSettings::_calculateHash(std::string_view settings, const FILETIME& lastWriteTime)
+{
+    const auto fileHash = til::hash(settings);
+    const ULARGE_INTEGER fileTime{ lastWriteTime.dwLowDateTime, lastWriteTime.dwHighDateTime };
+    const auto hash = fmt::format(L"{:016x}-{:016x}", fileHash, fileTime.QuadPart);
+    return winrt::hstring{ hash };
+}
+
 // function Description:
 // - Returns the full path to the settings file, either within the application
 //   package, or in its unpackaged location. This path is under the "Local
@@ -1058,7 +1074,7 @@ winrt::hstring CascadiaSettings::DefaultSettingsPath()
 // - <none>
 // Return Value:
 // - <none>
-void CascadiaSettings::WriteSettingsToDisk() const
+void CascadiaSettings::WriteSettingsToDisk()
 {
     const auto settingsPath = _settingsPath();
 
@@ -1067,8 +1083,11 @@ void CascadiaSettings::WriteSettingsToDisk() const
     wbuilder.settings_["indentation"] = "    ";
     wbuilder.settings_["enableYAMLCompatibility"] = true; // suppress spaces around colons
 
+    FILETIME lastWriteTime{};
     const auto styledString{ Json::writeString(wbuilder, ToJson()) };
-    WriteUTF8FileAtomic(settingsPath, styledString);
+    WriteUTF8FileAtomic(settingsPath, styledString, &lastWriteTime);
+
+    _hash = _calculateHash(styledString, lastWriteTime);
 
     // Persists the default terminal choice
     // GH#10003 - Only do this if _currentDefaultTerminal was actually initialized.

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -107,7 +107,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model
     }
     // Tries to read a file somewhat atomically without locking it.
     // Strips the UTF8 BOM if it exists.
-    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly)
+    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         // From some casual observations we can determine that:
         // * ReadFile() always returns the requested amount of data (unless the file is smaller)
@@ -179,18 +179,23 @@ namespace winrt::Microsoft::Terminal::Settings::Model
                 buffer.erase(0, Utf8Bom.size());
             }
 
+            if (lastWriteTime)
+            {
+                THROW_IF_WIN32_BOOL_FALSE(GetFileTime(file.get(), nullptr, nullptr, lastWriteTime));
+            }
+
             return buffer;
         }
 
         THROW_WIN32_MSG(ERROR_READ_FAULT, "file size changed while reading");
     }
 
     // Same as ReadUTF8File, but returns an empty optional, if the file couldn't be opened.
-    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly)
+    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         try
         {
-            return { ReadUTF8File(path, elevatedOnly) };
+            return { ReadUTF8File(path, elevatedOnly, lastWriteTime) };
         }
         catch (const wil::ResultException& exception)
         {
@@ -203,9 +208,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model
         }
     }
 
-    void WriteUTF8File(const std::filesystem::path& path,
-                       const std::string_view& content,
-                       const bool elevatedOnly)
+    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly, FILETIME* lastWriteTime)
     {
         SECURITY_ATTRIBUTES sa;
         // stash the security descriptor here, so it will stay in context until
@@ -277,10 +280,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model
         {
             THROW_WIN32_MSG(ERROR_WRITE_FAULT, "failed to write whole file");
         }
+
+        if (lastWriteTime)
+        {
+            THROW_IF_WIN32_BOOL_FALSE(GetFileTime(file.get(), nullptr, nullptr, lastWriteTime));
+        }
     }
 
-    void WriteUTF8FileAtomic(const std::filesystem::path& path,
-                             const std::string_view& content)
+    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content, FILETIME* lastWriteTime)
     {
         // GH#10787: rename() will replace symbolic links themselves and not the path they point at.
         // It's thus important that we first resolve them before generating temporary path.
@@ -298,15 +305,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model
             //   * resolve the link manually, which might be less accurate and more prone to race conditions
             //   * write to the file directly, which lets the system resolve the symbolic link but leaves the write non-atomic
             // The latter is chosen, as this is an edge case and our 'atomic' writes are only best-effort.
-            WriteUTF8File(path, content);
+            WriteUTF8File(path, content, false, lastWriteTime);
             return;
         }
 
         auto tmpPath = resolvedPath;
         tmpPath += L".tmp";
 
         // Writing to a file isn't atomic, but...
-        WriteUTF8File(tmpPath, content);
+        WriteUTF8File(tmpPath, content, false, lastWriteTime);
 
         // renaming one is (supposed to be) atomic.
         // Wait... "supposed to be"!? Well it's technically not always atomic,

src/cascadia/TerminalSettingsModel/FileUtils.h

@@ -5,8 +5,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model
 {
     std::filesystem::path GetBaseSettingsPath();
     std::filesystem::path GetReleaseSettingsPath();
-    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly = false);
-    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly = false);
-    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly = false);
-    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content);
+    std::string ReadUTF8File(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    std::optional<std::string> ReadUTF8FileIfExists(const std::filesystem::path& path, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    void WriteUTF8File(const std::filesystem::path& path, const std::string_view& content, const bool elevatedOnly = false, FILETIME* lastWriteTime = nullptr);
+    void WriteUTF8FileAtomic(const std::filesystem::path& path, const std::string_view& content, FILETIME* lastWriteTime = nullptr);
 }