[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [kate] part/completion: Fix assert in ~QPersistentModelIndex
From:       Kevin Funk <kevin () kfunk ! org>
Date:       2013-12-31 15:58:24
Message-ID: E1Vy1i4-0006Uy-Bw () scm ! kde ! org
[Download RAW message or body]

Git commit b768749230872b5acf0e2816dd855409f6d70733 by Kevin Funk.
Committed on 11/12/2013 at 23:51.
Pushed by kfunk into branch 'master'.

Fix assert in ~QPersistentModelIndex

KateCompletionWidget may call QAIM::setCurrentIndex during a reset of
the model, inside KateCompletionWidget::modelContentChanged(). At this
point, model indices are invalid, but QAIM wasn't informed about it.

The main problem is that KateCompletionModel doesn't use the
(relatively) new {begin,end}ResetModel blocks to make persistent model
indices invalid when we start resetting the model. Instead reset() is used
*after* we have reset the model. However, KateCompletionModel may emit
contentGeometryChanged too early (before a reset()) and hence triggering
QAIM::setCurrentIndex while QAIM is still referencing now invalid
persistent model indices.

Other changes:
Remove contentGeometryChanged. Instead, use layoutChanged() and
modelReset() signals to react to model changes.

This also ports KateCompletionModel away from deprecated QAIM methods
such as QAIM::reset()

BUG: 236948
REVIEW: 113917
FIXED-IN: 4.13

M  +38   -36   part/completion/katecompletionmodel.cpp
M  +5    -4    part/completion/katecompletionmodel.h
M  +4    -2    part/completion/katecompletionwidget.cpp

http://commits.kde.org/kate/b768749230872b5acf0e2816dd855409f6d70733

diff --git a/part/completion/katecompletionmodel.cpp \
b/part/completion/katecompletionmodel.cpp index 3fa8080..22af495 100644
--- a/part/completion/katecompletionmodel.cpp
+++ b/part/completion/katecompletionmodel.cpp
@@ -504,7 +504,7 @@ QModelIndex KateCompletionModel::indexForGroup( Group * g ) const
   return createIndex(row, 0, 0);
 }
 
-void KateCompletionModel::clearGroups( bool shouldReset )
+void KateCompletionModel::clearGroups()
 {
   clearExpanding();
   m_ungrouped->clear();
@@ -536,9 +536,6 @@ void KateCompletionModel::clearGroups( bool shouldReset )
 
   m_emptyGroups.append(m_bestMatches);
   m_groupHash.insert(BestMatchesProperty, m_bestMatches);
-
-  if(shouldReset)
-    reset();
 }
 
 QSet<KateCompletionModel::Group*> KateCompletionModel::createItems(const \
HierarchicalModelHandler& _handler, const QModelIndex& i, bool notifyModel) { @@ \
-577,9 +574,10 @@ QSet<KateCompletionModel::Group*> \
KateCompletionModel::deleteItems(const QModelI  
 void KateCompletionModel::createGroups()
 {
+  beginResetModel();
   //After clearing the model, it has to be reset, else we will be in an invalid \
state while inserting  //new groups.
-  clearGroups(true);
+  clearGroups();
 
   bool has_groups=false;
   foreach (CodeCompletionModel* sourceModel, m_completionModels) {
@@ -600,10 +598,7 @@ void KateCompletionModel::createGroups()
   makeGroupItemsUnique();
   
   updateBestMatches();
-  
-  reset();
-
-  emit contentGeometryChanged();
+  endResetModel();
 }
 
 KateCompletionModel::Group* KateCompletionModel::createItem(const \
HierarchicalModelHandler& handler, const QModelIndex& sourceIndex, bool notifyModel) \
@@ -660,9 +655,7 @@ void KateCompletionModel::slotRowsInserted( const QModelIndex & \
                parent, int star
     affectedGroups += createItems(handler, parent.isValid() ? parent.child(i, 0) :  \
handler.model()->index(i, 0), true);  
   foreach (Group* g, affectedGroups)
-      hideOrShowGroup(g);
-
-    emit contentGeometryChanged();
+      hideOrShowGroup(g, true);
 }
 
 void KateCompletionModel::slotRowsRemoved( const QModelIndex & parent, int start, \
int end ) @@ -678,9 +671,7 @@ void KateCompletionModel::slotRowsRemoved( const \
QModelIndex & parent, int start  }
 
   foreach (Group* g, affectedGroups)
-    hideOrShowGroup(g);
-
-  emit contentGeometryChanged();
+    hideOrShowGroup(g, true);
 }
 
 KateCompletionModel::Group* KateCompletionModel::fetchGroup( int attribute, const \
QString& scope, bool forceGrouping ) @@ -938,31 +929,32 @@ void \
KateCompletionModel::setCurrentCompletion( KTextEditor::CodeCompletionModel  
   m_currentMatch[model] = completion;
 
-  bool needsReset = false;
+  const bool resetModel = (changeType != Narrow);
+  if (resetModel) {
+    beginResetModel();
+  }
   
   if (!hasGroups()) {
-    needsReset |= changeCompletions(m_ungrouped, changeType);
+    changeCompletions(m_ungrouped, changeType, !resetModel);
   } else {
     foreach (Group* g, m_rowTable) {
       if(g != m_argumentHints)
-        needsReset |= changeCompletions(g, changeType);
+        changeCompletions(g, changeType, !resetModel);
     }
     foreach (Group* g, m_emptyGroups) {
       if(g != m_argumentHints)
-        needsReset |= changeCompletions(g, changeType);
+        changeCompletions(g, changeType, !resetModel);
     }
   }
 
   // NOTE: best matches are also updated in resort
   resort();
 
-  kDebug()<<"needsReset"<<needsReset;
-  if(needsReset)
-    reset();
+  if (resetModel) {
+    endResetModel();
+  }
 
   clearExpanding(); //We need to do this, or be aware of expanding-widgets while \
                filtering.
-  emit contentGeometryChanged();
-  kDebug();
 }
 
 QString KateCompletionModel::commonPrefixInternal(const QString &forcePrefix) const
@@ -1024,15 +1016,14 @@ QString KateCompletionModel::commonPrefix(QModelIndex \
selectedIndex) const  return commonPrefix;
 }
 
-bool KateCompletionModel::changeCompletions( Group * g, changeTypes changeType )
+void KateCompletionModel::changeCompletions( Group * g, changeTypes changeType, bool \
notifyModel )  {
-  bool notifyModel = true;
   if(changeType != Narrow) {
-    notifyModel = false;
     g->filtered = g->prefilter;
     //In the "Broaden" or "Change" case, just re-filter everything,
     //and don't notify the model. The model is notified afterwards through a \
reset().  }
+
   //This code determines what of the filtered items still fit, and computes the \
ranges that were removed, giving  //them to beginRemoveRows(..) in batches
   
@@ -1054,14 +1045,13 @@ bool KateCompletionModel::changeCompletions( Group * g, \
changeTypes changeType )  }
   }
   
