Add a browser test which checks if the 'Caps Lock by pressing both Shift keys' feature is working.

Add a browser test which checks if the 'Caps Lock by pressing both Shift keys' feature is working.

The new test should prevent a regression like crosbug.com/23067 in the future.

BUG=chromium-os:23067
TEST=ran the browser test


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112142

[Daniel Erat, 2011-11-29 16:39:49]

LGTM with some nits.  Thanks for adding this!

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
File chrome/browser/chromeos/system_key_event_listener_browsertest.cc (right):

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:15: #include
<X11/keysym.h>
nit: these should be above the chrome includes

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:27:
DCHECK(num_lock_mask_);
maybe make this a regular check?  dcheck seems a bit strange in a test

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:44: // Start
listening X events.
nit: Start listening _for_ X events

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:50: // Stop
listening X events.
listening for

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:109: // second
Shift key is pressed. Check if the behavior is intended.
yeah, i think it was -- i didn't want to prevent web pages from seeing the event

[Yusuke Sato, 2011-11-30 01:46:42]

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
File chrome/browser/chromeos/system_key_event_listener_browsertest.cc (right):

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:15: #include
<X11/keysym.h>
On 2011/11/29 16:39:49, Daniel Erat wrote:
> nit: these should be above the chrome includes

Moved X11/keysym.h. Let me keep the position of X11/Xlib.h since it #defines
lots of macros and they conflict with
chrome/test/base/in_process_browser_test.h.

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:27:
DCHECK(num_lock_mask_);
On 2011/11/29 16:39:49, Daniel Erat wrote:
> maybe make this a regular check?  dcheck seems a bit strange in a test

Done.

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:44: // Start
listening X events.
On 2011/11/29 16:39:49, Daniel Erat wrote:
> nit: Start listening _for_ X events

Done.

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:50: // Stop
listening X events.
On 2011/11/29 16:39:49, Daniel Erat wrote:
> listening for

Done.

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:109: // second
Shift key is pressed. Check if the behavior is intended.
On 2011/11/29 16:39:49, Daniel Erat wrote:
> yeah, i think it was -- i didn't want to prevent web pages from seeing the
event

Sure, removed the TODO.

[commit-bot: I haz the power, 2011-11-30 03:36:08]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusukes@chromium.org/8726028/8001

[commit-bot: I haz the power, 2011-11-30 06:24:36]

Change committed as 112142

[Daniel Erat, 2011-11-30 16:36:53]

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
File chrome/browser/chromeos/system_key_event_listener_browsertest.cc (right):

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:15: #include
<X11/keysym.h>
On 2011/11/30 01:46:42, Yusuke Sato wrote:
> On 2011/11/29 16:39:49, Daniel Erat wrote:
> > nit: these should be above the chrome includes
> 
> Moved X11/keysym.h. Let me keep the position of X11/Xlib.h since it #defines
> lots of macros and they conflict with
> chrome/test/base/in_process_browser_test.h.

Just #undef them, then?  That's what I've seen done in other places.

[Yusuke Sato, 2011-12-02 07:50:17]

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
File chrome/browser/chromeos/system_key_event_listener_browsertest.cc (right):

http://codereview.chromium.org/8726028/diff/1/chrome/browser/chromeos/system_...
chrome/browser/chromeos/system_key_event_listener_browsertest.cc:15: #include
<X11/keysym.h>
On 2011/11/30 16:36:53, Daniel Erat wrote:
> On 2011/11/30 01:46:42, Yusuke Sato wrote:
> > On 2011/11/29 16:39:49, Daniel Erat wrote:
> > > nit: these should be above the chrome includes
> > 
> > Moved X11/keysym.h. Let me keep the position of X11/Xlib.h since it #defines
> > lots of macros and they conflict with
> > chrome/test/base/in_process_browser_test.h.
> 
> Just #undef them, then?  That's what I've seen done in other places.

Done in r112616.