A workaround to close currently opened menu. Send escape key if screen locker fails to grab input.

chrome/browser/chromeos/login/screen_locker.cc

 // Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/login/screen_locker.h"
 
 #include <string>
 #include <vector>
+#include <X11/extensions/XTest.h>
+#include <X11/keysym.h>
 
 #include "app/l10n_util.h"
+#include "app/x11_util.h"

[Daniel Erat, 2010/11/16 15:38:13]

nit: sort

[oshima, 2010/11/16 21:06:29]

Done.

 #include "app/resource_bundle.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/message_loop.h"
 #include "base/singleton.h"
 #include "base/string_util.h"
 #include "base/timer.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_thread.h"
@@ skipping 250 matching lines @@
   }
 
   virtual void Show() {
     views::WidgetGtk::Show();
     // Now steal all inputs.
     TryGrabAllInputs();
   }
 
   void ClearGrab() {
     GtkWidget* current_grab_window;
-    // Grab gtk input first so that the menu holding grab will close itself.
+    // Grab gtk input first so that the menu holding gtk grab will
+    // close itself.
     gtk_grab_add(window_contents());
 
+    if (mouse_grab_status_ != GDK_GRAB_INVALID_TIME ||

[Daniel Erat, 2010/11/16 17:26:50]

This is wrong: if _only_ the pointer is grabbed, you'll end up sending an Escape
keypress to some unrelated client that has the input focus.  The
only-pointer-is-grabbed state is actually easy to get into; I think that it
happens whenever you hold the mouse button down (an implicit pointer grab gets
added in this case).  I think that the only-keyboard-is-grabbed state is
unlikely to happen (see e.g. the end of
http://mail.gnome.org/archives/gtk-devel-list/1999-January/msg00158.html).

One thing that's a bit weird is that this method will try to send the key events
even if we were successful in grabbing the pointer and keyboard earlier --
you're testing != GDK_GRAB_INVALID_TIME, which will also match the success code.
 This might not be a big deal since I don't think it gets called again after
we're successful, but it'd probably be safer to check ==
GDK_GRAB_ALREADY_GRABBED.

Also, there's a race here, in that the client could've released its grab between
the point where we failed to get the grab and here.  You could use something
like this instead:

  gdk_x11_grab_server();
  if (gdk_pointer_is_grabbed()) {
    // use XTEST to send events
  }
  gdk_x11_ungrab_server();

There's no gdk_keyboard_is_grabbed() method, unfortunately.  You could probably
just attempt to grab the keyboard and see if you're successful or not to test if
it's already grabbed.

[oshima, 2010/11/16 21:06:29]

BTW, this *was* how view menu was implemented. Only pointer grab, but not
keyboard grab and it
looked working fine on desktop (by accident i think).

  The
  I think that the only-keyboard-is-grabbed state is
Hmm, I'm not sure if I'm convinced.
1) I thought grabbing key without grabbing pointer happens when wm grab keyboard
to process
   modifier keys? Sorry if I'm mistaken.
2) If a client can do that, the screen locker should deal with it. It should not
misbehave
  because there is bad client.

But I agree we should not send escape if keyboard is not grabbed.
Yes, if both are successful, this method will not be called, and there is
GDK_GRAB_FROZEN,
so I thought this is safer. In any case, the condition has been updated to send
escape only when
keyboard grab failed.
Can you elaborate a bit? Are you suggesting that gdk_xxxx_ungrab may be called
in between?

+        kbd_grab_status_ != GDK_GRAB_INVALID_TIME) {
+      // Send escape key if the input is grabbed by other client.
+      // This happen when a plugin has a menu opened and sending

[Daniel Erat, 2010/11/16 15:38:13]

s/happen/happens/

[oshima, 2010/11/16 21:06:29]

Done.

+      // escape key should close the menu.
+      Display* display = x11_util::GetXDisplay();
+      KeyCode escape = XKeysymToKeycode(display, XK_Escape);  // escape
+      int event_base, error_base;
+      int major, minor;
+      // Make sure we have XTest extension.
+      DCHECK(XTestQueryExtension(display, &event_base, &error_base,
+                                 &major, &minor));
+
+      XTestGrabControl(display, True);

[Daniel Erat, 2010/11/16 15:38:13]

I don't think that you need this.  It lets you sidestep server grabs (not
keyboard or pointer grabs), which you probably don't need to do -- server grabs
are uncommon and usually brief, and if the server was grabbed, you'll still
block on your next X request after XTestGrabControl(display, False).

[oshima, 2010/11/16 21:06:29]

Done.

+      XTestFakeKeyEvent(display, escape, True, 0);
+      XTestFakeKeyEvent(display, escape, False, 0);
+      XSync(display, False);

[Daniel Erat, 2010/11/16 15:38:13]

This can just be XFlush(display) instead.

[oshima, 2010/11/16 21:06:29]

Done.

+      XTestGrabControl(display, False);
+    }
+
     // Make sure there is no grab widget so that gtk simply propagates
     // an event.  This is necessary to allow message bubble and password
     // field, button to process events simultaneously. GTK
     // maintains grab widgets in a linked-list, so we need to remove
     // until it's empty.
     while ((current_grab_window = gtk_grab_get_current()) != NULL)

[Daniel Erat, 2010/11/16 15:38:13]

Should this come before the XTest code?  I guess that none of the GTK grab stuff
matters as much after the first time that this method is called (since we'll
have released the GTK grab the first time through).

[oshima, 2010/11/16 21:06:29]

I believe the order isn't important and it was arbitrary. Moved.

       gtk_grab_remove(current_grab_window);
   }
 
   virtual gboolean OnButtonPress(GtkWidget* widget, GdkEventButton* event) {
     WidgetGtk::OnButtonPress(widget, event);
     // Never propagate event to parent.
     return true;
   }
 
   // Try to grab all inputs. It initiates another try if it fails to
@@ skipping 621 matching lines @@
 
 bool ScreenLocker::AcceleratorPressed(const views::Accelerator& accelerator) {
   if (!background_view_->IsScreenSaverVisible()) {
     StartScreenSaver();
     return true;
   }
   return false;
 }
 
 }  // namespace chromeos