Fix possible crash when hiding items

diff --git a/src/panels/places/placesitemmodel.cpp b/src/panels/places/placesitemmodel.cpp
index 4ddd60c0b39777e8f6a2ac895b0a2e54728517ad..4fb86fda88e3bbef9b685a239ecae76e3dd04578 100644 (file)
--- a/src/panels/places/placesitemmodel.cpp
+++ b/src/panels/places/placesitemmodel.cpp
@@ -149,28 +149,41 @@ void PlacesItemModel::setHiddenItemsShown(bool show)
 
     if (show) {
         // Move all items that are part of m_bookmarkedItems to the model.
 
     if (show) {
         // Move all items that are part of m_bookmarkedItems to the model.

+        QList<PlacesItem*> itemsToInsert;
+        QList<int> insertPos;

         int modelIndex = 0;
         for (int i = 0; i < m_bookmarkedItems.count(); ++i) {
             if (m_bookmarkedItems[i]) {
         int modelIndex = 0;
         for (int i = 0; i < m_bookmarkedItems.count(); ++i) {
             if (m_bookmarkedItems[i]) {

-                PlacesItem* visibleItem = new PlacesItem(*m_bookmarkedItems[i]);
-                delete m_bookmarkedItems[i];
-                m_bookmarkedItems.removeAt(i);
-                insertItem(modelIndex, visibleItem);
-                Q_ASSERT(!m_bookmarkedItems[i]);
+                itemsToInsert.append(m_bookmarkedItems[i]);
+                m_bookmarkedItems[i] = 0;
+                insertPos.append(modelIndex);

             }
             ++modelIndex;
         }
             }
             ++modelIndex;
         }

+
+        // Inserting the items will automatically insert an item
+        // to m_bookmarkedItems in PlacesItemModel::onItemsInserted().
+        // The items are temporary saved in itemsToInsert, so
+        // m_bookmarkedItems can be shrinked now.
+        m_bookmarkedItems.erase(m_bookmarkedItems.begin(),
+                                m_bookmarkedItems.begin() + itemsToInsert.count());
+
+        for (int i = 0; i < itemsToInsert.count(); ++i) {
+            insertItem(insertPos[i], itemsToInsert[i]);
+        }
+
+        Q_ASSERT(m_bookmarkedItems.count() == count());

     } else {
         // Move all items of the model, where the "isHidden" property is true, to
     } else {
         // Move all items of the model, where the "isHidden" property is true, to

-        // m_allItems.
+        // m_bookmarkedItems.

         Q_ASSERT(m_bookmarkedItems.count() == count());
         for (int i = count() - 1; i >= 0; --i) {
         Q_ASSERT(m_bookmarkedItems.count() == count());
         for (int i = count() - 1; i >= 0; --i) {

-            const PlacesItem* visibleItem = placesItem(i);
-            if (visibleItem->isHidden()) {
+            if (placesItem(i)->isHidden()) {

                 hideItem(i);
             }
         }
     }
                 hideItem(i);
             }
         }
     }

+

 #ifdef PLACESITEMMODEL_DEBUG
         kDebug() << "Changed visibility of hidden items";
         showModelState();
 #ifdef PLACESITEMMODEL_DEBUG
         kDebug() << "Changed visibility of hidden items";
         showModelState();


@@ -311,23 +324,23 @@ void PlacesItemModel::onItemInserted(int index)
         // case assure that it is also appended after the hidden items and
         // not before (like done otherwise).
         m_bookmarkedItems.append(0);
         // case assure that it is also appended after the hidden items and
         // not before (like done otherwise).
         m_bookmarkedItems.append(0);

-        return;
-    }
+    } else {

 
 

-    int modelIndex = -1;
-    int bookmarkIndex = 0;
-    while (bookmarkIndex < m_bookmarkedItems.count()) {
-        if (!m_bookmarkedItems[bookmarkIndex]) {
-            ++modelIndex;
-            if (modelIndex + 1 == index) {
-                break;
+        int modelIndex = -1;
+        int bookmarkIndex = 0;
+        while (bookmarkIndex < m_bookmarkedItems.count()) {
+            if (!m_bookmarkedItems[bookmarkIndex]) {
+                ++modelIndex;
+                if (modelIndex + 1 == index) {
+                    break;
+                }

             }
             }

+            ++bookmarkIndex;

         }
         }

-        ++bookmarkIndex;
+        m_bookmarkedItems.insert(bookmarkIndex, 0);

     }
     }

