Re: Updated unexpected window destruction patch.

[Owen Taylor <otaylor redhat com>]

I've been looking more at this patch in preparation for
applying it, and, thinking about it, I really don't like
the event-rewriting approach. If we do use that it, really
needs to be:

 event->window = gtk_window_destroy_notify_new (window);

But, I'm wondering why we really need to try to get it
right so early, as opposed to just surviving until we
get the destroy notify on the toplevel.

What I'd like to do is:

 - When we receive a DestroyNotify event, warn if it 
   is unexpected, and then mark the window and all
   children as destroyed.

 In GTK+:

 - if we receive a DestroyEvent for a child
   window, ignore it and do nothing.

   if we receive a DestroyEvent for a toplevel window,
   destroy that toplevel window. (conditionally,
   to be consistent with DeleteEvent and keep Tim happy)

 - Add a unrealize handler to GtkPlug to unreference
   the socket window.

This will result in a few more warnings being spewed
out then the other approach, but should be about as
robust, and considerably simpler. Patch appended.
Some testing indicates that this is pretty robust.

It should be easy enough to only produce one BadDrawable
warnings per window, something like:

===
int error_handler (Display *d, XErrorEvent *e)
{
  if ((e->error_code == BadWindow) || (e->error_code == BadDrawable))
    {
      GdkWindow *window = gdk_window_lookup (e->resourceid);

      if (window)
	{
	  if (!g_dataset_get_data (window, "bonobo-error"))
	    {
	      g_dataset_set_data_full (window, "bonobo-error",
				       g_new (gint, 1),
				       (GDestroyNotify)g_free);
	  
	      g_warning ("Error accessing window %ld", e->resourceid);
	    }
	}
      return 0;
    }
  else
    {
      return (*old_handler)(d, e);
    }
}
===