-  if(deleteUntil != -1) {
+  if(deleteUntil != -1 && notifyModel) {
     beginRemoveRows(indexForGroup(g), 0, deleteUntil);
     endRemoveRows();
   }
   
   g->filtered = newFiltered;
   hideOrShowGroup(g, notifyModel);
-  return !notifyModel;
 }
 
 int KateCompletionModel::Group::orderNumber() const {
@@ -1193,7 +1183,9 @@ void KateCompletionModel::setSortingEnabled( bool enable )
 {
   if (m_sortingEnabled != enable) {
     m_sortingEnabled = enable;
+    beginResetModel();
     resort();
+    endResetModel();
   }
 }
 
@@ -1256,8 +1248,9 @@ const QList< QList < int > > & \
KateCompletionModel::columnMerges( ) const  
 void KateCompletionModel::setColumnMerges( const QList< QList < int > > & \
columnMerges )  {
+  beginResetModel();
   m_columnMerges = columnMerges;
-  reset();
+  endResetModel();
 }
 
 int KateCompletionModel::translateColumn( int sourceColumn ) const
@@ -1557,7 +1550,9 @@ void KateCompletionModel::setSortingAlphabetical( bool \
alphabetical )  {
   if (m_sortingAlphabetical != alphabetical) {
     m_sortingAlphabetical = alphabetical;
+    beginResetModel();
     resort();
+    endResetModel();
   }
 }
 
@@ -1571,11 +1566,13 @@ void KateCompletionModel::setSortingCaseSensitivity( \
Qt::CaseSensitivity cs )  {
   if (m_sortingCaseSensitivity != cs) {
     m_sortingCaseSensitivity = cs;
+    beginResetModel();
     resort();
+    endResetModel();
   }
 }
 
-void KateCompletionModel::resort( )
+void KateCompletionModel::resort()
 {
   foreach (Group* g, m_rowTable)
     g->resort();
@@ -1585,7 +1582,6 @@ void KateCompletionModel::resort( )
 
   // call updateBestMatches here, so they are moved to the top again.
   updateBestMatches();
-  emit contentGeometryChanged();
 }
 
 bool KateCompletionModel::Item::isValid( ) const
@@ -1654,6 +1650,7 @@ void KateCompletionModel::setMaximumInheritanceDepth( int \
maxDepth )  
 void KateCompletionModel::refilter( )
 {
+  beginResetModel();
   m_ungrouped->refilter();
 
   foreach (Group* g, m_rowTable)
@@ -1667,6 +1664,7 @@ void KateCompletionModel::refilter( )
   updateBestMatches();
 
   clearExpanding(); //We need to do this, or be aware of expanding-widgets while \
filtering. +  endResetModel();
 }
 
 void KateCompletionModel::Group::refilter( )
@@ -2048,20 +2046,19 @@ void \
KateCompletionModel::removeCompletionModel(CodeCompletionModel * model)  if (!model \
|| !m_completionModels.contains(model))  return;
 
+  beginResetModel();
   m_currentMatch.remove(model);
 
-  clearGroups(false);
+  clearGroups();
 
   model->disconnect(this);
 
   m_completionModels.removeAll(model);
+  endResetModel();
 
   if (!m_completionModels.isEmpty()) {
     // This performs the reset
     createGroups();
-  }else{
-    emit contentGeometryChanged();
-    reset();
   }
 }
 
