[vlc-devel] [PATCH] qt: medialib: add async task handle

[Romain Vimont]

Use a smart pointer to "abandon" an AsyncTask using RAII. This avoids to
forget to call abandon() on client destruction.
---
 modules/gui/qt/util/asynctask.hpp | 22 +++++++++++++++++++
 modules/gui/qt/util/listcache.hpp | 36 +++++++++----------------------
 2 files changed, 32 insertions(+), 26 deletions(-)

diff --git a/modules/gui/qt/util/asynctask.hpp b/modules/gui/qt/util/asynctask.hpp
index fa4a6d3b2c..81fcc54022 100644
--- a/modules/gui/qt/util/asynctask.hpp
+++ b/modules/gui/qt/util/asynctask.hpp
@@ -128,6 +128,9 @@
  * to declare that it won't use it anymore (and don't want the result). When
  * both conditions are met, the task is deleted via `QObject::deleteLater()`.
  *
+ * For convenience, a `TaskHandle` smart pointer is provided to call
+ * `abandon()` using RAII.
+ *
  * The task result is provided via a separate method `takeResult()` instead of
  * a slot parameter, because otherwise, Qt would require its type to be:
  *  - non-template (like `std::vector<T>`),
@@ -243,4 +246,23 @@ private:
     AsyncTask<T> *m_task;
 };
 
+template <typename T>
+struct TaskDeleter
+{
+    void operator()(T *task) { task->abandon(); }
+};
+
+/**
+ * Task handle to abandon() a task using RAII
+ *
+ * It behaves like a unique_ptr, but its deleter calls AsyncTask<T>::abandon(),
+ * which indicates that the client cancels the task and requests deletion when
+ * possible.
+ *
+ * The actual deletion may happen later (when the runnable also finished its
+ * work).
+ */
+template <typename T>
+using TaskHandle = std::unique_ptr<T, TaskDeleter<T>>;
+
 #endif
diff --git a/modules/gui/qt/util/listcache.hpp b/modules/gui/qt/util/listcache.hpp
index e92b7d84df..20fa510832 100644
--- a/modules/gui/qt/util/listcache.hpp
+++ b/modules/gui/qt/util/listcache.hpp
@@ -115,7 +115,6 @@ public:
         : m_threadPool(threadPool)
         , m_loader(loader)
         , m_chunkSize(chunkSize) {}
-    ~ListCache();
 
     /**
      * Return the item at specified index
@@ -167,19 +166,10 @@ private:
     bool m_countRequested = false;
     MLRange m_lastRangeRequested;
 
-    LoadTask<T> *m_loadTask = nullptr;
-    CountTask<T> *m_countTask = nullptr;
+    TaskHandle<LoadTask<T>> m_loadTask;
+    TaskHandle<CountTask<T>> m_countTask;
 };
 
-template <typename T>
-ListCache<T>::~ListCache()
-{
-    if (m_countTask)
-        m_countTask->abandon();
-    if (m_loadTask)
-        m_loadTask->abandon();
-}
-
 template <typename T>
 const T *ListCache<T>::get(size_t index) const
 {
@@ -248,8 +238,8 @@ void ListCache<T>::asyncCount()
 {
     assert(!m_countTask);
 
-    m_countTask = new CountTask<T>(m_loader);
-    connect(m_countTask, &BaseAsyncTask::result,
+    m_countTask.reset(new CountTask<T>(m_loader));
+    connect(&*m_countTask, &BaseAsyncTask::result,
             this, &ListCache<T>::onCountResult);
     m_countRequested = true;
     m_countTask->start(m_threadPool);
@@ -259,15 +249,14 @@ template <typename T>
 void ListCache<T>::onCountResult()
 {
     CountTask<T> *task = static_cast<CountTask<T> *>(sender());
-    assert(task == m_countTask);
+    assert(task == &*m_countTask);
 
     m_offset = 0;
     m_list.clear();
     m_total_count = static_cast<ssize_t>(task->takeResult());
     emit localSizeChanged(m_total_count);
 
-    task->abandon();
-    m_countTask = nullptr;
+    m_countTask.reset();
 }
 
 template <typename T>
@@ -298,12 +287,8 @@ private:
 template <typename T>
 void ListCache<T>::asyncLoad(size_t offset, size_t count)
 {
-    if (m_loadTask)
-        /* Cancel any current pending task */
-        m_loadTask->abandon();
-
-    m_loadTask = new LoadTask<T>(m_loader, offset, count);
-    connect(m_loadTask, &BaseAsyncTask::result,
+    m_loadTask.reset(new LoadTask<T>(m_loader, offset, count));
+    connect(&*m_loadTask, &BaseAsyncTask::result,
             this, &ListCache<T>::onLoadResult);
     m_lastRangeRequested = { offset, count };
     m_loadTask->start(m_threadPool);
@@ -313,15 +298,14 @@ template <typename T>
 void ListCache<T>::onLoadResult()
 {
     LoadTask<T> *task = static_cast<LoadTask<T> *>(sender());
-    assert(task == m_loadTask);
+    assert(task == &*m_loadTask);
 
     m_offset = task->m_offset;
     m_list = task->takeResult();
     if (m_list.size())
         emit localDataChanged(m_offset, m_list.size());
 
-    task->abandon();
-    m_loadTask = nullptr;
+    m_loadTask.reset();
 }
 
 #endif
-- 
2.30.0