[ BTW, look at XAddToSaveSet(). It is _almost_ perfectly
  what we want, except that if we die, the child window is
  going to be reparented as a child of the window manager's frame
  window which is going to be destroyed immediately after,
  so it actually doesn't help. ]

Regards,
                                        Owen


============
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/gtk+/ChangeLog,v
retrieving revision 1.1061.2.128
diff -u -r1.1061.2.128 ChangeLog
--- ChangeLog	1999/09/02 05:41:56	1.1061.2.128
+++ ChangeLog	1999/09/02 20:28:43
@@ -1,3 +1,17 @@
+Thu Sep  2 16:33:59 1999  Owen Taylor  <otaylor@redhat.com>
+
+	* gdk/gdkwindow.c: When we receive an unexpected
+	destroy notify on one of our windows, don't just
+	warn about it, also mark our windows as destroyed.
+
+	* gtk/gtkmain.c: Ignore unexpected destroy notifies
+	for children, for toplevel windows handle them
+	like delete_event.
+
+	* gtk/gtkplug.c: Add an unrealize handler so that
+	we unref plug->socket_window when we are done
+	with it.
+
 Thu Sep  2 07:38:56 1999  Tim Janik  <timj@gtk.org>
 
 	* gtk/gtkwindow.c (gtk_window_move_resize):
Index: gdk/gdkwindow.c
===================================================================
RCS file: /cvs/gnome/gtk+/gdk/gdkwindow.c,v
retrieving revision 1.80.2.6
diff -u -r1.80.2.6 gdkwindow.c
--- gdk/gdkwindow.c	1999/09/02 03:41:41	1.80.2.6
+++ gdk/gdkwindow.c	1999/09/02 20:28:43
@@ -699,10 +699,10 @@
   
   if (!private->destroyed)
     {
-      if (private->window_type == GDK_WINDOW_FOREIGN)
-	gdk_window_internal_destroy (window, FALSE, FALSE);
-      else
+      if (private->window_type != GDK_WINDOW_FOREIGN)
 	g_warning ("GdkWindow %#lx unexpectedly destroyed", private->xwindow);
+
+      gdk_window_internal_destroy (window, FALSE, FALSE);
     }
   
   gdk_xid_table_remove (private->xwindow);
Index: gtk/gtkmain.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkmain.c,v
retrieving revision 1.112.2.5
diff -u -r1.112.2.5 gtkmain.c
--- gtk/gtkmain.c	1999/07/28 13:26:40	1.112.2.5
+++ gtk/gtkmain.c	1999/09/02 20:28:44
@@ -685,11 +685,17 @@
       break;
       
     case GDK_DESTROY:
-      gtk_widget_ref (event_widget);
-      gtk_widget_event (event_widget, event);
-      if (!GTK_OBJECT_DESTROYED (event_widget))
-	gtk_widget_destroy (event_widget);
-      gtk_widget_unref (event_widget);
+      /* Unexpected GDK_DESTROY from the outside, ignore for
+       * child windows, handle like a GDK_DELETE for toplevels
+       */
+      if (!event_widget->parent)
+	{
+	  gtk_widget_ref (event_widget);
+	  if (!gtk_widget_event (event_widget, event) &&
+	      !GTK_OBJECT_DESTROYED (event_widget))
+	    gtk_widget_destroy (event_widget);
+	  gtk_widget_unref (event_widget);
+	}
       break;
       
     case GDK_PROPERTY_NOTIFY:
Index: gtk/gtkplug.c
===================================================================
RCS file: /cvs/gnome/gtk+/gtk/gtkplug.c,v
retrieving revision 1.6
diff -u -r1.6 gtkplug.c
--- gtk/gtkplug.c	1999/02/24 07:35:47	1.6
+++ gtk/gtkplug.c	1999/09/02 20:28:44
@@ -33,6 +33,7 @@
 static void gtk_plug_init       (GtkPlug      *plug);
 
 static void gtk_plug_realize    (GtkWidget *widget);
+static void gtk_plug_unrealize	(GtkWidget *widget);
 static gint gtk_plug_key_press_event (GtkWidget   *widget,
 				      GdkEventKey *event);
 static void gtk_plug_forward_key_press (GtkPlug *plug, GdkEventKey *event);
@@ -44,6 +45,8 @@
 /* From Tk */
 #define EMBEDDED_APP_WANTS_FOCUS NotifyNormal+20
 
+static GtkWindowClass *parent_class = NULL;
+
 guint
 gtk_plug_get_type ()
 {
@@ -77,7 +80,10 @@
   widget_class = (GtkWidgetClass *)class;
   window_class = (GtkWindowClass *)class;
 
+  parent_class = gtk_type_class (gtk_window_get_type ());
+
   widget_class->realize = gtk_plug_realize;
+  widget_class->unrealize = gtk_plug_unrealize;
   widget_class->key_press_event = gtk_plug_key_press_event;
   widget_class->focus_in_event = gtk_plug_focus_in_event;
   widget_class->focus_out_event = gtk_plug_focus_out_event;
@@ -117,6 +123,27 @@
   plug = GTK_PLUG (gtk_type_new (gtk_plug_get_type ()));
   gtk_plug_construct (plug, socket_id);
   return GTK_WIDGET (plug);
+}
+
+static void
+gtk_plug_unrealize (GtkWidget *widget)
+{
+  GtkPlug *plug;
+
+  g_return_if_fail (widget != NULL);
+  g_return_if_fail (GTK_IS_PLUG (widget));
+
+  plug = GTK_PLUG (widget);
+
+  if (plug->socket_window != NULL)
+    {
+      gdk_window_set_user_data (plug->socket_window, NULL);
+      gdk_window_unref (plug->socket_window);
+      plug->socket_window = NULL;
+    }
+
+  if (GTK_WIDGET_CLASS (parent_class)->unrealize)
+    (* GTK_WIDGET_CLASS (parent_class)->unrealize) (widget);
 }
 
 static void