halfway through client changes but...

should fix crashes irt actions in the action queue for clients that have been destroyed. now those actions are skipped or performed without a client as possible.

openbox/client.c

@@ -50,6 +50,12 @@
 #define CLIENT_NOPROPAGATEMASK (ButtonPressMask | ButtonReleaseMask | \
 				ButtonMotionMask)
 
+typedef struct
+{
+    ObClientDestructor func;
+    gpointer data;
+} Destructor;
+
 GList      *client_list        = NULL;
 GSList     *client_destructors = NULL;
 
@@ -81,14 +87,26 @@ void client_shutdown(gboolean reconfig)
 {
 }
 
-void client_add_destructor(GDestroyNotify func)
+void client_add_destructor(ObClientDestructor func, gpointer data)
 {
-    client_destructors = g_slist_prepend(client_destructors, (gpointer)func);
+    Destructor *d = g_new(Destructor, 1);
+    d->func = func;
+    d->data = data;
+    client_destructors = g_slist_prepend(client_destructors, d);
 }
 
-void client_remove_destructor(GDestroyNotify func)
+void client_remove_destructor(ObClientDestructor func)
 {
-    client_destructors = g_slist_remove(client_destructors, (gpointer)func);
+    GSList *it;
+
+    for (it = client_destructors; it; it = g_slist_next(it)) {
+        Destructor *d = it->data;
+        if (d->func == func) {
+            g_free(d);
+            client_destructors = g_slist_delete_link(client_destructors, it);
+            break;
+        }
+    }
 }
 
 void client_set_list()
@@ -402,8 +420,8 @@ void client_unmanage(ObClient *self)
     screen_update_areas();
 
     for (it = client_destructors; it; it = g_slist_next(it)) {
-        GDestroyNotify func = (GDestroyNotify) it->data;
+        Destructor *d = it->data;
-        func(self);
+        d->func(self, d->data);
     }
         
     if (focus_client == self) {
@@ -473,9 +491,9 @@ void client_unmanage(ObClient *self)
     /* free all data allocated in the client struct */
     g_slist_free(self->transients);
     for (j = 0; j < self->nicons; ++j)
-	g_free(self->icons[j].data);
+        g_free(self->icons[j].data);
     if (self->nicons > 0)
-	g_free(self->icons);
+        g_free(self->icons);
     g_free(self->title);
     g_free(self->icon_title);
     g_free(self->name);
@@ -1498,35 +1516,35 @@ void client_update_icons(ObClient *self)
     guint w, h, i, j;
 
     for (i = 0; i < self->nicons; ++i)
-	g_free(self->icons[i].data);
+        g_free(self->icons[i].data);
     if (self->nicons > 0)
-	g_free(self->icons);
+        g_free(self->icons);
     self->nicons = 0;
 
     if (PROP_GETA32(self->window, net_wm_icon, cardinal, &data, &num)) {
-	/* figure out how many valid icons are in here */
+        /* figure out how many valid icons are in here */
-	i = 0;
+        i = 0;
-	while (num - i > 2) {
+        while (num - i > 2) {
-	    w = data[i++];
+            w = data[i++];
-	    h = data[i++];
+            h = data[i++];
-	    i += w * h;
+            i += w * h;
-	    if (i > num || w*h == 0) break;
+            if (i > num || w*h == 0) break;
-	    ++self->nicons;
+            ++self->nicons;
-	}
+        }
 
-	self->icons = g_new(ObClientIcon, self->nicons);
+        self->icons = g_new(ObClientIcon, self->nicons);
     
-	/* store the icons */
+        /* store the icons */
-	i = 0;
+        i = 0;
-	for (j = 0; j < self->nicons; ++j) {
+        for (j = 0; j < self->nicons; ++j) {
             guint x, y, t;
 
-	    w = self->icons[j].width = data[i++];
+            w = self->icons[j].width = data[i++];
-	    h = self->icons[j].height = data[i++];
+            h = self->icons[j].height = data[i++];
 
             if (w*h == 0) continue;
 
-	    self->icons[j].data = g_new(RrPixel32, w * h);
+            self->icons[j].data = g_new(RrPixel32, w * h);
             for (x = 0, y = 0, t = 0; t < w * h; ++t, ++x, ++i) {
                 if (x >= w) {
                     x = 0;
@@ -1538,10 +1556,10 @@ void client_update_icons(ObClient *self)
                     (((data[i] >> 8) & 0xff) << RrDefaultGreenOffset) +
                     (((data[i] >> 0) & 0xff) << RrDefaultBlueOffset);
             }
-	    g_assert(i <= num);
+            g_assert(i <= num);
-	}
+        }
 
-	g_free(data);
+        g_free(data);
     } else if (PROP_GETA32(self->window, kwm_win_icon,
                            kwm_win_icon, &data, &num)) {
         if (num == 2) {
@@ -1583,18 +1601,8 @@ void client_update_icons(ObClient *self)
         }
     }
 
-    if (!self->nicons) {
-        self->nicons++;
-        self->icons = g_new(ObClientIcon, self->nicons);
-        self->icons[self->nicons-1].width = 48;
-        self->icons[self->nicons-1].height = 48;
-        self->icons[self->nicons-1].data = g_memdup(ob_rr_theme->def_win_icon,
-                                                    sizeof(RrPixel32)
-                                                    * 48 * 48);
-    }
-
     if (self->frame)
-	frame_adjust_icon(self->frame);
+        frame_adjust_icon(self->frame);
 }
 
 static void client_change_state(ObClient *self)
@@ -2659,13 +2667,36 @@ gboolean client_focused(ObClient *self)
     return self == focus_client;
 }
 
