GNOME.org
 

[network-manager-applet] applet: fix up the shell-version-detecting code

commit 0c997a95d5b82ea6733e001478db1e2490b49575
Author: Dan Winship <danw gnome org>
Date:   Fri Mar 30 11:13:04 2012 -0400

    applet: fix up the shell-version-detecting code
    
    GNOME Shell claims its name on D-Bus without actually setting up its
    interfaces first, so if you try to query its properties right away, it
    fails. And GDBusProxy assumes (reasonably) that an object that doesn't
    implement the Properties interface when it first appears isn't going
    to start implementing it later, so the proxy will always think there
    are no properties there.
    
    So, to fix this, we have to destroy the proxy if we hit this bug, and
    then recreate it after a delay. Since this is getting complicated now,
    abstract all the shell-version-checking into a separate object.
    
    Also, don't do floating-point comparisons on version numbers, since
    floating point is inexact.

 src/Makefile.am     |    4 +-
 src/applet.c        |   70 +++-------------
 src/applet.h        |    4 +-
 src/shell-watcher.c |  242 +++++++++++++++++++++++++++++++++++++++++++++++++++
 src/shell-watcher.h |   60 +++++++++++++
 5 files changed, 318 insertions(+), 62 deletions(-)
---
diff --git a/src/Makefile.am b/src/Makefile.am
index c4808e6..2ec6416 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -59,7 +59,9 @@ nm_applet_SOURCES = \
 	applet-device-bt.c \
 	applet-device-wimax.h \
 	applet-device-wimax.c \
-	fallback-icon.h
+	fallback-icon.h \
+	shell-watcher.h \
+	shell-watcher.c
 
 nm_applet_LDADD = \
 	-lm \
diff --git a/src/applet.c b/src/applet.c
index 65357d1..e6a3e2e 100644
--- a/src/applet.c
+++ b/src/applet.c
@@ -79,6 +79,7 @@
 #include "applet-vpn-request.h"
 #include "utils.h"
 #include "gconf-helpers.h"
+#include "shell-watcher.h"
 
 #define NOTIFY_CAPS_ACTIONS_KEY "actions"
 
