xig-team team mailing list archive

[Sam Spilsbury <sam.spilsbury@xxxxxxxxxxxxx>]

Sam Spilsbury has proposed merging lp:~smspillaz/xig/xig.clean-up-after-clients into lp:xig with lp:~smspillaz/xig/xig.real-get-property-reply-message-length as a prerequisite.

Requested reviews:
  Xig Development Team (xig-team)

For more details, see:
https://code.launchpad.net/~smspillaz/xig/xig.clean-up-after-clients/+merge/89533

Remove window id's left behind by clients due to buggy X extension libraries or by SIGKILL.

Remove stale selections
-- 
https://code.launchpad.net/~smspillaz/xig/xig.clean-up-after-clients/+merge/89533
Your team Xig Development Team is requested to review the proposed merge of lp:~smspillaz/xig/xig.clean-up-after-clients into lp:xig.

=== modified file 'src/xig-remote-client-private.h'
--- src/xig-remote-client-private.h	2011-12-01 23:46:20 +0000
+++ src/xig-remote-client-private.h	2012-01-21 11:59:25 +0000
@@ -23,4 +23,9 @@
 
 void _xig_remote_client_circulate_request (XigRemoteClient *client, XigWindow *window, XigCirculatePlace place);
 
+GList * _xig_remote_client_get_owned_drawables (XigRemoteClient *client);
+
+void _xig_remote_client_add_drawable (XigRemoteClient *client, XigDrawable *drawable);
+void _xig_remote_client_remove_drawable (XigRemoteClient *client, XigDrawable *drawable);
+
 #endif /* _XIG_REMOTE_CLIENT_PRIVATE_H_ */

=== modified file 'src/xig-remote-client.c'
--- src/xig-remote-client.c	2012-01-10 16:45:57 +0000
+++ src/xig-remote-client.c	2012-01-21 11:59:25 +0000
@@ -47,6 +47,9 @@
 
     /* Event masks */
     GHashTable *window_events;
+
+    /* Owned windows */
+    GHashTable *owned_drawables;
 };
 
 enum
@@ -287,6 +290,24 @@
     xig_codec_feed_circulate_request (XIG_CODEC (client), &message);
 }
 
