[g-a-devel]DeviceEventController fixage ...



Hi Bill,

	Ok - so, now I seem to understand this fairly well - and the (improved)
regression tests pass nicely as well.

	So this patch:

		* Fixes the known re-enterancy issues notifying key
		  listeners / removing them.
		* Implements global key grab removal.
		* reduces the X Grab / Ungrab traffic & kills a race
		  there.
		* kills the 'timeout' inefficiency ( and race source,
		  causing my test to fail previously ).
		* substantially cleans the code, and makes it conform
		  to the coding convention.
		* renames to SpiDEController which is more readable, as
		  agreed.

	I'd be interested to see if it passes whatever you use for keyboard
testing - I havn't tried simple-at since I forget the keystrokes; if not
I'm most interested in fixing it.

	If it passes, I'd be very inclined to ship this on Monday, since it
fixes known bugs, and improves completeness, and is regression tested.

	Regards,

		Michael.

? =
? billh
? docs/reference/cspi/tmpl/at-spi-cspi-unused.sgml
? test/mag.sh
Index: ChangeLog
===================================================================
RCS file: /cvs/gnome/at-spi/ChangeLog,v
retrieving revision 1.142
diff -u -p -u -r1.142 ChangeLog
--- ChangeLog	2002/01/11 12:10:00	1.142
+++ ChangeLog	2002/01/11 16:13:05
@@ -1,5 +1,90 @@
 2002-01-11  Michael Meeks  <michael ximian com>
 
+	* registryd/deviceeventcontroller.c
+	(spi_device_event_controller_forward_key_event): kill
+	XUngrabKey / XKeyGrab race.
+	(spi_controller_grab_keyboard): rename to
+	(spi_controller_update_key_grabs): this, and deal
+	with incremental adding / removing grabs more
+	sensibly.
+	(_register_keygrab): ensure we're not pending a remove.
+	(spi_grab_mask_free): impl.
+	(spi_controller_register_global_keygrabs): split out
+	common code into:
+	(handle_keygrab): impl.
+	(_deregister_keygrab): impl.
+	(spi_controller_deregister_global_keygrabs): impl.
+	(spi_controller_update_key_grabs): re-issue the grab if
+	we just recieved a notification.
+
+	* test/test-simple.c (key_listener_cb): remove debug.
+
+	* registryd/deviceeventcontroller.c
+	(spi_controller_register_device_listener): after
+	registering a global keygrab, actualy register it !
+	don't wait for a timeout; doh !
+
+	* registryd/deviceeventcontroller.[ch]: s/DeviceEvent/DE/
+	to make it more readable / manipulable.
+	s/grabmask/grab_mask/ s/refcount/ref_count/
+	s/keyval/key_val/ s/modmask/mod_mask
+
+2002-01-08  Michael Meeks  <michael ximian com>
+
+	* registryd/deviceeventcontroller.c
+	(spi_controller_register_with_devices): use gdk calls to
+	setup a filter.
+	(global_filter_fn): implement the filter.
+	(spi_device_event_controller_check_key_event): rename to
+	(spi_device_event_controller_forward_key_event): this & upd.
+	(spi_get_display): replace with GDK_DISPLAY.
+
+	* registryd/deviceeventcontroller.c
+	(spi_controller_deregister_device_listener): unroll into
+	(impl_deregister_keystroke_listener): here to simplify.
+	(spi_controller_register_global_keygrabs): split cut and
+	paste (!) out into (_register_keygrab): here, shorter & sweeter.
+	(spi_controller_deregister_device_listener): remove.
+	(impl_register_mouse_listener): remove, no mouse listener
+	support in at-spi-1.0
+
+	* registryd/registry.c
+	(_device_event_controller_hook): kill.
+	(spi_registry_init): upd.
+
+	* registryd/deviceeventcontroller.c
+	(spi_device_event_controller_class_init): upd.
+	(spi_check_key_event): merge into.
+	(spi_device_event_controller_check_key_event):
+	here and kill strange static ev init, don't leak
+	the x_event - nor dynamicaly allocate it.
+
+	* registryd/registry-main.c (main): re-direct
+	timeout to remove strange vtable mess.
+
+	* registryd/deviceeventcontroller.c
+	(remove_listener_cb): impl.
+	(spi_controller_deregister_device_listener):
+	fix re-enterancy hazard.
+
+2002-01-07  Michael Meeks  <michael ximian com>
+
+	* registryd/deviceeventcontroller.c
+	(spi_device_event_controller_new): upd.
+	(impl_notify_listeners_sync): upd. debug.
+	(spi_notify_keylisteners): fix re-enterancy hazards,
+	prettify, remove O(n*n) iteration.
+	(spi_controller_grab_keyboard): fix iteration.
+	(spi_check_key_event): re-format to suit coding style.
+	Clean all the warnings - we're warning free.
+
+	* registryd/deviceeventcontroller.h:
+	* registryd/registry.h: make mutualy referential with
+	typesafe forward references instead of (!) void pointer
+	hacks.
+
+2002-01-11  Michael Meeks  <michael ximian com>
+
 	* cspi/spi_accessible.c (role_names): add a role name
 	to sync this array with the enum; and make the regression
 	tests pass, sigh.
