Merge lp:~larsu/telepathy-indicator/fix-warnings into lp:telepathy-indicator

[Lars Karlitski]

Listen to telepathy events even when empathy is not running

Listen to telepathy events even when empathy is not running

Clearing the approver and observer objects doesn't seem to disconnect all callbacks from telepathy. This patch keeps both alive and does nothing when empathy is not running.

This gets rid of a lot of warnings.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:75
http://jenkins.qa.ubuntu.com/job/telepathy-indicator-ci/2/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telepathy-indicator-trusty-amd64-ci/1
    SUCCESS: http://jenkins.qa.ubuntu.com/job/telepathy-indicator-trusty-armhf-ci/1
        deb: http://jenkins.qa.ubuntu.com/job/telepathy-indicator-trusty-armhf-ci/1/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/telepathy-indicator-ci/2/rebuild

review: Approve (continuous-integration)

[Charles Kerr (charlesk)]

Looks fine and seems to work. Approve.

On style points, I wish this had been a NullObject rather than a NULL pointer test -- there are so many more logic branches in the code now. This is going to hurt the coverage stats. Oh wait... ;-)

review: Approve

=== modified file 'src/indicator-approver.c'
--- src/indicator-approver.c	2013-02-20 00:35:00 +0000
+++ src/indicator-approver.c	2014-04-10 08:35:06 +0000
@@ -98,7 +98,8 @@
 		channel = g_ptr_array_index (channels, 0);
 		id = channel_id (channel);
 
-		messaging_menu_app_remove_source (mmapp, id);
+		if (mmapp)
+			messaging_menu_app_remove_source (mmapp, id);
 		g_hash_table_remove (dispatch_ops, id);
 		update_launcher (n_sources--);
 
@@ -125,7 +126,8 @@
 		{
 			g_warning ("Failed to handle operations: %s\n", error->message);
 			g_error_free (error);
-			messaging_menu_app_remove_source (mmapp, id);
+			if (mmapp)
+				messaging_menu_app_remove_source (mmapp, id);
 			update_launcher (n_sources--);
 		}
 	}
@@ -245,7 +247,6 @@
 {
 	TpContact *contact;
 	GtkWidget *dialog;
-	gchar *id;
 	const gchar *alias = "";
 	gchar *title;
 	const gchar *message;
@@ -312,10 +313,14 @@
 
 	gtk_widget_show (GTK_WIDGET (dialog));
 
-	id = contact_id (contact);
-	messaging_menu_app_remove_source (mmapp, id);
-
-	g_free (id);
+	if (mmapp) {
+		gchar *id;
+
+		id = contact_id (contact);
+		messaging_menu_app_remove_source (mmapp, id);
+
+		g_free (id);
+	}
 }
 
 static TpConnection *
@@ -393,9 +398,11 @@
 			  const gchar      *id,
 			  gpointer          user_data)
 {
-	TpAccountManager *tp_am = user_data;
+	TpAccountManager *tp_am;
 	gchar **sections;
 
+	tp_am = tp_account_manager_dup ();
+
 	/* message source ids contain the type of source, the
 	 * tpconnection's object path, and the object path of the actual
 	 * source object, separated by colons */
@@ -452,6 +459,7 @@
 
 out:
 	g_strfreev (sections);
+	g_object_unref (tp_am);
 }
 
 static void
@@ -460,7 +468,6 @@
 	TpChannel *channel = user_data;
 	GPtrArray *contacts;
 	GError *error = NULL;
-	gchar *id;
 	const gchar *alias = "";
 	GFile *avatar_file;
 	GIcon *avatar = NULL;
@@ -485,14 +492,19 @@
 	}
 	g_debug ("handle_contacts_pending_add_indicator_cb alias: %s", alias);
 
-	id = channel_id (channel);
-	messaging_menu_app_append_source (mmapp, id, avatar, alias);
-	messaging_menu_app_draw_attention (mmapp, id);
+	if (mmapp) {
+		gchar *id;
+
+		id = channel_id (channel);
+		messaging_menu_app_append_source (mmapp, id, avatar, alias);
+		messaging_menu_app_draw_attention (mmapp, id);
+
+		g_free (id);
+	}
 
 	update_launcher (++n_sources);
 
 	g_clear_object (&avatar);
-	g_free (id);
 	g_ptr_array_free (contacts, TRUE);
 }
 
@@ -516,7 +528,7 @@
 
 		id = channel_id (TP_CHANNEL (channel));
 
