D16857: Do not disconnect all StorageAccess signals when unmounting

[Thomas Surrel]

thsurrel created this revision.
thsurrel added reviewers: Dolphin, VDG.
Herald added a project: Dolphin.
Herald added a subscriber: kfm-devel.
thsurrel requested review of this revision.

REVISION SUMMARY
When unmounting a local device from the Places panel, the item
was disconnected from all signals from Solid::StorageAccess.
That resulted in a weird situation where trying again to access
the device was actually mounting it in Solid, but dolphin just
behaved as if nothing at all was happening.

BUG: 400992

TEST PLAN
In Dolphin, mount a local partition (e.g. a Windows partition)
then unmount it (right click on it in Places, then Unmount).
Try to access it again by clicking on it in Places, we should
get access to it correctly.

REPOSITORY
R318 Dolphin

BRANCH
arc_remount_local (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D16857

AFFECTED FILES
src/panels/places/placesitemmodel.cpp

To: thsurrel, #dolphin, #vdg
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Nathaniel Graham]

ngraham edited reviewers, added: Frameworks, bruns; removed: VDG.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D16857

To: thsurrel, #dolphin, #frameworks, bruns, #vdg
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Stefan Brüns]

bruns added a comment.


I think this is correct, but the summary can be improved a little bit.

The problem is caused by the fact device interfaces returned by Solid (e.g. `item->device().as<Solid::StorageAccess>()`) are not full objects, but only references/pointers to a per-device-object, i.e. requesting the same interface for a device will return the same address every time.

If the interface is used used in multiple places, calling disconnect on the interface address disconnects the signals for all users.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D16857

To: thsurrel, #dolphin, #frameworks, bruns
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Thomas Surrel]

thsurrel edited the summary of this revision.

REPOSITORY
R318 Dolphin

REVISION DETAIL
https://phabricator.kde.org/D16857

To: thsurrel, #dolphin, #frameworks, bruns
Cc: kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Elvis Angelaccio]

elvisangelaccio accepted this revision.
elvisangelaccio added a comment.
This revision is now accepted and ready to land.


Thanks, this fixes the bug for me.

I think we can ship it on the stable branch (`Applications/18.12`)

REPOSITORY
R318 Dolphin

BRANCH
arc_remount_local (branched from master)

REVISION DETAIL
https://phabricator.kde.org/D16857

To: thsurrel, #dolphin, #frameworks, bruns, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov

[Thomas Surrel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R318:e710a6431160: Do not disconnect all StorageAccess signals when unmounting (authored by thsurrel).

REPOSITORY
R318 Dolphin

CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16857?vs=45400&id=45665

REVISION DETAIL
https://phabricator.kde.org/D16857

AFFECTED FILES
src/panels/places/placesitemmodel.cpp

To: thsurrel, #dolphin, #frameworks, bruns, elvisangelaccio
Cc: elvisangelaccio, kfm-devel, alexde, sourabhboss, feverfew, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov