Re: Gdk unexpected window destruction patch.

[Tim Janik <timj gtk org>]

On 18 Aug 1999, Nat Friedman wrote:

> 
>     Problem number two (2) is that the component may have had some
> drawing queued on one of those now-defunct windows.  And those draw
> operations will result in BadDrawable X errors, terminating the
> component.  The source-level disfigurement required to fix this has
> been relegated to Bonobo, which basically just ignores all BadDrawable
> X errors for now (see bonobo/gnome-main.c).  Suggestions welcome.

hm, i'm not at all sure about this, but i'd actually imagine that your
BadDrawable X errors could be fixed by adding a GtkPlug.destroy_event
handler:

gboolean
gtk_plug_destroy_event (GtkWidget   *widget,
                        GdkEventAny *event)
{
  GtkPlug *plug = GTK_PLUG (widget);
  
  if (event->any.window == plug->socket_window)
    {
      /* our container or one of our child windows has been destroyed,
       * we don't have much of a chance here and need to unrealize
       * the whole tree.
       */
  
      gdk_error_trap_push ();

      gtk_widget_hide (widget); /* only invisble toplevels can be unrealized */
      gtk_widget_unrealize (widget);

      /* flush the X queue, so pending GdkWindow operations that can
       * produce BadDrawable errors are caught
       */
      gdk_sync ();
      gdk_error_trap_pop ();
    }
    
  return FALSE;
}

this could in theory work, since unrealizing the plug widget will unrealize
all of its children, and further redraw requests will be discarded at the
gtk level.
of course gtkplug.c doesn't currently come with an unrealize handler, but
that is a bug in its own right that has to be fixed.
it'd be nice if you could check this out Nat (you only need to add the
unrealize handler which will destroy widget->window and plug->socket_window).

and eventhough that is not vital for your case, this code portion in
gtkmain.c should also be adapted:
    case GDK_DELETE:
      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_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);
      break;

in a way that GDK_DESTROY behaves exactly like GDK_DELETE, so components can
have a means to recover by returning TRUE from destroy_event, though
that would change existing behaviour i doubt, tehre's actually code out there
taht would trigger this (and then again, i've never been much in favour of
that unconditioned widget destruction).

>     It would be nice if this patch could make it into the Gtk 1.2
> branch.  It doesn't affect Gtk's behavior under normal circumstances,
> and makes it deal infinitely better with the pathological case I've
> outlined above.  So consider this my obsequious appeal that this patch
> be applied to Gtk 1.2.

if that will enable bonobo to work with gtk 1.2, that would be great.
since the stable gtk line can actually not change function prototypes
without breaking binary compatibility, we'll have to slightly change
that code though. i'm introducing gdk_window_destroy_notify_new()
here, in 1.3 we can rename that back to gdk_window_destroy_notify()
then.

> 
> Regards,
> Nat (nat@nat.org)
> 
> Index: gdk/gdkevents.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk/gdkevents.c,v
> retrieving revision 1.20.2.5
> diff -u -r1.20.2.5 gdkevents.c
> --- gdk/gdkevents.c	1999/06/25 23:23:04	1.20.2.5
> +++ gdk/gdkevents.c	1999/08/19 00:47:51
> @@ -1622,7 +1622,8 @@
>        return_val = window_private && !window_private->destroyed;
>        
>        if(window && window_private->xwindow != GDK_ROOT_WINDOW())
           gdk_window_destroy_notify_new (window, event);
/*
> -	gdk_window_destroy_notify (window);
> +	gdk_window_destroy_notify (window, event);
> +
*/

>        break;
>        
>      case UnmapNotify:
> Index: gdk/gdkprivate.h
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk/gdkprivate.h,v
> retrieving revision 1.36
> diff -u -r1.36 gdkprivate.h
> --- gdk/gdkprivate.h	1999/02/24 07:33:09	1.36
> +++ gdk/gdkprivate.h	1999/08/19 00:47:51
> @@ -240,7 +240,7 @@
>  GdkVisual*   gdk_visual_lookup	 (Visual   *xvisual);
>  
>  void gdk_window_add_colormap_windows (GdkWindow *window);
   void gdk_window_destroy_notify	     (GdkWindow *window);
   void gdk_window_destroy_notify_new	     (GdkWindow *window,
                                              GdkEvent  *event);
/*
> -void gdk_window_destroy_notify	     (GdkWindow *window);
> +void gdk_window_destroy_notify	     (GdkWindow *window, GdkEvent *event);
*/

