Make sure that KItemListSizeHintResolver is always consistent

This was the root cause of bug 317827. The assert tried to make sure
that we never access KItemListSizeHintResolver from
KItemListViewLayouter inside the loop over the item ranges. This would
be dangerous because it might be in an inconsistent state - the removed
item ranges were removed step by step, so accessing the item size hints
before the operation was finished could lead to wrong results.

The solution is to insert/remove all item ranges immediately. A nice
side effect is that there are no sources of O(N^2) complexity in
KItemListSizeHintResolver any more if many item ranges are
inserted/removed.

BUG: 317827
FIXED-IN: 4.11.0
REVIEW: 111382

1 files changed, 6 insertions, 22 deletions

diff --git a/src/kitemviews/kitemlistview.cpp b/src/kitemviews/kitemlistview.cpp
index b5e105843..2ea6657a5 100644
--- a/src/kitemviews/kitemlistview.cpp
+++ b/src/kitemviews/kitemlistview.cpp
@@ -958,6 +958,8 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
 
     m_layouter->markAsDirty();
 
+    m_sizeHintResolver->itemsInserted(itemRanges);
+
     int previouslyInsertedCount = 0;
     foreach (const KItemRange& range, itemRanges) {
         // range.index is related to the model before anything has been inserted.
@@ -971,8 +973,6 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
         }
         previouslyInsertedCount += count;
 
-        m_sizeHintResolver->itemsInserted(index, count);
-
         // Determine which visible items must be moved
         QList<int> itemsToMove;
         QHashIterator<int, KItemListWidget*> it(m_visibleItems);
@@ -1030,12 +1030,6 @@ void KItemListView::slotItemsInserted(const KItemRangeList& itemRanges)
     }
 
     if (hasMultipleRanges) {
-#ifndef QT_NO_DEBUG
-        // Important: Don't read any m_layouter-property inside the for-loop in case if
-        // multiple ranges are given! m_layouter accesses m_sizeHintResolver which is
-        // updated in each loop-cycle and has only a consistent state after the loop.
-        Q_ASSERT(m_layouter->isDirty());
-#endif
         m_endTransactionAnimationHint = NoAnimation;
         endTransaction();
 
@@ -1074,6 +1068,8 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
         removedItemsCount += itemRanges[i].count;
     }
 
+    m_sizeHintResolver->itemsRemoved(itemRanges);
+
     for (int i = itemRanges.count() - 1; i >= 0; --i) {
         const KItemRange& range = itemRanges[i];
         const int index = range.index;
@@ -1083,8 +1079,6 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
             continue;
         }
 
-        m_sizeHintResolver->itemsRemoved(index, count);
-
         const int firstRemovedIndex = index;
         const int lastRemovedIndex = index + count - 1;
         const int lastIndex = m_model->count() - 1 + removedItemsCount;
@@ -1153,15 +1147,6 @@ void KItemListView::slotItemsRemoved(const KItemRangeList& itemRanges)
     }
 
     if (hasMultipleRanges) {
-#ifndef QT_NO_DEBUG
-        // Important: Don't read any m_layouter-property inside the for-loop in case if
-        // multiple ranges are given! m_layouter accesses m_sizeHintResolver which is
-        // updated in each loop-cycle and has only a consistent state after the loop.
-        // TODO: This assert can be hit when filtering in Icons and Compact view,
-        // see https://bugs.kde.org/show_bug.cgi?id=317827 comments 2 and 3.
-        // We should try to figure out if the assert is wrong or if there is a bug in the code.
-        //Q_ASSERT(m_layouter->isDirty());
-#endif
         m_endTransactionAnimationHint = NoAnimation;
         endTransaction();
         updateSiblingsInformation();
@@ -1540,9 +1525,9 @@ void KItemListView::setModel(KItemModelBase* model)
                    this,    SLOT(slotSortOrderChanged(Qt::SortOrder,Qt::SortOrder)));
         disconnect(m_model, SIGNAL(sortRoleChanged(QByteArray,QByteArray)),
                    this,    SLOT(slotSortRoleChanged(QByteArray,QByteArray)));
-    }
 
-    m_sizeHintResolver->clearCache();
+        m_sizeHintResolver->itemsRemoved(KItemRangeList() << KItemRange(0, m_model->count()));
+    }
 
     m_model = model;
     m_layouter->setModel(model);
@@ -1566,7 +1551,6 @@ void KItemListView::setModel(KItemModelBase* model)
 
         const int itemCount = m_model->count();
         if (itemCount > 0) {
-            m_sizeHintResolver->itemsInserted(0, itemCount);
             slotItemsInserted(KItemRangeList() << KItemRange(0, itemCount));
         }
     }