GNOME Bugzilla – Bug 658568
brightness vs active vt
Last modified: 2011-09-13 14:02:20 UTC
Here is the issue: I am on the login screen Things are nice and bright I switch to a vt Things are still bright...for a second Then the screen gets dimmed And stays dim until I switch back to the login screen I think I've seen a similar issue with fast user switching too.
Indeed, this makes fast user switching pretty much unusable
*** Bug 658187 has been marked as a duplicate of this bug. ***
Created attachment 196238 [details] [review] Shared functionality
Created attachment 196239 [details] [review] Make FUS work with the color plugin
Created attachment 196240 [details] [review] Make FUS work with the power plugin
Review of attachment 196238 [details] [review]: ::: gnome-settings-daemon/gnome-settings-session.c @@ +80,3 @@ + if (session->priv->proxy_session == NULL) { + g_warning ("no ConsoleKit session"); + goto out; return FALSE; @@ +89,3 @@ + -1, NULL, &error_local); + if (result == NULL) { + g_set_error (error, 1, 0, "IsLocal failed: %s", error_local->message); proper error quarks please. @@ +91,3 @@ + g_set_error (error, 1, 0, "IsLocal failed: %s", error_local->message); + g_error_free (error_local); + goto out; return FALSE; @@ +98,3 @@ + ret = TRUE; +out: + if (result != NULL) And remove that if. @@ +109,3 @@ + **/ +gboolean +gnome_settings_session_is_active (GnomeSettingsSession *session, Looks like the _only_ difference between this function and the above is the "IsActive" string. @@ +243,3 @@ + g_signal_connect (session->priv->proxy_session, "g-signal", + G_CALLBACK (gnome_settings_session_proxy_signal_cb), session); +out: Seriously. @@ +276,3 @@ + **/ +GnomeSettingsSession * +gnome_settings_session_new (void) Rename this, this isn't a new instance. "new_singleton" or something. ::: gnome-settings-daemon/gnome-settings-session.h @@ +41,3 @@ +{ + GObject parent; + GnomeSettingsSessionPrivate *priv; You can make this really private.
Review of attachment 196239 [details] [review]: ::: plugins/color/gsd-color-manager.c @@ +2011,3 @@ + priv->session = gnome_settings_session_new (); + g_signal_connect (priv->session, "active-changed", + G_CALLBACK (gcm_session_active_changed_cb), Not checking whether the session is active here?
Review of attachment 196240 [details] [review]: ::: plugins/power/gsd-power-manager.c @@ +2613,3 @@ + /* ensure we're still on an active console */ + ret = gnome_settings_session_is_active (manager->priv->session, You should cache this instead. @@ +3264,3 @@ + /* track the active session */ + manager->priv->session = gnome_settings_session_new (); + g_signal_connect (manager->priv->session, "active-changed", Again, check whether you're active here.
Created attachment 196274 [details] [review] Shared functionality Something more like this? Thanks.
Review of attachment 196274 [details] [review]: IsLocal not interesting anymore? ::: gnome-settings-daemon/gnome-settings-session.c @@ +118,3 @@ + NULL, + 0, + G_MAXUINT, It's an enum, not a uint, please do this properly. @@ +222,3 @@ + GnomeSettingsSession *session = GNOME_SETTINGS_SESSION (user_data); + + session->priv->proxy_manager = g_dbus_proxy_new_for_bus_finish (res, &error); You only use the proxy_manager here. Unref it after the g_dbus_proxy_call(), and you don't need to reference it, as long as you use the same cancellable. @@ +274,3 @@ + g_source_remove (session->priv->proxy_session_id); + if (session->priv->proxy_manager != NULL) + g_object_unref (session->priv->proxy_manager); If you finalize the object that the signal is coming from, you don't need to disconnect the signal manually.
Created attachment 196340 [details] [review] Shared functionality (In reply to comment #11) > Review of attachment 196274 [details] [review]: > > IsLocal not interesting anymore? In this context, no. I don't think plugins need to know when a session is local or not in this context. It might be important to PolicyKit, but i can't think of a situation where a plugin would care. > It's an enum, not a uint, please do this properly. Fixed. > @@ +222,3 @@ > + GnomeSettingsSession *session = GNOME_SETTINGS_SESSION (user_data); > + > + session->priv->proxy_manager = g_dbus_proxy_new_for_bus_finish (res, > &error); > > You only use the proxy_manager here. Unref it after the g_dbus_proxy_call(), > and you don't need to reference it, as long as you use the same cancellable. Agreed, fixed. > @@ +274,3 @@ > + g_source_remove (session->priv->proxy_session_id); > + if (session->priv->proxy_manager != NULL) > + g_object_unref (session->priv->proxy_manager); > > If you finalize the object that the signal is coming from, you don't need to > disconnect the signal manually. Fixed. Richard.
Review of attachment 196340 [details] [review]: ::: gnome-settings-daemon/gnome-settings-session.c @@ +156,3 @@ + return; + } + g_variant_get (result, "(b)", active); g_variant_get_boolean? @@ +159,3 @@ + session->priv->state = active ? GNOME_SETTINGS_SESSION_STATE_ACTIVE : + GNOME_SETTINGS_SESSION_STATE_INACTIVE; + g_object_notify (G_OBJECT (session), "state"); Share that code with gnome_settings_session_proxy_signal_cb()? gnome_settings_session_set_state ( *session, *variant) { }
Thanks for the super-quick review: commit ebd3bcf01333ac6fac21aea5cd564759cd6eafa0 Author: Richard Hughes <richard@hughsie.com> Date: Mon Sep 12 10:49:29 2011 +0100 power: Do not handle the idle state transaction when the session is not active Resolves https://bugzilla.gnome.org/show_bug.cgi?id=658568 :100644 100644 1192be7... 1d1e6ef... M plugins/power/gsd-power-manager.c commit 38ae5af292dd1d2665e74aef180fa984616745d8 Author: Richard Hughes <richard@hughsie.com> Date: Mon Sep 12 10:38:36 2011 +0100 color: Use the correct profiles when fast user switching :100644 100644 967d1c0... 14a4576... M plugins/color/gsd-color-manager.c commit 851b0cf4dd9ac098b053aac1360c157191299780 Author: Richard Hughes <richard@hughsie.com> Date: Mon Sep 12 10:30:58 2011 +0100 Add functionality shared between plugins to detect if the current session is active :100644 100644 69a614c... 2c72233... M gnome-settings-daemon/Makefile.am :000000 100644 0000000... 2302304... A gnome-settings-daemon/gnome-settings-session.c :000000 100644 0000000... 874318b... A gnome-settings-daemon/gnome-settings-session.h