From 702d6a69148091b23023aeafdb697919dff1a1d6 Mon Sep 17 00:00:00 2001
From: Eike Ziller <eike.ziller@qt.io>
Date: Wed, 27 Jun 2018 15:26:46 +0200
Subject: [PATCH] TaskModel: Bit of code cleanup

Do not send rowsRemoved before source model has removed the rows from
its data.
Add some QTC_ASSERTS and comments.
Replace trivial slot by lambda.

Change-Id: I7a0df404f757fca5f7724be66e516824ecd292dd
Reviewed-by: Orgad Shaneh <orgads@gmail.com>
---
 src/plugins/projectexplorer/taskmodel.cpp | 37 ++++++++++++-----------
 src/plugins/projectexplorer/taskmodel.h   |  1 -
 2 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/src/plugins/projectexplorer/taskmodel.cpp b/src/plugins/projectexplorer/taskmodel.cpp
index 35929efbaf2..d59162f5c55 100644
--- a/src/plugins/projectexplorer/taskmodel.cpp
+++ b/src/plugins/projectexplorer/taskmodel.cpp
@@ -331,10 +331,18 @@ TaskFilterModel::TaskFilterModel(TaskModel *sourceModel, QObject *parent) : QAbs
 
     connect(m_sourceModel, &QAbstractItemModel::rowsInserted,
             this, &TaskFilterModel::handleNewRows);
+
     connect(m_sourceModel, &QAbstractItemModel::rowsAboutToBeRemoved,
             this, &TaskFilterModel::handleRowsAboutToBeRemoved);
+    connect(m_sourceModel, &QAbstractItemModel::rowsRemoved,
+            this, [this](const QModelIndex &parent, int, int) {
+        QTC_ASSERT(!parent.isValid(), return);
+        endRemoveRows();
+    });
+
     connect(m_sourceModel, &QAbstractItemModel::modelReset,
-            this, &TaskFilterModel::handleReset);
+            this, &TaskFilterModel::invalidateFilter);
+
     connect(m_sourceModel, &QAbstractItemModel::dataChanged,
             this, &TaskFilterModel::handleDataChanged);
 
@@ -383,8 +391,7 @@ static QPair<int, int> findFilteredRange(int first, int last, const QList<int> &
 
 void TaskFilterModel::handleNewRows(const QModelIndex &index, int first, int last)
 {
-    if (index.isValid())
-        return;
+    QTC_ASSERT(!index.isValid(), return);
 
     QList<int> newMapping;
     for (int i = first; i <= last; ++i) {
@@ -421,18 +428,17 @@ void TaskFilterModel::handleNewRows(const QModelIndex &index, int first, int las
 
 void TaskFilterModel::handleRowsAboutToBeRemoved(const QModelIndex &index, int first, int last)
 {
-    if (index.isValid())
-        return;
+    QTC_ASSERT(!index.isValid(), return);
 
     const QPair<int, int> range = findFilteredRange(first, last, m_mapping);
-    if (range.first > range.second)
+    if (range.first > range.second) // rows to be removed are filtered out
         return;
 
     beginRemoveRows(QModelIndex(), range.first, range.second);
     m_mapping.erase(m_mapping.begin() + range.first, m_mapping.begin() + range.second + 1);
+    const int sourceRemovedCount = (last - first) + 1;
     for (int i = range.first; i < m_mapping.count(); ++i)
-        m_mapping[i] = m_mapping.at(i) - (last - first) - 1;
-    endRemoveRows();
+        m_mapping[i] = m_mapping.at(i) - sourceRemovedCount;
 }
 
 void TaskFilterModel::handleDataChanged(const QModelIndex &top, const QModelIndex &bottom)
@@ -444,24 +450,21 @@ void TaskFilterModel::handleDataChanged(const QModelIndex &top, const QModelInde
     emit dataChanged(index(range.first, top.column()), index(range.second, bottom.column()));
 }
 
-void TaskFilterModel::handleReset()
-{
-    invalidateFilter();
-}
-
 QModelIndex TaskFilterModel::mapFromSource(const QModelIndex &idx) const
 {
-    auto it = std::lower_bound(m_mapping.constBegin(), m_mapping.constEnd(), idx.row());
-    if (it == m_mapping.constEnd() || idx.row() != *it)
+    if (!idx.isValid())
         return QModelIndex();
+    auto it = std::lower_bound(m_mapping.constBegin(), m_mapping.constEnd(), idx.row());
+    QTC_ASSERT(it != m_mapping.constEnd() && idx.row() == *it, return QModelIndex());
     return index(it - m_mapping.constBegin(), 0);
 }
 
 QModelIndex TaskFilterModel::mapToSource(const QModelIndex &index) const
 {
-    int row = index.row();
-    if (row >= m_mapping.count())
+    if (!index.isValid())
         return QModelIndex();
+    int row = index.row();
+    QTC_ASSERT(row >= 0 && row < m_mapping.count(), return QModelIndex());
     return m_sourceModel->index(m_mapping.at(row), index.column(), index.parent());
 }
 
diff --git a/src/plugins/projectexplorer/taskmodel.h b/src/plugins/projectexplorer/taskmodel.h
index 0b761adabb4..b2b1c933e2b 100644
--- a/src/plugins/projectexplorer/taskmodel.h
+++ b/src/plugins/projectexplorer/taskmodel.h
@@ -159,7 +159,6 @@ private:
     void handleNewRows(const QModelIndex &index, int first, int last);
     void handleRowsAboutToBeRemoved(const QModelIndex &index, int first, int last);
     void handleDataChanged(const QModelIndex &top, const QModelIndex &bottom);
-    void handleReset();
 
     QModelIndex mapToSource(const QModelIndex &index) const;
     void invalidateFilter();