Review Request 128398: Fix KDescendantsProxyModel::setSourceModel(...) to reset internal data

[David Faure]

> On July 21, 2016, 5:43 a.m., Sune Vuorela wrote:
> > I tried writing a small test for it, and it seemed like I could reproduce the issue. But my test didn't pass when I added this changeset.
> > 
> > Maybe I should publish my test case somewhere...
> 
> Sune Vuorela wrote:
>     https://git.reviewboard.kde.org/r/128513/ - published there

https://git.reviewboard.kde.org/r/128515/ is a modified version of this patch, which actually fixes the unittest.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128398/#review97685
-----------------------------------------------------------


On July 8, 2016, 2:58 a.m., Friedrich W. H. Kossebau wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128398/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 2:58 a.m.)
> 
> 
> Review request for KDE Frameworks, Stephen  Kelly and Stephen Kelly.
> 
> 
> Repository: kitemmodels
> 
> 
> Description
> -------
> 
> KDescendantsProxyModel currently does not reset its internal data when a (new) source model is set.
> 
> Not sure the provided patch is the most correct one, but it works with the current unit tests and for the use case where this bug was hit.
> I am still confused why `KDescendantsProxyModelPrivate::synchronousMappingRefresh()` loops over `while (!m_pendingParents.isEmpty())` on calling `processPendingParents();` while `KDescendantsProxyModelPrivate::scheduleProcessPendingParents()` does not.
> Especially when the `KDescendantsProxyModelPrivate::sourceModelReset()` handler also only calls the latter.
> The sourceModelReset handler should be surely similar to what is done on setting a new source model, so the patch for now copies that code. But from what I understood by reading the code both of them should rather do a full loop perhaps?
> 
> And `m_relayouting` should get a better name now, but no idea yet what would be nice.
> 
> I have yet to grasp the proxymodeltest system to also write a matching unit test, any proposal where I should start?
> 
> 
> Diffs
> -----
> 
>   src/kdescendantsproxymodel.cpp 477cd96 
> 
> Diff: https://git.reviewboard.kde.org/r/128398/diff/
> 
> 
> Testing
> -------
> 
> Existing kitemmodels unit tests still pass.
> 
> 
> Thanks,
> 
> Friedrich W. H. Kossebau
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160724/e3baa9fb/attachment-0001.html>