From kwrite-devel Tue Dec 31 16:00:41 2013 From: "Kevin Funk" Date: Tue, 31 Dec 2013 16:00:41 +0000 To: kwrite-devel Subject: Re: Review Request 113917: Fix assert in ~QPersistentModelIndex Message-Id: <20131231160041.12418.76171 () probe ! kde ! org> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=138850566124902 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============8480730899848460454==" --===============8480730899848460454== Content-Type: multipart/alternative; boundary="===============6511544570170620811==" --===============6511544570170620811== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit > On Dec. 31, 2013, 3:58 p.m., Commit Hook wrote: > > This review has been submitted with commit b768749230872b5acf0e2816dd855409f6d70733 by Kevin Funk to branch master. Re. applying this in frameworks branch: I suggest reverting the change in katecompletionmodel.cpp as of e6374518f5a966b8badda3a5e90fb873cd7ec08e and then merge master back into frameworks. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113917/#review46509 ----------------------------------------------------------- On Dec. 31, 2013, 3:58 p.m., Kevin Funk wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/113917/ > ----------------------------------------------------------- > > (Updated Dec. 31, 2013, 3:58 p.m.) > > > Review request for Kate and Milian Wolff. > > > Bugs: 236948 > http://bugs.kde.org/show_bug.cgi?id=236948 > > > Repository: kate > > > Description > ------- > > 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 as soon as 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 > FIXED-IN: 4.13 > > > Diffs > ----- > > part/completion/katecompletionwidget.cpp 4bb077e0973fd1eb9f1f5a7bb3e9bccae1a79511 > part/completion/katecompletionmodel.cpp 3fa8080ff95deb1e24327063ccd19dbb13ff49a8 > part/completion/katecompletionmodel.h 5fedbccc37cd49b7403827dafce15b498e99b160 > > Diff: https://git.reviewboard.kde.org/r/113917/diff/ > > > Testing > ------- > > > Thanks, > > Kevin Funk > > --===============6511544570170620811== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113917/

On December 31st, 2013, 3:58 p.m. UTC, Commit Hook wrote:

This review has been submitted with commit b768749230872b5acf0e2816dd855409f6d70733 by Kevin Funk to branch master.
Re. applying this in frameworks branch:

I suggest reverting the change in katecompletionmodel.cpp as of e6374518f5a966b8badda3a5e90fb873cd7ec08e and then merge master back into frameworks.

- Kevin


On December 31st, 2013, 3:58 p.m. UTC, Kevin Funk wrote:

Review request for Kate and Milian Wolff.
By Kevin Funk.

Updated Dec. 31, 2013, 3:58 p.m.

Bugs: 236948
Repository: kate

Description 

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 as soon as 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
FIXED-IN: 4.13

Diffs

  • part/completion/katecompletionwidget.cpp (4bb077e0973fd1eb9f1f5a7bb3e9bccae1a79511)
  • part/completion/katecompletionmodel.cpp (3fa8080ff95deb1e24327063ccd19dbb13ff49a8)
  • part/completion/katecompletionmodel.h (5fedbccc37cd49b7403827dafce15b498e99b160)

View Diff
--===============6511544570170620811==-- --===============8480730899848460454== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ KWrite-Devel mailing list KWrite-Devel@kde.org https://mail.kde.org/mailman/listinfo/kwrite-devel --===============8480730899848460454==--