Merge pull request #21794 from enen92/move_timers_gui

[GUI] Kill skin timers thread, move to ProcessSlow

7 files changed, 35 insertions, 68 deletions

diff --git a/xbmc/Application.cpp b/xbmc/Application.cpp
index 0ba74ca98e..754036cef4 100644
--- a/xbmc/Application.cpp
+++ b/xbmc/Application.cpp
@@ -2983,6 +2983,9 @@ void CApplication::Process()
   // (this can only be done after CServiceBroker::GetGUI()->GetWindowManager().Render())
   CServiceBroker::GetAppMessenger()->ProcessWindowMessages();
 
+  // process skin resources (skin timers)
+  ProcessSkin();
+
   // handle any active scripts
 
   {
diff --git a/xbmc/addons/Skin.cpp b/xbmc/addons/Skin.cpp
index 1268ae13c3..1c2cd9f243 100644
--- a/xbmc/addons/Skin.cpp
+++ b/xbmc/addons/Skin.cpp
@@ -290,11 +290,10 @@ void CSkinInfo::LoadTimers()
   m_skinTimerManager.LoadTimers(timersPath);
 }
 
-void CSkinInfo::StartTimerEvaluation()
+void CSkinInfo::ProcessTimers()
 {
-  m_skinTimerManager.Start();
+  m_skinTimerManager.Process();
 }
-
 void CSkinInfo::ResolveIncludes(TiXmlElement* node,
                                 std::map<INFO::InfoPtr, bool>* xmlIncludeConditions /* = nullptr */)
 {
diff --git a/xbmc/addons/Skin.h b/xbmc/addons/Skin.h
index b22d11558c..cf62818b1a 100644
--- a/xbmc/addons/Skin.h
+++ b/xbmc/addons/Skin.h
@@ -178,7 +178,7 @@ public:
 
   /*! \brief Starts evaluating timers
    */
-  void StartTimerEvaluation();
+  void ProcessTimers();
 
   /*! \brief Called when unloading a skin, allows to cleanup specific
    * skin resources.
diff --git a/xbmc/addons/gui/skin/SkinTimerManager.cpp b/xbmc/addons/gui/skin/SkinTimerManager.cpp
index f1db0e5b88..771d5cc19d 100644
--- a/xbmc/addons/gui/skin/SkinTimerManager.cpp
+++ b/xbmc/addons/gui/skin/SkinTimerManager.cpp
@@ -20,19 +20,6 @@
 
 using namespace std::chrono_literals;
 
-CSkinTimerManager::CSkinTimerManager() : CThread("SkinTimers")
-{
-}
-
-void CSkinTimerManager::Start()
-{
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
-  if (!m_timers.empty())
-  {
-    Create();
-  }
-}
-
 void CSkinTimerManager::LoadTimers(const std::string& path)
 {
   CXBMCTinyXML doc;
@@ -51,7 +38,6 @@ void CSkinTimerManager::LoadTimers(const std::string& path)
   }
 
   const TiXmlElement* timerNode = root->FirstChildElement("timer");
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   while (timerNode)
   {
     LoadTimerInternal(timerNode);
@@ -146,7 +132,6 @@ void CSkinTimerManager::LoadTimerInternal(const TiXmlElement* node)
 
 bool CSkinTimerManager::TimerIsRunning(const std::string& timer) const
 {
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   if (m_timers.count(timer) == 0)
   {
     CLog::LogF(LOGERROR, "Couldn't find Skin Timer with name: {}", timer);
@@ -157,7 +142,6 @@ bool CSkinTimerManager::TimerIsRunning(const std::string& timer) const
 
 float CSkinTimerManager::GetTimerElapsedSeconds(const std::string& timer) const
 {
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   if (m_timers.count(timer) == 0)
   {
     CLog::LogF(LOGERROR, "Couldn't find Skin Timer with name: {}", timer);
@@ -168,7 +152,6 @@ float CSkinTimerManager::GetTimerElapsedSeconds(const std::string& timer) const
 
 void CSkinTimerManager::TimerStart(const std::string& timer) const
 {
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   if (m_timers.count(timer) == 0)
   {
     CLog::LogF(LOGERROR, "Couldn't find Skin Timer with name: {}", timer);
@@ -179,7 +162,6 @@ void CSkinTimerManager::TimerStart(const std::string& timer) const
 
 void CSkinTimerManager::TimerStop(const std::string& timer) const
 {
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   if (m_timers.count(timer) == 0)
   {
     CLog::LogF(LOGERROR, "Couldn't find Skin Timer with name: {}", timer);
@@ -190,14 +172,11 @@ void CSkinTimerManager::TimerStop(const std::string& timer) const
 
 void CSkinTimerManager::Stop()
 {
-  StopThread();
-
   // skintimers, as infomanager clients register info conditions/expressions in the infomanager.
   // The infomanager is linked to skins, being initialized or cleared when
   // skins are loaded (or unloaded). All the registered boolean conditions from
   // skin timers will end up being removed when the skin is unloaded. However, to
   // self-contain this component unregister them all here.
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
   for (auto const& [key, val] : m_timers)
   {
     const std::unique_ptr<CSkinTimer>::pointer timer = val.get();
@@ -217,33 +196,22 @@ void CSkinTimerManager::Stop()
   m_timers.clear();
 }
 
-void CSkinTimerManager::StopThread(bool bWait /*= true*/)
-{
-  std::unique_lock<CCriticalSection> lock(m_timerCriticalSection);
-  m_bStop = true;
-  CThread::StopThread(bWait);
-}
-
 void CSkinTimerManager::Process()
 {
-  while (!m_bStop)
+  for (const auto& [key, val] : m_timers)
   {
-    for (auto const& [key, val] : m_timers)
+    const std::unique_ptr<CSkinTimer>::pointer timer = val.get();
+    if (!timer->IsRunning() && timer->VerifyStartCondition())
+    {
+      timer->Start();
+    }
+    else if (timer->IsRunning() && timer->VerifyStopCondition())
+    {
+      timer->Stop();
+    }
+    if (timer->GetElapsedSeconds() > 0 && timer->VerifyResetCondition())
     {
-      const std::unique_ptr<CSkinTimer>::pointer timer = val.get();
-      if (!timer->IsRunning() && timer->VerifyStartCondition())
-      {
-        timer->Start();
-      }
-      else if (timer->IsRunning() && timer->VerifyStopCondition())
-      {
-        timer->Stop();
-      }
-      if (timer->GetElapsedSeconds() > 0 && timer->VerifyResetCondition())
-      {
-        timer->Reset();
-      }
+      timer->Reset();
     }
-    Sleep(500ms);
   }
 }