+GList *
+_xig_remote_client_get_owned_drawables (XigRemoteClient *client)
+{
+    return g_hash_table_get_keys (client->priv->owned_drawables);
+}
+
+void
+_xig_remote_client_add_drawable (XigRemoteClient *client, XigDrawable *drawable)
+{
+    g_hash_table_insert (client->priv->owned_drawables, GINT_TO_POINTER (xig_drawable_get_id (drawable)), g_object_ref (drawable));
+}
+
+void
+_xig_remote_client_remove_drawable (XigRemoteClient *client, XigDrawable *drawable)
+{
+    g_hash_table_remove (client->priv->owned_drawables, GINT_TO_POINTER (xig_drawable_get_id (drawable)));
+}
+
 static void
 circulate_notify_cb (XigWindow *window, XigRemoteClient *client)
 {
@@ -412,7 +433,7 @@
     if (n_read < 0)
         g_warning ("Error reading from socket: %s", strerror (errno));
     else if (n_read == 0)
-    {      
+    {
         g_signal_emit (client, signals[DISCONNECTED], 0);
         return FALSE;
     }
@@ -1574,6 +1595,7 @@
 
     reply.sequence_number = message->sequence_number;
     focus = xig_server_get_input_focus (client->priv->server, &reply.revert_to);
+
     reply.focus = focus ? xig_window_get_id (focus) : XIG_WINDOW_ID_NONE;
 
     xig_codec_feed_get_input_focus_reply (XIG_CODEC (codec), &reply);
@@ -2582,6 +2604,7 @@
     client->priv->server = server;
     client->priv->socket = g_object_ref (socket);
     client->priv->channel = g_io_channel_unix_new (g_socket_get_fd (socket));
+    client->priv->owned_drawables = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
     g_io_add_watch (client->priv->channel, G_IO_IN, socket_data_cb, client);
 
     return client;

=== modified file 'src/xig-server.c'
--- src/xig-server.c	2011-12-02 10:34:45 +0000
+++ src/xig-server.c	2012-01-21 11:59:25 +0000
@@ -150,12 +150,78 @@
     server->priv->listen_tcp = listen_tcp;
 }
 
+static gboolean
+_xig_server_hash_table_remove_selection_for_window (gpointer  key,
+                                                    gpointer  owner,
+                                                    gpointer  user_data)
+{
+    return owner == user_data ? TRUE : FALSE;
+}
+
+void
+_xig_server_remove_window (XigServer *server, XigWindow *window)
+{
+    g_hash_table_remove (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (window))));
+
+    /* Remove any selection owners too */
+    g_hash_table_foreach_remove (server->priv->selection_owners, _xig_server_hash_table_remove_selection_for_window, (gpointer) window);
+
+    /* Unset the focus window (revert to root window */
+    if (window == server->priv->focus_window)
+        server->priv->focus_window = NULL;
+}
+
+void
+_xig_server_add_window (XigServer *server, XigWindow *window)
+{
+    g_hash_table_insert (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (window))), g_object_ref (window));
+}
+
+void
+_xig_server_add_pixmap (XigServer *server, XigPixmap *pixmap)
+{
+    g_hash_table_insert (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (pixmap))), g_object_ref (pixmap));
+}
+
+void
+_xig_server_remove_pixmap (XigServer *server, XigPixmap *pixmap)
+{
+    g_hash_table_remove (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (pixmap))));
+}
+
+
 static void
 xig_remote_client_disconnected_cb (XigRemoteClient *client, XigServer *server)
 {
     g_signal_handlers_disconnect_matched (client, G_SIGNAL_MATCH_DATA, 0, 0, NULL, NULL, server);
     server->priv->clients = g_list_remove (server->priv->clients, client);
     g_signal_emit (server, signals[CLIENT_DISCONNECTED], 0, client);
+
+    GList *list = _xig_remote_client_get_owned_drawables (client);
+    GList *orig = list;
+
+    while (list)
+    {
+        gint id = GPOINTER_TO_INT (list->data);
+	XigDrawable *drawable = g_hash_table_lookup (server->priv->drawables, GINT_TO_POINTER (id));
+
+	if (XIG_IS_WINDOW (drawable))
+	{
+	    XigWindow *window = XIG_WINDOW (drawable);
+
+	    _xig_server_remove_window (server, window);
+	}
+	else if (XIG_IS_PIXMAP (drawable))
+	{
+	    XigPixmap *pixmap = XIG_PIXMAP (drawable);
+
+	    _xig_server_remove_pixmap (server, pixmap);
+	}
+        list = g_list_next (list);
+    }
+
+    g_list_free (orig);
+
     g_object_unref (client);
 }
 
@@ -257,12 +323,6 @@
 }
 
 void
-_xig_server_add_window (XigServer *server, XigWindow *window)
-{
-    g_hash_table_insert (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (window))), g_object_ref (window));
-}
-
-void
 _xig_server_set_selection_owner (XigServer *server, const gchar *name, XigWindow *window)
 {
     g_hash_table_insert (server->priv->selection_owners, g_strdup (name), g_object_ref (window));  
@@ -274,18 +334,6 @@
     return g_hash_table_lookup (server->priv->selection_owners, name);
 }
 
-void
-_xig_server_add_pixmap (XigServer *server, XigPixmap *pixmap)
-{
-    g_hash_table_insert (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (pixmap))), g_object_ref (pixmap));
-}
-
-void
-_xig_server_remove_pixmap (XigServer *server, XigPixmap *pixmap)
-{
-    g_hash_table_remove (server->priv->drawables, GINT_TO_POINTER (xig_drawable_get_id (XIG_DRAWABLE (pixmap))));
-}
-
 XigDrawable *
 xig_server_get_drawable (XigServer *server, guint32 drawable)
 {
@@ -346,7 +394,7 @@
 void
 xig_server_set_input_focus (XigServer *server, XigWindow *window, guint8 revert_to)
 {
-    server->priv->focus_window = g_object_ref (window);
+    server->priv->focus_window = window;
     // FIXME: Catch delete event
     server->priv->focus_revert_to = revert_to;
 }

=== modified file 'src/xig-window.c'
--- src/xig-window.c	2012-01-10 17:14:41 +0000
+++ src/xig-window.c	2012-01-21 11:59:25 +0000
@@ -76,6 +76,76 @@
 };
 static guint signals[LAST_SIGNAL] = { 0 };
 
