Fix custom-alt-tabs for single-handling of key events

The changes to enforce single handling of all key events were breaking
custom-alt-tab keypress handlers, since that code was assuming that
key event would get to process_tab_grab(), and then maybe to
process_event() and then to the plugin's xevent_filter to detect a
key release.

We centeralize all of this handling into process_tab_grab() and either

 - Invoke a custom handler for the key press
 - Select the current window on modifier release by calling a  new
   pseudo-binding "tab_popup_select"
 - Cancel the grab on an unbound key by calling a new pseudo-binding
   "tab_popup_cancel"

http://bugzilla.gnome.org/show_bug.cgi?id=590754

src/core/keybindings.c

@@ -475,11 +475,11 @@ regrab_key_bindings (MetaDisplay *display)
   g_slist_free (windows);
 }
 
-static MetaKeyBindingAction
-display_get_keybinding_action (MetaDisplay  *display,
-                               unsigned int  keysym,
-                               unsigned int  keycode,
-                               unsigned long mask)
+static MetaKeyBinding *
+display_get_keybinding (MetaDisplay  *display,
+                        unsigned int  keysym,
+                        unsigned int  keycode,
+                        unsigned long mask)
 {
   int i;
 
@@ -490,13 +490,29 @@ display_get_keybinding_action (MetaDisplay  *display,
           display->key_bindings[i].keycode == keycode &&
           display->key_bindings[i].mask == mask)
         {
-          return meta_prefs_get_keybinding_action (display->key_bindings[i].name);
+          return &display->key_bindings[i];
         }
       
       --i;
     }
 
-  return META_KEYBINDING_ACTION_NONE;
+  return NULL;
+}
+
+static MetaKeyBindingAction
+display_get_keybinding_action (MetaDisplay  *display,
+                               unsigned int  keysym,
+                               unsigned int  keycode,
+                               unsigned long mask)
+{
+  MetaKeyBinding *binding;
+
+  binding = display_get_keybinding (display, keysym, keycode, mask);
+
+  if (binding)
+    return meta_prefs_get_keybinding_action (binding->name);
+  else
+    return META_KEYBINDING_ACTION_NONE;
 }
 
 void
@@ -1197,6 +1213,45 @@ process_overlay_key (MetaDisplay *display,
   return TRUE;
 }
 
