When used by QML, only populate after component is complete

Summary:
This introduces the use of QQmlParserStatus to delay populating
the model until all properties have been set, to avoid delegate
churn.

TasksModel is also meant to be used by C++. There's no good way
to determine whether an object is being instanciated by QML
during construction time, therefore this patch also introduces a
delay in initial population of the model after construction via
a single-shot timer. At the time the slot is invoked we know if
we're used by QML (because QQmlParserStatus::classBegin has
either been called or not by then) so we can decide to populate
or wait more for QQmlParserStatus::componentComplete.

I'm not super happy with this behavior change for C++ users,
however as the model is usually used via QML currently, it's
pragmatic to optimize performance for the common case, and it
doesn't technically break QAbstractItemModel semantics, as model
population isn't required to be sync.

There's a decent change this fixes a recently-reported crash as a
by-product:
CCBUG:386630

Reviewers: #plasma, davidedmundson, mart

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D8723

libtaskmanager/tasksmodel.cpp

@@ -80,6 +80,9 @@ public:
     bool groupInline = false;
     int groupingWindowTasksThreshold = -1;
 
+    bool usedByQml = false;
+    bool componentComplete = false;
+
     void initModels();
     void initLauncherTasksModel();
     void updateAnyTaskDemandsAttention();
@@ -438,17 +441,6 @@ void TasksModel::Private::initModels()
     QObject::connect(groupingProxyModel, &QAbstractItemModel::modelReset, q,
         [this]() { updateAnyTaskDemandsAttention(); }
     );
-
-    abstractTasksSourceModel = groupingProxyModel;
-    q->setSourceModel(groupingProxyModel);
-
-    QObject::connect(q, &QAbstractItemModel::rowsInserted, q, &TasksModel::updateLauncherCount);
-    QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, &TasksModel::updateLauncherCount);
-    QObject::connect(q, &QAbstractItemModel::modelReset, q, &TasksModel::updateLauncherCount);
-
-    QObject::connect(q, &QAbstractItemModel::rowsInserted, q, &TasksModel::countChanged);
-    QObject::connect(q, &QAbstractItemModel::rowsRemoved, q, &TasksModel::countChanged);
-    QObject::connect(q, &QAbstractItemModel::modelReset, q, &TasksModel::countChanged);
 }
 
 void TasksModel::Private::updateAnyTaskDemandsAttention()
@@ -637,6 +629,12 @@ void TasksModel::Private::consolidateManualSortMapForGroup(const QModelIndex &gr
 
 void TasksModel::Private::updateGroupInline()
 {
+    if (usedByQml && !componentComplete) {
+        return;
+    }
+
+    bool hadSourceModel = (q->sourceModel() != nullptr);
+
     if (q->groupMode() != GroupDisabled && groupInline) {
         if (flattenGroupsProxyModel) {
             return;
@@ -661,7 +659,7 @@ void TasksModel::Private::updateGroupInline()
             forceResort();
         }
     } else {
-        if (!flattenGroupsProxyModel) {
+        if (hadSourceModel && !flattenGroupsProxyModel) {
             return;
         }
 
@@ -674,10 +672,28 @@ void TasksModel::Private::updateGroupInline()
         delete flattenGroupsProxyModel;
         flattenGroupsProxyModel = nullptr;
 
-        if (sortMode == SortManual) {
+        if (hadSourceModel && sortMode == SortManual) {
             forceResort();
         }
     }
+
+    // Minor optimization: We only make these connections after we populate for
+    // the first time to avoid some churn.
+    if (!hadSourceModel) {
+        QObject::connect(q, &QAbstractItemModel::rowsInserted, q,
+            &TasksModel::updateLauncherCount, Qt::UniqueConnection);
+        QObject::connect(q, &QAbstractItemModel::rowsRemoved, q,
+            &TasksModel::updateLauncherCount, Qt::UniqueConnection);
+        QObject::connect(q, &QAbstractItemModel::modelReset, q,
+            &TasksModel::updateLauncherCount, Qt::UniqueConnection);
+
+        QObject::connect(q, &QAbstractItemModel::rowsInserted, q,
+            &TasksModel::countChanged, Qt::UniqueConnection);
+        QObject::connect(q, &QAbstractItemModel::rowsRemoved, q,
+            &TasksModel::countChanged, Qt::UniqueConnection);
+        QObject::connect(q, &QAbstractItemModel::modelReset, q,
+            &TasksModel::countChanged, Qt::UniqueConnection);
+    }
 }
 
 QModelIndex TasksModel::Private::preFilterIndex(const QModelIndex &sourceIndex) const {
@@ -893,6 +909,18 @@ TasksModel::TasksModel(QObject *parent)
 
     // Start sorting.
     sort(0);
+
+    // Private::updateGroupInline() sets our source model, populating the model. We
+    // delay running this until the QML runtime had a chance to call our implementation
+    // of QQmlParserStatus::classBegin(), setting Private::usedByQml to true. If used
+    // by QML, Private::updateGroupInline() will abort if the component is not yet
+    // complete, instead getting called through QQmlParserStatus::componentComplete()
+    // only after all properties have been set. This avoids delegate churn in Qt Quick
+    // views using the model. If not used by QML, Private::updateGroupInline() will run
+    // directly.
+    QTimer::singleShot(0, this, [this]() {
+        d->updateGroupInline();
+    });
 }
 
 TasksModel::~TasksModel()
@@ -1684,6 +1712,19 @@ QModelIndex TasksModel::makeModelIndex(int row, int childRow) const
     return QModelIndex();
 }
 
+void TasksModel::classBegin()
+{
+    d->usedByQml = true;
+}
+
+void TasksModel::componentComplete()
+{
+    d->componentComplete = true;
+
+    // Sets our source model, populating the model.
+    d->updateGroupInline();
+}
+
 bool TasksModel::filterAcceptsRow(int sourceRow, const QModelIndex &sourceParent) const
 {
     // All our filtering occurs at the top-level; anything below always

libtaskmanager/tasksmodel.h

@@ -21,6 +21,7 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 #ifndef TASKSMODEL_H
 #define TASKSMODEL_H
 
+#include <QQmlParserStatus>
 #include <QSortFilterProxyModel>
 
 #include "abstracttasksmodeliface.h"
@@ -53,9 +54,10 @@ namespace TaskManager
  * @author Eike Hein <hein@kde.org>
  **/
 
-class TASKMANAGER_EXPORT TasksModel : public QSortFilterProxyModel, public AbstractTasksModelIface
+class TASKMANAGER_EXPORT TasksModel : public QSortFilterProxyModel, public AbstractTasksModelIface, public QQmlParserStatus
 {
     Q_OBJECT
+    Q_INTERFACES(QQmlParserStatus)
 
     Q_PROPERTY(int count READ rowCount NOTIFY countChanged)
     Q_PROPERTY(int launcherCount READ launcherCount NOTIFY launcherCountChanged)
@@ -809,6 +811,9 @@ public:
      */
     Q_INVOKABLE QModelIndex makeModelIndex(int row, int childRow = -1) const;
 
+    void classBegin() override;
+    void componentComplete() override;
+
 Q_SIGNALS:
     void countChanged() const;
     void launcherCountChanged() const;