@@ -2252,6 +2249,10 @@ void KateCompletionModel::rowSelected(const QModelIndex& row) \
{  
 void KateCompletionModel::clearCompletionModels()
 {
+  if (m_completionModels.isEmpty())
+    return;
+
+  beginResetModel();
   foreach (CodeCompletionModel * model, m_completionModels)
     model->disconnect(this);
 
@@ -2260,6 +2261,7 @@ void KateCompletionModel::clearCompletionModels()
   m_currentMatch.clear();
 
   clearGroups();
+  endResetModel();
 }
 
 #include "katecompletionmodel.moc"
diff --git a/part/completion/katecompletionmodel.h \
b/part/completion/katecompletionmodel.h index 5fedbcc..a10a533 100644
--- a/part/completion/katecompletionmodel.h
+++ b/part/completion/katecompletionmodel.h
@@ -183,7 +183,6 @@ class KATEPART_TESTS_EXPORT KateCompletionModel : public \
ExpandingWidgetModel  void expandIndex(const QModelIndex& index);
     //Emitted whenever something has changed about the group of argument-hints
     void argumentHintsChanged();
-    void contentGeometryChanged();
 
   public Q_SLOTS:
     void setSortingEnabled(bool enable);
@@ -311,6 +310,7 @@ class KATEPART_TESTS_EXPORT KateCompletionModel : public \
ExpandingWidgetModel  
   private:
     QString commonPrefixInternal(const QString &forcePrefix) const;
+    /// @note performs model reset
     void createGroups();
     ///Creates all sub-items of index i, or the item corresponding to index i. \
Returns the affected groups.  ///i must be an index in the source model
@@ -319,8 +319,9 @@ class KATEPART_TESTS_EXPORT KateCompletionModel : public \
ExpandingWidgetModel  ///i must be an index in the source model
     QSet<Group*> deleteItems(const QModelIndex& i);
     Group* createItem(const HierarchicalModelHandler&, const QModelIndex& i, bool \
                notifyModel = false);
-    void clearGroups(bool reset = true);
-    void hideOrShowGroup(Group* g, bool notifyModel = true);
+    /// @note Make sure you're in a {begin,end}ResetModel block when calling this!
+    void clearGroups();
+    void hideOrShowGroup(Group* g, bool notifyModel = false);
     /// When forceGrouping is enabled, all given attributes will be used for \
                grouping, regardless of the completion settings.
     Group* fetchGroup(int attribute, const QString& scope = QString(), bool \
                forceGrouping = false);
     //If this returns nonzero on an index, the index is the header of the returned \
group @@ -336,7 +337,7 @@ class KATEPART_TESTS_EXPORT KateCompletionModel : public \
ExpandingWidgetModel  };
 
     //Returns whether the model needs to be reset
-    bool changeCompletions(Group* g, changeTypes changeType);
+    void changeCompletions(Group* g, changeTypes changeType, bool notifyModel);
 
     bool hasCompletionModel() const;
 
diff --git a/part/completion/katecompletionwidget.cpp \
b/part/completion/katecompletionwidget.cpp index 4bb077e..41d6d7c 100644
--- a/part/completion/katecompletionwidget.cpp
+++ b/part/completion/katecompletionwidget.cpp
@@ -325,7 +325,8 @@ void KateCompletionWidget::startCompletion(const \
KTextEditor::Range& word, const  
   m_lastInvocationType = invocationType;
 
-  disconnect(this->model(), SIGNAL(contentGeometryChanged()), this, \
SLOT(modelContentChanged())); +  disconnect(this->model(), SIGNAL(layoutChanged()), \
this, SLOT(modelContentChanged())); +  disconnect(this->model(), \
SIGNAL(modelReset()), this, SLOT(modelContentChanged()));  
   m_dontShowArgumentHints = true;
 
@@ -412,7 +413,8 @@ void KateCompletionWidget::startCompletion(const \
KTextEditor::Range& word, const  cursorPositionChanged();
 
   if (!m_completionRanges.isEmpty()) {
-    connect(this->model(), SIGNAL(contentGeometryChanged()), this, \
SLOT(modelContentChanged())); +    connect(this->model(), SIGNAL(layoutChanged()), \
this, SLOT(modelContentChanged())); +    connect(this->model(), SIGNAL(modelReset()), \
                this, SLOT(modelContentChanged()));
     //Now that all models have been notified, check whether the widget should be \
displayed instantly  modelContentChanged();
   }


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic