Code review comment for lp:~chihchun/unity-scopes-shell/lp1535377

Revision history for this message
Paweł Stołowski (stolowski) wrote :

Thanks for looking into this. As discussed on IRC though, I think that while your fix likely avoids the crash, it probably hides the real problem at the same time, plus I think that getting rid of "row + (row > oldPos ? 1 : 0)" in the beginMoveRows notification for the qml view will cause subtle issues.

It's expected and absolutely fine that the size of new results list is different than current list; as the new results list is iterated in the 0..results.count() loop, new (previously unseen) results are inserted into m_lists (so it grows) and it's guaranteed that any moves happen only in the 0..row range, which is valid for m_results list. That's the theory at least, I cannot exclude a bug in this implementation.

I'm currently working on a branch which has a bunch of stress tests for model updates (it wasn't part of the existing code, because I started working on them just recently for some other changes in that area) and am trying to come up with a case that would fail with existing implementation, giving me the opportunity to better understand what happens.

review: Abstain

« Back to merge proposal