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

[Frank Reininghaus]

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

(Updated May 21, 2014, 4:25 p.m.)


Review request for Dolphin.


Changes
-------

Thanks for your feedback, which was very useful indeed! I agree that the purpose of the +0.5 thing was not obvious. I've replaced this approach with qRound to make sure that the percentage is rounded to the nearest integer, and not just truncated.

About the private constructors and the friends: these should prevent that instances of MountPointObserver and MountPointObserverCache are created manually (see my replies to the inline comments).

I'll push this patch in the next few days if there are no objections or further ideas for improvement.


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 (updated)
-----

  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/0d5343a7/attachment.htm>