[Folder View] Create KFilePlacesModel only when needed and listen for changes

Authored by broulik on Jul 27 2018, 12:32 PM.



When no label is displayed, there's no point in creating a KFilePlacesModel.
Also, only emit a change for displayLabel when it actually changed.
Moreover, listen to changes in the KFilePlacesModel and update the label if needed.
Also, make the KFilePlacesModel static and shared between all Folder View instances.

Test Plan
  • My test partition still has its proper "Device" name (devices are populated deferred, so if we didn't listen for changes, creating a model and asking for closestItem right away would return Root (/media/foo is closed to /)
  • Reduces the number of displayLabelChanged emissions to half on startup
  • Added a Pictures folder view, renamed my Pictures place in Dolphin, label updated immediately
  • Verified in GammaRay there's no KFilePlacesModel instance in plasmashell unless having a Folder View widget on desktop or panel (but not in the default containment case)

Diff Detail

R119 Plasma Desktop
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.
broulik created this revision.Jul 27 2018, 12:32 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 27 2018, 12:32 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
broulik requested review of this revision.Jul 27 2018, 12:32 PM
hein added a comment.Jul 30 2018, 7:24 AM

Can we make the KFilePlaces model static on top of this? It could be shared by all FVs that need it. Look at how tasksmodel.cpp keeps a refcount of when ActivityInfo is in use and deletes it when it drops to 0 for example ...

broulik planned changes to this revision.Jul 30 2018, 7:36 AM

Good idea, will do

broulik updated this revision to Diff 41318.Sep 10 2018, 8:08 AM
broulik edited the summary of this revision. (Show Details)
  • Make KFilePlacesModel static and ref-counted

For simplicify the ref count is increased/decreased in the constructor and not depending on whether the model is actually used, e.g. when user changes setting from default display label to something else. This would make the patch super complicated otherwise. The model is then only not loaded again on next startup.

hein added a comment.Sep 10 2018, 10:37 AM

Did you forget to update the diff?

broulik updated this revision to Diff 41331.Sep 10 2018, 10:56 AM

Indeed I did :)

hein accepted this revision.Sep 12 2018, 3:24 PM
This revision is now accepted and ready to land.Sep 12 2018, 3:24 PM
This revision was automatically updated to reflect the committed changes.