interface/screen_inhibit_control: Improve screen inhibit control for use on core #14134
Conversation
er-vin
commented
Jun 26, 2024
er-vin
commented
- Make it available on Ubuntu Core
- Receive notifications for inhibit changes, plugged application could change the inhibit status but had no way to be notified when it actually changes.
ernestl
added
the
Needs security review
bboozzoo
changed the title
er-vin
force-pushed
the
improve-screen-inhibit-control-for-use-on-core
branch
from
bd22eba
to
b6b55ef
Compare
er-vin
force-pushed
the
improve-screen-inhibit-control-for-use-on-core
branch
from
b6b55ef
to
88e6436
Compare
ernestl
removed
the
Needs security review
There was a problem hiding this comment.
The addition of access to the HasInhibitChanged signal looks fine (it is included in this copy of the spec). Making the interface implicit on core seems suspect.
Checking my KDE Core Desktop VM, org.freedesktop.PowerManagement on the session bus is owned by org_kde_powerdevil running with the AppArmor label snap.plasma-desktop-session.plasma-powerdevil. Similarly org.freedesktop.ScreenSaver is owned by a process with the label snap.plasma-desktop-session.plasma-kwin-wayland. So allowing communication with label=unconfined does not sound like it would actually enable any communication here.
For proper operation on Core, the interface would need to be modified to allow a snap to provide the slot.
|
Changed milestone to 2.67, needs more time. |
er-vin
force-pushed
the
improve-screen-inhibit-control-for-use-on-core
branch
from
88e6436
to
98d006b
Compare
It has been reworked in this direction. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14134 +/- ##
==========================================
+ Coverage 78.95% 79.01% +0.06%
==========================================
Files 1084 1085 +1
Lines 146638 147532 +894
==========================================
+ Hits 115773 116579 +806
- Misses 23667 23726 +59
- Partials 7198 7227 +29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
er-vin
force-pushed
the
improve-screen-inhibit-control-for-use-on-core
branch
from
dc77bc8
to
db79993
Compare
|
Just did a rebase on latest master to ease the re-review. |
This can be provided by a session snap in such a case
Plugged application could change the inhibit status but had no way to be notified when it actually changes.
er-vin
force-pushed
the
improve-screen-inhibit-control-for-use-on-core
branch
from
db79993
to
2838a0b
Compare
|
@Meulengracht static checks are fixed now, should be good to merge now. |
* master: (44 commits) wrappers: do not reload activation units (canonical#14724) gadget/install: support for no{exec,dev,suid} mount flags interfaces/builtin/mount_control: add support for nfs mounts (canonical#14694) tests: use gojq - part 1 (canonical#14686) interfaces/desktop-legacy: allow DBus access to com.canonical.dbusmenu interfaces/builtin/fwupd.go: allow access to nvmem for thunderbolt plugin tests: removing uc16 executions (canonical#14575) tests: Added arm github runner to build snapd (canonical#14504) tests: no need to run spread when there are not tests matching the filter (canonical#14728) tests/lib/tools/store-state: exit on errors, update relevant tests (canonical#14725) tests: udpate the github workflow to run tests suggested by spread-filter tool (canonical#14519) testtime: add mockable timers for use in tests (canonical#14672) interface/screen_inhibit_control: Improve screen inhibit control for use on core (canonical#14134) tests: use images with 20G disk in openstack (canonical#14720) i/builtin: allow @ in custom-device filepaths (canonical#14651) tests: refactor test-snapd-desktop-layout-with-content tests: fix broken app definition tests: capitalize sentences in comments tests: wrap very long shell line tests: fix raciness in async startup and sync install ...