Index: cspi/spi_registry.c
===================================================================
RCS file: /cvs/gnome/at-spi/cspi/spi_registry.c,v
retrieving revision 1.30
diff -u -p -u -r1.30 spi_registry.c
--- cspi/spi_registry.c	2002/01/11 12:00:45	1.30
+++ cspi/spi_registry.c	2002/01/11 16:13:07
@@ -310,10 +310,10 @@ SPI_freeDesktopList (Accessible **deskto
  * Returns: #TRUE if successful, otherwise #FALSE.
  **/
 SPIBoolean
-SPI_registerAccessibleKeystrokeListener (AccessibleKeystrokeListener *listener,
-					 AccessibleKeySet *keys,
-					 AccessibleKeyMaskType modmask,
-					 AccessibleKeyEventMask eventmask,
+SPI_registerAccessibleKeystrokeListener (AccessibleKeystrokeListener  *listener,
+					 AccessibleKeySet             *keys,
+					 AccessibleKeyMaskType         modmask,
+					 AccessibleKeyEventMask        eventmask,
 					 AccessibleKeyListenerSyncType sync_type)
 {
   gint                                i, mask;
Index: idl/Accessibility_Registry.idl
===================================================================
RCS file: /cvs/gnome/at-spi/idl/Accessibility_Registry.idl,v
retrieving revision 1.16
diff -u -p -u -r1.16 Accessibility_Registry.idl
--- idl/Accessibility_Registry.idl	2002/01/01 22:35:29	1.16
+++ idl/Accessibility_Registry.idl	2002/01/11 16:13:08
@@ -66,7 +66,7 @@ module Accessibility {
          *
 	 **/
         void registerGlobalEventListener (in EventListener listener,
-                                           in string eventName);
+					  in string eventName);
 
   	/**
          * deregisterGlobalEventListener:
Index: registryd/deviceeventcontroller.c
===================================================================
RCS file: /cvs/gnome/at-spi/registryd/deviceeventcontroller.c,v
retrieving revision 1.27
diff -u -p -u -r1.27 deviceeventcontroller.c
--- registryd/deviceeventcontroller.c	2002/01/08 09:11:46	1.27
+++ registryd/deviceeventcontroller.c	2002/01/11 16:13:10
@@ -26,17 +26,21 @@
 
 #undef SPI_DEBUG
 
-#ifdef SPI_DEBUG
-#  include <stdio.h>
-#endif
+#include <string.h>
+#include <ctype.h>
+#include <stdio.h>
+#include <bonobo/bonobo-exception.h>
 
 #include <X11/Xlib.h>
 #include <X11/extensions/XTest.h>
 #define XK_MISCELLANY
 #include <X11/keysymdef.h>
+#include <gdk/gdk.h>
 #include <gdk/gdkx.h> /* TODO: hide dependency (wrap in single porting file) */
+#include <gdk/gdkkeysyms.h>
 #include <gdk/gdkwindow.h>
 
+#include "../libspi/spi-private.h"
 #include "deviceeventcontroller.h"
 
 /* Our parent Gtk object type */
@@ -45,124 +49,94 @@
 /* A pointer to our parent object class */
 static GObjectClass *spi_device_event_controller_parent_class;
 
-static gboolean kbd_registered = FALSE;
-
-static Window root_window;
-
 typedef enum {
   SPI_DEVICE_TYPE_KBD,
-  SPI_DEVICE_TYPE_MOUSE,
   SPI_DEVICE_TYPE_LAST_DEFINED
 } SpiDeviceTypeCategory;
 
-struct _DEControllerGrabMask {
-  Accessibility_ControllerEventMask modmask;
-  CORBA_unsigned_long               keyval;
-  unsigned int                      refcount;
-};
+typedef struct {
+  guint                             ref_count : 30;
+  guint                             pending_add : 1;
+  guint                             pending_remove : 1;
+
+  Accessibility_ControllerEventMask mod_mask;
+  CORBA_unsigned_long               key_val;  /* KeyCode */
+} DEControllerGrabMask;
 
-typedef struct _DEControllerGrabMask DEControllerGrabMask;
-
-struct _DEControllerListener {
+typedef struct {
   CORBA_Object          object;
   SpiDeviceTypeCategory type;
-};
-
-typedef struct _DEControllerListener DEControllerListener;
+} DEControllerListener;
 
-struct _DEControllerKeyListener {
+typedef struct {
   DEControllerListener listener;
-  Accessibility_KeySet *keys;
+
+  Accessibility_KeySet             *keys;
   Accessibility_ControllerEventMask mask;
-  Accessibility_KeyEventTypeSeq *typeseq;
-  Accessibility_EventListenerMode *mode;	
-};
-
-typedef struct _DEControllerKeyListener DEControllerKeyListener;
-
-static gboolean spi_controller_register_with_devices (SpiDeviceEventController *controller);
-static gboolean spi_controller_grab_keyboard (SpiDeviceEventController *controller);
-
-static void spi_controller_register_device_listener (SpiDeviceEventController *controller,
-						     DEControllerListener *l,
-						     CORBA_Environment *ev);
+  Accessibility_KeyEventTypeSeq    *typeseq;
+  Accessibility_EventListenerMode  *mode;	
+} DEControllerKeyListener;
+
+static void     spi_controller_register_with_devices          (SpiDEController           *controller);
+static gboolean spi_controller_update_key_grabs               (SpiDEController           *controller,
+							       Accessibility_DeviceEvent *recv);
+static void     spi_controller_register_device_listener       (SpiDEController           *controller,
+							       DEControllerListener      *l,
+							       CORBA_Environment         *ev);
+static void     spi_device_event_controller_forward_key_event (SpiDEController           *controller,
+							       const XEvent              *event);
 
-/*
- * Private methods
- */
+/* Private methods */
 
-static Display *
-spi_get_display (void )
+static KeyCode
+keycode_for_keysym (long keysym)
 {
- static Display *display = NULL;
- /* We must open a new connection to the server to avoid clashing with the GDK event loop */
- /*
-  * TODO: fixme, this makes the foolish assumption that registryd uses
-  * the same display as the apps, and the the DISPLAY environment variable is set.
-  */
- 
- if (!display)
-   {
-     display = XOpenDisplay (g_getenv ("DISPLAY"));
-   }
- return display;
+  return XKeysymToKeycode (GDK_DISPLAY (), (KeySym) keysym);
 }
 
 static DEControllerGrabMask *
-spi_grabmask_clone (DEControllerGrabMask *grabmask)
+spi_grab_mask_clone (DEControllerGrabMask *grab_mask)
 {
-  DEControllerGrabMask *clone = g_new0 (DEControllerGrabMask, 1);
-  memcpy (clone, grabmask, sizeof (DEControllerGrabMask));
-  return clone;
-}
+  DEControllerGrabMask *clone = g_new (DEControllerGrabMask, 1);
 
-static gint
-spi_compare_corba_objects (gconstpointer p1, gconstpointer p2)
-{
-  CORBA_Environment ev;
-  gint retval;
-  retval = !CORBA_Object_is_equivalent ((CORBA_Object) p1, (CORBA_Object) p2, &ev);
+  memcpy (clone, grab_mask, sizeof (DEControllerGrabMask));
 
-#ifdef SPI_DEBUG
-  fprintf (stderr, "comparing %p to %p; result %d\n",
-	   p1, p2,
-	   retval);
-#endif
-  return retval;  
+  clone->ref_count = 1;
+  clone->pending_add = TRUE;
+  clone->pending_remove = FALSE;
+
+  return clone;
 }
 
-static gint
-spi_compare_listeners (gconstpointer p1, gconstpointer p2)
+static void
+spi_grab_mask_free (DEControllerGrabMask *grab_mask)
 {
-  DEControllerListener *l1 = (DEControllerListener *) p1;	
-  DEControllerListener *l2 = (DEControllerListener *) p2;	
-  return spi_compare_corba_objects (l1->object, l2->object);
+  g_free (grab_mask);
 }
 
 static gint
-spi_grabmask_compare_values (gconstpointer p1, gconstpointer p2)
+spi_grab_mask_compare_values (gconstpointer p1, gconstpointer p2)
 {
-  DEControllerGrabMask *l1;
-  DEControllerGrabMask *l2;
+  DEControllerGrabMask *l1 = (DEControllerGrabMask *) p1;
+  DEControllerGrabMask *l2 = (DEControllerGrabMask *) p2;
+
   if (p1 == p2)
     {
       return 0;
     }
   else
     { 
-      l1 = (DEControllerGrabMask *) p1;	
-      l2 = (DEControllerGrabMask *) p2;
-      return ((l1->modmask != l2->modmask) || (l1->keyval != l2->keyval));
+      return ((l1->mod_mask != l2->mod_mask) || (l1->key_val != l2->key_val));
     }
 }
 
 static DEControllerKeyListener *
-spi_dec_key_listener_new (CORBA_Object l,
-			  const Accessibility_KeySet *keys,
+spi_dec_key_listener_new (CORBA_Object                            l,
+			  const Accessibility_KeySet             *keys,
 			  const Accessibility_ControllerEventMask mask,
-			  const Accessibility_KeyEventTypeSeq *typeseq,
-			  const Accessibility_EventListenerMode *mode,
-			  CORBA_Environment *ev)
+			  const Accessibility_KeyEventTypeSeq    *typeseq,
+			  const Accessibility_EventListenerMode  *mode,
+			  CORBA_Environment                      *ev)
 {
   DEControllerKeyListener *key_listener = g_new0 (DEControllerKeyListener, 1);
   key_listener->listener.object = bonobo_object_dup_ref (l, ev);
@@ -176,16 +150,19 @@ spi_dec_key_listener_new (CORBA_Object l
     key_listener->mode = NULL;
 
 #ifdef SPI_DEBUG
-  g_print ("new listener, with mask %x, is_global %d, keys %p\n",
+  g_print ("new listener, with mask %x, is_global %d, keys %p (%d)\n",
 	   (unsigned int) key_listener->mask,
-           (int) mode->global,
-	   (void *) key_listener->keys);
+           (int) (mode ? mode->global : 0),
+	   (void *) key_listener->keys,
+	   (int) (key_listener->keys ? key_listener->keys->_length : 0));
 #endif
+
   return key_listener;	
 }
 
 static void
-spi_dec_key_listener_free (DEControllerKeyListener *key_listener, CORBA_Environment *ev)
+spi_dec_key_listener_free (DEControllerKeyListener *key_listener,
+			   CORBA_Environment       *ev)
 {
   bonobo_object_release_unref (key_listener->listener.object, ev);
   CORBA_free (key_listener->typeseq);
@@ -194,156 +171,173 @@ spi_dec_key_listener_free (DEControllerK
 }
 
 static void
-spi_controller_deregister_global_keygrabs (SpiDeviceEventController *controller,
-					   DEControllerKeyListener *key_listener)
+_register_keygrab (SpiDEController      *controller,
+		   DEControllerGrabMask *grab_mask)
+{
+  GList *l;
+
+  l = g_list_find_custom (controller->keygrabs_list, grab_mask,
+			  spi_grab_mask_compare_values);
+  if (l)
+    {
+      DEControllerGrabMask *cur_mask = l->data;
+
+      cur_mask->ref_count++;
+      if (cur_mask->pending_remove)
+        {
+          cur_mask->pending_remove = FALSE;
+	}
+    }
+  else
+    {
+      controller->keygrabs_list =
+        g_list_prepend (controller->keygrabs_list,
+			spi_grab_mask_clone (grab_mask));
+    }
+}
+
+static void
+_deregister_keygrab (SpiDEController      *controller,
+		     DEControllerGrabMask *grab_mask)
 {
-  GList *list_ptr;
-  DEControllerGrabMask *mask_ptr;
-  /* TODO: implement this! Also remember to release any keygrabs still held */
-  ;
+  GList *l;
+
+  l = g_list_find_custom (controller->keygrabs_list, grab_mask,
+			  spi_grab_mask_compare_values);
+
+  if (l)
+    {
+      DEControllerGrabMask *cur_mask = l->data;
+
+      cur_mask->ref_count--;
+      cur_mask->pending_remove = TRUE;
+    }
+  else
+    {
+      g_warning ("De-registering non-existant grab");
+    }
 }
 
 static void
-spi_controller_register_global_keygrabs (SpiDeviceEventController *controller,
-					 DEControllerKeyListener *key_listener)
+handle_keygrab (SpiDEController         *controller,
+		DEControllerKeyListener *key_listener,
+		void                   (*process_cb) (SpiDEController *controller,
+						      DEControllerGrabMask *grab_mask))
 {
-  DEControllerGrabMask grabmask, *grabmask_ptr;
-  GList *list_ptr;
-  gint i;
-  /* TODO: deregistration version of this function */
-  
-  grabmask.modmask = key_listener->mask;
+  DEControllerGrabMask grab_mask = { 0 };
+
+  grab_mask.mod_mask = key_listener->mask;
   if (key_listener->keys->_length == 0) /* special case means AnyKey/AllKeys */
     {
-      grabmask.keyval = AnyKey;
-      list_ptr = g_list_find_custom (controller->keygrabs_list, &grabmask,
-				     spi_grabmask_compare_values);
-      if (list_ptr)
-        {
-          grabmask_ptr = (DEControllerGrabMask *) list_ptr->data;
-	  grabmask_ptr->refcount++;
-        }
-      else
-        {
-	  controller->keygrabs_list =
-		  g_list_prepend (controller->keygrabs_list,
-				  spi_grabmask_clone (&grabmask));
-        }
+      grab_mask.key_val = AnyKey;
+      process_cb (controller, &grab_mask);
     }
   else
     {
+      int i;
+
       for (i = 0; i < key_listener->keys->_length; ++i)
         {
-	  long int keyval = key_listener->keys->_buffer[i];
+	  long int key_val = key_listener->keys->_buffer[i];
 	  /* X Grabs require keycodes, not keysyms */
-	  if (keyval >= 0)
+	  if (key_val >= 0)
 	    {
-	      keyval = XKeysymToKeycode(spi_get_display (), (KeySym) keyval);		    
+	      key_val = XKeysymToKeycode (GDK_DISPLAY (), (KeySym) key_val);
 	    }
-	  grabmask.keyval = keyval;
-          list_ptr = g_list_find_custom (controller->keygrabs_list, &grabmask,
-					     spi_grabmask_compare_values);
-          if (list_ptr)
-            {
-	      grabmask_ptr = (DEControllerGrabMask *) list_ptr->data;
-              grabmask_ptr->refcount++;
-            }
-          else
-            {
-	      controller->keygrabs_list =
-		  g_list_prepend (controller->keygrabs_list,
-				  spi_grabmask_clone (&grabmask));
-	      fprintf (stderr, "appending mask with val=%lu\n",
-		       (unsigned long) grabmask.modmask);
-            }
-        }
+	  grab_mask.key_val = key_val;
+
+	  process_cb (controller, &grab_mask);
+	}
     }
 }
 
 static void
-spi_controller_register_device_listener (SpiDeviceEventController *controller,
+spi_controller_register_global_keygrabs (SpiDEController         *controller,
+					 DEControllerKeyListener *key_listener)
+{
+  handle_keygrab (controller, key_listener, _register_keygrab);
+  spi_controller_update_key_grabs (controller, NULL);
+}
+
+static void
+spi_controller_deregister_global_keygrabs (SpiDEController         *controller,
+					   DEControllerKeyListener *key_listener)
+{
+  handle_keygrab (controller, key_listener, _deregister_keygrab);
+  spi_controller_update_key_grabs (controller, NULL);
+}
+
+static void
+spi_controller_register_device_listener (SpiDEController      *controller,
 					 DEControllerListener *listener,
-					 CORBA_Environment *ev)
+					 CORBA_Environment    *ev)
 {
   DEControllerKeyListener *key_listener;
   
   switch (listener->type) {
   case SPI_DEVICE_TYPE_KBD:
-      key_listener = (DEControllerKeyListener *) listener;  	  
-      controller->key_listeners = g_list_prepend (controller->key_listeners, key_listener);
+      key_listener = (DEControllerKeyListener *) listener;
+
+      controller->key_listeners = g_list_prepend (controller->key_listeners,
+						  key_listener);
       if (key_listener->mode->global)
         {
 	  spi_controller_register_global_keygrabs (controller, key_listener);	
 	}
       break;
-  case SPI_DEVICE_TYPE_MOUSE:
-/*    controller->mouse_listeners = g_list_append (controller->mouse_listeners,
-                                                   CORBA_Object_duplicate (l, ev));*/
-
-/* this interface should only be used for mouse motion events, not mouse clicks events */
+    default:
       break;
   }
 }
 
-static void
-spi_controller_deregister_device_listener (SpiDeviceEventController *controller,
-					   DEControllerListener *listener,
-					   CORBA_Environment *ev)
-{
-  Accessibility_ControllerEventMask *mask_ptr;
-  DEControllerListener *dec_listener;
-  GList *list_ptr;
-  switch (listener->type)
-    {
-      case SPI_DEVICE_TYPE_KBD:
-        spi_controller_deregister_global_keygrabs (controller,
-						   (DEControllerKeyListener *) listener);
-
-        /* now, remove this listener from the keylistener list */
-        list_ptr = g_list_find_custom (controller->key_listeners, listener, spi_compare_listeners);
-        if (list_ptr)
-          {
-	    dec_listener = (DEControllerListener *) list_ptr->data;
-#ifdef SPI_DEBUG	  
-	    g_print ("removing keylistener %p\n", dec_listener->object);
-#endif
-	    controller->key_listeners = g_list_remove_link (controller->key_listeners,
-							  list_ptr);
-	    spi_dec_key_listener_free ((DEControllerKeyListener *) dec_listener, ev);
-	  }
-        break;
-      case SPI_DEVICE_TYPE_MOUSE: /* TODO: implement */
-        break;
+static GdkFilterReturn
+global_filter_fn (GdkXEvent *gdk_xevent, GdkEvent *event, gpointer data)
+{
+  XEvent *xevent = gdk_xevent;
+  SpiDEController *controller;
+
+  if (xevent->type != KeyPress && xevent->type != KeyRelease)
+    {
+      return GDK_FILTER_CONTINUE;
     }
+
+  controller = SPI_DEVICE_EVENT_CONTROLLER (data);
+
+  spi_device_event_controller_forward_key_event (controller, xevent);
+
+  /* FIXME: is this right ? */
+  return GDK_FILTER_CONTINUE;
 }
 
-static gboolean
-spi_controller_register_with_devices (SpiDeviceEventController *controller)
+static void
+spi_controller_register_with_devices (SpiDEController *controller)
 {
-  gboolean retval = FALSE;
-
   /* calls to device-specific implementations and routines go here */
   /* register with: keyboard hardware code handler */
   /* register with: (translated) keystroke handler */
+
+  gdk_window_add_filter (NULL, global_filter_fn, controller);
 
-  /* We must open a new connection to the server to avoid clashing with the GDK event loop */
-  root_window = DefaultRootWindow (spi_get_display ());		
-  XSelectInput (spi_get_display (),
-		root_window,
+  gdk_window_set_events (gdk_get_default_root_window (),
+			 GDK_KEY_PRESS_MASK | GDK_KEY_RELEASE_MASK);
+
+  XSelectInput (GDK_DISPLAY (),
+		DefaultRootWindow (GDK_DISPLAY ()),
 		KeyPressMask | KeyReleaseMask);
-  /* register with: mouse hardware device handler? */
-  /* register with: mouse event handler */
-  return retval;
 }
 
 static gboolean
-spi_key_set_contains_key (Accessibility_KeySet *key_set, const Accessibility_DeviceEvent *key_event)
+spi_key_set_contains_key (Accessibility_KeySet            *key_set,
+			  const Accessibility_DeviceEvent *key_event)
 {
   gint i;
   gint len;
 
-  /* g_assert (key_set); */
-  if (!key_set) { g_print ("null key set!"); return TRUE; }
+  if (!key_set)
+    {
+      g_print ("null key set!");
+      return TRUE;
+    }
 
   len = key_set->_length;
   
@@ -353,17 +347,21 @@ spi_key_set_contains_key (Accessibility_
       return TRUE;
     }
 
-  for (i=0; i<len; ++i)
+  for (i = 0; i < len; ++i)
     {
 #ifdef SPI_KEYEVENT_DEBUG	    
       g_print ("key_set[%d] = %d; key_event %d, code %d\n",
-	        i,
-	       (int) key_set->_buffer[i],
-	       (int) key_event->id,
-	       (int) key_event->hw_code); 
+	        i, (int) key_set->_buffer[i],
+	       (int) key_event->id, (int) key_event->hw_code); 
 #endif
-      if (key_set->_buffer[i] == (CORBA_long) key_event->id) return TRUE;
-      if (key_set->_buffer[i] == (CORBA_long) -key_event->hw_code) return TRUE;
+      if (key_set->_buffer[i] == (CORBA_long) key_event->id)
+        {
+          return TRUE;
+	}
+      if (key_set->_buffer[i] == (CORBA_long) -key_event->hw_code)
+        {
+          return TRUE;
+	}
     }
   
   return FALSE;
@@ -376,9 +374,13 @@ spi_key_eventtype_seq_contains_event (Ac
   gint i;
   gint len;
 
-  /* g_assert (type_seq); */
-  if (!type_seq) { g_print ("null type seq!"); return TRUE; }
 
+  if (!type_seq)
+    {
+      g_print ("null type seq!");
+      return TRUE;
+    }
+
   len = type_seq->_length;
   
   if (len == 0) /* special case, means "all events/any event" */
@@ -386,13 +388,16 @@ spi_key_eventtype_seq_contains_event (Ac
       return TRUE;
     }
 
-  for (i=0; i<len; ++i)
+  for (i = 0; i < len; ++i)
     {
 #ifdef SPI_DEBUG	    
-      g_print ("type_seq[%d] = %d; key event type = %d\n", i, (int) type_seq->_buffer[i],
-	       (int) key_event->type);
+      g_print ("type_seq[%d] = %d; key event type = %d\n", i,
+	       (int) type_seq->_buffer[i], (int) key_event->type);
 #endif      
-      if (type_seq->_buffer[i] == (CORBA_long) key_event->type) return TRUE;	    
+      if (type_seq->_buffer[i] == (CORBA_long) key_event->type)
+        {
+          return TRUE;
+	}
     }
   
   return FALSE;
@@ -403,7 +408,6 @@ spi_key_event_matches_listener (const Ac
 				DEControllerKeyListener         *listener,
 				CORBA_boolean                    is_system_global)
 {
-  g_print ("checking keycode %d\n", (int) key_event->hw_code);
   if ((key_event->modifiers == (CORBA_unsigned_short) (listener->mask & 0xFFFF)) &&
        spi_key_set_contains_key (listener->keys, key_event) &&
        spi_key_eventtype_seq_contains_event (listener->typeseq, key_event) && 
@@ -412,39 +416,71 @@ spi_key_event_matches_listener (const Ac
       return TRUE;
     }
   else
-    return FALSE;
+    {
+      return FALSE;
+    }
 }
 
 static gboolean
-spi_notify_keylisteners (GList *key_listeners,
+spi_notify_keylisteners (GList                          **key_listeners,
 			 const Accessibility_DeviceEvent *key_event,
-			 CORBA_boolean is_system_global,
-			 CORBA_Environment *ev)
+			 CORBA_boolean                    is_system_global,
+			 CORBA_Environment               *ev)
 {
-  int i, n_listeners = g_list_length (key_listeners);
-  gboolean is_consumed = FALSE;
+  GList   *l;
+  GSList  *notify = NULL, *l2;
+  gboolean is_consumed;
 
-  for (i=0; i<n_listeners && !is_consumed; ++i)
+  if (!key_listeners)
     {
-      Accessibility_DeviceEventListener ls;
-      DEControllerKeyListener *key_listener = (DEControllerKeyListener *)
-	    g_list_nth_data (key_listeners, i);
-      ls = (Accessibility_DeviceEventListener) key_listener->listener.object;
-      if (spi_key_event_matches_listener (key_event, key_listener, is_system_global))
-        {
-          if (!CORBA_Object_is_nil(ls, ev))
-            {
-	      is_consumed = Accessibility_DeviceEventListener_notifyEvent (ls, key_event, ev);
-            }		
-        }
-      else
-        {
+      return FALSE;
+    }
+
+  for (l = *key_listeners; l; l = l->next)
+    {
+       DEControllerKeyListener *key_listener = l->data;
+
+       if (spi_key_event_matches_listener (key_event, key_listener, is_system_global))
+         {
+           Accessibility_DeviceEventListener ls = key_listener->listener.object;
+
+	   if (ls != CORBA_OBJECT_NIL)
+	     {
+               notify = g_slist_prepend (notify, CORBA_Object_duplicate (ls, ev));
+	     }
+         }
+    }
+
 #ifdef SPI_KEYEVENT_DEBUG
-	      g_print ("no match for listener %d\n", i);
+  if (!notify)
+    {
+      g_print ("no match for listener %d\n", i);
+    }
 #endif
-	      ;
-	}
+
+  is_consumed = FALSE;
+  for (l2 = notify; l2 && !is_consumed; l2 = l2->next)
+    {
+      Accessibility_DeviceEventListener ls = l2->data;
+
+      is_consumed = Accessibility_DeviceEventListener_notifyEvent (ls, key_event, ev);
+
+      if (BONOBO_EX (ev))
+        {
+          is_consumed = FALSE;
+	  CORBA_exception_free (ev);
+        }
+
+      CORBA_Object_release (ls, ev);
+    }
+
+  for (; l2; l2 = l2->next)
+    {
+      CORBA_Object_release (l2->data, ev);
     }
+
+  g_slist_free (notify);
+  
   return is_consumed;
 }
 
@@ -515,95 +551,86 @@ spi_keystroke_from_x_key_event (XKeyEven
   return key_event;	
 }
 
-
 static gboolean
-spi_check_key_event (SpiDeviceEventController *controller)
+spi_controller_update_key_grabs (SpiDEController           *controller,
+				 Accessibility_DeviceEvent *recv)
 {
-	static gboolean initialized = FALSE;
-	XEvent *x_event = g_new0 (XEvent, 1);
-	XKeyEvent *x_key_event;
-	gboolean is_consumed = FALSE;
-	Accessibility_DeviceEvent key_event;
-	static CORBA_Environment ev;
-
-	if (!initialized)
-	{
-	  initialized = TRUE;
-	  CORBA_exception_init (&ev);
-	}
+  GList *l, *next;
 
-	while (XPending(spi_get_display ()))
-	  {
-	    XNextEvent (spi_get_display (), x_event);
-	    if (XFilterEvent (x_event, None)) continue;	  
-	    if (x_event->type == KeyPress || x_event->type == KeyRelease)
-	      {
-	        key_event = spi_keystroke_from_x_key_event ((XKeyEvent *) x_event);
-	        /* relay to listeners, and decide whether to consume it or not */
-	        is_consumed = spi_notify_keylisteners (controller->key_listeners, &key_event, CORBA_TRUE, &ev);
-	      }
-	    else
-	      {
-#ifdef SPI_KEYEVENT_DEBUG
-	        fprintf (stderr, "other event, type %d\n", (int) x_event->type);
-#endif
-	      }
+  g_return_val_if_fail (controller != NULL, FALSE);
 
-	    if (is_consumed)
-	      {
-	        XAllowEvents (spi_get_display (), AsyncKeyboard, CurrentTime);
-	      }
-	    else
-	      {
-	        XAllowEvents (spi_get_display (), ReplayKeyboard, CurrentTime);
-	      }
-	  }
-	XUngrabKey (spi_get_display (), AnyKey, AnyModifier, root_window);
+  /*
+   * masks known to work with default RH 7.1+:
+   * 0 (no mods), LockMask, Mod1Mask, Mod2Mask, ShiftMask,
+   * ShiftMask|LockMask, Mod1Mask|LockMask, Mod2Mask|LockMask,
+   * ShiftMask|Mod1Mask, ShiftMask|Mod2Mask, Mod1Mask|Mod2Mask,
+   * ShiftMask|LockMask|Mod1Mask, ShiftMask|LockMask|Mod2Mask,
+   *
+   * ControlMask grabs are broken, must be in use already
+   */
+  for (l = controller->keygrabs_list; l; l = next)
+    {
+      gboolean do_remove;
+      gboolean re_issue_grab;
+      DEControllerGrabMask *grab_mask = l->data;
 
-	return spi_controller_grab_keyboard (controller);
-}
+      next = l->next;
 
-static gboolean
-spi_controller_grab_keyboard (SpiDeviceEventController *controller)
-{
-  GList *maskList = controller->keygrabs_list;
-  int i;
-  int last_mask;
-  last_mask = g_list_length (maskList);
+      re_issue_grab = recv &&
+	      (recv->modifiers & grab_mask->mod_mask) &&
+	      (grab_mask->key_val == keycode_for_keysym (recv->id));
 
-/*
- * masks known to work with default RH 7.1: 
- * 0 (no mods), LockMask, Mod1Mask, Mod2Mask, ShiftMask,
- * ShiftMask|LockMask, Mod1Mask|LockMask, Mod2Mask|LockMask,
- * ShiftMask|Mod1Mask, ShiftMask|Mod2Mask, Mod1Mask|Mod2Mask,
- * ShiftMask|LockMask|Mod1Mask, ShiftMask|LockMask|Mod2Mask,
- *
- * ControlMask grabs are broken, must be in use already
- */
-	
-  for (i=0; i < last_mask; ++i)
-    {
-      DEControllerGrabMask * grab_mask
-		= (DEControllerGrabMask *) g_list_nth_data (maskList, i);
-      unsigned long maskVal = 0xFFFFFFFF;
-      int           keyVal = AnyKey;
-      if (grab_mask)
-        {
-	  maskVal = (unsigned long) grab_mask->modmask;
-	  keyVal =  grab_mask->keyval;
-        }
 #ifdef SPI_DEBUG
-      fprintf (stderr, "mask=%lx\n", maskVal);
+      fprintf (stderr, "mask=%lx %lx (%c%c) %s\n",
+	       (long int) grab_mask->key_val,
+	       (long int) grab_mask->mod_mask,
+	       grab_mask->pending_add ? '+' : '.',
+	       grab_mask->pending_remove ? '-' : '.',
+	       re_issue_grab ? "re-issue": "");
 #endif
-      XGrabKey (spi_get_display (),
-		keyVal, 
-		maskVal,
-		root_window,
-		True,
-		GrabModeAsync,
-		GrabModeAsync);
-	  /* TODO: check call for errors and return FALSE if error occurs */
+
+      do_remove = FALSE;
+
+      if (grab_mask->pending_add && grab_mask->pending_remove)
+        {
+          do_remove = TRUE;
+	}
+      else if (grab_mask->pending_remove)
+        {
+	  XUngrabKey (GDK_DISPLAY (),
+		      grab_mask->key_val,
+		      grab_mask->mod_mask,
+		      gdk_x11_get_default_root_xwindow ());
+
+          do_remove = TRUE;
+	}
+      else if (grab_mask->pending_add || re_issue_grab)
+        {
+          XGrabKey (GDK_DISPLAY (),
+		    grab_mask->key_val,
+		    grab_mask->mod_mask,
+		    gdk_x11_get_default_root_xwindow (),
+		    True,
+		    GrabModeAsync,
+		    GrabModeAsync);
+	}
+
+      grab_mask->pending_add = FALSE;
+      grab_mask->pending_remove = FALSE;
+
+      if (do_remove)
+        {
+          g_assert (grab_mask->ref_count <= 0);
+
+	  controller->keygrabs_list = g_list_delete_link (
+	    controller->keygrabs_list, l);
+
+	  spi_grab_mask_free (grab_mask);
+	}
+
+      /* TODO: check calls for errors and return FALSE if error occurs */
     } 
+
   return TRUE;
 }
 
@@ -613,7 +640,10 @@ spi_controller_grab_keyboard (SpiDeviceE
 static void
 spi_device_event_controller_object_finalize (GObject *object)
 {
+  SpiDEController *controller;
 
+  controller = SPI_DEVICE_EVENT_CONTROLLER (object);
+
 #ifdef SPI_DEBUG
   fprintf(stderr, "spi_device_event_controller_object_finalize called\n");
 #endif
@@ -623,19 +653,19 @@ spi_device_event_controller_object_final
 }
 
 /*
- * CORBA Accessibility::DeviceEventController::registerKeystrokeListener
+ * CORBA Accessibility::DEController::registerKeystrokeListener
  *     method implementation
  */
 static void
-impl_register_keystroke_listener (PortableServer_Servant     servant,
+impl_register_keystroke_listener (PortableServer_Servant                  servant,
 				  const Accessibility_DeviceEventListener l,
-				  const Accessibility_KeySet *keys,
+				  const Accessibility_KeySet             *keys,
 				  const Accessibility_ControllerEventMask mask,
-				  const Accessibility_KeyEventTypeSeq *type,
-				  const Accessibility_EventListenerMode *mode,
-				  CORBA_Environment         *ev)
+				  const Accessibility_KeyEventTypeSeq    *type,
+				  const Accessibility_EventListenerMode  *mode,
+				  CORBA_Environment                      *ev)
 {
-  SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
+  SpiDEController *controller = SPI_DEVICE_EVENT_CONTROLLER (
 	  bonobo_object_from_servant (servant));
   DEControllerKeyListener *dec_listener;
 #ifdef SPI_DEBUG
@@ -643,80 +673,85 @@ impl_register_keystroke_listener (Portab
 	   (void *) l, (unsigned long) mask);
 #endif
   dec_listener = spi_dec_key_listener_new (l, keys, mask, type, mode, ev);
-  spi_controller_register_device_listener (controller, (DEControllerListener *) dec_listener, ev);
+  spi_controller_register_device_listener (
+    controller, (DEControllerListener *) dec_listener, ev);
+}
+
+
+typedef struct {
+	CORBA_Environment       *ev;
+	DEControllerKeyListener *key_listener;
+} RemoveKeyListenerClosure;
+
+static SpiReEnterantContinue
+remove_key_listener_cb (GList * const *list,
+			gpointer       user_data)
+{
+  DEControllerKeyListener  *key_listener = (*list)->data;
+  RemoveKeyListenerClosure *ctx = user_data;
+
+  if (CORBA_Object_is_equivalent (ctx->key_listener->listener.object,
+				  key_listener->listener.object, ctx->ev))
+    {
+      spi_re_enterant_list_delete_link (list);
+      spi_dec_key_listener_free (key_listener, ctx->ev);
+    }
+
+  return SPI_RE_ENTERANT_CONTINUE;
 }
 
 /*
- * CORBA Accessibility::DeviceEventController::deregisterKeystrokeListener
+ * CORBA Accessibility::DEController::deregisterKeystrokeListener
  *     method implementation
  */
 static void
-impl_deregister_keystroke_listener (PortableServer_Servant     servant,
+impl_deregister_keystroke_listener (PortableServer_Servant                  servant,
 				    const Accessibility_DeviceEventListener l,
-				    const Accessibility_KeySet *keys,
+				    const Accessibility_KeySet             *keys,
 				    const Accessibility_ControllerEventMask mask,
-				    const Accessibility_KeyEventTypeSeq *type,
-				    CORBA_Environment         *ev)
+				    const Accessibility_KeyEventTypeSeq    *type,
+				    CORBA_Environment                      *ev)
 {
-	SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
-		bonobo_object_from_servant (servant));
-	DEControllerKeyListener *key_listener = spi_dec_key_listener_new (l,
-									  keys,
-									  mask,
-									  type,
-									  NULL,
-									  ev);
+  DEControllerKeyListener  *key_listener;
+  RemoveKeyListenerClosure  ctx;
+  SpiDEController *controller;
+
+  controller = SPI_DEVICE_EVENT_CONTROLLER (bonobo_object (servant));
+
+  key_listener = spi_dec_key_listener_new (l, keys, mask, type, NULL, ev);
+
 #ifdef SPI_DEREGISTER_DEBUG
-	fprintf (stderr, "deregistering keystroke listener %p with maskVal %lu\n",
-		 (void *) l, (unsigned long) mask->value);
+  fprintf (stderr, "deregistering keystroke listener %p with maskVal %lu\n",
+	   (void *) l, (unsigned long) mask->value);
 #endif
-	spi_controller_deregister_device_listener(controller,
-					      (DEControllerListener *) key_listener,
-					      ev);
-	spi_dec_key_listener_free (key_listener, ev);
-}
 
-/*
- * CORBA Accessibility::DeviceEventController::registerMouseListener
- *     method implementation
- */
-/*
-static void
-impl_register_mouse_listener (PortableServer_Servant     servant,
-			      const Accessibility_MouseListener *l,
-			      CORBA_Environment         *ev)
-{
-	SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
-		bonobo_object_from_servant (servant));
-#ifdef SPI_DEBUG
-	fprintf (stderr, "registering mouse listener %p\n", l);
-#endif
-	spi_controller_register_device_listener(controller, DEVICE_TYPE_MOUSE, l, keys, mask, ev);
-}
-*/
+  spi_controller_deregister_global_keygrabs (controller, key_listener);
 
-static KeyCode
-keycode_for_keysym (long keysym)
-{
-  return XKeysymToKeycode (spi_get_display (), (KeySym) keysym);
-}
+  ctx.ev = ev;
+  ctx.key_listener = key_listener;
+
+  spi_re_enterant_list_foreach (&controller->key_listeners,
+				remove_key_listener_cb, &ctx);
 
-#define SPI_DEBUG
+  spi_dec_key_listener_free (key_listener, ev);
+}
 
 /*
- * CORBA Accessibility::DeviceEventController::registerKeystrokeListener
+ * CORBA Accessibility::DEController::registerKeystrokeListener
  *     method implementation
  */
 static void
-impl_generate_keyboard_event (PortableServer_Servant     servant,
-			      const CORBA_long           keycode,
-			      const CORBA_char          *keystring,
+impl_generate_keyboard_event (PortableServer_Servant           servant,
+			      const CORBA_long                 keycode,
+			      const CORBA_char                *keystring,
 			      const Accessibility_KeySynthType synth_type,
-			      CORBA_Environment          *ev)
+			      CORBA_Environment               *ev)
 {
   long key_synth_code;
+
 #ifdef SPI_DEBUG
-  fprintf (stderr, "synthesizing keystroke %ld, type %d\n", (long) keycode, (int) synth_type);
+  fprintf (stderr, "synthesizing keystroke %ld, type %d\n",
+	   (long) keycode, (int) synth_type);
 #endif
   /* TODO: hide/wrap/remove X dependency */
 
@@ -728,26 +763,34 @@ impl_generate_keyboard_event (PortableSe
    */
   
   /* TODO: implement keystring mode also */
-	
+  gdk_error_trap_push ();
+
   switch (synth_type)
     {
       case Accessibility_KEY_PRESS:
-	  XTestFakeKeyEvent (spi_get_display (), (unsigned int) keycode, True, CurrentTime);
-	  break;
+        XTestFakeKeyEvent (GDK_DISPLAY (), (unsigned int) keycode, True, CurrentTime);
+	break;
       case Accessibility_KEY_PRESSRELEASE:
-	  XTestFakeKeyEvent (spi_get_display (), (unsigned int) keycode, True, CurrentTime);
+	XTestFakeKeyEvent (GDK_DISPLAY (), (unsigned int) keycode, True, CurrentTime);
       case Accessibility_KEY_RELEASE:
-	  XTestFakeKeyEvent (spi_get_display (), (unsigned int) keycode, False, CurrentTime);
-	  break;
+	XTestFakeKeyEvent (GDK_DISPLAY (), (unsigned int) keycode, False, CurrentTime);
+	break;
       case Accessibility_KEY_SYM:
-	  key_synth_code = keycode_for_keysym (keycode);
-	  XTestFakeKeyEvent (spi_get_display (), (unsigned int) key_synth_code, True, CurrentTime);
-	  XTestFakeKeyEvent (spi_get_display (), (unsigned int) key_synth_code, False, CurrentTime);
-	  break;
-   }
+	key_synth_code = keycode_for_keysym (keycode);
+	XTestFakeKeyEvent (GDK_DISPLAY (), (unsigned int) key_synth_code, True, CurrentTime);
+	XTestFakeKeyEvent (GDK_DISPLAY (), (unsigned int) key_synth_code, False, CurrentTime);
+	break;
+      case Accessibility_KEY_STRING:
+	fprintf (stderr, "Not yet implemented\n");
+	break;
+    }
+  if (gdk_error_trap_pop ())
+    {
+      g_warning ("Error emitting keystroke");
+    }
 }
 
-/* Accessibility::DeviceEventController::generateMouseEvent */
+/* Accessibility::DEController::generateMouseEvent */
 static void
 impl_generate_mouse_event (PortableServer_Servant servant,
 			   const CORBA_long       x,
@@ -756,90 +799,121 @@ impl_generate_mouse_event (PortableServe
 			   CORBA_Environment     *ev)
 {
 #ifdef SPI_DEBUG
-  fprintf (stderr, "generating mouse %s event at %ld, %ld\n", eventName, x, y);
+  fprintf (stderr, "generating mouse %s event at %ld, %ld\n",
+	   eventName, (long int) x, (long int) y);
 #endif
+  g_warning ("not yet implemented");
 }
 
-/* Accessibility::DeviceEventController::notifyListenersSync */
+/* Accessibility::DEController::notifyListenersSync */
 static CORBA_boolean
-impl_notify_listeners_sync(PortableServer_Servant     servant,
-			   const Accessibility_DeviceEvent *event,
-			   CORBA_Environment         *ev)
+impl_notify_listeners_sync (PortableServer_Servant           servant,
+			    const Accessibility_DeviceEvent *event,
+			    CORBA_Environment               *ev)
 {
-  SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER (
-                                         bonobo_object_from_servant (servant));
+  SpiDEController *controller = SPI_DEVICE_EVENT_CONTROLLER (
+    bonobo_object_from_servant (servant));
 #ifdef SPI_DEBUG
-  g_print ("notifylistening listeners synchronously: controller %x, event id %d\n",
-	   (void *) controller, (int) event->id);
+  g_print ("notifylistening listeners synchronously: controller %p, event id %d\n",
+	   controller, (int) event->id);
 #endif
-  return (spi_notify_keylisteners (controller->key_listeners, event, CORBA_FALSE, ev) ?
-	  CORBA_TRUE : CORBA_FALSE); 
+  return spi_notify_keylisteners (&controller->key_listeners, event, CORBA_FALSE, ev) ?
+	  CORBA_TRUE : CORBA_FALSE; 
 }
 
-/* Accessibility::DeviceEventController::notifyListenersAsync */
+/* Accessibility::DEController::notifyListenersAsync */
 static void
-impl_notify_listeners_async (PortableServer_Servant     servant,
+impl_notify_listeners_async (PortableServer_Servant           servant,
 			     const Accessibility_DeviceEvent *event,
-			     CORBA_Environment         *ev)
+			     CORBA_Environment               *ev)
 {
-  SpiDeviceEventController *controller = SPI_DEVICE_EVENT_CONTROLLER(
-	                                 bonobo_object_from_servant (servant));
+  SpiDEController *controller = SPI_DEVICE_EVENT_CONTROLLER (
+    bonobo_object_from_servant (servant));
 #ifdef SPI_DEBUG
   fprintf (stderr, "notifying listeners asynchronously\n");
 #endif
-  spi_notify_keylisteners (controller->key_listeners, event, CORBA_FALSE, ev); 
+  spi_notify_keylisteners (&controller->key_listeners, event, CORBA_FALSE, ev); 
 }
 
 static void
-spi_device_event_controller_class_init (SpiDeviceEventControllerClass *klass)
+spi_device_event_controller_class_init (SpiDEControllerClass *klass)
 {
-        GObjectClass * object_class = (GObjectClass *) klass;
-        POA_Accessibility_DeviceEventController__epv *epv = &klass->epv;
-        spi_device_event_controller_parent_class = g_type_class_peek_parent (klass);
-
-        object_class->finalize = spi_device_event_controller_object_finalize;
+  GObjectClass * object_class = (GObjectClass *) klass;
+  POA_Accessibility_DeviceEventController__epv *epv = &klass->epv;
 
-        epv->registerKeystrokeListener = impl_register_keystroke_listener;
-        epv->deregisterKeystrokeListener = impl_deregister_keystroke_listener;
-/*        epv->registerMouseListener = impl_register_mouse_listener; */
-        epv->generateKeyboardEvent = impl_generate_keyboard_event;
-        epv->generateMouseEvent = impl_generate_mouse_event;
-	epv->notifyListenersSync = impl_notify_listeners_sync;
-	epv->notifyListenersAsync = impl_notify_listeners_async;
-	klass->check_key_event = spi_check_key_event;
+  spi_device_event_controller_parent_class = g_type_class_peek_parent (klass);
+  
+  object_class->finalize = spi_device_event_controller_object_finalize;
+	
+  epv->registerKeystrokeListener   = impl_register_keystroke_listener;
+  epv->deregisterKeystrokeListener = impl_deregister_keystroke_listener;
+  epv->generateKeyboardEvent       = impl_generate_keyboard_event;
+  epv->generateMouseEvent          = impl_generate_mouse_event;
+  epv->notifyListenersSync         = impl_notify_listeners_sync;
+  epv->notifyListenersAsync        = impl_notify_listeners_async;
 }
 
 static void
-spi_device_event_controller_init (SpiDeviceEventController *device_event_controller)
+spi_device_event_controller_init (SpiDEController *device_event_controller)
 {
-  device_event_controller->key_listeners = NULL;
+  device_event_controller->key_listeners   = NULL;
   device_event_controller->mouse_listeners = NULL;
-  device_event_controller->keygrabs_list = NULL;
-  kbd_registered = spi_controller_register_with_devices (device_event_controller);
+  device_event_controller->keygrabs_list   = NULL;
+
+  /*
+   * TODO: fixme, this module makes the foolish assumptions that
+   * registryd uses the same display as the apps, and that the
+   * DISPLAY environment variable is set.
+   */
+  gdk_init (NULL, NULL);
+  
+  spi_controller_register_with_devices (device_event_controller);
 }
 
-gboolean
-spi_device_event_controller_check_key_event (SpiDeviceEventController *controller)
+static void
+spi_device_event_controller_forward_key_event (SpiDEController *controller,
+					       const XEvent    *event)
 {
-  SpiDeviceEventControllerClass *klass = SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS (controller);
-  if (klass->check_key_event)
-	return (klass->check_key_event) (controller);
-  return FALSE;
+  gboolean is_consumed = FALSE;
+  CORBA_Environment ev;
+  Accessibility_DeviceEvent key_event;
+
+  g_assert (event->type == KeyPress || event->type == KeyRelease);
+
+  CORBA_exception_init (&ev);
+
+  key_event = spi_keystroke_from_x_key_event ((XKeyEvent *) event);
+  /* relay to listeners, and decide whether to consume it or not */
+  is_consumed = spi_notify_keylisteners (
+    &controller->key_listeners, &key_event, CORBA_TRUE, &ev);
+
+  CORBA_exception_free (&ev);
+
+  if (is_consumed)
+    {
+      XAllowEvents (GDK_DISPLAY (), AsyncKeyboard, CurrentTime);
+    }
+  else
+    {
+      XAllowEvents (GDK_DISPLAY (), ReplayKeyboard, CurrentTime);
+    }
+
+  spi_controller_update_key_grabs (controller, &key_event);
 }
 
-SpiDeviceEventController *
-spi_device_event_controller_new (void *registryp)
+SpiDEController *
+spi_device_event_controller_new (SpiRegistry *registry)
 {
-  BonoboObject *registry = (BonoboObject *) registryp;	
-  SpiDeviceEventController *retval = g_object_new (
-	  SPI_DEVICE_EVENT_CONTROLLER_TYPE, NULL);
-  retval->registry = registry;
-  bonobo_object_ref (registry);
+  SpiDEController *retval = g_object_new (
+    SPI_DEVICE_EVENT_CONTROLLER_TYPE, NULL);
+
+  retval->registry = SPI_REGISTRY (bonobo_object_ref (
+	  BONOBO_OBJECT (registry)));
+
   return retval;
 }
 
-BONOBO_TYPE_FUNC_FULL (SpiDeviceEventController,
+BONOBO_TYPE_FUNC_FULL (SpiDEController,
 		       Accessibility_DeviceEventController,
 		       PARENT_TYPE,
 		       spi_device_event_controller);
-
Index: registryd/deviceeventcontroller.h
===================================================================
RCS file: /cvs/gnome/at-spi/registryd/deviceeventcontroller.h,v
retrieving revision 1.10
diff -u -p -u -r1.10 deviceeventcontroller.h
--- registryd/deviceeventcontroller.h	2001/12/15 22:54:00	1.10
+++ registryd/deviceeventcontroller.h	2002/01/11 16:13:10
@@ -27,32 +27,36 @@
 #include <libspi/Accessibility.h>
 #include <libspi/keystrokelistener.h>
 
+typedef struct _SpiDEController SpiDEController;
+
+#include "registry.h"
+
 G_BEGIN_DECLS
 
 #define SPI_DEVICE_EVENT_CONTROLLER_TYPE        (spi_device_event_controller_get_type ())
-#define SPI_DEVICE_EVENT_CONTROLLER(o)          (G_TYPE_CHECK_INSTANCE_CAST ((o), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDeviceEventController))
-#define SPI_DEVICE_EVENT_CONTROLLER_CLASS(k)    (G_TYPE_CHECK_CLASS_CAST((k), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDeviceEventControllerClass))
+#define SPI_DEVICE_EVENT_CONTROLLER(o)          (G_TYPE_CHECK_INSTANCE_CAST ((o), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDEController))
+#define SPI_DEVICE_EVENT_CONTROLLER_CLASS(k)    (G_TYPE_CHECK_CLASS_CAST((k), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDEControllerClass))
 #define SPI_IS_DEVICE_EVENT_CONTROLLER(o)       (G_TYPE_CHECK_INSTANCE_TYPE ((o), SPI_DEVICE_EVENT_CONTROLLER_TYPE))
 #define SPI_IS_DEVICE_EVENT_CONTROLLER_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE ((k), SPI_DEVICE_EVENT_CONTROLLER_TYPE))
-#define SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDeviceEventControllerClass))
+#define SPI_DEVICE_EVENT_CONTROLLER_GET_CLASS(obj) (G_TYPE_INSTANCE_GET_CLASS ((obj), SPI_DEVICE_EVENT_CONTROLLER_TYPE, SpiDEControllerClass))
 
-typedef struct {
+struct _SpiDEController {
   BonoboObject parent;
-  void  *registry;
-  GList *key_listeners;
-  GList *mouse_listeners;
-  GList *keygrabs_list;
-} SpiDeviceEventController;
 
+  SpiRegistry *registry;
+  GList       *key_listeners;
+  GList       *mouse_listeners;
+  GList       *keygrabs_list;
+};
+
 typedef struct {
   BonoboObjectClass parent_class;
+
   POA_Accessibility_DeviceEventController__epv epv;
-  gboolean (*check_key_event) (SpiDeviceEventController *controller);
-} SpiDeviceEventControllerClass;
+} SpiDEControllerClass;
 
-GType                     spi_device_event_controller_get_type        (void);
-SpiDeviceEventController *spi_device_event_controller_new             (void *registry);
-gboolean                  spi_device_event_controller_check_key_event (SpiDeviceEventController *controller);
+GType            spi_device_event_controller_get_type (void);
+SpiDEController *spi_device_event_controller_new      (SpiRegistry *registry);
 
 G_END_DECLS
 
Index: registryd/registry-main.c
===================================================================
RCS file: /cvs/gnome/at-spi/registryd/registry-main.c,v
retrieving revision 1.10
diff -u -p -u -r1.10 registry-main.c
--- registryd/registry-main.c	2001/12/07 16:43:30	1.10
+++ registryd/registry-main.c	2002/01/11 16:13:10
@@ -24,6 +24,7 @@
 #include <stdlib.h>
 #endif
 
+#include <gdk/gdk.h>
 #include <libbonobo.h>
 #include <glib/gmain.h>
 #include "registry.h"
@@ -59,7 +60,6 @@ main (int argc, char **argv)
 #ifdef AT_SPI_DEBUG
       fprintf (stderr, "SpiRegistry Message: SpiRegistry daemon is running.\n");
 #endif
-      g_timeout_add_full (G_PRIORITY_HIGH_IDLE, 200, registry->kbd_event_hook, registry, NULL);
       bonobo_main ();
     }
 
Index: registryd/registry.c
===================================================================
RCS file: /cvs/gnome/at-spi/registryd/registry.c,v
retrieving revision 1.35
diff -u -p -u -r1.35 registry.c
--- registryd/registry.c	2002/01/08 09:11:46	1.35
+++ registryd/registry.c	2002/01/11 16:13:11
@@ -30,6 +30,7 @@
 #endif
 
 #include <bonobo/bonobo-exception.h>
+#include "../libspi/spi-private.h"
 #include "registry.h"
 
 /* Our parent GObject type  */
@@ -63,8 +64,6 @@ typedef struct {
   EventTypeCategory event_type_cat;
 } SpiListenerStruct;
 
-/* static function prototypes */
-static gboolean _device_event_controller_hook (gpointer source);
 
 SpiListenerStruct *
 spi_listener_struct_new (Accessibility_EventListener listener, CORBA_Environment *ev)
@@ -323,7 +322,9 @@ impl_accessibility_registry_register_glo
   EventTypeStruct etype;
   GList          **list;
 
-  fprintf(stderr, "registering for events of type %s\n", event_name);
+#ifdef SPI_DEBUG
+  fprintf (stderr, "registering for events of type %s\n", event_name);
+#endif
 
   /* parse, check major event type and add listener accordingly */
   parse_event_type (&etype, event_name);
@@ -347,7 +348,7 @@ impl_accessibility_registry_register_glo
     }
 }
 
-static void
+static SpiReEnterantContinue
 remove_listener_cb (GList * const *list, gpointer user_data)
 {
   SpiListenerStruct *ls = (SpiListenerStruct *) (*list)->data;
@@ -363,6 +364,8 @@ remove_listener_cb (GList * const *list,
     }
 
   CORBA_exception_free (&ev);
+
+  return SPI_RE_ENTERANT_CONTINUE;
 }
 
 /*
@@ -407,7 +410,7 @@ impl_accessibility_registry_deregister_g
   parse_event_type (&etype, (char *) event_name);
 
   spi_re_enterant_list_foreach (get_listener_list (registry, etype.type_cat),
-			    remove_listener_cb, listener);
+				remove_listener_cb, listener);
 }
 

@@ -484,16 +487,16 @@ impl_accessibility_registry_get_desktop_
 
 static Accessibility_DeviceEventController
 impl_accessibility_registry_get_device_event_controller (PortableServer_Servant servant,
-                                                         CORBA_Environment     *ev)
+							 CORBA_Environment     *ev)
 {
   SpiRegistry *registry = SPI_REGISTRY (bonobo_object_from_servant (servant));
 
-  if (!registry->device_event_controller)
+  if (!registry->de_controller)
     {
-      registry->device_event_controller = spi_device_event_controller_new (registry);
+      registry->de_controller = spi_device_event_controller_new (registry);
     }
 
-  return bonobo_object_dup_ref (BONOBO_OBJREF (registry->device_event_controller), ev);
+  return bonobo_object_dup_ref (BONOBO_OBJREF (registry->de_controller), ev);
 }
 
 typedef struct {
@@ -503,7 +506,7 @@ typedef struct {
   Accessibility_Event e_out;
 } NotifyContext;
 
-static void
+static SpiReEnterantContinue
 notify_listeners_cb (GList * const *list, gpointer user_data)
 {
   SpiListenerStruct *ls;
@@ -531,7 +534,9 @@ notify_listeners_cb (GList * const *list
       
       ctx->e_out.source = bonobo_object_dup_ref (ctx->source, ctx->ev);
       if (BONOBO_EX (ctx->ev))
-	      return;
+        {
+          return SPI_RE_ENTERANT_CONTINUE;;
+	}
 
       if ((*list) && (*list)->data == ls)
         {
@@ -549,6 +554,8 @@ notify_listeners_cb (GList * const *list
           bonobo_object_release_unref (ctx->e_out.source, ctx->ev);
 	}
     }  
+
+  return SPI_RE_ENTERANT_CONTINUE;
 }
 
 static void
@@ -582,37 +589,28 @@ impl_registry_notify_event (PortableServ
     }
 }
 
-static gboolean
-_device_event_controller_hook (gpointer p)
-{
-    SpiRegistry *registry = (SpiRegistry *)p;
-    SpiDeviceEventController *controller = registry->device_event_controller;
-    if (controller)
-	spi_device_event_controller_check_key_event (controller);
-    return TRUE;
-}
 
 static void
 spi_registry_class_init (SpiRegistryClass *klass)
 {
-        GObjectClass * object_class = (GObjectClass *) klass;
-        POA_Accessibility_Registry__epv *epv = &klass->epv;
-
-        spi_registry_parent_class = g_type_class_ref (SPI_LISTENER_TYPE);
-
-        object_class->finalize = spi_registry_object_finalize;
+  GObjectClass * object_class = (GObjectClass *) klass;
+  POA_Accessibility_Registry__epv *epv = &klass->epv;
 
-        epv->registerApplication = impl_accessibility_registry_register_application;
-        epv->deregisterApplication = impl_accessibility_registry_deregister_application;
-        epv->registerGlobalEventListener = impl_accessibility_registry_register_global_event_listener;
-        epv->deregisterGlobalEventListener = impl_accessibility_registry_deregister_global_event_listener;
-        epv->deregisterGlobalEventListenerAll = impl_accessibility_registry_deregister_global_event_listener_all;
-        epv->getDeviceEventController = impl_accessibility_registry_get_device_event_controller;
-        epv->getDesktopCount = impl_accessibility_registry_get_desktop_count;
-        epv->getDesktop = impl_accessibility_registry_get_desktop;
-        epv->getDesktopList = impl_accessibility_registry_get_desktop_list;
+  spi_registry_parent_class = g_type_class_ref (SPI_LISTENER_TYPE);
+  
+  object_class->finalize = spi_registry_object_finalize;
 
-        ((SpiListenerClass *) klass)->epv.notifyEvent = impl_registry_notify_event;
+  klass->parent_class.epv.notifyEvent   = impl_registry_notify_event;
+  
+  epv->registerApplication              = impl_accessibility_registry_register_application;
+  epv->deregisterApplication            = impl_accessibility_registry_deregister_application;
+  epv->registerGlobalEventListener      = impl_accessibility_registry_register_global_event_listener;
+  epv->deregisterGlobalEventListener    = impl_accessibility_registry_deregister_global_event_listener;
+  epv->deregisterGlobalEventListenerAll = impl_accessibility_registry_deregister_global_event_listener_all;
+  epv->getDeviceEventController         = impl_accessibility_registry_get_device_event_controller;
+  epv->getDesktopCount                  = impl_accessibility_registry_get_desktop_count;
+  epv->getDesktop                       = impl_accessibility_registry_get_desktop;
+  epv->getDesktopList                   = impl_accessibility_registry_get_desktop_list;
 }
 
 static void
@@ -622,8 +620,7 @@ spi_registry_init (SpiRegistry *registry
   registry->window_listeners = NULL;
   registry->toolkit_listeners = NULL;
   registry->desktop = spi_desktop_new ();
-  registry->device_event_controller = NULL;
-  registry->kbd_event_hook = _device_event_controller_hook;
+  registry->de_controller = NULL;
 }
 
 BONOBO_TYPE_FUNC_FULL (SpiRegistry,
@@ -634,7 +631,7 @@ BONOBO_TYPE_FUNC_FULL (SpiRegistry,
 SpiRegistry *
 spi_registry_new (void)
 {
-    SpiRegistry *retval = g_object_new (SPI_REGISTRY_TYPE, NULL);
-    bonobo_object_set_immortal (BONOBO_OBJECT (retval), TRUE);
-    return retval;
+  SpiRegistry *retval = g_object_new (SPI_REGISTRY_TYPE, NULL);
+  bonobo_object_set_immortal (BONOBO_OBJECT (retval), TRUE);
+  return retval;
 }
Index: registryd/registry.h
===================================================================
RCS file: /cvs/gnome/at-spi/registryd/registry.h,v
retrieving revision 1.13
diff -u -p -u -r1.13 registry.h
--- registryd/registry.h	2001/12/12 16:08:46	1.13
+++ registryd/registry.h	2002/01/11 16:13:11
@@ -26,6 +26,8 @@
 #include <glib/gmain.h>
 #include <libspi/listener.h>
 
+typedef struct _SpiRegistry SpiRegistry;
+
 #include "desktop.h"
 #include "deviceeventcontroller.h"
 
@@ -37,19 +39,20 @@ G_BEGIN_DECLS
 #define SPI_IS_REGISTRY(o)       (G_TYPE_CHECK__INSTANCE_TYPE ((o), SPI_REGISTRY_TYPE))
 #define SPI_IS_REGISTRY_CLASS(k) (G_TYPE_CHECK_CLASS_TYPE ((k), SPI_REGISTRY_TYPE))
 
-typedef struct {
-  SpiListener parent;
-  GList *object_listeners;
-  GList *window_listeners;
-  GList *toolkit_listeners;
-  SpiDeviceEventController *device_event_controller;
-  SpiDesktop *desktop;
-  gboolean (*kbd_event_hook) (gpointer source);
-} SpiRegistry;
+struct _SpiRegistry {
+  SpiListener      parent;
 
+  GList           *object_listeners;
+  GList           *window_listeners;
+  GList           *toolkit_listeners;
+  SpiDEController *de_controller;
+  SpiDesktop      *desktop;
+};
+
 typedef struct {
-        SpiListenerClass parent_class;
-        POA_Accessibility_Registry__epv epv;
+  SpiListenerClass parent_class;
+
+  POA_Accessibility_Registry__epv epv;
 } SpiRegistryClass;
 
 GType        spi_registry_get_type (void);
Index: test/test-simple.c
===================================================================
RCS file: /cvs/gnome/at-spi/test/test-simple.c,v
retrieving revision 1.17
diff -u -p -u -r1.17 test-simple.c
--- test/test-simple.c	2002/01/11 12:00:47	1.17
+++ test/test-simple.c	2002/01/11 16:13:12
@@ -651,38 +651,39 @@ key_listener_cb (const AccessibleKeystro
 
 	*s = *stroke;
 
-	g_warning ("Key listener callback");
-
 	return FALSE;
 }
 
 static void
 test_keylisteners (void)
 {
+	int i;
 	AccessibleKeystroke stroke;
 	AccessibleKeystrokeListener *key_listener;
 
+	fprintf (stderr, "Testing keyboard listeners ...\n");
+
 	key_listener = SPI_createAccessibleKeystrokeListener (
 		key_listener_cb, &stroke);
 
 	g_assert (SPI_registerAccessibleKeystrokeListener (
-		key_listener, SPI_KEYSET_ALL_KEYS, 0,
+		key_listener,
+		SPI_KEYSET_ALL_KEYS,
+		0,
 		SPI_KEY_PRESSED | SPI_KEY_RELEASED,
-		SPI_KEYLISTENER_CANCONSUME));
-
-#if FIXME_HOW_SHOULD_THIS_WORK
-	memset (&stroke, 0, sizeof (AccessibleKeystroke));
+		SPI_KEYLISTENER_CANCONSUME | SPI_KEYLISTENER_ALL_WINDOWS));
 
-	g_assert (SPI_generateKeyboardEvent (33, "!", SPI_KEY_PRESSRELEASE));
+	for (i = 0; i < 3; i++) {
+		memset (&stroke, 0, sizeof (AccessibleKeystroke));
+		g_assert (SPI_generateKeyboardEvent (33, "!", SPI_KEY_PRESSRELEASE));
+		while (stroke.type == 0)
+			g_main_iteration (TRUE);
+	}
 
-	while (stroke.type == 0)
-		g_main_iteration (TRUE);
+	g_assert (SPI_deregisterAccessibleKeystrokeListener (key_listener, 0));
 
-	g_assert (!strcmp (stroke.keystring, "!"));
+	/* FIXME: expand the validation here */
 	g_assert (stroke.type == SPI_KEY_PRESSRELEASE);
-#endif
-
-	g_assert (SPI_deregisterAccessibleKeystrokeListener (key_listener, 0));
 
 	AccessibleKeystrokeListener_unref (key_listener);
 }

-- 
 mmeeks gnu org  <><, Pseudo Engineer, itinerant idiot




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]