-ObClientIcon *client_icon(ObClient *self, int w, int h)
+static ObClientIcon* client_icon_recursive(ObClient *self, int w, int h)
 {
     guint i;
     /* si is the smallest image >= req */
     /* li is the largest image < req */
     unsigned long size, smallest = 0xffffffff, largest = 0, si = 0, li = 0;
 
+    g_message("icons %d", self->nicons);
+
+    if (!self->nicons) {
+        ObClientIcon *parent = NULL;
+
+        if (self->transient_for) {
+            if (self->transient_for != OB_TRAN_GROUP)
+                parent = client_icon_recursive(self->transient_for, w, h);
+            else {
+                GSList *it;
+                for (it = self->group->members; it; it = g_slist_next(it)) {
+                    ObClient *c = it->data;
+                    if (c != self && !c->transient_for) {
+                        if ((parent = client_icon_recursive(c, w, h)))
+                            break;
+                    }
+                }
+            }
+        }
+        
+        return parent;
+    }
+
     for (i = 0; i < self->nicons; ++i) {
         size = self->icons[i].width * self->icons[i].height;
         if (size < smallest && size >= (unsigned)(w * h)) {
@@ -2682,6 +2713,21 @@ ObClientIcon *client_icon(ObClient *self, int w, int h)
     return &self->icons[li];
 }
 
+const ObClientIcon* client_icon(ObClient *self, int w, int h)
+{
+    ObClientIcon *ret;
+    static ObClientIcon deficon;
+
+    g_message("going for broke");
+    if (!(ret = client_icon_recursive(self, w, h))) {
+        g_message("using default");
+        deficon.width = deficon.height = 48;
+        deficon.data = ob_rr_theme->def_win_icon;
+        ret = &deficon;
+    }
+    return ret;
+}
+
 /* this be mostly ripped from fvwm */
 ObClient *client_find_directional(ObClient *c, ObDirection dir) 
 {

openbox/client.h

@@ -272,8 +272,10 @@ extern GList *client_list;
 void client_startup(gboolean reconfig);
 void client_shutdown(gboolean reconfig);
 
-void client_add_destructor(GDestroyNotify func);
+typedef void (*ObClientDestructor)(ObClient *client, gpointer data);
-void client_remove_destructor(GDestroyNotify func);
+
+void client_add_destructor(ObClientDestructor func, gpointer data);
+void client_remove_destructor(ObClientDestructor func);
 
 /*! Manages all existing windows */
 void client_manage_all();
@@ -503,7 +505,7 @@ void client_setup_decor_and_functions(ObClient *self);
 /*! Retrieves the window's type and sets ObClient->type */
 void client_get_type(ObClient *self);
 
-ObClientIcon *client_icon(ObClient *self, int w, int h);
+const ObClientIcon *client_icon(ObClient *self, int w, int h);
 
 /*! Searches a client's direct parents for a focused window. The function does
   not check for the passed client, only for *ONE LEVEL* of its parents.

openbox/client_list_menu.c

@@ -53,7 +53,7 @@ static void desk_menu_update(ObMenuFrame *frame, gpointer data)
             GSList *acts;
             ObAction* act;
             ObMenuEntry *e;
-            ObClientIcon *icon;
+            const ObClientIcon *icon;
 
             empty = FALSE;
 

openbox/event.c

@@ -69,7 +69,7 @@ static void event_handle_client(ObClient *c, XEvent *e);
 static void event_handle_group(ObGroup *g, XEvent *e);
 
 static gboolean focus_delay_func(gpointer data);
-static void focus_delay_client_dest(gpointer data);
+static void focus_delay_client_dest(ObClient *client, gpointer data);
 
 static gboolean menu_hide_delay_func(gpointer data);
 
@@ -157,7 +157,7 @@ void event_startup(gboolean reconfig)
     IceAddConnectionWatch(ice_watch, NULL);
 #endif
 
-    client_add_destructor(focus_delay_client_dest);
+    client_add_destructor(focus_delay_client_dest, NULL);
 }
 
 void event_shutdown(gboolean reconfig)
@@ -1252,11 +1252,9 @@ static gboolean focus_delay_func(gpointer data)
     return FALSE; /* no repeat */
 }
 
-static void focus_delay_client_dest(gpointer data)
+static void focus_delay_client_dest(ObClient *client, gpointer data)
 {
-    ObClient *c = data;
+    ob_main_loop_timeout_remove_data(ob_main_loop, focus_delay_func, client);
-
-    ob_main_loop_timeout_remove_data(ob_main_loop, focus_delay_func, c);
 }
 
 void event_ignore_queued_enters()

openbox/focus.c

@@ -42,10 +42,10 @@ ObClient *focus_cycle_target;
 
 static ObIconPopup *focus_cycle_popup;
 
-static void focus_cycle_destructor(ObClient *c)
+static void focus_cycle_destructor(ObClient *client, gpointer data)
 {
     /* end cycling if the target disappears */
-    if (focus_cycle_target == c)
+    if (focus_cycle_target == client)
         focus_cycle(TRUE, TRUE, TRUE, TRUE, TRUE);
 }
 
@@ -54,7 +54,7 @@ void focus_startup(gboolean reconfig)
     focus_cycle_popup = icon_popup_new(TRUE);
 
     if (!reconfig) {
-        client_add_destructor((GDestroyNotify) focus_cycle_destructor);
+        client_add_destructor(focus_cycle_destructor, NULL);
 
         /* start with nothing focused */
         focus_set_client(NULL);
@@ -68,7 +68,7 @@ void focus_shutdown(gboolean reconfig)
     icon_popup_free(focus_cycle_popup);
 
     if (!reconfig) {
-        client_remove_destructor((GDestroyNotify) focus_cycle_destructor);
+        client_remove_destructor(focus_cycle_destructor);
 
         for (i = 0; i < screen_num_desktops; ++i)
             g_list_free(focus_order[i]);

openbox/framerender.c

@@ -238,9 +238,9 @@ static void framerender_icon(ObFrame *self, RrAppearance *a)
     if (self->icon_x < 0) return;
 
     if (self->client->nicons) {
-        ObClientIcon *icon = client_icon(self->client,
+        const ObClientIcon *icon = client_icon(self->client,
-                                         ob_rr_theme->button_size + 2,
+                                               ob_rr_theme->button_size + 2,
-                                         ob_rr_theme->button_size + 2);
+                                               ob_rr_theme->button_size + 2);
         a->texture[0].type = RR_TEXTURE_RGBA;
         a->texture[0].data.rgba.width = icon->width;
         a->texture[0].data.rgba.height = icon->height;

openbox/keyboard.c

@@ -197,17 +197,16 @@ void keyboard_interactive_end(ObInteractiveState *s,
     }
 }
 
-void keyboard_interactive_end_client(gpointer data)
+void keyboard_interactive_end_client(ObClient *client, gpointer data)
 {
     GSList *it, *next;
-    ObClient *c = data;
 
     for (it = interactive_states; it; it = next) {
         ObInteractiveState *s = it->data;
 
         next = g_slist_next(it);
 
-        if (s->client == c)
+        if (s->client == client)
             s->client = NULL;
     }
 }
@@ -291,7 +290,7 @@ void keyboard_startup(gboolean reconfig)
     grab_keys(TRUE);
 
     if (!reconfig)
-        client_add_destructor(keyboard_interactive_end_client);
+        client_add_destructor(keyboard_interactive_end_client, NULL);
 }
 
 void keyboard_shutdown(gboolean reconfig)

openbox/mainloop.c

@@ -18,6 +18,7 @@
 
 #include "mainloop.h"
 #include "action.h"
+#include "client.h"
 
 #include <stdio.h>
 #include <stdlib.h>
@@ -88,7 +89,7 @@ struct _ObMainLoop
     guint signals_fired[NUM_SIGNALS];
     GSList *signal_handlers[NUM_SIGNALS];
 
-    GQueue *action_queue;
+    GSList *action_queue;
 };
 
 struct _ObMainLoopTimer
@@ -178,7 +179,7 @@ ObMainLoop *ob_main_loop_new(Display *display)
 
     all_loops = g_slist_prepend(all_loops, loop);
 
-    loop->action_queue = g_queue_new();
+    loop->action_queue = NULL;
 
     return loop;
 }
@@ -230,7 +231,9 @@ void ob_main_loop_destroy(ObMainLoop *loop)
             }
         }
 
