Review Request 115326: fix theme cache cleanup and discarding

[Aaron J. Seigo]

> On Jan. 29, 2014, 1:50 p.m., Aaron J. Seigo wrote:
> > this patch doesn't actually improve anything. in fact, the actual bug is left:
> > 
> >  pixmapCache = new KImageCache(pixmapCacheFileName, config.themeCacheKb() * 1024);
> > 
> > pixmapCacheFileName should have the version # in it. this was an oversight on my part when fixing br320855 in 59e88c777b92935cc95f28e52f84fe97563dec9d
> > 
> > this patch should not go in as is, but instead have the cache file name ammended to include the version information.
> 
> Harald Sitter wrote:
>     the point of this patch is not to fix cache versioning but cache cleaning such that http://i.imgur.com/v0Zyees.png does not happen. further more as mentioned in the description, fixing the cache versioning goes beyond the KImageCache construction but also requires the entire logic to be created for the svgelements cache. to that extent this patch is nothing but a hotfix and a step towards correct cache handling, not a complete fix.
> 
> Aaron J. Seigo wrote:
>     "fixing the cache versioning goes beyond the KImageCache construction but also requires the entire logic to be created for the svgelements cache."
>     
>     right; so what should be happening is that the image cache and the elements cache should both be created with the version number in their name. that keeps them in sync with each other and with the current contents of the theme.
>     
>     the cache file is indeed being created w/out the version # in it, which is a regression introduced in 59e88c777b92935cc95f28e52f84fe97563dec9d. putting the version # back in the cache file name should be sufficient.
>     
>     then in discardCache the version string used in the image cache filename should also be used to initialize svgElementsCache.
>     
>     probably the easiest way to manage this is to add a member to the Private class for the version # string..

something like this: http://pastebin.kde.org/p6czlim0q


- Aaron J.


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


On Jan. 28, 2014, 9:18 a.m., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115326/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2014, 9:18 a.m.)
> 
> 
> Review request for Plasma, Aaron J. Seigo and Martin Klapetek.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> - introduce svgelements cache handling in useCache()
> - to decide whether or not to discard a cache type now the mtimes of metadata.desktop and the respective cache file are compared (previously this was an invalid file compared to cache change time)
> - cache cleanup now happens entirely before creation of the KImageCache instance to prevent any sort of problem there
> 
> - add fixme comment about broken version-based cache handling. for one the version specific file name is not actually used for the KImageCache and for another the entire shebang is missing from svgelements all together. this has the very unfortunate side effect that the *active* cache files are deleted ever so often, it may be worth removing the code portions until someone finds time/interest to actually implement this in a working fashion.
> 
> 
> Diffs
> -----
> 
>   plasma/theme.cpp cb44878 
> 
> Diff: https://git.reviewboard.kde.org/r/115326/diff/
> 
> 
> Testing
> -------
> 
> upgraded from 4.8 straight to 4.12 -> theme caches correctly discarded
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140129/6e24375f/attachment.html>