Allow Caps Lock to be remapped [part 2 of 2]

Allow Caps Lock to be remapped [part 2 of 2]. 

* Add a new Chrome command line flag: --has-chromeos-keyboard
* Show the new UI for remapping Caps Lock if --has-chromeos-keyboard is not specified.

This CL depends on http://codereview.chromium.org/11314020/ and https://gerrit.chromium.org/gerrit/#/c/36884/.

BUG=146204
TEST=manually confirmed that 1) the new UI is shown when /etc/session_manager_use_flags.txt does not have "chromeos_keyboard", is not shown when the file has the flag, 2) the UI for guest mode is the same as non-guest mode, 3) the UI correctly remaps the Caps Lock key.

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

[Daniel Erat, 2012-10-30 18:23:17]

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/keyboard_overlay.js (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/keyboard_overlay.js:32: //Forward
public APIs to private implementations.
nit: add space after '//'

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/keyboard_handler.cc (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/keyboard_handler.cc:91:
switches::kHasChromeOSKeyboard)) {
nit: don't think you need curly braces here (at least, i've received feedback to
omit them in this case)

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:1465: // If true, the Chrome OS keyboard (with
Search but without Caps Lock, Page Up,
i'm not sure that the "without ... page up, page down" part is correct.  the
keyboard connected to my stumpy has a search key instead of caps lock, but it
also has a numeric keypad with page up/down, home, end, and delete.

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:405: extern const char kHasChromeOSKeyboard[];
nit: keep this list alphabetized

[Yusuke Sato, 2012-10-30 19:27:23]

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/resources/op...
File chrome/browser/resources/options/chromeos/keyboard_overlay.js (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/resources/op...
chrome/browser/resources/options/chromeos/keyboard_overlay.js:32: //Forward
public APIs to private implementations.
On 2012/10/30 18:23:17, Daniel Erat wrote:
> nit: add space after '//'

Done.

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/chromeos/keyboard_handler.cc (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/chromeos/keyboard_handler.cc:91:
switches::kHasChromeOSKeyboard)) {
On 2012/10/30 18:23:17, Daniel Erat wrote:
> nit: don't think you need curly braces here (at least, i've received feedback
to
> omit them in this case)

Done.

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.cc (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
chrome/common/chrome_switches.cc:1465: // If true, the Chrome OS keyboard (with
Search but without Caps Lock, Page Up,
Good point. Updated the comment.

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (right):

http://codereview.chromium.org/11346028/diff/4001/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:405: extern const char kHasChromeOSKeyboard[];
On 2012/10/30 18:23:17, Daniel Erat wrote:
> nit: keep this list alphabetized

Done.

[Daniel Erat, 2012-10-30 19:56:16]

lgtm

[Yusuke Sato, 2012-10-30 20:06:30]

+ben for chrome/ owners review.
Could you have a look at this too?

[Ben Goodger (Google), 2012-10-31 16:08:07]

lgtm

[Yusuke Sato, 2012-11-01 18:41:26]

Daniel, sorry about that but could you take a look event_rewriter.cc and
event_rewriter_unittests.cc again? Other files are unchanged (only rebased).


Before I submitted code, I added some tests to event_rewriter_unittests.cc and
found one issue: When an external keyboard is connected to a Chromebook with
Chrome OS keyboard, the Caps Lock key on the external keyboard does not work
correctly. Patch Set #3 should fix the issue and have tests for that case.

In addition to git try, I manually tested the code as follows, just in case:

With the new Chrome flag confirmed that:

- On Login screen Caps Lock works as Caps Lock.
- After signing in, Caps Lock works as Caps Lock (even if the new pref is set to
Ctrl etc.)
- When Neo2 is selected, Caps Lock is XK_ISO_Level3_Shift + Mod3Mask.

Without the flag:

- Login screen: Caps Lock works as Caps Lock.
- After signing in:  Caps Lock can be remapped via the new UI.
- Neo2 is selected:  Caps Lock is XK_ISO_Level3_Shift + Mod3Mask.

Also manually confirmed that:
- Ctrl/Alt/Search remapping features are still working as expected.
- Command key (on Apple keyboard) remapping feature is still working as
expected.

Thanks,
Yusuke

[Daniel Erat, 2012-11-01 19:13:06]

lgtm

http://codereview.chromium.org/11346028/diff/23001/chrome/browser/ui/ash/even...
File chrome/browser/ui/ash/event_rewriter.cc (right):

http://codereview.chromium.org/11346028/diff/23001/chrome/browser/ui/ash/even...
chrome/browser/ui/ash/event_rewriter.cc:123: bool IsMod3InUse() {
nit: IsMod3UsedByCurrentInputMethod() may be a bit clearer

[Yusuke Sato, 2012-11-01 19:25:27]

http://codereview.chromium.org/11346028/diff/23001/chrome/browser/ui/ash/even...
File chrome/browser/ui/ash/event_rewriter.cc (right):

http://codereview.chromium.org/11346028/diff/23001/chrome/browser/ui/ash/even...
chrome/browser/ui/ash/event_rewriter.cc:123: bool IsMod3InUse() {
On 2012/11/01 19:13:06, Daniel Erat wrote:
> nit: IsMod3UsedByCurrentInputMethod() may be a bit clearer

Thanks, done.

[commit-bot: I haz the power, 2012-11-01 19:25:45]

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

[commit-bot: I haz the power, 2012-11-01 21:26:38]

Change committed as 165503