-        g_queue_free(loop->action_queue);
+        for (it = loop->action_queue; it; it = g_slist_next(it))
+            action_unref(it->data);
+        g_slist_free(loop->action_queue);
 
         g_free(loop);
     }
@@ -249,7 +252,20 @@ static void fd_handle_foreach(gpointer key,
 
 void ob_main_loop_queue_action(ObMainLoop *loop, ObAction *act)
 {
-    g_queue_push_tail(loop->action_queue, action_copy(act));
+    loop->action_queue = g_slist_append(loop->action_queue, action_copy(act));
+}
+
+static void ob_main_loop_client_destroy(ObClient *client, gpointer data)
+{
+    ObMainLoop *loop = data;
+    GSList *it;
+
+    for (it = loop->action_queue; it; it = g_slist_next(it)) {
+        ObAction *act = it->data;
+
+        if (act->data.any.c == client)
+            act->data.any.c = NULL;
+    }
 }
 
 void ob_main_loop_run(ObMainLoop *loop)
@@ -263,6 +279,8 @@ void ob_main_loop_run(ObMainLoop *loop)
     loop->run = TRUE;
     loop->running = TRUE;
 
+    client_add_destructor(ob_main_loop_client_destroy, loop);
+
     while (loop->run) {
         if (loop->signal_fired) {
             guint i;
@@ -294,12 +312,31 @@ void ob_main_loop_run(ObMainLoop *loop)
                     h->func(&e, h->data);
                 }
             } while (XPending(loop->display));
-        } else if ((act = g_queue_pop_head(loop->action_queue))) {
+        } else if (loop->action_queue) {
              /* only fire off one action at a time, then go back for more
                 X events, since the action might cause some X events (like
                 FocusIn :) */
-            act->func(&act->data);
+
-            action_unref(act);
+            do {
+                act = loop->action_queue->data;
+                if (act->data.any.client_action == OB_CLIENT_ACTION_ALWAYS &&
+                    !act->data.any.c)
+                {
+                    loop->action_queue =
+                        g_slist_delete_link(loop->action_queue,
+                                            loop->action_queue);
+                    action_unref(act);
+                    act = NULL;
+                }
+            } while (!act && loop->action_queue);
+
+            if  (act) {
+                act->func(&act->data);
+                loop->action_queue =
+                    g_slist_delete_link(loop->action_queue,
+                                        loop->action_queue);
+                action_unref(act);
+            }
         } else {
             /* this only runs if there were no x events received */
 
@@ -323,6 +360,8 @@ void ob_main_loop_run(ObMainLoop *loop)
         }
     }
 