-		if (!messaging_menu_app_has_source (mmapp, id))
+		if (mmapp && !messaging_menu_app_has_source (mmapp, id))
 		{
 			connection = tp_channel_get_connection (TP_CHANNEL (channel));
 			contact = tp_channel_get_initiator_contact (TP_CHANNEL (channel));
@@ -533,14 +545,18 @@
 static void
 pending_message_removed_cb (TpTextChannel *channel, TpMessage *message, gpointer user_data)
 {
-	gchar *id;
 	g_debug ("pending_message_removed_cb: %s", tp_channel_get_identifier (TP_CHANNEL (channel)));
 
-	id = channel_id (TP_CHANNEL (channel));
-	messaging_menu_app_remove_source (mmapp, id);
+	if (mmapp) {
+		gchar *id;
+
+		id = channel_id (TP_CHANNEL (channel));
+		messaging_menu_app_remove_source (mmapp, id);
+
+		g_free (id);
+	}
+
 	update_launcher (--n_sources);
-
-	g_free (id);
 }
 
 static void
@@ -572,29 +588,32 @@
 
 	id = channel_id (channel);
 
-	if (messaging_menu_app_has_source (mmapp, id))
-	{
-		messaging_menu_app_set_source_time (mmapp, id, g_get_real_time ());
-	}
-	else
-	{
-		TpContact *contact;
-		const gchar *alias = "";
-		GFile *avatar_file;
-		GIcon *avatar;
-
-		contact = g_ptr_array_index (contacts, 0);
-		alias = tp_contact_get_alias (contact);
-		avatar_file = tp_contact_get_avatar_file (contact);
-		avatar = avatar_file ? g_file_icon_new (avatar_file) : NULL;
-
-		messaging_menu_app_append_source (mmapp, id, avatar, alias);
-		update_launcher (++n_sources);
-
-		g_clear_object (&avatar);
-	}
-
-	messaging_menu_app_draw_attention (mmapp, id);
+	if (mmapp) {
+		if (messaging_menu_app_has_source (mmapp, id))
+		{
+			messaging_menu_app_set_source_time (mmapp, id, g_get_real_time ());
+		}
+		else
+		{
+			TpContact *contact;
+			const gchar *alias = "";
+			GFile *avatar_file;
+			GIcon *avatar;
+
+			contact = g_ptr_array_index (contacts, 0);
+			alias = tp_contact_get_alias (contact);
+			avatar_file = tp_contact_get_avatar_file (contact);
+			avatar = avatar_file ? g_file_icon_new (avatar_file) : NULL;
+
+			messaging_menu_app_append_source (mmapp, id, avatar, alias);
+
+			g_clear_object (&avatar);
+		}
+
+		messaging_menu_app_draw_attention (mmapp, id);
+	}
+
+	update_launcher (++n_sources);
 
 	g_hash_table_insert (dispatch_ops, g_strdup (id), dispatch_op);
 	g_signal_connect (dispatch_op, "invalidated", G_CALLBACK (dispatch_op_finished_cb), NULL);
@@ -687,22 +706,24 @@
 
 				id = contact_id (contact);
 
-				if (messaging_menu_app_has_source (mmapp, id))
-				{
-					messaging_menu_app_set_source_time (mmapp, id, g_get_real_time ());
-				}
-				else
-				{
-					alias = tp_contact_get_alias (contact);
-					avatar_file = tp_contact_get_avatar_file (contact);
-					avatar = avatar_file ? g_file_icon_new (avatar_file) : NULL;
-
-					messaging_menu_app_append_source (mmapp, id, avatar, alias);
-					messaging_menu_app_draw_attention (mmapp, id);
-
-					n_sources++;
-
-					g_clear_object (&avatar);
+				if (mmapp) {
+					if (messaging_menu_app_has_source (mmapp, id))
+					{
+						messaging_menu_app_set_source_time (mmapp, id, g_get_real_time ());
+					}
+					else
+					{
+						alias = tp_contact_get_alias (contact);
+						avatar_file = tp_contact_get_avatar_file (contact);
+						avatar = avatar_file ? g_file_icon_new (avatar_file) : NULL;
+
+						messaging_menu_app_append_source (mmapp, id, avatar, alias);
+						messaging_menu_app_draw_attention (mmapp, id);
+
+						n_sources++;
+
+						g_clear_object (&avatar);
+					}
 				}
 
 				g_debug ("contact_list_changed_cb: adding %s", alias);
@@ -722,7 +743,8 @@
 			contact_remove_from_contact_list (contact);
 		}
 
-		messaging_menu_app_remove_source (mmapp, tp_contact_get_identifier (contact));
+		if (mmapp)
+			messaging_menu_app_remove_source (mmapp, tp_contact_get_identifier (contact));
 		n_sources--;
 	}
 
@@ -765,7 +787,8 @@
 	g_debug ("add_account");
 	tp_g_signal_connect_object (account, "notify::connection",
 		G_CALLBACK (account_conn_changed_cb), manager, 0);
