Adding an 'UnWatch' command to the standard DBus interface

for graceful disconnection from a service.

9 files changed, 210 insertions, 21 deletions

diff --git a/.bzrignore b/.bzrignore
index f11a31f..1baab30 100644
--- a/.bzrignore
+++ b/.bzrignore
@@ -135,3 +135,5 @@ tests/service-manager-connect-tester
 tests/session.conf
 tests/service-manager-connect.service
 tools/indicator-loader
+tests/service-manager-connect-nostart-tester
+tests/service-manager-nostart-connect
diff --git a/debian/changelog b/debian/changelog
index 974460a..8df2c3c 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+libindicator (0.3.0~dev-0ubuntu1~ppa7) UNRELEASED; urgency=low
+
+  * Adding an 'UnWatch' command to the standard DBus interface
+    for graceful disconnection from a service.
+
+ -- Ted Gould <ted@ubuntu.com>  Wed, 02 Dec 2009 08:36:06 -0600
+
 libindicator (0.3.0~dev-0ubuntu1~ppa6) karmic; urgency=low
 
   * Adding a new tool to load indicators from the command line.
diff --git a/libindicator/indicator-service-manager.c b/libindicator/indicator-service-manager.c
index 4eaed23..13c9015 100644
--- a/libindicator/indicator-service-manager.c
+++ b/libindicator/indicator-service-manager.c
@@ -16,6 +16,7 @@ struct _IndicatorServiceManagerPrivate {
 	DBusGProxy * dbus_proxy;
 	DBusGProxy * service_proxy;
 	gboolean connected;
+	DBusGConnection * bus;
 };
 
 /* Signals Stuff */
@@ -51,6 +52,7 @@ static void indicator_service_manager_finalize   (GObject *object);
 static void set_property (GObject * object, guint prop_id, const GValue * value, GParamSpec * pspec);
 static void get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspec);
 static void start_service (IndicatorServiceManager * service);
+static void unwatch_cb (DBusGProxy *proxy, GError *error, gpointer userdata);
 
 G_DEFINE_TYPE (IndicatorServiceManager, indicator_service_manager, G_TYPE_OBJECT);
 
@@ -105,17 +107,18 @@ indicator_service_manager_init (IndicatorServiceManager *self)
 	priv->dbus_proxy = NULL;
 	priv->service_proxy = NULL;
 	priv->connected = FALSE;
+	priv->bus = NULL;
 
 	/* Start talkin' dbus */
 	GError * error = NULL;
-	DBusGConnection * session_bus = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
+	priv->bus = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
 	if (error != NULL) {
 		g_error("Unable to get session bus: %s", error->message);
 		g_error_free(error);
 		return;
 	}
 