+static void
+destroy_recursive (XigWindow *window)
+{
+    GList *link;
+
+    /* Destroy all children first */
+    for (link = window->priv->children; link; link = link->next)
+    {
+        XigWindow *w = link->data;
+        destroy_recursive (w);
+    }
+
+    g_signal_emit (window, signals[DESTROY_NOTIFY], 0);
+    g_signal_emit (window->priv->parent, signals[CHILD_DESTROY_NOTIFY], 0, window);
+
+    g_object_unref (window);
+}
+
+void
+xig_window_destroy (XigWindow *window)
+{
+    window->priv->parent->priv->children = g_list_remove (window->priv->parent->priv->children, window);
+    destroy_recursive (window);
+}
+
+void
+xig_window_destroy_subwindows (XigWindow *window)
+{
+    GList *link;
+    for (link = window->priv->children; link; link = link->next)
+    {
+        XigWindow *w = link->data;
+        destroy_recursive (w);
+    }
+    g_list_free (window->priv->children);
+    window->priv->children = NULL;
+}
+
+/* When clients disconnect, they are meant to (implicitly, through xcb)
+ * remove any resources that they held. When a client dies, it can't do that
+ * so clean up after it */
+static void
+xig_window_client_disconnected_cb (XigServer *server, XigRemoteClient *client, XigWindow *window)
+{
+    GList *list = _xig_remote_client_get_owned_drawables (client);
+    GList *orig = list;
+
+    while (list)
+    {
+        gint id = GPOINTER_TO_INT (list->data);
+
+        GList *child = window->priv->children;
+
+        while (child)
+        {
+            if (xig_window_get_id (XIG_WINDOW (child->data)) == id)
+	    {
+		xig_window_destroy (XIG_WINDOW (child->data));
+                break;
+            }
+
+            child = g_list_next (child);
+        }
+
+        list = g_list_next (list);
+    }
+
+    g_list_free (orig);
+}
+
 XigWindow *
 _xig_window_new (XigServer *server, XigRemoteClient *client, guint32 wid, XigWindow *parent, XigWindowClassType class, gint16 x, gint16 y, guint16 width, guint16 height, guint16 border_width, XigVisual *visual)
 {
@@ -97,6 +167,10 @@
     window->priv->border_width = border_width;
     window->priv->visual = g_object_ref (visual);
 
+    if (client)
+        _xig_remote_client_add_drawable (client, XIG_DRAWABLE (window));
+    g_signal_connect (server, "client-disconnected", G_CALLBACK (xig_window_client_disconnected_cb), window);
+
     if (parent)
         g_signal_emit (parent, signals[CREATE_NOTIFY], 0, window);
 
@@ -695,44 +769,6 @@
 }
 
 static void
-destroy_recursive (XigWindow *window)
-{
-    GList *link;
-
-    /* Destroy all children first */
-    for (link = window->priv->children; link; link = link->next)
-    {
-        XigWindow *w = link->data;
-        destroy_recursive (w);
-    }
-
-    g_signal_emit (window, signals[DESTROY_NOTIFY], 0);
-    g_signal_emit (window->priv->parent, signals[CHILD_DESTROY_NOTIFY], 0, window);
-
-    g_object_unref (window);
-}
-
-void
-xig_window_destroy (XigWindow *window)
-{
-    window->priv->parent->priv->children = g_list_remove (window->priv->parent->priv->children, window);
-    destroy_recursive (window);
-}
-
-void
-xig_window_destroy_subwindows (XigWindow *window)
-{
-    GList *link;
-    for (link = window->priv->children; link; link = link->next)
-    {
-        XigWindow *w = link->data;
-        destroy_recursive (w);
-    }
-    g_list_free (window->priv->children);
-    window->priv->children = NULL;
-}
-
-static void
 xig_window_init (XigWindow *window)
 {
     window->priv = G_TYPE_INSTANCE_GET_PRIVATE (window, xig_window_get_type (), XigWindowPrivate);
@@ -753,6 +789,10 @@
 xig_window_finalize (GObject *object)
 {
     XigWindow *window = (XigWindow *) object;
+
+    if (window->priv->client)
+        _xig_remote_client_remove_drawable (window->priv->client, XIG_DRAWABLE (window));
+
     if (window->priv->visual)
         g_object_unref (window->priv->visual);
     if (window->priv->background_pixmap)