GNOME Bugzilla – Bug 705127
Remove duplicate proxy handling
Last modified: 2013-08-06 19:22:39 UTC
In those 2 commits: commit 8d6732952787ac20e8eefec15b6d645b2d9c3087 Author: Ray Strode <rstrode@redhat.com> Date: Sun Jul 28 22:19:18 2013 -0400 smartcard: add session manager proxy We're going to need to be able to log the user out in some cases when they remove their smartcard. This commit adds the makefile goo and the xml file stolen from gnome-session source tree to get access to the session manager to log out. commit caf75452455fdf3b8e6b8dbdabc9f0da3e77fa7c Author: Ray Strode <rstrode@redhat.com> Date: Sun Jul 28 22:19:18 2013 -0400 smartcard: add screensaver proxy We're going to need to be able to lock the screen in some cases when a user removes their smartcard. This commit adds the goo needed to get access to the lock screen. You add a screensaver proxy which is already used in the power and media-keys plugin, so it would be good to share that code, and a session proxy, which we already export through gnome_settings_session_get_session_proxy().
makes sense to me, will look into it.
I have some untested patches that i'll attach for review. I have to go now, but I'll report back when I've tested them.
Created attachment 250741 [details] [review] common: generate session manager proxy Generating D-Bus proxies is more convienent and more typesafe than using naked GDBusProxy objects. As a first step this commit changes the common session manager proxy to be generated from XML. Since the generated proxy can be compatibly used in any existing GDBusProxy calls, this commit doesn't attempt to rewrite all the plugins to use the generated apis. That can happen in future clean ups.
Created attachment 250742 [details] [review] smartcard: use common session proxy Several existing plugins talk to the session manager, and so there's API for gaining access to a common proxy singleton. This commit drops the smartcard plugin specific way of getting a SessionManager D-Bus proxy in favor of using the common API.
Created attachment 250743 [details] [review] common: rename gnome-settings-session to gnome-settings-bus The gnome-settings-session code is so named because it used to exclusively house login session specific code (ConsoleKit/logind/gnome-session). Since then it's been used more generally as a place to stuff singleton dbus proxies used across multiple plugins. This commit renames it to gnome-settings-bus to give it a more appropriate name for its current role.
Created attachment 250744 [details] [review] common: move screensaver proxy fetching into common code Much like with the session manager, several plugins need to connect to the screen saver (to lock the screen in response to various events). This commit adds a new api: gnome_settings_bus_get_screen_saver_proxy that parallels the existing gnome_settings_bus_get_session_proxy call, and changes all users of their own screen saver proxies to use this shared proxy.
light testing checks out okay. The media-keys plugin still locks the screen if i hit super-L, and the power plugin still locks the screen on lid close if i inhibit suspend using the gnome-session inhibitor api. The proxy still tracks org.gnome.ScreenSaver coming and going across shell restarts. I did see a few whitespace issues and missing g_clear_object on finalize in the last patch of the series that i'll post an update for.
Created attachment 250967 [details] [review] common: move screensaver proxy fetching into common code Much like with the session manager, several plugins need to connect to the screen saver (to lock the screen in response to various events). This commit adds a new api: gnome_settings_bus_get_screen_saver_proxy that parallels the existing gnome_settings_bus_get_session_proxy call, and changes all users of their own screen saver proxies to use this shared proxy.
Review of attachment 250741 [details] [review]: Rest looks fine. ::: gnome-settings-daemon/Makefile.am @@ +20,3 @@ $(NULL) +session_manager_dbus_built_sources = org.gnome.SessionManager.c org.gnome.SessionManager.h Could you name those "glue" or something? I don't really like this naming scheme for files. ::: plugins/power/gsdpowerenums.py @@ -1,1 @@ -GSM_INHIBITOR_FLAG_LOGOUT = 1; I don't think you wanted to remove that file, did you?
Review of attachment 250742 [details] [review]: Yep.
Review of attachment 250743 [details] [review]: ++
Review of attachment 250967 [details] [review]: Negative line count, love it :)
Attachment 250741 [details] pushed as 541cf96 - common: generate session manager proxy Attachment 250742 [details] pushed as bed7ce8 - smartcard: use common session proxy Attachment 250743 [details] pushed as 2cabb3a - common: rename gnome-settings-session to gnome-settings-bus Attachment 250967 [details] pushed as 4a0fabd - common: move screensaver proxy fetching into common code