Review Request 118208: Keep the "free space" information updated in all views

[Emmanuel Pescosta]

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



dolphin/src/statusbar/mountpointobserver.h
<https://git.reviewboard.kde.org/r/118208/#comment40487>

    Maybe add a description for your "delayed delete" approach here and maybe add a hint that this class is not thread safe. (problem with unique application?)
    
    Esp. that the object will be destroyed when ref. count is zero and update is called. And remove the delete on update() description from deref() of course. This is a class-wide detail.
    
    Makes it easier to understand at first sight ;)



dolphin/src/statusbar/mountpointobserver.h
<https://git.reviewboard.kde.org/r/118208/#comment40488>

    "public:" before first class method



dolphin/src/statusbar/mountpointobserver.h
<https://git.reviewboard.kde.org/r/118208/#comment40490>

    assert(m_referenceCount >= 0);



dolphin/src/statusbar/mountpointobservercache.h
<https://git.reviewboard.kde.org/r/118208/#comment40491>

    I think QDebug can be removed as far as I see.



dolphin/src/statusbar/mountpointobservercache.h
<https://git.reviewboard.kde.org/r/118208/#comment40489>

    "public:" before first class method



dolphin/src/statusbar/spaceinfoobserver.h
<https://git.reviewboard.kde.org/r/118208/#comment40492>

    Remove empty line



dolphin/src/statusbar/statusbarspaceinfo.cpp
<https://git.reviewboard.kde.org/r/118208/#comment40493>

    QScopedPointer and reset() maybe?



dolphin/src/statusbar/statusbarspaceinfo.cpp
<https://git.reviewboard.kde.org/r/118208/#comment40495>

    Add assert(m_observer)



dolphin/src/statusbar/statusbarspaceinfo.cpp
<https://git.reviewboard.kde.org/r/118208/#comment40494>

    use size here instead of m_observer->size()


- Emmanuel Pescosta


On May 19, 2014, 7:13 p.m., Frank Reininghaus wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118208/
> -----------------------------------------------------------
> 
> (Updated May 19, 2014, 7:13 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 327708
>     http://bugs.kde.org/show_bug.cgi?id=327708
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Right now, the "free space" part of the status bar stops working properly as soon as it becomes invisible once. This also happens if the view is split, because the first view becomes invisible for a brief moment.
> 
> The easiest solution would be to re-start the periodic polling (in 10 second intervals) when it becomes visible again. The downside is that each view would call the function that queries the free space once, and the 10-second timers of all views would not necessarily be in sync, which might waste energy, because the system might be woken up more frequently than necessary.
> 
> I've tried to solve this problem, and focused on these things:
> 
> 1. The free space should only be queried once every 10 seconds for each mount point,
> 2. use only one timer to query all mount points,
> 3. stop querying a mount point if no view showing a directory that belongs to it is visible any more,
> 4. do not query the free space repeatedly when navigating between different directories on the same mount point (the current code does this).
> 
> Motivated by Emmanuel's ideas at https://git.reviewboard.kde.org/r/116047/, I've added three new classes:
> 
> * SpaceInfoObserver factors out some code from StatusBarSpaceInfo that is used to watch a certain URL.
> 
> * MountPointObserver watches one mount point, and can be shared by several SpaceInfoObservers. Reference counting is used to ensure that it is discarded if no SpaceInfoObserver uses it any more. I'll explain below why I implemented the reference counting manually, rather than using one of Qt's shared data classes for it.
> 
> * MountPointObserverCache keeps all currently active MountPointObservers, such that existing instances can be reused and shared. It creates a new instance if needed and connects a global 10-second timer's timeout() signal to the MountPointObserver's update() slot.
> 
> 
> MountPointObserver::update() will check if the reference count has reached zero, and delete itself in that case. The reason why it does not delete itself when the count reaches zero is this: If the URL of the view is changed, it is quite likely that the mount point will not change, i.e., the same MountPointObserver can be reused.
> 
> However, if it was deleted immediately when leaving the first URL, then it would not be available any more for the second URL. My "delayed delete" approach ensures that this does not happen, and quering the free space after each URL change is not needed.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 3f58479 
>   dolphin/src/statusbar/mountpointobserver.h PRE-CREATION 
>   dolphin/src/statusbar/mountpointobserver.cpp PRE-CREATION 
>   dolphin/src/statusbar/mountpointobservercache.h PRE-CREATION 
>   dolphin/src/statusbar/mountpointobservercache.cpp PRE-CREATION 
>   dolphin/src/statusbar/spaceinfoobserver.h PRE-CREATION 
>   dolphin/src/statusbar/spaceinfoobserver.cpp PRE-CREATION 
>   dolphin/src/statusbar/statusbarspaceinfo.h 1849462 
>   dolphin/src/statusbar/statusbarspaceinfo.cpp 61b2833 
> 
> Diff: https://git.reviewboard.kde.org/r/118208/diff/
> 
> 
> Testing
> -------
> 
> The "free space" part of the status bar is always up-to-date now, even if split views and tabs are used, and also after switching between different virtual desktops.
> 
> 
> Thanks,
> 
> Frank Reininghaus
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140521/0950100c/attachment.htm>