>  
>  void	 gdk_xid_table_insert (XID	*xid,
> 			       gpointer	 data);
> Index: gdk/gdkwindow.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gdk/gdkwindow.c,v
> retrieving revision 1.80.2.4
> diff -u -r1.80.2.4 gdkwindow.c
> --- gdk/gdkwindow.c	1999/05/06 03:12:08	1.80.2.4
> +++ gdk/gdkwindow.c	1999/08/19 00:47:57
> @@ -689,7 +689,7 @@
>  /* This function is called when the XWindow is really gone.  */
>

void
gdk_window_destroy_notify (GdkWindow *window)
{
  gdk_window_destroy_notify_new (window, NULL);
}

/*
>  void
> -gdk_window_destroy_notify (GdkWindow *window)
> +gdk_window_destroy_notify (GdkWindow *window, GdkEvent *event)
*/

void
gdk_window_destroy_notify_new (GdkWindow *window,
                               GdkEvent  *event)
>  {
>    GdkWindowPrivate *private;
>    
> @@ -701,8 +701,33 @@
>      {
>        if (private->window_type == GDK_WINDOW_FOREIGN)
> 	gdk_window_internal_destroy (window, FALSE, FALSE);
> -      else
> +      else {
> +	GdkWindowPrivate *parent;
> +
> 	g_warning ("GdkWindow %#lx unexpectedly destroyed", private->xwindow);

hm since this will become a valid code path for bonobo then, wouldn't it make
sense to actually take that warning out (it could be left as a debug option
though) and actually invoke gdk_window_internal_destroy (window, FALSE, FALSE)
here as well? especially since Gdk will then discard further operations on the
window as well (we got checks all over the place for window_private->destroyed).
owen?

> +
> +	/*
> +	 * A window has been unexpectedly destroyed by the X server.
> +	 *
> +	 * In the case where a foreign, reparented window (such as is
> +	 * used by GtkPlug), the X server will recursively destroy
> +	 * all of that window's subwindows.  In what is effectively
> +	 * a random order.  So what we do here is to walk up the
> +	 * window tree until we encounter the first foreign window,
> +	 * and rewrite the event to occur on that window.  That way,
> +	 * the first widget to be destroyed will be the GtkPlug,
> +	 * and its subwidgets can be killed politely.
> +	 *
> +	 * If there is no foreign ancestral window, we don't rewrite
> +	 * the event.
> +	 */
> +	parent = private;
> +	while (parent != NULL && parent->window_type != GDK_WINDOW_FOREIGN)
> +	  parent = parent->parent;
> +
> +	if (parent != NULL)
> +	  event->any.window = (GdkWindow *) parent;
> +      }
>      }
>    
>    gdk_xid_table_remove (private->xwindow);
> cvs server: Diffing gtk
> 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/08/19 00:48:03
> @@ -105,6 +105,7 @@
>    if (plug->socket_window == NULL)
>      {
>        plug->socket_window = gdk_window_foreign_new (socket_id);
> +      gdk_window_set_user_data (plug->socket_window, plug);
>        plug->same_app = FALSE;
>      }
>  }
> Index: gtk/gtksocket.c
> ===================================================================
> RCS file: /cvs/gnome/gtk+/gtk/gtksocket.c,v
> retrieving revision 1.8.2.1
> diff -u -r1.8.2.1 gtksocket.c
> --- gtk/gtksocket.c	1999/07/23 00:36:33	1.8.2.1
> +++ gtk/gtksocket.c	1999/08/19 00:48:06
> @@ -670,7 +670,7 @@
> 	    toplevel = gtk_widget_get_toplevel (GTK_WIDGET (socket));
> 	    if (toplevel && GTK_IS_WINDOW (toplevel))
> 	      gtk_window_remove_embedded_xid (GTK_WINDOW (toplevel), xdwe->window);

	    gdk_window_destroy_notify_new (socket->plug_window, event);

/*
> -	    gdk_window_destroy_notify (socket->plug_window);
> +	    gdk_window_destroy_notify (socket->plug_window, event);
*/

> 	    gtk_widget_destroy (widget);
>  
> 	    socket->plug_window = NULL;
> 
> 
> -- 
>          To unsubscribe: mail gtk-devel-list-request@redhat.com with 
>                        "unsubscribe" as the Subject.
> 
> 

---
ciaoTJ