-    m_bookmarkedItems.insert(bookmarkIndex, 0);

 
 

-    m_saveBookmarksTimer->start();
+    triggerBookmarksSaving();

 
 #ifdef PLACESITEMMODEL_DEBUG
     kDebug() << "Inserted item" << index;
 
 #ifdef PLACESITEMMODEL_DEBUG
     kDebug() << "Inserted item" << index;


@@ -347,7 +360,7 @@ void PlacesItemModel::onItemRemoved(int index, KStandardItem* removedItem)
     Q_ASSERT(!m_bookmarkedItems[boomarkIndex]);
     m_bookmarkedItems.removeAt(boomarkIndex);
 
     Q_ASSERT(!m_bookmarkedItems[boomarkIndex]);
     m_bookmarkedItems.removeAt(boomarkIndex);
 

-    m_saveBookmarksTimer->start();
+    triggerBookmarksSaving();

 
 #ifdef PLACESITEMMODEL_DEBUG
     kDebug() << "Removed item" << index;
 
 #ifdef PLACESITEMMODEL_DEBUG
     kDebug() << "Removed item" << index;


@@ -381,7 +394,7 @@ void PlacesItemModel::onItemChanged(int index, const QSet<QByteArray>& changedRo
         }
     }
 
         }
     }
 

-    m_saveBookmarksTimer->start();
+    triggerBookmarksSaving();

 }
 
 void PlacesItemModel::slotDeviceAdded(const QString& udi)
 }
 
 void PlacesItemModel::slotDeviceAdded(const QString& udi)


@@ -766,13 +779,20 @@ void PlacesItemModel::hideItem(int index)
             // bookmark should still be remembered, so readd it again:
             m_bookmarkManager->root().addBookmark(hiddenBookmark);
             m_bookmarkManager->root().moveBookmark(hiddenBookmark, previousBookmark);
             // bookmark should still be remembered, so readd it again:
             m_bookmarkManager->root().addBookmark(hiddenBookmark);
             m_bookmarkManager->root().moveBookmark(hiddenBookmark, previousBookmark);

-            m_saveBookmarksTimer->start();
+            triggerBookmarksSaving();

         }
 
         m_bookmarkedItems.insert(newIndex, hiddenItem);
     }
 }
 
         }
 
         m_bookmarkedItems.insert(newIndex, hiddenItem);
     }
 }
 

+void PlacesItemModel::triggerBookmarksSaving()
+{
+    if (m_saveBookmarksTimer) {
+        m_saveBookmarksTimer->start();
+    }
+}
+

 bool PlacesItemModel::equalBookmarkIdentifiers(const KBookmark& b1, const KBookmark& b2)
 {
     const QString udi1 = b1.metaDataItem("UDI");
 bool PlacesItemModel::equalBookmarkIdentifiers(const KBookmark& b1, const KBookmark& b2)
 {
     const QString udi1 = b1.metaDataItem("UDI");

diff --git a/src/panels/places/placesitemmodel.h b/src/panels/places/placesitemmodel.h
index 9444324334abab548229f6379c169e32bb852928..c0ff2e9c5a025f203d96047954f7d76abc6ea923 100644 (file)
--- a/src/panels/places/placesitemmodel.h
+++ b/src/panels/places/placesitemmodel.h
@@ -158,6 +158,12 @@ private:
      */
     void hideItem(int index);
 
      */
     void hideItem(int index);
 

+    /**
+     * Triggers a delayed saving of bookmarks by starting
+     * m_saveBookmarksTimer.
+     */
+    void triggerBookmarksSaving();
+

     /**
      * @return True if the bookmarks have the same identifiers. The identifier
      *         is the unique "ID"-property in case if no UDI is set, otherwise
     /**
      * @return True if the bookmarks have the same identifiers. The identifier
      *         is the unique "ID"-property in case if no UDI is set, otherwise