Fix crash #1 when filtering items

When filtering items it was possible that the current index got
an invalid value which resulted in accessing the URL of a
null-KFileItem.

There is still one (general) open issue in KFileItemModelRolesUpdater
(crash #2) where a KFileItem that is already null gets read.
It is not really related to filtering but can be triggered
quite easy when filtering huge directories with enabled previews.

CCBUG: 287642

3 files changed, 55 insertions, 30 deletions

diff --git a/src/kitemviews/kfileitemmodel.cpp b/src/kitemviews/kfileitemmodel.cpp
index bd83bf621..6ed10a602 100644
--- a/src/kitemviews/kfileitemmodel.cpp
+++ b/src/kitemviews/kfileitemmodel.cpp
@@ -480,46 +480,37 @@ void KFileItemModel::setExpanded(const QSet<KUrl>& urls)
 void KFileItemModel::setNameFilter(const QString& nameFilter)
 {
     if (m_nameFilter != nameFilter) {
-        // TODO #1: Assure that expanded items only can get hidden
-        // if no child item is visible
-
-        // TODO #2: If the user entered a '*' use a regular expression
+        dispatchPendingItemsToInsert();
 
         m_nameFilter = nameFilter;
 
-        const QString filter = nameFilter.toLower();
-
         // Check which shown items from m_itemData must get
         // hidden and hence moved to m_filteredItems.
         KFileItemList newFilteredItems;
 
         foreach (ItemData* itemData, m_itemData) {
-            if (!matchesNameFilter(itemData->item, filter)) {
-                m_filteredItems.append(itemData->item);
+            if (!matchesNameFilter(itemData->item)) {
                 newFilteredItems.append(itemData->item);
+                m_filteredItems.insert(itemData->item);
             }
         }
 
-        if (!newFilteredItems.isEmpty()) {
-            slotItemsDeleted(newFilteredItems);
-        }
+        removeItems(newFilteredItems);
 
         // Check which hidden items from m_filteredItems should
         // get visible again and hence removed from m_filteredItems.
         KFileItemList newVisibleItems;
 
-        for (int i = m_filteredItems.count() - 1; i >= 0; --i) {
-            const KFileItem item = m_filteredItems.at(i);
-            if (matchesNameFilter(item, filter)) {
+        QMutableSetIterator<KFileItem> it(m_filteredItems);
+        while (it.hasNext()) {
+            const KFileItem item = it.next();
+            if (matchesNameFilter(item)) {
                 newVisibleItems.append(item);
-                m_filteredItems.removeAt(i);
+                m_filteredItems.remove(item);
             }
         }
 
-        if (!newVisibleItems.isEmpty()) {
-            slotNewItems(newVisibleItems);
-            dispatchPendingItemsToInsert();
-        }
+        insertItems(newVisibleItems);
     }
 }
 
@@ -657,7 +648,23 @@ void KFileItemModel::slotCanceled()
 
 void KFileItemModel::slotNewItems(const KFileItemList& items)
 {
-    m_pendingItemsToInsert.append(items);
+    if (m_nameFilter.isEmpty()) {
+        m_pendingItemsToInsert.append(items);
+    } else {
+        // The name-filter is active. Hide filtered items
+        // before inserting them into the model and remember
+        // the filtered items in m_filteredItems.
+        KFileItemList filteredItems;
+        foreach (const KFileItem& item, items) {
+            if (matchesNameFilter(item)) {
+                filteredItems.append(item);
+            } else {
+                m_filteredItems.insert(item);
+            }
+        }
+
+        m_pendingItemsToInsert.append(filteredItems);
+    }
 
     if (useMaximumUpdateInterval() && !m_maximumUpdateIntervalTimer->isActive()) {
         // Assure that items get dispatched if no completed() or canceled() signal is
@@ -668,10 +675,14 @@ void KFileItemModel::slotNewItems(const KFileItemList& items)
 
 void KFileItemModel::slotItemsDeleted(const KFileItemList& items)
 {
-    if (!m_pendingItemsToInsert.isEmpty()) {
-        insertItems(m_pendingItemsToInsert);
-        m_pendingItemsToInsert.clear();
+    dispatchPendingItemsToInsert();
+
+    if (!m_filteredItems.isEmpty()) {
+        foreach (const KFileItem& item, items) {
+            m_filteredItems.remove(item);
+        }
     }
+
     removeItems(items);
 }
 
@@ -738,6 +749,7 @@ void KFileItemModel::slotClear()
     kDebug() << "Clearing all items";
 #endif
 
+    m_filteredItems.clear();
     m_groups.clear();
 
     m_minimumUpdateIntervalTimer->stop();
@@ -907,7 +919,11 @@ void KFileItemModel::removeItems(const KFileItemList& items)
     // Delete the items
     for (int i = indexesToRemove.count() - 1; i >= 0; --i) {
         const int indexToRemove = indexesToRemove.at(i);
-        delete m_itemData.at(indexToRemove);
+        ItemData* data = m_itemData.at(indexToRemove);
+
+        m_items.remove(data->item.url());
+
+        delete data;
         m_itemData.removeAt(indexToRemove);
     }
 
@@ -1707,10 +1723,15 @@ QList<QPair<int, QVariant> > KFileItemModel::genericStringRoleGroups(const QByte
     return groups;
 }
 
-bool KFileItemModel::matchesNameFilter(const KFileItem& item, const QString& nameFilter)
+bool KFileItemModel::matchesNameFilter(const KFileItem& item) const
 {
+    // TODO #1: A performance improvement would be possible by caching m_nameFilter.toLower().
+    // Before adding yet-another-member it should be checked whether it brings a noticable
+    // improvement at all.
+
+    // TODO #2: If the user entered a '*' use a regular expression
     const QString itemText = item.text().toLower();
-    return itemText.contains(nameFilter);
+    return itemText.contains(m_nameFilter.toLower());
 }
 
 #include "kfileitemmodel.moc"
diff --git a/src/kitemviews/kfileitemmodel.h b/src/kitemviews/kfileitemmodel.h
index a83d57ba8..5d0aa4203 100644
--- a/src/kitemviews/kfileitemmodel.h
+++ b/src/kitemviews/kfileitemmodel.h
@@ -275,9 +275,9 @@ private:
     bool isChildItem(int index) const;
 
     /**
-     * @return True if the given item matches with the name filter.
+     * @return True if the given item matches with the current set name filter.
      */
-    static bool matchesNameFilter(const KFileItem& item, const QString& nameFilter);
+    bool matchesNameFilter(const KFileItem& item) const;
 
 private:
     QWeakPointer<KDirLister> m_dirLister;
@@ -293,7 +293,7 @@ private:
     QHash<KUrl, int> m_items; // Allows O(1) access for KFileItemModel::index(const KFileItem& item)
 
     QString m_nameFilter;
-    KFileItemList m_filteredItems; // Items that got hidden by KFileItemModel::setNameFilter()
+    QSet<KFileItem> m_filteredItems; // Items that got hidden by KFileItemModel::setNameFilter()
 
     bool m_requestRole[RolesCount];
 
diff --git a/src/kitemviews/kitemlistselectionmanager.cpp b/src/kitemviews/kitemlistselectionmanager.cpp
index fb279796f..448e79293 100644
--- a/src/kitemviews/kitemlistselectionmanager.cpp
+++ b/src/kitemviews/kitemlistselectionmanager.cpp
@@ -293,15 +293,19 @@ void KItemListSelectionManager::itemsRemoved(const KItemRangeList& itemRanges)
             if (currentItem < itemRange.index) {
                 break;
             }
+
             if (currentItem >= itemRange.index + itemRange.count) {
                 currentItem -= itemRange.count;
-            } else if (currentItem >= m_model->count()) {
+            }
+
+            if (currentItem >= m_model->count()) {
                 currentItem = m_model->count() - 1;
             }
         }
         // Calling setCurrentItem would trigger the selectionChanged signal, but we want to
         // emit it only once in this function -> change the current item manually and emit currentChanged
         m_currentItem = currentItem;
+        Q_ASSERT(m_currentItem < m_model->count());
         emit currentChanged(m_currentItem, previousCurrent);
     }