+    client_remove_destructor(ob_main_loop_client_destroy);
+
     loop->running = FALSE;
 }
 

openbox/menu.c

@@ -51,7 +51,7 @@ static void parse_menu_separator(ObParseInst *i,
 static void parse_menu(ObParseInst *i, xmlDocPtr doc, xmlNodePtr node,
                        gpointer data);
 
-static void client_dest(gpointer client)
+static void client_dest(ObClient *client, gpointer data)
 {
     /* menus can be associated with a client, so close any that are since
        we are disappearing now */
@@ -98,7 +98,7 @@ void menu_startup(gboolean reconfig)
     g_assert(menu_parse_state.parent == NULL);
 
     if (!reconfig)
-        client_add_destructor(client_dest);
+        client_add_destructor(client_dest, NULL);
 }
 
 void menu_shutdown(gboolean reconfig)

openbox/moveresize.c

@@ -46,7 +46,7 @@ static ObCorner lockcorner;
 
 static ObPopup *popup = NULL;
 
-static void client_dest(gpointer client)
+static void client_dest(ObClient *client, gpointer data)
 {
     if (moveresize_client == client)
         moveresize_end(TRUE);    
@@ -57,7 +57,7 @@ void moveresize_startup(gboolean reconfig)
     popup = popup_new(FALSE);
 
     if (!reconfig)
-        client_add_destructor(client_dest);
+        client_add_destructor(client_dest, NULL);
 }
 
 void moveresize_shutdown(gboolean reconfig)

openbox/popup.c

@@ -248,7 +248,7 @@ void icon_popup_free(ObIconPopup *self)
 }
 
 void icon_popup_show(ObIconPopup *self,
-                     gchar *text, struct _ObClientIcon *icon)
+                     gchar *text, const ObClientIcon *icon)
 {
     if (icon) {
         self->a_icon->texture[0].type = RR_TEXTURE_RGBA;

openbox/popup.h

@@ -98,7 +98,7 @@ ObIconPopup *icon_popup_new();
 void icon_popup_free(ObIconPopup *self);
 
 void icon_popup_show(ObIconPopup *self,
-                     gchar *text, struct _ObClientIcon *icon);
+                     gchar *text, const struct _ObClientIcon *icon);
 #define icon_popup_hide(p) popup_hide((p)->popup)
 #define icon_popup_position(p, g, x, y) popup_position((p)->popup,(g),(x),(y))
 #define icon_popup_size(p, w, h) popup_size((p)->popup,(w),(h))