@@ -2955,7 +2956,7 @@ applet_agent_registered_cb (AppletAgent *agent,
 	NMApplet *applet = NM_APPLET (user_data);
 
 	/* If the shell is running and the agent just got registered, unregister it */
-	if (   (applet->shell_version >= 3.4)
+	if (   (nm_shell_watcher_version_at_least (applet->shell_watcher, 3, 4))
 	    && nm_secret_agent_get_registered (NM_SECRET_AGENT (agent))) {
 		g_message ("Stopping registered applet secret agent because GNOME Shell is running");
 		nm_secret_agent_unregister (NM_SECRET_AGENT (agent));
@@ -3249,55 +3250,16 @@ delayed_start_agent (gpointer user_data)
 	return FALSE;
 }
 
-static gboolean
-get_shell_version (GDBusProxy *proxy, gdouble *out_version)
-{
-	GVariant *v;
-	char *version, *p;
-	gboolean success = FALSE;
-	gdouble converted;
-
-	/* Ask for the shell's version */
-	v = g_dbus_proxy_get_cached_property (proxy, "ShellVersion");
-	if (v) {
-		g_warn_if_fail (g_variant_is_of_type (v, G_VARIANT_TYPE_STRING));
-		version = g_variant_dup_string (v, NULL);
-		if (version) {
-			/* Terminate at the second dot if there is one */
-			p = strchr (version, '.');
-			if (p && (p = strchr (p + 1, '.')))
-				*p = '\0';
-
-			converted = strtod (version, NULL);
-			g_warn_if_fail (converted > 0);
-			g_warn_if_fail (converted < 1000);
-			if (converted > 0 && converted < 1000) {
-				*out_version = converted;
-				success = TRUE;
-			}
-			g_free (version);
-		}
-		g_variant_unref (v);
-	}
-	return success;
-}
-
 static void
-name_owner_changed_cb (GDBusProxy *proxy, GParamSpec *pspec, gpointer user_data)
+shell_version_changed_cb (NMShellWatcher *watcher, GParamSpec *pspec, gpointer user_data)
 {
 	NMApplet *applet = user_data;
-	char *owner;
 
-	owner = g_dbus_proxy_get_name_owner (proxy);
-	if (owner) {
-		applet->shell_version = 0;
+	if (nm_shell_watcher_version_at_least (watcher, 3, 4)) {
 		if (applet->agent_start_id)
 			g_source_remove (applet->agent_start_id);
 
-		if (   get_shell_version (proxy, &applet->shell_version)
-		    && applet->shell_version >= 3.4
-		    && applet->agent
-		    && nm_secret_agent_get_registered (NM_SECRET_AGENT (applet->agent))) {
+		if (applet->agent && nm_secret_agent_get_registered (NM_SECRET_AGENT (applet->agent))) {
 			g_message ("Stopping applet secret agent because GNOME Shell appeared");
 			nm_secret_agent_unregister (NM_SECRET_AGENT (applet->agent));
 		}
@@ -3305,14 +3267,12 @@ name_owner_changed_cb (GDBusProxy *proxy, GParamSpec *pspec, gpointer user_data)
 		/* If the shell quit and our agent wasn't already registered, do it
 		 * now on a delay (just in case the shell is restarting.
 		 */
-		applet->shell_version = 0;
 		if (applet->agent_start_id)
 			g_source_remove (applet->agent_start_id);
 
 		if (nm_secret_agent_get_registered (NM_SECRET_AGENT (applet->agent)) == FALSE)
 			applet->agent_start_id = g_timeout_add_seconds (4, delayed_start_agent, applet);
 	}
-	g_free (owner);
 }
 #endif
 
@@ -3469,19 +3429,11 @@ constructor (GType type,
 
 #if GLIB_CHECK_VERSION(2,26,0)
 	/* Watch GNOME Shell so we can unregister our applet agent if it appears */
-	applet->shell_proxy = g_dbus_proxy_new_for_bus_sync (G_BUS_TYPE_SESSION,
-	                                                     G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
-	                                                     NULL,
-	                                                     "org.gnome.Shell",
-	                                                     "/org/gnome/Shell",
-	                                                     "org.gnome.Shell",
-	                                                     NULL,
-	                                                     NULL);
-	g_signal_connect (applet->shell_proxy,
-	                  "notify::g-name-owner",
-	                  G_CALLBACK (name_owner_changed_cb),
+	applet->shell_watcher = nm_shell_watcher_new ();
+	g_signal_connect (applet->shell_watcher,
+	                  "notify::shell-version",
+	                  G_CALLBACK (shell_version_changed_cb),
 	                  applet);
-	name_owner_changed_cb (applet->shell_proxy, NULL, applet);
 #endif
 
 	return G_OBJECT (applet);
@@ -3551,8 +3503,8 @@ static void finalize (GObject *object)
 		dbus_g_connection_unref (applet->session_bus);
 
 #if GLIB_CHECK_VERSION(2,26,0)
-	if (applet->shell_proxy)
-		g_object_unref (applet->shell_proxy);
+	if (applet->shell_watcher)
+		g_object_unref (applet->shell_watcher);
 #endif
 	if (applet->agent_start_id)
 		g_source_remove (applet->agent_start_id);
diff --git a/src/applet.h b/src/applet.h
index efc2eff..ab21ece 100644
--- a/src/applet.h
+++ b/src/applet.h
@@ -46,6 +46,7 @@
 #include <nm-active-connection.h>
 #include <nm-remote-settings.h>
 #include "applet-agent.h"
+#include "shell-watcher.h"
 
 #define NM_TYPE_APPLET			(nma_get_type())
 #define NM_APPLET(object)		(G_TYPE_CHECK_INSTANCE_CAST((object), NM_TYPE_APPLET, NMApplet))
@@ -85,10 +86,9 @@ typedef struct
 	DBusGConnection *session_bus;
 
 #if GLIB_CHECK_VERSION(2,26,0)
-	GDBusProxy *shell_proxy;
+	NMShellWatcher *shell_watcher;
 #endif
 	guint agent_start_id;
-	gdouble shell_version;
 
 	NMClient *nm_client;
 	NMRemoteSettings *settings;
diff --git a/src/shell-watcher.c b/src/shell-watcher.c
new file mode 100644
index 0000000..4cfdcb4
--- /dev/null
+++ b/src/shell-watcher.c
@@ -0,0 +1,242 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager Wireless Applet -- Display wireless access points and allow user control
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+
+#include "shell-watcher.h"
+
+#if GLIB_CHECK_VERSION(2,26,0)
+
+G_DEFINE_TYPE (NMShellWatcher, nm_shell_watcher, G_TYPE_OBJECT)
+
+struct NMShellWatcherPrivate {
+	GDBusProxy *shell_proxy;
+	guint signal_id;
+
+	guint retry_timeout;
+	guint retries;
+
+	guint shell_version;
+};
+
+enum {
+	PROP_0,
+	PROP_SHELL_VERSION,
+	LAST_PROP
+};
+
+static void create_gnome_shell_proxy (NMShellWatcher *watcher);
+
+static gboolean
+retry_create_shell_proxy (gpointer user_data)
+{
+	NMShellWatcher *watcher = user_data;
+	NMShellWatcherPrivate *priv = watcher->priv;
+
+	priv->retry_timeout = 0;
+	create_gnome_shell_proxy (watcher);
+	return FALSE;
+}
+
+static void
+try_update_version (NMShellWatcher *watcher)
+{
+	NMShellWatcherPrivate *priv = watcher->priv;
+	GVariant *v;
+	char *version, *p;
+
+	v = g_dbus_proxy_get_cached_property (priv->shell_proxy, "ShellVersion");
+	if (!v) {
+		/* The shell has claimed the name, but not yet registered its interfaces...
+		 * (https://bugzilla.gnome.org/show_bug.cgi?id=673182). There's no way
+		 * to make GDBusProxy re-read the properties at this point, so we
+		 * have to destroy this proxy and try again.
+		 */
+		if (priv->signal_id) {
+			g_signal_handler_disconnect (priv->shell_proxy, priv->signal_id);
+			priv->signal_id = 0;
+		}
+		g_object_unref (priv->shell_proxy);
+		priv->shell_proxy = NULL;
+
+		priv->retry_timeout = g_timeout_add_seconds (2, retry_create_shell_proxy, watcher); 
+		return;
+	}
+
+	g_warn_if_fail (g_variant_is_of_type (v, G_VARIANT_TYPE_STRING));
+	version = g_variant_dup_string (v, NULL);
+	if (version) {
+		guint major, minor;
+
+		major = strtoul (version, &p, 10);
+		if (*p == '.')
+			minor = strtoul (p + 1, NULL, 10);
+		else
+			minor = 0;
+
+		g_warn_if_fail (major < 256);
+		g_warn_if_fail (minor < 256);
+
+		priv->shell_version = (major << 8) | minor;
+		g_object_notify (G_OBJECT (watcher), "shell-version");
+	}
+
+	g_variant_unref (v);
+}
+
+static void
+name_owner_changed_cb (GDBusProxy *proxy, GParamSpec *pspec, gpointer user_data)
+{
+	NMShellWatcher *watcher = user_data;
+	NMShellWatcherPrivate *priv = watcher->priv;
+	char *owner;
+
+	owner = g_dbus_proxy_get_name_owner (proxy);
+	if (owner) {
+		try_update_version (watcher);
+		g_free (owner);
+	} else if (priv->shell_version) {
+		priv->shell_version = 0;
+		g_object_notify (G_OBJECT (watcher), "shell-version");
+	}
+}
+
+static void
+got_shell_proxy (GObject *source, GAsyncResult *result, gpointer user_data)
+{
+	NMShellWatcher *watcher = user_data;
+	NMShellWatcherPrivate *priv = watcher->priv;
+	GError *error = NULL;
+
+	priv->shell_proxy = g_dbus_proxy_new_for_bus_finish (result, &error);
+	if (!priv->shell_proxy) {
+		g_warning ("Could not create GDBusProxy for org.gnome.Shell: %s", error->message);
+		g_error_free (error);
+		return;
+	}
+
+	priv->signal_id = g_signal_connect (priv->shell_proxy,
+	                                    "notify::g-name-owner",
+	                                    G_CALLBACK (name_owner_changed_cb),
+	                                    watcher);
+
+	name_owner_changed_cb (priv->shell_proxy, NULL, watcher);
+	g_object_unref (watcher);
+}
+
+static void
+create_gnome_shell_proxy (NMShellWatcher *watcher)
+{
+	NMShellWatcherPrivate *priv = watcher->priv;
+
+	if (priv->retries++ == 5) {
+		g_warning ("Could not find ShellVersion property on org.gnome.Shell after 5 tries");
+		return;
+	}
+
+	g_dbus_proxy_new_for_bus (G_BUS_TYPE_SESSION,
+	                          G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START,
+	                          NULL,
+	                          "org.gnome.Shell",
+	                          "/org/gnome/Shell",
+	                          "org.gnome.Shell",
+	                          NULL,
+	                          got_shell_proxy,
+	                          g_object_ref (watcher));
+}
+
+static void
+nm_shell_watcher_init (NMShellWatcher *watcher)
+{
+	watcher->priv = G_TYPE_INSTANCE_GET_PRIVATE (watcher, NM_TYPE_SHELL_WATCHER,
+	                                             NMShellWatcherPrivate);
+	create_gnome_shell_proxy (watcher);
+}
+
+static void
+get_property (GObject *object, guint prop_id,
+              GValue *value, GParamSpec *pspec)
+{
+	NMShellWatcher *watcher = NM_SHELL_WATCHER (object);
+
+	switch (prop_id) {
+	case PROP_SHELL_VERSION:
+		g_value_set_uint (value, watcher->priv->shell_version);
+		break;
+	default:
+		G_OBJECT_WARN_INVALID_PROPERTY_ID (object, prop_id, pspec);
+		break;
+	}
+}
+
+static void
+finalize (GObject *object)
+{
+	NMShellWatcher *watcher = NM_SHELL_WATCHER (object);
+	NMShellWatcherPrivate *priv = watcher->priv;
+
+	if (priv->retry_timeout)
+		g_source_remove (priv->retry_timeout);
+	if (priv->signal_id)
+		g_signal_handler_disconnect (priv->shell_proxy, priv->signal_id);
+	if (priv->shell_proxy)
+		g_object_unref (priv->shell_proxy);
+
+	G_OBJECT_CLASS (nm_shell_watcher_parent_class)->finalize (object);
+}
+
+static void
+nm_shell_watcher_class_init (NMShellWatcherClass *klass)
+{
+	GObjectClass *oclass = G_OBJECT_CLASS (klass);
+
+	g_type_class_add_private (klass, sizeof (NMShellWatcherPrivate));
+
+	oclass->get_property = get_property;
+	oclass->finalize = finalize;
+
+	g_object_class_install_property (oclass, PROP_SHELL_VERSION,
+	                                 g_param_spec_uint ("shell-version",
+	                                                    "Shell version",
+	                                                    "Running GNOME Shell version, eg, 0x0304",
+	                                                    0, 0xFFFF, 0,
+	                                                    G_PARAM_READABLE |
+	                                                    G_PARAM_STATIC_STRINGS));
+}
+
+NMShellWatcher *
+nm_shell_watcher_new (void)
+{
+	return g_object_new (NM_TYPE_SHELL_WATCHER, NULL);
+}
+
+gboolean
+nm_shell_watcher_version_at_least (NMShellWatcher *watcher, guint major, guint minor)
+{
+	guint version = (major << 8) | minor;
+
+	return watcher->priv->shell_version >= version;
+}
+
+#endif /* GLIB_CHECK_VERSION(2,26,0) */
diff --git a/src/shell-watcher.h b/src/shell-watcher.h
new file mode 100644
index 0000000..ad18e71
--- /dev/null
+++ b/src/shell-watcher.h
@@ -0,0 +1,60 @@
+/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */
+/* NetworkManager Wireless Applet -- Display wireless access points and allow user control
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Copyright (C) 2012 Red Hat, Inc.
+ */
+
+#ifndef SHELL_WATCHER_H
+#define SHELL_WATCHER_H
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <gio/gio.h>
+
+#if GLIB_CHECK_VERSION(2,26,0)
+
+#define NM_TYPE_SHELL_WATCHER			(nm_shell_watcher_get_type())
+#define NM_SHELL_WATCHER(object)		(G_TYPE_CHECK_INSTANCE_CAST((object), NM_TYPE_SHELL_WATCHER, NMShellWatcher))
+#define NM_SHELL_WATCHER_CLASS(klass)		(G_TYPE_CHECK_CLASS_CAST((klass), NM_TYPE_SHELL_WATCHER, NMShellWatcherClass))
+#define NM_IS_SHELL_WATCHER(object)		(G_TYPE_CHECK_INSTANCE_TYPE((object), NM_TYPE_SHELL_WATCHER))
+#define NM_IS_SHELL_WATCHER_CLASS(klass)	(G_TYPE_CHECK_CLASS_TYPE((klass), NM_TYPE_SHELL_WATCHER))
+#define NM_SHELL_WATCHER_GET_CLASS(object)	(G_TYPE_INSTANCE_GET_CLASS((object), NM_TYPE_SHELL_WATCHER, NMShellWatcherClass))
+
+typedef struct NMShellWatcherPrivate NMShellWatcherPrivate;
+
+typedef struct {
+	GObject parent_instance;
+
+	NMShellWatcherPrivate *priv;
+} NMShellWatcher;
+
+typedef struct {
+	GObjectClass parent_class;
+} NMShellWatcherClass;
+
+GType nm_shell_watcher_get_type (void);
+
+NMShellWatcher *nm_shell_watcher_new (void);
+
+gboolean nm_shell_watcher_version_at_least (NMShellWatcher *watcher,
+                                            guint major, guint minor);
+
+#endif /* GLIB_CHECK_VERSION(2,26,0) */
+
+#endif
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]