+static void
+invoke_handler (MetaDisplay    *display,
+                MetaScreen     *screen,
+                MetaKeyHandler *handler,
+                MetaWindow     *window,
+                XEvent         *event,
+                MetaKeyBinding *binding)
+
+{
+  if (handler->func)
+    (* handler->func) (display, screen,
+                       handler->flags & BINDING_PER_WINDOW ?
+                           window : NULL,
+                       event,
+                       binding,
+                       handler->user_data);
+  else
+    (* handler->default_func) (display, screen,
+                               handler->flags & BINDING_PER_WINDOW ?
+                                   window: NULL,
+                               event,
+                               binding,
+                               NULL);
+}
+
+static void
+invoke_handler_by_name (MetaDisplay    *display,
+                        MetaScreen     *screen,
+                        const char     *handler_name,
+                        MetaWindow     *window,
+                        XEvent         *event)
+{
+  MetaKeyHandler *handler;
+
+  handler = find_handler (key_handlers, handler_name);
+  if (handler)
+    invoke_handler (display, screen, handler, window, event, NULL);
+}
+
 /* now called from only one place, may be worth merging */
 static gboolean
 process_event (MetaKeyBinding       *bindings,
@@ -1253,20 +1308,7 @@ process_event (MetaKeyBinding       *bindings,
        */
       display->allow_terminal_deactivation = TRUE;
 
-      if (handler->func)
-        (* handler->func) (display, screen,
-                           bindings[i].handler->flags & BINDING_PER_WINDOW ?
-                           window: NULL,
-                           event,
-                           &bindings[i],
-                           handler->user_data);
-      else
-        (* handler->default_func) (display, screen,
-                           bindings[i].handler->flags & BINDING_PER_WINDOW ?
-                           window: NULL,
-                           event,
-                           &bindings[i],
-                           NULL);
+      invoke_handler (display, screen, handler, window, event, &bindings[i]);
 
       return TRUE;
     }
@@ -1985,6 +2027,7 @@ process_tab_grab (MetaDisplay *display,
                   XEvent      *event,
                   KeySym       keysym)
 {
+  MetaKeyBinding *binding;
   MetaKeyBindingAction action;
   gboolean popup_not_showing;
   gboolean backward;
@@ -1994,79 +2037,78 @@ process_tab_grab (MetaDisplay *display,
   if (screen != display->grab_screen)
     return FALSE;
 
-  action = display_get_keybinding_action (display,
-                                          keysym,
-                                          event->xkey.keycode,
-                                          display->grab_mask);
+  binding = display_get_keybinding (display,
+                                    keysym,
+                                    event->xkey.keycode,
+                                    display->grab_mask);
+  if (binding)
+    action = meta_prefs_get_keybinding_action (binding->name);
+  else
+    action = META_KEYBINDING_ACTION_NONE;
 
   /*
-   * If there is no tab_pop up object, i.e., there is some custom handler
-   * implementing Alt+Tab & Co., we call this custom handler; we do not
-   * mess about with the grab, as that is up to the handler to deal with.
+   * There are currently two different ways of customizing Alt-Tab, you can either
+   * provide a replacement AltTabHandler object, or you can hook into the keybindings
+   * meta_keybindings_set_custom_handler() and call meta_display_begin_grab_op()
+   * yourself with one of the "tabbing" grab ops META_GRAB_OP_KEYBOARD_TABBING_NORMAL,
+   * etc. See meta_display_process_key_event() for the complete list. If screen->tab_handler
+   * is NULL, the latter mechanism is being used. We skip most of our normal
+   * processing and just make sure that the right custom handlers get called.
    */
   if (!screen->tab_handler)
     {
-      MetaKeyHandler *handler = NULL;
-      const gchar    *handler_name = NULL;
+      if (event->type == KeyRelease)
+        {
+          if (end_keyboard_grab (display, event->xkey.keycode))
+            {
+              invoke_handler_by_name (display, screen, "tab_popup_select", NULL, event);
+
+              /* We return FALSE to end the grab; if the handler ended the grab itself
+               * that will be a noop. If the handler didn't end the grab, then it's a
+               * safety measure to prevent a stuck grab.
+               */
+              return FALSE;
+            }
+
+          return TRUE;
+        }
 
       switch (action)
         {
         case META_KEYBINDING_ACTION_CYCLE_PANELS:
-          handler_name = "cycle_group";
-          break;
         case META_KEYBINDING_ACTION_CYCLE_WINDOWS:
-          handler_name = "cycle_windows";
-          break;
         case META_KEYBINDING_ACTION_CYCLE_PANELS_BACKWARD:
-          handler_name = "cycle_panels_backward";
-          break;
         case META_KEYBINDING_ACTION_CYCLE_WINDOWS_BACKWARD:
-          handler_name = "cycle_windows_backward";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_PANELS:
-          handler_name = "switch_panels";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_WINDOWS:
-          handler_name = "switch_windows";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_PANELS_BACKWARD:
-          handler_name = "switch_panels_backward";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_WINDOWS_BACKWARD:
-          handler_name = "switch_windows_backward";
-          break;
         case META_KEYBINDING_ACTION_CYCLE_GROUP:
-          handler_name = "cycle_group";
-          break;
         case META_KEYBINDING_ACTION_CYCLE_GROUP_BACKWARD:
-          handler_name = "cycle_group_backward";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_GROUP:
-          handler_name = "switch_group";
-          break;
         case META_KEYBINDING_ACTION_SWITCH_GROUP_BACKWARD:
-          handler_name = "switch_group_backward";
+          /* These are the tab-popup bindings. If a custom Alt-Tab implementation
+           * is in effect, we expect it to want to handle all of these as a group
+           *
+           * If there are some of them that the custom implementation didn't
+           * handle, we treat them as "unbound" for the duration - running the
+           * normal handlers could get us into trouble.
+           */
+          if (binding->handler &&
+              binding->handler->func &&
+              binding->handler->func != binding->handler->default_func)
+            {
+              invoke_handler (display, screen, binding->handler, NULL, event, binding);
+              return TRUE;
+            }
           break;
         default:
-          /*
-           * This is the case when the Alt key is released; we preserve
-           * the grab, as it is up to the custom implementaiton to free it
-           * (a plugin can catch this in their xevent_filter function).
-           */
-          return TRUE;
+          break;
         }
 
-      /*
-       * We do not want to actually call the handler, we just want to ensure
-       * that if a custom handler is installed, we do not release the grab here.
-       * The handler will get called as normal in the process_event() function.
-       */
-      handler = find_handler (key_handlers, handler_name);
-
-      if (!handler || !handler->func || handler->func == handler->default_func)
-        return FALSE;
-
-      return TRUE;
+      /* Some unhandled key press */
+      invoke_handler_by_name (display, screen, "tab_popup_cancel", NULL, event);
+      return FALSE;
     }
 
   if (event->type == KeyRelease &&
@@ -3048,6 +3090,27 @@ handle_cycle (MetaDisplay    *display,
                     backwards, FALSE);
 }
 
+static void
+handle_tab_popup_select (MetaDisplay    *display,
+                         MetaScreen     *screen,
+                         MetaWindow     *window,
+                         XEvent         *event,
+                         MetaKeyBinding *binding,
+                         gpointer        dummy)
+{
+  /* Stub for custom handlers; no default implementation */
+}
+
+static void
+handle_tab_popup_cancel (MetaDisplay    *display,
+                         MetaScreen     *screen,
+                         MetaWindow     *window,
+                         XEvent         *event,
+                         MetaKeyBinding *binding,
+                         gpointer        dummy)
+{
+  /* Stub for custom handlers; no default implementation */
+}
 
 static void
 handle_toggle_fullscreen  (MetaDisplay    *display,

src/include/all-keybindings.h

@@ -187,6 +187,18 @@ keybind (cycle_panels_backward,    handle_cycle,         META_TAB_LIST_DOCKS,
 
 /***********************************/
 
+/* These two are special pseudo-bindings that are provided for allowing
+ * custom handlers, but will never be bound to a key. While a tab
+ * grab is in effect, they are invoked for releasing the primary modifier
+ * or pressing some unbound key, respectively.
+ */
+keybind (tab_popup_select,        handle_tab_popup_select, 0, 0, NULL,
+         "Select window from tab popup")
+keybind (tab_popup_cancel,        handle_tab_popup_cancel, 0, 0, NULL,
+         "Cancel tab popup")
+
+/***********************************/
+     
 keybind (show_desktop, handle_show_desktop, 0, 0, "<Control><Alt>d",
       _("Hide all normal windows and set focus to the desktop"))
 keybind (panel_main_menu, handle_panel,