Adds ozone support for ime keyboard.

Adds ozone support for ime keyboard. This cl is doing these things:
1. Tidy up the structure of ime_keyboard.h/cc ime_keyboard_x11.h/cc ime_keyboard_ozone.h/cc
2. Implemented the interface - ImeKeyboard for ImeKeyboardOzone, leave placeholders where we need to call Ozone interfaces.

BUG=342336
TEST=Verified on linux_chromeos. Steps:
1. Switches among different keyboard layouts.
2. Checks the inputed characters is correctly transformed to the according keyboard layout.

Committed: https://crrev.com/6d0c1e11ec385ae495500839adee1e717334332f
Cr-Commit-Position: refs/heads/master@{#303033}

[FengYuan, 2014-11-01 09:09:15]

fengyuan@chromium.org changed reviewers:
+ shuchen@chromium.org

[FengYuan, 2014-11-03 19:18:11]

fengyuan@chromium.org changed reviewers:
+ satorux@chromium.org

[Shu Chen, 2014-11-04 06:17:08]

Please make sure the try-bot linux_chromium_chromeos_ozone_rel green.

https://codereview.chromium.org/693733002/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.h (right):

https://codereview.chromium.org/693733002/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.h:28: class
ImeKeyboardX11;
why add this?

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboard.h
File chromeos/ime/ime_keyboard.h (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard.h:117: std::string last_layout_;
caps_lock_is_enabled_ & last_layout_ should be protected members.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard.h:119: protected:
this should be private.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_ozone.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:37: return false;
why not return ImeKeyboard::CapsLockIsEnabled(); ?

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:45:
SetCapsLockEnabled(caps_lock_is_enabled_);
using SetCapsLockEnabled() to DisableNumLock() is the special trick for X11,
please leave this method empty.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:111: caps_lock_is_enabled_ = caps_lock_enabled;
replace this with SetCapsLockEnabled(caps_lock_enabled);

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:242: }
No need to duplicate code, just do:

SetLockedModifiers(enable_caps_lock);
ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);

[FengYuan, 2014-11-04 07:11:20]

PTAL

https://codereview.chromium.org/693733002/diff/60001/chrome/browser/chromeos/...
File chrome/browser/chromeos/input_method/input_method_manager_impl.h (right):

https://codereview.chromium.org/693733002/diff/60001/chrome/browser/chromeos/...
chrome/browser/chromeos/input_method/input_method_manager_impl.h:28: class
ImeKeyboardX11;
On 2014/11/04 06:17:08, Shu Chen wrote:
> why add this?

removed.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboard.h
File chromeos/ime/ime_keyboard.h (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard.h:117: std::string last_layout_;
On 2014/11/04 06:17:08, Shu Chen wrote:
> caps_lock_is_enabled_ & last_layout_ should be protected members.
> 

See :
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/chr...

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard.h:119: protected:
On 2014/11/04 06:17:08, Shu Chen wrote:
> this should be private.

Done.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_ozone.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:37: return false;
On 2014/11/04 06:17:08, Shu Chen wrote:
> why not return ImeKeyboard::CapsLockIsEnabled(); ?

It's a placeholder, we should call the interface in Ozone, and return the value
from that.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:45:
SetCapsLockEnabled(caps_lock_is_enabled_);
On 2014/11/04 06:17:08, Shu Chen wrote:
> using SetCapsLockEnabled() to DisableNumLock() is the special trick for X11,
> please leave this method empty.

Done.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:111: caps_lock_is_enabled_ = caps_lock_enabled;
On 2014/11/04 06:17:08, Shu Chen wrote:
> replace this with SetCapsLockEnabled(caps_lock_enabled);

nope, we can't. that will cause recursive problems, SetCapsLockEnabled will call
this method, and it calls that back again.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:242: }
On 2014/11/04 06:17:08, Shu Chen wrote:
> No need to duplicate code, just do:
> 
> SetLockedModifiers(enable_caps_lock);
> ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);

We can't, the order is important.

[Shu Chen, 2014-11-04 07:22:42]

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_ozone.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:37: return false;
On 2014/11/04 07:11:19, FengYuan wrote:
> On 2014/11/04 06:17:08, Shu Chen wrote:
> > why not return ImeKeyboard::CapsLockIsEnabled(); ?
> 
> It's a placeholder, we should call the interface in Ozone, and return the
value
> from that.

Set/Get inconsistency would potentially break capslock indicator feature with
use_ozone=1.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:242: }
On 2014/11/04 07:11:19, FengYuan wrote:
> On 2014/11/04 06:17:08, Shu Chen wrote:
> > No need to duplicate code, just do:
> > 
> > SetLockedModifiers(enable_caps_lock);
> > ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
> 
> We can't, the order is important.

How about:

ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
SetLockedModifiers();

SetLockedModifiers method actually doesn't require the parameter.
It can always use caps_lock_is_enabled_.

[FengYuan, 2014-11-04 07:29:00]