diff --git a/xbmc/addons/gui/skin/SkinTimerManager.h b/xbmc/addons/gui/skin/SkinTimerManager.h
index 0a79b7804f..fdf44d1c26 100644
--- a/xbmc/addons/gui/skin/SkinTimerManager.h
+++ b/xbmc/addons/gui/skin/SkinTimerManager.h
@@ -9,7 +9,6 @@
 #pragma once
 
 #include "SkinTimer.h"
-#include "threads/Thread.h"
 
 #include <map>
 #include <memory>
@@ -18,26 +17,24 @@
 /*! \brief CSkinTimerManager is the container and manager for Skin timers. Its role is that of
  * checking if the timer boolean conditions are valid, start or stop timers and execute the respective
  * builtin actions linked to the timer lifecycle
+ * \note This component should only be called by the main/rendering thread
  * \sa Skin_Timers
  * \sa CSkinTimer
  */
-class CSkinTimerManager : public CThread
+class CSkinTimerManager
 {
 public:
   /*! \brief Skin timer manager constructor */
-  CSkinTimerManager();
+  CSkinTimerManager() = default;
 
   /*! \brief Default skin timer manager destructor */
-  ~CSkinTimerManager() override = default;
+  ~CSkinTimerManager() = default;
 
   /*! \brief Loads all the skin timers
   * \param path - the path for the skin Timers.xml file
   */
   void LoadTimers(const std::string& path);
 
-  /*! \brief Starts the manager */
-  void Start();
-
   /*! \brief Stops the manager */
   void Stop();
 
@@ -65,13 +62,8 @@ public:
 
   // CThread methods
 
-  /*! \brief Start and run the main manager loop */
-  void Process() override;
-
-  /*! \brief Stop the manager thread
-   \param bWait - If the callee should wait for the thread to exit (default is true)
-   */
-  void StopThread(bool bWait = true) override;
+  /*! \brief Run the main manager processing loop */
+  void Process();
 
 private:
   /*! \brief Loads a specific timer
@@ -82,6 +74,4 @@ private:
 
   /*! Container for the skin timers */
   std::map<std::string, std::unique_ptr<CSkinTimer>> m_timers;
-
-  mutable CCriticalSection m_timerCriticalSection;
 };
diff --git a/xbmc/application/ApplicationSkinHandling.cpp b/xbmc/application/ApplicationSkinHandling.cpp
index cc925a50f1..5a96f255fa 100644
--- a/xbmc/application/ApplicationSkinHandling.cpp
+++ b/xbmc/application/ApplicationSkinHandling.cpp
@@ -206,10 +206,6 @@ bool CApplicationSkinHandling::LoadSkin(const std::string& skinID,
     }
   }
 
-  // start timer manager after all windows were loaded and skin state was restored since timers might depend on
-  // boolean conditions that reference specific windows
-  g_SkinInfo->StartTimerEvaluation();
-
   return true;
 }
 
@@ -504,3 +500,9 @@ bool CApplicationSkinHandling::OnSettingChanged(const CSetting& setting)
 
   return true;
 }
+
+void CApplicationSkinHandling::ProcessSkin() const
+{
+  if (g_SkinInfo != nullptr)
+    g_SkinInfo->ProcessTimers();
+}
diff --git a/xbmc/application/ApplicationSkinHandling.h b/xbmc/application/ApplicationSkinHandling.h
index 0fe8bd1ffb..396efec0bd 100644
--- a/xbmc/application/ApplicationSkinHandling.h
+++ b/xbmc/application/ApplicationSkinHandling.h
@@ -32,6 +32,11 @@ protected:
   bool LoadCustomWindows();
   void ReloadSkin(bool confirm, IMsgTargetCallback* msgCb, IWindowManagerCallback* wCb);
 
+  /*!
+ * \brief Called by the application main/render thread for processing operations belonging to the skin
+ */
+  void ProcessSkin() const;
+
   CApplicationPlayer& m_appPlayer;
   bool m_saveSkinOnUnloading = true;
   bool m_confirmSkinChange = true;