-	messaging_menu_app_register (mmapp);
+	if (mmapp != NULL)
+		messaging_menu_app_register (mmapp);
 	check_account (account, manager);
 }
 
@@ -893,6 +916,9 @@
 {
 	MessagingMenuStatus mm_status;
 
+	if (mmapp == NULL)
+		return;
+
 	switch (presence)
 	{
 	case TP_CONNECTION_PRESENCE_TYPE_AVAILABLE:
@@ -928,9 +954,11 @@
 			       MessagingMenuStatus mm_status,
 			       gpointer            user_data)
 {
-	TpAccountManager *tp_am = user_data;
+	TpAccountManager *tp_am;
 	TpConnectionPresenceType status;
 
+	tp_am = tp_account_manager_dup ();
+
 	switch (mm_status)
 	{
 	case MESSAGING_MENU_STATUS_AVAILABLE:
@@ -958,6 +986,8 @@
 	}
 
 	tp_account_manager_set_all_requested_presences (tp_am, status, "", "");
+
+	g_object_unref (tp_am);
 }
 
 gboolean check_enabled_accounts ()
@@ -988,13 +1018,14 @@
 static void on_enabled_event (TpAccountManager* self, TpAccount* account, gpointer user_data)
 {
 	g_debug ("on_enabled_event");
-	messaging_menu_app_register (mmapp);
+	if (mmapp)
+		messaging_menu_app_register (mmapp);
 }
 
 static void on_disabled_event (TpAccountManager* self, TpAccount* account, gpointer user_data)
 {
 	g_debug ("on_disabled_event");
-	if (!check_enabled_accounts ())
+	if (mmapp && !check_enabled_accounts ())
 	{
 		messaging_menu_app_unregister (mmapp);
 	}
@@ -1004,19 +1035,15 @@
 static void on_account_removed_event (TpAccountManager* self, TpAccount* account, gpointer user_data)
 {
 	g_debug ("on_account_removed_event");
-	if (!check_enabled_accounts ())
+	if (mmapp && !check_enabled_accounts ())
 	{
 		messaging_menu_app_unregister (mmapp);
 	}
 }
 
 static void
-empathy_appeared (GDBusConnection *connection,
-		  const gchar     *name,
-		  const gchar     *name_owner,
-		  gpointer         user_data)
+setup (GMainLoop *loop)
 {
-	GMainLoop *loop = user_data;
 	TpAccountManager *tp_am;
 	TpBaseClient *approver;
 	TpBaseClient *observer;
@@ -1047,16 +1074,6 @@
 		return;
 	}
 
-	mmapp = messaging_menu_app_new ("empathy.desktop");
-
-	if (check_enabled_accounts ())
-		messaging_menu_app_register (mmapp);
-
-	g_signal_connect (mmapp, "activate-source",
-			  G_CALLBACK (message_source_activated), tp_am);
-	g_signal_connect (mmapp, "status-changed",
-			  G_CALLBACK (messaging_menu_status_changed), tp_am);
-
 	presence = tp_account_manager_get_most_available_presence (tp_am, NULL, NULL);
 	presence_changed (tp_am, presence, "", "", NULL);
 	g_signal_connect(tp_am, "most-available-presence-changed",
@@ -1078,16 +1095,30 @@
 }
 
 static void
+empathy_appeared (GDBusConnection *connection,
+		  const gchar     *name,
+		  const gchar     *name_owner,
+		  gpointer         user_data)
+{
+	mmapp = messaging_menu_app_new ("empathy.desktop");
+
+	if (check_enabled_accounts ())
+		messaging_menu_app_register (mmapp);
+
+	g_signal_connect (mmapp, "activate-source",
+			  G_CALLBACK (message_source_activated), NULL);
+	g_signal_connect (mmapp, "status-changed",
+			  G_CALLBACK (messaging_menu_status_changed), NULL);
+}
+
+static void
 empathy_vanished (GDBusConnection *connection,
 		  const gchar     *name,
 		  gpointer         user_data)
 {
 	g_debug (G_STRFUNC);
 
-	g_clear_object (&approver);
-	g_clear_object (&observer);
 	g_clear_object (&launcher);
-	g_clear_pointer (&dispatch_ops, g_hash_table_unref);
 	g_clear_object (&mmapp);
 }
 
@@ -1099,13 +1130,14 @@
 	gtk_init (&argc, &argv);
 
 	loop = g_main_loop_new (NULL, FALSE);
+	setup (loop);
 
 	g_bus_watch_name (G_BUS_TYPE_SESSION,
 			  "org.gnome.Empathy",
 			  G_BUS_NAME_WATCHER_FLAGS_NONE,
 			  empathy_appeared,
 			  empathy_vanished,
-			  loop, NULL);
+			  NULL, NULL);
 
 	g_debug ("Telepathy Indicator started");
 	g_main_loop_run (loop);