PTAL

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_ozone.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_ozone.cc:37: return false;
On 2014/11/04 07:22:42, Shu Chen wrote:
> On 2014/11/04 07:11:19, FengYuan wrote:
> > On 2014/11/04 06:17:08, Shu Chen wrote:
> > > why not return ImeKeyboard::CapsLockIsEnabled(); ?
> > 
> > It's a placeholder, we should call the interface in Ozone, and return the
> value
> > from that.
> 
> Set/Get inconsistency would potentially break capslock indicator feature with
> use_ozone=1.

Done.

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/60001/chromeos/ime/ime_keyboar...
chromeos/ime/ime_keyboard_x11.cc:242: }
On 2014/11/04 07:22:42, Shu Chen wrote:
> On 2014/11/04 07:11:19, FengYuan wrote:
> > On 2014/11/04 06:17:08, Shu Chen wrote:
> > > No need to duplicate code, just do:
> > > 
> > > SetLockedModifiers(enable_caps_lock);
> > > ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
> > 
> > We can't, the order is important.
> 
> How about:
> 
> ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
> SetLockedModifiers();
> 
> SetLockedModifiers method actually doesn't require the parameter.
> It can always use caps_lock_is_enabled_.

Done.

[FengYuan, 2014-11-04 07:29:01]

PTAL

[Shu Chen, 2014-11-04 07:33:38]

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:110: value_mask |= (caps_lock_enabled ?
LockMask : 0);
s/caps_lock_enabled/caps_lock_is_enabled_

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:237:
ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
SetLockedModifiers();

[FengYuan, 2014-11-04 07:42:19]

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
File chromeos/ime/ime_keyboard_x11.cc (right):

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:110: value_mask |= (caps_lock_enabled ?
LockMask : 0);
On 2014/11/04 07:33:38, Shu Chen wrote:
> s/caps_lock_enabled/caps_lock_is_enabled_

Done.

https://codereview.chromium.org/693733002/diff/100001/chromeos/ime/ime_keyboa...
chromeos/ime/ime_keyboard_x11.cc:237:
ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
On 2014/11/04 07:33:38, Shu Chen wrote:
> ImeKeyboard::SetCapsLockEnabled(enable_caps_lock);
> SetLockedModifiers();

Done.

[Shu Chen, 2014-11-04 07:45:19]

lgtm

[satorux1, 2014-11-06 00:54:06]

Looked at the build files.

https://codereview.chromium.org/693733002/diff/120001/chromeos/BUILD.gn
File chromeos/BUILD.gn (right):

https://codereview.chromium.org/693733002/diff/120001/chromeos/BUILD.gn#newco...
chromeos/BUILD.gn:46: "ime/ime_keyboard_ozone.h"]
move ] to the next line? also put a comma at the end of the last item?

https://codereview.chromium.org/693733002/diff/120001/chromeos/chromeos.gyp
File chromeos/chromeos.gyp (right):

https://codereview.chromium.org/693733002/diff/120001/chromeos/chromeos.gyp#n...
chromeos/chromeos.gyp:498: 'ime/ime_keyboard.h'
keep the trailing comma?

[FengYuan, 2014-11-06 04:48:35]

PTAL :)

https://codereview.chromium.org/693733002/diff/120001/chromeos/BUILD.gn
File chromeos/BUILD.gn (right):

https://codereview.chromium.org/693733002/diff/120001/chromeos/BUILD.gn#newco...
chromeos/BUILD.gn:46: "ime/ime_keyboard_ozone.h"]
On 2014/11/06 00:54:06, satorux1 wrote:
> move ] to the next line? also put a comma at the end of the last item?

Done.

https://codereview.chromium.org/693733002/diff/120001/chromeos/chromeos.gyp
File chromeos/chromeos.gyp (right):

https://codereview.chromium.org/693733002/diff/120001/chromeos/chromeos.gyp#n...
chromeos/chromeos.gyp:498: 'ime/ime_keyboard.h'
On 2014/11/06 00:54:06, satorux1 wrote:
> keep the trailing comma?

Done.

[satorux1, 2014-11-06 04:50:15]

build files lgtm

[satorux1, 2014-11-06 05:05:51]

The patch description is too sparse for a patch of this size. Please describe
what this patch is for and how it is achieved, etc.

[satorux1, 2014-11-06 05:06:10]

also add a TEST= line to describe how to test this patch.

[FengYuan, 2014-11-06 15:29:17]

The CQ bit was checked by fengyuan@chromium.org

[commit-bot: I haz the power, 2014-11-06 15:30:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/693733002/160001

[commit-bot: I haz the power, 2014-11-06 16:15:35]

Committed patchset #9 (id:160001)

[commit-bot: I haz the power, 2014-11-06 16:16:28]

Patchset 9 (id:??) landed as
https://crrev.com/6d0c1e11ec385ae495500839adee1e717334332f
Cr-Commit-Position: refs/heads/master@{#303033}