-	priv->dbus_proxy = dbus_g_proxy_new_for_name_owner(session_bus,
+	priv->dbus_proxy = dbus_g_proxy_new_for_name_owner(priv->bus,
 	                                                   DBUS_SERVICE_DBUS,
 	                                                   DBUS_PATH_DBUS,
 	                                                   DBUS_INTERFACE_DBUS,
@@ -147,6 +150,12 @@ indicator_service_manager_dispose (GObject *object)
 		priv->dbus_proxy = NULL;
 	}
 
+	/* If we have a proxy, tell it we're shutting down.  Just
+	   to be polite about it. */
+	if (priv->service_proxy != NULL) {
+		org_ayatana_indicator_service_un_watch_async(priv->service_proxy, unwatch_cb, NULL);
+	}
+
 	/* Destory our service proxy, we won't need it. */
 	if (priv->service_proxy != NULL) {
 		g_object_unref(G_OBJECT(priv->service_proxy));
@@ -232,6 +241,12 @@ get_property (GObject * object, guint prop_id, GValue * value, GParamSpec * pspe
 }
 
 static void
+unwatch_cb (DBusGProxy *proxy, GError *error, gpointer userdata)
+{
+	return;
+}
+
+static void
 watch_cb (DBusGProxy * proxy, gint service_version, GError * error, gpointer user_data)
 {
 	IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(user_data);
@@ -244,6 +259,7 @@ watch_cb (DBusGProxy * proxy, gint service_version, GError * error, gpointer use
 
 	if (service_version != INDICATOR_SERVICE_VERSION) {
 		g_warning("Service is using a different version of the service interface.  Expecting %d and got %d.", INDICATOR_SERVICE_VERSION, service_version);
+		org_ayatana_indicator_service_un_watch_async(priv->service_proxy, unwatch_cb, NULL);
 		return;
 	}
 
@@ -271,14 +287,7 @@ start_service_cb (DBusGProxy * proxy, guint status, GError * error, gpointer use
 	}
 
 	/* Woot! it's running.  Let's do it some more. */
-	DBusGConnection * session_bus = dbus_g_bus_get(DBUS_BUS_SESSION, &error);
-	if (error != NULL) {
-		g_error("Unable to get session bus: %s", error->message);
-		g_error_free(error);
-		return;
-	}
-
-	priv->service_proxy = dbus_g_proxy_new_for_name_owner(session_bus,
+	priv->service_proxy = dbus_g_proxy_new_for_name_owner(priv->bus,
 	                                                  priv->name,
 	                                                  INDICATOR_SERVICE_OBJECT,
 	                                                  INDICATOR_SERVICE_INTERFACE,
@@ -294,16 +303,35 @@ start_service_cb (DBusGProxy * proxy, guint status, GError * error, gpointer use
 static void
 start_service (IndicatorServiceManager * service)
 {
+	GError * error = NULL;
 	IndicatorServiceManagerPrivate * priv = INDICATOR_SERVICE_MANAGER_GET_PRIVATE(service);
 
 	g_return_if_fail(priv->dbus_proxy != NULL);
 	g_return_if_fail(priv->name != NULL);
 
-	org_freedesktop_DBus_start_service_by_name_async (priv->dbus_proxy,
-	                                                  priv->name,
-	                                                  0,
-	                                                  start_service_cb,
-	                                                  service);
+	/* Check to see if we can get a proxy to it first. */
+	priv->service_proxy = dbus_g_proxy_new_for_name_owner(priv->bus,
+	                                                      priv->name,
+	                                                      INDICATOR_SERVICE_OBJECT,
+	                                                      INDICATOR_SERVICE_INTERFACE,
+	                                                      &error);
+
+	if (error != NULL) {
+		/* We don't care about the error, just start the service anyway. */
+		g_error_free(error);
+		org_freedesktop_DBus_start_service_by_name_async (priv->dbus_proxy,
+		                                                  priv->name,
+		                                                  0,
+		                                                  start_service_cb,
+		                                                  service);
+	} else {
+		/* If we got a proxy just because we're good people then
+		   we need to call watch on it just like 'start_service_cb'
+		   does. */
+		org_ayatana_indicator_service_watch_async(priv->service_proxy,
+		                                          watch_cb,
+		                                          service);
+	}
 
 	return;
 }
diff --git a/libindicator/indicator-service.c b/libindicator/indicator-service.c
index 69422c5..5fb939d 100644
--- a/libindicator/indicator-service.c
+++ b/libindicator/indicator-service.c
@@ -8,6 +8,7 @@
 
 /* DBus Prototypes */
 static gboolean _indicator_service_server_watch (IndicatorService * service, DBusGMethodInvocation * method);
+static gboolean _indicator_service_server_un_watch (IndicatorService * service, DBusGMethodInvocation * method);
 
 #include "indicator-service-server.h"
 #include "dbus-shared.h"
@@ -305,7 +306,48 @@ _indicator_service_server_watch (IndicatorService * service, DBusGMethodInvocati
 		priv->timeout = 0;
 	}
 
-	dbus_g_method_return(method, 1);
+	dbus_g_method_return(method, INDICATOR_SERVICE_VERSION);
+	return TRUE;
+}
+
+static gint
+find_watcher (gconstpointer a, gconstpointer b)
+{
+	return g_strcmp0((const gchar *)a, (const gchar *)b);
+}
+
+static gboolean
+_indicator_service_server_un_watch (IndicatorService * service, DBusGMethodInvocation * method)
+{
+	g_return_val_if_fail(INDICATOR_IS_SERVICE(service), FALSE);
+	IndicatorServicePrivate * priv = INDICATOR_SERVICE_GET_PRIVATE(service);
+
+	/* Remove us from the watcher list here */
+	GList * watcher_item = g_list_find_custom(priv->watchers, dbus_g_method_get_sender(method), find_watcher);
+	if (watcher_item != NULL) {
+		/* Free the watcher */
+		gchar * name = watcher_item->data;
+		priv->watchers = g_list_remove(priv->watchers, name);
+		g_free(name);
+	} else {
+		/* Odd that we couldn't find the person, but, eh */
+		g_warning("Unable to find watcher who is unwatching: %s", dbus_g_method_get_sender(method));
+	}
+
+	/* If we're out of watchers set the timeout for shutdown */
+	if (priv->watchers == NULL) {
+		if (priv->timeout != 0) {
+			/* This should never really happen, but let's ensure that
+			   bad things don't happen if it does. */
+			g_warning("No watchers timeout set twice.  Resolving, but odd.");
+			g_source_remove(priv->timeout);
+			priv->timeout = 0;
+		}
+		/* If we don't get a new watcher quickly, we'll shutdown. */
+		priv->timeout = g_timeout_add(500, timeout_no_watchers, service);
+	}
+
+	dbus_g_method_return(method);
 	return TRUE;
 }
 
diff --git a/libindicator/indicator-service.xml b/libindicator/indicator-service.xml
index d876ea8..8b389e2 100644
--- a/libindicator/indicator-service.xml
+++ b/libindicator/indicator-service.xml
@@ -9,6 +9,9 @@
 			<annotation name="org.freedesktop.DBus.GLib.Async" value="true" />
 			<arg type="i" name="version" direction="out" />
 		</method>
+		<method name="UnWatch">
+			<annotation name="org.freedesktop.DBus.GLib.Async" value="true" />
+		</method>
 
 <!-- Signals -->
 		<!-- None currently -->
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 80c4191..09a227d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -6,6 +6,7 @@ check_PROGRAMS = \
 	service-manager-no-connect \
 	service-manager-connect \
 	service-manager-connect-service \
+	service-manager-nostart-connect \
 	service-shutdown-timeout
 
 lib_LTLIBRARIES = \
@@ -200,6 +201,29 @@ TESTS += service-manager-connect-tester
 DISTCLEANFILES += service-manager-connect-tester session.conf service-manager-connect.service
 
 #############################
+# Service Manager Shutdown
+#############################
+
+service_manager_nostart_connect_SOURCES = \
+	service-manager-nostart-connect.c
+
+service_manager_nostart_connect_CFLAGS = \
+	-Wall -Werror \
+	$(LIBINDICATOR_CFLAGS) -I$(top_srcdir)
+
+service_manager_nostart_connect_LDADD = \
+	$(LIBINDICATOR_LIBS) \
+	$(top_builddir)/libindicator/.libs/libindicator.a
+
+service-manager-connect-nostart-tester: service-manager-nostart-connect service-manager-connect-service Makefile.am
+	@echo "#!/bin/sh" > $@
+	@echo dbus-test-runner --task ./service-manager-nostart-connect --task ./service-manager-connect-service >> $@
+	@chmod +x $@
+
+TESTS += service-manager-connect-nostart-tester
+DISTCLEANFILES += service-manager-connect-nostart-tester
+
+#############################
 # Test stuff
 #############################
 
diff --git a/tests/service-manager-connect-service.c b/tests/service-manager-connect-service.c
index 297f3ba..d60e414 100644
--- a/tests/service-manager-connect-service.c
+++ b/tests/service-manager-connect-service.c
@@ -8,7 +8,7 @@ static gboolean passed = FALSE;
 gboolean
 timeout (gpointer data)
 {
-	passed = TRUE;
+	passed = FALSE;
 	g_debug("Timeout with no shutdown.");
 	g_main_loop_quit(mainloop);
 	return FALSE;
@@ -17,8 +17,8 @@ timeout (gpointer data)
 void
 shutdown (void)
 {
-	g_error("Shutdown");
-	passed = FALSE;
+	g_debug("Shutdown");
+	passed = TRUE;
 	g_main_loop_quit(mainloop);
 	return;
 }
@@ -28,6 +28,8 @@ main (int argc, char ** argv)
 {
 	g_type_init();
 
+	g_debug("Starting service");
+
 	IndicatorService * is = indicator_service_new("org.ayatana.test");
 	g_signal_connect(G_OBJECT(is), INDICATOR_SERVICE_SIGNAL_SHUTDOWN, shutdown, NULL);
 
diff --git a/tests/service-manager-connect.c b/tests/service-manager-connect.c
index c252542..91d2bad 100644
--- a/tests/service-manager-connect.c
+++ b/tests/service-manager-connect.c
@@ -15,8 +15,22 @@ timeout (gpointer data)
 }
 
 void
-connection (void)
+connection (IndicatorServiceManager * sm, gboolean connected, gpointer user_data)
 {
+	static gboolean has_connected = FALSE;
+
+	if (has_connected && connected) {
+		g_warning("We got two connected signals.  FAIL.");
+		passed = FALSE;
+		return;
+	}
+
+	if (!connected) {
+		g_debug("Not connected");
+		return;
+	}
+
+	has_connected = TRUE;
 	g_debug("Connection");
 	passed = TRUE;
 	g_main_loop_quit(mainloop);
@@ -30,13 +44,15 @@ main (int argc, char ** argv)
 	g_log_set_always_fatal(G_LOG_LEVEL_CRITICAL);
 
 	IndicatorServiceManager * is = indicator_service_manager_new("org.ayatana.test");
-	g_signal_connect(G_OBJECT(is), INDICATOR_SERVICE_MANAGER_SIGNAL_CONNECTION_CHANGE, connection, NULL);
+	g_signal_connect(G_OBJECT(is), INDICATOR_SERVICE_MANAGER_SIGNAL_CONNECTION_CHANGE, G_CALLBACK(connection), NULL);
 
 	g_timeout_add_seconds(1, timeout, NULL);
 
 	mainloop = g_main_loop_new(NULL, FALSE);
 	g_main_loop_run(mainloop);
 
+	g_object_unref(is);
+
 	g_debug("Quiting");
 	if (passed) {
 		g_debug("Passed");
diff --git a/tests/service-manager-nostart-connect.c b/tests/service-manager-nostart-connect.c
new file mode 100644
index 0000000..7107f42
--- /dev/null
+++ b/tests/service-manager-nostart-connect.c
@@ -0,0 +1,65 @@
+
+#include <glib.h>
+#include "libindicator/indicator-service-manager.h"
+
+static GMainLoop * mainloop = NULL;
+static gboolean passed = FALSE;
+
+gboolean
+timeout (gpointer data)
+{
+	passed = FALSE;
+	g_error("Timeout with no connection.");
+	g_main_loop_quit(mainloop);
+	return FALSE;
+}
+
+void
+connection (IndicatorServiceManager * sm, gboolean connected, gpointer user_data)
+{
+	static gboolean has_connected = FALSE;
+
+	if (has_connected && connected) {
+		g_warning("We got two connected signals.  FAIL.");
+		passed = FALSE;
+		return;
+	}
+
+	if (!connected) {
+		g_debug("Not connected");
+		return;
+	}
+
+	has_connected = TRUE;
+	g_debug("Connection");
+	passed = TRUE;
+	g_main_loop_quit(mainloop);
+	return;
+}
+
+int
+main (int argc, char ** argv)
+{
+	g_type_init();
+	g_log_set_always_fatal(G_LOG_LEVEL_CRITICAL);
+
+	g_usleep(150000);
+
+	IndicatorServiceManager * is = indicator_service_manager_new("org.ayatana.test");
+	g_signal_connect(G_OBJECT(is), INDICATOR_SERVICE_MANAGER_SIGNAL_CONNECTION_CHANGE, G_CALLBACK(connection), NULL);
+
+	g_timeout_add_seconds(1, timeout, NULL);
+
+	mainloop = g_main_loop_new(NULL, FALSE);
+	g_main_loop_run(mainloop);
+
+	g_object_unref(is);
+
+	g_debug("Quiting");
+	if (passed) {
+		g_debug("Passed");
+		return 0;
+	}
+	g_debug("Failed");
+	return 1;
+}