D20024: Fixes crash when hiding devices

[Elvis Angelaccio]

elvisangelaccio added a comment.


  In D20024#440855 <https://phabricator.kde.org/D20024#440855>, @hallas wrote:
  
  > Could you try and move the `testHideItem` to be the first test? Also make sure to run `placesitemmodeltest` using valgrind, i.e. `valgrind placesitemmodeltest`
  
  
  Right, I was able to reproduce the stacktrace using valgrind.

INLINE COMMENTS

> kstandarditem.cpp:117-118
>          emit m_model->itemsChanged(KItemRangeList() << KItemRange(index, 1), changedRoles);
> +        // This must be last as PlacesItemModel will in some cases delete us in this function
> +        m_model->onItemChanged(index, changedRoles);
>      }

While this change fixes the crash, I consider it a workaround as it's relying on an implementation detail of the model.

It doesn't fix the actual problem, which I think is in `PlacesItemModel::onSourceModelDataChanged()`. Can you try to delay the `removeItem()` call (at line 569)  using a `QTimer::singleShot()` ?

Basically, we should do what dolphin used to do with the old PlacesItemModel implementation:

  void PlacesItemModel::onItemChanged(int index, const QSet<QByteArray>& changedRoles)
  {
      ...
  
      if (changedRoles.contains("isHidden")) {
          if (!m_hiddenItemsShown && changedItem->isHidden()) {
              m_hiddenItemToRemove = index;
              QTimer::singleShot(0, this, static_cast<void (PlacesItemModel::*)()>(&PlacesItemModel::hideItem));
          }
      }
  }

(commit da6f8fe0862585287 <https://phabricator.kde.org/R318:da6f8fe0862585287153f0d90e19eab0b34bfbef> dropped the QTimer and probably introduced this crash).

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D20024

To: hallas, #dolphin, elvisangelaccio
Cc: ngraham, kfm-devel, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190331/28ca9188/attachment.htm>