[prev in list] [next in list] [prev in thread] [next in thread]
List: kwrite-devel
Subject: Re: [kate] part/completion: Fix assert in ~QPersistentModelIndex
From: Christoph Cullmann <cullmann () absint ! com>
Date: 2013-12-31 16:23:46
Message-ID: 836872343.804462.1388507026611.JavaMail.root () absint ! com
[Download RAW message or body]
Hi,
could you merge that in frameworks branch, too?
Get some conflicts and wont mess up your change ;)
----- Ursprüngliche Mail -----
> 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();
> }
>
>
--
----------------------------- Dr.-Ing. Christoph Cullmann ---------
AbsInt Angewandte Informatik GmbH Email: cullmann@AbsInt.com
Science Park 1 Tel: +49-681-38360-22
66123 Saarbrücken Fax: +49-681-38360-20
GERMANY WWW: http://www.AbsInt.com
--------------------------------------------------------------------
Geschäftsführung: Dr.-Ing. Christian Ferdinand
Eingetragen im Handelsregister des Amtsgerichts Saarbrücken, HRB 11234
_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic