After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 764896 - Don't claim the light sensor if we're not active
Don't claim the light sensor if we're not active
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: power
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2016-04-11 14:03 UTC by Bastien Nocera
Modified: 2016-04-13 12:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
power: Don't claim the light sensor if we're not active (1.97 KB, patch)
2016-04-11 14:03 UTC, Bastien Nocera
none Details | Review
power: Fix typo in warning message (975 bytes, patch)
2016-04-11 14:03 UTC, Bastien Nocera
committed Details | Review
power: Don't claim the light sensor if we're not active (2.78 KB, patch)
2016-04-13 10:43 UTC, Bastien Nocera
none Details | Review
power: Don't claim the light sensor if we're not active (3.63 KB, patch)
2016-04-13 12:13 UTC, Bastien Nocera
none Details | Review
power: Don't claim the light sensor if we're not active (3.63 KB, patch)
2016-04-13 12:24 UTC, Bastien Nocera
committed Details | Review

Description Bastien Nocera 2016-04-11 14:03:47 UTC
.
Comment 1 Bastien Nocera 2016-04-11 14:03:51 UTC
Created attachment 325728 [details] [review]
power: Don't claim the light sensor if we're not active

Try not to claim the light sensor when our seat isn't active,
as this will just throw PolicyKit errors about not being authorised,
as happens with gdm trying to change the backlight when we're
logged in.
Comment 2 Bastien Nocera 2016-04-11 14:03:57 UTC
Created attachment 325729 [details] [review]
power: Fix typo in warning message
Comment 3 Rui Matos 2016-04-11 16:31:10 UTC
Review of attachment 325728 [details] [review]:

We should probably ensure that we read the active property on startup
Comment 4 Bastien Nocera 2016-04-13 09:53:15 UTC
Comment on attachment 325729 [details] [review]
power: Fix typo in warning message

Attachment 325729 [details] pushed as a9621e6 - power: Fix typo in warning message
Comment 5 Bastien Nocera 2016-04-13 10:43:42 UTC
Created attachment 325854 [details] [review]
power: Don't claim the light sensor if we're not active

Try not to claim the light sensor when our seat isn't active,
as this will just throw PolicyKit errors about not being authorised,
as happens with gdm trying to change the backlight when we're
logged in.
Comment 6 Rui Matos 2016-04-13 12:05:39 UTC
Review of attachment 325854 [details] [review]:

::: plugins/power/gsd-power-manager.c
@@ +142,3 @@
         gboolean                 lid_is_present;
         gboolean                 lid_is_closed;
+        gboolean                 session_is_active;

It seems like we already have an is_session_active() in this file which checks the dbus proxy directly. I think just re-using that should be good enough instead of keeping track of the variable again here. Or we could replace that function and its callers to just use this variable since you're updating it here when the dbus property changes.
Comment 7 Bastien Nocera 2016-04-13 12:13:51 UTC
Created attachment 325857 [details] [review]
power: Don't claim the light sensor if we're not active

Try not to claim the light sensor when our seat isn't active,
as this will just throw PolicyKit errors about not being authorised,
as happens with gdm trying to change the backlight when we're
logged in.

Also remove the calls to is_session_active() and use the cached variable
when possible.
Comment 8 Rui Matos 2016-04-13 12:19:23 UTC
Review of attachment 325857 [details] [review]:

looks good otherwise

::: plugins/power/gsd-power-manager.c
@@ +1668,2 @@
         /* are we inhibited from going idle */
+        if (manager->priv->session_is_active || is_idle_inhibited) {

this should be !m->p->session_is_active
Comment 9 Bastien Nocera 2016-04-13 12:24:36 UTC
Created attachment 325858 [details] [review]
power: Don't claim the light sensor if we're not active

Try not to claim the light sensor when our seat isn't active,
as this will just throw PolicyKit errors about not being authorised,
as happens with gdm trying to change the backlight when we're
logged in.

Also remove the calls to is_session_active() and use the cached variable
when possible.
Comment 10 Bastien Nocera 2016-04-13 12:25:03 UTC
Attachment 325858 [details] pushed as 76dab07 - power: Don't claim the light sensor if we're not active